From c42cffeb1d915cfd11f49bf12c3e37cf91680315 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 14 Jun 2023 15:34:28 -0600 Subject: [PATCH] zcash_client_backend: Replace `WalletWrite::advance_by_block` with `WalletWrite::put_block` Also, add assertions to prevent attempting the creation of zero-conf shielded spends. --- zcash_client_backend/CHANGELOG.md | 11 ++++++++-- zcash_client_backend/src/data_api.rs | 4 ++-- zcash_client_backend/src/data_api/chain.rs | 16 +++++++------- zcash_client_backend/src/data_api/wallet.rs | 22 ++++++++++++++++--- .../src/data_api/wallet/input_selection.rs | 17 +++++++++++++- zcash_client_backend/src/wallet.rs | 1 + zcash_client_sqlite/src/lib.rs | 2 +- 7 files changed, 56 insertions(+), 17 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index c896b6d70..28ab39864 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -7,9 +7,11 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Added -- `impl Eq for zcash_client_backend::address::RecipientAddress` -- `impl Eq for zcash_client_backend::zip321::{Payment, TransactionRequest}` +- `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} ### Changed - MSRV is now 1.65.0. @@ -21,9 +23,14 @@ and this library adheres to Rust's notion of - `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. ### Removed - `WalletRead::get_all_nullifiers` +- `WalletWrite::advance_by_block` ## [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 80ad24f55..eafa0e89c 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -401,7 +401,7 @@ pub trait WalletWrite: WalletRead { /// along with the note commitments that were detected when scanning the block for transactions /// pertaining to this wallet. #[allow(clippy::type_complexity)] - fn advance_by_block( + fn put_block( &mut self, block: PrunedBlock, ) -> Result, Self::Error>; @@ -660,7 +660,7 @@ pub mod testing { } #[allow(clippy::type_complexity)] - fn advance_by_block( + fn put_block( &mut self, _block: PrunedBlock, ) -> Result, Self::Error> { diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index ce0eb2a81..dbec6768d 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -175,7 +175,7 @@ where // comparing against the `validate_from` hash. block_source.with_blocks::<_, Infallible, Infallible>( - validate_from.map(|(h, _)| h), + validate_from.map(|(h, _)| h + 1), limit, move |block| { if let Some((valid_height, valid_hash)) = validate_from { @@ -260,18 +260,20 @@ where ); // Start at either the provided height, or where we synced up to previously. - let (last_scanned_height, commitment_tree_meta) = from_height.map_or_else( + let (from_height, commitment_tree_meta) = from_height.map_or_else( || { data_db.fully_scanned_height().map_or_else( |e| Err(Error::Wallet(e)), - |next| Ok(next.map_or_else(|| (None, None), |(h, m)| (Some(h), Some(m)))), + |last_scanned| { + Ok(last_scanned.map_or_else(|| (None, None), |(h, m)| (Some(h + 1), Some(m)))) + }, ) }, |h| Ok((Some(h), None)), )?; block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>( - last_scanned_height, + from_height, limit, |block: CompactBlock| { add_block_to_runner(params, block, &mut batch_runner); @@ -282,7 +284,7 @@ where batch_runner.flush(); block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>( - last_scanned_height, + from_height, limit, |block: CompactBlock| { let pruned_block = scan_block_with_runner( @@ -308,9 +310,7 @@ where .map(|out| (out.account(), *out.nf())) })); - data_db - .advance_by_block(pruned_block) - .map_err(Error::Wallet)?; + data_db.put_block(pruned_block).map_err(Error::Wallet)?; Ok(()) }, diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index b0930d966..d37a03329 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -119,7 +119,8 @@ where /// can allow the sender to view the resulting notes on the blockchain. /// * `min_confirmations`: The minimum number of confirmations that a previously /// received note must have in the blockchain in order to be considered for being -/// spent. A value of 10 confirmations is recommended. +/// spent. A value of 10 confirmations is recommended and 0-conf transactions are +/// not supported. /// /// # Examples /// @@ -318,6 +319,10 @@ where ParamsT: consensus::Parameters + Clone, InputsT: InputSelector, { + assert!( + min_confirmations > 0, + "zero-conf transactions are not supported" + ); let account = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) .map_err(Error::DataSource)? @@ -372,6 +377,10 @@ where ParamsT: consensus::Parameters + Clone, InputsT: InputSelector, { + assert!( + min_confirmations > 0, + "zero-conf transactions are not supported" + ); input_selector .propose_transaction( params, @@ -409,6 +418,10 @@ where DbT::NoteRef: Copy + Eq + Ord, InputsT: InputSelector, { + assert!( + min_confirmations > 0, + "zero-conf transactions are not supported" + ); input_selector .propose_shielding( params, @@ -453,6 +466,10 @@ where ParamsT: consensus::Parameters + Clone, FeeRuleT: FeeRule, { + assert!( + min_confirmations > 0, + "zero-conf transactions are not supported" + ); let account = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) .map_err(Error::DataSource)? @@ -495,8 +512,7 @@ where selected, usk.sapling(), &dfvk, - min_confirmations - .try_into() + usize::try_from(min_confirmations - 1) .expect("min_confirmations should never be anywhere close to usize::MAX"), )? .ok_or(Error::NoteMismatch(selected.note_id))?; 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 403497c0d..5017ae335 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -1,8 +1,8 @@ //! Types related to the process of selecting inputs to be spent given a transaction request. use core::marker::PhantomData; -use std::collections::BTreeSet; use std::fmt; +use std::{collections::BTreeSet, fmt::Debug}; use zcash_primitives::{ consensus::{self, BlockHeight}, @@ -124,6 +124,21 @@ impl Proposal { } } +impl Debug for Proposal { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Proposal") + .field("transaction_request", &self.transaction_request) + .field("transparent_inputs", &self.transparent_inputs) + .field("sapling_inputs", &self.sapling_inputs.len()) + .field("balance", &self.balance) + //.field("fee_rule", &self.fee_rule) + .field("min_target_height", &self.min_target_height) + .field("min_anchor_height", &self.min_anchor_height) + .field("is_shielding", &self.is_shielding) + .finish() + } +} + /// A strategy for selecting transaction inputs and proposing transaction outputs. /// /// Proposals should include only economically useful inputs, as determined by `Self::FeeRule`; diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index c702cfa73..ccfe5a75b 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -175,6 +175,7 @@ impl WalletSaplingOutput { /// Information about a note that is tracked by the wallet that is available for spending, /// with sufficient information for use in note selection. +#[derive(Debug)] pub struct ReceivedSaplingNote { pub note_id: NoteRef, pub diversifier: sapling::Diversifier, diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index cae3685e7..810a17bf4 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -392,7 +392,7 @@ impl WalletWrite for WalletDb #[tracing::instrument(skip_all, fields(height = u32::from(block.block_height)))] #[allow(clippy::type_complexity)] - fn advance_by_block( + fn put_block( &mut self, block: PrunedBlock, ) -> Result, Self::Error> {