Repository navigation
Add dynamic IVK traits to the backend - #121
czarcas7ic wants to merge 21 commits into
Conversation
0a148b4 to
745a187
Compare
czarcas7ic
changed the title
d8392b3 to
a66c69c
Compare
a66c69c to
49e0481
Compare
49e0481 to
deeadad
Compare
czarcas7ic
changed the title
With the orchard feature, the backend can trial-decrypt dynamic keys: DynamicScanningKey for compact scanning and full-transaction decryption, dynamic_key_id on decrypted outputs and received notes, and spending a dynamic-key note with its derived viewing key. A store opts in by implementing the new data_api::dynamic_ivk traits. DynamicIvkRead returns the keys to scan with, and DynamicIvkWrite::put_blocks_with_dynamic_ivks stores blocks with the key snapshot that scanned them, so a store can rescan blocks that a key activated mid-batch missed. scan_cached_blocks_with_dynamic_ivks and decrypt_and_store_transaction_with_dynamic_ivks are the twins of the existing entry points and share their scan loop. A decrypted transaction records whether dynamic keys were applied, so a store can refuse one decrypted without them while a key is open. The traits also carry the restore sweep steps that the receiver PIR client drives; a step that must wait returns a SweepDeferral, and errors are the store's own. WalletRead, WalletWrite and LowLevelWalletRead are unchanged. The one change outside dynamic IVKs: ll::wallet::put_blocks and put_blocks_rows take an explicit Ironwood nullifier retention floor (with orchard), for a store that finds Ironwood notes after scanning past them. The SQLite store passes None until it implements dynamic IVKs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
deeadad to
15cde0c
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15cde0c1c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) -> Result<Vec<DynamicScanningKey<Self::AccountId>>, Self::Error> { | ||
| self.get_dynamic_scanning_keys() |
There was a problem hiding this comment.
When a full transaction for a previously compact-scanned dynamic note is fetched after its key has closed, this default returns only get_dynamic_scanning_keys(), whose contract is the changing set used for current scan batches. The closed key is therefore not tried, so an externally funded shielded transaction can appear to have no wallet outputs; store_decrypted_tx then treats it as irrelevant and removes its retrieval request, permanently losing memo/full-transaction enhancement. Make this method required or have the default enumerate all historical keys, as its own documentation requires.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The default get_dynamic_transaction_keys returned only scanning keys, although the contract requires keeping known-note ownership after a key stops scanning. A store relying on it could treat a transaction for a closed key as irrelevant and drop its retrieval requests. There is no historical-key enumeration from which to build a sound default, so make the method required. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PaymentApplication::Applied said the memo was committed together with the note, but a memo recovered from directory ciphertext is not bound to the chain and zakura-dynamic-ivk no longer returns it. Drop the memo from that promise and point at where the trust boundary is documented. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The module doc named zakura-pir-receiver as the restore sweep's runner, which this PR does not contain. Describe the sweep in terms of the traits alone; the receiver client documents itself where it lands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A recent filter that lacks a receiver cannot show that no swap will pay it: a quote after the feed's last read is missing from the set, and a payment between the lookup's anchor and the wallet tip was scanned without the key. Drop ProviderView.recent and have apply_dynamic_sweep take only whether a seen set holds the receiver, with the contract that every finished key scans from the block after its lookup until the store's restore watch closes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A seen hit passed to apply_dynamic_sweep was lost whenever payment work waited or was rejected before the sweep finished, and a later publication's corrected answer could then finish the key unseen. begin_dynamic_sweep_attempt now takes the hint and stores a hit with the lease, before any lookup or payment work; it is never cleared. apply_dynamic_sweep loses its seen argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c15cc16028
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| [dependencies] | ||
| zakura-pir-enhance-types = { version = "=0.0.1-rc0", path = "../../zakura/pir-enhance-types" } | ||
| zakura-dynamic-ivk = { version = "0.0.0", path = "../../zakura/dynamic-ivk", optional = true } |
There was a problem hiding this comment.
When zakura-client-backend is packaged or published, Cargo replaces this path dependency with the registry requirement zakura-dynamic-ivk = "0.0.0"; however, that crate's manifest declares publish = false, so the required registry package cannot exist. This blocks the next backend release and would leave published consumers unable to resolve the orchard feature. Publish the dependency at a real version before the backend, or avoid exposing it from this publishable crate.
Useful? React with 👍 / 👎.
| .collect::<Vec<_>>(); | ||
| let scanning_keys = | ||
| ScanningKeys::from_account_ufvks(account_ufvks).with_dynamic_ivks(dynamic_keys); | ||
| let (scan_summary, scanned_blocks) = scan_blocks( |
There was a problem hiding this comment.
When this new scanner detects a dynamic-IVK payment, the output necessarily belongs to the Ironwood pool, but scan_blocks only increments the Sapling and Orchard counters and ScanSummary exposes no Ironwood counters. Consequently a batch can successfully persist one or more dynamic payments while the returned summary reports zero received notes, so callers using it for receipt notifications or sync metrics miss every dynamic payment. Extend the summary and accumulator to count Ironwood spends and outputs.
Useful? React with 👍 / 👎.
Require the crate's new prerelease version so the backend's optional dependency resolves from the registry once the crate is released. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A saved lookup let a deferred candidate skip later publications, so a corrected or withdrawn answer never replaced it. Drop `DiscoveryWork.lookup` and its "resume those first" contract: each attempt asks the directory again and passes the answer through `queue_directory_lookup`. Also note at the scan summary accumulator that it omits Ironwood notes; adding Ironwood counters is separate work. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d2ae9cb33
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let ironwood_tracking_floor = nullifier_tracking_floor | ||
| .map(|floor| ironwood_nullifier_retention.map_or(floor, |height| floor.min(height))); |
There was a problem hiding this comment.
When dynamic recovery spans more than 100 blocks, this floor causes historical Ironwood nullifiers to be inserted, but put_blocks_rows subsequently calls prune_tracked_nullifiers(PRUNING_DEPTH). That trait's contract removes every pool's entries below fully_scanned_height - pruning_depth, without receiving ironwood_nullifier_retention, so later scan batches delete the spend evidence this code was meant to retain. A restored wallet can consequently remain at AwaitingSpendHistory for old dynamic notes (or lose the evidence needed to classify them); the retention floor must also be propagated into pool-aware pruning.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
put_blocks_rows tracks Ironwood nullifiers from the store's ironwood_nullifier_retention height, but the prune_tracked_nullifiers contract described pruning every pool below the fully scanned height minus the pruning depth, which would delete that evidence on the next batch. The trait now requires a store that supplies a retention height to keep Ironwood nullifiers recorded at or above it, and put_blocks_rows points there. SQLite already does so by reading its stored floor while pruning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DecryptedTransaction::dynamic_ivks_applied now returns the account and ID of each dynamic key with_dynamic_ivks tried, rather than a flag, so a store can refuse a decryption that missed a key opened after the keys were read, as block storage already does for scanned blocks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only a key a current, complete provider recent set holds can still be paid by a swap, so apply_dynamic_sweep takes that flag again: a recent key scans from the block after its lookup for the restore watch, and any other closes at the lookup. A closed key with no known payment is covered instead by a newer publication's paid filter: the new read dynamic_keys_to_recheck lists such keys, and record_paid_filter_check queues the sweep of each one the filter holds and finishes the others' at that publication. The Applied doc points at decrypt's rule that no dynamic-key memo is stored instead of calling it untrusted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
With the orchard feature, the backend can trial-decrypt dynamic keys: DynamicScanningKey for compact scanning and full-transaction decryption, dynamic_key_id on decrypted outputs and received notes, and spending a dynamic-key note with its derived viewing key.
A store opts in by implementing the new data_api::dynamic_ivk traits. DynamicIvkRead returns the keys to scan with, and DynamicIvkWrite::put_blocks_with_dynamic_ivks stores blocks with the key snapshot that scanned them, so a store can rescan blocks that a key activated mid-batch missed. scan_cached_blocks_with_dynamic_ivks and decrypt_and_store_transaction_with_dynamic_ivks are the twins of the existing entry points and share their scan loop. A decrypted transaction records whether dynamic keys were applied, so a store can refuse one decrypted without them while a key is open. The traits also carry the restore sweep steps that the receiver PIR client drives; a step that must wait returns a SweepDeferral, and errors are the store's own.
WalletRead, WalletWrite and LowLevelWalletRead are unchanged. The one change outside dynamic IVKs: ll::wallet::put_blocks and put_blocks_rows take an explicit Ironwood nullifier retention floor (with orchard), for a store that finds Ironwood notes after scanning past them. The SQLite store passes None until it implements dynamic IVKs.
Tests: the library CI matrix (fmt, every
scripts/dev.pylane and the three verify scripts) passes at the top of the stack, and the lanes this PR affects pass at this PR.🤖 Generated with Claude Code