diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 28ab39864..0110fa88b 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -7,30 +7,73 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Added -- `impl Eq for address::RecipientAddress` -- `impl Eq for zip321::{Payment, TransactionRequest}` -- `data_api::NullifierQuery` for use with `WalletRead::get_sapling_nullifiers` -- `WalletWrite::put_block` -- `impl Debug` for `{data_api::wallet::input_selection::Proposal, wallet::ReceivedSaplingNote} +- `impl Eq for zcash_client_backend::address::RecipientAddress` +- `impl Eq for zcash_client_backend::zip321::{Payment, TransactionRequest}` +- `impl Debug` for `zcash_client_backend::{data_api::wallet::input_selection::Proposal, wallet::ReceivedSaplingNote}` +- `zcash_client_backend::data_api`: + - `WalletRead::{fully_scanned_height, suggest_scan_ranges}` + - `WalletWrite::put_block` + - `WalletCommitmentTrees` + - `testing::MockWalletDb::new` + - `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers` + - `wallet::input_sellection::Proposal::{min_target_height, min_anchor_height}`: +- `zcash_client_backend::wallet::WalletSaplingOutput::note_commitment_tree_position` +- `zcash_client_backend::welding_rig::SyncError` ### Changed - MSRV is now 1.65.0. - Bumped dependencies to `hdwallet 0.4`, `zcash_primitives 0.12`, `zcash_note_encryption 0.4`, `incrementalmerkletree 0.4`, `orchard 0.5`, `bs58 0.5` -- `WalletRead::get_memo` now returns `Result, Self::Error>` - instead of `Result` in order to make representable - wallet states where the full note plaintext is not available. -- `WalletRead::get_nullifiers` has been renamed to `WalletRead::get_sapling_nullifiers` - and its signature has changed; it now subsumes the removed `WalletRead::get_all_nullifiers`. -- `wallet::SpendableNote` has been renamed to `wallet::ReceivedSaplingNote`. -- `data_api::chain::scan_cached_blocks` now takes a `from_height` argument that - permits the caller to control the starting position of the scan range. -- `WalletWrite::advance_by_block` has been replaced by `WalletWrite::put_block` - to reflect the semantic change that scanning is no longer a linear operation. +- `zcash_client_backend::data_api`: + - `WalletRead::get_memo` now returns `Result, Self::Error>` + instead of `Result` in order to make representable + wallet states where the full note plaintext is not available. + - `WalletRead::get_nullifiers` has been renamed to `WalletRead::get_sapling_nullifiers` + and its signature has changed; it now subsumes the removed `WalletRead::get_all_nullifiers`. + - `chain::scan_cached_blocks` now takes a `from_height` argument that + permits the caller to control the starting position of the scan range. + - A new `CommitmentTree` variant has been added to `data_api::error::Error` + - `data_api::wallet::{create_spend_to_address, create_proposed_transaction, + shield_transparent_funds}` all now require that `WalletCommitmentTrees` be + implemented for the type passed to them for the `wallet_db` parameter. + - `data_api::wallet::create_proposed_transaction` now takes an additional + `min_confirmations` argument. + - A new `Sync` variant has been added to `data_api::chain::error::Error`. + - A new `SyncRequired` variant has been added to `data_api::wallet::input_selection::InputSelectorError`. +- `zcash_client_backend::wallet`: + - `SpendableNote` has been renamed to `ReceivedSaplingNote`. + - Arguments to `WalletSaplingOutput::from_parts` have changed. +- `zcash_client_backend::data_api::wallet::input_selection::InputSelector`: + - Arguments to `{propose_transaction, propose_shielding}` have changed. +- `zcash_client_backend::wallet::ReceivedSaplingNote::note_commitment_tree_position` + has replaced the `witness` field in the same struct. +- `zcash_client_backend::welding_rig::ScanningKey::sapling_nf` has been changed to + take a note position instead of an incremental witness for the note. +- Arguments to `zcash_client_backend::welding_rig::scan_block` have changed. This + method now takes an optional `CommitmentTreeMeta` argument instead of a base commitment + tree and incremental witnesses for each previously-known note. + ### Removed -- `WalletRead::get_all_nullifiers` -- `WalletWrite::advance_by_block` +- `zcash_client_backend::data_api`: + - `WalletRead::get_all_nullifiers` + - `WalletRead::{get_commitment_tree, get_witnesses}` (use + `WalletRead::fully_scanned_height` instead). + - `WalletWrite::advance_by_block` (use `WalletWrite::put_block` instead). + - The `commitment_tree` and `transactions` properties of the `PrunedBlock` + struct are now owned values instead of references, making this a fully owned type. + It is now parameterized by the nullifier type instead of by a lifetime for the + fields that were previously references. In addition, two new properties, + `sapling_commitment_tree_size` and `sapling_commitments` have been added. + - `testing::MockWalletDb`, which is available under the `test-dependencies` + feature flag, has been modified by the addition of a `sapling_tree` property. + - `wallet::input_selection`: + - `Proposal::target_height` (use `Proposal::min_target_height` instead). +- `zcash_client_backend::wallet::WalletSaplingOutput::{witness, witness_mut}` + have been removed as individual incremental witnesses are no longer tracked on a + per-note basis. The global note commitment tree for the wallet should be used + to obtain witnesses for spend operations instead. + ## [0.9.0] - 2023-04-28 ### Added diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index eafa0e89c..d1031e570 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -438,6 +438,10 @@ pub trait WalletWrite: WalletRead { ) -> Result; } +/// This trait describes a capability for manipulating wallet note commitment trees. +/// +/// At present, this only serves the Sapling protocol, but it will be modified to +/// also provide operations related to Orchard note commitment trees in the future. pub trait WalletCommitmentTrees { type Error; type SaplingShardStore<'a>: ShardStore< diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index dbec6768d..66355b06a 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -101,16 +101,20 @@ use crate::{ pub mod error; use error::{ChainError, Error}; +/// Metadata describing the sizes of the zcash note commitment trees as of a particular block. pub struct CommitmentTreeMeta { sapling_tree_size: u64, //TODO: orchard_tree_size: u64 } impl CommitmentTreeMeta { + /// Constructs a new [`CommitmentTreeMeta`] value from its constituent parts. pub fn from_parts(sapling_tree_size: u64) -> Self { Self { sapling_tree_size } } + /// Returns the size of the Sapling note commitment tree as of the block that this + /// [`CommitmentTreeMeta`] describes. pub fn sapling_tree_size(&self) -> u64 { self.sapling_tree_size } @@ -199,23 +203,19 @@ where /// Scans at most `limit` new blocks added to the block source for any transactions received by the /// tracked accounts. /// +/// If the `from_height` argument is not `None`, then the block source will begin requesting blocks +/// from the provided block source at the specified height; if `from_height` is `None then this +/// will begin scanning at first block after the position to which the wallet has previously +/// fully scanned the chain, thereby beginning or continuing a linear scan over all blocks. +/// /// This function will return without error after scanning at most `limit` new blocks, to enable -/// the caller to update their UI with scanning progress. Repeatedly calling this function will -/// process sequential ranges of blocks, and is equivalent to calling `scan_cached_blocks` and -/// passing `None` for the optional `limit` value. +/// the caller to update their UI with scanning progress. Repeatedly calling this function with +/// `from_height == None` will process sequential ranges of blocks. /// -/// This function pays attention only to cached blocks with heights greater than the highest -/// scanned block in `data`. Cached blocks with lower heights are not verified against -/// previously-scanned blocks. In particular, this function **assumes** that the caller is handling -/// rollbacks. -/// -/// For brand-new light client databases, this function starts scanning from the Sapling activation -/// height. This height can be fast-forwarded to a more recent block by initializing the client -/// database with a starting block (for example, calling `init_blocks_table` before this function -/// if using `zcash_client_sqlite`). -/// -/// Scanned blocks are required to be height-sequential. If a block is missing from the block -/// source, an error will be returned with cause [`error::Cause::BlockHeightDiscontinuity`]. +/// For brand-new light client databases, if `from_height == None` this function starts scanning +/// from the Sapling activation height. This height can be fast-forwarded to a more recent block by +/// initializing the client database with a starting block (for example, calling +/// `init_blocks_table` before this function if using `zcash_client_sqlite`). #[tracing::instrument(skip(params, block_source, data_db))] #[allow(clippy::type_complexity)] pub fn scan_cached_blocks( @@ -260,7 +260,7 @@ where ); // Start at either the provided height, or where we synced up to previously. - let (from_height, commitment_tree_meta) = from_height.map_or_else( + let (scan_from, commitment_tree_meta) = from_height.map_or_else( || { data_db.fully_scanned_height().map_or_else( |e| Err(Error::Wallet(e)), @@ -273,7 +273,7 @@ where )?; block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>( - from_height, + scan_from, limit, |block: CompactBlock| { add_block_to_runner(params, block, &mut batch_runner); @@ -283,10 +283,22 @@ where batch_runner.flush(); + let mut last_scanned = None; block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>( - from_height, + scan_from, limit, |block: CompactBlock| { + // block heights should be sequential within a single scan range + let block_height = block.height(); + if let Some(h) = last_scanned { + if block_height != h + 1 { + return Err(Error::Chain(ChainError::block_height_discontinuity( + block.height(), + h, + ))); + } + } + let pruned_block = scan_block_with_runner( params, block, @@ -312,6 +324,7 @@ where data_db.put_block(pruned_block).map_err(Error::Wallet)?; + last_scanned = Some(block_height); Ok(()) }, )?; diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index d37a03329..a47528920 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -122,6 +122,10 @@ where /// spent. A value of 10 confirmations is recommended and 0-conf transactions are /// not supported. /// +/// # Panics +/// +/// Panics if `min_confirmations == 0`; 0-conf transactions are not supported. +/// /// # Examples /// /// ``` diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index 5017ae335..90c701336 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -62,7 +62,9 @@ impl fmt::Display for InputSelectorError write!(f, "No chain data is available."), + InputSelectorError::SyncRequired => { + write!(f, "Insufficient chain data is available, sync required.") + } } } } @@ -135,7 +137,7 @@ impl Debug for Proposal { .field("min_target_height", &self.min_target_height) .field("min_anchor_height", &self.min_anchor_height) .field("is_shielding", &self.is_shielding) - .finish() + .finish_non_exhaustive() } } diff --git a/zcash_client_backend/src/welding_rig.rs b/zcash_client_backend/src/welding_rig.rs index a9e7d3f56..a8a98069e 100644 --- a/zcash_client_backend/src/welding_rig.rs +++ b/zcash_client_backend/src/welding_rig.rs @@ -672,6 +672,22 @@ mod tests { assert_eq!(tx.sapling_outputs[0].index(), 0); assert_eq!(tx.sapling_outputs[0].account(), AccountId::from(0)); assert_eq!(tx.sapling_outputs[0].note().value().inner(), 5); + + assert_eq!( + pruned_block + .sapling_commitments + .iter() + .map(|(_, retention)| *retention) + .collect::>(), + vec![ + Retention::Ephemeral, + Retention::Marked, + Retention::Checkpoint { + id: pruned_block.block_height, + is_marked: false + } + ] + ); } go(false); @@ -714,5 +730,20 @@ mod tests { assert_eq!(tx.sapling_spends[0].index(), 0); assert_eq!(tx.sapling_spends[0].nf(), &nf); assert_eq!(tx.sapling_spends[0].account(), account); + + assert_eq!( + pruned_block + .sapling_commitments + .iter() + .map(|(_, retention)| *retention) + .collect::>(), + vec![ + Retention::Ephemeral, + Retention::Checkpoint { + id: pruned_block.block_height, + is_marked: false + } + ] + ); } } diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index d72fe90ac..b1e332913 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -6,10 +6,19 @@ and this library adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- `zcash_client_sqlite::serialization` Serialization formats for data stored + as SQLite BLOBs in the wallet database. + ### Changed - MSRV is now 1.65.0. - Bumped dependencies to `hdwallet 0.4`, `incrementalmerkletree 0.4`, `bs58 0.5`, `zcash_primitives 0.12` +- A `CommitmentTree` variant has been added to `zcash_client_sqlite::wallet::init::WalletMigrationError` +- `min_confirmations` parameter values are now more strongly enforced. Previously, + a note could be spent with fewer than `min_confirmations` confirmations if the + wallet did not contain enough observed blocks to satisfy the `min_confirmations` + value specified; this situation is now treated as an error. ### Removed - The empty `wallet::transact` module has been removed. diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 87f88b844..6eb0939f2 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -78,7 +78,8 @@ pub enum SqliteClientError { #[cfg(feature = "transparent-inputs")] AddressNotRecognized(TransparentAddress), - /// An error occurred in inserting data into one of the wallet's note commitment trees. + /// An error occurred in inserting data into or accessing data from one of the wallet's note + /// commitment trees. CommitmentTree(ShardTreeError>), } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 60447a55e..9c1ac261a 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -85,6 +85,8 @@ pub mod wallet; /// this delta from the chain tip to be pruned. pub(crate) const PRUNING_HEIGHT: u32 = 100; +pub(crate) const SAPLING_TABLES_PREFIX: &'static str = "sapling"; + /// A newtype wrapper for sqlite primary key values for the notes /// table. #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] @@ -396,10 +398,9 @@ impl WalletWrite for WalletDb ) -> Result, Self::Error> { self.transactionally(|wdb| { // Insert the block into the database. - let block_height = block.block_height; wallet::put_block( wdb.conn.0, - block_height, + block.block_height, block.block_hash, block.block_time, block.sapling_commitment_tree_size.map(|s| s.into()), @@ -423,18 +424,19 @@ impl WalletWrite for WalletDb } } + let sapling_commitments_len = block.sapling_commitments.len(); let mut sapling_commitments = block.sapling_commitments.into_iter(); wdb.with_sapling_tree_mut::<_, _, SqliteClientError>(move |sapling_tree| { if let Some(sapling_tree_size) = block.sapling_commitment_tree_size { let start_position = Position::from(u64::from(sapling_tree_size)) - - u64::try_from(sapling_commitments.len()).unwrap(); + - u64::try_from(sapling_commitments_len).unwrap(); sapling_tree.batch_insert(start_position, &mut sapling_commitments)?; } Ok(()) })?; // Update now-expired transactions that didn't get mined. - wallet::update_expired_notes(wdb.conn.0, block_height)?; + wallet::update_expired_notes(wdb.conn.0, block.block_height)?; Ok(wallet_note_ids) }) @@ -633,10 +635,10 @@ impl WalletCommitmentTrees for WalletDb WalletCommitmentTrees for WalletDb>>, { let mut shardtree = ShardTree::new( - SqliteShardStore::from_connection(self.conn.0, "sapling") + SqliteShardStore::from_connection(self.conn.0, SAPLING_TABLES_PREFIX) .map_err(|e| ShardTreeError::Storage(Either::Right(e)))?, - 100, + PRUNING_HEIGHT.try_into().unwrap(), ); let result = callback(&mut shardtree)?; diff --git a/zcash_client_sqlite/src/serialization.rs b/zcash_client_sqlite/src/serialization.rs index 99cb90dd8..eb1176465 100644 --- a/zcash_client_sqlite/src/serialization.rs +++ b/zcash_client_sqlite/src/serialization.rs @@ -14,10 +14,11 @@ const NIL_TAG: u8 = 0; const LEAF_TAG: u8 = 1; const PARENT_TAG: u8 = 2; -pub fn write_shard_v1( - writer: &mut W, - tree: &PrunableTree, -) -> io::Result<()> { +/// Writes a [`PrunableTree`] to the provided [`Write`] instance. +/// +/// This is the primary method used for ShardTree shard persistence. It writes a version identifier +/// for the most-current serialized form, followed by the tree data. +pub fn write_shard(writer: &mut W, tree: &PrunableTree) -> io::Result<()> { fn write_inner( mut writer: &mut W, tree: &PrunableTree, @@ -80,6 +81,11 @@ fn read_shard_v1(mut reader: &mut R) -> io::Result(mut reader: R) -> io::Result> { match reader.read_u8()? { SER_V1 => read_shard_v1(&mut reader), @@ -97,7 +103,7 @@ mod tests { use shardtree::testing::arb_prunable_tree; use std::io::Cursor; - use super::{read_shard, write_shard_v1}; + use super::{read_shard, write_shard}; proptest! { #[test] @@ -105,7 +111,7 @@ mod tests { tree in arb_prunable_tree(arb_test_node(), 8, 32) ) { let mut tree_data = vec![]; - write_shard_v1(&mut tree_data, &tree).unwrap(); + write_shard(&mut tree_data, &tree).unwrap(); let cursor = Cursor::new(tree_data); let tree_result = read_shard::(cursor).unwrap(); assert_eq!(tree, tree_result); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 757fde8bf..108b5f5d3 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -640,7 +640,7 @@ pub(crate) fn get_min_unspent_height( /// block, this function does nothing. /// /// This should only be executed inside a transactional context. -pub(crate) fn truncate_to_height( +pub(crate) fn truncate_to_height( conn: &rusqlite::Transaction, params: &P, block_height: BlockHeight, diff --git a/zcash_client_sqlite/src/wallet/commitment_tree.rs b/zcash_client_sqlite/src/wallet/commitment_tree.rs index 8ce583f9d..d28b5c701 100644 --- a/zcash_client_sqlite/src/wallet/commitment_tree.rs +++ b/zcash_client_sqlite/src/wallet/commitment_tree.rs @@ -11,7 +11,7 @@ use shardtree::{Checkpoint, LocatedPrunableTree, PrunableTree, ShardStore, TreeS use zcash_primitives::{consensus::BlockHeight, merkle_tree::HashSer}; -use crate::serialization::{read_shard, write_shard_v1}; +use crate::serialization::{read_shard, write_shard}; pub struct SqliteShardStore { pub(crate) conn: C, @@ -327,7 +327,7 @@ pub(crate) fn put_shard( .map_err(Either::Left)?; let mut subtree_data = vec![]; - write_shard_v1(&mut subtree_data, subtree.root()).map_err(Either::Left)?; + write_shard(&mut subtree_data, subtree.root()).map_err(Either::Left)?; let mut stmt_put_shard = conn .prepare_cached(&format!( @@ -423,7 +423,7 @@ pub(crate) fn put_cap( .map_err(Either::Right)?; let mut cap_data = vec![]; - write_shard_v1(&mut cap_data, &cap).map_err(Either::Left)?; + write_shard(&mut cap_data, &cap).map_err(Either::Left)?; stmt.execute([cap_data]).map_err(Either::Right)?; Ok(()) diff --git a/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs b/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs index 0d292adb1..f4bce16b8 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs @@ -18,10 +18,10 @@ use zcash_primitives::{ sapling, }; -use crate::wallet::{ +use crate::{wallet::{ commitment_tree::SqliteShardStore, init::{migrations::received_notes_nullable_nf, WalletMigrationError}, -}; +}, SAPLING_TABLES_PREFIX}; pub(super) const MIGRATION_ID: Uuid = Uuid::from_fields( 0x7da6489d, @@ -94,7 +94,7 @@ impl RusqliteMigration for Migration { let shard_store = SqliteShardStore::<_, sapling::Node, SAPLING_SHARD_HEIGHT>::from_connection( transaction, - "sapling", + SAPLING_TABLES_PREFIX, )?; let mut shard_tree: ShardTree< _, @@ -187,14 +187,8 @@ impl RusqliteMigration for Migration { Ok(()) } - fn down(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { - transaction.execute_batch( - "DROP TABLE sapling_tree_checkpoint_marks_removed; - DROP TABLE sapling_tree_checkpoints; - DROP TABLE sapling_tree_cap; - DROP TABLE sapling_tree_shards;", - )?; - - Ok(()) + fn down(&self, _transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + // TODO: something better than just panic? + panic!("Cannot revert this migration."); } } diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 7b3269140..6544b166e 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -425,7 +425,7 @@ pub(crate) mod tests { }, }; - pub fn test_prover() -> impl TxProver { + pub(crate) fn test_prover() -> impl TxProver { match LocalTxProver::with_default_location() { Some(tx_prover) => tx_prover, None => { diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 47c2f3a4b..f2404ea94 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -12,6 +12,9 @@ and this library adheres to Rust's notion of - `Builder::add_orchard_spend` - `Builder::add_orchard_output` - `zcash_primitives::transaction::components::orchard::builder` module +- `impl HashSer for String` is provided under the `test-dependencies` feature + flag. This is a test-only impl; the identity leaf value is `_` and the combining + operation is concatenation. ### Changed - `zcash_primitives::transaction`: