From aa063ae3fd76b3eb8ae10ef0e233476c7a03da3a 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. --- .github/workflows/ci.yml | 2 +- Cargo.lock | 5 +- Cargo.toml | 2 +- zcash_client_backend/CHANGELOG.md | 51 ++++- zcash_client_backend/Cargo.toml | 2 + zcash_client_backend/src/data_api.rs | 168 ++++++++------- zcash_client_backend/src/data_api/wallet.rs | 131 +++++++----- .../src/data_api/wallet/input_selection.rs | 193 +++++++++++------- zcash_client_backend/src/fees.rs | 2 +- zcash_client_sqlite/src/lib.rs | 145 ++++++------- zcash_client_sqlite/src/testing.rs | 18 +- zcash_client_sqlite/src/wallet.rs | 39 ++-- .../src/wallet/commitment_tree.rs | 74 ++++--- zcash_client_sqlite/src/wallet/sapling.rs | 60 +----- zcash_primitives/src/transaction/sighash.rs | 4 +- 15 files changed, 461 insertions(+), 435 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b0585a939..0637ab65f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -162,7 +162,7 @@ jobs: - run: cargo fetch # Requires #![deny(rustdoc::broken_intra_doc_links)] in crates. - name: Check intra-doc links - run: cargo doc --all --document-private-items + run: cargo doc --all --all-features --document-private-items fmt: name: Rustfmt diff --git a/Cargo.lock b/Cargo.lock index 314ffd5ef..6794c1254 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2183,9 +2183,9 @@ dependencies = [ [[package]] name = "shardtree" -version = "0.1.0" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c19f96dde3a8693874f7e7c53d95616569b4009379a903789efbd448f4ea9cc7" +checksum = "dbf20c7a2747d9083092e3a3eeb9a7ed75577ae364896bebbc5e0bdcd4e97735" dependencies = [ "assert_matches", "bitflags 2.4.0", @@ -2963,6 +2963,7 @@ dependencies = [ "jubjub", "memuse", "nom", + "nonempty", "orchard", "percent-encoding", "proptest", diff --git a/Cargo.toml b/Cargo.toml index bb3cf1231..c51b93b61 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ zcash_proofs = { version = "0.13", path = "zcash_proofs", default-features = fal ff = "0.13" group = "0.13" incrementalmerkletree = "0.5" -shardtree = "0.1" +shardtree = "0.2" # Payment protocols # - Sapling diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index d8932ab5d..35497ef65 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -8,10 +8,16 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Added -- `zcash_client_backend::data_api::wallet::propose_standard_transfer_to_address` +- `zcash_client_backend::data_api`: + - `TransparentInputSource` + - `SaplingInputSource` + - `wallet::propose_standard_transfer_to_address` + - `wallet::input_selection::SaplingInputs` + - `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. - `zcash_client_backend::fees::standard` - `zcash_client_backend::wallet`: - - `input_selection::Proposal::min_confirmations` - `ReceivedSaplingNote::from_parts` - `ReceivedSaplingNote::{txid, output_index, diversifier, rseed, note_commitment_tree_position}` @@ -39,7 +45,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. - The error type of `wallet::create_spend_to_address` has been changed to use @@ -51,6 +57,39 @@ and this library adheres to Rust's notion of - `wallet::create_spend_to_address` - `wallet::shield_transparent_funds` - `wallet::spend` + - In order to support better reusability for input selection code, two new + traits have been factored out from `WalletRead`: + - `zcash_client_backend::data_api::TransparentInputSource` + - `zcash_client_backend::data_api::SaplingInputSource` + - `wallet::input_selection::InputSelector::propose_shielding`, + has been moved out to the newly-created `ShieldingSelector` trait. + - `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`. + - The `wallet::input_selection::InputSelector::DataSource` + associated type has been renamed to `InputSource`. + - The signature of `wallet:input_selection::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. + - `WalletRead::get_checkpoint_depth` has been removed without replacement. This + is no longer needed given the change to use the stored anchor height for transaction + proposal execution. + - `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. + - `wallet::create_proposed_transaction` now forces implementations to ignore + the database identifiers for its contained notes by universally quantifying + the `NoteRef` type parameter. + - `wallet::input_selection::Proposal::sapling_inputs` now returns type + `Option<&SaplingInputs>`. + - `wallet::input_selection::Proposal::min_anchor_height` has been removed in + favor of storing this value in `SaplingInputs`. + - `wallet::input_selection::GreedyInputSelector` now has relaxed requirements + for its `InputSource` associated type. + - `zcash_client_backend::fees`: - `ChangeValue::Sapling` is now a structured variant. In addition to the existing change value, it now also carries an optional memo to be associated @@ -92,8 +131,12 @@ and this library adheres to Rust's notion of ### 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 2522f80ef..04b9fe1b0 100644 --- a/zcash_client_backend/Cargo.toml +++ b/zcash_client_backend/Cargo.toml @@ -30,6 +30,7 @@ zcash_primitives.workspace = true # (Breaking upgrades to these require a breaking upgrade to this crate.) # - Data Access API time = "0.3.22" +nonempty.workspace = true # - Encodings base64.workspace = true @@ -87,6 +88,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 284247403..c623de26d 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -17,10 +17,7 @@ use zcash_primitives::{ memo::{Memo, MemoBytes}, sapling::{self, Node, NOTE_COMMITMENT_TREE_DEPTH}, transaction::{ - components::{ - amount::{Amount, NonNegativeAmount}, - OutPoint, - }, + components::amount::{Amount, NonNegativeAmount}, Transaction, TxId, }, zip32::AccountId, @@ -34,6 +31,9 @@ use crate::{ wallet::{ReceivedSaplingNote, WalletTransparentOutput, WalletTx}, }; +#[cfg(feature = "transparent-inputs")] +use zcash_primitives::transaction::components::OutPoint; + use self::chain::CommitmentTreeRoot; use self::scanning::ScanRange; @@ -210,12 +210,9 @@ 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 { +/// A trait representing the capability to query a data store for unspent Sapling notes belonging +/// to a wallet. +pub trait SaplingInputSource { /// The type of errors produced by a wallet backend. type Error; @@ -225,6 +222,43 @@ 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>; +} + +/// A trait representing the capability to query a data store for unspent transparent UTXOs +/// belonging to a wallet. +#[cfg(feature = "transparent-inputs")] +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 { + /// The type of errors that may be generated when querying a wallet data store. + 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 +385,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 +395,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 +904,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( @@ -939,10 +937,7 @@ pub mod testing { legacy::TransparentAddress, memo::Memo, sapling, - transaction::{ - components::{Amount, OutPoint}, - Transaction, TxId, - }, + transaction::{components::Amount, Transaction, TxId}, zip32::AccountId, }; @@ -954,8 +949,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, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, + SAPLING_SHARD_HEIGHT, }; pub struct MockWalletDb { @@ -967,6 +963,9 @@ pub mod testing { >, } + #[cfg(feature = "transparent-inputs")] + use {super::TransparentInputSource, zcash_primitives::transaction::components::OutPoint}; + impl MockWalletDb { pub fn new(network: Network) -> Self { Self { @@ -976,10 +975,38 @@ 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()) + } + } + + #[cfg(feature = "transparent-inputs")] + 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 +1106,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 +1113,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 +1213,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 79d2509ad..5a80fdc36 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}, }; @@ -216,7 +221,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 @@ -324,10 +331,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()) @@ -368,21 +377,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) } @@ -421,7 +441,7 @@ pub fn propose_standard_transfer_to_address( ) -> Result< Proposal, Error< - DbT::Error, + ::Error, CommitmentTreeErrT, GreedyInputSelectorError, Zip317FeeError, @@ -429,7 +449,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 { @@ -467,23 +487,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) @@ -499,14 +529,14 @@ 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, spend_prover: &impl SpendProver, output_prover: &impl OutputProver, usk: &UnifiedSpendingKey, ovk_policy: OvkPolicy, - proposal: &Proposal, + proposal: &Proposal, ) -> Result< TxId, Error< @@ -518,7 +548,6 @@ pub fn create_proposed_transaction( > where DbT: WalletWrite + WalletCommitmentTrees, - DbT::NoteRef: Copy + Eq + Ord, ParamsT: consensus::Parameters + Clone, FeeRuleT: FeeRule, { @@ -557,28 +586,29 @@ 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( - sapling_tree, - selected, - usk.sapling(), - &dfvk, - checkpoint_depth, - )? - .ok_or_else(|| { - Error::NoteMismatch(NoteId { - txid: *selected.txid(), - protocol: ShieldedProtocol::Sapling, - output_index: selected.output_index(), - }) - })?; + if let Some(sapling_inputs) = proposal.sapling_inputs() { + wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _>>(|sapling_tree| { + for selected in sapling_inputs.notes() { + let (note, key, merkle_path) = select_key_for_note( + sapling_tree, + selected, + usk.sapling(), + &dfvk, + sapling_inputs.anchor_height(), + )? + .ok_or_else(|| { + Error::NoteMismatch(NoteId { + txid: *selected.txid(), + protocol: ShieldedProtocol::Sapling, + output_index: selected.output_index(), + }) + })?; - builder.add_sapling_spend(key, selected.diversifier(), note, merkle_path)?; - } - Ok(()) - })?; + builder.add_sapling_spend(key, selected.diversifier(), note, merkle_path)?; + } + Ok(()) + })?; + } #[cfg(feature = "transparent-inputs")] let utxos = { @@ -807,7 +837,7 @@ pub fn shield_transparent_funds( shielding_threshold: NonNegativeAmount, usk: &UnifiedSpendingKey, from_addrs: &[TransparentAddress], - min_confirmations: NonZeroU32, + min_confirmations: u32, ) -> Result< TxId, Error< @@ -819,9 +849,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, @@ -853,7 +884,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, @@ -868,9 +899,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 297ab4c84..ccba6de00 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -1,10 +1,9 @@ //! 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 nonempty::NonEmpty; use zcash_primitives::{ consensus::{self, BlockHeight}, legacy::TransparentAddress, @@ -12,7 +11,7 @@ use zcash_primitives::{ components::{ amount::{BalanceError, NonNegativeAmount}, sapling::fees as sapling, - OutPoint, TxOut, + TxOut, }, fees::FeeRule, }, @@ -21,12 +20,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. @@ -73,17 +78,14 @@ impl fmt::Display for InputSelectorError { transaction_request: TransactionRequest, transparent_inputs: Vec, - sapling_inputs: Vec>, + sapling_inputs: Option>, balance: TransactionBalance, fee_rule: FeeRuleT, - min_confirmations: NonZeroU32, min_target_height: BlockHeight, - min_anchor_height: BlockHeight, is_shielding: bool, } @@ -97,8 +99,8 @@ impl Proposal { &self.transparent_inputs } /// Returns the Sapling inputs that have been selected to fund the transaction. - pub fn sapling_inputs(&self) -> &[ReceivedSaplingNote] { - &self.sapling_inputs + pub fn sapling_inputs(&self) -> Option<&SaplingInputs> { + self.sapling_inputs.as_ref() } /// Returns the change outputs to be added to the transaction and the fee to be paid. pub fn balance(&self) -> &TransactionBalance { @@ -108,10 +110,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,14 +117,6 @@ impl Proposal { pub fn min_target_height(&self) -> BlockHeight { self.min_target_height } - /// Returns the 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 - } /// Returns a flag indicating whether or not the proposed transaction /// is exclusively wallet-internal (if it does not involve any external /// recipients). @@ -140,16 +130,41 @@ impl Debug for Proposal { f.debug_struct("Proposal") .field("transaction_request", &self.transaction_request) .field("transparent_inputs", &self.transparent_inputs) - .field("sapling_inputs", &self.sapling_inputs.len()) + .field( + "sapling_inputs", + &self.sapling_inputs().map(|i| i.notes.len()), + ) + .field( + "sapling_anchor_height", + &self.sapling_inputs().map(|i| i.anchor_height), + ) .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_non_exhaustive() } } +/// The Sapling inputs to a proposed transaction. +pub struct SaplingInputs { + anchor_height: BlockHeight, + notes: NonEmpty>, +} + +impl SaplingInputs { + /// Returns the anchor height for Sapling inputs that should be used when constructing the + /// proposed transaction. + pub fn anchor_height(&self) -> BlockHeight { + self.anchor_height + } + + /// Returns the list of Sapling notes to be used as inputs to the proposed transaction. + pub fn notes(&self) -> &NonEmpty> { + &self.notes + } +} + /// A strategy for selecting transaction inputs and proposing transaction outputs. /// /// Proposals should include only economically useful inputs, as determined by `Self::FeeRule`; @@ -158,12 +173,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 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; + /// 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 + /// `SaplingInputSource` does not provide sufficiently fine-grained operations for a particular + /// backing store to optimally perform input selection. + type InputSource: SaplingInputSource; /// The type of the fee rule that this input selector uses when computing fees. type FeeRule: FeeRule; @@ -181,21 +196,38 @@ 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; +} + +/// A strategy for selecting transaction inputs and proposing transaction outputs +/// for shielding-only transactions (transactions which spend transparent UTXOs and +/// send all transaction outputs to the wallet's shielded internal address(es)). +#[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 + /// `TransparentInputSource` does not provide sufficiently fine-grained operations for a + /// particular backing store to optimally perform input selection. + type InputSource: TransparentInputSource; + /// 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,18 +237,18 @@ 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; @@ -296,8 +328,8 @@ impl sapling::OutputView for SaplingPayment { /// An [`InputSelector`] implementation that uses a greedy strategy to select between available /// notes. /// -/// This implementation performs input selection using methods available via the [`WalletRead`] -/// interface. +/// This implementation performs input selection using methods available via the +/// [`SaplingInputSource`] and [`TransparentInputSource`] interfaces. pub struct GreedyInputSelector { change_strategy: ChangeT, dust_output_policy: DustOutputPolicy, @@ -318,32 +350,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() { @@ -401,12 +432,15 @@ where return Ok(Proposal { transaction_request, transparent_inputs: vec![], - sapling_inputs, + sapling_inputs: NonEmpty::from_vec(sapling_inputs).map(|notes| { + SaplingInputs { + anchor_height, + notes, + } + }), balance, fee_rule: (*self.change_strategy.fee_rule()).clone(), - min_confirmations, min_target_height: target_height, - min_anchor_height: anchor_height, is_shielding: false, }); } @@ -446,27 +480,44 @@ 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; #[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 +529,7 @@ where target_height, &transparent_inputs, &Vec::::new(), - &Vec::>::new(), + &Vec::>::new(), &Vec::::new(), &self.dust_output_policy, ); @@ -494,7 +545,7 @@ where target_height, &transparent_inputs, &Vec::::new(), - &Vec::>::new(), + &Vec::>::new(), &Vec::::new(), &self.dust_output_policy, )? @@ -508,12 +559,10 @@ where Ok(Proposal { transaction_request: TransactionRequest::empty(), transparent_inputs, - sapling_inputs: vec![], + sapling_inputs: None, balance, fee_rule: (*self.change_strategy.fee_rule()).clone(), - min_confirmations, min_target_height: target_height, - min_anchor_height: latest_anchor, is_shielding: true, }) } else { diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 7a1568d15..a10a119e3 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 37f59cbf8..047c1d6ea 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}, @@ -55,10 +52,7 @@ use zcash_primitives::{ memo::{Memo, MemoBytes}, sapling, transaction::{ - components::{ - amount::{Amount, NonNegativeAmount}, - OutPoint, - }, + components::amount::{Amount, NonNegativeAmount}, Transaction, TxId, }, zip32::{AccountId, DiversifierIndex}, @@ -71,8 +65,8 @@ 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, + WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, @@ -82,6 +76,12 @@ use zcash_client_backend::{ use crate::{error::SqliteClientError, wallet::commitment_tree::SqliteShardStore}; +#[cfg(feature = "transparent-inputs")] +use { + zcash_client_backend::data_api::TransparentInputSource, + zcash_primitives::transaction::components::OutPoint, +}; + #[cfg(feature = "unstable")] use { crate::chain::{fsblockdb_with_blocks, BlockMeta}, @@ -94,7 +94,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 +167,54 @@ 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, + ) + } +} + +#[cfg(feature = "transparent-inputs")] +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> { + wallet::get_unspent_transparent_outputs( + self.conn.borrow(), + &self.params, + address, + max_height, + exclude, + ) + } +} + +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 +321,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 +334,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 +527,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 +785,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 +825,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 dd90a755b..aa624c542 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, }; @@ -483,7 +485,7 @@ impl TestState { >, > where - InputsT: InputSelector>, + InputsT: InputSelector>, { let params = self.network(); let prover = test_prover(); @@ -518,7 +520,7 @@ impl TestState { >, > where - InputsT: InputSelector>, + InputsT: InputSelector>, { let params = self.network(); propose_transfer::<_, _, _, Infallible>( @@ -575,9 +577,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, @@ -586,7 +588,7 @@ impl TestState { >, > where - InputsT: InputSelector>, + InputsT: ShieldingSelector>, { let params = self.network(); propose_shielding::<_, _, _, Infallible>( @@ -639,7 +641,7 @@ impl TestState { shielding_threshold: NonNegativeAmount, usk: &UnifiedSpendingKey, from_addrs: &[TransparentAddress], - min_confirmations: NonZeroU32, + min_confirmations: u32, ) -> Result< TxId, data_api::error::Error< @@ -650,7 +652,7 @@ impl TestState { >, > where - InputsT: InputSelector>, + InputsT: ShieldingSelector>, { let params = self.network(); let prover = test_prover(); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 650eb3175..eaa45485f 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()), - )); + 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, + )?; - (target_height, anchor_height) - }) - }) + 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 d0654063d..8cd0b4d73 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); +) -> 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 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() + // 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 01f3d39e7..f6fd92d91 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, @@ -769,7 +713,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, @@ -1468,7 +1412,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 9ab8bf409..9d89fde97 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;