diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 368a48a51..31cd69b75 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -132,7 +132,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 8fe811a95..970e9e33d 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, @@ -764,7 +708,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, @@ -1463,7 +1407,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;