Add wallet scanning and storage for the Ironwood shielded pool#2539
Conversation
abc36d0 to
a345f04
Compare
ab673ff to
7a2771f
Compare
Status update: Ironwood receive and send paths completeThe branch now implements the full Ironwood wallet path end to end (scan -> store -> tree -> balance -> select -> spend), rebased on the latest Scanning / receive
Send / spend
Correctness bugs found and fixed (via the tests)
TestsAll new tests are proptests with reusable amount strategies:
Verification
Not in this PRThe Ironwood proposal/protobuf |
| #[cfg(feature = "orchard")] | ||
| fn compact_tx_contains_last_orchard_actions_in_block(&self, tx: &CompactTx) -> bool { | ||
| self.orchard_tree_position | ||
| + u32::try_from(tx.actions.len()).expect("Orchard action count cannot exceed a u32") |
There was a problem hiding this comment.
Note: this field should be renamed into something that contains orchard, like orchard_actions, to follow the pattern for ironwood (meaningfully coined ironwood_actions by Claude).
There was a problem hiding this comment.
Would be in a follow-up.
| /// | ||
| /// Note commitment tree sizes are `u32`-bounded by the protocol, so a valid transaction | ||
| /// can never contain more than `u32::MAX` actions. | ||
| #[cfg(feature = "orchard")] |
There was a problem hiding this comment.
Should be moved into a impl declaration.
`truncate_to_height_internal` truncated the Sapling and Orchard note commitment trees to the target checkpoint but never truncated the Ironwood tree, and `truncate_to_chain_state` re-inserted the Sapling and Orchard frontiers from the supplied chain state but not the Ironwood frontier. After a reorg the Ironwood tree therefore retained commitments and checkpoints for the rolled-back blocks; a subsequent re-scan appended the new chain's commitments onto a stale frontier, corrupting the Ironwood roots and producing invalid anchors for Ironwood spends. Both paths now handle the Ironwood tree alongside Sapling and Orchard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`WalletCommitmentTrees` provided `put_sapling_subtree_roots` and `put_orchard_subtree_roots` but no Ironwood equivalent, so a backend restoring its note commitment trees from a subtree-root source could not populate the Ironwood tree at all. Adds `put_ironwood_subtree_roots` — defaulting to a no-op for backends that do not track an Ironwood tree, mirroring `with_ironwood_tree_mut` — and implements it in `zcash_client_sqlite` against the `ironwood_tree_shards` table, exactly as the Orchard equivalent does for its own. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ation `update_chain_tip` computed the ChainTip-priority scan range's lower bound from the minimum of the Sapling and Orchard shard tips only, omitting the Ironwood tip. Post-NU6.3 the Ironwood pool is sparse, so its last (incomplete) shard can end well below the Sapling and Orchard tips; omitting it meant the ChainTip range started at the higher tip and the incomplete Ironwood shard was not scheduled for scanning at ChainTip priority, delaying (or missing) the point at which a recently received Ironwood note near the chain tip becomes spendable. The Ironwood shard tip is now folded into the minimum alongside the others. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`check_witnesses` scans the wallet's unspent notes for incomplete Merkle witnesses and returns the block ranges that must be re-scanned to repair them. It previously examined only the Sapling and Orchard trees, so an Ironwood note with missing witness data would silently fail to schedule a repair scan. Add an Ironwood pass mirroring the Orchard one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`get_funding_accounts` identifies the wallet accounts that funded a transaction by matching the nullifiers it reveals against the wallet's received notes, but it consulted only the Sapling and Orchard notes. A transaction whose only connection to the wallet was an Ironwood spend was therefore seen as unrelated: `store_decrypted_tx` found no funding account, no wallet outputs, and no decrypted outputs, so it dropped the transaction without recording the spend — leaving the spent Ironwood note counted as spendable. Add `TxMeta::ironwood_spent_note_nullifiers` and `LowLevelWalletRead::detect_accounts_ironwood`, and consult them in `get_funding_accounts`. The SQLite implementation reuses the Orchard spend-detection query, parameterized by table prefix, since Ironwood notes share the Orchard nullifier type but live in a separate table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c722c82 to
abf61a1
Compare
A proposal step binds a single anchor height for all of its shielded-tree lookups. A step that spends no shielded notes has nothing to witness against that anchor, yet was still required to resolve one: a proposal serialized by an earlier version encoded such a step's anchor height as zero, and decoding fed that zero into the tree lookups, failing to build with `AnchorNotFound(0)`. Make `Step::anchor_height` an `Option<BlockHeight>` that is `Some` only when the step spends shielded notes (which must be witnessed against a specific anchor). A step with no shielded inputs carries `None` and defers the choice of anchor to interpretation time. The `Proposal` now records the `ConfirmationsPolicy` under which it was constructed (serialized alongside the proposal), and `create_proposed_transaction` resolves a deferred anchor via the new `ConfirmationsPolicy::anchor_height`, as the target height less the policy's trusted confirmations. A proposal decoded without the confirmations policy field (produced by an earlier version) falls back to the default policy, and a zero anchor height decodes as deferred. The resolved anchor is a real, recent anchor applied to every shielded-output bundle, so the Orchard turnstile indistinguishability property is preserved: a transaction with only routed shielded outputs remains indistinguishable from one that spends real notes in that pool. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| tree::MerkleHashOrchard, | ||
| }; | ||
|
|
||
| #[cfg(feature = "orchard")] |
There was a problem hiding this comment.
These changes are making me wonder if Ironwood logic be gated under a new Ironwood feature instead of orchard? I guess it depends if the orchard feature is gating the protocol or the pool. FMV the pool is the feature to be gated, but that's just my opinion.
e.g. maybe I want to use librustzcash to build a post-NU6.3 tool which is only aware of Ironwood pool
There was a problem hiding this comment.
Not blocking this PR, but want to raise it for discussion
| block_hash, | ||
| txid, | ||
| OrchardDomain::for_compact_action, | ||
| tx.actions |
There was a problem hiding this comment.
(nit) I would prefer to rename tx.actions to tx.orchard_actions to avoid ambiguity and be consistent with tx.ironwood_actions
|
|
||
| /// The nullifier type for Ironwood notes. See [`IronwoodDomain`]. | ||
| #[cfg(feature = "orchard")] | ||
| pub(crate) type IronwoodNullifier = orchard::note::Nullifier; |
There was a problem hiding this comment.
(nb) The symmetry seeker in me wants to see a corresponding OrchardNullifier type alias
| // already a transitive dependency.) | ||
| const DEPENDENCIES: &[Uuid] = &[ | ||
| account_delete_cascade::MIGRATION_ID, | ||
| ironwood_shardtree::MIGRATION_ID, |
There was a problem hiding this comment.
(nit) this requires a graph update in zcash_client_sqlite/src/wallet/init/migration.rs
| /// domain. The key material is the same as for Orchard; only the domain (and thus the accepted | ||
| /// note plaintext version) differs. | ||
| #[cfg(feature = "orchard")] | ||
| impl<AccountId> ScanningKeyOps<IronwoodDomain, AccountId, orchard::note::Nullifier> |
There was a problem hiding this comment.
Shouldn't this be generic on the IronwoodNullifier type?
| /// domain. The key material is the same as for Orchard; only the domain (and thus the accepted | ||
| /// note plaintext version) differs. | ||
| #[cfg(feature = "orchard")] | ||
| impl<AccountId> ScanningKeyOps<IronwoodDomain, AccountId, orchard::note::Nullifier> |
There was a problem hiding this comment.
Would be nice to have a generic OrchardFamily trait to abstract these implementations which are common to both
There was a problem hiding this comment.
Orchard & Ironwood inputs both pass into this closure, right? This disagrees with the involves(Orchard) filter above, no?
| let (ironwood_anchor, ironwood_inputs) = if proposal_step | ||
| .involves(PoolType::Shielded(ShieldedPool::Ironwood)) | ||
| { | ||
| let Some(inputs) = proposal_step.shielded_inputs() else { |
There was a problem hiding this comment.
Similar comment as above, won't this capture Orchard inputs, despite the apparent intent to filter above with involves(Ironwood)
| let Some(inputs) = ironwood_spend_inputs else { | ||
| return Err(Error::ProposalNotSupported); | ||
| }; | ||
| let (ironwood_anchor, ironwood_inputs) = if proposal_step |
There was a problem hiding this comment.
(nit) the shape of this Ironwood section does not match the Orchard section, which begs the question: Why are they different if the logic is ostensibly identical?
015527e to
5a433de
Compare
…ted from `to_received_note` (and its Sapling counterpart) hardcoded the pool in the `ReceivedNoteId` it produced. The Orchard mapper is also used by the Ironwood note-selection queries, so an Ironwood note received a `ReceivedNoteId` tagged `Orchard`. Pool-filtered exclusion in `select_unspent_notes` matches an excluded id only when its pool equals the pool being queried, so an Orchard-tagged Ironwood id would silently escape exclusion (and, applied to an Orchard query, could exclude an unrelated Orchard note sharing its rowid). Thread the pool being queried through the mapper (a shared generic function type) so the `ReceivedNoteId` carries the correct pool; the Sapling mapper now receives and records `Sapling` from its callers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The mapping from an Orchard-protocol `orchard::ValuePool` to the shielded pool it belongs to (Orchard for version-2 notes, Ironwood for version-3 notes) was open-coded at four sites. Route them all through a single `shielded_pool_for_value_pool` helper: `Note::pool` and `decrypt.rs` call it directly, `proposal.rs` and the sqlite send path reuse `Note::pool`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…fully The compact-block scanner extracted each shielded spend's nullifier with `.expect()`, so a compact block served by a buggy or malicious lightwalletd whose spend carries a wrong-length nullifier panicked the sync loop (a remote denial of service) rather than surfacing a handleable error. In the single-threaded scan path `find_spent` runs before the output-side validation, so this fired before the graceful `CompactAction::try_from` check could. Validate each pool's spend nullifiers up front, mapping a malformed nullifier to `ScanError::EncodingInvalid` (mirroring the output-side handling), so `scan_block_with_runners` returns an error the caller can handle. This covers the newly-introduced Ironwood path along with the pre-existing Sapling and Orchard ones. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`get_ironwood_nullifiers` defaulted to `unimplemented!()`. It is called on the scan path (via `Nullifiers::unspent` from `scan_cached_blocks`), so a `WalletRead` backend that compiled against the new version without overriding it would abort the process on its first scan — the API addition was masked at compile time and surfaced only as a runtime panic. Make it a required method, like `get_sapling_nullifiers`, so the addition is a compile-time break that implementers must address, and add the trivial stub to the in-memory test backend. The pre-existing `get_orchard_nullifiers` default is left as-is, as it shipped in a released version. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…decode boundary `Step::from_parts` enforced "no payment may be directed to the Orchard pool once Ironwood is active" with `assert!`, which panics. That invariant holds for the internal input-selection caller, which routes Orchard-receiver payments to the Ironwood pool — but `Step::from_parts` is also reachable from `Proposal::try_into_standard_proposal`, so a protobuf proposal serialized by an older release (which classifies an Orchard-receiver payment as Orchard) or a malicious one aborted the process at the decode boundary rather than returning a decoding error. Downgrade the internal invariant to `debug_assert!` — it remains a programming-error guard for the input-selection path — and reject an Orchard-pool payment while Ironwood is active in `try_into_standard_proposal`, before calling `from_parts`, with the new `ProposalDecodingError::OrchardPaymentProhibited`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `proposed_version: Option<TxVersion>` parameter of `propose_transfer`, `propose_standard_transfer_to_address`, `create_proposed_transactions`, and `InputSelector::propose_transaction`, along with the `ProposalError::IncompatibleTxVersion` variant, were gated behind the `unstable` feature. Remove that gating so callers can constrain the transaction version (e.g. to V5, as PCZT construction requires) without opting into `unstable`. Callers that did not enable `unstable` must now pass the argument; `None` retains the prior behavior of selecting the version from the target height. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5a433de to
1105d82
Compare
…ayments After NU6.3 the Orchard turnstile (a consensus rule) forbids adding value to the Orchard pool, so a payment to an Orchard receiver must be delivered through the Ironwood pool, which only a version 6 transaction carries. `create_pczt_from_proposal` now builds the transaction at the version implied by the target height (version 6 from NU6.3 onward) rather than pinning version 5, and populates the Ironwood bundle the same way it populates the Orchard bundle: output recipient metadata, and — for Ironwood note spends — the Orchard key derivation, since Ironwood notes are spent with the account's Orchard key. An Orchard-receiver payment is therefore realized as a version 6 PCZT carrying an Ironwood bundle, rather than being rejected. The version 6 format and its Ironwood bundle are both fully representable as a PCZT; the earlier rejection conflated the version 6 transaction format with the Ironwood bundle it may carry. When a caller instead explicitly requests a version that cannot carry an Ironwood bundle (notably V5), proposal construction previously routed the payment into the Ironwood pool anyway and the build later failed with an opaque `Builder(TargetIncompatible)` error. Reject such a payment at proposal time with the new `ProposalError::OrchardReceiverRequiresIronwood`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Proposal The transaction version requested at proposal time (the `proposed_version` argument to `propose_transfer` and friends) was previously used only to validate and route the proposal, then discarded, and had to be supplied again separately to `create_proposed_transactions`. The two version inputs could silently disagree. Record the requested version on the `Proposal` — and in its serialized proto form, so it survives a propose-then-build round-trip — and read it during building. `create_proposed_transactions` no longer takes a `proposed_version` argument, and `create_pczt_from_proposal` builds at the proposal's version, falling back to the version implied by the target height when none was requested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1105d82 to
52a19f0
Compare
| let mut preference = Vec::with_capacity(3); | ||
| if prefer_orchard_family { | ||
| if ironwood_selectable { | ||
| preference.push(ShieldedPool::Ironwood); |
There was a problem hiding this comment.
Wouldn't it be better to prefer Orchard first, to eagerly select Orchard coins to be moved to Ironwood?
This PR adds wallet support for the Ironwood shielded pool (ZIP 2005, NU6.3) to
zcash_client_backendandzcash_client_sqlite: assigning the pool code,scanning blocks for received Ironwood notes, storing them, and building the
Ironwood note commitment tree.
Ironwood is treated throughout as a pool distinct from Orchard. Its notes are
Orchard-shaped and are decrypted with the account's Orchard viewing keys, but
they carry version 3 note plaintexts and are decrypted under a separate
note-encryption domain. Where Ironwood reuses Orchard's cryptographic primitives
(commitment, nullifier, and tree types), that reuse is expressed through aliases
rather than by modelling Ironwood as a variant of Orchard.
Tracking issue: #2530. Adapted from the Ironwood work in
valargroup/librustzcash (branch
adam/qleak-pr44-orchard-dummy-ciphertexts,Adam Tucker).
What this adds
Pool code and views
output_poolcode 4.v_received_outputsandv_received_output_spendsviews.Scanning
ScanningKeysand the compact- and full-blockBatchRunnersgain a distinctIronwood set, so the scanner trial-decrypts
tx.ironwood_actionsand thetransaction's Ironwood bundle.
(
orchard::note_encryption::IronwoodDomain), which accepts version 3plaintexts and is distinct from the Orchard domain.
pool distinct from Orchard, across
ScannedBlock,BlockMetadata,WalletTx,and
Nullifiers.Storage and commitment tree
ironwood_received_notes(note version3), Ironwood nullifiers are tracked for spend detection, and the Ironwood note
commitment tree is persisted during
put_blocks, reusing the Orchard shardheight and
with_ironwood_tree_mut.ChainStateand the gRPCTreeStategain the Ironwood note commitment treefrontier; an absent treestate field parses as an empty tree, the correct
Ironwood treestate at pool activation.
two pools to three, so each tree gains a checkpoint at every height
checkpointed in any pool.
AccountBalancegains an Ironwood component and
get_wallet_summarycomputes it, so scannedIronwood notes are reflected in the wallet's total and spendable balances.
Tests
as an Ironwood output, distinct from Orchard.
it is stored in
ironwood_received_notesas note version 3 and the Ironwoodnote commitment tree is persisted. This required threading the Ironwood tree
through the block-generation test harness and adding an
IronwoodFvktestviewing key that emits version 3 notes.
Known limitations / follow-ups
serialization, and note selection for spends) is not implemented; those
todo!sites remain for a later change.