Skip to content

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

Merged
dmidem merged 23 commits intozsa1from
zsa1-v2.4.2-partial-sync
Oct 28, 2025
Merged

Manual partial sync of upstream (ZcashFoundation) v2.4.2 to reduce future merge conflicts#85
dmidem merged 23 commits intozsa1from
zsa1-v2.4.2-partial-sync

Conversation

@dmidem
Copy link
Copy Markdown
Collaborator

@dmidem dmidem commented Sep 22, 2025

This PR cherry-picks a subset of changes from the ZcashFoundation/zebra v2.4.2 tag onto our zsa branch to minimize future merge conflicts.

Included changes:

  • Rename feature flag tx-v6 to tx_v6.
  • Remove the tx_v5_and_v6 macro and inline its logic to match upstream.
  • Move Transaction methods gated by the proptest-impl feature into a dedicated impl Transaction block with #[cfg(any(test, feature = "proptest-impl"))] at the bottom of the file, mirroring upstream layout.

Each item above is a separate commit.
No other upstream updates or behavioral changes are included.

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.

minor comments

Comment on lines +342 to +344
pub fn has_transparent_or_shielded_outputs(&self) -> bool {
!self.outputs().is_empty() || self.has_shielded_outputs()
}
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's call it has_any_outputs().
I don't see where it is used.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is needed?

Copy link
Copy Markdown
Collaborator Author

@dmidem dmidem Oct 3, 2025

Choose a reason for hiding this comment

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

Yes, it’s used here:

https://github.com/QED-it/zebra/blob/zsa1-v2.4.2-partial-sync/zebra-consensus/src/transaction/check.rs#L127

I didn’t change the function itself; several functions were just re-ordered to match upstream:

  1. In QED-it/zebra/zsa1 we had:
has_shielded_inputs
has_transparent_or_shielded_outputs
has_shielded_outputs

See: https://github.com/QED-it/zebra/blob/zsa1/zebra-chain/src/transaction.rs#L332-L361

  1. In ZcashFoundation/zebra/main they have:
has_shielded_inputs
has_shielded_outputs
has_transparent_or_shielded_outputs

See: https://github.com/ZcashFoundation/zebra/blob/main/zebra-chain/src/transaction.rs#L314-L340

So I moved has_transparent_or_shielded_outputs to match the upstream order and reduce future merge conflicts.

Comment on lines +1657 to +1673
} => outputs,
Transaction::V3 {
ref mut outputs, ..
} => outputs,
Transaction::V4 {
ref mut outputs, ..
} => outputs,
Transaction::V5 {
ref mut outputs, ..
} => outputs,
#[cfg(feature = "tx_v6")]
Transaction::V6 {
ref mut outputs, ..
} => outputs,
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

does the big chunk of code moved to enable an easier merge?

Copy link
Copy Markdown
Collaborator Author

@dmidem dmidem Oct 3, 2025

Choose a reason for hiding this comment

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

Yes, in ZcashFoundation/zebra/main they grouped and moved all functions gated by #[cfg(any(test, feature = "proptest-impl"))] into a dedicated impl Transaction block at the bottom of the file:

#[cfg(any(test, feature = "proptest-impl"))]
impl Transaction {
    // ...
}

https://github.com/ZcashFoundation/zebra/blob/main/zebra-chain/src/transaction.rs#L1542

In particular, in QED-it/zebra/zsa1 the old style is still used for outputs_mut:

https://github.com/QED-it/zebra/blob/zsa1/zebra-chain/src/transaction.rs#L591

In ZcashFoundation/zebra/main it’s moved into the new impl Transaction:

https://github.com/ZcashFoundation/zebra/blob/main/zebra-chain/src/transaction.rs#L1766

So I applied the same change in our fork to reduce the diff and minimize merge conflicts.

@dmidem dmidem force-pushed the zsa1-v2.4.2-partial-sync branch from 1ef660f to eb71aa2 Compare October 7, 2025 05:46
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.

looks good

@dmidem dmidem merged commit 871d70b into zsa1 Oct 28, 2025
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants