Skip to content

[zero] librustzcash: bucket the anchor of every Orchard-spending transaction - #36

Open
aphelionz wants to merge 4 commits into
mainfrom
claude/orchard-anchor-policy-widening
Open

[zero] librustzcash: bucket the anchor of every Orchard-spending transaction#36
aphelionz wants to merge 4 commits into
mainfrom
claude/orchard-anchor-policy-widening

Conversation

@aphelionz

@aphelionz aphelionz commented Aug 18, 2026

Copy link
Copy Markdown
Member

Applies the ZIP 318 anchor and expiry policy to any transaction that spends Orchard notes, instead of only to a canonical ZIP 318 pool crossing (one payment, of a {1, 2, 5} * 10^k denomination, funded from a single note).

Builds on #35, which vendored the upstream machinery this uses. It now implements the Priority 2 recommendation in full, including the anchor age draw and the uniform fallback from Appendix A.

What this changes

Two gates decided the old, narrow scope. Both move:

  1. propose_transfer computed a bucketed confirmations policy only when canonical_crossing_candidate() liked the request. It now draws an anchor for any request the caller permits Orchard spends for, and the canonical-crossing attempt keeps that gate for itself.
  2. build_proposed_transaction (and the PCZT path) applied the ZIP 318 rolling expiry only to a canonical crossing. The condition is now that the step's own anchor lies on the bucket grid.

The anchor is drawn as Appendix A specifies:

  • A boundary at an age in [1, ANCHOR_AGE_CAP] from the recency-weighted Geometric(1/2) distribution, among the boundaries at or above the newest note the transaction spends and strictly above NU6.3 activation. The draw is by rejection over fair coin flips from OsRng, which conditions the geometric on the available ages — the same distribution as the explicit 2^(max_age - a) weights, and the same one zcash_pool_migration::scheduling::draw_anchor_boundary gives a migration transfer.
  • A uniform height in [min_anchor_height, target_height - 1] when no boundary is admissible, so a wallet holding only recent notes spends immediately instead of waiting for the next boundary to settle.
  • The ordinary anchor if the drawn height is not computable, or the wallet cannot fund the payment at it. Anonymity is worth confirmations; it is never worth refusing to spend.

The expiry formula needed no new code: zcash_protocol::zip318::expiry_height is the same function as Appendix A's (target // 34560 + 2) * 34560.

Why the ordinary proposal is built first

The draw needs min_anchor_height, and nothing before input selection knows which notes will be spent. So the ordinary proposal is built first and does triple duty: it bounds the draw, it is re-proposed at the drawn anchor, and it stays the fallback. Every note it selected is at or below that bound, so the same notes remain eligible at the drawn boundary and the re-proposal funds.

Checkpoint retention

The uniform fallback can reach two grid intervals back (288 blocks on mainnet), and an anchor at a height whose checkpoint was pruned cannot be witnessed against. Rather than deepen PRUNING_DEPTH, which also governs how far the wallet may rewind and how far scanning re-verifies, this adds CHECKPOINT_RETENTION_DEPTH (300) for the commitment trees alone. Raising the shared constant instead changed scan-range planning and broke nine tests; the two constants answer different questions and are now separate. Durable anchor checkpoints remain exempt from the budget, so nothing about boundary retention changes.

What is deliberately not widened

The canonical denomination, the unpadded Ironwood bundle, and the canonical fee. Those three are what make a crossing indistinguishable from a migration transfer, and an ordinary payment cannot satisfy them: it pays an arbitrary amount to someone else. Adopting part of the shape produces a transaction that is canonical in every respect but one, which is a fingerprint rather than a disguise (see the reasoning on Step::is_canonical_crossing).

So this does not enlarge the migration-transfer anonymity set. It enlarges the anchor cohort at each boundary and puts ordinary Orchard payments in the shared expiry window — which is what removes the per-block partition inside a zero-indexer batch.

Why the expiry keys on the anchor

Because bucketing is attempted, not guaranteed. A step that fell back to the ordinary anchor also keeps the ordinary expiry: a rolling expiry on a tip-anchored transaction would re-identify exactly what the shared anchor was supposed to hide. A uniform fallback anchor that happens to land on the grid takes the rolling expiry, and the tests assert that the two observables agree either way.

Consequences worth an explicit decision

  • An Orchard-spending payment needs up to four grid intervals (576 blocks, about 12 hours on mainnet) of additional confirmations on its inputs, and one interval on the modal draw. Bucketing is expressed as a raised confirmation requirement, so the anchor and the spendability bound move together and a note can never be selected that has no witness at the chosen anchor.
  • Ordinary Orchard payments now carry an expiry 1 to 2 months out instead of the builder's 40-block delta.
  • Each Orchard-spending payment now costs one extra input-selection pass.

Known gap

The canonical-crossing path still takes age 1 deterministically, because that is what upstream's ConfirmationsPolicy::bucketed does and this PR leaves that path's behavior alone. Real migration transfers draw an age, so a canonical-shaped payment from this fork is distinguishable from a migration transfer by its anchor age — a pre-existing upstream mismatch, narrowed but not closed here. Making that path draw too would trade canonical-shape success rate for age uniformity, which is a decision worth taking on its own.

Testing

  • cargo clippy --all-features --all-targets -- -D warnings: clean.
  • cargo test -p zcash_client_sqlite -p zcash_client_backend --all-features: green, 0 failures.
  • The randomized tests were run five times over to check for draw-dependent flakiness.

Two existing tests asserted the old policy directly and now assert the new one:

  • canonical_crossing_is_bucketed_and_unpadded, case (2): a payment one zatoshi off a canonical denomination was required NOT to be bucketed. It now must be bucketed, at an age the ZIP 318 draw admits, while keeping the padded Ironwood bundle its fee was charged for. This is the load-bearing test of the change: it holds the shape observable fixed and moves only the anchor.
  • multi_note_crossing_is_not_bucketed (renamed multi_note_crossing_is_bucketed_but_not_canonical): same flip for a multi-input payment, plus a new assertion that the built transaction carries the ZIP 318 rolling expiry.

One test is new:

  • orchard_payment_falls_back_when_notes_are_too_new: spends the wallet's pre-activation note, mines the result, then immediately spends the resulting Orchard change. Because the pool is closed to new value, change from the wallet's own spend is the only way an Orchard note can be younger than every candidate boundary. The second payment must draw above the newest candidate boundary rather than wait, and must not be refused.

canonical_crossing_abandoned_without_anchor_checkpoint was also updated: with a drawn age, removing one boundary's checkpoint no longer forces the ordinary anchor, since the draw can land on another provable boundary. The test now asserts what actually matters — the proposal does not anchor at the unprovable boundary, and it builds.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

…g 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 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 18, 2026 16:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Extends the Zero fork’s ZIP 318 privacy policy so that (when possible) any transaction spending Orchard notes is proposed using a bucket-boundary Orchard anchor and is built with the ZIP 318 rolling expiry, instead of limiting these behaviors to canonical ZIP 318 crossings.

Changes:

  • Update propose_transfer to attempt boundary-bucketed anchoring for any Orchard-permitting spend policy, and keep the bucketed proposal only if Orchard inputs are actually selected.
  • Replace the “canonical crossing” expiry gate with a “boundary-anchored step” gate, and rename the associated expiry-conflict error variant.
  • Adjust and add tests to cover widened bucketing/expiry behavior and the fallback-to-ordinary-anchor case.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
librustzcash/zcash_client_sqlite/src/wallet/orchard.rs Wires new/renamed pool tests into the sqlite crate’s Orchard test module.
librustzcash/zcash_client_sqlite/src/testing/pool.rs Adds sqlite wrapper for the new backend pool test and renames the multi-note test wrapper.
librustzcash/zcash_client_backend/src/data_api/wallet.rs Widens bucketing beyond canonical crossings and applies ZIP 318 rolling expiry based on boundary anchoring.
librustzcash/zcash_client_backend/src/data_api/testing/pool.rs Updates existing tests for widened anchor/expiry policy and adds a new fallback behavior test.
librustzcash/zcash_client_backend/src/data_api/error.rs Renames and updates the expiry-conflict error variant and its display text.
librustzcash/zcash_client_backend/CHANGELOG.md Documents the widened Orchard bucketing + expiry behavior and the error variant rename.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread librustzcash/zcash_client_backend/src/data_api/wallet.rs Outdated
Comment thread librustzcash/zcash_client_backend/src/data_api/error.rs
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 18, 2026 16:27
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

librustzcash/zcash_client_backend/src/data_api/wallet.rs:1057

  • step_is_boundary_anchored only checks whether the step’s anchor height lands on a bucket boundary; it doesn’t verify that the step actually spends Orchard notes. As a result, a Sapling-only spend whose anchor height happens to be on the boundary grid would also be forced onto the ZIP 318 rolling expiry (and would reject a caller-supplied expiry height), which appears to exceed the PR’s stated scope of “transactions that spend Orchard notes.”
    step.anchor_height().is_some_and(|anchor| {
        wallet_db
            .pool_migration_params()
            .anchor_bucket_interval()
            .is_boundary(anchor)

@zookoatshieldedlabs zookoatshieldedlabs self-assigned this Aug 20, 2026
@zookoatshieldedlabs

Copy link
Copy Markdown

The motivation for this change is that zero-indexer batches and mixes all transactions that touch Orchard (because any transaction that touches Orchard is potentially privacy-sensitive to its user). zero-indexer batches are 20 blocks—about 25 minutes—per batch. Without this change, "migration" transactions that touch Orchard get effectively mixed among all "migration" transactions in their batch, but "non-migration" transactions that touch Orchard are visibly different in the output from zero-indexer, so that they are effectively only mixed with other Orchard-touching non-migration transactions that happened in the same block as each other. So, without this change "migration" and "non-migration" are partitioned sets, and "non-migration" mixes only with other non-migration in the same block. With this change, migration and non-migration might be indistinguishable from each other (if the non-migration transaction's amount happens to be a ZIP-318-compatible amount), and in any case all non-migration transactions are mixed with all other non-migration transactions in their 20-block batch.

See also the Priority 2 tab of this doc for further explanation and sample code.

@zookoatshieldedlabs zookoatshieldedlabs removed their assignment Aug 20, 2026
… anchor when none is admissible

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 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 21, 2026 03:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (3)

librustzcash/zcash_client_backend/src/data_api/wallet.rs:2259

  • The ZIP 318 rolling-expiry override is currently keyed only on step_is_boundary_anchored(...). Before NU6.3 activation, ordinary confirmation-policy anchors can still coincide with a grid boundary, which would incorrectly apply ZIP 318 expiry / reject requested expiry on pre-NU6.3 transactions. Gate this behavior on NU6.3 being active at min_target_height.
        if step_is_boundary_anchored(wallet_db, proposal_step) {

librustzcash/zcash_client_backend/src/data_api/wallet.rs:3267

  • The PCZT path rejects caller-supplied expiry_height whenever step_is_boundary_anchored(...) is true, but it does not check NU6.3 activation. This can cause pre-NU6.3 transactions to spuriously hit ExpiryHeightConflictsWithBoundaryAnchor when their ordinary anchor happens to fall on a boundary. Add the same NU6.3 activation gate as in build_proposed_transaction.
    if let Some(requested) = expiry_height
        && step_is_boundary_anchored(wallet_db, proposal_step)

librustzcash/zcash_client_backend/src/data_api/wallet.rs:1316

  • step_is_boundary_anchored only checks whether the anchor height happens to lie on the ZIP 318 grid, but does not verify that the step actually spends Orchard notes. As a result, Sapling-only shielded steps whose anchors coincide with a boundary could incorrectly take the ZIP 318 rolling expiry / reject a caller-specified expiry, even though this PR’s policy is scoped to Orchard-spending transactions.

This issue also appears in the following locations of the same file:

  • line 2259
  • line 3266
    step.anchor_height().is_some_and(|anchor| {
        wallet_db
            .pool_migration_params()
            .anchor_bucket_interval()
            .is_boundary(anchor)

@aphelionz

Copy link
Copy Markdown
Member Author

@zookoatshieldedlabs have another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants