From 970886bf915d3f96961bbd551c92fe93bf92e27b Mon Sep 17 00:00:00 2001 From: Greg Nagy Date: Fri, 6 Mar 2026 15:47:54 +0100 Subject: [PATCH 01/12] Expose PCZT getters needed for shielded voting Co-Authored-By: roman Co-Authored-By: Claude Code Remove duplicate shielded_sighash getter in pczt signer Bad merge left two identical methods. Keep the one with the fuller doc comment, fix stray braces in doc comments on nearby methods. --- pczt/CHANGELOG.md | 1 + pczt/src/roles/signer/mod.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pczt/CHANGELOG.md b/pczt/CHANGELOG.md index 81875c8457..55d2f55353 100644 --- a/pczt/CHANGELOG.md +++ b/pczt/CHANGELOG.md @@ -14,6 +14,7 @@ workspace. - `pczt::ExtractError` - `pczt::EffectsOnly` - `pczt::orchard::Spend::spend_auth_sig` getter (via `getset`). +- `pczt::roles::signer::Signer::shielded_sighash` getter. - `pczt::roles::signer`: - `Signer::sighash` - `Signer::append_transparent_signature` diff --git a/pczt/src/roles/signer/mod.rs b/pczt/src/roles/signer/mod.rs index 4235a3b473..82820232f0 100644 --- a/pczt/src/roles/signer/mod.rs +++ b/pczt/src/roles/signer/mod.rs @@ -80,7 +80,7 @@ impl Signer { /// Calculates the signature digest that must be signed to authorize shielded spends. /// /// This can be used to produce a signature externally suitable for passing to e.g. - /// [`Self::apply_orchard_signature`].} + /// [`Self::apply_orchard_signature`]. pub fn shielded_sighash(&self) -> [u8; 32] { self.shielded_sighash } @@ -89,7 +89,7 @@ impl Signer { /// spend at the given index. /// /// This can be used to produce a signature externally suitable for passing to e.g. - /// [`Self::append_transparent_signature`].} + /// [`Self::append_transparent_signature`]. /// /// Returns an error if `index` is invalid for this PCZT. pub fn transparent_sighash(&self, index: usize) -> Result<[u8; 32], Error> { From 5f1251ac8adb09d61112cd5500693df42c86ccd3 Mon Sep 17 00:00:00 2001 From: roman Date: Sat, 11 Apr 2026 13:02:35 -0600 Subject: [PATCH 02/12] zcash_client_sqlite: Add method to query historical unspent Orchard notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- zcash_client_sqlite/CHANGELOG.md | 2 + zcash_client_sqlite/src/lib.rs | 22 ++++ zcash_client_sqlite/src/wallet/orchard.rs | 126 ++++++++++++++++++++++ 3 files changed, 150 insertions(+) diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 554ae9d159..b9aa4d5e82 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -30,6 +30,8 @@ workspace. - `impl<'conn, P, CL, R> WalletWrite for WalletDb, P, CL, R>` to enable calling `WalletWrite` methods inside `WalletDb::transactionally` (amortizing the database transaction overhead). +- `WalletDb::get_unspent_orchard_notes_at_historical_height` returns all Orchard + notes that existed and were unspent at a given height. ### Changed - The `accounts` table now stores IVK item caches instead of FVK item caches for diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 321d9a3395..c0ea14ed64 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -2400,6 +2400,28 @@ impl<'a, C: Borrow>, P: consensus::Parameters, CL: Clo } } +#[cfg(feature = "orchard")] +impl, P: consensus::Parameters, CL, R> WalletDb { + /// Return all Orchard notes received at or before `height` + /// and unspent as of that height, for the given account. + /// + /// Unlike [`InputSource::select_unspent_notes`] (which applies confirmation, + /// dust, and expiry filters for transaction construction), this returns every + /// note that existed and was unspent at the given height. + pub fn get_unspent_orchard_notes_at_historical_height( + &self, + account: AccountUuid, + height: BlockHeight, + ) -> Result>, SqliteClientError> { + wallet::orchard::get_unspent_orchard_notes_at_historical_height( + self.conn.borrow(), + &self.params, + account, + height, + ) + } +} + /// A handle for the SQLite block source. pub struct BlockDb(rusqlite::Connection); diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 1d15380464..441a4ab1d8 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -167,6 +167,56 @@ pub(crate) fn select_spendable_orchard_notes( ) } +/// Return all Orchard notes that were received at or before `height` +/// and unspent as of `height`, for the given account. +/// +/// Unlike `select_spendable_notes` (which applies confirmation, dust, and +/// expiry filters for transaction construction), this returns every note +/// that existed and was unspent at the given height. +pub(crate) fn get_unspent_orchard_notes_at_historical_height( + conn: &Connection, + params: &P, + account: AccountUuid, + height: BlockHeight, +) -> Result>, SqliteClientError> { + let mut stmt = conn.prepare_cached( + "SELECT + rn.id AS id, t.txid, rn.action_index, + rn.diversifier, rn.value, rn.rho, rn.rseed, rn.commitment_tree_position, + accounts.ufvk AS ufvk, rn.recipient_key_scope, + t.block AS mined_height, + NULL AS max_shielding_input_height + FROM orchard_received_notes rn + INNER JOIN accounts ON accounts.id = rn.account_id + INNER JOIN transactions t ON t.id_tx = rn.transaction_id + WHERE accounts.uuid = :account_uuid + AND t.block IS NOT NULL + AND t.block <= :height + AND rn.nf IS NOT NULL + AND rn.commitment_tree_position IS NOT NULL + AND rn.recipient_key_scope IN (0, 1) + AND accounts.ufvk IS NOT NULL + AND rn.id NOT IN ( + SELECT rns.orchard_received_note_id + FROM orchard_received_note_spends rns + JOIN transactions t_spend ON t_spend.id_tx = rns.transaction_id + WHERE t_spend.block IS NOT NULL + AND t_spend.block <= :height + ) + ORDER BY rn.commitment_tree_position", + )?; + + let rows = stmt.query_and_then( + named_params![ + ":account_uuid": account.0, + ":height": u32::from(height), + ], + |row| to_received_note(params, row), + )?; + + rows.filter_map(|r| r.transpose()).collect() +} + pub(crate) fn ensure_address< T: ReceivedOrchardOutput, P: consensus::Parameters, @@ -639,4 +689,80 @@ pub(crate) mod tests { fn coinbase_only_filtering() { testing::pool::coinbase_only_filtering::(); } + + #[test] + #[cfg(feature = "orchard")] + fn get_unspent_orchard_notes_at_historical_height_boundary_heights() { + use zcash_client_backend::data_api::Account; + use zcash_client_backend::data_api::testing::{ + AddressType, TestBuilder, pool::ShieldedPoolTester, + }; + use zcash_primitives::block::BlockHash; + use zcash_protocol::value::Zatoshis; + + use crate::testing::{BlockCache, db::TestDbFactory}; + + let mut st = TestBuilder::new() + .with_data_store_factory(TestDbFactory::default()) + .with_block_cache(BlockCache::new()) + .with_account_from_sapling_activation(BlockHash([0; 32])) + .build(); + + let account = st.test_account().cloned().unwrap(); + let dfvk = OrchardPoolTester::test_account_fvk(&st); + + // Receive a note at h1 + let value = Zatoshis::const_from_u64(50000); + let (h1, _, nf) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + st.scan_cached_blocks(h1, 1); + + // Spend that note at h2 (produces change back to us) + let not_our_key = OrchardPoolTester::sk_to_fvk(&OrchardPoolTester::sk(&[0xf5; 32])); + let to = OrchardPoolTester::fvk_default_address(¬_our_key); + let spend_value = Zatoshis::const_from_u64(20000); + let (h2, _) = st.generate_next_block_spending(&dfvk, (nf, value), to, spend_value); + st.scan_cached_blocks(h2, 1); + + // Receive another note at h3 + let value3 = Zatoshis::const_from_u64(70000); + let (h3, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value3); + st.scan_cached_blocks(h3, 1); + + let db = st.wallet().db(); + + // Before any notes: nothing (h1 - 1 is before the note was mined) + let notes = db + .get_unspent_orchard_notes_at_historical_height(account.id(), h1 - 1) + .unwrap(); + assert_eq!(notes.len(), 0); + + // At h1: original note received and unspent + let notes = db + .get_unspent_orchard_notes_at_historical_height(account.id(), h1) + .unwrap(); + assert_eq!(notes.len(), 1); + assert_eq!(notes[0].note_value().unwrap(), value); + + // At h2: original spent, only change note remains + let notes = db + .get_unspent_orchard_notes_at_historical_height(account.id(), h2) + .unwrap(); + assert_eq!(notes.len(), 1); + assert_eq!( + notes[0].note_value().unwrap(), + (value - spend_value).unwrap() + ); + + // At h3: change note + new note + let notes = db + .get_unspent_orchard_notes_at_historical_height(account.id(), h3) + .unwrap(); + assert_eq!(notes.len(), 2); + let total: Zatoshis = notes + .iter() + .map(|n| n.note_value().unwrap()) + .sum::>() + .unwrap(); + assert_eq!(total, ((value - spend_value).unwrap() + value3).unwrap()); + } } From a751733a04a74885d712014c657b8c53229a3fc8 Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 10 Apr 2026 23:27:43 -0600 Subject: [PATCH 03/12] zcash_client_sqlite: Add historical height witness generation Add `WalletDb::generate_orchard_witnesses_at_historical_height`, which produces Orchard Merkle witnesses anchored at an earlier tree state. This is needed for applications such as token-holder voting, where the wallet tree has advanced past the height at which witnesses are required. The implementation copies shard and cap data into an ephemeral in-memory database, inserts the caller-provided frontier as a checkpoint, and generates a witness for each requested note position. The wallet DB is strictly read-only. Made-with: Cursor --- zcash_client_sqlite/CHANGELOG.md | 2 + zcash_client_sqlite/src/lib.rs | 32 +++ .../src/wallet/commitment_tree.rs | 266 ++++++++++++++++++ 3 files changed, 300 insertions(+) diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index b9aa4d5e82..61d558e585 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -32,6 +32,8 @@ workspace. database transaction overhead). - `WalletDb::get_unspent_orchard_notes_at_historical_height` returns all Orchard notes that existed and were unspent at a given height. +- `WalletDb::generate_orchard_witnesses_at_historical_height` generates Merkle + witnesses at a historical height using an ephemeral in-memory tree. ### Changed - The `accounts` table now stores IVK item caches instead of FVK item caches for diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index c0ea14ed64..58fe8d5901 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -2420,6 +2420,38 @@ impl, P: consensus::Parameters, CL, R> WalletDb< height, ) } + + /// Generates Orchard Merkle witnesses at a historical height. + /// + /// Copies the wallet's Orchard shard data into an ephemeral in-memory database, + /// inserts the provided frontier at `height` as a checkpoint, and generates a + /// witness for each of the given note positions. + /// + /// The caller must provide the valid frontier at the given height. The wallet DB + /// is strictly read-only; shard data is copied, not modified. + pub fn generate_orchard_witnesses_at_historical_height( + &self, + note_positions: &[Position], + frontier_at_height: incrementalmerkletree::frontier::NonEmptyFrontier< + orchard::tree::MerkleHashOrchard, + >, + height: BlockHeight, + ) -> Result< + Vec< + incrementalmerkletree::MerklePath< + orchard::tree::MerkleHashOrchard, + { orchard::NOTE_COMMITMENT_TREE_DEPTH as u8 }, + >, + >, + SqliteClientError, + > { + wallet::commitment_tree::generate_orchard_witnesses_at_historical_height( + self.conn.borrow(), + note_positions, + frontier_at_height, + height, + ) + } } /// A handle for the SQLite block source. diff --git a/zcash_client_sqlite/src/wallet/commitment_tree.rs b/zcash_client_sqlite/src/wallet/commitment_tree.rs index e1a9ad516e..246a8a00c8 100644 --- a/zcash_client_sqlite/src/wallet/commitment_tree.rs +++ b/zcash_client_sqlite/src/wallet/commitment_tree.rs @@ -1171,6 +1171,180 @@ pub(crate) fn check_witnesses( Ok(scan_ranges) } +/// Generate Orchard Merkle witnesses at a historical height. +/// +/// Copies the wallet's Orchard shard data into an ephemeral in-memory database, +/// inserts the provided frontier at that height as a checkpoint, and +/// generates a witness for each of the given note positions. +/// +/// It is assumed that the caller provides the valid frontier at the given height. +/// +/// How it works: +/// To construct witnesses at a historical height, we need: +/// 1. Authentication path within each note's shard — the scanner marks the +/// wallet's notes as MARKED, preventing them and their siblings within a +/// shard from being pruned. +/// 2. Cap — the upper tree above the shard level. +/// 3. Frontier — the right edge at the historical height. It lets ShardTree +/// know exactly where the tree ended at that height. +/// +/// The wallet automatically prunes the tree after PRUNING_DEPTH checkpoints. +/// These three components are sufficient to reconstruct the tree structure +/// needed for witness generation even after pruning has occurred. +/// +/// The wallet DB is strictly read-only. Shard data is copied, not modified. +/// An ephemeral in-memory database is created to avoid tampering with the primary wallet DB. +/// +/// Example application: token holder voting. The wallet tree may have advanced past +/// the historical height, but we need witnesses anchored at that frontier. +#[cfg(feature = "orchard")] +pub(crate) fn generate_orchard_witnesses_at_historical_height( + conn: &rusqlite::Connection, + note_positions: &[Position], + frontier_at_height: incrementalmerkletree::frontier::NonEmptyFrontier< + orchard::tree::MerkleHashOrchard, + >, + height: BlockHeight, +) -> Result< + Vec< + incrementalmerkletree::MerklePath< + orchard::tree::MerkleHashOrchard, + { orchard::NOTE_COMMITMENT_TREE_DEPTH as u8 }, + >, + >, + SqliteClientError, +> { + use incrementalmerkletree::Marking; + use shardtree::ShardTree; + use shardtree::store::Checkpoint; + use zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT; + + let frontier_position = frontier_at_height.position(); + + let mem_conn = rusqlite::Connection::open_in_memory().map_err(SqliteClientError::DbError)?; + create_orchard_tree_tables(&mem_conn).map_err(SqliteClientError::DbError)?; + copy_orchard_tree_data(conn, &mem_conn).map_err(SqliteClientError::DbError)?; + + let store = SqliteShardStore::< + _, + orchard::tree::MerkleHashOrchard, + ORCHARD_SHARD_HEIGHT, + >::from_connection(mem_conn, "orchard") + .map_err(SqliteClientError::DbError)?; + + // Only one checkpoint is needed (the historical frontier), but ShardTree + // requires a nonzero checkpoint limit. + let mut tree = + ShardTree::<_, { orchard::NOTE_COMMITMENT_TREE_DEPTH as u8 }, ORCHARD_SHARD_HEIGHT>::new( + store, 1, + ); + + // Insert frontier + checkpoint + tree.insert_frontier_nodes( + frontier_at_height, + Retention::Checkpoint { + id: height, + marking: Marking::None, + }, + ) + .map_err(|e| { + SqliteClientError::CorruptedData(format!("failed to insert frontier nodes: {}", e)) + })?; + + tree.store_mut() + .add_checkpoint(height, Checkpoint::at_position(frontier_position)) + .map_err(|e| { + SqliteClientError::CorruptedData(format!("failed to add checkpoint: {}", e)) + })?; + + // Generate witness per note position + let mut witnesses = Vec::with_capacity(note_positions.len()); + for &pos in note_positions { + let merkle_path = tree + .witness_at_checkpoint_id(pos, &height) + .map_err(|e| { + SqliteClientError::CorruptedData(format!( + "failed to generate witness for position {}: {} \ + (wallet may need to sync through historical height)", + u64::from(pos), + e + )) + })? + .ok_or_else(|| { + SqliteClientError::CorruptedData(format!( + "no witness available for position {} \ + (wallet missing shard data — sync through historical height)", + u64::from(pos) + )) + })?; + + witnesses.push(merkle_path); + } + + Ok(witnesses) +} + +/// Create the Orchard shard-tree schema tables in an existing connection. +/// +/// Reuses the canonical DDL constants from [`super::db`] so that the +/// ephemeral schema stays in sync with the wallet schema. +#[cfg(feature = "orchard")] +fn create_orchard_tree_tables(conn: &rusqlite::Connection) -> Result<(), rusqlite::Error> { + use super::db::{ + TABLE_ORCHARD_TREE_CAP, TABLE_ORCHARD_TREE_CHECKPOINT_MARKS_REMOVED, + TABLE_ORCHARD_TREE_CHECKPOINTS, TABLE_ORCHARD_TREE_SHARDS, + }; + conn.execute_batch(&format!( + "{TABLE_ORCHARD_TREE_SHARDS};\ + {TABLE_ORCHARD_TREE_CAP};\ + {TABLE_ORCHARD_TREE_CHECKPOINTS};\ + {TABLE_ORCHARD_TREE_CHECKPOINT_MARKS_REMOVED};", + )) +} + +/// Copy Orchard shard and cap data from one connection to another. +/// +/// Both connections must already have the `orchard_tree_shards` and +/// `orchard_tree_cap` tables (see [`create_orchard_tree_tables`]). +#[cfg(feature = "orchard")] +fn copy_orchard_tree_data( + src: &rusqlite::Connection, + dst: &rusqlite::Connection, +) -> Result<(), rusqlite::Error> { + use rusqlite::types::Value; + + let mut stmt = src.prepare( + "SELECT shard_index, subtree_end_height, root_hash, shard_data, contains_marked + FROM orchard_tree_shards", + )?; + let mut rows = stmt.query([])?; + while let Some(row) = rows.next()? { + dst.execute( + "INSERT INTO orchard_tree_shards + (shard_index, subtree_end_height, root_hash, shard_data, contains_marked) + VALUES (?1, ?2, ?3, ?4, ?5)", + rusqlite::params![ + row.get::<_, Value>(0)?, + row.get::<_, Value>(1)?, + row.get::<_, Value>(2)?, + row.get::<_, Value>(3)?, + row.get::<_, Value>(4)?, + ], + )?; + } + + let mut stmt = src.prepare("SELECT cap_id, cap_data FROM orchard_tree_cap")?; + let mut rows = stmt.query([])?; + while let Some(row) = rows.next()? { + dst.execute( + "INSERT INTO orchard_tree_cap (cap_id, cap_data) VALUES (?1, ?2)", + rusqlite::params![row.get::<_, Value>(0)?, row.get::<_, Value>(1)?], + )?; + } + + Ok(()) +} + #[cfg(test)] mod tests { use tempfile::NamedTempFile; @@ -1257,6 +1431,11 @@ mod tests { super::check_rewind_remove_mark(new_tree::); } + #[test] + fn witnesses_at_historical_height() { + super::witnesses_at_historical_height() + } + #[test] fn put_shard_roots() { super::put_shard_roots::() @@ -1373,4 +1552,91 @@ mod tests { ] ); } + + /// Test that `generate_orchard_witnesses_at_historical_height` produces valid + /// witnesses when given a frontier extracted from an earlier tree state. + #[cfg(feature = "orchard")] + fn witnesses_at_historical_height() { + use ::orchard::tree::MerkleHashOrchard; + use incrementalmerkletree::frontier::Frontier; + use rand::SeedableRng; + use rand_chacha::ChaChaRng; + use zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT; + + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path( + data_file.path(), + Network::TestNetwork, + test_clock(), + test_rng(), + ) + .unwrap(); + data_file.keep().unwrap(); + + WalletMigrator::new().init_or_migrate(&mut db_data).unwrap(); + + let mut rng = ChaChaRng::seed_from_u64(0); + + // We build two parallel trees: the wallet's ShardTree (persisted to the DB) + // and a lightweight Frontier that captures the tree state at the historical height. + let mut frontier_tree: Frontier = Frontier::empty(); + let historical_height = BlockHeight::from(100); + let note_position = Position::from(2); + let note_leaf; + + { + let tx = db_data.conn.transaction().unwrap(); + let store = + SqliteShardStore::<_, MerkleHashOrchard, ORCHARD_SHARD_HEIGHT>::from_connection( + &tx, "orchard", + ) + .unwrap(); + let mut tree = ShardTree::< + _, + { ::orchard::NOTE_COMMITMENT_TREE_DEPTH as u8 }, + ORCHARD_SHARD_HEIGHT, + >::new(store, 100); + + let mut leaves = Vec::new(); + for _ in 0u64..5 { + leaves.push(MerkleHashOrchard::random(&mut rng)); + } + note_leaf = leaves[u64::from(note_position) as usize]; + + for (i, &leaf) in leaves.iter().enumerate() { + let retention = if i == u64::from(note_position) as usize { + Retention::Marked + } else { + Retention::Ephemeral + }; + tree.append(leaf, retention).unwrap(); + frontier_tree.append(leaf); + } + + // Advance the tree past the historical height, simulating the + // wallet continuing to sync afterward. + tree.checkpoint(historical_height).unwrap(); + for _ in 0..5 { + tree.append(MerkleHashOrchard::random(&mut rng), Retention::Ephemeral) + .unwrap(); + } + tree.checkpoint(BlockHeight::from(200)).unwrap(); + + tx.commit().unwrap(); + } + + let expected_root = frontier_tree.root(); + let frontier = frontier_tree.take().expect("frontier is non-empty"); + + let witnesses = super::generate_orchard_witnesses_at_historical_height( + &db_data.conn, + &[note_position], + frontier, + historical_height, + ) + .expect("witness generation should succeed"); + + assert_eq!(witnesses.len(), 1); + assert_eq!(witnesses[0].root(note_leaf), expected_root); + } } From 3f4e63f98e270a1e2434f9eb709ca4ff4b85ac98 Mon Sep 17 00:00:00 2001 From: roman Date: Sat, 11 Apr 2026 12:04:21 -0600 Subject: [PATCH 04/12] zcash_client_sqlite: Move function-scoped imports to module level Moves `Marking`, `ShardTree`, `Checkpoint`, and `ORCHARD_SHARD_HEIGHT` imports out of `generate_orchard_witnesses_at_historical_height` to the module-level import block, consistent with the rest of the codebase. Made-with: Cursor --- zcash_client_sqlite/src/wallet/commitment_tree.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/commitment_tree.rs b/zcash_client_sqlite/src/wallet/commitment_tree.rs index 246a8a00c8..8f8c28188f 100644 --- a/zcash_client_sqlite/src/wallet/commitment_tree.rs +++ b/zcash_client_sqlite/src/wallet/commitment_tree.rs @@ -12,9 +12,9 @@ use std::{ sync::Arc, }; -use incrementalmerkletree::{Address, Hashable, Level, Position, Retention}; +use incrementalmerkletree::{Address, Hashable, Level, Marking, Position, Retention}; use shardtree::{ - LocatedPrunableTree, LocatedTree, PrunableTree, RetentionFlags, + LocatedPrunableTree, LocatedTree, PrunableTree, RetentionFlags, ShardTree, error::{QueryError, ShardTreeError}, store::{Checkpoint, ShardStore, TreeState}, }; @@ -30,6 +30,8 @@ use crate::{error::SqliteClientError, sapling_tree}; #[cfg(feature = "orchard")] use crate::orchard_tree; +#[cfg(feature = "orchard")] +use zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT; use super::common::{TableConstants, table_constants}; @@ -1214,11 +1216,6 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( >, SqliteClientError, > { - use incrementalmerkletree::Marking; - use shardtree::ShardTree; - use shardtree::store::Checkpoint; - use zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT; - let frontier_position = frontier_at_height.position(); let mem_conn = rusqlite::Connection::open_in_memory().map_err(SqliteClientError::DbError)?; From d7ade3e665d27ec2a67be2e0a7349bec4325c2f5 Mon Sep 17 00:00:00 2001 From: roman Date: Thu, 23 Apr 2026 15:55:55 -0300 Subject: [PATCH 05/12] zcash_client_sqlite: Use MemoryShardStore for historical witness generation Replaces the in-memory SQLite scratch DB used by `generate_orchard_witnesses_at_historical_height` with shardtree's `MemoryShardStore`. The wallet DB shards and cap are loaded directly into the in-memory `ShardStore`, avoiding the round-trip through SQLite DDL, row-by-row blob copying, and the dependency on `TABLE_ORCHARD_TREE_*` schema constants. Behavior is preserved: the same `LocatedPrunableTree` values end up in the store (both paths go through `get_shard`/`get_cap`), the ShardTree algorithms are unchanged, and the public API is identical. A side benefit is that corrupt shard blobs are now detected at load time rather than lazily on first access. Addresses review feedback from @nuttycom on #2283. Made-with: Cursor Co-Authored-By: Claude --- zcash_client_sqlite/CHANGELOG.md | 3 +- zcash_client_sqlite/src/lib.rs | 8 +- .../src/wallet/commitment_tree.rs | 108 +++++------------- 3 files changed, 34 insertions(+), 85 deletions(-) diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 61d558e585..aae252d4ed 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -33,7 +33,8 @@ workspace. - `WalletDb::get_unspent_orchard_notes_at_historical_height` returns all Orchard notes that existed and were unspent at a given height. - `WalletDb::generate_orchard_witnesses_at_historical_height` generates Merkle - witnesses at a historical height using an ephemeral in-memory tree. + witnesses at a historical height using an ephemeral in-memory + `shardtree::store::memory::MemoryShardStore`. ### Changed - The `accounts` table now stores IVK item caches instead of FVK item caches for diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 58fe8d5901..ab5914856a 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -2423,12 +2423,12 @@ impl, P: consensus::Parameters, CL, R> WalletDb< /// Generates Orchard Merkle witnesses at a historical height. /// - /// Copies the wallet's Orchard shard data into an ephemeral in-memory database, - /// inserts the provided frontier at `height` as a checkpoint, and generates a - /// witness for each of the given note positions. + /// Loads the wallet's Orchard shard data into an ephemeral in-memory + /// `ShardStore`, inserts the provided frontier at `height` as a checkpoint, + /// and generates a witness for each of the given note positions. /// /// The caller must provide the valid frontier at the given height. The wallet DB - /// is strictly read-only; shard data is copied, not modified. + /// is strictly read-only; shard data is read but not modified. pub fn generate_orchard_witnesses_at_historical_height( &self, note_positions: &[Position], diff --git a/zcash_client_sqlite/src/wallet/commitment_tree.rs b/zcash_client_sqlite/src/wallet/commitment_tree.rs index 8f8c28188f..5e8643d9df 100644 --- a/zcash_client_sqlite/src/wallet/commitment_tree.rs +++ b/zcash_client_sqlite/src/wallet/commitment_tree.rs @@ -29,10 +29,13 @@ use zcash_protocol::{ShieldedProtocol, consensus::BlockHeight}; use crate::{error::SqliteClientError, sapling_tree}; #[cfg(feature = "orchard")] -use crate::orchard_tree; +use shardtree::store::memory::MemoryShardStore; #[cfg(feature = "orchard")] use zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT; +#[cfg(feature = "orchard")] +use crate::{ORCHARD_TABLES_PREFIX, orchard_tree}; + use super::common::{TableConstants, table_constants}; /// Errors that can appear in SQLite-back [`ShardStore`] implementation operations. @@ -1175,9 +1178,9 @@ pub(crate) fn check_witnesses( /// Generate Orchard Merkle witnesses at a historical height. /// -/// Copies the wallet's Orchard shard data into an ephemeral in-memory database, -/// inserts the provided frontier at that height as a checkpoint, and -/// generates a witness for each of the given note positions. +/// Loads the wallet's Orchard shard data into an ephemeral in-memory +/// [`MemoryShardStore`], inserts the provided frontier at that height as a +/// checkpoint, and generates a witness for each of the given note positions. /// /// It is assumed that the caller provides the valid frontier at the given height. /// @@ -1194,8 +1197,9 @@ pub(crate) fn check_witnesses( /// These three components are sufficient to reconstruct the tree structure /// needed for witness generation even after pruning has occurred. /// -/// The wallet DB is strictly read-only. Shard data is copied, not modified. -/// An ephemeral in-memory database is created to avoid tampering with the primary wallet DB. +/// The wallet DB is strictly read-only. Shard data is read, decoded, and +/// inserted into an ephemeral in-memory [`ShardStore`] to avoid tampering with +/// the primary wallet DB. /// /// Example application: token holder voting. The wallet tree may have advanced past /// the historical height, but we need witnesses anchored at that frontier. @@ -1218,16 +1222,23 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( > { let frontier_position = frontier_at_height.position(); - let mem_conn = rusqlite::Connection::open_in_memory().map_err(SqliteClientError::DbError)?; - create_orchard_tree_tables(&mem_conn).map_err(SqliteClientError::DbError)?; - copy_orchard_tree_data(conn, &mem_conn).map_err(SqliteClientError::DbError)?; - - let store = SqliteShardStore::< - _, - orchard::tree::MerkleHashOrchard, - ORCHARD_SHARD_HEIGHT, - >::from_connection(mem_conn, "orchard") - .map_err(SqliteClientError::DbError)?; + // `get_shard_roots` returns addresses ordered by shard index, matching the + // ascending insertion order required by `MemoryShardStore::put_shard`. + let mut store = MemoryShardStore::::empty(); + let shard_root_level = Level::new(ORCHARD_SHARD_HEIGHT); + let shard_roots = get_shard_roots(conn, ORCHARD_TABLES_PREFIX, shard_root_level) + .map_err(ShardTreeError::Storage)?; + for shard_root in shard_roots { + if let Some(shard) = + get_shard::(conn, ORCHARD_TABLES_PREFIX, shard_root) + .map_err(ShardTreeError::Storage)? + { + store.put_shard(shard).expect("put_shard is infallible"); + } + } + let cap = get_cap::(conn, ORCHARD_TABLES_PREFIX) + .map_err(ShardTreeError::Storage)?; + store.put_cap(cap).expect("put_cap is infallible"); // Only one checkpoint is needed (the historical frontier), but ShardTree // requires a nonzero checkpoint limit. @@ -1250,9 +1261,7 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( tree.store_mut() .add_checkpoint(height, Checkpoint::at_position(frontier_position)) - .map_err(|e| { - SqliteClientError::CorruptedData(format!("failed to add checkpoint: {}", e)) - })?; + .expect("add_checkpoint is infallible"); // Generate witness per note position let mut witnesses = Vec::with_capacity(note_positions.len()); @@ -1281,67 +1290,6 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( Ok(witnesses) } -/// Create the Orchard shard-tree schema tables in an existing connection. -/// -/// Reuses the canonical DDL constants from [`super::db`] so that the -/// ephemeral schema stays in sync with the wallet schema. -#[cfg(feature = "orchard")] -fn create_orchard_tree_tables(conn: &rusqlite::Connection) -> Result<(), rusqlite::Error> { - use super::db::{ - TABLE_ORCHARD_TREE_CAP, TABLE_ORCHARD_TREE_CHECKPOINT_MARKS_REMOVED, - TABLE_ORCHARD_TREE_CHECKPOINTS, TABLE_ORCHARD_TREE_SHARDS, - }; - conn.execute_batch(&format!( - "{TABLE_ORCHARD_TREE_SHARDS};\ - {TABLE_ORCHARD_TREE_CAP};\ - {TABLE_ORCHARD_TREE_CHECKPOINTS};\ - {TABLE_ORCHARD_TREE_CHECKPOINT_MARKS_REMOVED};", - )) -} - -/// Copy Orchard shard and cap data from one connection to another. -/// -/// Both connections must already have the `orchard_tree_shards` and -/// `orchard_tree_cap` tables (see [`create_orchard_tree_tables`]). -#[cfg(feature = "orchard")] -fn copy_orchard_tree_data( - src: &rusqlite::Connection, - dst: &rusqlite::Connection, -) -> Result<(), rusqlite::Error> { - use rusqlite::types::Value; - - let mut stmt = src.prepare( - "SELECT shard_index, subtree_end_height, root_hash, shard_data, contains_marked - FROM orchard_tree_shards", - )?; - let mut rows = stmt.query([])?; - while let Some(row) = rows.next()? { - dst.execute( - "INSERT INTO orchard_tree_shards - (shard_index, subtree_end_height, root_hash, shard_data, contains_marked) - VALUES (?1, ?2, ?3, ?4, ?5)", - rusqlite::params![ - row.get::<_, Value>(0)?, - row.get::<_, Value>(1)?, - row.get::<_, Value>(2)?, - row.get::<_, Value>(3)?, - row.get::<_, Value>(4)?, - ], - )?; - } - - let mut stmt = src.prepare("SELECT cap_id, cap_data FROM orchard_tree_cap")?; - let mut rows = stmt.query([])?; - while let Some(row) = rows.next()? { - dst.execute( - "INSERT INTO orchard_tree_cap (cap_id, cap_data) VALUES (?1, ?2)", - rusqlite::params![row.get::<_, Value>(0)?, row.get::<_, Value>(1)?], - )?; - } - - Ok(()) -} - #[cfg(test)] mod tests { use tempfile::NamedTempFile; From 216281df038a0d627b1e6dc65bd83c43d2af87c5 Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 24 Apr 2026 15:18:54 -0300 Subject: [PATCH 06/12] zcash_client_sqlite: Remove redundant add_checkpoint in historical witness generation `ShardTree::insert_frontier_nodes` registers a checkpoint internally when called with `Retention::Checkpoint { id, .. }`, so the explicit `tree.store_mut().add_checkpoint(height, ...)` immediately after it was dead weight at best and confusing at worst: a reader is left wondering whether the frontier insertion really registered the checkpoint and why we're doing it again. Drop the redundant call (and the now-unused `frontier_position` binding) and replace the "Insert frontier + checkpoint" comment with one that names `Retention::Checkpoint` as the mechanism doing the work. Also move the `Marking` and `ShardTree` imports behind `#[cfg(feature = "orchard")]`: they are only used inside the orchard-gated function, and the top-level imports produced `unused_imports` warnings under `cargo check --no-default-features`. Behavior is unchanged in all feature configurations. Addresses review feedback from @nuttycom on #2283. Made-with: Cursor Co-Authored-By: Claude --- .../src/wallet/commitment_tree.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/commitment_tree.rs b/zcash_client_sqlite/src/wallet/commitment_tree.rs index 5e8643d9df..cd88c03b49 100644 --- a/zcash_client_sqlite/src/wallet/commitment_tree.rs +++ b/zcash_client_sqlite/src/wallet/commitment_tree.rs @@ -12,9 +12,9 @@ use std::{ sync::Arc, }; -use incrementalmerkletree::{Address, Hashable, Level, Marking, Position, Retention}; +use incrementalmerkletree::{Address, Hashable, Level, Position, Retention}; use shardtree::{ - LocatedPrunableTree, LocatedTree, PrunableTree, RetentionFlags, ShardTree, + LocatedPrunableTree, LocatedTree, PrunableTree, RetentionFlags, error::{QueryError, ShardTreeError}, store::{Checkpoint, ShardStore, TreeState}, }; @@ -29,7 +29,9 @@ use zcash_protocol::{ShieldedProtocol, consensus::BlockHeight}; use crate::{error::SqliteClientError, sapling_tree}; #[cfg(feature = "orchard")] -use shardtree::store::memory::MemoryShardStore; +use incrementalmerkletree::Marking; +#[cfg(feature = "orchard")] +use shardtree::{ShardTree, store::memory::MemoryShardStore}; #[cfg(feature = "orchard")] use zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT; @@ -1220,8 +1222,6 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( >, SqliteClientError, > { - let frontier_position = frontier_at_height.position(); - // `get_shard_roots` returns addresses ordered by shard index, matching the // ascending insertion order required by `MemoryShardStore::put_shard`. let mut store = MemoryShardStore::::empty(); @@ -1247,7 +1247,9 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( store, 1, ); - // Insert frontier + checkpoint + // Insert the frontier. `Retention::Checkpoint` causes `ShardTree` to + // register a checkpoint at `height` internally, so no separate + // `add_checkpoint` call is required. tree.insert_frontier_nodes( frontier_at_height, Retention::Checkpoint { @@ -1259,10 +1261,6 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( SqliteClientError::CorruptedData(format!("failed to insert frontier nodes: {}", e)) })?; - tree.store_mut() - .add_checkpoint(height, Checkpoint::at_position(frontier_position)) - .expect("add_checkpoint is infallible"); - // Generate witness per note position let mut witnesses = Vec::with_capacity(note_positions.len()); for &pos in note_positions { From b1fd19fb8b58eaec1c3bd6d528edf90ddc146648 Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 24 Apr 2026 15:21:14 -0300 Subject: [PATCH 07/12] zcash_client_sqlite: Return actionable errors from generate_orchard_witnesses_at_historical_height MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the function funneled every internal `ShardTree` failure through `SqliteClientError::CorruptedData(String)`, including the `(wallet may need to sync through historical height)` hint. That was misleading on two counts: - `CorruptedData` is documented for wallet-database corruption, not for recoverable caller conditions such as a frontier mismatch or an out-of-sync wallet. - Collapsing every case to a formatted string forces the caller to string-match on the message to tell "the wallet hasn't reached this height yet" from "the supplied frontier is wrong", which is both fragile and not actionable. This commit adds two `orchard`-gated variants to `SqliteClientError` that name the remaining failure modes precisely: - `HistoricalFrontierInvalid(shardtree::error::InsertionError)` — the caller-supplied frontier is inconsistent with the shard data reconstructed from the wallet. Wraps the underlying `InsertionError` and surfaces it through `Error::source`. - `HistoricalWitnessUnavailable { position, height }` — no witness can be produced for the specified position at the specified height (most commonly because the wallet has not synced through that height, the checkpoint was pruned, or the position does not belong to the wallet). Shard-read failures already had a home in `SqliteClientError::CommitmentTree`, so those now flow through it directly via the existing `From>` impl instead of being stringified. The only `ShardTreeError` variant that cannot occur in this function is `Storage(Infallible)` — the in-memory store's error type is `Infallible`, so those arms are `match inf {}`. Docs for `WalletDb::generate_orchard_witnesses_at_historical_height` and the internal `pub(crate)` helper now include `# Errors` sections that enumerate the concrete variants, and `CHANGELOG.md` records the new public variants. `sqlite_client_error_to_wallet_migration_error` is updated to exhaustively match these new variants (both `unreachable!()` — we do not generate historical witnesses in migrations). Addresses review feedback from @nuttycom on #2283. Made-with: Cursor Co-Authored-By: Claude --- zcash_client_sqlite/CHANGELOG.md | 11 ++++ zcash_client_sqlite/src/error.rs | 46 +++++++++++++++ zcash_client_sqlite/src/lib.rs | 15 +++++ .../src/wallet/commitment_tree.rs | 58 +++++++++++++------ zcash_client_sqlite/src/wallet/init.rs | 5 ++ 5 files changed, 117 insertions(+), 18 deletions(-) diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index aae252d4ed..e229d0131b 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -35,6 +35,17 @@ workspace. - `WalletDb::generate_orchard_witnesses_at_historical_height` generates Merkle witnesses at a historical height using an ephemeral in-memory `shardtree::store::memory::MemoryShardStore`. +- Two new `orchard`-gated variants have been added to + `zcash_client_sqlite::error::SqliteClientError` to surface the failure modes + of `WalletDb::generate_orchard_witnesses_at_historical_height`: + - `HistoricalFrontierInvalid(shardtree::error::InsertionError)` — + the caller-supplied frontier is inconsistent with the shard data + reconstructed from the wallet at the requested height. + - `HistoricalWitnessUnavailable { position, height }` — no witness can be + produced for the specified position at the specified height (the wallet + most likely has not synced through that height). + Shard-read failures continue to surface via the existing + `SqliteClientError::CommitmentTree` variant. ### Changed - The `accounts` table now stores IVK item caches instead of FVK item caches for diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index db35dd18d4..6a53dddfbe 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -3,7 +3,11 @@ use std::error; use std::fmt; +#[cfg(feature = "orchard")] +use incrementalmerkletree::Position; use nonempty::NonEmpty; +#[cfg(feature = "orchard")] +use shardtree::error::InsertionError; use shardtree::error::ShardTreeError; #[cfg(feature = "transparent-key-import")] @@ -116,6 +120,35 @@ pub enum SqliteClientError { /// commitment trees. CommitmentTree(ShardTreeError), + /// The caller-supplied frontier passed to + /// [`WalletDb::generate_orchard_witnesses_at_historical_height`] is + /// inconsistent with the shard data reconstructed from the wallet at the + /// requested height. + /// + /// [`WalletDb::generate_orchard_witnesses_at_historical_height`]: + /// crate::WalletDb::generate_orchard_witnesses_at_historical_height + #[cfg(feature = "orchard")] + HistoricalFrontierInvalid(InsertionError), + + /// A witness could not be generated for the specified position at the + /// specified historical height in a call to + /// [`WalletDb::generate_orchard_witnesses_at_historical_height`]. + /// + /// The wallet most likely has not synced through `height`, the checkpoint + /// at `height` has been pruned, or `position` does not belong to the + /// wallet. + /// + /// [`WalletDb::generate_orchard_witnesses_at_historical_height`]: + /// crate::WalletDb::generate_orchard_witnesses_at_historical_height + #[cfg(feature = "orchard")] + HistoricalWitnessUnavailable { + /// The note commitment tree position for which a witness was + /// requested. + position: Position, + /// The historical height at which the witness was requested. + height: BlockHeight, + }, + /// The block at the specified height was not available from the block cache. CacheMiss(BlockHeight), @@ -187,6 +220,8 @@ impl error::Error for SqliteClientError { SqliteClientError::Io(e) => Some(e), SqliteClientError::BalanceError(e) => Some(e), SqliteClientError::AddressGeneration(e) => Some(e), + #[cfg(feature = "orchard")] + SqliteClientError::HistoricalFrontierInvalid(e) => Some(e), _ => None, } } @@ -272,6 +307,17 @@ impl fmt::Display for SqliteClientError { f, "An error occurred accessing or updating note commitment tree data: {err}." ), + #[cfg(feature = "orchard")] + SqliteClientError::HistoricalFrontierInvalid(err) => write!( + f, + "The frontier supplied to generate_orchard_witnesses_at_historical_height is inconsistent with the wallet's shard data: {err}" + ), + #[cfg(feature = "orchard")] + SqliteClientError::HistoricalWitnessUnavailable { position, height } => write!( + f, + "No witness is available for position {} at height {height} (the wallet may need to sync through this height).", + u64::from(*position), + ), SqliteClientError::CacheMiss(height) => write!( f, "Requested height {height} does not exist in the block cache." diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index ab5914856a..2cbfa46477 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -2429,6 +2429,21 @@ impl, P: consensus::Parameters, CL, R> WalletDb< /// /// The caller must provide the valid frontier at the given height. The wallet DB /// is strictly read-only; shard data is read but not modified. + /// + /// # Errors + /// + /// Returns: + /// - [`SqliteClientError::CommitmentTree`] if reading the wallet's shard + /// or cap data fails, or if the shard data reconstructed from the + /// wallet is internally inconsistent at a node the computation + /// requires. + /// - [`SqliteClientError::HistoricalFrontierInvalid`] if + /// `frontier_at_height` is inconsistent with the shard data + /// reconstructed from the wallet at `height`. + /// - [`SqliteClientError::HistoricalWitnessUnavailable`] if a witness + /// cannot be generated for one of `note_positions` at `height` (most + /// commonly because the wallet has not yet synced through that + /// height). pub fn generate_orchard_witnesses_at_historical_height( &self, note_positions: &[Position], diff --git a/zcash_client_sqlite/src/wallet/commitment_tree.rs b/zcash_client_sqlite/src/wallet/commitment_tree.rs index cd88c03b49..c23e5b714d 100644 --- a/zcash_client_sqlite/src/wallet/commitment_tree.rs +++ b/zcash_client_sqlite/src/wallet/commitment_tree.rs @@ -1205,6 +1205,17 @@ pub(crate) fn check_witnesses( /// /// Example application: token holder voting. The wallet tree may have advanced past /// the historical height, but we need witnesses anchored at that frontier. +/// +/// # Errors +/// +/// - [`SqliteClientError::CommitmentTree`] if reading the wallet's shard or +/// cap data fails, or if the shard data reconstructed from the wallet is +/// internally inconsistent at a node the computation requires. +/// - [`SqliteClientError::HistoricalFrontierInvalid`] if `frontier_at_height` +/// is inconsistent with the shard data reconstructed from the wallet. +/// - [`SqliteClientError::HistoricalWitnessUnavailable`] if a witness cannot +/// be generated for one of the requested positions at `height` (most +/// commonly because the wallet has not yet synced through that height). #[cfg(feature = "orchard")] pub(crate) fn generate_orchard_witnesses_at_historical_height( conn: &rusqlite::Connection, @@ -1224,6 +1235,8 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( > { // `get_shard_roots` returns addresses ordered by shard index, matching the // ascending insertion order required by `MemoryShardStore::put_shard`. + // Storage errors flow through `From>` + // into `SqliteClientError::CommitmentTree`. let mut store = MemoryShardStore::::empty(); let shard_root_level = Level::new(ORCHARD_SHARD_HEIGHT); let shard_roots = get_shard_roots(conn, ORCHARD_TABLES_PREFIX, shard_root_level) @@ -1248,8 +1261,12 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( ); // Insert the frontier. `Retention::Checkpoint` causes `ShardTree` to - // register a checkpoint at `height` internally, so no separate - // `add_checkpoint` call is required. + // register a checkpoint at `height` internally. + // + // `MemoryShardStore::Error` is `Infallible`, so the only variants of + // `ShardTreeError` we can observe are `Insert` (a caller-supplied + // frontier inconsistent with the loaded shards) and `Query` (an + // inconsistency in the loaded shards themselves). tree.insert_frontier_nodes( frontier_at_height, Retention::Checkpoint { @@ -1257,29 +1274,34 @@ pub(crate) fn generate_orchard_witnesses_at_historical_height( marking: Marking::None, }, ) - .map_err(|e| { - SqliteClientError::CorruptedData(format!("failed to insert frontier nodes: {}", e)) + .map_err(|e| match e { + ShardTreeError::Insert(e) => SqliteClientError::HistoricalFrontierInvalid(e), + ShardTreeError::Query(q) => SqliteClientError::CommitmentTree(ShardTreeError::Query(q)), + ShardTreeError::Storage(inf) => match inf {}, })?; - // Generate witness per note position + // Generate a witness per note position. Any `Query` failure (or a `None` + // result) at this stage means the tree reconstructed from the wallet's + // shards does not contain enough information to compute a witness at + // `height`; we surface that as `HistoricalWitnessUnavailable` so the + // caller can either sync further or stop requesting that position. let mut witnesses = Vec::with_capacity(note_positions.len()); for &pos in note_positions { let merkle_path = tree .witness_at_checkpoint_id(pos, &height) - .map_err(|e| { - SqliteClientError::CorruptedData(format!( - "failed to generate witness for position {}: {} \ - (wallet may need to sync through historical height)", - u64::from(pos), - e - )) + .map_err(|e| match e { + ShardTreeError::Query(_) => SqliteClientError::HistoricalWitnessUnavailable { + position: pos, + height, + }, + ShardTreeError::Insert(i) => { + SqliteClientError::CommitmentTree(ShardTreeError::Insert(i)) + } + ShardTreeError::Storage(inf) => match inf {}, })? - .ok_or_else(|| { - SqliteClientError::CorruptedData(format!( - "no witness available for position {} \ - (wallet missing shard data — sync through historical height)", - u64::from(pos) - )) + .ok_or(SqliteClientError::HistoricalWitnessUnavailable { + position: pos, + height, })?; witnesses.push(merkle_path); diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 1ba396cbaf..9eaa90ddc0 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -250,6 +250,11 @@ fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> Wallet SqliteClientError::StandaloneImportConflict(_) => { unreachable!("we do not import standalone transparent addresses in migrations") } + #[cfg(feature = "orchard")] + SqliteClientError::HistoricalFrontierInvalid(_) + | SqliteClientError::HistoricalWitnessUnavailable { .. } => { + unreachable!("we do not generate historical witnesses in migrations") + } } } From 946993cd29773034cb456baca1eab4e2433c040f Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 24 Apr 2026 13:42:39 -0300 Subject: [PATCH 08/12] Resolve `orchard` via valar-orchard on crates.io MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes the workspace-level `orchard` dep from the crates.io `orchard` package (patched to zcash/orchard git rev 6b12c77) to the `valar-orchard` package on crates.io, aliased locally to `orchard` via cargo's `package =` rename trick. Every member keeps its existing `orchard.workspace = true` line and its source-level `use orchard::…` imports — no other changes needed. Why --- `valar-orchard` is upstream 0.12.0 + the same post-release fixes up to zcash/orchard 6b12c77 that the previous `[patch.crates-io] orchard` entry pointed at, plus the `pub` visibility additions that valargroup/voting-circuits needs for the shielded-voting Halo 2 circuits (constants, spec, shared_primitives gadget). Before this change, consumers that also depend on `voting-circuits` (e.g. `zcash_voting`) ended up with two distinct Orchard crates in their dep-graph: `orchard` (from this fork, via our patch) and `valar-orchard` (pulled by `voting-circuits` from crates.io). Cargo treats them as different crates regardless of content, which forced a byte-round-trip `orchard_compat` bridge at the zcash_keys / pczt boundary. Routing our fork through `valar-orchard` too collapses that back to a single node and deletes the bridge. The `[patch.crates-io] orchard = { git = "zcash/orchard", rev = "6b12c77…" }` entry is obsolete after this change (no dep resolves crates.io `orchard` anymore) and is removed. Verified: `cargo check --workspace` clean. `valar-orchard v0.12.0` appears in the build graph; no `orchard v0.12.x` or git `orchard` entry. Made-with: Cursor --- Cargo.lock | 87 +++++++++++++++++++++++++++--------------------------- Cargo.toml | 13 ++++++-- 2 files changed, 55 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d676b1b8a6..46d227d7b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2964,42 +2964,6 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" -[[package]] -name = "orchard" -version = "0.12.0" -source = "git+https://github.com/zcash/orchard.git?rev=b0bf2670e248958c6ce7c1deed466032e0dbd4d9#b0bf2670e248958c6ce7c1deed466032e0dbd4d9" -dependencies = [ - "aes", - "bitvec", - "blake2b_simd", - "corez", - "ff", - "fpe", - "getset", - "group", - "halo2_gadgets", - "halo2_poseidon", - "halo2_proofs", - "hex", - "incrementalmerkletree", - "lazy_static", - "memuse", - "nonempty", - "pasta_curves", - "proptest", - "rand 0.8.5", - "rand_core 0.6.4", - "reddsa", - "serde", - "sinsemilla", - "subtle", - "tracing", - "visibility", - "zcash_note_encryption", - "zcash_spec", - "zip32", -] - [[package]] name = "ordered-float" version = "2.10.1" @@ -3154,7 +3118,6 @@ dependencies = [ "incrementalmerkletree", "jubjub", "nonempty", - "orchard", "pasta_curves", "postcard", "rand_core 0.6.4", @@ -3166,6 +3129,7 @@ dependencies = [ "serde_with", "sha2 0.10.8", "shardtree", + "valar-orchard", "zcash_note_encryption", "zcash_primitives", "zcash_proofs", @@ -6312,6 +6276,43 @@ dependencies = [ "serde", ] +[[package]] +name = "valar-orchard" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0415fc8aba029019c974bb0b0cc47e0bca53a27fdc2da263a2e6c99c9b3727f8" +dependencies = [ + "aes", + "bitvec", + "blake2b_simd", + "corez", + "ff", + "fpe", + "getset", + "group", + "halo2_gadgets", + "halo2_poseidon", + "halo2_proofs", + "hex", + "incrementalmerkletree", + "lazy_static", + "memuse", + "nonempty", + "pasta_curves", + "proptest", + "rand 0.8.5", + "rand_core 0.6.4", + "reddsa", + "serde", + "sinsemilla", + "subtle", + "tracing", + "visibility", + "zcash_note_encryption", + "zcash_spec", + "zip32", +] + [[package]] name = "valuable" version = "0.1.0" @@ -7065,7 +7066,6 @@ dependencies = [ "jubjub", "memuse", "nonempty", - "orchard", "pasta_curves", "pczt", "percent-encoding", @@ -7096,6 +7096,7 @@ dependencies = [ "tower", "tracing", "trait-variant", + "valar-orchard", "webpki-roots 1.0.3", "which", "zcash_address", @@ -7125,7 +7126,6 @@ dependencies = [ "incrementalmerkletree", "jubjub", "nonempty", - "orchard", "postcard", "proptest", "prost", @@ -7143,6 +7143,7 @@ dependencies = [ "time", "tokio", "tracing", + "valar-orchard", "wasm_sync", "which", "zcash_address", @@ -7175,7 +7176,6 @@ dependencies = [ "jubjub", "maybe-rayon", "nonempty", - "orchard", "pasta_curves", "proptest", "prost", @@ -7198,6 +7198,7 @@ dependencies = [ "time", "tracing", "uuid", + "valar-orchard", "zcash_address", "zcash_client_backend", "zcash_encoding", @@ -7229,9 +7230,9 @@ dependencies = [ "blake2b_simd", "ff", "jubjub", - "orchard", "rand_core 0.6.4", "sapling-crypto", + "valar-orchard", "zcash_address", "zcash_primitives", "zcash_proofs", @@ -7269,7 +7270,6 @@ dependencies = [ "jubjub", "memuse", "nonempty", - "orchard", "proptest", "rand 0.8.5", "rand_chacha 0.3.1", @@ -7280,6 +7280,7 @@ dependencies = [ "secrecy", "subtle", "tracing", + "valar-orchard", "zcash_address", "zcash_encoding", "zcash_protocol", @@ -7321,7 +7322,6 @@ dependencies = [ "jubjub", "memuse", "nonempty", - "orchard", "pprof", "proptest", "rand_core 0.6.4", @@ -7330,6 +7330,7 @@ dependencies = [ "sapling-crypto", "secp256k1", "sha2 0.10.8", + "valar-orchard", "zcash_encoding", "zcash_note_encryption", "zcash_protocol", diff --git a/Cargo.toml b/Cargo.toml index 6dff6ec5b0..aeefb4f536 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,7 +68,14 @@ redjubjub = { version = "0.8", default-features = false } sapling = { package = "sapling-crypto", version = "0.6.2", default-features = false } # - Orchard -orchard = { version = "0.12", default-features = false } +# +# Points at the Valar Group `valar-orchard` fork on crates.io, aliased locally to +# `orchard` so every member still writes `use orchard::…`. The fork is +# upstream-0.12.0 + post-release fixes (up to zcash/orchard 6b12c77) + governance- +# visibility additions needed by the shielded-voting circuits in +# valargroup/voting-circuits. Drop back to `orchard = "0.12"` once those +# visibility changes land in zcash/orchard upstream. +orchard = { version = "0.12.0", package = "valar-orchard", default-features = false } pasta_curves = "0.5" # - Transparent @@ -231,4 +238,6 @@ unexpected_cfgs = { level = "warn", check-cfg = [ [patch.crates-io] sapling = { package = "sapling-crypto", git = "https://github.com/zcash/sapling-crypto.git", rev = "b8a81c22f034d68f9bbd6cba728aab807b9ba2ea" } -orchard = { package = "orchard", git = "https://github.com/zcash/orchard.git", rev = "b0bf2670e248958c6ce7c1deed466032e0dbd4d9" } +# No orchard patch: the workspace resolves `orchard` via its `package = "valar-orchard"` +# alias on crates.io, which already carries the post-0.12.0 upstream fixes plus the +# governance-visibility additions. From a25aa1b32f02d7d9aa1f2a9b6e2382fd85df1c67 Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 24 Apr 2026 16:27:09 -0300 Subject: [PATCH 09/12] ci: unblock cargo-vet and readme-graph for valar-orchard rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workspace now resolves `orchard` through the `valar-orchard` package on crates.io via `package = "valar-orchard"`. Two CI jobs keyed on the original crate name and needed to learn about the rename: - supply-chain/config.toml: add a `safe-to-deploy` exemption for `valar-orchard 0.12.0` (matches the criteria previously granted to `orchard 0.12.0@git:b0bf2670e2…`) and drop the now-unreachable orchard@git exemption: nothing in Cargo.lock resolves to it anymore. `corez 0.1.1` is already exempted via the upstream `core2 → corez` migration on this base, so no change needed there. - .github/helpers/check-dep-graph.py: add a `PACKAGE_NAME_REMAP` table that maps cargo-tree's package names back to the aliases used in the README mermaid graph, and apply it while building `cargo_edges`. Without this, every README edge mentioning `orchard` looks stale because `cargo tree -f ' {p}'` reports the real package name (`valar-orchard`), and every real edge mentioning `valar-orchard` is silently dropped because it is not in `CRATES_IN_GRAPH`. Verified locally: - `cargo vet --locked` → `Vetting Succeeded (262 fully audited, 120 partially audited, 294 exempted)` - `python3 .github/helpers/check-dep-graph.py` → exit 0, no output Made-with: Cursor --- .github/helpers/check-dep-graph.py | 9 +++++++++ supply-chain/config.toml | 8 ++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/.github/helpers/check-dep-graph.py b/.github/helpers/check-dep-graph.py index d3aec508a6..b5f0d74454 100644 --- a/.github/helpers/check-dep-graph.py +++ b/.github/helpers/check-dep-graph.py @@ -32,6 +32,14 @@ 'zip32', ]) +# Maps cargo package names to the logical name used in README.md's mermaid graph. +# The workspace imports `orchard` via the `valar-orchard` crates.io package using +# cargo's `package = "valar-orchard"` rename, so `cargo tree` reports the real +# package name `valar-orchard` while the README uses the alias `orchard`. +PACKAGE_NAME_REMAP = { + 'valar-orchard': 'orchard', +} + def main(): script_dir = os.path.dirname(os.path.realpath(__file__)) base_dir = os.path.dirname(os.path.dirname(script_dir)) @@ -79,6 +87,7 @@ def main(): continue (depth, crate, _) = line.strip().split(' ', 2) depth = int(depth) + crate = PACKAGE_NAME_REMAP.get(crate, crate) if depth == 0: crate_stack = [crate] diff --git a/supply-chain/config.toml b/supply-chain/config.toml index 64757fc85b..8983c511b4 100644 --- a/supply-chain/config.toml +++ b/supply-chain/config.toml @@ -844,10 +844,6 @@ criteria = "safe-to-deploy" version = "11.1.4" criteria = "safe-to-deploy" -[[exemptions.orchard]] -version = "0.12.0@git:b0bf2670e248958c6ce7c1deed466032e0dbd4d9" -criteria = "safe-to-deploy" - [[exemptions.ordered-float]] version = "2.10.1" criteria = "safe-to-deploy" @@ -1588,6 +1584,10 @@ criteria = "safe-to-deploy" version = "1.8.0" criteria = "safe-to-deploy" +[[exemptions.valar-orchard]] +version = "0.12.0" +criteria = "safe-to-deploy" + [[exemptions.wait-timeout]] version = "0.2.0" criteria = "safe-to-deploy" From bca4c6f32afee7c6b62d5a9584ab49dba647ff2c Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 24 Apr 2026 16:11:03 -0300 Subject: [PATCH 10/12] zcash_client_sqlite: Use `mined_height` in historical unspent-notes query 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 --- zcash_client_sqlite/src/wallet/orchard.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 441a4ab1d8..14255205dd 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -173,6 +173,15 @@ pub(crate) fn select_spendable_orchard_notes( /// Unlike `select_spendable_notes` (which applies confirmation, dust, and /// expiry filters for transaction construction), this returns every note /// that existed and was unspent at the given height. +/// +/// Height filtering uses `transactions.mined_height`, not `transactions.block`. +/// A transaction is considered to have occurred at its mined height as soon +/// as the wallet learns of that height (for example, from transparent UTXO +/// retrieval), even if the containing compact block has not been fully +/// scanned. In practice the two columns are equivalent for the notes this +/// query can return, because `nf IS NOT NULL` and +/// `commitment_tree_position IS NOT NULL` already require a scan of the +/// block that contains the receiving transaction. pub(crate) fn get_unspent_orchard_notes_at_historical_height( conn: &Connection, params: &P, @@ -184,14 +193,13 @@ pub(crate) fn get_unspent_orchard_notes_at_historical_height Date: Fri, 24 Apr 2026 16:13:18 -0300 Subject: [PATCH 11/12] zcash_client_sqlite: Avoid hardcoded key-scope codes in historical unspent-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 --- zcash_client_sqlite/src/wallet/orchard.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 14255205dd..80682527a8 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -188,7 +188,10 @@ pub(crate) fn get_unspent_orchard_notes_at_historical_height Result>, SqliteClientError> { - let mut stmt = conn.prepare_cached( + let external_scope = KeyScope::EXTERNAL.encode(); + let internal_scope = KeyScope::INTERNAL.encode(); + + let mut stmt = conn.prepare_cached(&format!( "SELECT rn.id AS id, t.txid, rn.action_index, rn.diversifier, rn.value, rn.rho, rn.rseed, rn.commitment_tree_position, @@ -202,7 +205,7 @@ pub(crate) fn get_unspent_orchard_notes_at_historical_height Date: Fri, 24 Apr 2026 16:22:32 -0300 Subject: [PATCH 12/12] zcash_client_sqlite: Document witness-availability as a caller concern 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 --- zcash_client_sqlite/src/lib.rs | 8 ++++++++ zcash_client_sqlite/src/wallet/orchard.rs | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 2cbfa46477..3b2d87e042 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -2408,6 +2408,14 @@ impl, P: consensus::Parameters, CL, R> WalletDb< /// Unlike [`InputSource::select_unspent_notes`] (which applies confirmation, /// dust, and expiry filters for transaction construction), this returns every /// note that existed and was unspent at the given height. + /// + /// This function does not verify that a Merkle witness can be constructed + /// for each returned note at `height`. Witness construction is a separate + /// concern intended to be handled by the callers. As an example, a companion + /// `WalletDb::generate_orchard_witnesses_at_historical_height` returns an + /// actionable error for any position the wallet cannot witness at `height` + /// (for example, because the wallet has not synced through `height`, the checkpoint was pruned, + /// or the position does not belong to the wallet). pub fn get_unspent_orchard_notes_at_historical_height( &self, account: AccountUuid, diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 80682527a8..f00939d938 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -182,6 +182,14 @@ pub(crate) fn select_spendable_orchard_notes( /// query can return, because `nf IS NOT NULL` and /// `commitment_tree_position IS NOT NULL` already require a scan of the /// block that contains the receiving transaction. +/// +/// This function does not verify that a Merkle witness can be constructed +/// for each returned note at `height`. Witness construction is a separate +/// concern intended to be handled by the callers. As an example, a companion +/// `WalletDb::generate_orchard_witnesses_at_historical_height` returns an +/// actionable error for any position the wallet cannot witness at `height` +/// (for example, because the wallet has not synced through `height`, the checkpoint was pruned, +/// or the position does not belong to the wallet). pub(crate) fn get_unspent_orchard_notes_at_historical_height( conn: &Connection, params: &P,