Skip to content

ZSA integration (step 4): Refactor Orchard structures to generics and add Orchard ZSA support for Transaction V6#17

Closed
dmidem wants to merge 36 commits into
zsa-integration-txv6from
zsa-integration-generics
Closed

ZSA integration (step 4): Refactor Orchard structures to generics and add Orchard ZSA support for Transaction V6#17
dmidem wants to merge 36 commits into
zsa-integration-txv6from
zsa-integration-generics

Conversation

@dmidem
Copy link
Copy Markdown
Collaborator

@dmidem dmidem commented Oct 17, 2024

This PR refactors the Orchard-related structures to be generics and adds support for both Orchard Vanilla and Orchard ZSA in Zebra's Transaction V6. This change allows the codebase to handle both versions of the Orchard protocol and implements necessary serialization/deserialization logic.

Key changes

  • Generic Orchard Structures:
    • Refactored ShieldedData and Action structures to be generic, parameterized by Orchard flavor (OrchardVanilla or OrchardZSA), enabling support for both protocols in Tx V6.
  • burn Field in ShieldedData:
    • Added a burn field to ShieldedData to support ZSA: a unit type for Tx V5 and a vector of burn items for Tx V6.
  • Updates to Transaction Enum Methods:
    • Modified methods in the Transaction enum to handle the new generic structures properly and support both Orchard flavors.
  • Serialization and Deserialization for Tx V6:
    • Implemented serialization and deserialization for Transaction V6, ensuring the new structures are correctly processed while avoiding code redundancy with Tx V5.

…(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.
… with the upstream halo2/librustcash/orchard/sapling versions
@dmidem dmidem requested a review from PaulLaux October 17, 2024 09:11
…t_desc to convert slice to vec, as slices require implementation of the serialization from scratch
@dmidem dmidem changed the title Step 4: Refactor Orchard structures to generics and add Orchard ZSA support for Transaction V6 ZSA integration, step 4: Refactor Orchard structures to generics and add Orchard ZSA support for Transaction V6 Oct 18, 2024
@dmidem dmidem changed the title ZSA integration, step 4: Refactor Orchard structures to generics and add Orchard ZSA support for Transaction V6 ZSA integration (step 4): Refactor Orchard structures to generics and add Orchard ZSA support for Transaction V6 Oct 18, 2024
@PaulLaux PaulLaux requested a review from arya2 October 31, 2024 15:08
Copy link
Copy Markdown

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments.
In addition,

  • for Serialize / De- Serialize let's use the functions from librustzcash instead of duplicating them. Especially for Issuance (a completely new component in Zebra.)
  • Need to properly handle feature flags. Currently, it is an unclear why 2 different flags are used.

Comment thread .github/workflows/ci-basic.yml Outdated
RUSTFLAGS: '--cfg zcash_unstable="nu6"'
RUSTDOCFLAGS: '--cfg zcash_unstable="nu6"'
#RUSTFLAGS: '--cfg zcash_unstable="nu6"'
#RUSTDOCFLAGS: '--cfg zcash_unstable="nu6"'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why comment out?

Copy link
Copy Markdown
Collaborator Author

@dmidem dmidem Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because now they are defined in .cargo/config.toml - it's a better way as it enables them for local builds as well, not for CI only. But I agree that we need to remove commented lines from CI config.
Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed - removed the commented lines.


pub(crate) use shielded_data::ActionCommon;

#[cfg(feature = "tx-v6")]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not #[cfg(zcash_unstable = "nu6")]
we don't need them both, need to decide on one.

Copy link
Copy Markdown
Collaborator

@arya2 arya2 Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like tx-v6 better because I think we just need the feature to ensure that v6 transactions can't be (de)serialized by versions of Zebra that aren't ready to deploy NU7, but either seems fine.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left tx-v6.

/// the transactions `V5` and `V6`.
pub trait OrchardFlavorExt: Clone + Debug {
/// A type representing an encrypted note for this protocol version.
/// A type representing an encrypted note for this protocol version.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicated line

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the duplicated line.

@@ -0,0 +1,84 @@
//! This module defines traits and structures for supporting the Orchard Shielded Protocol
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this file? why not use orchard:: orchard_flavor directly and impl the missing parts?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(needs further discussion !!!)

Comment thread zebra-chain/src/orchard/action.rs Outdated
// 580 bytes in https://zips.z.cash/protocol/protocol.pdf#outputencodingandconsensus
// See [`note::EncryptedNote::zcash_deserialize`].
enc_ciphertext: note::EncryptedNote::zcash_deserialize(&mut reader)?,
// FIXME: don't mention about 580 here as this should work for OrchardZSA too?
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, can remove comment.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

let note_value = 10;

let shielded_data: Vec<zebra_chain::orchard::ShieldedData> = (1..=4)
let shielded_data: Vec<zebra_chain::orchard::ShieldedData<OrchardVanilla>> = (1..=4)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let shielded_data: Vec<ShieldedData<OrchardVanilla>> = (1..=4)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subjective: I think orchard::ShieldedData would be slightly more readable, though every option seems fine.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed zebra_chain::orchard::ShieldedData to ShieldedData everywhere in this file (did't use orchard::ShieldedData because orchard crate is also used in this file, so it might be confusing.

.unwrap();

zebra_chain::orchard::ShieldedData {
zebra_chain::orchard::ShieldedData::<OrchardVanilla> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ShieldedData::<OrchardVanilla> {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed (see also #17 (comment)).

// FIXME: consider more "type safe" way to do the following conversion
// (now it goes through &[u8])
enc_ciphertext: <[u8; 580]>::try_from(
enc_ciphertext: <[u8; OrchardVanilla::ENCRYPTED_NOTE_SIZE]>::try_from(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type is already known
enc_ciphertext: a.encrypted_note().enc_ciphertext.as_ref().try_into().unwrap().into(),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already changed to enc_ciphertext: a.encrypted_note().enc_ciphertext.0.into(), see #24 (comment))

let maybe_shielded_data: Option<zebra_chain::orchard::ShieldedData<OrchardVanilla>> =
bytes
.zcash_deserialize_into()
.expect("a valid orchard::ShieldedData instance");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let shielded_data = zebra_test::vectors::ORCHARD_SHIELDED_DATA
    .clone()
    .iter()
    .map(|bytes| {
        bytes
            .zcash_deserialize_into::<ShieldedData<OrchardVanilla>>()
            .expect("a valid orchard::ShieldedData instance")
    })
    .collect();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should deserialize to None if there are no actions for OrchardVanilla (see comment on ZcashDeserialize impl about burns).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

@dmidem dmidem Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, there's no deserialization function into ShieldedData<...> directly, there's one only to Option<ShieldedData<...>>, see this comment: https://github.com/QED-it/zebra/blob/zsa-integration-generics/zebra-chain/src/transaction/serialize.rs#L390

let maybe_shielded_data: Option<zebra_chain::orchard::ShieldedData<OrchardVanilla>> =
bytes
.zcash_deserialize_into()
.expect("a valid orchard::ShieldedData instance");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator

@arya2 arya2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partial initial review, looking great so far!

// TODO: use const generics https://github.com/ZcashFoundation/zebra/issues/2042

impl Copy for EncryptedNote {}
impl<const N: usize> Copy for EncryptedNote<N> {}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: The Copy, Clone, and Debug traits here can be derived now.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines +48 to +49
/// A structure representing a tag for Orchard protocol variant used for the transaction version `V6`
/// (which ZSA features support).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick/Optional:

Suggested change
/// A structure representing a tag for Orchard protocol variant used for the transaction version `V6`
/// (which ZSA features support).
/// A structure representing a tag for Orchard protocol variant used for the transaction version 6
/// (with support for ZSAs).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.


impl ZcashSerialize for NoBurn {
fn zcash_serialize<W: io::Write>(&self, mut _writer: W) -> Result<(), io::Error> {
Ok(())
Copy link
Copy Markdown
Collaborator

@arya2 arya2 Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, anytime there's a Vec being serialized, there's a CompactSize first, so it does need to insert a 0. Serializing an empty Vec should work too. Or zcash_serialize_empty_list().

Update:

There should be a 0 there for version 6 transactions and nothing for version 5 transactions, the code is correct as-is.


use super::OrchardFlavorExt;

#[cfg(not(feature = "tx-v6"))]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need OrchardVanilla for v5 transactions

Suggested change
#[cfg(not(feature = "tx-v6"))]

Copy link
Copy Markdown
Collaborator Author

@dmidem dmidem Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean we need to use OrchardVanilla for both (tx-vx and non-tx-v6) cases in max_allocation as mention in #17 (comment) and #17 (comment)?

Because that's the only place where OrchardVanilla is used in this file.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// The following expression doesn't work for generics, so a workaround with _ACTION_MAX_ALLOCATION_OK in
// AuthorizedAction impl is used instead:
// static_assertions::const_assert!(AuthorizedAction::<V>::ACTION_MAX_ALLOCATION < (1 << 16));
AuthorizedAction::<V>::ACTION_MAX_ALLOCATION
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size_of() might work here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(needs further discussion !!!)

let result = Action::<OrchardVanilla>::max_allocation();

// TODO: FIXME: Check this: V6 is used as it provides the max size of the action.
// So it's used even for V5 - is this correct?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to use the smaller one. It's okay if the max allocation is a little too large.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add copies of the updated tests for OrchardZSA as well?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add them in this PR?

(needs further discussion !!!)


/// Represents an Orchard ZSA burn item.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct BurnItem(AssetBase, Amount);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valueBurn is specified as a uint64, but Amount is an i64.

Suggested change
pub struct BurnItem(AssetBase, Amount);
pub struct BurnItem(AssetBase, u64);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #29.

Comment on lines +281 to +289
#[cfg(not(feature = "tx-v6"))]
let result = Action::<OrchardVanilla>::max_allocation();

// TODO: FIXME: Check this: V6 is used as it provides the max size of the action.
// So it's used even for V5 - is this correct?
#[cfg(feature = "tx-v6")]
let result = Action::<OrchardZSA>::max_allocation();

result
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggestion relies on moving OrchardVanilla out from behind the feature flag.

Suggested change
#[cfg(not(feature = "tx-v6"))]
let result = Action::<OrchardVanilla>::max_allocation();
// TODO: FIXME: Check this: V6 is used as it provides the max size of the action.
// So it's used even for V5 - is this correct?
#[cfg(feature = "tx-v6")]
let result = Action::<OrchardZSA>::max_allocation();
result
Action::<OrchardVanilla>::max_allocation()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment thread zebra-chain/src/orchard_zsa/burn.rs Outdated
impl TrustedPreallocate for BurnItem {
fn max_allocation() -> u64 {
// FIXME: is this a correct calculation way?
// The longest Vec<BurnItem> we receive from an honest peer must fit inside a valid block.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is correct as-is, it should be (MAX_BLOCK_BYTES - 1) / BURN_ITEM_SIZE and it should be implemented for BurnItem, not Vec<BurnItem>.

}

#[cfg(any(test, feature = "proptest-impl"))]
impl serde::Serialize for BurnItem {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that it's qualified as serde::Serialize since ZcashSerialize is also used in this file, but we probably don't need this implementation at all. If we do need it, it could probably be conditionally derived with #[cfg_attr(condition, attribute)].

let note_value = 10;

let shielded_data: Vec<zebra_chain::orchard::ShieldedData> = (1..=4)
let shielded_data: Vec<zebra_chain::orchard::ShieldedData<OrchardVanilla>> = (1..=4)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subjective: I think orchard::ShieldedData would be slightly more readable, though every option seems fine.

// FIXME: consider more "type safe" way to do the following conversion
// (now it goes through &[u8])
enc_ciphertext: <[u8; 580]>::try_from(
enc_ciphertext: <[u8; OrchardVanilla::ENCRYPTED_NOTE_SIZE]>::try_from(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enc_ciphertext: a.encrypted_note().enc_ciphertext.0.into(),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but see #17 (comment) from @PaulLaux as an alternative, not sure which way of modification is better.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed (used enc_ciphertext: a.encrypted_note().enc_ciphertext.0.into()).

let maybe_shielded_data: Option<zebra_chain::orchard::ShieldedData<OrchardVanilla>> =
bytes
.zcash_deserialize_into()
.expect("a valid orchard::ShieldedData instance");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should deserialize to None if there are no actions for OrchardVanilla (see comment on ZcashDeserialize impl about burns).

fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
// Denoted as `nActionsOrchard` and `vActionsOrchard` in the spec.
let actions: Vec<orchard::Action> = (&mut reader).zcash_deserialize_into()?;
let actions: Vec<orchard::Action<V>> = (&mut reader).zcash_deserialize_into()?;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method returns None if actions.is_empty(), but it likely needs to check for asset burns as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean we should return Some(...) if there're no action but there're burns?

(needs further discussion !!!)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add copies of the updated tests for OrchardZSA as well?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to this comment copied to #37.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add them in this PR?

(needs further discussion !!!)

@dmidem
Copy link
Copy Markdown
Collaborator Author

dmidem commented Mar 3, 2025

Closed in favour of #37

@dmidem dmidem closed this Mar 3, 2025
dmidem added a commit that referenced this pull request Jun 19, 2025
dmidem added a commit that referenced this pull request Jun 19, 2025
dmidem added a commit that referenced this pull request Jun 19, 2025
dmidem added a commit that referenced this pull request Jun 19, 2025
dmidem added a commit that referenced this pull request Jun 19, 2025
dmidem added a commit that referenced this pull request Jun 19, 2025
dmidem added a commit that referenced this pull request Jun 23, 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)**
   #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)**
   #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)**
   #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)**
   #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)**
   #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)**
   #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)**
   #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 a commit that referenced this pull request May 20, 2026
* Minor fix in comments

* Temporary comment of #[cfg(feature = tx-v6)] in zebra-chain/src/transaction/arbitrary.rs

* Update ZSA test blocks (use test data with action groups)

* Update Nu7 consensus branch id in rpc test snapshot

* Make changes according to #17 PR review (partially)

* Derive Copy, Clone, Debug, Eq, PartialEq for EncryptedNote instead of explicit imlps as const generic are stabilized

* Make changes according to #17 PR review

* Fix wrong resolving of merge conflict in the previous commit

* Minor fix in FIXME comment

* Minor spelling fixes in comments

* Fix compilation error in tests

* Fix clippy warning

* Fix clippy warning (2)

* Fix clippy warning (3)

* Refactor burn serialization to use read_burn, write_burn functions from librustzcash

* Remove TrustedPreallocate impl and size constants from orchard_zsa::issuance module as they are not used (librutzcash serialization is used)

* Switch to updated version of orchard and librustzcash crates in Cargo.toml, which add PartialEq and Eq derives to orchard flavor marker types

* Update orchard_flavor_ext module to re-use OrchardVanilla and OrchardZSA marker types from orchard, rename OrchardFlavorExt to ShieldedDataFlavor and rest of the code according those changes

* Rename orchard_flavor_ext module to shielded_data_flavor

* Fix several comments

* Use NoiteValue instead of Amount ffor BurnItem

* Fix compilation error

* Fix compilation error (2)

* Refactor zebra-chain/src/transaction/serialize.rs (add comments, reduce code duplication)

* Fix remaining merge conflicts

* Fix compilation erros

* Fix clippy warning

* Fix compilation errors appeared after the previous merge

* Fix compilation error

* Add support of V6 to remove_orchard_conflicts proptest function in zebrad

* Fix to resolve some review comment for #37

* Update comments and rename some modules according to #37 review notes

* More updates according to #37 review notes

* Add OrchardZSA version of encrypted_ciphertext_roundtrip test

* Fix a couple of comments and orchard_zsa/tests items visibility according to #37 review remarks

* Additional fixes according #37 review comments

* Add support of V6 to property tests for mempool storage in zebrad

* Add comments according to #37 review

* Add v6_strategy to zebra-chain/src/transaction/arbitrary code

* Fix compilation errors

* Fix a mistake in zebra-chain/src/primitives/zcash_note_encryption

* Fix compilation warnings

* Comment out v6_strategy usage in zebra-chain/src/transaction/arbitrary.rs as it causes several tests to fail

* Fix cargo fmt issue

* Fix and return support of Nu7/V6 (i.e. a call of v6_strategy) to Transaction::arbitrary_with function in zebra-chain/src/transaction/arbitrary.rs

* Ensure orchard_shielded_data can not be None for V6 in v6_strategy of zebra-chain/src/transaction/arbitrary.rs

* Fix doc comment for ACTION_SIZE in orchard::ShieldedData

* Move burn field inside action group in serialization functions for OrchardZSA

* Update zebra-consensus/src/primitives/halo2/tests to add OrchardZSA tests

* Fix cargo clippy warning

* Update orchard and librustzcash versions

* Fix order of fields for OrchardZS serialization/deserialization to be the same as in librustzcash

* Update test vectors (with ones that contain burn inside AG)

* Rename and fix orchard shielded data test vectors and releated tests

* Added a doc comment for orchard_zsa workflow test (zebra-consensus/src/orchard_zsa tests.rs module)

* Remove ValueCommitment::with_asset and introduce a ValueCommitment::new variant gated by the tx-v6 feature flag. Use ValueSum from the orchard crate as the value type to support both Amount (value balance) and NoteValue (burn). Add TODO comments for future orchard crate enhancements to simplify type conversions by reusing existing APIs.

* Rename orchard test vectors modules, data files and constants in zebra-test (for vanilla - restore the original names, i.e. remove vanilla suffix, for zsa - move zsa suffix to the end)

* Remove serialization tests from orchard_zsa in zebra-chain (as there're no such tests for Vanilla Orchard, tests from zebra-chain/src/transaction/tests/vectors.rs used instead - so we need to add ZSA related tests there)

* Added short comments/descriptions for test blocks in orchard_workflow_blocks_zsa.rs

* Make fixes according to the second #37 review

* Fix cargo clippy error

* Generalize verify_v5_transaction and verify_v6_transaction functions by converting them into verify_v5_and_v6_transaction generic. The ame for verify_v6_transaction_network_upgrade function

* Fix FIXME comments in transaction.rs

* Create push-deploy.yaml

* Remove allow(missing_docs) from zebra-test/.../orchard_workflow_blocks_zsa.rs and add a doc comment there

* Fix the code to support compilation with tx-v6 feature flag disabled, make this flag non-default in Cargo.toml files and enable it by default in .cargo/config.toml

* Allow manual triggering for push-deploy

* Fix compilation errors in zebra-state happened without tx-v6 feature flag enabled

* Allow finalizing issued assets via the issue action when no notes are provided and the finalize flag is set to true

* Refactor orchard_workflow_blocks_zsa.rs (zebra-test crate) to read test data from files, introduce and use OrchardWorkflowBlock there

* Fix clippy errors

* Copy tests from zsa-issued-assets-tests here and fix compilation errors

* Temporarily comment out verify_issuance_blocks_test test (it fails now)

* Add split of workflow, ecr and ecs

* Update

* Update PR comments

* Update PR comments

* Update by comments

* Repo var for DOCKERFILE_PATH

* Add env

* Add env

* Update deploy-ecs.yaml

* Update deploy-ecs.yaml

* 1 time task definition download

* First-stage ZSA integration: apply upstream feedback & refactoring (ZcashFoundation#9560) (#63)

* Quote variables in push-deploy.yaml to pass upstream actionlint checks

The actionlint job in the ZcashFoundation/zebra repository CI
reported multiple SC2086 errors. This commit quotes shell variables
in push-deploy.yaml to fix the linter failures.

* Avoid to use Into ValueCommitment for ShieldedDataFlavor::BurnType, and use compute_burn_value_commitment instead

* Rename generic const parameter N of EncryptedNote to SIZE

* Remove shielded_data::ActionCommon introduced during ZSA integration, and replace Transaction::orchard_actions() with Transaction::orchard_action_count()

* Remove Default trait constraint for BurnType, also remove FIXME comment there

* Address PR #63 review comments: make compute_burn_value_commitment non-generic and move comment to the top of the function

* Add more V6 tests to zebra-chain, refactor OrchardZSA tests in zebra-test (#62)

* Rename orchard_..._zsa test vector files to orchard_zsa_..., same for structs and consts declardd there

* Add tests for empty tx v6 into zebra-chain

* Add V6 roundtrip tests based on ORCHARD_ZSA_WORKFLOW_BLOCKS test vectors to zebra-chain

* Address PR #62 review comments

* Improve OrchardZSA V6 round-trip and conversion tests

* Clean up rust-toolchain.toml and move rustfmt & clippy installs to CI (#73)

* Drop extra components and refresh TODO in rust-toolchain.toml

* Move rustfmt and clippy installation to CI instead of rust-toolchain.toml

* Update push ecr on zsa1 branch

* Sync Zebra with orchard and librustzcash synced to upstream (#71)

* Quote variables in push-deploy.yaml to pass upstream actionlint checks

The actionlint job in the ZcashFoundation/zebra repository CI
reported multiple SC2086 errors. This commit quotes shell variables
in push-deploy.yaml to fix the linter failures.

* Rename nu6 zcash_unstabl flag to nu7, update orchard and librustzcash revisions in Cargo.toml, other small changes

* Replace deprecated librustzcash types with updated protocol equivalents

* Fix compilation errors occured after the previous commit

* Additionally rename nu6 zcash_unstable flag to nu7 in rustdocflags section of .cargo/config.toml

* Fix cargo clippy issue

* zebra-chain: port TryFrom<HashType>→SighashType from upstream Zebra v2.4.2 (to sync with librustzcash changes)

* Removed resolved FIXME

* Switch to the latest zsa1 rev of librustzcash

* Attempt to refactor asset state management

* Continue refactoring asset state management

* CI: reclaim runner disk, add resource logs, and use --locked/--no-deps to avoid 'No space left on device' (#91)

* Manual partial sync of upstream (ZcashFoundation) v2.4.2 to reduce future merge conflicts (#85)

* Quote variables in push-deploy.yaml to pass upstream actionlint checks

The actionlint job in the ZcashFoundation/zebra repository CI
reported multiple SC2086 errors. This commit quotes shell variables
in push-deploy.yaml to fix the linter failures.

* Rename tx-v6 feature flag to tx_v6 to match with the upstream (ZF) changes

* Expand and remove tx_v5_and_v6 macro

* Reorder transaction methods to match the upstream

* Remove a duplicated comment in zebra-chain's expiry_heigh function

* CI: add --locked arg to cargo calls

* ci: add root probe + safe cleanup and shrink Rust artefacts to avoid runner disk exhaustion

* CI: fix ci-basic.yml for the previous commit

* ci: ensure deps see cfgs by mirroring .cargo/config.toml in RUSTFLAGS/RUSTDOCFLAGS

* Reove RUSTFLAGS from ci-basic.yml

* CI: comment out du commands

* CI: dump largest folders in ci-basic.yml

* CI: simplify ci-basic.yml by removing the cleanup step and reducing disk checks

* CI: expand disk check to show full system resource summary (disk, memory, CPU)

* Try running ci-basic.yml on ubuntu-latest-m2 to check system resources in CI logs

* CI: print runner/image identity in ci-basic.yml

* CI: switch ci-basic.yml back to ubuntu-latest

* CI: Switch to ubuntu-latest explicitly in ci-basic.yml

* Revert "CI: print runner/image identity in ci-basic.yml"

This reverts commit 8b334cd.

* CI: add disk cleanup

* CI: log free disk space after each disk cleanup

* CI: Disk cleanup only for Android tools and preinstalled tool caches

* Add transaction_sighashes to semantically and contextually verified blocks

* Update orchard and librustzcash, align Zebra with upstream API/module changes

* Fix compilation errors that appeared after the previous merge

* zebra-chain: add ZIP-230 vSighashInfo(V0) serialization/deserialization for V6

* zebra-chain: add ZIP-230 vSighashInfo(V0) serialization/deserialization for V6

* zebra-chain: add ZIP-230/ZIP-246 versioned signature support

* zebra-chain: add NU6_1 activation heights

* zebra-chain: add ZIP-230/ZIP-246 versioned signature support

* zebra-chain: add NU6_1 activation heights

* zebra-chain: add missing new sapling_v6.rs module

* zebra-chain: add missing new sapling_v6.rs module

* Update ZSA test blocks

* Update ZSA test blocks again and refactor orchard_zsa_workflow_blocks.rs to use the include_str! macro for storing test blocks in external files

* Add 1-hour limit for cargo test in CI and a specific 1-minute limit for the Orchard ZSA workflow test in zebra-consensus to prevent it from getting stuck when block heights are not sequential

* Fix compilation error from the previous commit

* Update orchard-zsa-shielded-data test vectors

* Update get_blockchain_info@mainnet_10.snap test snapshot

* Update get_blockchain_info@testnet_10.snap test snapshot

* Make fixes according to #92 review comments

* zebra-chain: refactor versioned_sig_v0_roundtrip test according to #92 review comments

* Add testnet-singlenode-deploy

* Add testnet-singlenode-deploy to dockerignore exceptions

* Sync Zebra with latest Orchard and librustzcash (#92)

* Update orchard and librustzcash, align Zebra with upstream API/module changes

* zebra-chain: add ZIP-230 vSighashInfo(V0) serialization/deserialization for V6

* zebra-chain: add ZIP-230/ZIP-246 versioned signature support

* zebra-chain: add NU6_1 activation heights

* zebra-chain: add missing new sapling_v6.rs module

* Update ZSA test blocks

* Update ZSA test blocks again and refactor orchard_zsa_workflow_blocks.rs to use the include_str! macro for storing test blocks in external files

* Add 1-hour limit for cargo test in CI and a specific 1-minute limit for the Orchard ZSA workflow test in zebra-consensus to prevent it from getting stuck when block heights are not sequential

* Fix compilation error from the previous commit

* Update orchard-zsa-shielded-data test vectors

* Update get_blockchain_info@mainnet_10.snap test snapshot

* Update get_blockchain_info@testnet_10.snap test snapshot

* Make fixes according to #92 review comments

* zebra-chain: refactor versioned_sig_v0_roundtrip test according to #92 review comments

* Add testnet-singlenode-deploy

* Add testnet-singlenode-deploy to dockerignore exceptions

* Add TODO for missing ZSA ZIP-317 logical action terms

* Upgrade librustzcash crate revisions in Cargo.toml/Cargo.lock

* Fix according to the second #92 review comments

---------

Co-authored-by: alexeykoren <2365507+alexeykoren@users.noreply.github.com>

* Create stop-ecs workflow (#99)

* Create stop-ecs.yaml

* Clean

* Update Orchard and librustzcash refs for PR471 review sync (#104)

This PR updates Zebra to use the current `orchard` and `librustzcash` crate versions that contain the upstream Orchard PR zcash/orchard#471 review-related changes.

Other than updating the `orchard` and `librustzcash` references in `Cargo.toml` it also includes the follow-up changes needed to keep Zebra building against the updated `orchard`/`librustzcash` APIs:

* Adjust `NoteValue` → `ValueSum` conversions (including updating `orchard_zsa/burn.rs`, and switching `zebra-chain` to `ValueSum::from_raw` where needed). This is a placeholder change and the code does not compile yet — see the `FIXME` comments and the note below.
* Fix test vectors so they compile when `zcash_unstable="nu7"` is enabled but `tx_v6` is not.
* Rename the `orchard_flavor` module to `flavor`.
* Update Orchard's `Bundle::build` usage in `zebra-consensus` tests.
* Upgrade Rust to 1.85.1 (to align with `librustzcash`) and downgrade the `blake2b_simd` dependency to `1.0.1` to align with `orchard`.
* Remove `halo2_gadgets` patch in Cargo.toml

* Refactor to use the latest zsa1 branch commits of Orchard and librustzcash

* Pin blake2b_simd to 1.0.1 for compatibility with Orchard

* Fix test compilation errors

* Add reference_note field to impl serde::Serialize for AssetState

* Final refactoring of asset_base.rs

* Remove 'nonempty' crate dependency

* Add missing doc comment for pup mod testing

* Update Zebra Orchard and librustzcash refs in Cargo.toml

* Skip signature verification for checkpoint verified blocks

* Remove (comment out) failing assert in update_chain_tip_with_block_parallel and add a FIXME comment with an explanation

* Fix zebra-rpc asset_state test snapshots to add reference_note

* Fix zebra-rpc asset_state test snapshots to rename total_supply to amount

* Return MIN_TRANSPARENT_COINBASE_MATURITY to 100

* Make transaction_sighashes optional in semantically and contextually verified blocks

* Fix cargo clippy errors

* Drop global issuance state unrelated changes

* Minor refactoring, FIXME resolution, etc.

* Update deploy-ecs.yaml (#107)

* Update Orchard and librustzcash versions and fix the code accordingly

* Update push-ecr.yaml (#108)

* Update push-ecr.yaml

* Fix CI: update librustzcash rev and derive test reference notes via IssueBundle

- Bump librustzcash to the fixed revision (removes the extra expiry_height check).
- Rewrite zebra-chain test reference-note generation to derive notes with rho via
  IssueBundle (replaces the removed Orchard create_test_reference_note helper).

* Fix zebra-rpc test compilation error

* Add initial/partial support for ZIP-233 (zip233_amount field in Transaction::V6 and serialization only — without changing consensus rules — they will be updated automatically after merging with the upstream version of Zebra, where those ZIP-233-related changes are already implemented)

* Revert "Add initial/partial support for ZIP-233 (zip233_amount field in Transaction::V6 and serialization only — without changing consensus rules — they will be updated automatically after merging with the upstream version of Zebra, where those ZIP-233-related changes are already implemented)"

This reverts commit 8e3076f.

* zebra-test: zebra-test: update Orchard ZSA workflow blocks

* zebra-test: update Orchard ZSA workflow blocks

* zebra-rpc: update get_asset_state snapshots

* Fix cargo clippy issues

* Add initial/partial support for ZIP-233 (zip233_amount field in Transaction::V6 and serialization only — without changing consensus rules — they will be updated automatically after merging with the upstream version of Zebra, where those ZIP-233-related changes are already implemented)

* Fix compilation errors that appeared after the previous commit (i.e., after merging with the upstream Zebra). The code contains new FIXME comments that need to be resolved.

* Add tx_sighash to VerifiedUnminedTx

* zebra-chain: Align Nu7 consensus branch ID and TxV6 version group ID with the values used in the ZSA version of librustzcash. zebra-consensus: Allow v4 transactions for Nu7

* zebra-consensus: Set activation heights for Canopy and NU7 to 1 for regtest creation in Orchard ZSA workflow tests

* Sync .github/workflows with upstream

* Add missed .github/workflows files for the previous commit

* Update Orchard ZSA workflow blocks to support ZIP-233

* Update Orchard ZSA workflow blocks to support ZIP-233

* Fix small cargo fmt issue

* zebra-test: fix Orchard ZSA workflow vectors after correcting hashBlockCommitments

zebra-test/src/vectors/orchard-zsa-workflow-block-1.txt had an all-zero
hashBlockCommitments (ZIP-244) field. Updating it changes the block hash and
chain history tree roots, so blocks 2–4 required updated hashBlockCommitments as
well, and blocks 2–5 required previous_block_hash relinking.

This regenerates the workflow block *.txt vectors so the orchard_zsa workflow
test passes without bypasses.

* zebra-rpc: update get_asset_state_not_found snapshots, assign TransactionObject's orchard to Some(default) instead of None for non-v5/v6 transactions

* ci-basic.yml: rename elided_named_lifetimes to mismatched_lifetime_syntaxes

* ci-basic.yml: match clippy flags with upstream lint workflow

* zebra-test: increase MockService default request wait to 1s to avoid CI failures

* Address PR #111 review comments (tx_v6 feature gating to be fixed separately)

* Clear ENABLE_ZSA in Flags arbitrary strategy to fix V5 transaction roundtrip test

* Make Zebra compilable with tx_v6/nu7 flags disabled

* ci-basic.yml: test both with and without nu7/tx_v6 flags using a matrix

* Fix tv_v6 flag guard style to align with that used in upstream Zebra (now requires both tx_v6 and nu7)

* Fix zsa-integration-state->sync-zcash-v4.1.0 merge conflict resolution mistakes

* Add ZIP-230 coinbase ENABLE_ZSA consensus check

* zebra-consensus: temporarily allow empty NU6.1 lockbox disbursements on Regtest

Regtest/configured testnets can be created with an empty NU6.1 lockbox disbursement list, but
block subsidy validation treated an empty list as an error on the NU6.1 activation block.

After syncing with upstream Zebra v4.2.0, this started breaking our Orchard ZSA Regtest
workflow tests.

Temporarily skip the NU6.1 lockbox disbursement check when no disbursements are configured,
and leave a FIXME to revisit whether empty configured disbursements should remain allowed or
Regtest should provide defaults.

* CI: restore repo cleanliness check, revert temporary Cargo config changes beforehand

* zebra-test: refactor orchard_zsa_workflow_blocks.rs to use a u8 static ref for bytes OrchardWorkflowBlock instead of a vec, matching the approach used in block.rs there

* zebra-test: refactor orchard_zsa_workflow_blocks.rs to use a u8 static ref for bytes OrchardWorkflowBlock instead of a vec, matching the approach used in block.rs there

* Refactor insert_fake_orchard_shielded_data into separate V5 and V6 helpers

* Add v6_coinbase_transaction_with_enable_zsa_flag_fails_validation test

* Refactor insert_fake_orchard_shielded_data into separate V5 and V6 helpers

* Add v6_coinbase_transaction_with_enable_zsa_flag_fails_validation test

* Update orchard and librustzcash crate versions to the latest from the zsa1 branches and adjust related code for compatibility

* Update orchard and librustzcash crate versions to the latest from the zsa1 branches (2)

* Use new IssueBundle::note_commitments method from the orchard crate in IssueData::note_commitments

* Remove zebra-scan files (as they were removed upstream)

* Remove get_block_template_rpcs.rs from zebra-rpc (as it was removed in the upstream)

* Remove orchard/test-dependencies from zebra-chain deps to keep proptest out of the default build

* Bump MSRV to Rust 1.85.1, as librustzcash crates use it

* Correct codespell typo

* zebra-scan: sync CHANGELOG with upstream

* ci: specify stable toolchain for cargo-deny job

CI was failing in the deny job because rustup attempted to use an uninstalled
stable-x86_64-unknown-linux-musl override toolchain (see
https://github.com/QED-it/zebra/actions/runs/24046905754/job/70132458979?pr=114).
Pinning the toolchain to stable makes the job deterministic and avoids
inheriting an unexpected runner/toolchain override.

* Implement ZIP-0227 global issued-assets state in Zebra (#111)

* Defines and implements the issued asset state types

* Adds issued assets to the finalized state

* Validates issuance actions and burns before committing blocks to a non-finalized chain.

* Adds `issued_assets` fields on `ChainInner` and `ContextuallyValidatedBlock`

* Adds issued assets map to non-finalized chains

* adds new column family to list of state column families

* Updates AssetState, AssetStateChange, IssuedAssetsOrChange, & SemanticallyVerifiedBlock types, updates `IssuedAssetsChange::from_transactions()` method return type

* Fixes tests by computing an `IssuedAssetsChange` for conversions to CheckpointVerifiedBlock

* fixes finalization checks

* Adds documentation to types and methods in `asset_state` module, fixes several bugs.

* Fix compilation errors that appeared after the previous merge

* Avoid using NonEmpty in orchard_zsa/issuance

* Fix BurnItem serialization/deserializartioon errors (use LE instead of BE for amount, read amount after asset base)

* Make a minor fix and add FIXME comment in orchard_flavor_ext.rs

* Fix the sign of burn value in SupplyChange::add in orchard_zsa/asset_state, add a couple of FIXMEs

* Fix the 'transactions must have only one burn item per asset base' error in the  function of the  crate (this may not be a fully correct fix). Add a couple of FIXME comments explaining the problem.

* Use NoteValue from the orchard crate for BurnItem amount instead of u64 to prevent serialization errors and enable defining BurnItem in orchard, facilitating its reuse along with related functions

* Use BurnItem::from instead of try_from

* Fix a compilation error for the previous commit ('Use BurnItem::from instead of try_from')

* Fix a compilation error for the previous commit ('Use BurnItem::from instead of try_from') (2)

* Modify ValueCommitment::with_asset to accept value as a NoteValue instead of amount (with_asset is used to process orchard burn) - this allows avoiding the use of try_into for burn in binding_verification_key function

* Adds TODOs

* Adds state request/response variants for querying asset states

* Adds a `getassetstate` RPC method

* Adds snapshot test

* Addesses some FIXMEs and replaces a couple others with TODOs.

* Removes `issued_assets_change` field from `SemanticallyVerifiedBlock`

* Temporarily disable specific Clippy checks for Rust 1.83.0 compatibility

* Disable clippy warning about doc comment for empty line

* Update Orchard ZSA consensus tests to calculate and check asset supply

* Rename ZSA workflow tests (including file, constant and variable names) to Orchard ZSA

* Add amount method to BurnItem and make BurnItem pub (visible for other crates)

* Fix Orchard ZSA workflow tests to make it compilable with getblocktemplate-rpcs feature enabled

* Fix clippy error

* Add rust-toolchain.toml with Rust version 1.82.0 to avoid clippy errors came with Rust 1.83.0

* Fix revert_chain_with function in zebra-state to support V6/OrchardZSA

* Minor fix in comments

* Rename transaction_to_fake_v5 function to transaction_to_fake_min_v5 and panic if V6 passed into it

* Minor fixes in comments

* Revert "Minor fix in comments"

This reverts commit 59fec59.

* Revert "Rename transaction_to_fake_v5 function to transaction_to_fake_min_v5 and panic if V6 passed into it"

This reverts commit 2ce58ef.

* Revert " Minor fixes in comments"

This reverts commit fac3abd.

* Fix remaining merge conflicts

* Fix compilation erros

* Fix clippy warning

* Fix compilation errors appeared after the previous merge

* Fix compilation error

* Fix compilation errors in zebra-state happened without tx-v6 feature flag enabled

* Allow finalizing issued assets via the issue action when no notes are provided and the finalize flag is set to true

* Refactor orchard_workflow_blocks_zsa.rs (zebra-test crate) to read test data from files, introduce and use OrchardWorkflowBlock there

* Fix clippy errors

* Copy tests from zsa-issued-assets-tests here and fix compilation errors

* Temporarily comment out verify_issuance_blocks_test test (it fails now)

* Attempt to refactor asset state management

* Continue refactoring asset state management

* Add transaction_sighashes to semantically and contextually verified blocks

* Update orchard and librustzcash, align Zebra with upstream API/module changes

* Fix compilation errors that appeared after the previous merge

* zebra-chain: add ZIP-230 vSighashInfo(V0) serialization/deserialization for V6

* zebra-chain: add ZIP-230 vSighashInfo(V0) serialization/deserialization for V6

* zebra-chain: add ZIP-230/ZIP-246 versioned signature support

* zebra-chain: add NU6_1 activation heights

* zebra-chain: add ZIP-230/ZIP-246 versioned signature support

* zebra-chain: add NU6_1 activation heights

* zebra-chain: add missing new sapling_v6.rs module

* zebra-chain: add missing new sapling_v6.rs module

* Update ZSA test blocks

* Update ZSA test blocks again and refactor orchard_zsa_workflow_blocks.rs to use the include_str! macro for storing test blocks in external files

* Add 1-hour limit for cargo test in CI and a specific 1-minute limit for the Orchard ZSA workflow test in zebra-consensus to prevent it from getting stuck when block heights are not sequential

* Fix compilation error from the previous commit

* Update orchard-zsa-shielded-data test vectors

* Update get_blockchain_info@mainnet_10.snap test snapshot

* Update get_blockchain_info@testnet_10.snap test snapshot

* Make fixes according to #92 review comments

* zebra-chain: refactor versioned_sig_v0_roundtrip test according to #92 review comments

* Add testnet-singlenode-deploy

* Add testnet-singlenode-deploy to dockerignore exceptions

* Update Orchard and librustzcash refs for PR471 review sync (#104)

This PR updates Zebra to use the current `orchard` and `librustzcash` crate versions that contain the upstream Orchard PR zcash/orchard#471 review-related changes.

Other than updating the `orchard` and `librustzcash` references in `Cargo.toml` it also includes the follow-up changes needed to keep Zebra building against the updated `orchard`/`librustzcash` APIs:

* Adjust `NoteValue` → `ValueSum` conversions (including updating `orchard_zsa/burn.rs`, and switching `zebra-chain` to `ValueSum::from_raw` where needed). This is a placeholder change and the code does not compile yet — see the `FIXME` comments and the note below.
* Fix test vectors so they compile when `zcash_unstable="nu7"` is enabled but `tx_v6` is not.
* Rename the `orchard_flavor` module to `flavor`.
* Update Orchard's `Bundle::build` usage in `zebra-consensus` tests.
* Upgrade Rust to 1.85.1 (to align with `librustzcash`) and downgrade the `blake2b_simd` dependency to `1.0.1` to align with `orchard`.
* Remove `halo2_gadgets` patch in Cargo.toml

* Refactor to use the latest zsa1 branch commits of Orchard and librustzcash

* Pin blake2b_simd to 1.0.1 for compatibility with Orchard

* Fix test compilation errors

* Add reference_note field to impl serde::Serialize for AssetState

* Final refactoring of asset_base.rs

* Remove 'nonempty' crate dependency

* Add missing doc comment for pup mod testing

* Update Zebra Orchard and librustzcash refs in Cargo.toml

* Skip signature verification for checkpoint verified blocks

* Remove (comment out) failing assert in update_chain_tip_with_block_parallel and add a FIXME comment with an explanation

* Fix zebra-rpc asset_state test snapshots to add reference_note

* Fix zebra-rpc asset_state test snapshots to rename total_supply to amount

* Return MIN_TRANSPARENT_COINBASE_MATURITY to 100

* Make transaction_sighashes optional in semantically and contextually verified blocks

* Fix cargo clippy errors

* Drop global issuance state unrelated changes

* Minor refactoring, FIXME resolution, etc.

* Update Orchard and librustzcash versions and fix the code accordingly

* Fix CI: update librustzcash rev and derive test reference notes via IssueBundle

- Bump librustzcash to the fixed revision (removes the extra expiry_height check).
- Rewrite zebra-chain test reference-note generation to derive notes with rho via
  IssueBundle (replaces the removed Orchard create_test_reference_note helper).

* Fix zebra-rpc test compilation error

* Add initial/partial support for ZIP-233 (zip233_amount field in Transaction::V6 and serialization only — without changing consensus rules — they will be updated automatically after merging with the upstream version of Zebra, where those ZIP-233-related changes are already implemented)

* Revert "Add initial/partial support for ZIP-233 (zip233_amount field in Transaction::V6 and serialization only — without changing consensus rules — they will be updated automatically after merging with the upstream version of Zebra, where those ZIP-233-related changes are already implemented)"

This reverts commit 8e3076f.

* zebra-test: zebra-test: update Orchard ZSA workflow blocks

* zebra-test: update Orchard ZSA workflow blocks

* zebra-rpc: update get_asset_state snapshots

* Fix cargo clippy issues

* Add initial/partial support for ZIP-233 (zip233_amount field in Transaction::V6 and serialization only — without changing consensus rules — they will be updated automatically after merging with the upstream version of Zebra, where those ZIP-233-related changes are already implemented)

* Update Orchard ZSA workflow blocks to support ZIP-233

* Address PR #111 review comments (tx_v6 feature gating to be fixed separately)

* Clear ENABLE_ZSA in Flags arbitrary strategy to fix V5 transaction roundtrip test

* Minor fixes in comments

* Add ZIP-230 coinbase ENABLE_ZSA consensus check

* Add unexpected_cfgs for nu7 to Cargo.toml of zebra-consensus

* zebra-test: refactor orchard_zsa_workflow_blocks.rs to use a u8 static ref for bytes OrchardWorkflowBlock instead of a vec, matching the approach used in block.rs there

* Refactor insert_fake_orchard_shielded_data into separate V5 and V6 helpers

* Start addressing additional #111 review comments

* Continue addressing additional #111 review comments: reuse existing regtest genesis test vector

* Continue addressing additional #111 review comments: remove test-gap TODO/FIXME comments

---------

Co-authored-by: Arya <aryasolhi@gmail.com>
Co-authored-by: alexeykoren <2365507+alexeykoren@users.noreply.github.com>
Co-authored-by: Constance Beguier <constance.beguier@gmail.com>

* Start addressing additional #111 review comments

* Continue addressing additional #111 review comments: reuse existing regtest genesis test vector

* Continue addressing additional #111 review comments: remove test-gap TODO/FIXME comments

* Update cargo-deny git source allowlist for new dependency repos

* Allow remaining upstream git sources in cargo-deny

* Address the first part of #114 review comments

* Address the first part of #114 review comments (2)

* Apply the same fix as the upstream branch for the new
clippy::manual_option_zip lint in address UTXO reading code.

Our fork is intentionally pinned to v4.2.0, so we need this
small local patch instead of waiting for a later upstream merge.

* Revert "Apply the same fix as the upstream branch for the new"

This reverts commit 9fe1f13.

* ci: temporarily allow clippy::manual_option_zip in lint job

* ci: allow manual_option_zip clippy lint across toolchain versions

* Remove "toolchain: stable" for deny in lint.yml

* Refactor transaction handling and update related tests per #114 review feedback

* Add tx_v6/nu7 feature guard before verify_v6_transaction_network_upgrade

* zebra-rpc: update snapshots to use new burnexists/issuanceexists fields and add snapshots for new get_asset_state test cases

* zebra-consensus: skip NU6.1 lockbox disbursement check on Regtest

* Make ZSA transaction RPC flags optional

* Add zip233amount to transaction RPC output

* check_orchard_zsa_workflow test: remove extra lockbox_disbursements.is_empty check

* Add ZSA issue note count to ZIP-317 logical actions

* Temporarily skip empty NU6.1 lockbox disbursement checks on Regtest-like test networks

* re-added testnet-single-node-deploy/ folder

* updated network upgrade table

* fix(ci): make unit tests build with nu7 cfg

The setup-rust-toolchain action exports RUSTFLAGS=-D warnings, which
replaces (rather than merges with) .cargo/config.toml rustflags, so
--cfg zcash_unstable="nu7" was silently dropped in CI. Pass it via the
action's rustflags input instead.

Also drop the cfg gate on the Nu7 consensus branch ID so chain history
works in builds without zcash_unstable="nu7"; Nu7-specific transaction
support remains cfg-gated elsewhere.

* updated comments and todos

---------

Co-authored-by: Arseni Kalma <senia.kalma@gmail.com>
Co-authored-by: Paul <3682187+PaulLaux@users.noreply.github.com>
Co-authored-by: alexeykoren <2365507+alexeykoren@users.noreply.github.com>
Co-authored-by: Constance Beguier <constance.beguier@gmail.com>
Co-authored-by: Arya <aryasolhi@gmail.com>
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.

3 participants