Introduce initial ZSA support in Zebra (draft PR for CI checks) #8872
Conversation
…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.
…py V5 behaviour for now)
…(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.
…m/QED-it/zebra into switch-to-zsa-crates-nu6-txv6-gen
github-actions
bot
added
the
C-trivial
dmidem
requested review from
arya2 and
upbqdn
and removed request for
a team
… with the upstream halo2/librustcash/orchard/sapling versions
|
It's not possible to fix all of the complaints from cargo-deny until the changes in librustzcash have been published, and not urgent to fix any of them, but see Continuous Integration in the Zebra book for how to fix duplicate dependencies in the deny.toml checks for later. Note: at least the Building @gustavovalverde Is there anything else we could try to get this one working? |
|
I think probably it's good enough to see where it's failing now and see if there are any actionable failures on the QEDIT side, e.g maybe https://github.com/ZcashFoundation/zebra/actions/runs/11015467979/job/30589574007?pr=8872 |
…-zsa-crates-nu6-txv6-gen
…witch-to-zsa-crates-nu6-txv6-gen
| // FIXME: is this a correct calculation way? | ||
| // The longest Vec<BurnItem> we receive from an honest peer must fit inside a valid block. | ||
| // Since encoding the length of the vec takes at least one byte, we use MAX_BLOCK_BYTES - 1 | ||
| (MAX_BLOCK_BYTES - 1) / BURN_ITEM_SIZE |
There was a problem hiding this comment.
👍 This looks perfect, it just needs to be higher than the protocol limit without being unreasonably large, and this should be just ~0.1% more than the protocol limit.
| // FIXME: is it possible to convert V6 shielded data to V5? | ||
| // FIXME: add another function for V6, like transaction_to_fake_v6? |
There was a problem hiding this comment.
There's probably no need to convert v6 transactions to v5, it won't be used until test blocks with v6 transactions have been added, which may never happen. A function for converting to fake v6 transactions may be useful for testing or might not be necessary either.
|
|
||
| // Denoted as `nExpiryHeight` in the spec. | ||
| writer.write_u32::<LittleEndian>(expiry_height.0)?; | ||
|
|
There was a problem hiding this comment.
Nitpick: ZIP 230 is still a draft, but, if the explicit fee ends up being included it should be added here and as a field on the Transaction::V6 variant.
Everything else here looks good.
| writer.write_u8(self.is_finalized().as_u8())?; | ||
| self.notes().zcash_serialize(&mut writer)?; | ||
| self.asset_desc().zcash_serialize(&mut writer)?; |
There was a problem hiding this comment.
These look like they're in the opposite order as they're listed in ZIP 230, but otherwise looks good, as does the serialization for IssueNote.
| const NOTE_VALUE_SIZE: u64 = 4; | ||
| const RANDOM_SEED_SIZE: u64 = 32; | ||
| // FIXME: is this a correct way to calculate (simple sum of sizes of components)? | ||
| const NOTE_SIZE: u64 = |
There was a problem hiding this comment.
It says 147 here but these add up to 143, this or the ZIP may need an update (probably this implementation, using a u64 for note value seems good).
|
|
||
| impl TrustedPreallocate for Note { | ||
| fn max_allocation() -> u64 { | ||
| // FIXME: is this a correct calculation way? |
| // FIXME: impl correct calculation | ||
| 10 |
There was a problem hiding this comment.
This looks good, the function that calls max_allocation() (zcash_deserialize_external_count()) will deserialize and pre-allocate sequentially based on the actual counts, so until it goes through each item and reads their CompactSize, it'll just be pre-allocating memory for the pointers, if one of them is shorter than expected it'll return an error, and their combined real size can't be longer than the Codec's builder.max_len field, which is MAX_PROTOCOL_MESSAGE_LEN.
| // FIXME: impl correct calculation | |
| 10 | |
| (MAX_BLOCK_BYTES - 1) / NOTE_SIZE |
This draft PR introduces initial support for Zcash Shielded Assets (ZSA) in the Zebra codebase. The primary goal of this PR is to run CI checks provided by the ZcashFoundation repository to verify compilation correctness and execute unit and integration tests.