diff --git a/librustzcash/zcash_client_backend/CHANGELOG.md b/librustzcash/zcash_client_backend/CHANGELOG.md index 876a5fa7..71bb796e 100644 --- a/librustzcash/zcash_client_backend/CHANGELOG.md +++ b/librustzcash/zcash_client_backend/CHANGELOG.md @@ -10,6 +10,37 @@ 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 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 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. + ### 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/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/error.rs b/librustzcash/zcash_client_backend/src/data_api/error.rs index 9fb1ea19..02d1c6b1 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..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); } @@ -9101,7 +9106,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 +9126,21 @@ 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" + ); + // 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 @@ -9329,14 +9351,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 +9437,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 +9447,156 @@ 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" + ); + + // 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!( + 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(); + 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 @@ -9663,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 dc030352..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`]. @@ -838,12 +990,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 +1015,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 +1037,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,25 +1094,109 @@ where Some(Err(other)) => return Err(other.into()), }; - #[cfg(not(feature = "orchard"))] - let canonical_proposal = None; - + // 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 proposal = match canonical_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, - )?, + None => { + let ordinary = input_selector.propose_transaction( + params, + wallet_db, + target_height, + anchor_height, + &zip318, + confirmations_policy, + spend_from_account, + request.clone(), + change_strategy, + spend_policy, + proposed_version, + )?; + + 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) + } + 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 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(); @@ -979,25 +1233,87 @@ 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`. +/// 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. /// -/// 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`. +/// 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 step_is_canonical_crossing( - wallet_db: &DbT, +fn drawn_anchor_height( + proposal: &Proposal, + zip318: &PoolMigrationParams, params: &ParamsT, - step: &Step, target_height: TargetHeight, -) -> bool + rng: &mut R, +) -> Option where - DbT: WalletRead, ParamsT: consensus::Parameters, + R: RngCore, { - crate::fees::canonical_crossing_fee(params, target_height.into()) - .is_ok_and(|fee| step.is_canonical_crossing(&wallet_db.pool_migration_params(), fee)) + 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. +/// +/// 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_boundary_anchored(wallet_db: &DbT, step: &Step) -> bool +where + DbT: WalletRead, +{ + 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 +2245,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 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. #[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 +3264,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/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 d3b70a47..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, @@ -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(), ) @@ -519,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 @@ -561,7 +569,7 @@ pub(crate) fn truncate_to_chain_state_commitment_tree_error