[zero] librustzcash: bucket the anchor of every Orchard-spending transaction - #36
[zero] librustzcash: bucket the anchor of every Orchard-spending transaction#36aphelionz wants to merge 4 commits into
Conversation
…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>
There was a problem hiding this comment.
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_transferto 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.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
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_anchoredonly 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)
|
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. |
… 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>
There was a problem hiding this comment.
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 atmin_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_heightwheneverstep_is_boundary_anchored(...)is true, but it does not check NU6.3 activation. This can cause pre-NU6.3 transactions to spuriously hitExpiryHeightConflictsWithBoundaryAnchorwhen their ordinary anchor happens to fall on a boundary. Add the same NU6.3 activation gate as inbuild_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_anchoredonly 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)
|
@zookoatshieldedlabs have another look? |
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^kdenomination, 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:
propose_transfercomputed a bucketed confirmations policy only whencanonical_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.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:
[1, ANCHOR_AGE_CAP]from the recency-weightedGeometric(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 fromOsRng, which conditions the geometric on the available ages — the same distribution as the explicit2^(max_age - a)weights, and the same onezcash_pool_migration::scheduling::draw_anchor_boundarygives a migration transfer.[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 expiry formula needed no new code:
zcash_protocol::zip318::expiry_heightis 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 addsCHECKPOINT_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
Known gap
The canonical-crossing path still takes age 1 deterministically, because that is what upstream's
ConfirmationsPolicy::bucketeddoes 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.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(renamedmulti_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_checkpointwas 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