Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZSA integration (cumulative steps 1–7): up to Orchard ZSA consensus modifications #37

Open
wants to merge 127 commits into
base: zsa1
Choose a base branch
from

Conversation

dmidem
Copy link

@dmidem dmidem commented Jan 5, 2025

This pull request merges the changes from several incremental PRs into one cumulative set of updates on top of the zsa1 branch. It introduces ZSA-compatible crates, Network Upgrade 7 (Nu7), initial Transaction V6 support, Orchard generics for ZSA, property-based testing enhancements, note commitment handling for ZSA issuance, and initial consensus modifications for Orchard ZSA.

It does not include the final state management changes or additional tests — those come in subsequent PRs.

Below is a high-level overview of the merged changes:

  1. ZSA-Compatible Crates Integration (Step 1)
    ZSA integration (step 1): Integrate ZSA-compatible crates into Zebra while maintaining original Orchard (Vanilla) support for now [to upstream] #24:

    • Replaces core libraries (halo2, zcash_note_encryption, etc.) with QED-it’s ZSA-compatible forks.
    • Maintains full support for Orchard “Vanilla” without activating any new ZSA features yet.
  2. Network Upgrade 7 (Nu7) Support (Step 2)
    ZSA integration (step 2): Add Network Upgrade 7 (Nu7) support to Zebra #15

    • Introduces code paths and placeholders for Nu7, the upcoming network upgrade required for ZSA.
    • Adds FIXME comments where final activation heights and other specifics must be filled in.
  3. Transaction V6 Foundations (Step 3)
    ZSA integration (step 3): Add initial Transaction V6 support to Zebra (currently copies V5 behavior) #16

    • Adds a V6 variant to Zebra’s Transaction enum, initially mirroring V5 logic.
    • Sets a baseline for future ZSA-related modifications, including placeholders for relevant fields and logic.
  4. Refactor Orchard Structures to Generics (Step 4)
    ZSA integration (step 4): Refactor Orchard structures to generics and add Orchard ZSA support for Transaction V6 #17

    • Converts key Orchard data structures (e.g., ShieldedData) to generics, enabling a single code path for both Orchard Vanilla and Orchard ZSA.
    • Implements serialization/deserialization for V6 transactions, including a burn field in the ZSA flavor.
  5. Orchard Proptests with ZSA Enhancements (Step 5)
    ZSA integration (step 5): Modify Orchard proptest implementations to support ZSA #18

    • Extends the property-based testing framework to handle ZSA-specific fields and behaviors in Transaction V6.
    • Refactors code organization (e.g., extracting Burn types) for better clarity and future expansion.
  6. Integration of ZSA Issuance Commitments (Step 6)
    ZSA integration (step 6): Integrate Orchard ZSA Issuance note commitments with Shielded Data action commitments for V6 transactions #25

    • Merges issuance action note commitments with existing shielded data commitments for V6 transactions.
    • Ensures Transaction::orchard_note_commitments includes issuance note commitments when present, preserving V5 behavior.
  7. Initial ZSA Consensus Support (Step 7)
    ZSA integration (step 7): Modify zebra-consensus to support Orchard ZSA #28

    • Modifies zebra-consensus to support Orchard ZSA.

Next Steps

  • State Management & Additional Testing: Future PRs will introduce state-layer modifications, refine consensus checks, and add more comprehensive tests.

By consolidating these first several steps into a single PR, we aim to simplify the review process .

dmidem added 30 commits July 29, 2024 10:13
…l Orchard support only, without supporting and enabling ZSA features.
This commit introduces basic support for Transaction version 6 (Tx V6). This initial implementation treats Tx V6
as a simple copy of Tx V5, without yet integrating ZSA-specific features or the new transaction structure.

- Added a new V6 variant to the Transaction enum in the zebra-chain crate.
- Updated relevant code to handle the new V6 variant.

Note: Tests and additional adjustments are still pending, and will be addressed in subsequent commits.
…(without unit tests fixing for now).

- Refactored `ShieldedData` and `Action` structures to be generics parameterized by Orchard flavor
  (`OrchardVanilla` or `OrchardZSA`), enabling support for both Orchard protocols in Tx V6.
- Introduced a `burn` field in `ShieldedData` to support ZSA, with unit type for Tx V5 and a vector of burn items for Tx V6.
- Modified `Transaction` enum methods (orchard_...) to handle generics properly, ensuring compatibility with both Orchard flavors.
- Implemented serialization and deserialization for Tx V6 while avoiding code redundancy with Tx V5 wherever possible.
… now - the same as used in librustzcash to make it possible to run tests)
…add issuance_block test function there (now it simply deserialized issuance block from the test vector to check if deserialization functions work properly)
…dd checks of deserialization of transfer and burn blocks
…'re two keys now - VERIFYING_KEY_VANILLA and VERIFYING_KEY_ZSA
… use 77777777 as consensus branch id for Nu7 (to ajdust it with ones used in librustzcash)
…ract ValueCommitment of burn items if they are present
@dmidem
Copy link
Author

dmidem commented Jan 11, 2025

Unresolved review comments from previous PRs:

From Step 1:

#24 (comment)
#24 (comment)
#24 (comment)
#24 (comment)
#24 (comment)
#24 (comment)

#24 (comment) and #24 (comment) - now
all #[cfg(zcash_unstable = "nu6")] are commeted out in the code now as nu6 was stabilized - should we uncomment them back (all or some?) and use nu7 instead?

#24 (comment) - I used the existing style of short names used in this file, see other map_authorization functions above in this file.

#24 (comment)

From Step 2:

#15 (comment) (#15 (comment))
#15 (comment)
#15 (comment)
#15 (comment)
#15 (comment)
#15 (comment)
#15 (comment)

From Step 3:

#16 (comment) - changed to 0x124A69F8 now.
#16 (comment)
#16 (comment)
#16 (comment) (#16 (comment))
#16 (comment)
#16 (comment)
#16 (comment)
#16 (comment)
#16 (comment) ?
#16 (comment)
#16 (comment)
#16 (comment)
#16 (comment)
#16 (comment)
#16 (comment)

From Step 4:

#17 (review) - now serlialization functions from librustzcash are used for issuance (as both zebra and librustzcash use IssueBundle struct from the orchard), for the bundle/shielded data - we have to use separate serialization functions as zebra uses its own types.

Need to properly handle feature flags. Currently, it is an unclear why 2 different flags are used.
?

#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment) (#17 (comment)) - looks like the existing code is correct.
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment) (#17 (comment))
#17 (comment)
#17 (comment) (#17 (comment)) - remove FIXME.
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment) - ask Vivek.
#17 (comment)
#17 (comment)
#17 (comment) - remove FIXME.
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment) (#17 (comment)) - remove FIXME.
#17 (comment)
#17 (comment)
#17 (comment) - need to make consts pub in orchard before.
#17 (comment) - need to make consts pub in orchard before.
#17 (comment) - need to make consts pub in orchard before.
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment) (#17 (comment))
#17 (comment)
#17 (comment) (#17 (comment))
#17 (comment)
#17 (comment)
#17 (comment)
#17 (comment) (#17 (comment), #17 (comment)) ?
#17 (comment) (#17 (comment))
#17 (comment)
#17 (comment) (#17 (comment)) - see a comment there.
#17 (comment) (#17 (comment)) ?
#17 (comment) ?

#17 (comment)
#17 (comment)
#17 (comment) (#17 (comment), #17 (comment)) - see a comment there.
#17 (comment) ?
#17 (comment) - add tests for OrchardZSA.
#17 (comment) - see a comment there.
#17 (comment) ?
#17 (comment) - add tests for OrchardZSA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant