zcash_client_sqlite: Add WalletDb::get_unspent_orchard_notes_at_historical_height#2284
Merged
nuttycom merged 4 commits intoApr 24, 2026
Conversation
…otes `WalletDb::get_unspent_orchard_notes_at_historical_height` returns all Orchard notes that existed and were unspent at a given block height for a specified account. Unlike `select_unspent_notes` (which applies confirmation, dust, and expiry filters for transaction construction), this provides an unfiltered view of the historical note set—useful for auditing or reconstructing wallet state at a past point in time. Made-with: Cursor
nuttycom
force-pushed
the
roman/zcash_client_sqlite-0.19.x/get-unspent-notes-historical
branch
from
600a92c to
2d0fa79
Compare
nuttycom
requested changes
Apr 24, 2026
Collaborator
nuttycom
left a comment
nuttycom
left a comment
There was a problem hiding this comment.
The suggestions in this review don't necessarily require changes, but I'd like to get responses to the questions they raise before ACK'ing.
nuttycom
reviewed
Apr 24, 2026
…uery Switch `get_unspent_orchard_notes_at_historical_height` to filter on `transactions.mined_height` instead of `transactions.block`, and drop the now-redundant `IS NOT NULL` guards on both the receive and spend sides. `block` is set only after the containing compact block has been scanned, whereas `mined_height` is set as soon as the wallet learns of the mined height (including through paths such as transparent UTXO graph retrieval). For the notes this query can return the two are equivalent in practice (`nf` and `commitment_tree_position` already require a scan of the receiving block), but `mined_height` is the right column to filter on semantically, and matches the existing `mark_orchard_note_spent` and `get_spendable_note` queries. The `IS NOT NULL` guards were dead code: `NULL <= :height` is always false in SQLite, so `mined_height <= :height` already excludes transactions the wallet has not observed as mined. Addresses review feedback from @nuttycom on zcash#2284. Made-with: Cursor Co-Authored-By: Claude <noreply@anthropic.com>
…spent-notes query Replace `rn.recipient_key_scope IN (0, 1)` with an `IN` clause that interpolates `KeyScope::EXTERNAL.encode()` and `KeyScope::INTERNAL.encode()` via `format!`, matching the pattern used in `wallet/transparent.rs` and `wallet/init/migrations/transparent_gap_limit_handling.rs`. This keeps the SQL filter in sync with the `KeyScope` definition if its encoding ever changes. Addresses review feedback from @nuttycom on zcash#2284. Made-with: Cursor Co-Authored-By: Claude <noreply@anthropic.com>
…n for historical note getter Clarify in the rustdoc of both the `pub(crate)` helper in `wallet::orchard` and the public `WalletDb::get_unspent_orchard_notes_at_historical_height` wrapper that this function does not verify whether a Merkle witness can be constructed for each returned note at `height`. Witness construction is a separate concern intended to be handled by callers; as an example, a companion `WalletDb::generate_orchard_witnesses_at_historical_height` method returns an actionable error for any position the wallet cannot witness at the given height (wallet not synced through that height, checkpoint pruned, or position not in the wallet). The companion method is referenced by name only rather than via an intra-doc link because it is introduced in a separate change. Addresses review feedback from @nuttycom on zcash#2284. Made-with: Cursor Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
|
@nuttycom thanks for the review. All suggestions have been addressed in three follow-up commits. Each inline thread has a reply with the specific commit that addresses it.
Ready for another look when you have a moment. |
nuttycom
approved these changes
Apr 24, 2026
p0mvn
added a commit
to valargroup/librustzcash
that referenced
this pull request
Apr 24, 2026
…uery Switch `get_unspent_orchard_notes_at_historical_height` to filter on `transactions.mined_height` instead of `transactions.block`, and drop the now-redundant `IS NOT NULL` guards on both the receive and spend sides. `block` is set only after the containing compact block has been scanned, whereas `mined_height` is set as soon as the wallet learns of the mined height (including through paths such as transparent UTXO graph retrieval). For the notes this query can return the two are equivalent in practice (`nf` and `commitment_tree_position` already require a scan of the receiving block), but `mined_height` is the right column to filter on semantically, and matches the existing `mark_orchard_note_spent` and `get_spendable_note` queries. The `IS NOT NULL` guards were dead code: `NULL <= :height` is always false in SQLite, so `mined_height <= :height` already excludes transactions the wallet has not observed as mined. Addresses review feedback from @nuttycom on zcash#2284. Made-with: Cursor Co-Authored-By: Claude <noreply@anthropic.com>
p0mvn
added a commit
to valargroup/librustzcash
that referenced
this pull request
Apr 24, 2026
…spent-notes query Replace `rn.recipient_key_scope IN (0, 1)` with an `IN` clause that interpolates `KeyScope::EXTERNAL.encode()` and `KeyScope::INTERNAL.encode()` via `format!`, matching the pattern used in `wallet/transparent.rs` and `wallet/init/migrations/transparent_gap_limit_handling.rs`. This keeps the SQL filter in sync with the `KeyScope` definition if its encoding ever changes. Addresses review feedback from @nuttycom on zcash#2284. Made-with: Cursor Co-Authored-By: Claude <noreply@anthropic.com>
p0mvn
added a commit
to valargroup/librustzcash
that referenced
this pull request
Apr 24, 2026
…n for historical note getter Clarify in the rustdoc of both the `pub(crate)` helper in `wallet::orchard` and the public `WalletDb::get_unspent_orchard_notes_at_historical_height` wrapper that this function does not verify whether a Merkle witness can be constructed for each returned note at `height`. Witness construction is a separate concern intended to be handled by callers; as an example, a companion `WalletDb::generate_orchard_witnesses_at_historical_height` method returns an actionable error for any position the wallet cannot witness at the given height (wallet not synced through that height, checkpoint pruned, or position not in the wallet). The companion method is referenced by name only rather than via an intra-doc link because it is introduced in a separate change. Addresses review feedback from @nuttycom on zcash#2284. Made-with: Cursor Co-Authored-By: Claude <noreply@anthropic.com>
nuttycom
deleted the
roman/zcash_client_sqlite-0.19.x/get-unspent-notes-historical
branch
p0mvn
added a commit
to valargroup/librustzcash
that referenced
this pull request
Apr 24, 2026
…tness-historical Resolves conflicts in zcash_client_sqlite/CHANGELOG.md and zcash_client_sqlite/src/lib.rs, both caused by get_unspent_orchard_notes_at_historical_height (added upstream in zcash#2284) and generate_orchard_witnesses_at_historical_height (added in this branch) landing side-by-side in the same `## [Unreleased] ### Added` block and the same `#[cfg(feature = "orchard")] impl WalletDb<...>` block. Both methods are kept; the upstream method is listed/declared first to preserve the upstream ordering, with the witness-generation API following it. The cross-reference in get_unspent_orchard_notes_at_historical_height to the companion generate_orchard_witnesses_at_historical_height now points at a method that actually exists on this branch. Co-Authored-By: Claude <noreply@anthropic.com> Made-with: Cursor
This was referenced Apr 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WalletDb::get_unspent_orchard_notes_at_historical_height, which returns all Orchard notes that existed and were unspent at a given block height for a specific account.See the full integration note: https://hackmd.io/HstXV5RfSdOSPdEauotrXw
Branching note
This PR targets maint/zcash_client_sqlite-0.19.x as a
SemVer-compatible addition (new public method, no breaking changes).
How We Use It
Token Holder Voting.
Query the set of notes a wallet held at a governance snapshot height, then
pair with witnesses from
generate_orchard_witnesses_at_historical_heightto construct voting proofs. See #2283
Changes
zcash_client_sqlite/src/wallet/orchard.rs— newpub(crate) fn get_unspent_orchard_notes_at_historical_heightquery that selects received notesmined at or before the given height, excluding any spent at or before
that height.
zcash_client_sqlite/src/lib.rs— publicWalletDb::get_unspent_orchard_notes_at_historical_heightmethod that delegatesto the above.
zcash_client_sqlite/CHANGELOG.md— added entry under[Unreleased].get_unspent_orchard_notes_at_historical_height_boundary_heights)verifying correct note visibility across receive, spend, and subsequent
receive heights.