From 09d66dc6dd4eceafb5be371eabbf7e711c7c22c2 Mon Sep 17 00:00:00 2001 From: Mark Henderson Date: Tue, 18 Aug 2026 12:20:52 -0400 Subject: [PATCH 1/4] [zero] feat(librustzcash): bucket the anchor of every Orchard-spending transaction ZIP 318 proves a canonical pool crossing against a boundary of the anchor bucket grid and gives it the rolling expiry, so that many wallets' crossings share a small set of anchors instead of each pinning the block its wallet happened to be synced to. Both observables are independent of the transaction's shape: any transaction can share them at no cost beyond confirmations on its inputs. Widen both to every proposal that spends Orchard notes. The bucketed attempt now runs for any Orchard-permitted request rather than only one whose payment value is a canonical denomination, keeping the existing anchor-computability check and the fallback to the ordinary anchor when no boundary is reachable or the wallet holds no note old enough for one. The expiry rule keys on the step's own anchor being a grid boundary, so the two travel together: a step that fell back keeps the ordinary expiry, which is what prevents an expiry no other transaction shares from re-identifying what a shared anchor anonymized. The rest of the crossing shape is deliberately NOT widened. The canonical denomination, the unpadded Ironwood bundle and the canonical fee are what make a crossing indistinguishable from a migration transfer, and an ordinary payment cannot wear that disguise: it pays an arbitrary amount to someone else. Half a disguise is a fingerprint, so those stay gated on the canonical attempt. Co-Authored-By: Claude Opus 5 --- .../zcash_client_backend/CHANGELOG.md | 16 ++ .../src/data_api/error.rs | 21 +- .../src/data_api/testing/pool.rs | 174 +++++++++++++++-- .../src/data_api/wallet.rs | 179 ++++++++++++------ .../zcash_client_sqlite/src/testing/pool.rs | 12 +- .../zcash_client_sqlite/src/wallet/orchard.rs | 9 +- 6 files changed, 327 insertions(+), 84 deletions(-) diff --git a/librustzcash/zcash_client_backend/CHANGELOG.md b/librustzcash/zcash_client_backend/CHANGELOG.md index 876a5fa7..f3245d4f 100644 --- a/librustzcash/zcash_client_backend/CHANGELOG.md +++ b/librustzcash/zcash_client_backend/CHANGELOG.md @@ -10,6 +10,22 @@ workspace. ## [Unreleased] +### Changed - Zero fork +- `zcash_client_backend::data_api::wallet::propose_transfer` now proves EVERY + proposal that spends Orchard notes against a boundary of the wallet's anchor + bucket grid, not only a canonical ZIP 318 crossing. When no boundary is + reachable, or the wallet cannot fund the payment from notes old enough for + one, the proposal falls back to the ordinary anchor as before. The remaining + ZIP 318 crossing properties (canonical denomination, Ironwood bundle padding, + canonical fee) are unchanged and still apply only to a canonical crossing. + Callers should expect an Orchard-spending payment to require up to one grid + interval of additional confirmations on its inputs. +- Any step proved against a bucket boundary now takes the ZIP 318 rolling + expiry, where previously only a canonical crossing did. +- `zcash_client_backend::data_api::error::Error::ExpiryHeightConflictsWithCanonicalCrossing` + has been renamed to `ExpiryHeightConflictsWithBoundaryAnchor`, and is now + returned for any boundary-anchored step rather than only a canonical crossing. + ### Added - `zcash_client_backend::data_api::WalletWrite::import_standalone_transparent_address` (requires the `transparent-key-import` feature): imports a transparent diff --git a/librustzcash/zcash_client_backend/src/data_api/error.rs b/librustzcash/zcash_client_backend/src/data_api/error.rs index 9fb1ea19..aea879f3 100644 --- a/librustzcash/zcash_client_backend/src/data_api/error.rs +++ b/librustzcash/zcash_client_backend/src/data_api/error.rs @@ -114,14 +114,14 @@ pub enum Error write!( + Error::ExpiryHeightConflictsWithBoundaryAnchor { requested } => write!( f, - "An expiry height of {requested} was requested for a canonical ZIP 318 crossing, \ - which takes the ZIP 318 rolling expiry; pass `None` to accept it." + "An expiry height of {requested} was requested for a transaction proved against \ + an anchor bucket boundary, which takes the ZIP 318 rolling expiry; pass `None` \ + to accept it." ), Error::ExpiryHeightBelowTargetHeight { expiry_height, diff --git a/librustzcash/zcash_client_backend/src/data_api/testing/pool.rs b/librustzcash/zcash_client_backend/src/data_api/testing/pool.rs index 9d73a7b0..8505b1e4 100644 --- a/librustzcash/zcash_client_backend/src/data_api/testing/pool.rs +++ b/librustzcash/zcash_client_backend/src/data_api/testing/pool.rs @@ -9101,7 +9101,11 @@ pub fn canonical_crossing_is_bucketed_and_unpadded( "the Ironwood bundle must be a single unpadded action" ); - // (2) One zatoshi off a canonical denomination: ordinary anchor, padded Ironwood bundle. + // (2) One zatoshi off a canonical denomination: BUCKETED anchor, padded Ironwood bundle. The + // anchor and the shape are widened separately. Every Orchard-spending payment is proved + // against a boundary, so this one shares its anchor with the crossing above; its VALUE is not + // a canonical denomination, so it remains distinguishable as a crossing and keeps the padding + // its fee was charged for. let off_by_one = propose( &mut st, (MAX_RESIDUAL_VALUE + Zatoshis::const_from_u64(1)).unwrap(), @@ -9117,8 +9121,14 @@ pub fn canonical_crossing_is_bucketed_and_unpadded( Ok(2) ); assert!( - !interval.is_boundary(step.anchor_height().unwrap()), - "a non-canonical payment must not pay for a bucketed anchor" + interval.is_boundary(step.anchor_height().unwrap()), + "an Orchard-spending payment must anchor to a grid boundary even when its shape is not \ + a canonical crossing" + ); + assert_eq!( + step.anchor_height(), + canonical.steps().first().anchor_height(), + "and to the SAME boundary as the crossing: one cohort, not two" ); // Building the canonical proposal is left until last: it spends the wallet's only note, so the @@ -9329,14 +9339,14 @@ pub fn canonical_crossing_builds_at_empty_boundary_block( } /// A canonical amount that cannot be funded from a single Orchard note is NOT a canonical -/// crossing, and must not pay for a bucketed anchor it gains nothing from. +/// crossing, but is still bucketed: it spends Orchard notes, and its anchor is shared. /// -/// A migration transfer spends exactly one note, so a multi-input transaction resembles none. The -/// decision is therefore made before the proposal is kept: had it been made by falling back only -/// on insufficient funds, this transaction would have funded perfectly well from several notes and -/// been left carrying an anchor up to a full interval older than necessary, for no benefit. +/// A migration transfer spends exactly one note, so a multi-input transaction resembles none and +/// keeps the Ironwood padding its fee was charged for. What it does NOT keep is a chain-tip anchor: +/// the anchor is the one observable a transaction of any shape can share at no cost beyond +/// confirmations, so it is bucketed here exactly as it is for the canonical crossing. #[cfg(feature = "orchard")] -pub fn multi_note_crossing_is_not_bucketed( +pub fn multi_note_crossing_is_bucketed_but_not_canonical( ds_factory: Dsf, cache: impl TestCache, ) { @@ -9415,8 +9425,8 @@ pub fn multi_note_crossing_is_not_bucketed( ); assert!(!step.is_canonical_crossing(&zip318, canonical_fee)); assert!( - !interval.is_boundary(step.anchor_height().unwrap()), - "a multi-input transaction must not pay for a bucketed anchor" + interval.is_boundary(step.anchor_height().unwrap()), + "a multi-input Orchard payment must still anchor to a grid boundary" ); assert_eq!( step.ironwood_action_count( @@ -9425,6 +9435,148 @@ pub fn multi_note_crossing_is_not_bucketed( ), Ok(2) ); + + // The expiry travels with the anchor, not with the shape: a boundary-anchored transaction takes + // the ZIP 318 rolling expiry even though its padding says it is no migration transfer. An + // ordinary per-transaction expiry would re-identify what the shared anchor just anonymized. + let txids = st.create_proposed_expecting(&proposal, 1); + let tx = st.get_tx_from_history(txids[0]).unwrap().unwrap(); + assert_eq!( + tx.expiry_height(), + Some(zcash_protocol::zip318::expiry_height(BlockHeight::from( + proposal.min_target_height() + ))), + "a boundary-anchored payment must carry the ZIP 318 rolling expiry" + ); +} + +/// A payment whose Orchard notes are all younger than the boundary is built IMMEDIATELY against +/// the ordinary anchor, rather than waiting for a boundary they would be old enough for. +/// +/// Bucketing is expressed as a raised confirmation requirement, so a wallet whose only notes +/// arrived since the age-1 boundary cannot fund the bucketed attempt at all. That miss is a +/// fallback, not a failure: the payment is proposed again under the caller's own policy, takes the +/// ordinary anchor, and with it the ordinary per-transaction expiry. Anonymity is worth +/// confirmations; it is never worth refusing to spend. +#[cfg(feature = "orchard")] +pub fn orchard_payment_falls_back_when_notes_are_too_new( + ds_factory: Dsf, + cache: impl TestCache, +) { + let interval = AnchorBucketInterval::custom(NonZeroU32::new(12).expect("nonzero")); + let activation = BlockHeight::from_u32(100_000); + let ironwood_active_network = LocalNetwork { + nu6: Some(activation), + nu6_1: Some(activation), + nu6_2: Some(activation), + nu6_3: Some(activation), + ..TestBuilder::<(), ()>::DEFAULT_NETWORK + }; + + let mut st = TestDsl::from( + TestBuilder::new() + .with_network(ironwood_active_network) + .with_data_store_factory(ds_factory) + .with_block_cache(cache) + .with_anchor_retention_interval(interval) + .with_account_from_sapling_activation(BlockHash([0; 32])), + ) + .build::(); + + let account = st.test_account().cloned().unwrap(); + let fvk = OrchardPoolTester::test_account_fvk(&st); + let recipient = OrchardPoolTester::fvk_default_address(&fvk).to_zcash_address(st.network()); + + // The wallet's starting note predates NU6.3, as every Orchard note on a real chain now does: + // the pool is closed to new value, so the only Orchard note that can be YOUNGER than a + // boundary is change from the wallet's own Orchard spend. This test makes one. + let note_value = Zatoshis::const_from_u64(10_000_000); + let (received_height, _, _) = st.add_a_single_note_checking_balance(note_value); + + let interval_blocks = interval.block_count().get(); + let tip = u32::from(interval.boundary_at_or_above(received_height)) + 3 * interval_blocks + 5; + let filler_count = tip - u32::from(received_height); + let not_our_fvk = OrchardPoolTester::sk_to_fvk(&OrchardPoolTester::sk(&[0xf5; 32])); + for _ in 0..filler_count { + st.generate_next_block( + ¬_our_fvk, + AddressType::DefaultExternal, + Zatoshis::const_from_u64(10_000), + ); + } + st.scan_cached_blocks(received_height + 1, filler_count as usize); + + let input_selector = GreedyInputSelector::new(); + let change_strategy = + single_output_change_strategy(StandardFeeRule::Zip317, None, ShieldedPool::Orchard); + let propose = |st: &mut TestState<_, _, _>| { + let request = TransactionRequest::new(vec![Payment::without_memo( + recipient.clone(), + MAX_RESIDUAL_VALUE, + )]) + .unwrap(); + st.propose_transfer( + account.id(), + &input_selector, + &change_strategy, + request, + ConfirmationsPolicy::MIN, + ) + }; + + // The original note is older than the age-1 boundary, so this payment is bucketed. + let first = propose(&mut st).expect("the original note can fund this"); + assert!( + interval.is_boundary( + first + .steps() + .first() + .anchor_height() + .expect("a shielded step binds an anchor") + ), + "the scenario starts from a bucketed payment" + ); + + // Mine it. Its Orchard change is now the wallet's only Orchard note, received at the tip and + // so younger than every boundary the bucketed policy can reach. + let txids = st.create_proposed_expecting(&first, 1); + let (mined_height, _) = st.generate_next_block_including(txids[0]); + st.scan_cached_blocks(mined_height, 1); + for _ in 0..2 { + st.generate_next_block( + ¬_our_fvk, + AddressType::DefaultExternal, + Zatoshis::const_from_u64(10_000), + ); + } + st.scan_cached_blocks(mined_height + 1, 2); + assert!( + u32::from(mined_height) + > u32::from(interval.boundary_at_or_below(mined_height)) - interval_blocks, + "the change note must be younger than the age-1 boundary for this scenario to be meaningful" + ); + + // The bucketed attempt cannot fund from a note that young, so the payment falls back rather + // than waiting for the boundary the change will eventually be old enough for. + let second = propose(&mut st).expect("the change must be spendable immediately"); + let step = second.steps().first(); + assert!( + !interval.is_boundary( + step.anchor_height() + .expect("a shielded step binds an anchor") + ), + "a wallet with no note old enough for the boundary must fall back to the ordinary anchor" + ); + + let txids = st.create_proposed_expecting(&second, 1); + let tx = st.get_tx_from_history(txids[0]).unwrap().unwrap(); + assert_ne!( + tx.expiry_height(), + Some(zcash_protocol::zip318::expiry_height(BlockHeight::from( + second.min_target_height() + ))), + "and with the ordinary anchor, the ordinary expiry: the two travel together" + ); } /// A canonical payment is funded from the single oldest covering note even when accumulation diff --git a/librustzcash/zcash_client_backend/src/data_api/wallet.rs b/librustzcash/zcash_client_backend/src/data_api/wallet.rs index dc030352..00ae44ed 100644 --- a/librustzcash/zcash_client_backend/src/data_api/wallet.rs +++ b/librustzcash/zcash_client_backend/src/data_api/wallet.rs @@ -838,12 +838,24 @@ where #[cfg(feature = "orchard")] let canonical_fee = crate::fees::canonical_crossing_fee(params, target_height.into()).ok(); - // The whole attempt is Orchard-gated: without that feature there is no Ironwood pool to cross - // into, so there is no canonical crossing to construct. + // The whole attempt is Orchard-gated: without that feature there is no Orchard pool to anchor + // against, and no Ironwood pool to cross into. + // + // The bucketed anchor is NOT reserved for a canonical crossing. EVERY step that spends Orchard + // notes is proved against a boundary of the anchor bucket grid, so that its anchor joins the + // cohort of transactions sharing that boundary instead of pinning the block this wallet + // happened to be synced to when it proved. An anchor is one of only two observables an + // ordinary payment can share with a migration transfer at no cost beyond confirmations; the + // rolling expiry is the other. + // + // The crossing's REMAINING observables are deliberately not widened with it. Its denomination, + // its bundle padding and its fee are what make a crossing indistinguishable from a migration + // transfer, and an ordinary payment cannot wear that disguise: it pays an arbitrary amount to + // someone else. Adopting half of the shape would be a fingerprint rather than a disguise (see + // `Step::is_canonical_crossing`), so those stay gated on the canonical attempt below. #[cfg(feature = "orchard")] - let bucketed_policy = canonical_crossing_candidate(params, &zip318, &request, target_height) - .then(|| params.activation_height(NetworkUpgrade::Nu6_3)) - .flatten() + let bucketed_policy = params + .activation_height(NetworkUpgrade::Nu6_3) .and_then(|activation| { confirmations_policy.bucketed( zip318.anchor_bucket_interval(), @@ -851,8 +863,8 @@ where activation, ) }) - // A canonical crossing spends an Orchard note; if the caller forbids that, there is no - // canonical path to attempt. + // Bucketing buys anonymity for an Orchard anchor; if the caller forbids spending Orchard + // notes, there is no anchor to bucket. .filter(|_| spend_policy.permits_shielded(ShieldedPool::Orchard)); // An anchor must be COMPUTABLE at the chosen boundary, not merely arithmetically valid: the @@ -873,33 +885,39 @@ where None => None, }; + // The canonical crossing attempt runs only for a request whose shape could reach it: a single + // payment of a canonical denomination. Every other Orchard-spending request skips straight to + // the bucketed attempt below, which asks only for the anchor. #[cfg(feature = "orchard")] - let canonical_attempt = bucketed_policy.map(|bucketed_policy| { - // Single-note funding is PREFERRED, not merely hoped for: a migration transfer - // spends exactly one note, and accumulation reaches the target through several - // small notes whenever the oldest notes are small — funding perfectly well while - // losing the canonical shape. Preferring the oldest single covering note makes the - // canonical outcome the common one; when no single note covers the payment, the - // fallback accumulation funds it and the shape check below discards the attempt, - // exactly as before. - let orchard_only = input_selection::SpendPolicy::shielded_pools([ShieldedPool::Orchard]) - .with_locked_input_policy(spend_policy.locked_input_policy().clone()) - .with_note_selection(input_selection::NoteSelection::PreferSingle); - - input_selector.propose_transaction( - params, - wallet_db, - target_height, - bucketed_policy.anchor_height(target_height), - &zip318, - bucketed_policy, - spend_from_account, - request.clone(), - change_strategy, - &orchard_only, - proposed_version, - ) - }); + let canonical_attempt = bucketed_policy + .filter(|_| canonical_crossing_candidate(params, &zip318, &request, target_height)) + .map(|bucketed_policy| { + // Single-note funding is PREFERRED, not merely hoped for: a migration transfer + // spends exactly one note, and accumulation reaches the target through several + // small notes whenever the oldest notes are small — funding perfectly well while + // losing the canonical shape. Preferring the oldest single covering note makes the + // canonical outcome the common one; when no single note covers the payment, the + // fallback accumulation funds it and the shape check below discards the attempt, + // exactly as before. + let orchard_only = + input_selection::SpendPolicy::shielded_pools([ShieldedPool::Orchard]) + .with_locked_input_policy(spend_policy.locked_input_policy().clone()) + .with_note_selection(input_selection::NoteSelection::PreferSingle); + + input_selector.propose_transaction( + params, + wallet_db, + target_height, + bucketed_policy.anchor_height(target_height), + &zip318, + bucketed_policy, + spend_from_account, + request.clone(), + change_strategy, + &orchard_only, + proposed_version, + ) + }); // Only two outcomes justify falling back to an ordinary proposal: the wallet cannot fund the // payment under the stricter policy, or it funded one that is not in fact canonical. Every @@ -924,10 +942,47 @@ where Some(Err(other)) => return Err(other.into()), }; + // A request that is not a canonical crossing still spends Orchard notes, and its ANCHOR is + // the observable worth sharing: proposed against the same boundary, with the caller's own + // spend policy and note selection, so that nothing but the anchor — and the confirmations + // that anchor implies — differs from what the ordinary attempt would have produced. + #[cfg(feature = "orchard")] + let bucketed_proposal = match canonical_proposal { + Some(proposal) => Some(proposal), + None => match bucketed_policy { + Some(bucketed_policy) => match input_selector.propose_transaction( + params, + wallet_db, + target_height, + bucketed_policy.anchor_height(target_height), + &zip318, + bucketed_policy, + spend_from_account, + request.clone(), + change_strategy, + spend_policy, + proposed_version, + ) { + // Kept only when the selection actually reached for Orchard notes. A proposal + // funded entirely from another pool has no Orchard anchor to hide, and would have + // paid up to one grid interval of extra confirmations on its inputs for nothing. + Ok(proposal) => { + (proposal.input_count_in_pool(PoolType::ORCHARD) > 0).then_some(proposal) + } + // The wallet cannot fund the payment from notes old enough for the boundary. The + // ordinary proposal below is the fallback, exactly as for a missed crossing: an + // anchor is worth confirmations, never a payment that cannot be made at all. + Err(InputSelectorError::InsufficientFunds { .. }) => None, + Err(other) => return Err(other.into()), + }, + None => None, + }, + }; + #[cfg(not(feature = "orchard"))] - let canonical_proposal = None; + let bucketed_proposal = None; - let proposal = match canonical_proposal { + let proposal = match bucketed_proposal { Some(proposal) => proposal, None => input_selector.propose_transaction( params, @@ -979,25 +1034,28 @@ fn canonical_crossing_candidate( } } -/// Returns whether `step` will be built as a canonical ZIP 318 crossing, under the ZIP 318 -/// parameters `wallet_db` reports and the fee the canonical shape costs at `target_height`. +/// Returns whether `step` is proved against a boundary of the anchor bucket grid `wallet_db` +/// retains, which is the condition for taking the ZIP 318 rolling expiry. /// -/// Shared by every path that builds a proposal, so that each asks the same question of the same -/// data. `create_pczt_from_proposal` in particular applies its expiry override after the builder -/// has run, and so cannot rely on the check inside `build_proposed_transaction`. +/// This is the anchor clause of [`Step::is_canonical_crossing`] on its own. A canonical crossing +/// satisfies it, and so does every other step `propose_transfer` succeeded in bucketing; the rest +/// of the crossing's shape decides only the Ironwood bundle's padding, never the expiry. +/// +/// The test is made against the step's own anchor rather than against the policy that produced it, +/// because bucketing is attempted and may be abandoned: a step whose anchor is not on the grid +/// took the ordinary anchor, and an expiry no other transaction in its period shares would then +/// single it out just as surely as a unique anchor would. #[cfg(feature = "orchard")] -fn step_is_canonical_crossing( - wallet_db: &DbT, - params: &ParamsT, - step: &Step, - target_height: TargetHeight, -) -> bool +fn step_is_boundary_anchored(wallet_db: &DbT, step: &Step) -> bool where DbT: WalletRead, - ParamsT: consensus::Parameters, { - crate::fees::canonical_crossing_fee(params, target_height.into()) - .is_ok_and(|fee| step.is_canonical_crossing(&wallet_db.pool_migration_params(), fee)) + step.anchor_height().is_some_and(|anchor| { + wallet_db + .pool_migration_params() + .anchor_bucket_interval() + .is_boundary(anchor) + }) } /// Proposes making a payment to the specified address from the given account. @@ -1929,18 +1987,21 @@ where ironwood_padding, }, ); - // A canonical crossing takes the ZIP 318 rolling expiry, which every crossing in the same - // modulus period shares. The builder's ordinary per-transaction expiry (target height plus a - // small delta) would single it out immediately, undoing the shape the unpadded bundle and the - // bucketed anchor were chosen to produce. A caller-supplied expiry is refused rather than - // silently overridden: the padding and anchor are already fixed by this point, so honouring it - // would emit a transaction that is canonical in every respect but one. + // A boundary-anchored step takes the ZIP 318 rolling expiry, which every transaction in the + // same modulus period shares. The builder's ordinary per-transaction expiry (target height + // plus a small delta) would single it out immediately, undoing the anonymity the bucketed + // anchor was chosen to produce. A caller-supplied expiry is refused rather than silently + // overridden: the anchor is already fixed by this point, so honouring it would emit a + // transaction that is canonical in every respect but one. + // + // The condition is the ANCHOR, not the canonical crossing shape: this fork buckets the anchor + // of every Orchard-spending transaction, and the two observables travel together. #[cfg(feature = "orchard")] let expiry_height = { - if step_is_canonical_crossing(wallet_db, params, proposal_step, min_target_height) { + if step_is_boundary_anchored(wallet_db, proposal_step) { match expiry_height { Some(requested) => { - return Err(Error::ExpiryHeightConflictsWithCanonicalCrossing { requested }); + return Err(Error::ExpiryHeightConflictsWithBoundaryAnchor { requested }); } None => Some(zcash_protocol::zip318::expiry_height( min_target_height.into(), @@ -2945,9 +3006,9 @@ where // one that is committed and publicly visible. #[cfg(feature = "orchard")] if let Some(requested) = expiry_height - && step_is_canonical_crossing(wallet_db, params, proposal_step, min_target_height) + && step_is_boundary_anchored(wallet_db, proposal_step) { - return Err(Error::ExpiryHeightConflictsWithCanonicalCrossing { requested }); + return Err(Error::ExpiryHeightConflictsWithBoundaryAnchor { requested }); } // Build the transaction with the specified fee rule diff --git a/librustzcash/zcash_client_sqlite/src/testing/pool.rs b/librustzcash/zcash_client_sqlite/src/testing/pool.rs index d3b70a47..dd91d783 100644 --- a/librustzcash/zcash_client_sqlite/src/testing/pool.rs +++ b/librustzcash/zcash_client_sqlite/src/testing/pool.rs @@ -405,8 +405,16 @@ pub(crate) fn canonical_crossing_prefers_single_note() { } #[cfg(feature = "orchard")] -pub(crate) fn multi_note_crossing_is_not_bucketed() { - zcash_client_backend::data_api::testing::pool::multi_note_crossing_is_not_bucketed( +pub(crate) fn orchard_payment_falls_back_when_notes_are_too_new() { + zcash_client_backend::data_api::testing::pool::orchard_payment_falls_back_when_notes_are_too_new( + TestDbFactory::default(), + BlockCache::new(), + ) +} + +#[cfg(feature = "orchard")] +pub(crate) fn multi_note_crossing_is_bucketed_but_not_canonical() { + zcash_client_backend::data_api::testing::pool::multi_note_crossing_is_bucketed_but_not_canonical( TestDbFactory::default(), BlockCache::new(), ) diff --git a/librustzcash/zcash_client_sqlite/src/wallet/orchard.rs b/librustzcash/zcash_client_sqlite/src/wallet/orchard.rs index d1492edd..be20e7bf 100644 --- a/librustzcash/zcash_client_sqlite/src/wallet/orchard.rs +++ b/librustzcash/zcash_client_sqlite/src/wallet/orchard.rs @@ -993,8 +993,13 @@ pub(crate) mod tests { } #[test] - fn multi_note_crossing_is_not_bucketed() { - testing::pool::multi_note_crossing_is_not_bucketed() + fn multi_note_crossing_is_bucketed_but_not_canonical() { + testing::pool::multi_note_crossing_is_bucketed_but_not_canonical() + } + + #[test] + fn orchard_payment_falls_back_when_notes_are_too_new() { + testing::pool::orchard_payment_falls_back_when_notes_are_too_new() } #[test] From a7c4f6da0a119765edec1553af8d5504055ec0e1 Mon Sep 17 00:00:00 2001 From: Mark Henderson Date: Tue, 18 Aug 2026 12:27:49 -0400 Subject: [PATCH 2/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- librustzcash/zcash_client_backend/src/data_api/wallet.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/librustzcash/zcash_client_backend/src/data_api/wallet.rs b/librustzcash/zcash_client_backend/src/data_api/wallet.rs index 00ae44ed..f27042ac 100644 --- a/librustzcash/zcash_client_backend/src/data_api/wallet.rs +++ b/librustzcash/zcash_client_backend/src/data_api/wallet.rs @@ -1991,8 +1991,8 @@ where // same modulus period shares. The builder's ordinary per-transaction expiry (target height // plus a small delta) would single it out immediately, undoing the anonymity the bucketed // anchor was chosen to produce. A caller-supplied expiry is refused rather than silently - // overridden: the anchor is already fixed by this point, so honouring it would emit a - // transaction that is canonical in every respect but one. + // overridden: the anchor is already fixed by this point, so honouring it would make the + // transaction's expiry unique within its shared-anchor cohort. // // The condition is the ANCHOR, not the canonical crossing shape: this fork buckets the anchor // of every Orchard-spending transaction, and the two observables travel together. From 9897f90bcf36489cf2b51ea6d9d4cf39503c751a Mon Sep 17 00:00:00 2001 From: Mark Henderson Date: Tue, 18 Aug 2026 12:27:56 -0400 Subject: [PATCH 3/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- librustzcash/zcash_client_backend/src/data_api/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/librustzcash/zcash_client_backend/src/data_api/error.rs b/librustzcash/zcash_client_backend/src/data_api/error.rs index aea879f3..02d1c6b1 100644 --- a/librustzcash/zcash_client_backend/src/data_api/error.rs +++ b/librustzcash/zcash_client_backend/src/data_api/error.rs @@ -120,7 +120,7 @@ pub enum Error Date: Thu, 20 Aug 2026 23:27:08 -0400 Subject: [PATCH 4/4] [zero] feat(librustzcash): draw the ZIP 318 anchor age, and a uniform anchor when none is admissible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bucketed anchor was always taken at age 1, the newest admissible boundary, because that is what `ConfirmationsPolicy::bucketed` offered. ZIP 318 instead draws an age in [1, ANCHOR_AGE_CAP] from a recency-weighted Geometric(1/2) distribution, which is what `zcash_pool_migration` does for a migration transfer. A payment that always took age 1 would be distinguishable from those transfers by the one observable this fork set out to share, so draw the age here too. The draw needs the newest note the transaction spends, which nothing knows before input selection has run. The ordinary proposal is therefore built first and serves three purposes: it bounds the draw, it is re-proposed at the drawn anchor, and it remains the fallback. Because every note it selected is at or below that bound, the same notes stay eligible at the drawn boundary. When no boundary is admissible — every candidate predates a note the transaction spends — ZIP 318 draws a uniform height between that note and the target instead of waiting for the next boundary to settle. A uniform anchor is shared with nobody, but the alternative on this path is the ordinary anchor, which is a fixed offset from the chain tip and so times the transaction to the block it was created in; a draw reveals only a lower bound on that height. That fallback can reach two grid intervals back, which the commitment trees' checkpoint budget must cover, so the budget is separated from the wallet's rewind bound: they answer different questions, and only the budget decides which historical heights a note can still be witnessed against. The rewind bound stays at 100, leaving scan-range planning and rewind behavior unchanged. Co-Authored-By: Claude Opus 5 --- .../zcash_client_backend/CHANGELOG.md | 29 +- .../src/data_api/anchor_retention.rs | 17 + .../src/data_api/testing/pool.rs | 134 +++--- .../src/data_api/wallet.rs | 392 +++++++++++++++--- librustzcash/zcash_client_sqlite/src/lib.rs | 8 +- .../zcash_client_sqlite/src/testing/pool.rs | 10 +- 6 files changed, 454 insertions(+), 136 deletions(-) diff --git a/librustzcash/zcash_client_backend/CHANGELOG.md b/librustzcash/zcash_client_backend/CHANGELOG.md index f3245d4f..71bb796e 100644 --- a/librustzcash/zcash_client_backend/CHANGELOG.md +++ b/librustzcash/zcash_client_backend/CHANGELOG.md @@ -10,18 +10,33 @@ workspace. ## [Unreleased] +### Added - Zero fork +- `zcash_client_backend::data_api::wallet::ConfirmationsPolicy::bucketed_at_age` +- `zcash_client_backend::data_api::wallet::ConfirmationsPolicy::anchored_at` +- `zcash_client_backend::data_api::anchor_retention::CHECKPOINT_RETENTION_DEPTH` + ### Changed - Zero fork -- `zcash_client_backend::data_api::wallet::propose_transfer` now proves EVERY - proposal that spends Orchard notes against a boundary of the wallet's anchor - bucket grid, not only a canonical ZIP 318 crossing. When no boundary is - reachable, or the wallet cannot fund the payment from notes old enough for - one, the proposal falls back to the ordinary anchor as before. The remaining +- `zcash_client_backend::data_api::wallet::propose_transfer` now draws a ZIP 318 + anchor for EVERY proposal that spends Orchard notes, not only for a canonical + ZIP 318 crossing. The anchor is a boundary of the wallet's anchor bucket grid + at an age drawn from the ZIP 318 recency-weighted distribution when one is + admissible, and a uniformly drawn height between the newest note the proposal + spends and the target height when none is. When the drawn anchor is not + computable, or the wallet cannot fund the payment from notes old enough for + it, the proposal falls back to the ordinary anchor as before. The remaining ZIP 318 crossing properties (canonical denomination, Ironwood bundle padding, canonical fee) are unchanged and still apply only to a canonical crossing. - Callers should expect an Orchard-spending payment to require up to one grid - interval of additional confirmations on its inputs. + Callers should expect an Orchard-spending payment to require up to four grid + intervals of additional confirmations on its inputs. - Any step proved against a bucket boundary now takes the ZIP 318 rolling expiry, where previously only a canonical crossing did. +- `ConfirmationsPolicy::bucketed` now derives the most recent boundary from the + latest observed block rather than from the anchor its own confirmation + requirement implies, so that wallets with different confirmation requirements + agree on the grid. +- The note commitment trees now retain `CHECKPOINT_RETENTION_DEPTH` ordinary + checkpoints rather than as many as the backend's rewind bound allows, so that + a uniformly drawn fallback anchor remains witnessable. - `zcash_client_backend::data_api::error::Error::ExpiryHeightConflictsWithCanonicalCrossing` has been renamed to `ExpiryHeightConflictsWithBoundaryAnchor`, and is now returned for any boundary-anchored step rather than only a canonical crossing. diff --git a/librustzcash/zcash_client_backend/src/data_api/anchor_retention.rs b/librustzcash/zcash_client_backend/src/data_api/anchor_retention.rs index 8a4b9393..877c76bf 100644 --- a/librustzcash/zcash_client_backend/src/data_api/anchor_retention.rs +++ b/librustzcash/zcash_client_backend/src/data_api/anchor_retention.rs @@ -32,6 +32,23 @@ use zcash_protocol::consensus::BlockHeight; /// which is `AnchorBucketInterval::ZIP_318`. pub use zcash_protocol::zip318::AnchorBucketInterval as AnchorRetentionInterval; +/// The number of ORDINARY checkpoints a wallet's note commitment trees retain, beyond which the +/// oldest are pruned. Checkpoints retained as durable anchors under an [`AnchorRetention`] policy +/// are exempt from this budget. +/// +/// This answers a different question from a backend's rewind bound, which governs how far the +/// wallet may roll back and how far scanning re-verifies; this one governs which historical heights +/// a note can still be witnessed against, and is deeper. The [ZIP 318] FALLBACK anchor is drawn +/// uniformly from a window this depth must cover, and that window reaches back at most two anchor +/// bucket intervals: the fallback is taken only when the newest note a transaction spends postdates +/// the newest candidate boundary, which places it within one interval of the most recent boundary, +/// itself within one interval of the chain tip. At [`AnchorRetentionInterval::ZIP_318`] that is 288 +/// blocks; an anchor drawn at a height whose checkpoint had been pruned could not be witnessed +/// against. +/// +/// [ZIP 318]: https://zips.z.cash/zip-0318 +pub const CHECKPOINT_RETENTION_DEPTH: u32 = 300; + /// The anchor-retention policy in force while a range of blocks is being added to the wallet: the /// set of grids whose boundaries are retained, and the height from which retention applies. /// diff --git a/librustzcash/zcash_client_backend/src/data_api/testing/pool.rs b/librustzcash/zcash_client_backend/src/data_api/testing/pool.rs index 8505b1e4..c7270232 100644 --- a/librustzcash/zcash_client_backend/src/data_api/testing/pool.rs +++ b/librustzcash/zcash_client_backend/src/data_api/testing/pool.rs @@ -59,8 +59,8 @@ use crate::{ use super::{DataStoreFactory, Reset, TestCache, TestFvk, TestState}; -use crate::data_api::ll::wallet::PRUNING_DEPTH; use crate::data_api::wallet::input_selection::GreedyInputSelectorError; +use crate::data_api::{anchor_retention::CHECKPOINT_RETENTION_DEPTH, ll::wallet::PRUNING_DEPTH}; use crate::{ data_api::BlockMetadata, scanning::{ @@ -4094,13 +4094,13 @@ where /// A wallet-level test for note-commitment-tree *anchor retention*: once NU6.3 (Ironwood) is /// active, checkpoints on the anchor-retention interval are retained as durable anchors, exempt -/// from the ordinary `PRUNING_DEPTH`-checkpoint pruning budget, so that their roots and the +/// from the ordinary `CHECKPOINT_RETENTION_DEPTH`-checkpoint pruning budget, so that their roots and the /// witnesses anchored to them remain computable even after they age far behind the chain tip. /// /// The interval is supplied by the caller (the pruning depth is read from /// `crate::data_api::ll::wallet`, so this test tracks whatever value the implementation defines). /// Passing a short interval keeps the test cheap: the scan must reach a boundary more than -/// `PRUNING_DEPTH` checkpoints behind the tip, which at the ZIP 318 interval means generating +/// `CHECKPOINT_RETENTION_DEPTH` checkpoints behind the tip, which at the ZIP 318 interval means generating /// several hundred blocks. /// /// The test: @@ -4108,13 +4108,13 @@ where /// floor equals the account birthday. /// - Receives a single note early, capturing its note-commitment-tree position. /// - Scans forward in a single batch until an interval-aligned anchor has aged *more than -/// `PRUNING_DEPTH` checkpoints* behind the chain tip — so it would have been pruned to enforce +/// `CHECKPOINT_RETENTION_DEPTH` checkpoints* behind the chain tip — so it would have been pruned to enforce /// the checkpoint budget had it not been retained. /// - Proves survival behaviorally: a witness for the received note *as of that buried anchor* is /// still constructible (it would be `None` if the anchor checkpoint had been pruned). /// - Confirms the anchors did not consume the pruning budget: the ordinary checkpoint immediately /// above the buried anchor *was* pruned, exactly the interval-aligned anchors at/above the floor -/// are retained, and the full `PRUNING_DEPTH` window of checkpoints at the chain tip survives. +/// are retained, and the full `CHECKPOINT_RETENTION_DEPTH` window of checkpoints at the chain tip survives. pub fn anchor_checkpoints_retained_across_deep_scan< T: ShieldedPoolTester, Dsf: DataStoreFactory, @@ -4159,9 +4159,13 @@ pub fn anchor_checkpoints_retained_across_deep_scan< anchor += interval_blocks; } - // Scan forward in a single batch so the anchor ages more than `PRUNING_DEPTH` checkpoints - // behind the tip: without retention it would be pruned to enforce the checkpoint budget. - let tip = anchor + PRUNING_DEPTH + 10; + // Scan forward in a single batch so the anchor ages more than `CHECKPOINT_RETENTION_DEPTH` + // ORDINARY checkpoints behind the tip: without retention it would be pruned to enforce the + // checkpoint budget. The window is sized in ordinary blocks, because the boundaries within it + // are themselves retained and so consume none of that budget; a window sized in plain blocks + // would leave the wallet under budget at the shorter intervals and prune nothing at all. + let ordinary_budget = CHECKPOINT_RETENTION_DEPTH + 10; + let tip = anchor + ordinary_budget + 1 + ordinary_budget / interval_blocks; // Fillers pay a non-wallet key, so each block still adds a commitment (and thus a checkpoint) // without changing the received note's position or the wallet's spendable set. @@ -4237,9 +4241,9 @@ pub fn anchor_checkpoints_retained_across_deep_scan< "the ordinary checkpoint just above the buried anchor must have been pruned", ); - // The anchors did not consume the pruning budget: the full `PRUNING_DEPTH` window of + // The anchors did not consume the pruning budget: the full `CHECKPOINT_RETENTION_DEPTH` window of // checkpoints at the chain tip is still retained. - for h in (tip - PRUNING_DEPTH + 1)..=tip { + for h in (tip - CHECKPOINT_RETENTION_DEPTH + 1)..=tip { assert!( survivors.contains(&BlockHeight::from(h)), "checkpoint at tip-window height {h} must be retained", @@ -5005,7 +5009,7 @@ where // 1. Set up test environment with account // 2. Generate and scan initial blocks to populate the note commitment tree // 3. Capture the chain state at a specific height - // 4. Generate and scan blocks beyond PRUNING_DEPTH to ensure early checkpoints are pruned + // 4. Generate and scan blocks beyond CHECKPOINT_RETENTION_DEPTH to ensure early checkpoints are pruned // 5. Verify that normal truncate_to_height fails due to missing checkpoints // 6. Test that truncate_to_chain_state succeeds using the captured chain state // 7. Verify wallet state after truncation @@ -5054,9 +5058,9 @@ where .clone(); assert_eq!(captured_chain_state.block_height(), capture_height); - // Step 4: Generate and scan blocks well beyond PRUNING_DEPTH so that the checkpoint + // Step 4: Generate and scan blocks well beyond CHECKPOINT_RETENTION_DEPTH so that the checkpoint // at capture_height is pruned from the note commitment tree. - let extra_blocks = PRUNING_DEPTH + 10; + let extra_blocks = CHECKPOINT_RETENTION_DEPTH + 10; for _ in 0..extra_blocks { st.generate_next_block( &other_fvk, @@ -5072,7 +5076,7 @@ where .unwrap() .expect("chain tip should be set"); assert!( - tip >= capture_height + PRUNING_DEPTH, + tip >= capture_height + CHECKPOINT_RETENTION_DEPTH, "tip should be beyond pruning depth from capture height" ); @@ -5185,9 +5189,9 @@ pub fn truncate_to_chain_state_below_birthday( } st.scan_cached_blocks(birthday_height, 5); - // Generate and scan blocks well beyond PRUNING_DEPTH to ensure early checkpoints + // Generate and scan blocks well beyond CHECKPOINT_RETENTION_DEPTH to ensure early checkpoints // are pruned from the note commitment tree. - let extra_blocks = PRUNING_DEPTH + 10; + let extra_blocks = CHECKPOINT_RETENTION_DEPTH + 10; for _ in 0..extra_blocks { st.generate_next_block( &other_fvk, @@ -5240,7 +5244,7 @@ pub fn truncate_to_chain_state_above_scanned( let birthday_height = st.test_account().unwrap().birthday().height(); - // Generate and scan initial blocks, then scan beyond PRUNING_DEPTH to ensure + // Generate and scan initial blocks, then scan beyond CHECKPOINT_RETENTION_DEPTH to ensure // early checkpoints are pruned. let other_fvk = T::random_fvk(st.rng_mut()); let initial_blocks = 5u32; @@ -5253,7 +5257,7 @@ pub fn truncate_to_chain_state_above_scanned( } st.scan_cached_blocks(birthday_height, initial_blocks as usize); - let extra_blocks = PRUNING_DEPTH + 10; + let extra_blocks = CHECKPOINT_RETENTION_DEPTH + 10; for _ in 0..extra_blocks { st.generate_next_block( &other_fvk, @@ -5389,9 +5393,10 @@ pub fn rewind_to_chain_state_deep( // The rewind target is the tip of the initial range. let rewind_target = sapling_activation + initial_block_count - 1; - // Scan more than PRUNING_DEPTH extra blocks so that the checkpoint at rewind_target is pruned - // AND rewind_target is below `tip - PRUNING_DEPTH`. - let extra_blocks = PRUNING_DEPTH + 10; + // Scan enough extra blocks that the checkpoint at rewind_target is pruned from the commitment + // tree — which takes more than `CHECKPOINT_RETENTION_DEPTH`, the deeper of the two bounds — and + // that rewind_target is below the wallet's rewind floor at `tip - PRUNING_DEPTH`. + let extra_blocks = CHECKPOINT_RETENTION_DEPTH + 10; for _ in 0..extra_blocks { st.generate_next_block( &other_fvk, @@ -5676,7 +5681,7 @@ where // so `mark_stabilized_notes` has the `subtree_end_height` it needs to flip // the shard 2 notes' `witness_stabilized` flag once the pruning floor rises // above the shard. - // 4. Scan `PRUNING_DEPTH + 10` one-output post-note blocks. They land in shard + // 4. Scan `CHECKPOINT_RETENTION_DEPTH + 10` one-output post-note blocks. They land in shard // 3 (positions 196608+), pushing the pruning-floor checkpoint's tree // position into shard 3 so `shardtree::truncate_shards(3)` — invoked by the // upcoming rewind — preserves shard 2 and every row it indexes. @@ -5825,10 +5830,10 @@ where ) .unwrap(); - // Step 4: scan more than `PRUNING_DEPTH` blocks past the note-filled block + // Step 4: scan more than `CHECKPOINT_RETENTION_DEPTH` blocks past the note-filled block // into shard 3, so the rewind's truncation position is in shard 3 and the // ensuing `truncate_shards(3)` leaves shard 2 intact. - let extra_blocks = PRUNING_DEPTH + 10; + let extra_blocks = CHECKPOINT_RETENTION_DEPTH + 10; for _ in 0..extra_blocks { st.generate_next_block(¬_our_key, AddressType::DefaultExternal, filler_value); } @@ -5912,7 +5917,7 @@ where // finishes shard 1 and fills shard 2, with three A-owned outputs // and three outputs for a not-yet-imported account B, non-wallet // filler elsewhere; - // (c) generate `PRUNING_DEPTH + 10` filler blocks past the note block + // (c) generate `CHECKPOINT_RETENTION_DEPTH + 10` filler blocks past the note block // to push the pruning floor past shard 2; // (d) scan the note block, declare shard 2 complete via // `put_subtree_roots`, then scan the filler blocks — the @@ -6080,7 +6085,7 @@ where // Step 1c: filler blocks past the note block, sized to put the pruning // floor past shard 2's end height in step 1d. - let extra_blocks = PRUNING_DEPTH + 10; + let extra_blocks = CHECKPOINT_RETENTION_DEPTH + 10; for _ in 0..extra_blocks { st.generate_next_block(¬_our_key, AddressType::DefaultExternal, filler_value); } @@ -9125,10 +9130,17 @@ pub fn canonical_crossing_is_bucketed_and_unpadded( "an Orchard-spending payment must anchor to a grid boundary even when its shape is not \ a canonical crossing" ); - assert_eq!( - step.anchor_height(), - canonical.steps().first().anchor_height(), - "and to the SAME boundary as the crossing: one cohort, not two" + // At an age the ZIP 318 draw admits, so that it joins one of the cohorts a migration transfer + // could have joined rather than forming a cohort of its own. The exact age is drawn, so this + // pins the admissible range rather than a single boundary. + let most_recent = interval.boundary_at_or_below(BlockHeight::from( + u32::from(off_by_one.min_target_height()) - 1, + )); + let age = (u32::from(most_recent) - u32::from(step.anchor_height().unwrap())) / interval_blocks; + assert!( + (1..=zcash_protocol::zip318::ANCHOR_AGE_CAP).contains(&age), + "the drawn anchor age {age} must lie in 1..={}", + zcash_protocol::zip318::ANCHOR_AGE_CAP ); // Building the canonical proposal is left until last: it spends the wallet's only note, so the @@ -9556,27 +9568,35 @@ pub fn orchard_payment_falls_back_when_notes_are_too_new( "the change note must be younger than the age-1 boundary for this scenario to be meaningful" ); - // The bucketed attempt cannot fund from a note that young, so the payment falls back rather - // than waiting for the boundary the change will eventually be old enough for. + // No boundary is admissible for a note that young, so the payment takes the uniform fallback + // anchor rather than waiting for the boundary the change will eventually be old enough for. let second = propose(&mut st).expect("the change must be spendable immediately"); let step = second.steps().first(); + let anchor = step + .anchor_height() + .expect("a shielded step binds an anchor"); + let most_recent_boundary = interval.boundary_at_or_below(BlockHeight::from_u32( + u32::from(second.min_target_height()) - 1, + )); + let newest_candidate = BlockHeight::from_u32(u32::from(most_recent_boundary) - interval_blocks); assert!( - !interval.is_boundary( - step.anchor_height() - .expect("a shielded step binds an anchor") - ), - "a wallet with no note old enough for the boundary must fall back to the ordinary anchor" + anchor > newest_candidate, + "an anchor at or below the newest candidate boundary {newest_candidate:?} would have \ + excluded the change note; the fallback must draw above it, got {anchor:?}" ); + // The expiry travels with the anchor either way: the ZIP 318 rolling window exactly when the + // drawn height lands on the grid, and the builder's ordinary expiry otherwise. A uniform draw + // may land on a boundary by chance, and the two observables must agree when it does. let txids = st.create_proposed_expecting(&second, 1); let tx = st.get_tx_from_history(txids[0]).unwrap().unwrap(); - assert_ne!( - tx.expiry_height(), - Some(zcash_protocol::zip318::expiry_height(BlockHeight::from( - second.min_target_height() - ))), - "and with the ordinary anchor, the ordinary expiry: the two travel together" - ); + let rolling = + zcash_protocol::zip318::expiry_height(BlockHeight::from(second.min_target_height())); + if interval.is_boundary(anchor) { + assert_eq!(tx.expiry_height(), Some(rolling)); + } else { + assert_ne!(tx.expiry_height(), Some(rolling)); + } } /// A canonical payment is funded from the single oldest covering note even when accumulation @@ -9815,20 +9835,26 @@ pub fn canonical_crossing_abandoned_without_anchor_checkpoint( "no anchor is computable at the boundary after removal" ); - // The payment now falls back to an ordinary crossing: proposed against the ordinary anchor, - // padded, and — decisively — BUILDABLE. Without the gate the canonical proposal would be + // The payment now abandons that boundary and is proposed against another anchor: whichever + // the draw yields whose checkpoint the wallet still holds, or the ordinary anchor if none + // does. Whether the result keeps the canonical shape is incidental — recovering it at a + // provable boundary is a better outcome, not a worse one. What is decisive is that the + // proposal is BUILDABLE: without the computability gate the unprovable boundary would be // kept and building would fail with `AnchorNotFound`. let fallback = propose(&mut st); let step = fallback.steps().first(); - assert!( - !step.is_canonical_crossing(&zip318, canonical_fee), - "the attempt must be abandoned when its anchor cannot be proved" - ); + let fallback_anchor = step + .anchor_height() + .expect("a shielded step binds an anchor"); assert_ne!( - step.anchor_height() - .expect("a shielded step binds an anchor"), - boundary, - "the fallback anchors at the ordinary height, not the unprovable boundary" + fallback_anchor, boundary, + "the fallback must not anchor at the unprovable boundary" + ); + assert!( + st.wallet() + .anchor_computable(ShieldedPool::Orchard, fallback_anchor) + .unwrap(), + "the fallback must anchor at a height the wallet can still prove" ); st.create_proposed_expecting(&fallback, 1); } diff --git a/librustzcash/zcash_client_backend/src/data_api/wallet.rs b/librustzcash/zcash_client_backend/src/data_api/wallet.rs index f27042ac..25f529e0 100644 --- a/librustzcash/zcash_client_backend/src/data_api/wallet.rs +++ b/librustzcash/zcash_client_backend/src/data_api/wallet.rs @@ -35,7 +35,7 @@ to a wallet-internal shielded address, as described in [ZIP 316](https://zips.z. //! [`propose_transfer`]: crate::data_api::wallet::propose_transfer use nonempty::NonEmpty; -use rand_core::OsRng; +use rand_core::{OsRng, RngCore}; use std::{ num::NonZeroU32, ops::{Add, Sub}, @@ -598,15 +598,15 @@ impl ConfirmationsPolicy { /// /// The boundary chosen is one interval BELOW the most recent one — an anchor age of 1, the /// smallest ZIP 318 admits. Anchoring to the most recent boundary would be an age of 0, which - /// no migration transfer uses. + /// no migration transfer uses. See [`Self::bucketed_at_age`] for any other age. /// /// `activation_height` is the activation of the pool being crossed into; the chosen boundary /// must lie strictly above it. /// - /// Returns `None` when no usable boundary is reachable: when the ordinary anchor lies below the - /// first boundary after `activation_height`, or where the required confirmations would reach - /// back past the genesis block. Reporting that as "unable to bucket" is what lets a caller fall - /// back, rather than build a proposal that is bound to fail. + /// Returns `None` when no usable boundary is reachable: when the most recent boundary lies + /// below the first boundary after `activation_height`, or where the required confirmations + /// would reach back past the genesis block. Reporting that as "unable to bucket" is what lets + /// a caller fall back, rather than build a proposal that is bound to fail. /// /// [ZIP 318]: https://zips.z.cash/zip-0318 pub fn bucketed( @@ -615,26 +615,46 @@ impl ConfirmationsPolicy { target_height: TargetHeight, activation_height: BlockHeight, ) -> Option { - // ZIP 318 draws an anchor of AGE `a` in `[1, ANCHOR_AGE_CAP]` boundaries behind the most - // recent one, so the chosen boundary is always strictly below it. Age 1 is taken here: the - // newest admissible boundary, and the modal age under the migration's `Geometric(1/2)` - // draw. Anchoring to the most recent boundary instead would be an age of 0, which no - // migration transfer ever uses. - let most_recent = interval.boundary_at_or_below(self.anchor_height(target_height)); - let boundary = BlockHeight::from_u32( - u32::from(most_recent).checked_sub(interval.block_count().get())?, - ); + self.bucketed_at_age(interval, target_height, activation_height, NonZeroU32::MIN) + } - // The boundary must lie strictly above the activation of the pool being crossed into. - // Rounding down can otherwise land on a PRE-ACTIVATION boundary in the window between - // activation and the first boundary after it, and ZIP 318's candidate set contains no such - // height — anchoring there would be distinguishable rather than shared. Height zero is - // excluded by the same bound, its note commitment tree being empty. - if boundary <= activation_height { - return None; - } - let bucketed = u32::from(target_height).checked_sub(u32::from(boundary))?; - let trusted = NonZeroU32::new(bucketed)?; + /// Returns this policy adjusted so that the shielded anchor for `target_height` is the + /// boundary of `interval` at anchor `age`: that many boundaries below the most recent one the + /// wallet has observed. Returns `None` when that boundary is not reachable (see + /// [`Self::bucketed`]). + /// + /// [ZIP 318] admits ages `1` through `ANCHOR_AGE_CAP`, drawn from a recency-weighted + /// `Geometric(1/2)` distribution; age `0` is excluded, so the boundary is always strictly + /// below the most recent one. + /// + /// [ZIP 318]: https://zips.z.cash/zip-0318 + pub fn bucketed_at_age( + &self, + interval: AnchorBucketInterval, + target_height: TargetHeight, + activation_height: BlockHeight, + age: NonZeroU32, + ) -> Option { + self.anchored_at( + target_height, + anchor_boundary_at_age(interval, target_height, activation_height, age)?, + ) + } + + /// Returns this policy adjusted so that the shielded anchor for `target_height` is exactly + /// `anchor`, or `None` if `anchor` is not below `target_height`. + /// + /// The adjustment is expressed as a RAISED CONFIRMATION REQUIREMENT rather than as a + /// separately lowered anchor, because the anchor and the bound on which notes may be spent are + /// the same quantity: `target_height` less the required confirmations. Moving that one number + /// moves both together, which is what makes it impossible to select a note having no witness + /// at the chosen anchor. Lowering the anchor on its own would leave two numbers free to drift. + /// + /// The untrusted requirement is raised to match when it would otherwise fall below the trusted + /// one, preserving this type's `trusted <= untrusted` invariant. + pub fn anchored_at(&self, target_height: TargetHeight, anchor: BlockHeight) -> Option { + let confirmations = u32::from(target_height).checked_sub(u32::from(anchor))?; + let trusted = NonZeroU32::new(confirmations)?; Self::new( trusted, core::cmp::max(self.untrusted(), trusted), @@ -736,6 +756,138 @@ impl ConfirmationsPolicy { } } +/// The boundary of `interval` at anchor `age` for a transaction targeting `target_height`: `age` +/// boundaries below the most recent boundary at or below the latest block the wallet has observed +/// (`target_height - 1`). +/// +/// Returns `None` when that boundary would fall at or below `activation_height`, or below the +/// genesis block. The boundary must lie strictly above the activation of the pool being crossed +/// into: rounding down can otherwise land on a PRE-ACTIVATION boundary in the window between +/// activation and the first boundary after it, and [ZIP 318]'s candidate set contains no such +/// height, so anchoring there would be distinguishable rather than shared. Height zero is excluded +/// by the same bound, its note commitment tree being empty. +/// +/// The grid is read from the latest OBSERVED block rather than from the anchor an ordinary +/// confirmation requirement would give, so that every wallet computes the same boundary from the +/// same chain tip regardless of how many confirmations it requires. Two wallets that disagreed +/// there would anchor to different boundaries and form two cohorts where there should be one. +/// +/// [ZIP 318]: https://zips.z.cash/zip-0318 +fn anchor_boundary_at_age( + interval: AnchorBucketInterval, + target_height: TargetHeight, + activation_height: BlockHeight, + age: NonZeroU32, +) -> Option { + let latest_observed = BlockHeight::from_u32(u32::from(target_height).checked_sub(1)?); + let most_recent = u32::from(interval.boundary_at_or_below(latest_observed)); + let boundary = BlockHeight::from_u32( + most_recent.checked_sub(age.get().checked_mul(interval.block_count().get())?)?, + ); + + (boundary > activation_height).then_some(boundary) +} + +/// The greatest [ZIP 318] anchor age available to a transaction targeting `target_height` that +/// spends notes mined no later than `min_anchor_height`, or `None` when no admissible boundary +/// exists. +/// +/// The candidate boundaries are those at or above `min_anchor_height` (every note the transaction +/// spends must exist in the anchor's tree state) and strictly above `activation_height`, and the +/// age is capped at [`ANCHOR_AGE_CAP`]. +/// +/// [ZIP 318]: https://zips.z.cash/zip-0318 +/// [`ANCHOR_AGE_CAP`]: zcash_protocol::zip318::ANCHOR_AGE_CAP +#[cfg(feature = "orchard")] +fn max_anchor_age( + interval: AnchorBucketInterval, + target_height: TargetHeight, + activation_height: BlockHeight, + min_anchor_height: BlockHeight, +) -> Option { + let latest_observed = BlockHeight::from_u32(u32::from(target_height).checked_sub(1)?); + let most_recent = u32::from(interval.boundary_at_or_below(latest_observed)); + let first_post_activation = u32::from(interval.boundary_at_or_above(activation_height + 1)); + let lowest_allowed = core::cmp::max(u32::from(min_anchor_height), first_post_activation); + let available = most_recent.checked_sub(lowest_allowed)? / interval.block_count().get(); + + NonZeroU32::new(core::cmp::min( + zcash_protocol::zip318::ANCHOR_AGE_CAP, + available, + )) +} + +/// Draws a [ZIP 318] anchor age in `[1, max_age]` from the recency-weighted `Geometric(1/2)` +/// distribution: `P(a) = 2^(max_age - a) / (2^max_age - 1)`, so the modal age is 1 and age 0 is +/// never produced. +/// +/// The draw is by rejection — an age beyond `max_age` is discarded and redrawn — which is what +/// conditions the geometric on the available range. Each bit of a fresh `u64` is one fair coin +/// flip. Acceptance probability is at least `1/2`, so the loop terminates promptly. +/// +/// [ZIP 318]: https://zips.z.cash/zip-0318 +#[cfg(feature = "orchard")] +fn draw_anchor_age(max_age: NonZeroU32, rng: &mut R) -> NonZeroU32 { + loop { + let mut age: u32 = 1; + let mut bits = rng.next_u64(); + for _ in 0..u64::BITS { + if bits & 1 == 1 { + break; + } + bits >>= 1; + age += 1; + } + + if let Some(age) = NonZeroU32::new(age) + && age <= max_age + { + return age; + } + } +} + +/// Draws a uniform anchor height in `[min_anchor_height, target_height - 1]`, or `None` if that +/// range is empty. +/// +/// This is the [ZIP 318] FALLBACK anchor, used only when no boundary is admissible: a wallet whose +/// notes are all younger than the newest candidate boundary would otherwise have to wait for the +/// next boundary to settle before it could spend at all. A uniform draw is shared with nobody, but +/// the alternative on this path is the ordinary anchor, which is a fixed offset from the chain tip +/// and so times the transaction to the block in which it was created. A draw reveals only a lower +/// bound on that height. +/// +/// [ZIP 318]: https://zips.z.cash/zip-0318 +#[cfg(feature = "orchard")] +fn draw_uniform_anchor( + target_height: TargetHeight, + min_anchor_height: BlockHeight, + rng: &mut R, +) -> Option { + // The span of `[min_anchor_height, target_height - 1]`, which is empty unless the minimum lies + // below the target. + let span = u64::from(u32::from(target_height).checked_sub(u32::from(min_anchor_height))?); + if span == 0 { + return None; + } + + // Rejection sampling over whole multiples of `span`, so that the modulo below is unbiased: the + // final partial block of the `u64` range is discarded rather than folded onto the low values. + let zone = (u64::MAX / span) * span; + let draw = loop { + let candidate = rng.next_u64(); + if candidate < zone { + break candidate % span; + } + }; + + Some( + min_anchor_height + + u32::try_from(draw) + .expect("a draw below `span` fits in the height it was taken from"), + ) +} + /// Select transaction inputs, compute fees, and construct a proposal for a transaction or series /// of transactions that can then be authorized and made ready for submission to the network with /// [`create_proposed_transactions`]. @@ -942,62 +1094,109 @@ where Some(Err(other)) => return Err(other.into()), }; - // A request that is not a canonical crossing still spends Orchard notes, and its ANCHOR is - // the observable worth sharing: proposed against the same boundary, with the caller's own - // spend policy and note selection, so that nothing but the anchor — and the confirmations - // that anchor implies — differs from what the ordinary attempt would have produced. + // A request that is not a canonical crossing still spends Orchard notes, and its ANCHOR is the + // observable worth sharing. The ORDINARY proposal is built first here, because the anchor + // depends on which notes are spent — the drawn anchor must be at or above the newest of them, + // and nothing before input selection knows what those are. That pass is not wasted: it is both + // the source of that bound and the fallback taken when no anchor can be drawn or funded. #[cfg(feature = "orchard")] - let bucketed_proposal = match canonical_proposal { - Some(proposal) => Some(proposal), - None => match bucketed_policy { - Some(bucketed_policy) => match input_selector.propose_transaction( + let proposal = match canonical_proposal { + Some(proposal) => proposal, + None => { + let ordinary = input_selector.propose_transaction( params, wallet_db, target_height, - bucketed_policy.anchor_height(target_height), + anchor_height, &zip318, - bucketed_policy, + confirmations_policy, spend_from_account, request.clone(), change_strategy, spend_policy, proposed_version, - ) { - // Kept only when the selection actually reached for Orchard notes. A proposal - // funded entirely from another pool has no Orchard anchor to hide, and would have - // paid up to one grid interval of extra confirmations on its inputs for nothing. - Ok(proposal) => { - (proposal.input_count_in_pool(PoolType::ORCHARD) > 0).then_some(proposal) + )?; + + let drawn = drawn_anchor_height(&ordinary, &zip318, params, target_height, &mut OsRng) + // Bucketing buys anonymity for an Orchard anchor; if the caller forbids spending + // Orchard notes, there is no anchor to hide. + .filter(|_| spend_policy.permits_shielded(ShieldedPool::Orchard)); + + // An anchor must be COMPUTABLE at the drawn height, not merely admissible: the data + // source must be able to produce the tree root there for every pool the transaction + // spends from, since one anchor serves them all. A wallet that scanned past NU6.3 + // activation before boundary checkpointing was repaired is permanently missing the + // boundaries whose blocks carried no shielded outputs, and a uniform fallback draw can + // land on any height at all. Abandoning the attempt here keeps the ordinary anchor, + // rather than proposing a transaction whose build must fail with `AnchorNotFound`. + let computable = match drawn { + Some(anchor) => { + let mut computable = true; + for (pool, protocol) in [ + (PoolType::SAPLING, ShieldedPool::Sapling), + (PoolType::ORCHARD, ShieldedPool::Orchard), + (PoolType::IRONWOOD, ShieldedPool::Ironwood), + ] { + if ordinary.input_count_in_pool(pool) > 0 + && !wallet_db + .anchor_computable(protocol, anchor) + .map_err(|e| Error::from(InputSelectorError::DataSource(e)))? + { + computable = false; + break; + } + } + computable.then_some(anchor) } - // The wallet cannot fund the payment from notes old enough for the boundary. The - // ordinary proposal below is the fallback, exactly as for a missed crossing: an - // anchor is worth confirmations, never a payment that cannot be made at all. - Err(InputSelectorError::InsufficientFunds { .. }) => None, - Err(other) => return Err(other.into()), - }, - None => None, - }, + None => None, + }; + + // Re-proposed at the drawn anchor with the caller's own spend policy and note + // selection, so that nothing but the anchor — and the confirmations that anchor + // implies — differs from the proposal above. + match computable + .and_then(|anchor| confirmations_policy.anchored_at(target_height, anchor)) + { + Some(policy) => match input_selector.propose_transaction( + params, + wallet_db, + target_height, + policy.anchor_height(target_height), + &zip318, + policy, + spend_from_account, + request, + change_strategy, + spend_policy, + proposed_version, + ) { + Ok(proposal) => proposal, + // The wallet cannot fund the payment from notes old enough for the drawn + // anchor. The ordinary proposal is the fallback, exactly as for a missed + // crossing: an anchor is worth confirmations, never a payment that cannot be + // made at all. + Err(InputSelectorError::InsufficientFunds { .. }) => ordinary, + Err(other) => return Err(other.into()), + }, + None => ordinary, + } + } }; #[cfg(not(feature = "orchard"))] - let bucketed_proposal = None; - - let proposal = match bucketed_proposal { - Some(proposal) => proposal, - None => input_selector.propose_transaction( - params, - wallet_db, - target_height, - anchor_height, - &zip318, - confirmations_policy, - spend_from_account, - request, - change_strategy, - spend_policy, - proposed_version, - )?, - }; + let proposal = input_selector.propose_transaction( + params, + wallet_db, + target_height, + anchor_height, + &zip318, + confirmations_policy, + spend_from_account, + request, + change_strategy, + spend_policy, + proposed_version, + )?; proposal.check_transaction_size()?; if let Some(request) = lock_inputs { let lock_expiry_height = target_height + request.for_blocks(); @@ -1034,6 +1233,65 @@ fn canonical_crossing_candidate( } } +/// The [ZIP 318] anchor height for `proposal`, drawn afresh for this transaction, or `None` when +/// the proposal has no Orchard anchor to hide or no anchor can be drawn for it. +/// +/// A boundary of the anchor bucket grid is drawn when one is admissible: at an age in +/// `[1, ANCHOR_AGE_CAP]` from the recency-weighted distribution ([`draw_anchor_age`]), among the +/// boundaries at or above the newest note the proposal spends and strictly above NU6.3 activation. +/// When no boundary is admissible — every candidate would precede a note the proposal spends — a +/// uniform height is drawn instead ([`draw_uniform_anchor`]), so that a wallet holding only recent +/// notes still spends immediately rather than waiting for the next boundary to settle. +/// +/// Returns `None` before NU6.3 activation, where there is no grid to share. +/// +/// [ZIP 318]: https://zips.z.cash/zip-0318 +/// [`ANCHOR_AGE_CAP`]: zcash_protocol::zip318::ANCHOR_AGE_CAP +#[cfg(feature = "orchard")] +fn drawn_anchor_height( + proposal: &Proposal, + zip318: &PoolMigrationParams, + params: &ParamsT, + target_height: TargetHeight, + rng: &mut R, +) -> Option +where + ParamsT: consensus::Parameters, + R: RngCore, +{ + if proposal.input_count_in_pool(PoolType::ORCHARD) == 0 { + return None; + } + + let activation = params.activation_height(NetworkUpgrade::Nu6_3)?; + if !params.is_nu_active(NetworkUpgrade::Nu6_3, target_height.into()) { + return None; + } + + // One anchor serves every shielded pool in the transaction, so it must be at or above the + // newest note the proposal spends in ANY of them. A note that is not yet mined has no height + // to compare, and could not be witnessed at a past anchor in any case. + let min_anchor_height = proposal + .steps() + .iter() + .filter_map(|step| step.shielded_inputs()) + .flat_map(|inputs| inputs.notes().iter()) + .try_fold(BlockHeight::from_u32(0), |newest, note| { + Some(core::cmp::max(newest, note.mined_height()?)) + })?; + + let interval = zip318.anchor_bucket_interval(); + match max_anchor_age(interval, target_height, activation, min_anchor_height) { + Some(max_age) => anchor_boundary_at_age( + interval, + target_height, + activation, + draw_anchor_age(max_age, rng), + ), + None => draw_uniform_anchor(target_height, min_anchor_height, rng), + } +} + /// Returns whether `step` is proved against a boundary of the anchor bucket grid `wallet_db` /// retains, which is the condition for taking the ZIP 318 rolling expiry. /// diff --git a/librustzcash/zcash_client_sqlite/src/lib.rs b/librustzcash/zcash_client_sqlite/src/lib.rs index 221d5d70..8993bd48 100644 --- a/librustzcash/zcash_client_sqlite/src/lib.rs +++ b/librustzcash/zcash_client_sqlite/src/lib.rs @@ -179,6 +179,8 @@ pub mod testing; /// this delta from the chain tip to be pruned. pub(crate) const PRUNING_DEPTH: u32 = 100; +pub(crate) use zcash_client_backend::data_api::anchor_retention::CHECKPOINT_RETENTION_DEPTH; + /// The number of blocks to verify ahead when the chain tip is updated. pub(crate) const VERIFY_LOOKAHEAD: u32 = 10; @@ -3096,7 +3098,7 @@ where Ok(ShardTree::new( SqliteShardStore::from_connection(conn, SAPLING_TABLES_PREFIX) .map_err(|e| ShardTreeError::Storage(commitment_tree::Error::Query(e)))?, - PRUNING_DEPTH.try_into().unwrap(), + CHECKPOINT_RETENTION_DEPTH.try_into().unwrap(), )) } @@ -3122,7 +3124,7 @@ where Ok(ShardTree::new( SqliteShardStore::from_connection(conn, ORCHARD_TABLES_PREFIX) .map_err(|e| ShardTreeError::Storage(commitment_tree::Error::Query(e)))?, - PRUNING_DEPTH.try_into().unwrap(), + CHECKPOINT_RETENTION_DEPTH.try_into().unwrap(), )) } @@ -3154,7 +3156,7 @@ where Ok(ShardTree::new( SqliteShardStore::from_connection(conn, IRONWOOD_TABLES_PREFIX) .map_err(|e| ShardTreeError::Storage(commitment_tree::Error::Query(e)))?, - PRUNING_DEPTH.try_into().unwrap(), + CHECKPOINT_RETENTION_DEPTH.try_into().unwrap(), )) } diff --git a/librustzcash/zcash_client_sqlite/src/testing/pool.rs b/librustzcash/zcash_client_sqlite/src/testing/pool.rs index dd91d783..4f941c19 100644 --- a/librustzcash/zcash_client_sqlite/src/testing/pool.rs +++ b/librustzcash/zcash_client_sqlite/src/testing/pool.rs @@ -11,7 +11,7 @@ use crate::{ }; use zcash_client_backend::data_api::{ WalletWrite, - anchor_retention::AnchorRetentionInterval, + anchor_retention::{AnchorRetentionInterval, CHECKPOINT_RETENTION_DEPTH}, chain::{ChainState, error::Error}, testing::{ AddressType, @@ -527,9 +527,9 @@ pub(crate) fn truncate_to_chain_state_above_scanned() { /// one captured from a second wallet that scanned the same number of blocks with different note /// values, so it has the same tree shape but conflicting node hashes. pub(crate) fn truncate_to_chain_state_commitment_tree_error() { - // `zcash_client_backend::data_api::ll::wallet::PRUNING_DEPTH` is crate-private; mirror it - // here. Scanning this far past the captured height guarantees its checkpoint is pruned. - const PRUNING_DEPTH: u32 = 100; + // Scanning past `CHECKPOINT_RETENTION_DEPTH` blocks guarantees the captured height's checkpoint + // is pruned: the commitment tree's checkpoint budget, not the wallet's shallower rewind bound, + // is what decides when a historical checkpoint is discarded. // Wallet A: scan blocks to populate the note commitment tree, capture a consistent chain // state, then scan well past the pruning depth so that the captured height's checkpoint is @@ -569,7 +569,7 @@ pub(crate) fn truncate_to_chain_state_commitment_tree_error