Repository navigation
Add the zakura-dynamic-ivk crate - #120
czarcas7ic wants to merge 16 commits into
Conversation
b9f6e9f to
8941c41
Compare
czarcas7ic
changed the title
Dynamic IVKs are Ironwood receiving keys that keep an account's spending authority (ak and nk) with their own rivk, so each has its own incoming viewing key and address, and a wallet trial-decrypts with a changing set of them. Wallets use them for swap refund and incoming addresses that cannot be linked to the wallet or to each other. The unpublished crate derives a key's rivk as ToScalar(PRF^expand_rivk([0x85] || ak || nk || [purpose] || index)), ZIP 32's internal-key construction with another domain byte and a suffix, so a ZIP 2005 recovery circuit could check it the same way. 0x85 must be reserved with the ZIP editors before live addresses. It also holds the refund memo, REFUND_MEMO_MAGIC followed by the refund index, that a funding transaction carries on its internal Ironwood change; the shared completion limits and NEAR status normalization; and authentication of privately discovered notes. Tests include independent KDF vectors and an Ironwood receive/restore/spend proof test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8941c41 to
691d27c
Compare
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. |
![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: 691d27c7ab
ℹ️ 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".
| The SQLite backend implements this contract with its `orchard` feature: durable | ||
| key registration, scanning, completion, and the restore sweeps that | ||
| `zakura-pir-receiver` runs. Its rustdoc, starting at | ||
| `zcash_client_sqlite::wallet::dynamic_ivk`, documents the calls a wallet makes. |
There was a problem hiding this comment.
At this commit, a repository-wide search finds no dynamic_ivk module, tests, or dependency under zakura-client-sqlite, and zakura-pir-receiver is also absent. Integrators therefore cannot resolve the documented rustdoc path, and the advertised durable registration, scanning, and restore flow does not exist; either include that integration or describe it as future work.
Useful? React with 👍 / 👎.
|
|
||
| ```sh | ||
| cargo test -p zakura-dynamic-ivk --locked | ||
| cargo test -p zakura-client-sqlite --features orchard,test-dependencies --locked dynamic_ivk |
There was a problem hiding this comment.
At this commit, zakura-client-sqlite contains no test whose name matches dynamic_ivk, so this Cargo command selects zero tests and still exits successfully. It therefore gives a false green for the claimed SQLite integration; point it at an existing test or add the missing test and invoke it through the repository runner, which explicitly treats an empty filtered selection as failure.
AGENTS.md reference: AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
| Some(RecoveredNote { | ||
| nullifier: note.nullifier(&key), | ||
| note, | ||
| memo, | ||
| }) |
There was a problem hiding this comment.
For outputs using the documented public zero OVK, any discovery provider can recover the complete Note, re-encrypt that same committed note with an arbitrary memo, and return the forged ciphertext here. The commitment, action nullifier, ephemeral key, 52-byte compact prefix, Merkle path, and accepted root remain valid, so decrypt, matches_compact, and verify_position all accept while this field exposes the substituted memo as authenticated; require an independently obtained full on-chain ciphertext comparison or keep the memo explicitly untrusted.
Useful? React with 👍 / 👎.
| ("SUCCESS", Purpose::Refund) => Terminal(match refund { | ||
| Some(value) => Positive(Some(value)), | ||
| None if status.swap_type == Some("EXACT_OUTPUT") => Positive(None), | ||
| None => ReceiptExpectation::None, |
There was a problem hiding this comment.
When a refund-key operation reaches SUCCESS with no positive refunded_amount, an absent, malformed, or newly introduced swap_type falls through to ReceiptExpectation::None. If the original quote was exact-output, unused ZEC can still be sent to this key, so treating missing type data as proof that no receipt exists can end scanning and leave the refund undiscovered; only a recognized non-exact-output type should select None, while unavailable or unrecognized types should remain Unknown.
Useful? React with 👍 / 👎.
| let rivk = expand_rivk(&bytes, RIVK_DOMAIN, &suffix); | ||
| bytes[64..].copy_from_slice(&rivk); |
There was a problem hiding this comment.
expand_rivk returns the derived viewing-key material into this ordinary stack array, which is not wiped after it is copied into the zeroizing FVK byte buffer. Thus each derivation leaves an avoidable extra rivk copy available to later stack reuse or process-memory inspection, despite the surrounding code explicitly zeroizing the account bytes and wide PRF output; wrap this result in Zeroizing or write it directly into zeroizing storage.
Useful? React with 👍 / 👎.
The README described the SQLite integration and the receiver client, neither of which exists in this crate's PR, and its validation section named a SQLite test filter that selects nothing here. Keep only the crate's own test command, run through the repository runner. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A refund key's SUCCESS without a positive refunded amount mapped every swap type other than EXACT_OUTPUT, including an absent or unrecognized one, to "no Zcash receipt expected", which lets storage settle the key early. Only a recognized EXACT_INPUT now means no receipt; a missing, empty or unrecognized type is Unknown. A positive refunded amount stays authoritative. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The derived rivk was left in an ordinary array between the zeroizing FVK and hash buffers around it. Wrap it in Zeroizing so this local is cleared on drop, as its neighbours are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RecoveredNote returned the decrypted memo as authenticated, but nothing binds it to the chain. Anyone who recovers the note, for example through the public zero OVK, can re-encrypt the same note with another memo; the action nullifier, cmx, ephemeral key and 52-byte compact prefix are unchanged and decryption still succeeds. Drop the memo field and accessor, keep authenticating the note itself, and correct the docs. Add a regression test that recovers a public-zero-OVK output as a provider would, substitutes the memo, and shows decrypt accepts both. 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: e9edeceb22
ℹ️ 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".
| keys) and `COMPLETION_LIMIT_SECS` (30 days), and direction-aware NEAR | ||
| status normalization, `lifecycle::near_observation`, including refunded amounts | ||
| and exact-output leftovers. Durable scheduling and receipt accounting live in | ||
| SQLite. |
There was a problem hiding this comment.
Although the earlier README assertion was removed, this new changelog entry still tells consumers that durable scheduling and receipt accounting live in SQLite. A repo-wide search of this squashed commit finds no dynamic_ivk module, dependency, schema, or tests under librustzcash/zcash_client_sqlite, so the release note still advertises integration that cannot be used; either land that integration or describe it as future work.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
The changelog said durable scheduling and receipt accounting live in SQLite, which this crate's PR does not contain. Say they are left to the wallet's store instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Packaging rewrites a path dependency to a registry requirement, so a publishable workspace package whose normal, build or optional dependency is a local `publish = false` package cannot be released. Check that edge in the graph verification. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The backend depends on this crate, so it must be releasable before the next backend release. Give it the zakura prerelease version and registry metadata, and drop the "unpublished" wording from its docs. Nothing is published here. 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: f42480f974
ℹ️ 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".
| ("SUCCESS", Purpose::Refund) => Terminal(match (refund, status.swap_type) { | ||
| (Some(value), _) => Positive(Some(value)), | ||
| (None, Some("EXACT_OUTPUT")) => Positive(None), | ||
| (None, Some("EXACT_INPUT")) => ReceiptExpectation::None, |
There was a problem hiding this comment.
When an exact-input swap was over-deposited but a SUCCESS response omits or cannot parse refundedAmount, this branch returns ReceiptExpectation::None. The 1Click quote contract states that excess tokens are refunded even for exact-input swaps, so wallet receipt accounting may stop scanning and miss a later ZEC refund. Keep this case Unknown unless the adapter can independently prove that no excess was deposited.
Useful? React with 👍 / 👎.
1Click refunds excess deposits for exact-input swaps too, so a refund key's exact-input SUCCESS proves no ZEC receipt only when the response reports a zero refunded amount. When the amount is absent, the expectation is now Unknown and the key keeps scanning. A positive amount stays authoritative and exact-output handling is unchanged. 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: 7cba47147f
ℹ️ 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".
| }), | ||
| ("REFUNDED", Purpose::Refund) => Terminal(Positive(refund)), | ||
| ("REFUNDED", Purpose::Receive) => Terminal(ReceiptExpectation::None), | ||
| ("FAILED", _) => Terminal(Unknown), |
There was a problem hiding this comment.
When a deposited operation reaches FAILED, 1Click specifies that the origin funds are automatically returned to the refund address (1Click API documentation). For Purpose::Refund this means a ZEC receipt is expected (and any positive refunded_amount should be preserved), while for Purpose::Receive the refund is on the non-ZEC origin chain and no ZEC receipt is expected. Collapsing both cases to Unknown discards this direction and amount information, causing completion logic to keep already-resolved operations pending until its timeout instead of reconciling the correct receipt.
Useful? React with 👍 / 👎.
1Click returns a FAILED swap's deposit to the refund address on the origin chain, as it does for REFUNDED. near_observation reported FAILED as Unknown for both purposes, which kept resolved operations pending until the completion limit. A refund key now expects a ZEC receipt after FAILED, carrying a positive refunded amount when reported, and an incoming key expects none, since its refund lands on the source chain. 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: d67a472920
ℹ️ 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".
| } | ||
| ("SUCCESS", Purpose::Refund) => Terminal(match (refund, status.swap_type) { | ||
| (Some(value), _) => Positive(Some(value)), | ||
| (None, Some("EXACT_OUTPUT")) => Positive(None), |
There was a problem hiding this comment.
When an EXACT_OUTPUT operation succeeds with refunded_amount == Some(Zatoshis::ZERO), this arm still returns Positive(None), so completion waits for a ZEC receipt that the provider explicitly reported as zero and can remain pending until the 30-day cap. This is distinct from the earlier missing-amount case: the fresh evidence is the parsed zero value, and the 1Click quote contract says exact-output excess is refunded only when the input exceeds maxAmountIn; distinguish None from Some(0) here and cover both cases.
AGENTS.md reference: AGENTS.md:L43-L46
Useful? React with 👍 / 👎.
An exact-output SUCCESS that reports a zero refunded amount was still normalized to Positive(None), so completion waited for a ZEC refund the provider had explicitly reported as zero and could stay pending until the 30-day cap. Treat a reported zero like the exact-input case: no receipt is expected. An absent amount still expects a refund of unknown amount, and a positive amount stays authoritative. 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: 280818699b
ℹ️ 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 refund_reported = status.refunded_amount.is_some(); | ||
| let outcome = match (status.status, purpose) { | ||
| ("PENDING_DEPOSIT", _) => AwaitingDeposit, | ||
| ("KNOWN_DEPOSIT_TX" | "INCOMPLETE_DEPOSIT" | "PROCESSING", _) => Active, |
There was a problem hiding this comment.
When an underpayment reaches INCOMPLETE_DEPOSIT, 1Click treats it as a terminal state and refunds the deposit; the Defuse settlement specification explicitly includes it among terminal states and says every non-success terminal case is refunded. Mapping it to Active here bypasses the direction-specific terminal handling: a ZEC-origin Refund key never records its expected refund, while a Receive key never records that its source-chain refund produces no ZEC receipt, leaving either operation pending and scanning until the completion cap. Normalize this status by purpose like the other refunded failure states.
Useful? React with 👍 / 👎.
1Click reports INCOMPLETE_DEPOSIT for an underpaid deposit and later moves the operation to REFUNDED once the deadline passes. That refund status is already normalized by direction, so INCOMPLETE_DEPOSIT stays Active until then (or until the completion limit). Say so at the arm. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1Click documents that an underpayment is refunded around the deadline, but its status reference does not promise an INCOMPLETE_DEPOSIT to REFUNDED transition. Say only that the status is not final and that the completion limit bounds the wait. 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: c36ab7c97a
ℹ️ 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".
| upstream_manifest = "librustzcash/upstream-workspace.toml" | ||
| # `zakura/` members are added here as they are written. | ||
| extra_members = ["wallet-lib", "zakura/pir-enhance", "zakura/pir-enhance-types", "zakura/pir-native", "zakura/pir-status", "zakura/transaction-status", "zakura/pir-transparent"] | ||
| extra_members = ["wallet-lib", "zakura/pir-enhance", "zakura/pir-enhance-types", "zakura/pir-native", "zakura/pir-status", "zakura/transaction-status", "zakura/pir-transparent", "zakura/dynamic-ivk"] |
There was a problem hiding this comment.
Adding this crate only to layout.extra_members leaves it outside the graph verifier's reachability roots: verify-zakura-graph.sh seeds its queue solely from manifest["crate"], and a repo-wide dependency search confirms no vendored crate reaches zakura-dynamic-ivk. Consequently, forbidden crates or duplicate Zakura versions introduced through this new dependency-family boundary would still pass the repository's advertised graph check; include applicable extra members as verification roots while continuing to exclude the dual-family facade.
AGENTS.md reference: AGENTS.md:L43-L46
Useful? React with 👍 / 👎.
| .to_state() | ||
| .update(&fvk[64..]) | ||
| .update(&[domain]) | ||
| .update(&fvk[..64]) | ||
| .update(suffix) |
There was a problem hiding this comment.
Whenever a key is derived, these updates total 106 bytes, below BLAKE2b's 128-byte block size, so the temporary blake2b_simd::State retains rivk || domain || ak || nk || suffix in its internal buffer and drops it without wiping; wrapping only the copied digest in Zeroizing does not clear that account-FVK copy. The fresh evidence after the earlier returned-rivk fix is this separate non-zeroizing hasher buffer (upstream State layout); use a PRF implementation or explicitly controlled state that guarantees zeroization so later stack reuse or process-memory inspection cannot recover the viewing material.
Useful? React with 👍 / 👎.
The graph verifier seeded its walk from the vendored crates alone, so a dependency reachable only from a `layout.extra_members` crate such as `zakura-dynamic-ivk` escaped the forbidden-crate and duplicate-version checks. Seed it from those members as well, still leaving out the dual-backend facade. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`blake2b_simd::State` offers no zeroization, so wrapping the PRF output in `Zeroizing` is hygiene for the values this code owns rather than erasure of every temporary. Say so at the call site. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Dynamic IVKs are Ironwood receiving keys that keep an account's spending authority (ak and nk) with their own rivk, so each has its own incoming viewing key and address, and a wallet trial-decrypts with a changing set of them. Wallets use them for swap refund and incoming addresses that cannot be linked to the wallet or to each other.
The unpublished crate derives a key's rivk as ToScalar(PRF^expand_rivk([0x85] || ak || nk || [purpose] || index)), ZIP 32's internal-key construction with another domain byte and a suffix, so a ZIP 2005 recovery circuit could check it the same way. 0x85 must be reserved with the ZIP editors before live addresses.
It also holds the refund memo, REFUND_MEMO_MAGIC followed by the refund index, that a funding transaction carries on its internal Ironwood change; the shared completion limits and NEAR status normalization; and authentication of privately discovered notes. Tests include independent KDF vectors and an Ironwood receive/restore/spend proof test.
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