From dd821b0085f4d001e4370ece2184e5db5935bfd9 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 2 Oct 2023 18:02:06 -0600 Subject: [PATCH] zcash_client_backend: Factor out input source traits from `WalletRead` Prior to this change, it's necessary to implement the entirety of the `WalletRead` trait in order to be able to use the input selection functionality provided by `zcash_client_backend::data_api::input_selection`. This change factors out the minimal operations required for transaction proposal construction to better reflect the principle of least authority and make the input selection code reusable in more contexts. In order to minimize the operations of the newly-created `InputSource` and `ShieldingSource` traits, this change also removes the `min_confirmations` field from transaction proposals, in favor of storing explicit target and anchor heights. This has the effect of limiting the lifetime of transaction proposals to `PRUNING_DEPTH - min_confirmations` blocks. --- Cargo.lock | 6 +- Cargo.toml | 4 + zcash_client_backend/CHANGELOG.md | 43 +++++- zcash_client_backend/Cargo.toml | 1 + zcash_client_backend/src/data_api.rs | 145 ++++++++---------- zcash_client_backend/src/data_api/wallet.rs | 91 +++++++---- .../src/data_api/wallet/input_selection.rs | 142 ++++++++++------- zcash_client_backend/src/fees.rs | 2 +- zcash_client_sqlite/src/lib.rs | 140 +++++++---------- zcash_client_sqlite/src/testing.rs | 18 ++- zcash_client_sqlite/src/wallet.rs | 41 +++-- .../src/wallet/commitment_tree.rs | 76 +++++---- zcash_client_sqlite/src/wallet/sapling.rs | 60 +------- zcash_primitives/src/transaction/sighash.rs | 4 +- 14 files changed, 384 insertions(+), 389 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 314ffd5efd..cbe0ed0e7e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1088,8 +1088,7 @@ dependencies = [ [[package]] name = "incrementalmerkletree" version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "361c467824d4d9d4f284be4b2608800839419dccc4d4608f28345237fe354623" +source = "git+https://github.com/nuttycom/incrementalmerkletree.git?rev=c8351fc4f0863b8e69d7c0617bf5c82fc0035ce4#c8351fc4f0863b8e69d7c0617bf5c82fc0035ce4" dependencies = [ "either", "proptest", @@ -2184,8 +2183,7 @@ dependencies = [ [[package]] name = "shardtree" version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c19f96dde3a8693874f7e7c53d95616569b4009379a903789efbd448f4ea9cc7" +source = "git+https://github.com/nuttycom/incrementalmerkletree.git?rev=c8351fc4f0863b8e69d7c0617bf5c82fc0035ce4#c8351fc4f0863b8e69d7c0617bf5c82fc0035ce4" dependencies = [ "assert_matches", "bitflags 2.4.0", diff --git a/Cargo.toml b/Cargo.toml index bb3cf1231f..b84f6f344d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -108,3 +108,7 @@ fpe = "0.6" lto = true panic = 'abort' codegen-units = 1 + +[patch.crates-io] +incrementalmerkletree = { git = "https://github.com/nuttycom/incrementalmerkletree.git", rev = "c8351fc4f0863b8e69d7c0617bf5c82fc0035ce4" } +shardtree = { git = "https://github.com/nuttycom/incrementalmerkletree.git", rev = "c8351fc4f0863b8e69d7c0617bf5c82fc0035ce4" } diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 147f529aaf..7d225a24ef 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -15,13 +15,18 @@ and this library adheres to Rust's notion of - `zcash_client_backend::data_api::error::Error` has new error variant: - `Error::UnsupportedPoolType(zcash_client_backend::data_api::PoolType)` - Added module `zcash_client_backend::fees::standard` -- Added `zcash_client_backend::wallet::input_selection::Proposal::min_confirmations` - Added methods to `zcash_client_backend::wallet::ReceivedSaplingNote`: `{from_parts, txid, output_index, diversifier, rseed, note_commitment_tree_position}`. +- `zcash_client_backend::data_api::TransparentInputSource` +- `zcash_client_backend::data_api::SaplingInputSource` +- `zcash_client_backend::data_api::wallet::input_selection::ShieldingSelector` has been + factored out from the `InputSelector` trait to separate out transparent functionality + and move it behind the `transparent-inputs` feature flag. ### Changed - `zcash_client_backend::data_api::chain::scan_cached_blocks` now returns a `ScanSummary` containing metadata about the scanned blocks on success. + - The fields of `zcash_client_backend::wallet::ReceivedSaplingNote` are now private. Use `ReceivedSaplingNote::from_parts` for construction instead. Accessor methods are provided for each previously-public field. @@ -49,7 +54,7 @@ and this library adheres to Rust's notion of - `wallet::create_proposed_transaction` now takes its `proposal` argument by reference instead of as an owned value. - `wallet::create_proposed_transaction` no longer takes a `min_confirmations` - argument. Instead, `min_confirmations` is stored in the `Proposal` + argument. Instead, it uses the anchor height from its `proposal` argument. - `wallet::create_spend_to_address` now takes an additional `change_memo` argument. - `zcash_client_backend::fees::ChangeValue::Sapling` is now a structured variant. @@ -68,11 +73,43 @@ and this library adheres to Rust's notion of updated accordingly. - Almost all uses of `Amount` in `zcash_client_backend::zip321` have been replaced with `NonNegativeAmount`. +- In order to support better reusability for input selection code, three new + supertraits have been factored out from `zcash_client_backend::data_api::WalletRead`: + - `zcash_client_backend::data_api::TransparentInputSource` + - `zcash_client_backend::data_api::SaplingInputSource` +- `zcash_client_backend::data_api::wallet::input_selection::InputSelector::propose_shielding`, + has been moved out to the newly-created `ShieldingSelector` trait. +- The `zcash_client_backend::data_api::wallet::input_selection::InputSelector::DataSource` + associated type has been renamed to `InputSource`. +- The signature of `InputSelector::propose_transaction` has been altered + such that it longer takes `min_confirmations` as an argument, instead taking + explicit `target_height` and `anchor_height` arguments. This helps to + minimize the set of capabilities that the `data_api::SaplingInputSource` must + expose. +- `ShieldingSelector::propose_shielding` has been altered such that it takes + an explicit `target_height` in order to minimize the capabilities that the + `data_api::TransparentInputSource` trait must expose. Also, it now takes its + `min_confirmations` argument as `u32` instead of `NonZeroU32`. +- `zcash_client_backend::data_api::wallet::{propose_shielding, shield_transparent_funds}` + now takes their `min_confirmations` arguments as `u32` rather than a + `NonZeroU32` to permit implmentations to enable zero-conf shielding. +- `zcash_client_backend::data_api::wallet::create_proposed_transaction` now forces + implementations to ignore the database identifiers for its contained notes by + universally quantifying the `NoteRef` type parameter. +- `zcash_client_backend::data_api::wallet::input_selection::Proposal::min_anchor_height` + has been renamed to `sapling_anchor_height` and now produces an `Option` + instead of a `BlockHeight`. +- `zcash_client_backend::data_api::wallet::input_selection::GreedyInputSelector` + now has relaxed requirements for its `InputSource` associated type. ### Removed - `zcash_client_backend::data_api::WalletRead::is_valid_account_extfvk` has been - removed; it was unused in the ECC mobile wallet SDKs and has been superseded by + removed; it was unused in the ECC mobile wallet SDKs and has been superseded by `get_account_for_ufvk`. +- `zcash_client_backend::data_api::WalletRead::get_spendable_sapling_notes` has been + removed without replacement as it was unused, and its functionality will be + fully reproduced by `SaplingInputSource::select_spendable_sapling_notes` in a future + change. ## [0.10.0] - 2023-09-25 diff --git a/zcash_client_backend/Cargo.toml b/zcash_client_backend/Cargo.toml index 2522f80efd..cf6a3e76df 100644 --- a/zcash_client_backend/Cargo.toml +++ b/zcash_client_backend/Cargo.toml @@ -87,6 +87,7 @@ gumdrop = "0.8" jubjub.workspace = true proptest.workspace = true rand_core.workspace = true +shardtree = { workspace = true, features = ["test-dependencies"] } zcash_proofs.workspace = true zcash_address = { workspace = true, features = ["test-dependencies"] } diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 2842474037..d2b5d31fc6 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -210,12 +210,7 @@ impl WalletSummary { } } -/// Read-only operations required for light wallet functions. -/// -/// This trait defines the read-only portion of the storage interface atop which -/// higher-level wallet operations are implemented. It serves to allow wallet functions to -/// be abstracted away from any particular data storage substrate. -pub trait WalletRead { +pub trait SaplingInputSource { /// The type of errors produced by a wallet backend. type Error; @@ -225,6 +220,39 @@ pub trait WalletRead { /// or a UUID. type NoteRef: Copy + Debug + Eq + Ord; + /// Returns a list of spendable Sapling notes sufficient to cover the specified target value, + /// if possible. + fn select_spendable_sapling_notes( + &self, + account: AccountId, + target_value: Amount, + anchor_height: BlockHeight, + exclude: &[Self::NoteRef], + ) -> Result>, Self::Error>; +} + +pub trait TransparentInputSource { + /// The type of errors produced by a wallet backend. + type Error; + + /// Returns a list of unspent transparent UTXOs that appear in the chain at heights up to and + /// including `max_height`. + fn get_unspent_transparent_outputs( + &self, + address: &TransparentAddress, + max_height: BlockHeight, + exclude: &[OutPoint], + ) -> Result, Self::Error>; +} + +/// Read-only operations required for light wallet functions. +/// +/// This trait defines the read-only portion of the storage interface atop which +/// higher-level wallet operations are implemented. It serves to allow wallet functions to +/// be abstracted away from any particular data storage substrate. +pub trait WalletRead { + type Error; + /// Returns the height of the chain as known to the wallet as of the most recent call to /// [`WalletWrite::update_chain_tip`]. /// @@ -351,24 +379,6 @@ pub trait WalletRead { query: NullifierQuery, ) -> Result, Self::Error>; - /// Return all unspent Sapling notes, excluding the specified note IDs. - fn get_spendable_sapling_notes( - &self, - account: AccountId, - anchor_height: BlockHeight, - exclude: &[Self::NoteRef], - ) -> Result>, Self::Error>; - - /// Returns a list of spendable Sapling notes sufficient to cover the specified target value, - /// if possible. - fn select_spendable_sapling_notes( - &self, - account: AccountId, - target_value: Amount, - anchor_height: BlockHeight, - exclude: &[Self::NoteRef], - ) -> Result>, Self::Error>; - /// Returns the set of all transparent receivers associated with the given account. /// /// The set contains all transparent receivers that are known to have been derived @@ -379,15 +389,6 @@ pub trait WalletRead { account: AccountId, ) -> Result, Self::Error>; - /// Returns a list of unspent transparent UTXOs that appear in the chain at heights up to and - /// including `max_height`. - fn get_unspent_transparent_outputs( - &self, - address: &TransparentAddress, - max_height: BlockHeight, - exclude: &[OutPoint], - ) -> Result, Self::Error>; - /// Returns a mapping from transparent receiver to not-yet-shielded UTXO balance, /// for each address associated with a nonzero balance. fn get_transparent_balances( @@ -897,16 +898,7 @@ pub trait WalletCommitmentTrees { Error = Self::Error, >; - /// Returns the depth of the checkpoint in the tree that can be used to create a witness at the - /// anchor having the given number of confirmations. /// - /// This assumes that at any time a note is added to the tree, a checkpoint is created for the - /// end of the block in which that note was discovered. - fn get_checkpoint_depth( - &self, - min_confirmations: NonZeroU32, - ) -> Result>; - fn with_sapling_tree_mut(&mut self, callback: F) -> Result where for<'a> F: FnMut( @@ -954,8 +946,9 @@ pub mod testing { use super::{ chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata, - DecryptedTransaction, NoteId, NullifierQuery, ScannedBlock, SentTransaction, - WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + DecryptedTransaction, NoteId, NullifierQuery, SaplingInputSource, ScannedBlock, + SentTransaction, TransparentInputSource, WalletCommitmentTrees, WalletRead, WalletSummary, + WalletWrite, SAPLING_SHARD_HEIGHT, }; pub struct MockWalletDb { @@ -976,10 +969,37 @@ pub mod testing { } } - impl WalletRead for MockWalletDb { + impl SaplingInputSource for MockWalletDb { type Error = (); type NoteRef = u32; + fn select_spendable_sapling_notes( + &self, + _account: AccountId, + _target_value: Amount, + _anchor_height: BlockHeight, + _exclude: &[Self::NoteRef], + ) -> Result>, Self::Error> { + Ok(Vec::new()) + } + } + + impl TransparentInputSource for MockWalletDb { + type Error = (); + + fn get_unspent_transparent_outputs( + &self, + _address: &TransparentAddress, + _anchor_height: BlockHeight, + _exclude: &[OutPoint], + ) -> Result, Self::Error> { + Ok(Vec::new()) + } + } + + impl WalletRead for MockWalletDb { + type Error = (); + fn chain_height(&self) -> Result, Self::Error> { Ok(None) } @@ -1079,25 +1099,6 @@ pub mod testing { Ok(Vec::new()) } - fn get_spendable_sapling_notes( - &self, - _account: AccountId, - _anchor_height: BlockHeight, - _exclude: &[Self::NoteRef], - ) -> Result>, Self::Error> { - Ok(Vec::new()) - } - - fn select_spendable_sapling_notes( - &self, - _account: AccountId, - _target_value: Amount, - _anchor_height: BlockHeight, - _exclude: &[Self::NoteRef], - ) -> Result>, Self::Error> { - Ok(Vec::new()) - } - fn get_transparent_receivers( &self, _account: AccountId, @@ -1105,15 +1106,6 @@ pub mod testing { Ok(HashMap::new()) } - fn get_unspent_transparent_outputs( - &self, - _address: &TransparentAddress, - _anchor_height: BlockHeight, - _exclude: &[OutPoint], - ) -> Result, Self::Error> { - Ok(Vec::new()) - } - fn get_transparent_balances( &self, _account: AccountId, @@ -1214,12 +1206,5 @@ pub mod testing { Ok(()) } - - fn get_checkpoint_depth( - &self, - min_confirmations: NonZeroU32, - ) -> Result> { - Ok(usize::try_from(u32::from(min_confirmations)).unwrap()) - } } } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index a5b9fef552..28db8e31c2 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -35,13 +35,18 @@ use crate::{ }; pub mod input_selection; -use input_selection::{GreedyInputSelector, GreedyInputSelectorError, InputSelector}; +use input_selection::{ + GreedyInputSelector, GreedyInputSelectorError, InputSelector, InputSelectorError, +}; -use super::{NoteId, ShieldedProtocol}; +use super::{NoteId, SaplingInputSource, ShieldedProtocol}; #[cfg(feature = "transparent-inputs")] use { + super::TransparentInputSource, crate::wallet::WalletTransparentOutput, + input_selection::ShieldingSelector, + std::convert::Infallible, zcash_primitives::{legacy::TransparentAddress, sapling::keys::OutgoingViewingKey}, }; @@ -210,7 +215,9 @@ pub fn create_spend_to_address( > where ParamsT: consensus::Parameters + Clone, - DbT: WalletWrite + WalletCommitmentTrees, + DbT: WalletWrite + + WalletCommitmentTrees + + SaplingInputSource::Error>, DbT::NoteRef: Copy + Eq + Ord, { let account = wallet_db @@ -305,10 +312,12 @@ pub fn spend( >, > where - DbT: WalletWrite + WalletCommitmentTrees, + DbT: WalletWrite + + WalletCommitmentTrees + + SaplingInputSource::Error>, DbT::NoteRef: Copy + Eq + Ord, ParamsT: consensus::Parameters + Clone, - InputsT: InputSelector, + InputsT: InputSelector, { let account = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) @@ -341,21 +350,32 @@ pub fn propose_transfer( min_confirmations: NonZeroU32, ) -> Result< Proposal, - Error::Error>, + Error< + ::Error, + CommitmentTreeErrT, + InputsT::Error, + ::Error, + >, > where - DbT: WalletWrite, + DbT: WalletRead + SaplingInputSource::Error>, DbT::NoteRef: Copy + Eq + Ord, ParamsT: consensus::Parameters + Clone, - InputsT: InputSelector, + InputsT: InputSelector, { + let (target_height, anchor_height) = wallet_db + .get_target_and_anchor_heights(min_confirmations) + .map_err(|e| Error::from(InputSelectorError::DataSource(e)))? + .ok_or_else(|| Error::from(InputSelectorError::SyncRequired))?; + input_selector .propose_transaction( params, wallet_db, + target_height, + anchor_height, spend_from_account, request, - min_confirmations, ) .map_err(Error::from) } @@ -394,7 +414,7 @@ pub fn propose_standard_transfer_to_address( ) -> Result< Proposal, Error< - DbT::Error, + ::Error, CommitmentTreeErrT, GreedyInputSelectorError, Zip317FeeError, @@ -402,7 +422,7 @@ pub fn propose_standard_transfer_to_address( > where ParamsT: consensus::Parameters + Clone, - DbT: WalletWrite, + DbT: WalletRead + SaplingInputSource::Error>, DbT::NoteRef: Copy + Eq + Ord, { let request = zip321::TransactionRequest::new(vec![Payment { @@ -440,23 +460,33 @@ pub fn propose_shielding( input_selector: &InputsT, shielding_threshold: NonNegativeAmount, from_addrs: &[TransparentAddress], - min_confirmations: NonZeroU32, + min_confirmations: u32, ) -> Result< - Proposal, - Error::Error>, + Proposal, + Error< + ::Error, + CommitmentTreeErrT, + InputsT::Error, + ::Error, + >, > where ParamsT: consensus::Parameters, - DbT: WalletWrite, - DbT::NoteRef: Copy + Eq + Ord, - InputsT: InputSelector, + DbT: WalletRead + TransparentInputSource::Error>, + InputsT: ShieldingSelector, { + let chain_tip_height = wallet_db + .chain_height() + .map_err(|e| Error::from(InputSelectorError::DataSource(e)))? + .ok_or_else(|| Error::from(InputSelectorError::SyncRequired))?; + input_selector .propose_shielding( params, wallet_db, shielding_threshold, from_addrs, + chain_tip_height + 1, min_confirmations, ) .map_err(Error::from) @@ -472,13 +502,13 @@ where /// to fall back to the transparent receiver until full Orchard support is implemented. #[allow(clippy::too_many_arguments)] #[allow(clippy::type_complexity)] -pub fn create_proposed_transaction( +pub fn create_proposed_transaction( wallet_db: &mut DbT, params: &ParamsT, prover: impl SaplingProver, usk: &UnifiedSpendingKey, ovk_policy: OvkPolicy, - proposal: &Proposal, + proposal: &Proposal, ) -> Result< TxId, Error< @@ -490,7 +520,6 @@ pub fn create_proposed_transaction( > where DbT: WalletWrite + WalletCommitmentTrees, - DbT::NoteRef: Copy + Eq + Ord, ParamsT: consensus::Parameters + Clone, FeeRuleT: FeeRule, { @@ -529,7 +558,6 @@ where // are no possible transparent inputs, so we ignore those let mut builder = Builder::new(params.clone(), proposal.min_target_height(), None); - let checkpoint_depth = wallet_db.get_checkpoint_depth(proposal.min_confirmations())?; wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _>>(|sapling_tree| { for selected in proposal.sapling_inputs() { let (note, key, merkle_path) = select_key_for_note( @@ -537,7 +565,7 @@ where selected, usk.sapling(), &dfvk, - checkpoint_depth, + proposal.sapling_anchor_height().expect("It should be impossible to construct a proposal for a Sapling spend without selecting a Sapling anchor.") )? .ok_or_else(|| { Error::NoteMismatch(NoteId { @@ -773,7 +801,7 @@ pub fn shield_transparent_funds( shielding_threshold: NonNegativeAmount, usk: &UnifiedSpendingKey, from_addrs: &[TransparentAddress], - min_confirmations: NonZeroU32, + min_confirmations: u32, ) -> Result< TxId, Error< @@ -785,9 +813,10 @@ pub fn shield_transparent_funds( > where ParamsT: consensus::Parameters, - DbT: WalletWrite + WalletCommitmentTrees, - DbT::NoteRef: Copy + Eq + Ord, - InputsT: InputSelector, + DbT: WalletWrite + + WalletCommitmentTrees + + TransparentInputSource::Error>, + InputsT: ShieldingSelector, { let proposal = propose_shielding( wallet_db, @@ -811,7 +840,7 @@ fn select_key_for_note>( selected: &ReceivedSaplingNote, extsk: &ExtendedSpendingKey, dfvk: &DiversifiableFullViewingKey, - checkpoint_depth: usize, + anchor_height: BlockHeight, ) -> Result< Option<(sapling::Note, ExtendedSpendingKey, sapling::MerklePath)>, ShardTreeError, @@ -826,9 +855,11 @@ fn select_key_for_note>( .diversified_change_address(selected.diversifier()) .map(|addr| addr.create_note(selected.value().try_into().unwrap(), selected.rseed())); - let expected_root = commitment_tree.root_at_checkpoint(checkpoint_depth)?; - let merkle_path = commitment_tree - .witness_caching(selected.note_commitment_tree_position(), checkpoint_depth)?; + let expected_root = commitment_tree.root_at_checkpoint_id(&anchor_height)?; + let merkle_path = commitment_tree.witness_at_checkpoint_id_caching( + selected.note_commitment_tree_position(), + &anchor_height, + )?; Ok(external_note .filter(|n| expected_root == merkle_path.root(Node::from_cmu(&n.cmu()))) 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 297ab4c84e..9ba991eca5 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -1,9 +1,7 @@ //! Types related to the process of selecting inputs to be spent given a transaction request. use core::marker::PhantomData; -use std::fmt; -use std::num::NonZeroU32; -use std::{collections::BTreeSet, fmt::Debug}; +use std::fmt::{self, Debug}; use zcash_primitives::{ consensus::{self, BlockHeight}, @@ -12,7 +10,7 @@ use zcash_primitives::{ components::{ amount::{BalanceError, NonNegativeAmount}, sapling::fees as sapling, - OutPoint, TxOut, + TxOut, }, fees::FeeRule, }, @@ -21,12 +19,18 @@ use zcash_primitives::{ use crate::{ address::{RecipientAddress, UnifiedAddress}, - data_api::WalletRead, + data_api::SaplingInputSource, fees::{ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance}, wallet::{ReceivedSaplingNote, WalletTransparentOutput}, zip321::TransactionRequest, }; +#[cfg(feature = "transparent-inputs")] +use { + crate::data_api::TransparentInputSource, std::collections::BTreeSet, std::convert::Infallible, + zcash_primitives::transaction::components::OutPoint, +}; + /// The type of errors that may be produced in input selection. pub enum InputSelectorError { /// An error occurred accessing the underlying data store. @@ -81,9 +85,8 @@ pub struct Proposal { sapling_inputs: Vec>, balance: TransactionBalance, fee_rule: FeeRuleT, - min_confirmations: NonZeroU32, min_target_height: BlockHeight, - min_anchor_height: BlockHeight, + sapling_anchor_height: Option, is_shielding: bool, } @@ -108,10 +111,6 @@ impl Proposal { pub fn fee_rule(&self) -> &FeeRuleT { &self.fee_rule } - /// Returns the value of `min_confirmations` used in input selection. - pub fn min_confirmations(&self) -> NonZeroU32 { - self.min_confirmations - } /// Returns the target height for which the proposal was prepared. /// /// The chain must contain at least this many blocks in order for the proposal to @@ -119,13 +118,13 @@ impl Proposal { pub fn min_target_height(&self) -> BlockHeight { self.min_target_height } - /// Returns the anchor height used in preparing the proposal. + /// Returns the Sapling anchor height used in preparing the proposal. /// /// If, at the time that the proposal is executed, the anchor height required to satisfy /// the minimum confirmation depth is less than this height, the proposal execution /// API should return an error. - pub fn min_anchor_height(&self) -> BlockHeight { - self.min_anchor_height + pub fn sapling_anchor_height(&self) -> Option { + self.sapling_anchor_height } /// Returns a flag indicating whether or not the proposed transaction /// is exclusively wallet-internal (if it does not involve any external @@ -144,7 +143,7 @@ impl Debug for Proposal { .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("sapling_anchor_height", &self.sapling_anchor_height) .field("is_shielding", &self.is_shielding) .finish_non_exhaustive() } @@ -158,12 +157,12 @@ impl Debug for Proposal { pub trait InputSelector { /// The type of errors that may be generated in input selection type Error; - /// The type of data source that the input selector expects to access to obtain input notes and - /// UTXOs. This associated type permits input selectors that may use specialized knowledge of + /// The type of data source that the input selector expects to access to obtain input Sapling + /// notes. This associated type permits input selectors that may use specialized knowledge of /// the internals of a particular backing data store, if the generic API of `WalletRead` does /// not provide sufficiently fine-grained operations for a particular backing store to /// optimally perform input selection. - type DataSource: WalletRead; + type InputSource; /// The type of the fee rule that this input selector uses when computing fees. type FeeRule: FeeRule; @@ -181,21 +180,36 @@ pub trait InputSelector { /// /// If insufficient funds are available to satisfy the required outputs for the shielding /// request, this operation must fail and return [`InputSelectorError::InsufficientFunds`]. - #[allow(clippy::too_many_arguments)] #[allow(clippy::type_complexity)] fn propose_transaction( &self, params: &ParamsT, - wallet_db: &Self::DataSource, + wallet_db: &Self::InputSource, + target_height: BlockHeight, + anchor_height: BlockHeight, account: AccountId, transaction_request: TransactionRequest, - min_confirmations: NonZeroU32, ) -> Result< - Proposal::DataSource as WalletRead>::NoteRef>, - InputSelectorError<<::DataSource as WalletRead>::Error, Self::Error>, + Proposal::NoteRef>, + InputSelectorError<::Error, Self::Error>, > where - ParamsT: consensus::Parameters; + ParamsT: consensus::Parameters, + Self::InputSource: SaplingInputSource; +} + +#[cfg(feature = "transparent-inputs")] +pub trait ShieldingSelector { + /// The type of errors that may be generated in input selection + type Error; + /// The type of data source that the input selector expects to access to obtain input + /// transparent UTXOs. This associated type permits input selectors that may use specialized + /// knowledge of the internals of a particular backing data store, if the generic API of + /// `WalletRead` does not provide sufficiently fine-grained operations for a particular backing + /// store to optimally perform input selection. + type InputSource; + /// The type of the fee rule that this input selector uses when computing fees. + type FeeRule: FeeRule; /// Performs input selection and returns a proposal for the construction of a shielding /// transaction. @@ -205,21 +219,22 @@ pub trait InputSelector { /// specified source addresses. If insufficient funds are available to satisfy the required /// outputs for the shielding request, this operation must fail and return /// [`InputSelectorError::InsufficientFunds`]. - #[allow(clippy::too_many_arguments)] #[allow(clippy::type_complexity)] fn propose_shielding( &self, params: &ParamsT, - wallet_db: &Self::DataSource, + wallet_db: &Self::InputSource, shielding_threshold: NonNegativeAmount, source_addrs: &[TransparentAddress], - min_confirmations: NonZeroU32, + target_height: BlockHeight, + min_confirmations: u32, ) -> Result< - Proposal::DataSource as WalletRead>::NoteRef>, - InputSelectorError<<::DataSource as WalletRead>::Error, Self::Error>, + Proposal, + InputSelectorError<::Error, Self::Error>, > where - ParamsT: consensus::Parameters; + ParamsT: consensus::Parameters, + Self::InputSource: TransparentInputSource; } /// Errors that can occur as a consequence of greedy input selection. @@ -318,32 +333,31 @@ impl GreedyInputSelector { impl InputSelector for GreedyInputSelector where - DbT: WalletRead, + DbT: SaplingInputSource, ChangeT: ChangeStrategy, ChangeT::FeeRule: Clone, { type Error = GreedyInputSelectorError; - type DataSource = DbT; + type InputSource = DbT; type FeeRule = ChangeT::FeeRule; #[allow(clippy::type_complexity)] fn propose_transaction( &self, params: &ParamsT, - wallet_db: &Self::DataSource, + wallet_db: &Self::InputSource, + target_height: BlockHeight, + anchor_height: BlockHeight, account: AccountId, transaction_request: TransactionRequest, - min_confirmations: NonZeroU32, - ) -> Result, InputSelectorError> + ) -> Result< + Proposal, + InputSelectorError<::Error, Self::Error>, + > where ParamsT: consensus::Parameters, + Self::InputSource: SaplingInputSource, { - // Target the next block, assuming we are up-to-date. - let (target_height, anchor_height) = wallet_db - .get_target_and_anchor_heights(min_confirmations) - .map_err(InputSelectorError::DataSource) - .and_then(|x| x.ok_or(InputSelectorError::SyncRequired))?; - let mut transparent_outputs = vec![]; let mut sapling_outputs = vec![]; for payment in transaction_request.payments() { @@ -404,9 +418,8 @@ where sapling_inputs, balance, fee_rule: (*self.change_strategy.fee_rule()).clone(), - min_confirmations, min_target_height: target_height, - min_anchor_height: anchor_height, + sapling_anchor_height: Some(anchor_height), is_shielding: false, }); } @@ -446,27 +459,45 @@ where } } } +} +#[cfg(feature = "transparent-inputs")] +impl ShieldingSelector for GreedyInputSelector +where + DbT: TransparentInputSource, + ChangeT: ChangeStrategy, + ChangeT::FeeRule: Clone, +{ + type Error = GreedyInputSelectorError; + type InputSource = DbT; + type FeeRule = ChangeT::FeeRule; + + #[cfg(feature = "transparent-inputs")] #[allow(clippy::type_complexity)] fn propose_shielding( &self, params: &ParamsT, - wallet_db: &Self::DataSource, + wallet_db: &Self::InputSource, shielding_threshold: NonNegativeAmount, source_addrs: &[TransparentAddress], - min_confirmations: NonZeroU32, - ) -> Result, InputSelectorError> + target_height: BlockHeight, + min_confirmations: u32, + ) -> Result< + Proposal, + InputSelectorError<::Error, Self::Error>, + > where ParamsT: consensus::Parameters, { - let (target_height, latest_anchor) = wallet_db - .get_target_and_anchor_heights(min_confirmations) - .map_err(InputSelectorError::DataSource) - .and_then(|x| x.ok_or(InputSelectorError::SyncRequired))?; - let mut transparent_inputs: Vec = source_addrs .iter() - .map(|taddr| wallet_db.get_unspent_transparent_outputs(taddr, latest_anchor, &[])) + .map(|taddr| { + wallet_db.get_unspent_transparent_outputs( + taddr, + target_height - min_confirmations, + &[], + ) + }) .collect::>, _>>() .map_err(InputSelectorError::DataSource)? .into_iter() @@ -478,7 +509,7 @@ where target_height, &transparent_inputs, &Vec::::new(), - &Vec::>::new(), + &Vec::>::new(), &Vec::::new(), &self.dust_output_policy, ); @@ -494,7 +525,7 @@ where target_height, &transparent_inputs, &Vec::::new(), - &Vec::>::new(), + &Vec::>::new(), &Vec::::new(), &self.dust_output_policy, )? @@ -511,9 +542,8 @@ where sapling_inputs: vec![], balance, fee_rule: (*self.change_strategy.fee_rule()).clone(), - min_confirmations, min_target_height: target_height, - min_anchor_height: latest_anchor, + sapling_anchor_height: None, is_shielding: true, }) } else { diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 2f72158d01..3f9f9ea5c1 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -112,7 +112,7 @@ pub enum ChangeError { DustInputs { /// The outpoints corresponding to transparent inputs having no current economic value. transparent: Vec, - /// The identifiers for Sapling inputs having not current economic value + /// The identifiers for Sapling inputs having no current economic value sapling: Vec, }, /// An error occurred that was specific to the change selection strategy in use. diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 37f59cbf80..8a9945eb6f 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -44,10 +44,7 @@ use std::{ }; use incrementalmerkletree::Position; -use shardtree::{ - error::{QueryError, ShardTreeError}, - ShardTree, -}; +use shardtree::{error::ShardTreeError, ShardTree}; use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight}, @@ -71,8 +68,9 @@ use zcash_client_backend::{ chain::{BlockSource, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, AccountBirthday, BlockMetadata, DecryptedTransaction, NoteId, NullifierQuery, PoolType, - Recipient, ScannedBlock, SentTransaction, ShieldedProtocol, WalletCommitmentTrees, - WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + Recipient, SaplingInputSource, ScannedBlock, SentTransaction, ShieldedProtocol, + TransparentInputSource, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, + SAPLING_SHARD_HEIGHT, }, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, @@ -94,7 +92,7 @@ pub mod error; pub mod wallet; use wallet::{ - commitment_tree::{self, get_checkpoint_depth, put_shard_roots}, + commitment_tree::{self, put_shard_roots}, SubtreeScanProgress, }; @@ -167,10 +165,59 @@ impl WalletDb { } } -impl, P: consensus::Parameters> WalletRead for WalletDb { +impl, P: consensus::Parameters> SaplingInputSource + for WalletDb +{ type Error = SqliteClientError; type NoteRef = ReceivedNoteId; + fn select_spendable_sapling_notes( + &self, + account: AccountId, + target_value: Amount, + anchor_height: BlockHeight, + exclude: &[Self::NoteRef], + ) -> Result>, Self::Error> { + wallet::sapling::select_spendable_sapling_notes( + self.conn.borrow(), + account, + target_value, + anchor_height, + exclude, + ) + } +} + +impl, P: consensus::Parameters> TransparentInputSource + for WalletDb +{ + type Error = SqliteClientError; + + fn get_unspent_transparent_outputs( + &self, + _address: &TransparentAddress, + _max_height: BlockHeight, + _exclude: &[OutPoint], + ) -> Result, Self::Error> { + #[cfg(feature = "transparent-inputs")] + return wallet::get_unspent_transparent_outputs( + self.conn.borrow(), + &self.params, + _address, + _max_height, + _exclude, + ); + + #[cfg(not(feature = "transparent-inputs"))] + panic!( + "The wallet must be compiled with the transparent-inputs feature to use this method." + ); + } +} + +impl, P: consensus::Parameters> WalletRead for WalletDb { + type Error = SqliteClientError; + fn chain_height(&self) -> Result, Self::Error> { wallet::scan_queue_extrema(self.conn.borrow()) .map(|h| h.map(|range| *range.end())) @@ -277,36 +324,6 @@ impl, P: consensus::Parameters> WalletRead for W } } - fn get_spendable_sapling_notes( - &self, - account: AccountId, - anchor_height: BlockHeight, - exclude: &[Self::NoteRef], - ) -> Result>, Self::Error> { - wallet::sapling::get_spendable_sapling_notes( - self.conn.borrow(), - account, - anchor_height, - exclude, - ) - } - - fn select_spendable_sapling_notes( - &self, - account: AccountId, - target_value: Amount, - anchor_height: BlockHeight, - exclude: &[Self::NoteRef], - ) -> Result>, Self::Error> { - wallet::sapling::select_spendable_sapling_notes( - self.conn.borrow(), - account, - target_value, - anchor_height, - exclude, - ) - } - fn get_transparent_receivers( &self, _account: AccountId, @@ -320,27 +337,6 @@ impl, P: consensus::Parameters> WalletRead for W ); } - fn get_unspent_transparent_outputs( - &self, - _address: &TransparentAddress, - _max_height: BlockHeight, - _exclude: &[OutPoint], - ) -> Result, Self::Error> { - #[cfg(feature = "transparent-inputs")] - return wallet::get_unspent_transparent_outputs( - self.conn.borrow(), - &self.params, - _address, - _max_height, - _exclude, - ); - - #[cfg(not(feature = "transparent-inputs"))] - panic!( - "The wallet must be compiled with the transparent-inputs feature to use this method." - ); - } - fn get_transparent_balances( &self, _account: AccountId, @@ -534,7 +530,7 @@ impl WalletWrite for WalletDb // Update the Sapling note commitment tree with all newly read note commitments let mut subtrees = subtrees.into_iter(); - wdb.with_sapling_tree_mut::<_, _, SqliteClientError>(move |sapling_tree| { + wdb.with_sapling_tree_mut::<_, _, Self::Error>(move |sapling_tree| { for (tree, checkpoints) in &mut subtrees { sapling_tree.insert_tree(tree, checkpoints)?; } @@ -792,18 +788,6 @@ impl WalletCommitmentTrees for WalletDb Result> { - get_checkpoint_depth(&self.conn, SAPLING_TABLES_PREFIX, min_confirmations) - .map_err(|e| ShardTreeError::Storage(commitment_tree::Error::Query(e)))? - // `CheckpointPruned` is perhaps a little misleading; in this case it's that - // the chain tip is unknown, but if that were the case we should never have been - // calling this anyway. - .ok_or(ShardTreeError::Query(QueryError::CheckpointPruned)) - } } impl<'conn, P: consensus::Parameters> WalletCommitmentTrees for WalletDb, P> { @@ -844,18 +828,6 @@ impl<'conn, P: consensus::Parameters> WalletCommitmentTrees for WalletDb Result> { - get_checkpoint_depth(self.conn.0, SAPLING_TABLES_PREFIX, min_confirmations) - .map_err(|e| ShardTreeError::Storage(commitment_tree::Error::Query(e)))? - // `CheckpointPruned` is perhaps a little misleading; in this case it's that - // the chain tip is unknown, but if that were the case we should never have been - // calling this anyway. - .ok_or(ShardTreeError::Query(QueryError::CheckpointPruned)) - } } /// A handle for the SQLite block source. diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index d2c6d91928..b40089fea1 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -70,7 +70,9 @@ use super::BlockDb; #[cfg(feature = "transparent-inputs")] use { - zcash_client_backend::data_api::wallet::{propose_shielding, shield_transparent_funds}, + zcash_client_backend::data_api::wallet::{ + input_selection::ShieldingSelector, propose_shielding, shield_transparent_funds, + }, zcash_primitives::legacy::TransparentAddress, }; @@ -481,7 +483,7 @@ impl TestState { >, > where - InputsT: InputSelector>, + InputsT: InputSelector>, { let params = self.network(); spend( @@ -514,7 +516,7 @@ impl TestState { >, > where - InputsT: InputSelector>, + InputsT: InputSelector>, { let params = self.network(); propose_transfer::<_, _, _, Infallible>( @@ -571,9 +573,9 @@ impl TestState { input_selector: &InputsT, shielding_threshold: NonNegativeAmount, from_addrs: &[TransparentAddress], - min_confirmations: NonZeroU32, + min_confirmations: u32, ) -> Result< - Proposal, + Proposal, data_api::error::Error< SqliteClientError, Infallible, @@ -582,7 +584,7 @@ impl TestState { >, > where - InputsT: InputSelector>, + InputsT: ShieldingSelector>, { let params = self.network(); propose_shielding::<_, _, _, Infallible>( @@ -633,7 +635,7 @@ impl TestState { shielding_threshold: NonNegativeAmount, usk: &UnifiedSpendingKey, from_addrs: &[TransparentAddress], - min_confirmations: NonZeroU32, + min_confirmations: u32, ) -> Result< TxId, data_api::error::Error< @@ -644,7 +646,7 @@ impl TestState { >, > where - InputsT: InputSelector>, + InputsT: ShieldingSelector>, { let params = self.network(); shield_transparent_funds( diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 650eb3175b..eaa45485f1 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -67,7 +67,6 @@ use incrementalmerkletree::Retention; use rusqlite::{self, named_params, OptionalExtension}; use shardtree::ShardTree; -use std::cmp; use std::collections::{BTreeMap, HashMap}; use std::convert::TryFrom; use std::io::{self, Cursor}; @@ -100,7 +99,7 @@ use zcash_client_backend::{ wallet::WalletTx, }; -use crate::wallet::commitment_tree::SqliteShardStore; +use crate::wallet::commitment_tree::{get_max_checkpointed_height, SqliteShardStore}; use crate::{ error::SqliteClientError, SqlTransaction, WalletCommitmentTrees, WalletDb, PRUNING_DEPTH, SAPLING_TABLES_PREFIX, @@ -932,19 +931,19 @@ pub(crate) fn get_target_and_anchor_heights( conn: &rusqlite::Connection, min_confirmations: NonZeroU32, ) -> Result, rusqlite::Error> { - scan_queue_extrema(conn).map(|heights| { - heights.map(|range| { - let target_height = *range.end() + 1; - // Select an anchor min_confirmations back from the target block, - // unless that would be before the earliest block we have. - let anchor_height = BlockHeight::from(cmp::max( - u32::from(target_height).saturating_sub(min_confirmations.into()), - u32::from(*range.start()), - )); - - (target_height, anchor_height) - }) - }) + match scan_queue_extrema(conn)?.map(|range| *range.end()) { + Some(chain_tip_height) => { + let sapling_anchor_height = get_max_checkpointed_height( + conn, + SAPLING_TABLES_PREFIX, + chain_tip_height, + min_confirmations, + )?; + + Ok(sapling_anchor_height.map(|h| (chain_tip_height + 1, h))) + } + None => Ok(None), + } } fn parse_block_metadata( @@ -1915,7 +1914,9 @@ mod tests { PRUNING_DEPTH, }, zcash_client_backend::{ - data_api::{wallet::input_selection::GreedyInputSelector, WalletWrite}, + data_api::{ + wallet::input_selection::GreedyInputSelector, TransparentInputSource, WalletWrite, + }, encoding::AddressCodec, fees::{fixed, DustOutputPolicy}, wallet::WalletTransparentOutput, @@ -2141,13 +2142,7 @@ mod tests { DustOutputPolicy::default(), ); let txid = st - .shield_transparent_funds( - &input_selector, - value, - &usk, - &[*taddr], - NonZeroU32::new(1).unwrap(), - ) + .shield_transparent_funds(&input_selector, value, &usk, &[*taddr], 1) .unwrap(); // The wallet should have zero transparent balance, because the shielding diff --git a/zcash_client_sqlite/src/wallet/commitment_tree.rs b/zcash_client_sqlite/src/wallet/commitment_tree.rs index d0654063d0..8cd0b4d73a 100644 --- a/zcash_client_sqlite/src/wallet/commitment_tree.rs +++ b/zcash_client_sqlite/src/wallet/commitment_tree.rs @@ -21,8 +21,6 @@ use zcash_primitives::{consensus::BlockHeight, merkle_tree::HashSer}; use zcash_client_backend::serialization::shardtree::{read_shard, write_shard}; -use super::scan_queue_extrema; - /// Errors that can appear in SQLite-back [`ShardStore`] implementation operations. #[derive(Debug)] pub enum Error { @@ -173,6 +171,7 @@ impl<'conn, 'a: 'conn, H: HashSer, const SHARD_HEIGHT: u8> ShardStore checkpoint_depth: usize, ) -> Result, Self::Error> { get_checkpoint_at_depth(self.conn, self.table_prefix, checkpoint_depth) + .map_err(Error::Query) } fn get_checkpoint( @@ -280,6 +279,7 @@ impl ShardStore checkpoint_depth: usize, ) -> Result, Self::Error> { get_checkpoint_at_depth(&self.conn, self.table_prefix, checkpoint_depth) + .map_err(Error::Query) } fn get_checkpoint( @@ -750,38 +750,37 @@ pub(crate) fn get_checkpoint( .transpose() } -pub(crate) fn get_checkpoint_depth( +pub(crate) fn get_max_checkpointed_height( conn: &rusqlite::Connection, table_prefix: &'static str, + chain_tip_height: BlockHeight, min_confirmations: NonZeroU32, -) -> Result, rusqlite::Error> { - scan_queue_extrema(conn)? - .map(|range| *range.end()) - .map(|chain_tip| { - let max_checkpoint_height = - u32::from(chain_tip).saturating_sub(u32::from(min_confirmations) - 1); - - // We exclude from consideration all checkpoints having heights greater than the maximum - // checkpoint height. The checkpoint depth is the number of excluded checkpoints + 1. - conn.query_row( - &format!( - "SELECT COUNT(*) - FROM {}_tree_checkpoints - WHERE checkpoint_id > :max_checkpoint_height", - table_prefix - ), - named_params![":max_checkpoint_height": max_checkpoint_height], - |row| row.get::<_, usize>(0).map(|s| s + 1), - ) - }) - .transpose() +) -> Result, rusqlite::Error> { + let max_checkpoint_height = + u32::from(chain_tip_height).saturating_sub(u32::from(min_confirmations) - 1); + + // We exclude from consideration all checkpoints having heights greater than the maximum + // checkpoint height. The checkpoint depth is the number of excluded checkpoints + 1. + conn.query_row( + &format!( + "SELECT checkpoint_id + FROM {}_tree_checkpoints + WHERE checkpoint_id <= :max_checkpoint_height + ORDER BY checkpoint_id DESC + LIMIT 1", + table_prefix + ), + named_params![":max_checkpoint_height": max_checkpoint_height], + |row| row.get::<_, u32>(0).map(BlockHeight::from), + ) + .optional() } pub(crate) fn get_checkpoint_at_depth( conn: &rusqlite::Connection, table_prefix: &'static str, checkpoint_depth: usize, -) -> Result, Error> { +) -> Result, rusqlite::Error> { if checkpoint_depth == 0 { return Ok(None); } @@ -806,27 +805,21 @@ pub(crate) fn get_checkpoint_at_depth( )) }, ) - .optional() - .map_err(Error::Query)?; + .optional()?; checkpoint_parts .map(|(checkpoint_id, pos_opt)| { - let mut stmt = conn - .prepare_cached(&format!( - "SELECT mark_removed_position + let mut stmt = conn.prepare_cached(&format!( + "SELECT mark_removed_position FROM {}_tree_checkpoint_marks_removed WHERE checkpoint_id = ?", - table_prefix - )) - .map_err(Error::Query)?; - let mark_removed_rows = stmt - .query([u32::from(checkpoint_id)]) - .map_err(Error::Query)?; + table_prefix + ))?; + let mark_removed_rows = stmt.query([u32::from(checkpoint_id)])?; let marks_removed = mark_removed_rows .mapped(|row| row.get::<_, u64>(0).map(Position::from)) - .collect::, _>>() - .map_err(Error::Query)?; + .collect::, _>>()?; Ok(( checkpoint_id, @@ -1168,6 +1161,7 @@ mod tests { // simulate discovery of a note let mut tree = ShardTree::<_, 6, 3>::new(store, 10); + let checkpoint_height = BlockHeight::from(3); tree.batch_insert( Position::from(24), ('a'..='h').into_iter().map(|c| { @@ -1176,7 +1170,7 @@ mod tests { match c { 'c' => Retention::Marked, 'h' => Retention::Checkpoint { - id: BlockHeight::from(3), + id: checkpoint_height, is_marked: false, }, _ => Retention::Ephemeral, @@ -1187,7 +1181,9 @@ mod tests { .unwrap(); // construct a witness for the note - let witness = tree.witness(Position::from(26), 0).unwrap(); + let witness = tree + .witness_at_checkpoint_id(Position::from(26), &checkpoint_height) + .unwrap(); assert_eq!( witness.path_elems(), &[ diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 129c65f5db..1ffbe850c0 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -159,62 +159,6 @@ fn unscanned_tip_exists( ) } -pub(crate) fn get_spendable_sapling_notes( - conn: &Connection, - account: AccountId, - anchor_height: BlockHeight, - exclude: &[ReceivedNoteId], -) -> Result>, SqliteClientError> { - let birthday_height = match wallet_birthday(conn)? { - Some(birthday) => birthday, - None => { - // the wallet birthday can only be unknown if there are no accounts in the wallet; in - // such a case, the wallet has no notes to spend. - return Ok(vec![]); - } - }; - - if unscanned_tip_exists(conn, anchor_height)? { - return Ok(vec![]); - } - - let mut stmt_select_notes = conn.prepare_cached( - "SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position - FROM sapling_received_notes - INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx - WHERE account = :account - AND commitment_tree_position IS NOT NULL - AND spent IS NULL - AND transactions.block <= :anchor_height - AND id_note NOT IN rarray(:exclude) - AND NOT EXISTS ( - SELECT 1 FROM v_sapling_shard_unscanned_ranges unscanned - -- select all the unscanned ranges involving the shard containing this note - WHERE sapling_received_notes.commitment_tree_position >= unscanned.start_position - AND sapling_received_notes.commitment_tree_position < unscanned.end_position_exclusive - -- exclude unscanned ranges that start above the anchor height (they don't affect spendability) - AND unscanned.block_range_start <= :anchor_height - -- exclude unscanned ranges that end below the wallet birthday - AND unscanned.block_range_end > :wallet_birthday - )", - )?; - - let excluded: Vec = exclude.iter().map(|n| Value::from(n.0)).collect(); - let excluded_ptr = Rc::new(excluded); - - let notes = stmt_select_notes.query_and_then( - named_params![ - ":account": u32::from(account), - ":anchor_height": u32::from(anchor_height), - ":exclude": &excluded_ptr, - ":wallet_birthday": u32::from(birthday_height) - ], - to_spendable_note, - )?; - - notes.collect::>() -} - pub(crate) fn select_spendable_sapling_notes( conn: &Connection, account: AccountId, @@ -768,7 +712,7 @@ pub(crate) mod tests { st.propose_standard_transfer::( account, StandardFeeRule::Zip317, - NonZeroU32::new(10).unwrap(), + NonZeroU32::new(2).unwrap(), &to, NonNegativeAmount::const_from_u64(70000), None, @@ -1467,7 +1411,7 @@ pub(crate) mod tests { NonNegativeAmount::from_u64(10000).unwrap(), &usk, &[*taddr], - NonZeroU32::new(1).unwrap() + 1 ), Ok(_) ); diff --git a/zcash_primitives/src/transaction/sighash.rs b/zcash_primitives/src/transaction/sighash.rs index 9ab8bf4090..9d89fde97a 100644 --- a/zcash_primitives/src/transaction/sighash.rs +++ b/zcash_primitives/src/transaction/sighash.rs @@ -5,7 +5,7 @@ use super::{ components::{ amount::NonNegativeAmount, sapling::{self, GrothProofBytes}, - transparent, Amount, + transparent, }, sighash_v4::v4_signature_hash, sighash_v5::v5_signature_hash, @@ -13,7 +13,7 @@ use super::{ }; #[cfg(feature = "zfuture")] -use crate::extensions::transparent::Precondition; +use {super::components::Amount, crate::extensions::transparent::Precondition}; pub const SIGHASH_ALL: u8 = 0x01; pub const SIGHASH_NONE: u8 = 0x02;