feat: Ironwood#44
Open
ValarDragon wants to merge 86 commits into
Open
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
czarcas7ic
force-pushed
the
ironwood-integration
branch
3 times, most recently
from
June 13, 2026 04:04
8155cd8 to
be0053a
Compare
nuttycom
reviewed
Jun 15, 2026
czarcas7ic
force-pushed
the
ironwood-integration
branch
from
June 16, 2026 18:03
1e28e74 to
213e74e
Compare
Handle the provisional NU6.3 branch ID in the v5 transaction paths that remain active in the foundation split. Also keep existing nu7 cfg sites valid under the workspace lint configuration so clippy does not fail before the later v6 transaction split lands.
czarcas7ic
force-pushed
the
ironwood-integration
branch
8 times, most recently
from
June 18, 2026 18:31
df9eb6e to
bdf48c5
Compare
czarcas7ic
force-pushed
the
ironwood-integration
branch
from
June 19, 2026 21:53
dc95dce to
d94102c
Compare
orchard 1df50eba collapses the bundle-commitment API onto
BundleCommitmentDomain, dropping the `_for_domain` / `_empty_with_domain`
suffixes. Adapt the Orchard and Ironwood txid/auth digest call sites in
zcash_primitives::transaction::txid to the renamed methods
(commitment / authorizing_commitment / hash_bundle_{txid,auth}_empty).
Same BundleCommitmentDomain argument, same return types, same underlying
digest functions, so computed txid/auth digests are unchanged.
…rde) Picks up the downstream-integration API additions on adam/protocol-api-on-ironwood: re-exported Flags/BundleFormat, public Flags::from_parts, and serde on Flags. Additive only; no code changes needed.
#63) * Bump orchard to the Ironwood TX-id head (3819a09) and port the integration zcash/orchard PR#510 (dev/tx-id) rebased the Ironwood orchard work with public-API renames and a restructured bundle/commitment/flag model. This ports the Ironwood integration onto that orchard head (rev 3819a096). API changes adopted: - BundleProtocol -> bundle::BundlePoolRestrictions, 1:1 variant rename (OrchardPreNu6_2; OrchardPreNu6_3 -> OrchardNu6_2Only; OrchardPostNu6_3 -> OrchardNu6_3Onward; IronwoodPostNu6_3 -> IronwoodNu6_3Onward). - BundleCommitmentDomain and BundleFormat removed. The txid, authorizing-data, and empty-bundle commitments, and Flags::to_byte/from_byte, now take a (BundlePoolRestrictions, TxVersion) pair / a pool restriction. The mapping is txid-preserving: every commitment personalization is byte-identical to the old domains (confirmed by the ZIP-244 vectors). - Bundle::commitment/authorizing_commitment, BundleType::num_actions, Note::new (arg order), RandomSeed::rcm -> rcm_v2, and Spend::parse/Output::parse (note_version moved before proprietary) signatures updated. - Builder::protocol() accessor removed; the transaction Builder now tracks the Orchard pool restriction in a field kept in sync with orchard_builder. - Pool-aware note encryption at store time: decrypt_output_with_key takes a pool restriction (Orchard bundle -> Orchard/V2; Ironwood bundle -> IronwoodNu6_3Onward/V3). Consensus behavior adopted from upstream: the Orchard pool at NU6.3 (OrchardNu6_3Onward) forbids cross-address transfers; only Ironwood (IronwoodNu6_3Onward) v6 bundles may set the cross-address flag bit. The Ironwood bundle serializes/parses/commits under IronwoodNu6_3Onward, distinct from the Orchard v6 bundle (new read_ironwood_v6_bundle/write_ironwood_v6_bundle). PCZT build_from_parts now rejects an Ironwood-as-Orchard bundle gracefully instead of panicking. Test generators were updated to produce pool-valid flags. Verified under --cfg zcash_unstable="nu6.3" (--all-features, lib+tests): workspace compiles; zcash_primitives ZIP-244 txid vectors pass; full zcash_primitives, pczt (incl. 11 real-proof end_to_end), and zcash_client_backend suites green. Known follow-up (separate, stacked PR): the wallet block-scanning / decryption paths (scanning/full.rs, scanning/compact.rs, decrypt.rs, and Ironwood sent-output memo recovery in data_api/wallet.rs) still trial-decrypt Ironwood (V3) actions with the now-V2-only OrchardDomain and must use IronwoodDomain. This is a pre-existing, untested scanning path the API split exposed. * test: ignore the two Ironwood note-scanning tests pending the follow-up fix create_proposed_transfer_spends_v3_notes_as_ironwood and send_max_from_orchard_note_after_nu6_3_outputs_ironwood receive V3 (Ironwood) notes via block scanning. The orchard bump makes OrchardDomain V2-only, so the existing scanner no longer decrypts those V3 notes and these tests regress. The scanner is repaired in the stacked follow-up PR (IronwoodDomain scanning), which removes these #[ignore]s. Ignoring them here keeps this PR green without masking the gap, which is documented in the commit and the follow-up. * Enforce ZIP 229 Orchard cross-address restriction for v5 transactions The Orchard pool restriction and its proving/verifying circuit were selected by transaction version, so a v5 transaction at NU6.3 decoded as cross-address-permissive (OrchardNu6_2Only) — the bypass ZIP 229 forbids. Select the restriction and circuit from the consensus branch instead, in the Orchard bundle read, build, txid, and PCZT construction/verification paths. A v5 Orchard bundle at NU6.3 now decodes cross_address_enabled = false (disableCrossAddress = 1), enforced by the PostNu6_3 circuit, and the stale "post-NU6.3 Orchard implies v6" build assertion is removed. Practical effect: a cross-address Orchard output (an ordinary, non-change recipient) can no longer be built into a v5 transaction after NU6.3; unshield and change-to-self still work. * Fix non-nu6.3 build and PCZT extraction tests after the ZIP 229 change Two regressions in the default (non-nu6.3) configuration that the prior commit only exercised under --cfg zcash_unstable="nu6.3": - Builder::orchard_in_use became dead code without the nu6.3 cfg (its only caller, orchard_post_nu6_3_in_use, is nu6.3-gated), tripping the -D warnings Clippy job. Gate it to match. - The PCZT test harness selected the Orchard proving circuit with a hardcoded OrchardNu6_2Only in the non-nu6.3 arm, diverging from the branch-derived circuit the extractor now uses; at a pre-NU6.2 branch the prover and verifier disagreed, failing extraction with InvalidProof. Select the circuit by consensus branch in every cfg, mirroring orchard_protocol_for_branch. * Gate pczt orchard_bundle_format and import BranchId for the orchard feature The ZIP 229 change left orchard_bundle_format (which returns an Orchard type) without its #[cfg(feature = "orchard")], and used BranchId from an import gated only on the io-finalizer/signer/tx-extractor roles. Builds with the orchard feature but none of those roles (the Orchard and ZFuture CI states) and builds without orchard (Sapling-only) failed to compile pczt. Gate orchard_bundle_format and import BranchId under the union of the orchard feature and the role features. * Rename the stale `protocol` parameter to `pool_restrictions` in Orchard fees `transactional_action_count` took a `BundlePoolRestrictions` under the old `protocol` name (a leftover from the former `BundleProtocol` type); rename it to match the convention used elsewhere. * Derive Orchard v5 serialization params from the consensus branch read_v5_bundle/write_v5_bundle now take the consensus BranchId and derive both the proof-size enforcement and the pool restriction internally, instead of having Transaction::read/write_v5 compute and pass them. Both are functions of the branch (not of each other -- e.g. ZFuture maps to OrchardPreNu6_2 yet proof-size Strict), so deriving them together in one place removes the duplicated branch match from the call sites and makes a mismatched (proof-size, restriction) pair unrepresentable. Behavior is unchanged. * Count Orchard fees with branch restrictions * Document branch-keyed Orchard serialization changes; clarify txid invariant Follow-up to the orchard 3819a09 bump, addressing pre-merge review: - zcash_primitives CHANGELOG: record the breaking read_v5_bundle/ write_v5_bundle signature changes (these now take the transaction's consensus_branch_id) and the new nu6.3-gated read_ironwood_v6_bundle/ write_ironwood_v6_bundle functions. Reword the stale builder entry to say the Orchard pool restriction is selected by consensus branch rather than transaction version (ZIP 229), so a v5 request after NU6.3 still enforces the NU6.3 restriction. - txid.rs: note that orchard_pool_restrictions_for_flags returns a restriction matching the bundle's own cross-address flag, so Flags::to_byte always succeeds and the commitment/authorizing_commitment expect() is unreachable by construction. - wallet.rs: fix non-canonical indentation in the Ironwood decrypt_output_with_key call. * Bump orchard to a0c1b83 (feat/ironwood) Non-breaking orchard update; serialization, txid, and Orchard proving/verification tests pass unchanged on nu6.3 and the default build. * Call `orchard_commitment_domain` once in `digest_orchard` Previously the commitment domain was recomputed in both arms of the `map_or_else`; hoist it to a single call before the match. Behavior is unchanged (the function is pure); the empty-bundle arm uses the derived pool restriction and the present-bundle arm still derives its restriction from the bundle flags. * Call `ironwood_v6_domain` once in `digest_ironwood` Both arms of the `map_or_else` recomputed the Ironwood commitment domain; hoist it to a single call before the match. `ironwood_v6_domain` returns a constant, so this is behavior-preserving. * Name the Ironwood fee action count `ironwood_actions` In both fee helpers (`get_fee` and the zfuture variant) the Ironwood action count was folded directly into a shadowed `orchard_actions` binding, so the name no longer reflected its contents. Extract it into its own `ironwood_actions` binding and form the orchard-pool total as `orchard_actions + ironwood_actions` (the value the fee rule expects). Behavior is unchanged. * Document the commitment-only role of `orchard_commitment_domain` Clarify that this helper is used solely to compute Orchard txid and authorizing-data commitments, and so is keyed on the transaction version rather than a consensus branch id: NU6.2 and NU6.3 produce identical Orchard commitments, so there is nothing to distinguish between those branches here. The returned `BundlePoolRestrictions` is a commitment-domain selector, not the consensus pool restriction the bundle was built under.
* Use IronwoodDomain for Ironwood (V3) note scanning and decryption
The orchard bump split the version-agnostic OrchardDomain into OrchardDomain
(V2-only) and IronwoodDomain (V3-only) note-encryption domains. The wallet's
block-scanning and decryption paths were still trial-decrypting Ironwood (V3)
actions with the now-V2-only OrchardDomain, so foreign-received Ironwood notes
were never detected and Ironwood sent-output memos were silently dropped. (The
wallet's own Ironwood notes were unaffected: they are recorded via the
store-time decrypt_output_with_key(IronwoodNu6_3Onward, ...) path.)
Thread IronwoodDomain through the Ironwood paths:
- scanning/full.rs, scanning/compact.rs: the `ironwood` batch runner now uses
IronwoodDomain (new TaggedIronwood* batch aliases; OrchardTasks widened to
also require Tasks<TaggedIronwoodBatch> under nu6.3), and the inline
find_received for the Ironwood bundle uses IronwoodDomain plus the new
Ironwood scanning-key set.
- scanning.rs: the Orchard ScanningKeyOps impl is now generic over the
note-encryption domain version, and ScanningKeys carries a parallel `ironwood`
key set (the same Orchard FVK-derived keys boxed for IronwoodDomain; the note
version travels in the decrypted Note, so the Orchard viewing keys are correct
for both pools).
- decrypt.rs: decrypt_orchard_outputs is generic over the domain version; the
Orchard bundle decrypts with OrchardVersion, the Ironwood bundle with
IronwoodVersion.
- data_api/wallet.rs: Ironwood sent-output memo recovery uses IronwoodDomain.
This also removes the #[ignore] that the base PR placed on the two
zcash_client_sqlite Ironwood note-scanning tests
(create_proposed_transfer_spends_v3_notes_as_ironwood and
send_max_from_orchard_note_after_nu6_3_outputs_ironwood); they receive V3 notes
via scanning and now pass with this fix.
All Ironwood scanning code is gated behind cfg(zcash_unstable = "nu6.3").
Verified under --cfg zcash_unstable="nu6.3": the workspace compiles
(--all-features, lib+tests) in both the nu6.3 and default configs;
zcash_client_backend (68 lib tests), zcash_client_sqlite (incl. the two
un-ignored Ironwood scanning tests), and zcash_client_memory suites green.
* Address codex review: seed Ironwood batch runners from Ironwood keys; changelog
- scanning/{compact,full}.rs: `BatchRunners::for_keys` now seeds the Ironwood
batch runner from `scanning_keys.ironwood()` (was `.orchard()`), matching where
`find_received` resolves decrypted Ironwood tags (`&scanning_keys.ironwood`).
Without this, a caller whose `orchard`/`ironwood` key maps differ (e.g.
Ironwood-only scanning or differing tags) could miss Ironwood notes.
- CHANGELOG: document the NU6.3-gated `ironwood` parameter added to
`ScanningKeys::new`, and the Ironwood (v3) scanning/decryption domain fix.
ValarDragon
commented
Jun 26, 2026
Comment on lines
+142
to
+166
| /// Sets spend witnesses for Orchard actions by action index. | ||
| /// | ||
| /// Returns an error if any witness references an action index that does not exist, | ||
| /// or if an Orchard proof is already present. | ||
| #[cfg(feature = "orchard")] | ||
| pub fn set_orchard_spend_witnesses( | ||
| mut self, | ||
| witnesses: impl IntoIterator<Item = OrchardSpendWitness>, | ||
| ) -> Result<Self, OrchardSpendWitnessError> { | ||
| self.pczt | ||
| .orchard | ||
| .validate_orchard_note_plaintext_versions() | ||
| .map_err(OrchardSpendWitnessError::NotePlaintextVersion)?; | ||
| ensure_no_orchard_proof(&self.pczt.orchard)?; | ||
| for witness in witnesses { | ||
| let action = self | ||
| .pczt | ||
| .orchard | ||
| .actions | ||
| .get_mut(witness.action_index()) | ||
| .ok_or(OrchardSpendWitnessError::InvalidActionIndex( | ||
| witness.action_index(), | ||
| ))?; | ||
| action.spend.witness = Some(witness.serialized_witness()); | ||
| } |
Author
There was a problem hiding this comment.
Why do we need a new fn for this? Shouldn't this already be handled?
If not, maybe needs to be one API with anchor update
ValarDragon
commented
Jun 26, 2026
Comment on lines
+171
to
+219
| /// Sets the Ironwood bundle anchor for a version 6 PCZT on NU6.3. | ||
| /// | ||
| /// Ironwood signatures in v6 do not commit to this anchor, so this may be | ||
| /// called after shielded signatures have been added. Ironwood proofs do | ||
| /// depend on the anchor, so this must be called before proof creation. | ||
| /// | ||
| /// Returns an error if the PCZT is not version 6 on NU6.3, or if an Ironwood proof is | ||
| /// already present. | ||
| #[cfg(all(feature = "orchard", zcash_unstable = "nu6.3"))] | ||
| pub fn set_v6_ironwood_anchor( | ||
| mut self, | ||
| anchor: ::orchard::Anchor, | ||
| ) -> Result<Self, OrchardSpendWitnessError> { | ||
| ensure_v6_consensus_branch(&self.pczt.global)?; | ||
| ensure_no_orchard_proof(&self.pczt.ironwood)?; | ||
| self.pczt.ironwood.anchor = anchor.to_bytes(); | ||
| Ok(self) | ||
| } | ||
|
|
||
| /// Sets spend witnesses for Ironwood actions by action index. | ||
| /// | ||
| /// Returns an error if the PCZT is not version 6 on NU6.3, if any witness references | ||
| /// an action index that does not exist, or if an Ironwood proof is already present. | ||
| #[cfg(all(feature = "orchard", zcash_unstable = "nu6.3"))] | ||
| pub fn set_ironwood_spend_witnesses( | ||
| mut self, | ||
| witnesses: impl IntoIterator<Item = OrchardSpendWitness>, | ||
| ) -> Result<Self, OrchardSpendWitnessError> { | ||
| ensure_v6_consensus_branch(&self.pczt.global)?; | ||
| self.pczt | ||
| .ironwood | ||
| .validate_ironwood_note_plaintext_versions() | ||
| .map_err(OrchardSpendWitnessError::NotePlaintextVersion)?; | ||
| ensure_no_orchard_proof(&self.pczt.ironwood)?; | ||
| for witness in witnesses { | ||
| let action = self | ||
| .pczt | ||
| .ironwood | ||
| .actions | ||
| .get_mut(witness.action_index()) | ||
| .ok_or(OrchardSpendWitnessError::InvalidActionIndex( | ||
| witness.action_index(), | ||
| ))?; | ||
| action.spend.witness = Some(witness.serialized_witness()); | ||
| } | ||
|
|
||
| Ok(self) | ||
| } | ||
| } |
Author
There was a problem hiding this comment.
Ditto for compressing into one API call
orchard's feat/ironwood now returns Result<_, CommitmentError> from Bundle::commitment / authorizing_commitment and the hash_bundle_*_empty helpers. The txid digesters .expect() these: the pool restriction is derived from the bundle's own flags (so to_byte always succeeds, never UnrepresentableFlags), and the Orchard/Ironwood digesters never pass an invalid (restriction, version) pair, so the error is unreachable by construction.
* Migrate to orchard 0.15.0-pre.1 (BundlePoolRestrictions -> BundleVersion) orchard 0.15.0-pre.1 (crates.io) replaces BundlePoolRestrictions and ProofSizeEnforcement with BundleVersion = (ValuePool, ProtocolVersion). Each bundle carries its BundleVersion, so commitments take only tx_version, Flags::to_byte/from_byte and the bundle constructors take a BundleVersion, and hash_bundle_*_empty take a ValuePool. The branch->version mapping is 1:1 (OrchardPreNu6_2->orchard_insecure_v1, OrchardNu6_2Only->orchard_v2, OrchardNu6_3Onward->orchard_v3, IronwoodNu6_3Onward->ironwood_v3); txid, proof-size enforcement, and circuit selection are preserved (zip_0244 and the real-proving suites pass unchanged). * Fix CI: resolve BundleVersion intra-doc link and exempt orchard 0.15.0-pre.1 in cargo vet The intra-doc job (--document-private-items) flagged a [`BundleVersion`] link in a test-only helper whose short name was out of scope; use the fully-qualified path. cargo vet flagged the new crates.io orchard 0.15.0-pre.1; add a safe-to-deploy exemption.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
ValarDragon
force-pushed
the
feat/ironwood
branch
from
July 4, 2026 07:53
412a918 to
4d98d56
Compare
This was referenced Jul 4, 2026
ValarDragon
added a commit
that referenced
this pull request
Jul 4, 2026
Transplant the file-level changes from #44 onto the updated upstream feat/ironwood base. Co-Authored-By: OpenAI Codex <codex@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR is the librustzcash integration base for the Ironwood NU7 work used by Zebra and downstream wallet testing.
Review Notes
Ironwood uses the Orchard circuit and proof flow, but it has separate pool semantics, transaction hashing, note commitment handling, bundle placement, and wallet note storage. This branch is intended to provide the shared transaction and wallet substrate for the rest of the Ironwood stack.
zcash_client_backend/src/proto/{compact_formats,service}.rsandzcash_client_memory/src/proto/memwallet.rsare generated prost outputs from the changed.protofiles and are already markedlinguist-generated. Review the.protochanges and handwrittenzcash_client_backend/src/proto.rsfor semantics. Existing protobuf consistency CI verifies regeneration drift.Validation
cargo fmt --allgit diff --checkcargo check --workspace --all-featuresRUSTFLAGS='--cfg zcash_unstable="nu7"' cargo check --workspace --all-featurescargo check -p zcash_primitivesRUSTFLAGS='--cfg zcash_unstable="nu7"' cargo check -p zcash_primitivesRUSTFLAGS='--cfg zcash_unstable="nu7"' cargo test -p zcash_primitives transaction