diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index f68c6a16b..720c7b35c 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -28,6 +28,7 @@ and this library adheres to Rust's notion of - `AddressMetadata` - `zcash_client_backend::data_api`: - `PoolType` + - `ShieldedPool` - `Recipient` - `SentTransactionOutput` - `WalletRead::get_unified_full_viewing_keys` @@ -42,6 +43,11 @@ and this library adheres to Rust's notion of - `WalletWrite::get_next_available_address` - `WalletWrite::put_received_transparent_utxo` - `impl From for error::Error` + - `chain::error`: a module containing error types type that that can occur only + in chain validation and sync have been separated out from errors related to + other wallet operations. + - `input_selection`: a module containing types related to the process + of selecting inputs to be spent, given a transaction request. - `zcash_client_backend::decrypt`: - `TransferType` - `zcash_client_backend::proto`: @@ -50,6 +56,7 @@ and this library adheres to Rust's notion of - gRPC bindings for the `lightwalletd` server, behind a `lightwalletd-tonic` feature flag. - `zcash_client_backend::zip321::TransactionRequest` methods: + - `TransactionRequest::empty` for constructing a new empty request. - `TransactionRequest::new` for constructing a request from `Vec`. - `TransactionRequest::payments` for accessing the `Payments` that make up a request. @@ -64,6 +71,10 @@ and this library adheres to Rust's notion of - `TransactionBalance` - `BasicFixedFeeChangeStrategy` - a `ChangeStrategy` implementation that reproduces current wallet change behavior + - `fixed`, a new module containing of change selection strategies for the + existing fixed fee rule. + - `zip317`, a new module containing change selection strategies for the ZIP + 317 fee rule. - New experimental APIs that should be considered unstable, and are likely to be modified and/or moved to a different module in a future release: @@ -72,6 +83,7 @@ and this library adheres to Rust's notion of - `zcash_client_backend::encoding::AddressCodec` - `zcash_client_backend::encoding::encode_payment_address` - `zcash_client_backend::encoding::encode_transparent_address` +- `impl Eq for zcash_client_backend::address::UnifiedAddress` ### Changed - MSRV is now 1.56.1. @@ -90,13 +102,8 @@ and this library adheres to Rust's notion of been replaced by `ephemeral_key`. - `zcash_client_backend::proto::compact_formats::CompactSaplingOutput`: the `epk` method has been replaced by `ephemeral_key`. -- `data_api::wallet::spend_to_address` now takes a `min_confirmations` - parameter, which the caller can provide to specify the minimum number of - confirmations required for notes being selected. A default value of 10 - confirmations is recommended. - Renamed the following in `zcash_client_backend::data_api` to use lower-case abbreviations (matching Rust naming conventions): - - `error::Error::InvalidExtSK` to `Error::InvalidExtSk` - `testing::MockWalletDB` to `testing::MockWalletDb` - Changes to the `data_api::WalletRead` trait: - `WalletRead::get_target_and_anchor_heights` now takes @@ -104,9 +111,13 @@ and this library adheres to Rust's notion of the anchor height being returned; this had previously been hardcoded to `data_api::wallet::ANCHOR_OFFSET`. - `WalletRead::get_spendable_notes` has been renamed to - `get_spendable_sapling_notes` + `get_spendable_sapling_notes` and now takes as an argument a vector of + note IDs to be excluded from consideration. - `WalletRead::select_spendable_notes` has been renamed to - `select_spendable_sapling_notes` + `select_spendable_sapling_notes` and now takes as an argument a vector of + note IDs to be excluded from consideration. + - The `WalletRead::NoteRef` and `WalletRead::TxRef` associated types are now + required to implement `Eq` and `Ord` - The `zcash_client_backend::data_api::SentTransaction` type has been substantially modified to accommodate handling of transparent inputs. Per-output data has been split out into a new struct `SentTransactionOutput` @@ -116,24 +127,28 @@ and this library adheres to Rust's notion of `store_decrypted_tx`. - `data_api::ReceivedTransaction` has been renamed to `DecryptedTransaction`, and its `outputs` field has been renamed to `sapling_outputs`. -- `data_api::error::Error::Protobuf` now wraps `prost::DecodeError` instead of - `protobuf::ProtobufError`. -- `data_api::error::Error` has the following additional cases: - - `Error::BalanceError` in the case of amount addition overflow - or subtraction underflow. - - `Error::MemoForbidden` to report the condition where a memo was - specified to be sent to a transparent recipient. - - `Error::TransparentInputsNotSupported` to represent the condition - where a transparent spend has been requested of a wallet compiled without - the `transparent-inputs` feature. - - `Error::AddressNotRecognized` to indicate that a transparent address from - which funds are being requested to be spent does not appear to be associated - with this wallet. - - `Error::ChildIndexOutOfRange` to indicate that a diversifier index for an - address is out of range for valid transparent child indices. - - `Error::NoteMismatch` to indicate that a note being spent is not associated - with either the internal or external full viewing keys corresponding to the - provided spending key. +- `data_api::error::Error` has been substantially modified. It now wraps database, + note selection, builder, and other errors + - `Error::DataSource` has been added. + - `Error::NoteSelection` has been added. + - `Error::BalanceError` has been added. + - `Error::MemoForbidden` has been added. + - `Error::AddressNotRecognized` has been added. + - `Error::ChildIndexOutOfRange` has been added. + - `Error::NoteMismatch` has been added. + - `Error::InsufficientBalance` has been renamed `InsufficientFunds` and + restructured to have named fields. + - `Error::Protobuf` has been removed; these decoding errors are now + produced as data source and/or block-source implementation-specific + errors. + - `Error::InvalidChain` has been removed; its former purpose is now served + by `zcash_client_backend::data_api::chain::ChainError`. + - `Error::InvalidNewWitnessAnchor` and `Error::InvalidWitnessAnchor` have + been moved to `zcash_client_backend::data_api::chain::error::ContinuityError` + - `Error::InvalidExtSk` (now unused) has been removed. + - `Error::KeyNotFound` (now unused) has been removed. + - `Error::KeyDerivationError` (now unused) has been removed. + - `Error::SaplingNotActive` (now unused) has been removed. - `zcash_client_backend::decrypt`: - `decrypt_transaction` now takes a `HashMap<_, UnifiedFullViewingKey>` instead of `HashMap<_, ExtendedFullViewingKey>`. @@ -160,8 +175,38 @@ and this library adheres to Rust's notion of - `decode_extended_spending_key` - `decode_extended_full_viewing_key` - `decode_payment_address` -- `data_api::wallet::create_spend_to_address` has been modified to use a unified - spending key rather than a Sapling extended spending key. +- `zcash_client_backend::wallet::SpendableNote` is now parameterized by a note + identifier type and has an additional `note_id` field that is used to hold the + identifier used to refer to the note in the wallet database. +- `zcash_client_backend::data_api::BlockSource` has been moved to the + `zcash_client_backend::data_api::chain` module. +- The types of the `with_row` callback argument to `BlockSource::with_blocks` + and the return type of this method have been modified to return + `zcash_client_backend::data_api::chain::error::Error`. +- `zcash_client_backend::data_api::testing::MockBlockSource` has been moved to + `zcash_client_backend::data_api::chain::testing::MockBlockSource` module. +- `zcash_client_backend::data_api::chain::{validate_chain, scan_cached_blocks}` + have altered parameters and result types. The latter have been modified to + return`zcash_client_backend::data_api::chain::error::Error` instead of + abstract error types. This new error type now wraps the errors of the block + source and wallet database to which these methods delegate IO operations + directly, which simplifies error handling in cases where callback functions + are involved. +- `zcash_client_backend::data_api::error::ChainInvalid` has been moved to + `zcash_client_backend::data_api::chain::error`. +- The error type of `zcash_client_backend::data_api::wallet::decrypt_and_store_transaction` + has been changed; the error type produced by the provided `WalletWrite` instance is + returned directly. + +### Deprecated +- `zcash_client_backend::data_api::wallet::create_spend_to_address` has been + deprecated. Use `zcash_client_backend::data_api::wallet::spend` instead. If + you wish to continue using `create_spend_to_address`, note that the arguments + to the function has been modified to take a unified spending key instead of a + Sapling extended spending key, and now also requires a `min_confirmations` + argument that the caller can provide to specify a minimum number of + confirmations required for notes being selected. A minimum of 10 + confirmations is recommended. ### Removed - `zcash_client_backend::data_api`: diff --git a/zcash_client_backend/Cargo.toml b/zcash_client_backend/Cargo.toml index 3314c9498..5b0310e51 100644 --- a/zcash_client_backend/Cargo.toml +++ b/zcash_client_backend/Cargo.toml @@ -76,6 +76,7 @@ tonic-build = "0.8" which = "4" [dev-dependencies] +assert_matches = "1.5" gumdrop = "0.8" hex = "0.4" jubjub = "0.9" diff --git a/zcash_client_backend/src/address.rs b/zcash_client_backend/src/address.rs index b83cea085..a5f4a36d6 100644 --- a/zcash_client_backend/src/address.rs +++ b/zcash_client_backend/src/address.rs @@ -36,7 +36,7 @@ impl AddressMetadata { } /// A Unified Address. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct UnifiedAddress { orchard: Option, sapling: Option, diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 28a4032ee..6379b186b 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -12,7 +12,10 @@ use zcash_primitives::{ memo::{Memo, MemoBytes}, merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::{Node, Nullifier, PaymentAddress}, - transaction::{components::Amount, Transaction, TxId}, + transaction::{ + components::{amount::Amount, OutPoint}, + Transaction, TxId, + }, zip32::{AccountId, ExtendedFullViewingKey}, }; @@ -20,13 +23,9 @@ use crate::{ address::{AddressMetadata, UnifiedAddress}, decrypt::DecryptedOutput, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, - proto::compact_formats::CompactBlock, wallet::{SpendableNote, WalletTransparentOutput, WalletTx}, }; -#[cfg(feature = "transparent-inputs")] -use zcash_primitives::transaction::components::transparent::OutPoint; - pub mod chain; pub mod error; pub mod wallet; @@ -44,14 +43,14 @@ pub trait WalletRead { /// /// For example, this might be a database identifier type /// or a UUID. - type NoteRef: Copy + Debug; + type NoteRef: Copy + Debug + Eq + Ord; /// Backend-specific transaction identifier. /// /// For example, this might be a database identifier type /// or a TxId if the backend is able to support that type /// directly. - type TxRef: Copy + Debug; + type TxRef: Copy + Debug + Eq + Ord; /// Returns the minimum and maximum block heights for stored blocks. /// @@ -188,7 +187,8 @@ pub trait WalletRead { &self, account: AccountId, anchor_height: BlockHeight, - ) -> Result, Self::Error>; + exclude: &[Self::NoteRef], + ) -> Result>, Self::Error>; /// Returns a list of spendable Sapling notes sufficient to cover the specified /// target value, if possible. @@ -197,7 +197,8 @@ pub trait WalletRead { account: AccountId, target_value: Amount, anchor_height: BlockHeight, - ) -> Result, Self::Error>; + exclude: &[Self::NoteRef], + ) -> Result>, Self::Error>; /// Returns the set of all transparent receivers associated with the given account. /// @@ -215,6 +216,7 @@ pub trait WalletRead { &self, address: &TransparentAddress, max_height: BlockHeight, + exclude: &[OutPoint], ) -> Result, Self::Error>; /// Returns a mapping from transparent receiver to not-yet-shielded UTXO balance, @@ -227,7 +229,9 @@ pub trait WalletRead { } /// The subset of information that is relevant to this wallet that has been -/// decrypted and extracted from a [CompactBlock]. +/// decrypted and extracted from a [`CompactBlock`]. +/// +/// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock pub struct PrunedBlock<'a> { pub block_height: BlockHeight, pub block_hash: BlockHash, @@ -268,6 +272,7 @@ pub enum PoolType { Transparent, /// The Sapling value pool Sapling, + // TODO: Orchard } /// A type that represents the recipient of a transaction output; a recipient address (and, for @@ -386,23 +391,6 @@ pub trait WalletWrite: WalletRead { ) -> Result; } -/// This trait provides sequential access to raw blockchain data via a callback-oriented -/// API. -pub trait BlockSource { - type Error; - - /// Scan the specified `limit` number of blocks from the blockchain, starting at - /// `from_height`, applying the provided callback to each block. - fn with_blocks( - &self, - from_height: BlockHeight, - limit: Option, - with_row: F, - ) -> Result<(), Self::Error> - where - F: FnMut(CompactBlock) -> Result<(), Self::Error>; -} - #[cfg(feature = "test-dependencies")] pub mod testing { use secrecy::{ExposeSecret, SecretVec}; @@ -415,46 +403,27 @@ pub mod testing { memo::Memo, merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::{Node, Nullifier}, - transaction::{components::Amount, Transaction, TxId}, + transaction::{ + components::{Amount, OutPoint}, + Transaction, TxId, + }, zip32::{AccountId, ExtendedFullViewingKey}, }; use crate::{ address::{AddressMetadata, UnifiedAddress}, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, - proto::compact_formats::CompactBlock, wallet::{SpendableNote, WalletTransparentOutput}, }; - use super::{ - error::Error, BlockSource, DecryptedTransaction, PrunedBlock, SentTransaction, WalletRead, - WalletWrite, - }; - - pub struct MockBlockSource {} - - impl BlockSource for MockBlockSource { - type Error = Error; - - fn with_blocks( - &self, - _from_height: BlockHeight, - _limit: Option, - _with_row: F, - ) -> Result<(), Self::Error> - where - F: FnMut(CompactBlock) -> Result<(), Self::Error>, - { - Ok(()) - } - } + use super::{DecryptedTransaction, PrunedBlock, SentTransaction, WalletRead, WalletWrite}; pub struct MockWalletDb { pub network: Network, } impl WalletRead for MockWalletDb { - type Error = Error; + type Error = (); type NoteRef = u32; type TxRef = TxId; @@ -514,7 +483,7 @@ pub mod testing { } fn get_transaction(&self, _id_tx: Self::TxRef) -> Result { - Err(Error::ScanRequired) // wrong error but we'll fix it later. + Err(()) } fn get_commitment_tree( @@ -544,7 +513,8 @@ pub mod testing { &self, _account: AccountId, _anchor_height: BlockHeight, - ) -> Result, Self::Error> { + _exclude: &[Self::NoteRef], + ) -> Result>, Self::Error> { Ok(Vec::new()) } @@ -553,7 +523,8 @@ pub mod testing { _account: AccountId, _target_value: Amount, _anchor_height: BlockHeight, - ) -> Result, Self::Error> { + _exclude: &[Self::NoteRef], + ) -> Result>, Self::Error> { Ok(Vec::new()) } @@ -568,6 +539,7 @@ pub mod testing { &self, _address: &TransparentAddress, _anchor_height: BlockHeight, + _exclude: &[OutPoint], ) -> Result, Self::Error> { Ok(Vec::new()) } @@ -591,7 +563,7 @@ pub mod testing { let account = AccountId::from(0); UnifiedSpendingKey::from_seed(&self.network, seed.expose_secret(), account) .map(|k| (account, k)) - .map_err(|_| Error::KeyDerivationError(account)) + .map_err(|_| ()) } fn get_next_available_address( diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index c70dba4f9..401b2e9f9 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -12,42 +12,47 @@ //! //! use zcash_client_backend::{ //! data_api::{ -//! BlockSource, WalletRead, WalletWrite, +//! WalletRead, WalletWrite, //! chain::{ -//! validate_chain, +//! BlockSource, +//! error::Error, //! scan_cached_blocks, +//! validate_chain, +//! testing as chain_testing, //! }, -//! error::Error, //! testing, //! }, //! }; //! +//! # use std::convert::Infallible; +//! //! # fn main() { //! # test(); //! # } //! # -//! # fn test() -> Result<(), Error> { +//! # fn test() -> Result<(), Error<(), Infallible, u32>> { //! let network = Network::TestNetwork; -//! let db_cache = testing::MockBlockSource {}; +//! let block_source = chain_testing::MockBlockSource; //! let mut db_data = testing::MockWalletDb { //! network: Network::TestNetwork //! }; //! -//! // 1) Download new CompactBlocks into db_cache. +//! // 1) Download new CompactBlocks into block_source. //! //! // 2) Run the chain validator on the received blocks. //! // //! // Given that we assume the server always gives us correct-at-the-time blocks, any //! // errors are in the blocks we have previously cached or scanned. -//! if let Err(e) = validate_chain(&network, &db_cache, db_data.get_max_height_hash()?) { +//! let max_height_hash = db_data.get_max_height_hash().map_err(Error::Wallet)?; +//! if let Err(e) = validate_chain(&network, &block_source, max_height_hash) { //! match e { -//! Error::InvalidChain(lower_bound, _) => { +//! Error::Chain(e) => { //! // a) Pick a height to rewind to. //! // //! // This might be informed by some external chain reorg information, or //! // heuristics such as the platform, available bandwidth, size of recent //! // CompactBlocks, etc. -//! let rewind_height = lower_bound - 10; +//! let rewind_height = e.at_height() - 10; //! //! // b) Rewind scanned block information. //! db_data.rewind_to_height(rewind_height); @@ -74,12 +79,12 @@ //! // At this point, the cache and scanned data are locally consistent (though not //! // necessarily consistent with the latest chain tip - this would be discovered the //! // next time this codepath is executed after new blocks are received). -//! scan_cached_blocks(&network, &db_cache, &mut db_data, None) +//! scan_cached_blocks(&network, &block_source, &mut db_data, None) //! # } //! # } //! ``` -use std::fmt::Debug; +use std::convert::Infallible; use zcash_primitives::{ block::BlockHash, @@ -90,56 +95,75 @@ use zcash_primitives::{ }; use crate::{ - data_api::{ - error::{ChainInvalid, Error}, - BlockSource, PrunedBlock, WalletWrite, - }, + data_api::{PrunedBlock, WalletWrite}, proto::compact_formats::CompactBlock, scan::BatchRunner, wallet::WalletTx, welding_rig::{add_block_to_runner, scan_block_with_runner}, }; +pub mod error; +use error::{ChainError, Error}; + +/// This trait provides sequential access to raw blockchain data via a callback-oriented +/// API. +pub trait BlockSource { + type Error; + + /// Scan the specified `limit` number of blocks from the blockchain, starting at + /// `from_height`, applying the provided callback to each block. + /// + /// * `WalletErrT`: the types of errors produced by the wallet operations performed + /// as part of processing each row. + /// * `NoteRefT`: the type of note identifiers in the wallet data store, for use in + /// reporting errors related to specific notes. + fn with_blocks( + &self, + from_height: BlockHeight, + limit: Option, + with_row: F, + ) -> Result<(), error::Error> + where + F: FnMut(CompactBlock) -> Result<(), error::Error>; +} + /// Checks that the scanned blocks in the data database, when combined with the recent -/// `CompactBlock`s in the cache database, form a valid chain. +/// `CompactBlock`s in the block_source database, form a valid chain. /// /// This function is built on the core assumption that the information provided in the -/// cache database is more likely to be accurate than the previously-scanned information. +/// block source is more likely to be accurate than the previously-scanned information. /// This follows from the design (and trust) assumption that the `lightwalletd` server /// provides accurate block information as of the time it was requested. /// /// Arguments: /// - `parameters` Network parameters -/// - `cache` Source of compact blocks +/// - `block_source` Source of compact blocks /// - `from_tip` Height & hash of last validated block; if no validation has previously /// been performed, this will begin scanning from `sapling_activation_height - 1` /// /// Returns: /// - `Ok(())` if the combined chain is valid. -/// - `Err(ErrorKind::InvalidChain(upper_bound, cause))` if the combined chain is invalid. -/// `upper_bound` is the height of the highest invalid block (on the assumption that the -/// highest block in the cache database is correct). +/// - `Err(Error::Chain(cause))` if the combined chain is invalid. /// - `Err(e)` if there was an error during validation unrelated to chain validity. /// /// This function does not mutate either of the databases. -pub fn validate_chain( - parameters: &P, - cache: &C, +pub fn validate_chain( + parameters: &ParamsT, + block_source: &BlockSourceT, validate_from: Option<(BlockHeight, BlockHash)>, -) -> Result<(), E> +) -> Result<(), Error> where - E: From>, - P: consensus::Parameters, - C: BlockSource, + ParamsT: consensus::Parameters, + BlockSourceT: BlockSource, { let sapling_activation_height = parameters .activation_height(NetworkUpgrade::Sapling) - .ok_or(Error::SaplingNotActive)?; + .expect("Sapling activation height must be known."); - // The cache will contain blocks above the `validate_from` height. Validate from that maximum - // height up to the chain tip, returning the hash of the block found in the cache at the - // `validate_from` height, which can then be used to verify chain integrity by comparing - // against the `validate_from` hash. + // The block source will contain blocks above the `validate_from` height. Validate from that + // maximum height up to the chain tip, returning the hash of the block found in the block + // source at the `validate_from` height, which can then be used to verify chain integrity by + // comparing against the `validate_from` hash. let from_height = validate_from .map(|(height, _)| height) .unwrap_or(sapling_activation_height - 1); @@ -147,74 +171,74 @@ where let mut prev_height = from_height; let mut prev_hash: Option = validate_from.map(|(_, hash)| hash); - cache.with_blocks(from_height, None, move |block| { + block_source.with_blocks::<_, Infallible, Infallible>(from_height, None, move |block| { let current_height = block.height(); let result = if current_height != prev_height + 1 { - Err(ChainInvalid::block_height_discontinuity( - prev_height + 1, - current_height, - )) + Err(ChainError::block_height_discontinuity(prev_height + 1, current_height).into()) } else { match prev_hash { None => Ok(()), Some(h) if h == block.prev_hash() => Ok(()), - Some(_) => Err(ChainInvalid::prev_hash_mismatch(current_height)), + Some(_) => Err(ChainError::prev_hash_mismatch(current_height).into()), } }; prev_height = current_height; prev_hash = Some(block.hash()); - result.map_err(E::from) + result }) } -#[allow(clippy::needless_doctest_main)] -/// Scans at most `limit` new blocks added to the cache for any transactions received by -/// the tracked accounts. +/// Scans at most `limit` new blocks added to the block source for any transactions received by the +/// tracked accounts. /// -/// This function will return without error after scanning at most `limit` new blocks, to -/// enable the caller to update their UI with scanning progress. Repeatedly calling this -/// function will process sequential ranges of blocks, and is equivalent to calling -/// `scan_cached_blocks` and passing `None` for the optional `limit` value. +/// This function will return without error after scanning at most `limit` new blocks, to enable +/// the caller to update their UI with scanning progress. Repeatedly calling this function will +/// process sequential ranges of blocks, and is equivalent to calling `scan_cached_blocks` and +/// passing `None` for the optional `limit` value. /// -/// This function pays attention only to cached blocks with heights greater than the -/// highest scanned block in `data`. Cached blocks with lower heights are not verified -/// against previously-scanned blocks. In particular, this function **assumes** that the -/// caller is handling rollbacks. +/// This function pays attention only to cached blocks with heights greater than the highest +/// scanned block in `data`. Cached blocks with lower heights are not verified against +/// previously-scanned blocks. In particular, this function **assumes** that the caller is handling +/// rollbacks. /// -/// For brand-new light client databases, this function starts scanning from the Sapling -/// activation height. This height can be fast-forwarded to a more recent block by -/// initializing the client database with a starting block (for example, calling -/// `init_blocks_table` before this function if using `zcash_client_sqlite`). +/// For brand-new light client databases, this function starts scanning from the Sapling activation +/// height. This height can be fast-forwarded to a more recent block by initializing the client +/// database with a starting block (for example, calling `init_blocks_table` before this function +/// if using `zcash_client_sqlite`). /// -/// Scanned blocks are required to be height-sequential. If a block is missing from the -/// cache, an error will be returned with kind [`ChainInvalid::BlockHeightDiscontinuity`]. -pub fn scan_cached_blocks( - params: &P, - cache: &C, - data: &mut D, +/// Scanned blocks are required to be height-sequential. If a block is missing from the block +/// source, an error will be returned with cause [`error::Cause::BlockHeightDiscontinuity`]. +#[allow(clippy::type_complexity)] +pub fn scan_cached_blocks( + params: &ParamsT, + block_source: &BlockSourceT, + data_db: &mut DbT, limit: Option, -) -> Result<(), E> +) -> Result<(), Error> where - P: consensus::Parameters + Send + 'static, - C: BlockSource, - D: WalletWrite, - N: Copy + Debug, - E: From>, + ParamsT: consensus::Parameters + Send + 'static, + BlockSourceT: BlockSource, + DbT: WalletWrite, { let sapling_activation_height = params .activation_height(NetworkUpgrade::Sapling) - .ok_or(Error::SaplingNotActive)?; + .expect("Sapling activation height is known."); // Recall where we synced up to previously. // If we have never synced, use sapling activation height to select all cached CompactBlocks. - let mut last_height = data.block_height_extrema().map(|opt| { - opt.map(|(_, max)| max) - .unwrap_or(sapling_activation_height - 1) - })?; + let mut last_height = data_db + .block_height_extrema() + .map(|opt| { + opt.map(|(_, max)| max) + .unwrap_or(sapling_activation_height - 1) + }) + .map_err(Error::Wallet)?; // Fetch the UnifiedFullViewingKeys we are tracking - let ufvks = data.get_unified_full_viewing_keys()?; + let ufvks = data_db + .get_unified_full_viewing_keys() + .map_err(Error::Wallet)?; // TODO: Change `scan_block` to also scan Orchard. // https://github.com/zcash/librustzcash/issues/403 let dfvks: Vec<_> = ufvks @@ -223,15 +247,16 @@ where .collect(); // Get the most recent CommitmentTree - let mut tree = data + let mut tree = data_db .get_commitment_tree(last_height) - .map(|t| t.unwrap_or_else(CommitmentTree::empty))?; + .map(|t| t.unwrap_or_else(CommitmentTree::empty)) + .map_err(Error::Wallet)?; // Get most recent incremental witnesses for the notes we are tracking - let mut witnesses = data.get_witnesses(last_height)?; + let mut witnesses = data_db.get_witnesses(last_height).map_err(Error::Wallet)?; // Get the nullifiers for the notes we are tracking - let mut nullifiers = data.get_nullifiers()?; + let mut nullifiers = data_db.get_nullifiers().map_err(Error::Wallet)?; let mut batch_runner = BatchRunner::<_, _, _, ()>::new( 100, @@ -246,91 +271,133 @@ where .map(|(tag, ivk)| (tag, PreparedIncomingViewingKey::new(&ivk))), ); - cache.with_blocks(last_height, limit, |block: CompactBlock| { - add_block_to_runner(params, block, &mut batch_runner); - Ok(()) - })?; + block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>( + last_height, + limit, + |block: CompactBlock| { + add_block_to_runner(params, block, &mut batch_runner); + Ok(()) + }, + )?; batch_runner.flush(); - cache.with_blocks(last_height, limit, |block: CompactBlock| { - let current_height = block.height(); + block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>( + last_height, + limit, + |block: CompactBlock| { + let current_height = block.height(); - // Scanned blocks MUST be height-sequential. - if current_height != (last_height + 1) { - return Err( - ChainInvalid::block_height_discontinuity(last_height + 1, current_height).into(), - ); - } - - let block_hash = BlockHash::from_slice(&block.hash); - let block_time = block.time; - - let txs: Vec> = { - let mut witness_refs: Vec<_> = witnesses.iter_mut().map(|w| &mut w.1).collect(); - - scan_block_with_runner( - params, - block, - &dfvks, - &nullifiers, - &mut tree, - &mut witness_refs[..], - Some(&mut batch_runner), - ) - }; - - // Enforce that all roots match. This is slow, so only include in debug builds. - #[cfg(debug_assertions)] - { - let cur_root = tree.root(); - for row in &witnesses { - if row.1.root() != cur_root { - return Err(Error::InvalidWitnessAnchor(row.0, current_height).into()); - } + // Scanned blocks MUST be height-sequential. + if current_height != (last_height + 1) { + return Err(ChainError::block_height_discontinuity( + last_height + 1, + current_height, + ) + .into()); } - for tx in &txs { - for output in tx.shielded_outputs.iter() { - if output.witness.root() != cur_root { - return Err(Error::InvalidNewWitnessAnchor( - output.index, - tx.txid, - current_height, - output.witness.root(), - ) - .into()); + + let block_hash = BlockHash::from_slice(&block.hash); + let block_time = block.time; + + let txs: Vec> = { + let mut witness_refs: Vec<_> = witnesses.iter_mut().map(|w| &mut w.1).collect(); + + scan_block_with_runner( + params, + block, + &dfvks, + &nullifiers, + &mut tree, + &mut witness_refs[..], + Some(&mut batch_runner), + ) + }; + + // Enforce that all roots match. This is slow, so only include in debug builds. + #[cfg(debug_assertions)] + { + let cur_root = tree.root(); + for row in &witnesses { + if row.1.root() != cur_root { + return Err( + ChainError::invalid_witness_anchor(current_height, row.0).into() + ); + } + } + for tx in &txs { + for output in tx.shielded_outputs.iter() { + if output.witness.root() != cur_root { + return Err(ChainError::invalid_new_witness_anchor( + current_height, + tx.txid, + output.index, + output.witness.root(), + ) + .into()); + } } } } - } - let new_witnesses = data.advance_by_block( - &(PrunedBlock { - block_height: current_height, - block_hash, - block_time, - commitment_tree: &tree, - transactions: &txs, - }), - &witnesses, - )?; + let new_witnesses = data_db + .advance_by_block( + &(PrunedBlock { + block_height: current_height, + block_hash, + block_time, + commitment_tree: &tree, + transactions: &txs, + }), + &witnesses, + ) + .map_err(Error::Wallet)?; - let spent_nf: Vec = txs - .iter() - .flat_map(|tx| tx.shielded_spends.iter().map(|spend| spend.nf)) - .collect(); - nullifiers.retain(|(_, nf)| !spent_nf.contains(nf)); - nullifiers.extend( - txs.iter() - .flat_map(|tx| tx.shielded_outputs.iter().map(|out| (out.account, out.nf))), - ); + let spent_nf: Vec = txs + .iter() + .flat_map(|tx| tx.shielded_spends.iter().map(|spend| spend.nf)) + .collect(); + nullifiers.retain(|(_, nf)| !spent_nf.contains(nf)); + nullifiers.extend( + txs.iter() + .flat_map(|tx| tx.shielded_outputs.iter().map(|out| (out.account, out.nf))), + ); - witnesses.extend(new_witnesses); + witnesses.extend(new_witnesses); - last_height = current_height; + last_height = current_height; - Ok(()) - })?; + Ok(()) + }, + )?; Ok(()) } + +#[cfg(feature = "test-dependencies")] +pub mod testing { + use std::convert::Infallible; + use zcash_primitives::consensus::BlockHeight; + + use crate::proto::compact_formats::CompactBlock; + + use super::{error::Error, BlockSource}; + + pub struct MockBlockSource; + + impl BlockSource for MockBlockSource { + type Error = Infallible; + + fn with_blocks( + &self, + _from_height: BlockHeight, + _limit: Option, + _with_row: F, + ) -> Result<(), Error> + where + F: FnMut(CompactBlock) -> Result<(), Error>, + { + Ok(()) + } + } +} diff --git a/zcash_client_backend/src/data_api/chain/error.rs b/zcash_client_backend/src/data_api/chain/error.rs new file mode 100644 index 000000000..b35334c6a --- /dev/null +++ b/zcash_client_backend/src/data_api/chain/error.rs @@ -0,0 +1,190 @@ +//! Types for chain scanning error handling. + +use std::error; +use std::fmt::{self, Debug, Display}; + +use zcash_primitives::{consensus::BlockHeight, sapling, transaction::TxId}; + +/// The underlying cause of a [`ChainError`]. +#[derive(Copy, Clone, Debug)] +pub enum Cause { + /// The hash of the parent block given by a proposed new chain tip does not match the hash of + /// the current chain tip. + PrevHashMismatch, + + /// The block height field of the proposed new chain tip is not equal to the height of the + /// previous chain tip + 1. This variant stores a copy of the incorrect height value for + /// reporting purposes. + BlockHeightDiscontinuity(BlockHeight), + + /// The root of an output's witness tree in a newly arrived transaction does not correspond to + /// root of the stored commitment tree at the recorded height. + /// + /// This error is currently only produced when performing the slow checks that are enabled by + /// compiling with `-C debug-assertions`. + InvalidNewWitnessAnchor { + /// The id of the transaction containing the mismatched witness. + txid: TxId, + /// The index of the shielded output within the transaction where the witness root does not + /// match. + index: usize, + /// The root of the witness that failed to match the root of the current note commitment + /// tree. + node: sapling::Node, + }, + + /// The root of an output's witness tree in a previously stored transaction does not correspond + /// to root of the current commitment tree. + /// + /// This error is currently only produced when performing the slow checks that are enabled by + /// compiling with `-C debug-assertions`. + InvalidWitnessAnchor(NoteRef), +} + +/// Errors that may occur in chain scanning or validation. +#[derive(Copy, Clone, Debug)] +pub struct ChainError { + at_height: BlockHeight, + cause: Cause, +} + +impl fmt::Display for ChainError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match &self.cause { + Cause::PrevHashMismatch => write!( + f, + "The parent hash of proposed block does not correspond to the block hash at height {}.", + self.at_height + ), + Cause::BlockHeightDiscontinuity(h) => { + write!(f, "Block height discontinuity at height {}; next height is : {}", self.at_height, h) + } + Cause::InvalidNewWitnessAnchor { txid, index, node } => write!( + f, + "New witness for output {} in tx {} at height {} has incorrect anchor: {:?}", + index, txid, self.at_height, node, + ), + Cause::InvalidWitnessAnchor(id_note) => { + write!(f, "Witness for note {} has incorrect anchor for height {}", id_note, self.at_height) + } + } + } +} + +impl ChainError { + /// Constructs an error that indicates block hashes failed to chain. + /// + /// * `at_height` the height of the block whose parent hash does not match the hash of the + /// previous block + pub fn prev_hash_mismatch(at_height: BlockHeight) -> Self { + ChainError { + at_height, + cause: Cause::PrevHashMismatch, + } + } + + /// Constructs an error that indicates a gap in block heights. + /// + /// * `at_height` the height of the block being added to the chain. + /// * `prev_chain_tip` the height of the previous chain tip. + pub fn block_height_discontinuity(at_height: BlockHeight, prev_chain_tip: BlockHeight) -> Self { + ChainError { + at_height, + cause: Cause::BlockHeightDiscontinuity(prev_chain_tip), + } + } + + /// Constructs an error that indicates a mismatch between an updated note's witness and the + /// root of the current note commitment tree. + pub fn invalid_witness_anchor(at_height: BlockHeight, note_ref: NoteRef) -> Self { + ChainError { + at_height, + cause: Cause::InvalidWitnessAnchor(note_ref), + } + } + + /// Constructs an error that indicates a mismatch between a new note's witness and the root of + /// the current note commitment tree. + pub fn invalid_new_witness_anchor( + at_height: BlockHeight, + txid: TxId, + index: usize, + node: sapling::Node, + ) -> Self { + ChainError { + at_height, + cause: Cause::InvalidNewWitnessAnchor { txid, index, node }, + } + } + + /// Returns the block height at which this error was discovered. + pub fn at_height(&self) -> BlockHeight { + self.at_height + } + + /// Returns the cause of this error. + pub fn cause(&self) -> &Cause { + &self.cause + } +} + +/// Errors related to chain validation and scanning. +#[derive(Debug)] +pub enum Error { + /// An error that was produced by wallet operations in the course of scanning the chain. + Wallet(WalletError), + + /// An error that was produced by the underlying block data store in the process of validation + /// or scanning. + BlockSource(BlockSourceError), + + /// A block that was received violated rules related to chain continuity or contained note + /// commitments that could not be reconciled with the note commitment tree(s) maintained by the + /// wallet. + Chain(ChainError), +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match &self { + Error::Wallet(e) => { + write!( + f, + "The underlying datasource produced the following error: {}", + e + ) + } + Error::BlockSource(e) => { + write!( + f, + "The underlying block store produced the following error: {}", + e + ) + } + Error::Chain(err) => { + write!(f, "{}", err) + } + } + } +} + +impl error::Error for Error +where + WE: Debug + Display + error::Error + 'static, + BE: Debug + Display + error::Error + 'static, + N: Debug + Display, +{ + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match &self { + Error::Wallet(e) => Some(e), + Error::BlockSource(e) => Some(e), + _ => None, + } + } +} + +impl From> for Error { + fn from(e: ChainError) -> Self { + Error::Chain(e) + } +} diff --git a/zcash_client_backend/src/data_api/error.rs b/zcash_client_backend/src/data_api/error.rs index efb5da5c3..0614a612d 100644 --- a/zcash_client_backend/src/data_api/error.rs +++ b/zcash_client_backend/src/data_api/error.rs @@ -1,36 +1,32 @@ //! Types for wallet error handling. use std::error; -use std::fmt; -use zcash_address::unified::Typecode; +use std::fmt::{self, Debug, Display}; use zcash_primitives::{ - consensus::BlockHeight, - sapling::Node, transaction::{ builder, - components::amount::{Amount, BalanceError}, - TxId, + components::{ + amount::{Amount, BalanceError}, + sapling, transparent, + }, }, zip32::AccountId, }; +use crate::data_api::wallet::input_selection::InputSelectorError; + #[cfg(feature = "transparent-inputs")] use zcash_primitives::{legacy::TransparentAddress, zip32::DiversifierIndex}; +/// Errors that can occur as a consequence of wallet operations. #[derive(Debug)] -pub enum ChainInvalid { - /// The hash of the parent block given by a proposed new chain tip does - /// not match the hash of the current chain tip. - PrevHashMismatch, +pub enum Error { + /// An error occurred retrieving data from the underlying data source + DataSource(DataSourceError), - /// The block height field of the proposed new chain tip is not equal - /// to the height of the previous chain tip + 1. This variant stores - /// a copy of the incorrect height value for reporting purposes. - BlockHeightDiscontinuity(BlockHeight), -} + /// An error in note selection + NoteSelection(SelectionError), -#[derive(Debug)] -pub enum Error { /// No account could be found corresponding to a provided spending key. KeyNotRecognized, @@ -41,60 +37,21 @@ pub enum Error { BalanceError(BalanceError), /// Unable to create a new spend because the wallet balance is not sufficient. - /// The first argument is the amount available, the second is the amount needed - /// to construct a valid transaction. - InsufficientBalance(Amount, Amount), - - /// Chain validation detected an error in the block at the specified block height. - InvalidChain(BlockHeight, ChainInvalid), - - /// A provided extsk is not associated with the specified account. - InvalidExtSk(AccountId), - - /// The root of an output's witness tree in a newly arrived transaction does - /// not correspond to root of the stored commitment tree at the recorded height. - /// - /// The `usize` member of this struct is the index of the shielded output within - /// the transaction where the witness root does not match. - InvalidNewWitnessAnchor(usize, TxId, BlockHeight, Node), - - /// The root of an output's witness tree in a previously stored transaction - /// does not correspond to root of the current commitment tree. - InvalidWitnessAnchor(NoteId, BlockHeight), - - /// No key of the given type was associated with the specified account. - KeyNotFound(AccountId, Typecode), + InsufficientFunds { available: Amount, required: Amount }, /// The wallet must first perform a scan of the blockchain before other /// operations can be performed. ScanRequired, /// An error occurred building a new transaction. - Builder(builder::Error), - - /// An error occurred decoding a protobuf message. - Protobuf(prost::DecodeError), - - /// The wallet attempted a sapling-only operation at a block - /// height when Sapling was not yet active. - SaplingNotActive, + Builder(builder::Error), /// It is forbidden to provide a memo when constructing a transparent output. MemoForbidden, - /// An error occurred deriving a spending key from a seed and an account - /// identifier. - KeyDerivationError(AccountId), - /// A note being spent does not correspond to either the internal or external /// full viewing key for an account. - // TODO: Return the note id for the note that caused the failure - NoteMismatch, - - /// An error indicating that a call was attempted to a method providing - /// support - #[cfg(not(feature = "transparent-inputs"))] - TransparentInputsNotSupported, + NoteMismatch(NoteRef), #[cfg(feature = "transparent-inputs")] AddressNotRecognized(TransparentAddress), @@ -103,95 +60,119 @@ pub enum Error { ChildIndexOutOfRange(DiversifierIndex), } -impl ChainInvalid { - pub fn prev_hash_mismatch(at_height: BlockHeight) -> Error { - Error::InvalidChain(at_height, ChainInvalid::PrevHashMismatch) - } - - pub fn block_height_discontinuity(at_height: BlockHeight, found: BlockHeight) -> Error { - Error::InvalidChain(at_height, ChainInvalid::BlockHeightDiscontinuity(found)) - } -} - -impl fmt::Display for Error { +impl fmt::Display for Error +where + DE: fmt::Display, + SE: fmt::Display, + FE: fmt::Display, + N: fmt::Display, +{ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match &self { + Error::DataSource(e) => { + write!( + f, + "The underlying datasource produced the following error: {}", + e + ) + } + Error::NoteSelection(e) => { + write!(f, "Note selection encountered the following error: {}", e) + } Error::KeyNotRecognized => { - write!(f, "Wallet does not contain an account corresponding to the provided spending key") + write!( + f, + "Wallet does not contain an account corresponding to the provided spending key" + ) } Error::AccountNotFound(account) => { write!(f, "Wallet does not contain account {}", u32::from(*account)) } Error::BalanceError(e) => write!( f, - "The value lies outside the valid range of Zcash amounts: {:?}.", e + "The value lies outside the valid range of Zcash amounts: {:?}.", + e ), - Error::InsufficientBalance(have, need) => write!( + Error::InsufficientFunds { available, required } => write!( f, "Insufficient balance (have {}, need {} including fee)", - i64::from(*have), i64::from(*need) + i64::from(*available), + i64::from(*required) ), - Error::InvalidChain(upper_bound, cause) => { - write!(f, "Invalid chain (upper bound: {}): {:?}", u32::from(*upper_bound), cause) - } - Error::InvalidExtSk(account) => { - write!(f, "Incorrect ExtendedSpendingKey for account {}", u32::from(*account)) - } - Error::InvalidNewWitnessAnchor(output, txid, last_height, anchor) => write!( - f, - "New witness for output {} in tx {} has incorrect anchor after scanning block {}: {:?}", - output, txid, last_height, anchor, - ), - Error::InvalidWitnessAnchor(id_note, last_height) => write!( - f, - "Witness for note {} has incorrect anchor after scanning block {}", - id_note, last_height - ), - Error::KeyNotFound(account, typecode) => { - write!(f, "No {:?} key was available for account {}", typecode, u32::from(*account)) - } Error::ScanRequired => write!(f, "Must scan blocks first"), - Error::Builder(e) => write!(f, "{:?}", e), - Error::Protobuf(e) => write!(f, "{}", e), - Error::SaplingNotActive => write!(f, "Could not determine Sapling upgrade activation height."), + Error::Builder(e) => write!(f, "An error occurred building the transaction: {}", e), Error::MemoForbidden => write!(f, "It is not possible to send a memo to a transparent address."), - Error::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {:?}", acct_id), - Error::NoteMismatch => write!(f, "A note being spent does not correspond to either the internal or external full viewing key for the provided spending key."), + Error::NoteMismatch(n) => write!(f, "A note being spent ({}) does not correspond to either the internal or external full viewing key for the provided spending key.", n), - #[cfg(not(feature = "transparent-inputs"))] - Error::TransparentInputsNotSupported => { - write!(f, "This wallet does not support spending or manipulating transparent UTXOs.") - } #[cfg(feature = "transparent-inputs")] Error::AddressNotRecognized(_) => { write!(f, "The specified transparent address was not recognized as belonging to the wallet.") } #[cfg(feature = "transparent-inputs")] Error::ChildIndexOutOfRange(i) => { - write!(f, "The diversifier index {:?} is out of range for transparent addresses.", i) + write!( + f, + "The diversifier index {:?} is out of range for transparent addresses.", + i + ) } } } } -impl error::Error for Error { +impl error::Error for Error +where + DE: Debug + Display + error::Error + 'static, + SE: Debug + Display + error::Error + 'static, + FE: Debug + Display + 'static, + N: Debug + Display, +{ fn source(&self) -> Option<&(dyn error::Error + 'static)> { match &self { + Error::DataSource(e) => Some(e), + Error::NoteSelection(e) => Some(e), Error::Builder(e) => Some(e), - Error::Protobuf(e) => Some(e), _ => None, } } } -impl From for Error { - fn from(e: builder::Error) -> Self { +impl From> for Error { + fn from(e: builder::Error) -> Self { Error::Builder(e) } } -impl From for Error { - fn from(e: prost::DecodeError) -> Self { - Error::Protobuf(e) +impl From for Error { + fn from(e: BalanceError) -> Self { + Error::BalanceError(e) + } +} + +impl From> for Error { + fn from(e: InputSelectorError) -> Self { + match e { + InputSelectorError::DataSource(e) => Error::DataSource(e), + InputSelectorError::Selection(e) => Error::NoteSelection(e), + InputSelectorError::InsufficientFunds { + available, + required, + } => Error::InsufficientFunds { + available, + required, + }, + } + } +} + +impl From for Error { + fn from(e: sapling::builder::Error) -> Self { + Error::Builder(builder::Error::SaplingBuild(e)) + } +} + +impl From for Error { + fn from(e: transparent::builder::Error) -> Self { + Error::Builder(builder::Error::TransparentBuild(e)) } } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index bff79260e..a280bbc7f 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -1,14 +1,17 @@ use std::fmt::Debug; + use zcash_primitives::{ consensus::{self, NetworkUpgrade}, memo::MemoBytes, - sapling::prover::TxProver, + merkle_tree::MerklePath, + sapling::{self, prover::TxProver as SaplingProver}, transaction::{ builder::Builder, - components::amount::{Amount, BalanceError, DEFAULT_FEE}, + components::amount::{Amount, BalanceError}, + fees::{fixed, FeeRule}, Transaction, }, - zip32::Scope, + zip32::{sapling::DiversifiableFullViewingKey, sapling::ExtendedSpendingKey, Scope}, }; use crate::{ @@ -18,29 +21,31 @@ use crate::{ SentTransactionOutput, WalletWrite, }, decrypt_transaction, - fees::{BasicFixedFeeChangeStrategy, ChangeError, ChangeStrategy, ChangeValue}, + fees::{self, ChangeValue, DustOutputPolicy}, keys::UnifiedSpendingKey, - wallet::OvkPolicy, - zip321::{Payment, TransactionRequest}, + wallet::{OvkPolicy, SpendableNote}, + zip321::{self, Payment}, }; +pub mod input_selection; +use input_selection::{GreedyInputSelector, GreedyInputSelectorError, InputSelector}; + #[cfg(feature = "transparent-inputs")] -use { - crate::wallet::WalletTransparentOutput, - zcash_primitives::{legacy::TransparentAddress, sapling::keys::OutgoingViewingKey}, +use zcash_primitives::{ + legacy::TransparentAddress, sapling::keys::OutgoingViewingKey, + transaction::components::amount::NonNegativeAmount, }; /// Scans a [`Transaction`] for any information that can be decrypted by the accounts in /// the wallet, and saves it to the wallet. -pub fn decrypt_and_store_transaction( - params: &P, - data: &mut D, +pub fn decrypt_and_store_transaction( + params: &ParamsT, + data: &mut DbT, tx: &Transaction, -) -> Result<(), E> +) -> Result<(), DbT::Error> where - E: From>, - P: consensus::Parameters, - D: WalletWrite, + ParamsT: consensus::Parameters, + DbT: WalletWrite, { // Fetch the UnifiedFullViewingKeys we are tracking let ufvks = data.get_unified_full_viewing_keys()?; @@ -53,7 +58,7 @@ where .block_height_extrema()? .map(|(_, max_height)| max_height + 1)) .or_else(|| params.activation_height(NetworkUpgrade::Sapling)) - .ok_or(Error::SaplingNotActive)?; + .expect("Sapling activation height must be known."); data.store_decrypted_tx(&DecryptedTransaction { tx, @@ -93,7 +98,7 @@ where /// Parameters: /// * `wallet_db`: A read/write reference to the wallet database /// * `params`: Consensus parameters -/// * `prover`: The TxProver to use in constructing the shielded transaction. +/// * `prover`: The [`sapling::TxProver`] to use in constructing the shielded transaction. /// * `usk`: The unified spending key that controls the funds that will be spent /// in the resulting transaction. This procedure will return an error if the /// USK does not correspond to an account known to the wallet. @@ -125,11 +130,18 @@ where /// wallet::OvkPolicy, /// }; /// +/// # use std::convert::Infallible; +/// # use zcash_primitives::transaction::components::amount::BalanceError; +/// # use zcash_client_backend::{ +/// # data_api::wallet::input_selection::GreedyInputSelectorError, +/// # }; +/// # /// # fn main() { /// # test(); /// # } /// # -/// # fn test() -> Result> { +/// # #[allow(deprecated)] +/// # fn test() -> Result, Infallible, u32>> { /// /// let tx_prover = match LocalTxProver::with_default_location() { /// Some(tx_prover) => tx_prover, @@ -161,25 +173,35 @@ where /// # } /// # } /// ``` +/// [`sapling::TxProver`]: zcash_primitives::sapling::prover::TxProver #[allow(clippy::too_many_arguments)] -pub fn create_spend_to_address( - wallet_db: &mut D, - params: &P, - prover: impl TxProver, +#[allow(clippy::type_complexity)] +#[deprecated(note = "Use `spend` instead.")] +pub fn create_spend_to_address( + wallet_db: &mut DbT, + params: &ParamsT, + prover: impl SaplingProver, usk: &UnifiedSpendingKey, to: &RecipientAddress, amount: Amount, memo: Option, ovk_policy: OvkPolicy, min_confirmations: u32, -) -> Result +) -> Result< + DbT::TxRef, + Error< + DbT::Error, + GreedyInputSelectorError, + core::convert::Infallible, + DbT::NoteRef, + >, +> where - E: From>, - P: consensus::Parameters + Clone, - R: Copy + Debug, - D: WalletWrite, + ParamsT: consensus::Parameters + Clone, + DbT: WalletWrite, + DbT::NoteRef: Copy + Eq + Ord, { - let req = TransactionRequest::new(vec![Payment { + let req = zip321::TransactionRequest::new(vec![Payment { recipient_address: to.clone(), amount, memo, @@ -191,12 +213,14 @@ where "It should not be possible for this to violate ZIP 321 request construction invariants.", ); + let change_strategy = fees::fixed::SingleOutputChangeStrategy::new(fixed::FeeRule::standard()); spend( wallet_db, params, prover, + &GreedyInputSelector::::new(change_strategy, DustOutputPolicy::default()), usk, - &req, + req, ovk_policy, min_confirmations, ) @@ -235,7 +259,10 @@ where /// Parameters: /// * `wallet_db`: A read/write reference to the wallet database /// * `params`: Consensus parameters -/// * `prover`: The TxProver to use in constructing the shielded transaction. +/// * `prover`: The [`sapling::TxProver`] to use in constructing the shielded transaction. +/// * `input_selector`: The [`InputSelector`] that will be used to select available +/// inputs from the wallet database, choose change amounts and compute required +/// transaction fees. /// * `usk`: The unified spending key that controls the funds that will be spent /// in the resulting transaction. This procedure will return an error if the /// USK does not correspond to an account known to the wallet. @@ -246,24 +273,33 @@ where /// * `min_confirmations`: The minimum number of confirmations that a previously /// received note must have in the blockchain in order to be considered for being /// spent. A value of 10 confirmations is recommended. +/// +/// [`sapling::TxProver`]: zcash_primitives::sapling::prover::TxProver #[allow(clippy::too_many_arguments)] -pub fn spend( - wallet_db: &mut D, - params: &P, - prover: impl TxProver, +#[allow(clippy::type_complexity)] +pub fn spend( + wallet_db: &mut DbT, + params: &ParamsT, + prover: impl SaplingProver, + input_selector: &InputsT, usk: &UnifiedSpendingKey, - request: &TransactionRequest, + request: zip321::TransactionRequest, ovk_policy: OvkPolicy, min_confirmations: u32, -) -> Result +) -> Result< + DbT::TxRef, + Error::Error, DbT::NoteRef>, +> where - E: From>, - P: consensus::Parameters + Clone, - R: Copy + Debug, - D: WalletWrite, + DbT: WalletWrite, + DbT::TxRef: Copy + Debug, + DbT::NoteRef: Copy + Eq + Ord, + ParamsT: consensus::Parameters + Clone, + InputsT: InputSelector, { let account = wallet_db - .get_account_for_ufvk(&usk.to_unified_full_viewing_key())? + .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) + .map_err(Error::DataSource)? .ok_or(Error::KeyNotRecognized)?; let dfvk = usk.sapling().to_diversifiable_full_viewing_key(); @@ -278,135 +314,74 @@ where // Target the next block, assuming we are up-to-date. let (target_height, anchor_height) = wallet_db .get_target_and_anchor_heights(min_confirmations) - .and_then(|x| x.ok_or_else(|| Error::ScanRequired.into()))?; + .map_err(Error::DataSource) + .and_then(|x| x.ok_or(Error::ScanRequired))?; - let value = request - .payments() - .iter() - .map(|p| p.amount) - .sum::>() - .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?; - let target_value = (value + DEFAULT_FEE) - .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?; - let spendable_notes = - wallet_db.select_spendable_sapling_notes(account, target_value, anchor_height)?; + let proposal = input_selector.propose_transaction( + params, + wallet_db, + account, + anchor_height, + target_height, + request, + )?; - // Confirm we were able to select sufficient value - let selected_value = spendable_notes - .iter() - .map(|n| n.note_value) - .sum::>() - .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?; - if selected_value < target_value { - return Err(E::from(Error::InsufficientBalance( - selected_value, - target_value, - ))); - } - - // Create the transaction + // Create the transaction. The type of the proposal ensures that there + // are no possible transparent inputs, so we ignore those let mut builder = Builder::new(params.clone(), target_height); - for selected in spendable_notes { - let merkle_path = selected.witness.path().expect("the tree is not empty"); + for selected in proposal.sapling_inputs() { + let (note, key, merkle_path) = select_key_for_note(selected, usk.sapling(), &dfvk) + .ok_or(Error::NoteMismatch(selected.note_id))?; - // Attempt to reconstruct the note being spent using both the internal and external dfvks - // corresponding to the unified spending key, checking against the witness we are using - // to spend the note that we've used the correct key. - let (note, key) = { - let external_note = dfvk - .diversified_address(selected.diversifier) - .and_then(|addr| addr.create_note(selected.note_value.into(), selected.rseed)); - let internal_note = dfvk - .diversified_change_address(selected.diversifier) - .and_then(|addr| addr.create_note(selected.note_value.into(), selected.rseed)); - - let expected_root = selected.witness.root(); - external_note - .filter(|n| expected_root == merkle_path.root(n.commitment())) - .map(|n| (n, usk.sapling().clone())) - .or_else(|| { - internal_note - .filter(|n| expected_root == merkle_path.root(n.commitment())) - .map(|n| (n, usk.sapling().derive_internal())) - }) - .ok_or_else(|| E::from(Error::NoteMismatch)) - }?; - - builder - .add_sapling_spend(key, selected.diversifier, note, merkle_path) - .map_err(Error::Builder)?; + builder.add_sapling_spend(key, selected.diversifier, note, merkle_path)?; } - for payment in request.payments() { + for payment in proposal.transaction_request().payments() { match &payment.recipient_address { - RecipientAddress::Unified(ua) => builder - .add_sapling_output( + RecipientAddress::Unified(ua) => { + builder.add_sapling_output( ovk, ua.sapling() .expect("TODO: Add Orchard support to builder") .clone(), payment.amount, payment.memo.clone().unwrap_or_else(MemoBytes::empty), - ) - .map_err(Error::Builder), - RecipientAddress::Shielded(to) => builder - .add_sapling_output( + )?; + } + RecipientAddress::Shielded(to) => { + builder.add_sapling_output( ovk, to.clone(), payment.amount, payment.memo.clone().unwrap_or_else(MemoBytes::empty), - ) - .map_err(Error::Builder), + )?; + } RecipientAddress::Transparent(to) => { if payment.memo.is_some() { - Err(Error::MemoForbidden) + return Err(Error::MemoForbidden); } else { - builder - .add_transparent_output(to, payment.amount) - .map_err(Error::Builder) + builder.add_transparent_output(to, payment.amount)?; } } - }? - } - - let fee_strategy = BasicFixedFeeChangeStrategy::new(DEFAULT_FEE); - let balance = fee_strategy - .compute_balance( - params, - target_height, - builder.transparent_inputs(), - builder.transparent_outputs(), - builder.sapling_inputs(), - builder.sapling_outputs(), - ) - .map_err(|e| match e { - ChangeError::InsufficientFunds { - available, - required, - } => Error::InsufficientBalance(available, required), - ChangeError::StrategyError(e) => Error::BalanceError(e), - })?; - - for change_value in balance.proposed_change() { - match change_value { - ChangeValue::Sapling(amount) => { - builder - .add_sapling_output( - Some(dfvk.to_ovk(Scope::Internal)), - dfvk.change_address().1, - *amount, - MemoBytes::empty(), - ) - .map_err(Error::Builder)?; - } } } - let (tx, tx_metadata) = builder - .build(&prover, &fee_strategy.fee_rule()) - .map_err(Error::Builder)?; + for change_value in proposal.balance().proposed_change() { + match change_value { + ChangeValue::Sapling(amount) => { + builder.add_sapling_output( + Some(dfvk.to_ovk(Scope::Internal)), + dfvk.change_address().1, + *amount, + MemoBytes::empty(), + )?; + } + } + } - let sent_outputs = request.payments().iter().enumerate().map(|(i, payment)| { + let (tx, tx_metadata) = builder.build(&prover, proposal.fee_rule())?; + + let sent_outputs = proposal.transaction_request().payments().iter().enumerate().map(|(i, payment)| { let (output_index, recipient) = match &payment.recipient_address { // Sapling outputs are shuffled, so we need to look up where the output ended up. RecipientAddress::Shielded(addr) => { @@ -443,15 +418,17 @@ where } }).collect(); - wallet_db.store_sent_tx(&SentTransaction { - tx: &tx, - created: time::OffsetDateTime::now_utc(), - account, - outputs: sent_outputs, - fee_amount: balance.fee_required(), - #[cfg(feature = "transparent-inputs")] - utxos_spent: vec![], - }) + wallet_db + .store_sent_tx(&SentTransaction { + tx: &tx, + created: time::OffsetDateTime::now_utc(), + account, + outputs: sent_outputs, + fee_amount: proposal.balance().fee_required(), + #[cfg(feature = "transparent-inputs")] + utxos_spent: vec![], + }) + .map_err(Error::DataSource) } /// Constructs a transaction that consumes available transparent UTXOs belonging to @@ -465,13 +442,18 @@ where /// Parameters: /// * `wallet_db`: A read/write reference to the wallet database /// * `params`: Consensus parameters -/// * `prover`: The TxProver to use in constructing the shielded transaction. +/// * `prover`: The [`sapling::TxProver`] to use in constructing the shielded transaction. +/// * `input_selector`: The [`InputSelector`] to for note selection and change and fee +/// determination /// * `usk`: The unified spending key that will be used to detect and spend transparent UTXOs, /// and that will provide the shielded address to which funds will be sent. Funds will be /// shielded to the internal (change) address associated with the most preferred shielded /// receiver corresponding to this account, or if no shielded receiver can be used for this /// account, this function will return an error. This procedure will return an error if the /// USK does not correspond to an account known to the wallet. +/// * `from_addrs`: The list of transparent addresses that will be used to filter transaparent +/// UTXOs received by the wallet. Only UTXOs received at one of the provided addresses will +/// be selected to be shielded. /// * `memo`: A memo to be included in the output to the (internal) recipient. /// This can be used to take notes about auto-shielding operations internal /// to the wallet that the wallet can use to improve how it represents those @@ -479,25 +461,33 @@ where /// * `min_confirmations`: The minimum number of confirmations that a previously /// received UTXO must have in the blockchain in order to be considered for being /// spent. +/// +/// [`sapling::TxProver`]: zcash_primitives::sapling::prover::TxProver #[cfg(feature = "transparent-inputs")] #[allow(clippy::too_many_arguments)] -pub fn shield_transparent_funds( - wallet_db: &mut D, - params: &P, - prover: impl TxProver, +#[allow(clippy::type_complexity)] +pub fn shield_transparent_funds( + wallet_db: &mut DbT, + params: &ParamsT, + prover: impl SaplingProver, + input_selector: &InputsT, usk: &UnifiedSpendingKey, from_addrs: &[TransparentAddress], memo: &MemoBytes, min_confirmations: u32, -) -> Result +) -> Result< + DbT::TxRef, + Error::Error, DbT::NoteRef>, +> where - E: From>, - P: consensus::Parameters, - R: Copy + Debug, - D: WalletWrite, + ParamsT: consensus::Parameters, + DbT: WalletWrite, + DbT::NoteRef: Copy + Eq + Ord, + InputsT: InputSelector, { let account = wallet_db - .get_account_for_ufvk(&usk.to_unified_full_viewing_key())? + .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) + .map_err(Error::DataSource)? .ok_or(Error::KeyNotRecognized)?; let shielding_address = usk @@ -507,29 +497,39 @@ where .1; let (target_height, latest_anchor) = wallet_db .get_target_and_anchor_heights(min_confirmations) - .and_then(|x| x.ok_or_else(|| Error::ScanRequired.into()))?; + .map_err(Error::DataSource) + .and_then(|x| x.ok_or(Error::ScanRequired))?; + let dfvk = usk.sapling().to_diversifiable_full_viewing_key(); let account_pubkey = usk.transparent().to_account_pubkey(); let ovk = OutgoingViewingKey(account_pubkey.internal_ovk().as_bytes()); // get UTXOs from DB for each address - let mut utxos: Vec = vec![]; - for from_addr in from_addrs { - let mut outputs = wallet_db.get_unspent_transparent_outputs(from_addr, latest_anchor)?; - utxos.append(&mut outputs); - } + let proposal = input_selector.propose_shielding( + params, + wallet_db, + NonNegativeAmount::from_u64(100000).unwrap(), + from_addrs, + latest_anchor, + target_height, + )?; - let _total_amount = utxos - .iter() - .map(|utxo| utxo.txout().value) - .sum::>() - .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?; - - let addr_metadata = wallet_db.get_transparent_receivers(account)?; + let known_addrs = wallet_db + .get_transparent_receivers(account) + .map_err(Error::DataSource)?; let mut builder = Builder::new(params.clone(), target_height); - for utxo in &utxos { - let diversifier_index = addr_metadata + let mut utxos = vec![]; + for selected in proposal.sapling_inputs() { + let (note, key, merkle_path) = select_key_for_note(selected, usk.sapling(), &dfvk) + .ok_or(Error::NoteMismatch(selected.note_id))?; + + builder.add_sapling_spend(key, selected.diversifier, note, merkle_path)?; + } + + for utxo in proposal.transparent_inputs() { + utxos.push(utxo); + let diversifier_index = known_addrs .get(utxo.recipient_address()) .ok_or_else(|| Error::AddressNotRecognized(*utxo.recipient_address()))? .diversifier_index(); @@ -542,64 +542,85 @@ where .derive_external_secret_key(child_index) .unwrap(); - builder - .add_transparent_input(secret_key, utxo.outpoint().clone(), utxo.txout().clone()) - .map_err(Error::Builder)?; + builder.add_transparent_input(secret_key, utxo.outpoint().clone(), utxo.txout().clone())?; } - // Compute the balance of the transaction. We have only added inputs, so the total change - // amount required will be the total of the UTXOs minus fees. - let fee_strategy = BasicFixedFeeChangeStrategy::new(DEFAULT_FEE); - let balance = fee_strategy - .compute_balance( - params, - target_height, - builder.transparent_inputs(), - builder.transparent_outputs(), - builder.sapling_inputs(), - builder.sapling_outputs(), - ) - .map_err(|e| match e { - ChangeError::InsufficientFunds { - available, - required, - } => Error::InsufficientBalance(available, required), - ChangeError::StrategyError(e) => Error::BalanceError(e), - })?; - - let fee = balance.fee_required(); - let mut total_out = Amount::zero(); - for change_value in balance.proposed_change() { - total_out = (total_out + change_value.value()) - .ok_or(Error::BalanceError(BalanceError::Overflow))?; + for change_value in proposal.balance().proposed_change() { match change_value { ChangeValue::Sapling(amount) => { - builder - .add_sapling_output(Some(ovk), shielding_address.clone(), *amount, memo.clone()) - .map_err(Error::Builder)?; + builder.add_sapling_output( + Some(ovk), + shielding_address.clone(), + *amount, + memo.clone(), + )?; } } } // The transaction build process will check that the inputs and outputs balance - let (tx, tx_metadata) = builder - .build(&prover, &fee_strategy.fee_rule()) - .map_err(Error::Builder)?; - let output_index = tx_metadata.output_index(0).expect( - "No sapling note was created in autoshielding transaction. This is a programming error.", - ); + let (tx, tx_metadata) = builder.build(&prover, proposal.fee_rule())?; - wallet_db.store_sent_tx(&SentTransaction { - tx: &tx, - created: time::OffsetDateTime::now_utc(), - account, - outputs: vec![SentTransactionOutput { - output_index, - recipient: Recipient::InternalAccount(account, PoolType::Sapling), - value: total_out, - memo: Some(memo.clone()), - }], - fee_amount: fee, - utxos_spent: utxos.iter().map(|utxo| utxo.outpoint().clone()).collect(), - }) + wallet_db + .store_sent_tx(&SentTransaction { + tx: &tx, + created: time::OffsetDateTime::now_utc(), + account, + // TODO: After Orchard is implemented, this will need to change to correctly + // determine the Sapling output indices; `enumerate` will no longer suffice + outputs: proposal + .balance() + .proposed_change() + .iter() + .enumerate() + .map(|(idx, change_value)| match change_value { + ChangeValue::Sapling(value) => { + let output_index = tx_metadata.output_index(idx).expect( + "Missing Sapling output of autoshielding transaction. This is a programming error.", + ); + SentTransactionOutput { + output_index, + recipient: Recipient::InternalAccount(account, PoolType::Sapling), + value: *value, + memo: Some(memo.clone()), + } + } + }) + .collect(), + fee_amount: proposal.balance().fee_required(), + utxos_spent: utxos.iter().map(|utxo| utxo.outpoint().clone()).collect(), + }) + .map_err(Error::DataSource) +} + +fn select_key_for_note( + selected: &SpendableNote, + extsk: &ExtendedSpendingKey, + dfvk: &DiversifiableFullViewingKey, +) -> Option<( + sapling::Note, + ExtendedSpendingKey, + MerklePath, +)> { + let merkle_path = selected.witness.path().expect("the tree is not empty"); + + // Attempt to reconstruct the note being spent using both the internal and external dfvks + // corresponding to the unified spending key, checking against the witness we are using + // to spend the note that we've used the correct key. + let external_note = dfvk + .diversified_address(selected.diversifier) + .and_then(|addr| addr.create_note(selected.note_value.into(), selected.rseed)); + let internal_note = dfvk + .diversified_change_address(selected.diversifier) + .and_then(|addr| addr.create_note(selected.note_value.into(), selected.rseed)); + + let expected_root = selected.witness.root(); + external_note + .filter(|n| expected_root == merkle_path.root(n.commitment())) + .map(|n| (n, extsk.clone(), merkle_path.clone())) + .or_else(|| { + internal_note + .filter(|n| expected_root == merkle_path.root(n.commitment())) + .map(|n| (n, extsk.derive_internal(), merkle_path)) + }) } diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs new file mode 100644 index 000000000..d95fed8c9 --- /dev/null +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -0,0 +1,452 @@ +//! Types related to the process of selecting inputs to be spent given a transaction request. + +use core::marker::PhantomData; +use std::collections::BTreeSet; +use std::fmt; + +use zcash_primitives::{ + consensus::{self, BlockHeight}, + legacy::TransparentAddress, + transaction::{ + components::{ + amount::{Amount, BalanceError, NonNegativeAmount}, + sapling::fees as sapling, + OutPoint, TxOut, + }, + fees::FeeRule, + }, + zip32::AccountId, +}; + +use crate::{ + address::{RecipientAddress, UnifiedAddress}, + data_api::WalletRead, + fees::{ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance}, + wallet::{SpendableNote, WalletTransparentOutput}, + zip321::TransactionRequest, +}; + +/// The type of errors that may be produced in input selection. +pub enum InputSelectorError { + /// An error occurred accessing the underlying data store. + DataSource(DbErrT), + /// An error occurred specific to the provided input selector's selection rules. + Selection(SelectorErrT), + /// Insufficient funds were available to satisfy the payment request that inputs were being + /// selected to attempt to satisfy. + InsufficientFunds { available: Amount, required: Amount }, +} + +impl fmt::Display for InputSelectorError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match &self { + InputSelectorError::DataSource(e) => { + write!( + f, + "The underlying datasource produced the following error: {}", + e + ) + } + InputSelectorError::Selection(e) => { + write!(f, "Note selection encountered the following error: {}", e) + } + InputSelectorError::InsufficientFunds { + available, + required, + } => write!( + f, + "Insufficient balance (have {}, need {} including fee)", + i64::from(*available), + i64::from(*required) + ), + } + } +} + +/// A data structure that describes the inputs to be consumed and outputs to +/// be produced in a proposed transaction. +pub struct Proposal { + transaction_request: TransactionRequest, + transparent_inputs: Vec, + sapling_inputs: Vec>, + balance: TransactionBalance, + fee_rule: FeeRuleT, +} + +impl Proposal { + /// Returns the transaction request that describes the payments to be made. + pub fn transaction_request(&self) -> &TransactionRequest { + &self.transaction_request + } + /// Returns the transparent inputs that have been selected to fund the transaction. + pub fn transparent_inputs(&self) -> &[TransparentInput] { + &self.transparent_inputs + } + /// Returns the Sapling inputs that have been selected to fund the transaction. + pub fn sapling_inputs(&self) -> &[SpendableNote] { + &self.sapling_inputs + } + /// Returns the change outputs to be added to the transaction and the fee to be paid. + pub fn balance(&self) -> &TransactionBalance { + &self.balance + } + /// Returns the fee rule to be used by the transaction builder. + pub fn fee_rule(&self) -> &FeeRuleT { + &self.fee_rule + } +} + +/// A strategy for selecting transaction inputs and proposing transaction outputs. +/// +/// Proposals should include only economically useful inputs, as determined by `Self::FeeRule`; +/// that is, do not return inputs that cause fees to increase by an amount greater than the value +/// of the input. +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 the fee rule that this input selector uses when computing fees. + type FeeRule: FeeRule; + + /// Performs input selection and returns a proposal for transaction construction including + /// change and fee outputs. + /// + /// Implementations of this method should return inputs sufficient to satisfy the given + /// transaction request using a best-effort strategy to preserve user privacy, as follows: + /// * If it is possible to satisfy the specified transaction request by creating + /// a fully-shielded transaction without requiring value to cross pool boundaries, + /// return the inputs necessary to construct such a transaction; otherwise + /// * If it is possible to satisfy the transaction request by creating a fully-shielded + /// transaction with some amounts crossing between shielded pools, return the inputs + /// necessary. + /// + /// 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, + account: AccountId, + anchor_height: BlockHeight, + target_height: BlockHeight, + transaction_request: TransactionRequest, + ) -> Result< + Proposal< + Self::FeeRule, + std::convert::Infallible, + <::DataSource as WalletRead>::NoteRef, + >, + InputSelectorError<<::DataSource as WalletRead>::Error, Self::Error>, + > + where + ParamsT: consensus::Parameters; + + /// Performs input selection and returns a proposal for the construction of a shielding + /// transaction. + /// + /// Implementations should return the maximum possible number of economically useful inputs + /// required to supply at least the requested value, choosing only inputs received at the + /// 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, + shielding_threshold: NonNegativeAmount, + source_addrs: &[TransparentAddress], + confirmed_height: BlockHeight, + target_height: BlockHeight, + ) -> Result< + Proposal< + Self::FeeRule, + WalletTransparentOutput, + <::DataSource as WalletRead>::NoteRef, + >, + InputSelectorError<<::DataSource as WalletRead>::Error, Self::Error>, + > + where + ParamsT: consensus::Parameters; +} + +/// Errors that can occur as a consequence of greedy input selection. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GreedyInputSelectorError { + /// An intermediate value overflowed or underflowed the valid monetary range. + Balance(BalanceError), + /// A unified address did not contain a supported receiver. + UnsupportedAddress(Box), + /// An error was encountered in change selection. + Change(ChangeError), +} + +impl + From> + for InputSelectorError> +{ + fn from(err: GreedyInputSelectorError) -> Self { + InputSelectorError::Selection(err) + } +} + +impl From> + for InputSelectorError> +{ + fn from(err: ChangeError) -> Self { + InputSelectorError::Selection(GreedyInputSelectorError::Change(err)) + } +} + +impl From + for InputSelectorError> +{ + fn from(err: BalanceError) -> Self { + InputSelectorError::Selection(GreedyInputSelectorError::Balance(err)) + } +} + +pub(crate) struct SaplingPayment(Amount); + +#[cfg(test)] +impl SaplingPayment { + pub(crate) fn new(amount: Amount) -> Self { + SaplingPayment(amount) + } +} + +impl sapling::OutputView for SaplingPayment { + fn value(&self) -> Amount { + self.0 + } +} + +/// 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. +pub struct GreedyInputSelector { + change_strategy: ChangeT, + dust_output_policy: DustOutputPolicy, + _ds_type: PhantomData, +} + +impl GreedyInputSelector { + /// Constructs a new greedy input selector that uses the provided change strategy to determine + /// change values and fee amounts. + pub fn new(change_strategy: ChangeT, dust_output_policy: DustOutputPolicy) -> Self { + GreedyInputSelector { + change_strategy, + dust_output_policy, + _ds_type: PhantomData, + } + } +} + +impl InputSelector for GreedyInputSelector +where + DbT: WalletRead, + ChangeT: ChangeStrategy, + ChangeT::FeeRule: Clone, +{ + type Error = GreedyInputSelectorError; + type DataSource = DbT; + type FeeRule = ChangeT::FeeRule; + + #[allow(clippy::type_complexity)] + fn propose_transaction( + &self, + params: &ParamsT, + wallet_db: &Self::DataSource, + account: AccountId, + anchor_height: BlockHeight, + target_height: BlockHeight, + transaction_request: TransactionRequest, + ) -> Result< + Proposal, + InputSelectorError, + > + where + ParamsT: consensus::Parameters, + { + let mut transparent_outputs = vec![]; + let mut sapling_outputs = vec![]; + let mut output_total = Amount::zero(); + for payment in transaction_request.payments() { + output_total = (output_total + payment.amount).ok_or(BalanceError::Overflow)?; + + let mut push_transparent = |taddr: TransparentAddress| { + transparent_outputs.push(TxOut { + value: payment.amount, + script_pubkey: taddr.script(), + }); + }; + let mut push_sapling = || { + sapling_outputs.push(SaplingPayment(payment.amount)); + }; + + match &payment.recipient_address { + RecipientAddress::Transparent(addr) => { + push_transparent(*addr); + } + RecipientAddress::Shielded(_) => { + push_sapling(); + } + RecipientAddress::Unified(addr) => { + if addr.sapling().is_some() { + push_sapling(); + } else if let Some(addr) = addr.transparent() { + push_transparent(*addr); + } else { + return Err(InputSelectorError::Selection( + GreedyInputSelectorError::UnsupportedAddress(Box::new(addr.clone())), + )); + } + } + } + } + + let mut sapling_inputs: Vec> = vec![]; + let mut prior_available = Amount::zero(); + let mut amount_required = Amount::zero(); + let mut exclude: Vec = vec![]; + // This loop is guaranteed to terminate because on each iteration we check that the amount + // of funds selected is strictly increasing. The loop will either return a successful + // result or the wallet will eventually run out of funds to select. + loop { + let balance = self.change_strategy.compute_balance( + params, + target_height, + &Vec::::new(), + &transparent_outputs, + &sapling_inputs, + &sapling_outputs, + &self.dust_output_policy, + ); + + match balance { + Ok(balance) => { + return Ok(Proposal { + transaction_request, + transparent_inputs: vec![], + sapling_inputs, + balance, + fee_rule: (*self.change_strategy.fee_rule()).clone(), + }); + } + Err(ChangeError::DustInputs { mut sapling, .. }) => { + exclude.append(&mut sapling); + } + Err(ChangeError::InsufficientFunds { required, .. }) => { + amount_required = required; + } + Err(other) => return Err(other.into()), + } + + sapling_inputs = wallet_db + .select_spendable_sapling_notes(account, amount_required, anchor_height, &exclude) + .map_err(InputSelectorError::DataSource)?; + + let new_available = sapling_inputs + .iter() + .map(|n| n.note_value) + .sum::>() + .ok_or(BalanceError::Overflow)?; + + if new_available <= prior_available { + return Err(InputSelectorError::InsufficientFunds { + required: amount_required, + available: new_available, + }); + } else { + // If the set of selected inputs has changed after selection, we will loop again + // and see whether we now have enough funds. + prior_available = new_available; + } + } + } + + #[allow(clippy::type_complexity)] + fn propose_shielding( + &self, + params: &ParamsT, + wallet_db: &Self::DataSource, + shielding_threshold: NonNegativeAmount, + source_addrs: &[TransparentAddress], + confirmed_height: BlockHeight, + target_height: BlockHeight, + ) -> Result< + Proposal, + InputSelectorError, + > + where + ParamsT: consensus::Parameters, + { + let mut transparent_inputs: Vec = source_addrs + .iter() + .map(|taddr| wallet_db.get_unspent_transparent_outputs(taddr, confirmed_height, &[])) + .collect::>, _>>() + .map_err(InputSelectorError::DataSource)? + .into_iter() + .flat_map(|v| v.into_iter()) + .collect(); + + let trial_balance = self.change_strategy.compute_balance( + params, + target_height, + &transparent_inputs, + &Vec::::new(), + &Vec::>::new(), + &Vec::::new(), + &self.dust_output_policy, + ); + + let balance = match trial_balance { + Ok(balance) => balance, + Err(ChangeError::DustInputs { transparent, .. }) => { + let exclusions: BTreeSet = transparent.into_iter().collect(); + transparent_inputs = transparent_inputs + .into_iter() + .filter(|i| !exclusions.contains(i.outpoint())) + .collect(); + + self.change_strategy.compute_balance( + params, + target_height, + &transparent_inputs, + &Vec::::new(), + &Vec::>::new(), + &Vec::::new(), + &self.dust_output_policy, + )? + } + Err(other) => { + return Err(other.into()); + } + }; + + if balance.total() >= shielding_threshold.into() { + Ok(Proposal { + transaction_request: TransactionRequest::empty(), + transparent_inputs, + sapling_inputs: vec![], + balance, + fee_rule: (*self.change_strategy.fee_rule()).clone(), + }) + } else { + Err(InputSelectorError::InsufficientFunds { + available: balance.total(), + required: shielding_threshold.into(), + }) + } + } +} diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 9b13d2e24..aaaca4ed7 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -5,12 +5,17 @@ use zcash_primitives::{ amount::{Amount, BalanceError}, sapling::fees as sapling, transparent::fees as transparent, + OutPoint, }, - fees::{FeeRule, FixedFeeRule}, + fees::FeeRule, }, }; +pub mod fixed; +pub mod zip317; + /// A proposed change amount and output pool. +#[derive(Clone, Debug, PartialEq, Eq)] pub enum ChangeValue { Sapling(Amount), } @@ -26,18 +31,26 @@ impl ChangeValue { /// The amount of change and fees required to make a transaction's inputs and /// outputs balance under a specific fee rule, as computed by a particular /// [`ChangeStrategy`] that is aware of that rule. +#[derive(Clone, Debug, PartialEq, Eq)] pub struct TransactionBalance { proposed_change: Vec, fee_required: Amount, + total: Amount, } impl TransactionBalance { /// Constructs a new balance from its constituent parts. - pub fn new(proposed_change: Vec, fee_required: Amount) -> Self { - TransactionBalance { - proposed_change, - fee_required, - } + pub fn new(proposed_change: Vec, fee_required: Amount) -> Option { + proposed_change + .iter() + .map(|v| v.value()) + .chain(Some(fee_required)) + .sum::>() + .map(|total| TransactionBalance { + proposed_change, + fee_required, + total, + }) } /// The change values proposed by the [`ChangeStrategy`] that computed this balance. @@ -50,14 +63,95 @@ impl TransactionBalance { pub fn fee_required(&self) -> Amount { self.fee_required } + + /// Returns the sum of the proposed change outputs and the required fee. + pub fn total(&self) -> Amount { + self.total + } } /// Errors that can occur in computing suggested change and/or fees. -pub enum ChangeError { - InsufficientFunds { available: Amount, required: Amount }, +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ChangeError { + /// Insufficient inputs were provided to change selection to fund the + /// required outputs and fees. + InsufficientFunds { + /// The total of the inputs provided to change selection + available: Amount, + /// The total amount of input value required to fund the requested outputs, + /// including the required fees. + required: Amount, + }, + /// Some of the inputs provided to the transaction were determined to currently have no + /// economic value (i.e. their inclusion in a transaction causes fees to rise in an amount + /// greater than their value.) + DustInputs { + /// The outpoints corresponding to transparent inputs having no current economic value. + transparent: Vec, + /// The identifiers for Sapling inputs having not current economic value + sapling: Vec, + }, + /// An error occurred that was specific to the change selection strategy in use. StrategyError(E), } +impl From for ChangeError { + fn from(err: BalanceError) -> ChangeError { + ChangeError::StrategyError(err) + } +} + +/// An enumeration of actions to tak when a transaction would potentially create dust +/// outputs (outputs that are likely to be without economic value due to fee rules.) +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum DustAction { + /// Do not allow creation of dust outputs; instead, require that additional inputs be provided. + Reject, + /// Explicitly allow the creation of dust change amounts greater than the specified value. + AllowDustChange, + /// Allow dust amounts to be added to the transaction fee + AddDustToFee, +} + +/// A policy describing how a [`ChangeStrategy`] should treat potentially dust-valued change +/// outputs (outputs that are likely to be without economic value due to fee rules.) +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct DustOutputPolicy { + action: DustAction, + dust_threshold: Option, +} + +impl DustOutputPolicy { + /// Constructs a new dust output policy. + /// + /// A dust policy created with `None` as the dust threshold will delegate determination + /// of the dust threshold to the change strategy that is evaluating the strategy; this + /// recommended, but an explicit value (including zero) may be provided to explicitly + /// override the determination of the change strategy. + pub fn new(action: DustAction, dust_threshold: Option) -> Self { + Self { + action, + dust_threshold, + } + } + + /// Returns the action to take in the event that a dust change amount would be produced + pub fn action(&self) -> DustAction { + self.action + } + /// Returns a value that will be used to override the dust determination logic of the + /// change policy, if any. + pub fn dust_threshold(&self) -> Option { + self.dust_threshold + } +} + +impl Default for DustOutputPolicy { + fn default() -> Self { + DustOutputPolicy::new(DustAction::Reject, None) + } +} + /// A trait that represents the ability to compute the suggested change and fees that must be paid /// by a transaction having a specified set of inputs and outputs. pub trait ChangeStrategy { @@ -66,7 +160,7 @@ pub trait ChangeStrategy { /// Returns the fee rule that this change strategy will respect when performing /// balance computations. - fn fee_rule(&self) -> Self::FeeRule; + fn fee_rule(&self) -> &Self::FeeRule; /// Computes the totals of inputs, suggested change amounts, and fees given the /// provided inputs and outputs being used to construct a transaction. @@ -75,89 +169,52 @@ pub trait ChangeStrategy { /// change outputs recommended by this operation. If insufficient funds are available to /// supply the requested outputs and required fees, implementations should return /// [`ChangeError::InsufficientFunds`]. - fn compute_balance( + #[allow(clippy::too_many_arguments)] + fn compute_balance( &self, params: &P, target_height: BlockHeight, transparent_inputs: &[impl transparent::InputView], transparent_outputs: &[impl transparent::OutputView], - sapling_inputs: &[impl sapling::InputView], + sapling_inputs: &[impl sapling::InputView], sapling_outputs: &[impl sapling::OutputView], - ) -> Result>; + dust_output_policy: &DustOutputPolicy, + ) -> Result>; } -/// A change strategy that uses a fixed fee amount and proposes change as a single output -/// to the most current supported pool. -pub struct BasicFixedFeeChangeStrategy { - fixed_fee: Amount, -} +#[cfg(test)] +pub(crate) mod tests { + use zcash_primitives::transaction::components::{ + amount::Amount, + sapling::fees as sapling, + transparent::{fees as transparent, OutPoint, TxOut}, + }; -impl BasicFixedFeeChangeStrategy { - // Constructs a new [`BasicFixedFeeChangeStrategy`] with the specified fixed fee - // amount. - pub fn new(fixed_fee: Amount) -> Self { - Self { fixed_fee } - } -} - -impl ChangeStrategy for BasicFixedFeeChangeStrategy { - type FeeRule = FixedFeeRule; - type Error = BalanceError; - - fn fee_rule(&self) -> Self::FeeRule { - FixedFeeRule::new(self.fixed_fee) + pub(crate) struct TestTransparentInput { + pub outpoint: OutPoint, + pub coin: TxOut, } - fn compute_balance( - &self, - _params: &P, - _target_height: BlockHeight, - transparent_inputs: &[impl transparent::InputView], - transparent_outputs: &[impl transparent::OutputView], - sapling_inputs: &[impl sapling::InputView], - sapling_outputs: &[impl sapling::OutputView], - ) -> Result> { - let overflow = || ChangeError::StrategyError(BalanceError::Overflow); - let underflow = || ChangeError::StrategyError(BalanceError::Underflow); + impl transparent::InputView for TestTransparentInput { + fn outpoint(&self) -> &OutPoint { + &self.outpoint + } + fn coin(&self) -> &TxOut { + &self.coin + } + } - let t_in = transparent_inputs - .iter() - .map(|t_in| t_in.coin().value) - .sum::>() - .ok_or_else(overflow)?; - let t_out = transparent_outputs - .iter() - .map(|t_out| t_out.value()) - .sum::>() - .ok_or_else(overflow)?; - let sapling_in = sapling_inputs - .iter() - .map(|s_in| s_in.value()) - .sum::>() - .ok_or_else(overflow)?; - let sapling_out = sapling_outputs - .iter() - .map(|s_out| s_out.value()) - .sum::>() - .ok_or_else(overflow)?; + pub(crate) struct TestSaplingInput { + pub note_id: u32, + pub value: Amount, + } - let total_in = (t_in + sapling_in).ok_or_else(overflow)?; - let total_out = [t_out, sapling_out, self.fixed_fee] - .iter() - .sum::>() - .ok_or_else(overflow)?; - - let proposed_change = (total_in - total_out).ok_or_else(underflow)?; - if proposed_change < Amount::zero() { - Err(ChangeError::InsufficientFunds { - available: total_in, - required: total_out, - }) - } else { - Ok(TransactionBalance::new( - vec![ChangeValue::Sapling(proposed_change)], - self.fixed_fee, - )) + impl sapling::InputView for TestSaplingInput { + fn note_id(&self) -> &u32 { + &self.note_id + } + fn value(&self) -> Amount { + self.value } } } diff --git a/zcash_client_backend/src/fees/fixed.rs b/zcash_client_backend/src/fees/fixed.rs new file mode 100644 index 000000000..d57953399 --- /dev/null +++ b/zcash_client_backend/src/fees/fixed.rs @@ -0,0 +1,234 @@ +//! Change strategies designed for use with a fixed fee. +use std::cmp::Ordering; + +use zcash_primitives::{ + consensus::{self, BlockHeight}, + transaction::{ + components::{ + amount::{Amount, BalanceError}, + sapling::fees as sapling, + transparent::fees as transparent, + }, + fees::{fixed::FeeRule as FixedFeeRule, FeeRule}, + }, +}; + +use super::{ + ChangeError, ChangeStrategy, ChangeValue, DustAction, DustOutputPolicy, TransactionBalance, +}; + +/// A change strategy that and proposes change as a single output to the most current supported +/// shielded pool and delegates fee calculation to the provided fee rule. +pub struct SingleOutputChangeStrategy { + fee_rule: FixedFeeRule, +} + +impl SingleOutputChangeStrategy { + /// Constructs a new [`SingleOutputChangeStrategy`] with the specified fee rule. + pub fn new(fee_rule: FixedFeeRule) -> Self { + Self { fee_rule } + } +} + +impl ChangeStrategy for SingleOutputChangeStrategy { + type FeeRule = FixedFeeRule; + type Error = BalanceError; + + fn fee_rule(&self) -> &Self::FeeRule { + &self.fee_rule + } + + fn compute_balance( + &self, + params: &P, + target_height: BlockHeight, + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling_inputs: &[impl sapling::InputView], + sapling_outputs: &[impl sapling::OutputView], + dust_output_policy: &DustOutputPolicy, + ) -> Result> { + let t_in = transparent_inputs + .iter() + .map(|t_in| t_in.coin().value) + .sum::>() + .ok_or(BalanceError::Overflow)?; + let t_out = transparent_outputs + .iter() + .map(|t_out| t_out.value()) + .sum::>() + .ok_or(BalanceError::Overflow)?; + let sapling_in = sapling_inputs + .iter() + .map(|s_in| s_in.value()) + .sum::>() + .ok_or(BalanceError::Overflow)?; + let sapling_out = sapling_outputs + .iter() + .map(|s_out| s_out.value()) + .sum::>() + .ok_or(BalanceError::Overflow)?; + + let fee_amount = self + .fee_rule + .fee_required( + params, + target_height, + transparent_inputs, + transparent_outputs, + sapling_inputs.len(), + sapling_outputs.len() + 1, + ) + .unwrap(); // fixed::FeeRule::fee_required is infallible. + + let total_in = (t_in + sapling_in).ok_or(BalanceError::Overflow)?; + + if (!transparent_inputs.is_empty() || !sapling_inputs.is_empty()) && fee_amount > total_in { + // For the fixed-fee selection rule, the only time we consider inputs dust is when the fee + // exceeds the value of all input values. + Err(ChangeError::DustInputs { + transparent: transparent_inputs + .iter() + .map(|i| i.outpoint()) + .cloned() + .collect(), + sapling: sapling_inputs + .iter() + .map(|i| i.note_id()) + .cloned() + .collect(), + }) + } else { + let total_out = [t_out, sapling_out, fee_amount] + .iter() + .sum::>() + .ok_or(BalanceError::Overflow)?; + + let proposed_change = (total_in - total_out).ok_or(BalanceError::Underflow)?; + match proposed_change.cmp(&Amount::zero()) { + Ordering::Less => Err(ChangeError::InsufficientFunds { + available: total_in, + required: total_out, + }), + Ordering::Equal => TransactionBalance::new(vec![], fee_amount) + .ok_or_else(|| BalanceError::Overflow.into()), + Ordering::Greater => { + let dust_threshold = dust_output_policy + .dust_threshold() + .unwrap_or_else(|| self.fee_rule.fixed_fee()); + + if dust_threshold > proposed_change { + match dust_output_policy.action() { + DustAction::Reject => { + let shortfall = (dust_threshold - proposed_change) + .ok_or(BalanceError::Underflow)?; + Err(ChangeError::InsufficientFunds { + available: total_in, + required: (total_in + shortfall) + .ok_or(BalanceError::Overflow)?, + }) + } + DustAction::AllowDustChange => TransactionBalance::new( + vec![ChangeValue::Sapling(proposed_change)], + fee_amount, + ) + .ok_or_else(|| BalanceError::Overflow.into()), + DustAction::AddDustToFee => TransactionBalance::new( + vec![], + (fee_amount + proposed_change).unwrap(), + ) + .ok_or_else(|| BalanceError::Overflow.into()), + } + } else { + TransactionBalance::new( + vec![ChangeValue::Sapling(proposed_change)], + fee_amount, + ) + .ok_or_else(|| BalanceError::Overflow.into()) + } + } + } + } + } +} + +#[cfg(test)] +mod tests { + use zcash_primitives::{ + consensus::{Network, NetworkUpgrade, Parameters}, + transaction::{ + components::{amount::Amount, transparent::TxOut}, + fees::fixed::FeeRule as FixedFeeRule, + }, + }; + + use super::SingleOutputChangeStrategy; + use crate::{ + data_api::wallet::input_selection::SaplingPayment, + fees::{ + tests::{TestSaplingInput, TestTransparentInput}, + ChangeError, ChangeStrategy, ChangeValue, DustOutputPolicy, + }, + }; + + #[test] + fn change_without_dust() { + let change_strategy = SingleOutputChangeStrategy::new(FixedFeeRule::standard()); + + // spend a single Sapling note that is sufficient to pay the fee + let result = change_strategy.compute_balance( + &Network::TestNetwork, + Network::TestNetwork + .activation_height(NetworkUpgrade::Nu5) + .unwrap(), + &Vec::::new(), + &Vec::::new(), + &[TestSaplingInput { + note_id: 0, + value: Amount::from_u64(45000).unwrap(), + }], + &[SaplingPayment::new(Amount::from_u64(40000).unwrap())], + &DustOutputPolicy::default(), + ); + + assert_matches!( + result, + Ok(balance) if balance.proposed_change() == [ChangeValue::Sapling(Amount::from_u64(4000).unwrap())] + && balance.fee_required() == Amount::from_u64(1000).unwrap() + ); + } + + #[test] + fn dust_change() { + let change_strategy = SingleOutputChangeStrategy::new(FixedFeeRule::standard()); + + // spend a single Sapling note that is sufficient to pay the fee + let result = change_strategy.compute_balance( + &Network::TestNetwork, + Network::TestNetwork + .activation_height(NetworkUpgrade::Nu5) + .unwrap(), + &Vec::::new(), + &Vec::::new(), + &[ + TestSaplingInput { + note_id: 0, + value: Amount::from_u64(40000).unwrap(), + }, + // enough to pay a fee, plus dust + TestSaplingInput { + note_id: 0, + value: Amount::from_u64(1100).unwrap(), + }, + ], + &[SaplingPayment::new(Amount::from_u64(40000).unwrap())], + &DustOutputPolicy::default(), + ); + + assert_matches!( + result, + Err(ChangeError::InsufficientFunds { available, required }) + if available == Amount::from_u64(41100).unwrap() && required == Amount::from_u64(42000).unwrap() + ); + } +} diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs new file mode 100644 index 000000000..174671eb1 --- /dev/null +++ b/zcash_client_backend/src/fees/zip317.rs @@ -0,0 +1,337 @@ +//! Change strategies designed to implement the ZIP 317 fee rules. +//! +//! Change selection in ZIP 317 requires careful handling of low-valued inputs +//! to ensure that inputs added to a transaction do not cause fees to rise by +//! an amount greater than their value. +use core::cmp::Ordering; + +use zcash_primitives::{ + consensus::{self, BlockHeight}, + transaction::{ + components::{ + amount::{Amount, BalanceError}, + sapling::fees as sapling, + transparent::fees as transparent, + }, + fees::{ + zip317::{FeeError as Zip317FeeError, FeeRule as Zip317FeeRule}, + FeeRule, + }, + }, +}; + +use super::{ + ChangeError, ChangeStrategy, ChangeValue, DustAction, DustOutputPolicy, TransactionBalance, +}; + +/// A change strategy that and proposes change as a single output to the most current supported +/// shielded pool and delegates fee calculation to the provided fee rule. +pub struct SingleOutputChangeStrategy { + fee_rule: Zip317FeeRule, +} + +impl SingleOutputChangeStrategy { + /// Constructs a new [`SingleOutputChangeStrategy`] with the specified ZIP 317 + /// fee parameters. + pub fn new(fee_rule: Zip317FeeRule) -> Self { + Self { fee_rule } + } +} + +impl ChangeStrategy for SingleOutputChangeStrategy { + type FeeRule = Zip317FeeRule; + type Error = Zip317FeeError; + + fn fee_rule(&self) -> &Self::FeeRule { + &self.fee_rule + } + + fn compute_balance( + &self, + params: &P, + target_height: BlockHeight, + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling_inputs: &[impl sapling::InputView], + sapling_outputs: &[impl sapling::OutputView], + dust_output_policy: &DustOutputPolicy, + ) -> Result> { + let mut transparent_dust: Vec<_> = transparent_inputs + .iter() + .filter_map(|i| { + // for now, we're just assuming p2pkh inputs, so we don't check the size of the input + // script + if i.coin().value < self.fee_rule.marginal_fee() { + Some(i.outpoint().clone()) + } else { + None + } + }) + .collect(); + + let mut sapling_dust: Vec<_> = sapling_inputs + .iter() + .filter_map(|i| { + if i.value() < self.fee_rule.marginal_fee() { + Some(i.note_id().clone()) + } else { + None + } + }) + .collect(); + + // Depending on the shape of the transaction, we may be able to spend up to + // `grace_actions - 1` dust inputs. If we don't have any dust inputs though, + // we don't need to worry about any of that. + if !(transparent_dust.is_empty() && sapling_dust.is_empty()) { + let t_non_dust = transparent_inputs.len() - transparent_dust.len(); + let t_allowed_dust = transparent_outputs.len().saturating_sub(t_non_dust); + + // We add one to the sapling outputs for the (single) change output Note that this + // means that wallet-internal shielding transactions are an opportunity to spend a dust + // note. + let s_non_dust = sapling_inputs.len() - sapling_dust.len(); + let s_allowed_dust = (sapling_outputs.len() + 1).saturating_sub(s_non_dust); + + let available_grace_inputs = self + .fee_rule + .grace_actions() + .saturating_sub(t_non_dust) + .saturating_sub(s_non_dust); + + let mut t_disallowed_dust = transparent_dust.len().saturating_sub(t_allowed_dust); + let mut s_disallowed_dust = sapling_dust.len().saturating_sub(s_allowed_dust); + + if available_grace_inputs > 0 { + // If we have available grace inputs, allocate them first to transparent dust + // and then to Sapling dust. The caller has provided inputs that it is willing + // to spend, so we don't need to consider privacy effects at this layer. + let t_grace_dust = available_grace_inputs.saturating_sub(t_disallowed_dust); + t_disallowed_dust = t_disallowed_dust.saturating_sub(t_grace_dust); + + let s_grace_dust = available_grace_inputs + .saturating_sub(t_grace_dust) + .saturating_sub(s_disallowed_dust); + s_disallowed_dust = s_disallowed_dust.saturating_sub(s_grace_dust); + } + + // Truncate the lists of inputs to be disregarded in input selection to just the + // disallowed lengths. This has the effect of prioritizing inputs for inclusion by the + // order of the original input slices, with the most preferred inputs first. + transparent_dust.reverse(); + transparent_dust.truncate(t_disallowed_dust); + sapling_dust.reverse(); + sapling_dust.truncate(s_disallowed_dust); + + if !(transparent_dust.is_empty() && sapling_dust.is_empty()) { + return Err(ChangeError::DustInputs { + transparent: transparent_dust, + sapling: sapling_dust, + }); + } + } + + let overflow = || ChangeError::StrategyError(Zip317FeeError::from(BalanceError::Overflow)); + let underflow = + || ChangeError::StrategyError(Zip317FeeError::from(BalanceError::Underflow)); + + let t_in = transparent_inputs + .iter() + .map(|t_in| t_in.coin().value) + .sum::>() + .ok_or_else(overflow)?; + let t_out = transparent_outputs + .iter() + .map(|t_out| t_out.value()) + .sum::>() + .ok_or_else(overflow)?; + let sapling_in = sapling_inputs + .iter() + .map(|s_in| s_in.value()) + .sum::>() + .ok_or_else(overflow)?; + let sapling_out = sapling_outputs + .iter() + .map(|s_out| s_out.value()) + .sum::>() + .ok_or_else(overflow)?; + + let fee_amount = self + .fee_rule + .fee_required( + params, + target_height, + transparent_inputs, + transparent_outputs, + sapling_inputs.len(), + sapling_outputs.len() + 1, + ) + .map_err(ChangeError::StrategyError)?; + + let total_in = (t_in + sapling_in).ok_or_else(overflow)?; + + let total_out = [t_out, sapling_out, fee_amount] + .iter() + .sum::>() + .ok_or_else(overflow)?; + + let proposed_change = (total_in - total_out).ok_or_else(underflow)?; + match proposed_change.cmp(&Amount::zero()) { + Ordering::Less => Err(ChangeError::InsufficientFunds { + available: total_in, + required: total_out, + }), + Ordering::Equal => TransactionBalance::new(vec![], fee_amount).ok_or_else(overflow), + Ordering::Greater => { + let dust_threshold = dust_output_policy + .dust_threshold() + .unwrap_or_else(|| self.fee_rule.marginal_fee()); + + if dust_threshold > proposed_change { + match dust_output_policy.action() { + DustAction::Reject => { + let shortfall = + (dust_threshold - proposed_change).ok_or_else(underflow)?; + + Err(ChangeError::InsufficientFunds { + available: total_in, + required: (total_in + shortfall).ok_or_else(overflow)?, + }) + } + DustAction::AllowDustChange => TransactionBalance::new( + vec![ChangeValue::Sapling(proposed_change)], + fee_amount, + ) + .ok_or_else(overflow), + DustAction::AddDustToFee => { + TransactionBalance::new(vec![], (fee_amount + proposed_change).unwrap()) + .ok_or_else(overflow) + } + } + } else { + TransactionBalance::new(vec![ChangeValue::Sapling(proposed_change)], fee_amount) + .ok_or_else(overflow) + } + } + } + } +} + +#[cfg(test)] +mod tests { + + use zcash_primitives::{ + consensus::{Network, NetworkUpgrade, Parameters}, + transaction::{ + components::{amount::Amount, transparent::TxOut}, + fees::zip317::FeeRule as Zip317FeeRule, + }, + }; + + use super::SingleOutputChangeStrategy; + use crate::{ + data_api::wallet::input_selection::SaplingPayment, + fees::{ + tests::{TestSaplingInput, TestTransparentInput}, + ChangeError, ChangeStrategy, ChangeValue, DustOutputPolicy, + }, + }; + + #[test] + fn change_without_dust() { + let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard()); + + // spend a single Sapling note that is sufficient to pay the fee + let result = change_strategy.compute_balance( + &Network::TestNetwork, + Network::TestNetwork + .activation_height(NetworkUpgrade::Nu5) + .unwrap(), + &Vec::::new(), + &Vec::::new(), + &[TestSaplingInput { + note_id: 0, + value: Amount::from_u64(55000).unwrap(), + }], + &[SaplingPayment::new(Amount::from_u64(40000).unwrap())], + &DustOutputPolicy::default(), + ); + + assert_matches!( + result, + Ok(balance) if balance.proposed_change() == [ChangeValue::Sapling(Amount::from_u64(5000).unwrap())] + && balance.fee_required() == Amount::from_u64(10000).unwrap() + ); + } + + #[test] + fn change_with_allowable_dust() { + let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard()); + + // spend a single Sapling note that is sufficient to pay the fee + let result = change_strategy.compute_balance( + &Network::TestNetwork, + Network::TestNetwork + .activation_height(NetworkUpgrade::Nu5) + .unwrap(), + &Vec::::new(), + &Vec::::new(), + &[ + TestSaplingInput { + note_id: 0, + value: Amount::from_u64(49000).unwrap(), + }, + TestSaplingInput { + note_id: 1, + value: Amount::from_u64(1000).unwrap(), + }, + ], + &[SaplingPayment::new(Amount::from_u64(40000).unwrap())], + &DustOutputPolicy::default(), + ); + + assert_matches!( + result, + Ok(balance) if balance.proposed_change().is_empty() + && balance.fee_required() == Amount::from_u64(10000).unwrap() + ); + } + + #[test] + fn change_with_disallowed_dust() { + let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard()); + + // spend a single Sapling note that is sufficient to pay the fee + let result = change_strategy.compute_balance( + &Network::TestNetwork, + Network::TestNetwork + .activation_height(NetworkUpgrade::Nu5) + .unwrap(), + &Vec::::new(), + &Vec::::new(), + &[ + TestSaplingInput { + note_id: 0, + value: Amount::from_u64(29000).unwrap(), + }, + TestSaplingInput { + note_id: 1, + value: Amount::from_u64(20000).unwrap(), + }, + TestSaplingInput { + note_id: 2, + value: Amount::from_u64(1000).unwrap(), + }, + ], + &[SaplingPayment::new(Amount::from_u64(40000).unwrap())], + &DustOutputPolicy::default(), + ); + + // We will get an error here, because the dust input now isn't free to add + // to the transaction. + assert_matches!( + result, + Err(ChangeError::DustInputs { sapling, .. }) if sapling == vec![2] + ); + } +} diff --git a/zcash_client_backend/src/lib.rs b/zcash_client_backend/src/lib.rs index 931cf99a8..f73711662 100644 --- a/zcash_client_backend/src/lib.rs +++ b/zcash_client_backend/src/lib.rs @@ -21,3 +21,7 @@ pub mod welding_rig; pub mod zip321; pub use decrypt::{decrypt_transaction, DecryptedOutput, TransferType}; + +#[cfg(test)] +#[macro_use] +extern crate assert_matches; diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 619e73ef6..f9f5d4fe8 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -10,7 +10,8 @@ use zcash_primitives::{ sapling::{Diversifier, Node, Note, Nullifier, PaymentAddress, Rseed}, transaction::{ components::{ - transparent::{OutPoint, TxOut}, + sapling, + transparent::{self, OutPoint, TxOut}, Amount, }, TxId, @@ -69,6 +70,19 @@ impl WalletTransparentOutput { pub fn recipient_address(&self) -> &TransparentAddress { &self.recipient_address } + + pub fn value(&self) -> Amount { + self.txout.value + } +} + +impl transparent::fees::InputView for WalletTransparentOutput { + fn outpoint(&self) -> &OutPoint { + &self.outpoint + } + fn coin(&self) -> &TxOut { + &self.txout + } } /// A subset of a [`SpendDescription`] relevant to wallets and light clients. @@ -97,13 +111,24 @@ pub struct WalletShieldedOutput { /// Information about a note that is tracked by the wallet that is available for spending, /// with sufficient information for use in note selection. -pub struct SpendableNote { +pub struct SpendableNote { + pub note_id: NoteRef, pub diversifier: Diversifier, pub note_value: Amount, pub rseed: Rseed, pub witness: IncrementalWitness, } +impl sapling::fees::InputView for SpendableNote { + fn note_id(&self) -> &NoteRef { + &self.note_id + } + + fn value(&self) -> Amount { + self.note_value + } +} + /// Describes a policy for which outgoing viewing key should be able to decrypt /// transaction outputs. /// diff --git a/zcash_client_backend/src/zip321.rs b/zcash_client_backend/src/zip321.rs index cf433ecd0..391c96e46 100644 --- a/zcash_client_backend/src/zip321.rs +++ b/zcash_client_backend/src/zip321.rs @@ -115,6 +115,11 @@ pub struct TransactionRequest { } impl TransactionRequest { + /// Constructs a new empty transaction request. + pub fn empty() -> Self { + Self { payments: vec![] } + } + /// Constructs a new transaction request that obeys the ZIP-321 invariants pub fn new(payments: Vec) -> Result { let request = TransactionRequest { payments }; diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index f8f71bbc1..b34436ad6 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -22,6 +22,8 @@ and this library adheres to Rust's notion of to initialize the accounts table with a noncontiguous set of account identifiers. - `SqliteClientError::AccountIdOutOfRange`, to report when the maximum account identifier has been reached. + - `SqliteClientError::Protobuf`, to support handling of errors in serialized + protobuf data decoding. - An `unstable` feature flag; this is added to parts of the API that may change in any release. It enables `zcash_client_backend`'s `unstable` feature flag. - New summary views that may be directly accessed in the sqlite database. @@ -40,6 +42,7 @@ and this library adheres to Rust's notion of this block source. - `zcash_client_sqlite::chain::init::init_blockmeta_db` creates the required metadata cache database. +- Implementations of `PartialEq`, `Eq`, `PartialOrd`, and `Ord` for `NoteId` ### Changed - Various **BREAKING CHANGES** have been made to the database tables. These will @@ -76,6 +79,9 @@ and this library adheres to Rust's notion of - `zcash_client_sqlite::wallet`: - `get_spendable_notes` to `get_spendable_sapling_notes`. - `select_spendable_notes` to `select_spendable_sapling_notes`. +- `zcash_client_sqlite::wallet::{get_spendable_sapling_notes, select_spendable_sapling_notes}` + have also been changed to take a parameter that permits the caller to + specify a set of notes to exclude from consideration. - `zcash_client_sqlite::wallet::init_wallet_db` has been modified to take the wallet seed as an argument so that it can correctly perform migrations that require re-deriving key material. In particular for @@ -95,6 +101,10 @@ and this library adheres to Rust's notion of - `delete_utxos_above` (use `WalletWrite::rewind_to_height` instead) - `zcash_client_sqlite::with_blocks` (use `zcash_client_backend::data_api::BlockSource::with_blocks` instead) +- `zcash_client_sqlite::error::SqliteClientError` variants: + - `SqliteClientError::IncorrectHrpExtFvk` + - `SqliteClientError::Base58` + - `SqliteClientError::BackendError` ### Fixed - The `zcash_client_backend::data_api::WalletRead::get_address` implementation diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index e17e1c250..76d2110b4 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -32,7 +32,7 @@ secrecy = "0.8" # - SQLite databases group = "0.12" jubjub = "0.9" -rusqlite = { version = "0.25", features = ["bundled", "time"] } +rusqlite = { version = "0.25", features = ["bundled", "time", "array"] } schemer = "0.2" schemer-rusqlite = "0.2" time = "0.2" @@ -42,6 +42,7 @@ uuid = "1.1" # (Breaking upgrades to these are usually backwards-compatible, but check MSRVs.) [dev-dependencies] +assert_matches = "1.5" proptest = "1.0.0" rand_core = "0.6" regex = "1.4" diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 6a2ae177e..4c3f422c2 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -5,13 +5,13 @@ use rusqlite::params; use zcash_primitives::consensus::BlockHeight; -use zcash_client_backend::{data_api::error::Error, proto::compact_formats::CompactBlock}; +use zcash_client_backend::{data_api::chain::error::Error, proto::compact_formats::CompactBlock}; use crate::{error::SqliteClientError, BlockDb}; #[cfg(feature = "unstable")] use { - crate::{BlockHash, FsBlockDb}, + crate::{BlockHash, FsBlockDb, FsBlockDbError}, rusqlite::Connection, std::fs::File, std::io::Read, @@ -21,53 +21,51 @@ use { pub mod init; pub mod migrations; -struct CompactBlockRow { - height: BlockHeight, - data: Vec, -} - /// Implements a traversal of `limit` blocks of the block cache database. /// /// Starting at the next block above `last_scanned_height`, the `with_row` callback is invoked with /// each block retrieved from the backing store. If the `limit` value provided is `None`, all /// blocks are traversed up to the maximum height. -pub(crate) fn blockdb_with_blocks( - cache: &BlockDb, +pub(crate) fn blockdb_with_blocks( + block_source: &BlockDb, last_scanned_height: BlockHeight, limit: Option, mut with_row: F, -) -> Result<(), SqliteClientError> +) -> Result<(), Error> where - F: FnMut(CompactBlock) -> Result<(), SqliteClientError>, + F: FnMut(CompactBlock) -> Result<(), Error>, { - // Fetch the CompactBlocks we need to scan - let mut stmt_blocks = cache.0.prepare( - "SELECT height, data FROM compactblocks WHERE height > ? ORDER BY height ASC LIMIT ?", - )?; + fn to_chain_error, N>(err: E) -> Error { + Error::BlockSource(err.into()) + } - let rows = stmt_blocks.query_map( - params![ + // Fetch the CompactBlocks we need to scan + let mut stmt_blocks = block_source + .0 + .prepare( + "SELECT height, data FROM compactblocks + WHERE height > ? + ORDER BY height ASC LIMIT ?", + ) + .map_err(to_chain_error)?; + + let mut rows = stmt_blocks + .query(params![ u32::from(last_scanned_height), limit.unwrap_or(u32::max_value()), - ], - |row| { - Ok(CompactBlockRow { - height: BlockHeight::from_u32(row.get(0)?), - data: row.get(1)?, - }) - }, - )?; + ]) + .map_err(to_chain_error)?; - for row_result in rows { - let cbr = row_result?; - let block = CompactBlock::decode(&cbr.data[..]).map_err(Error::from)?; - - if block.height() != cbr.height { - return Err(SqliteClientError::CorruptedData(format!( + while let Some(row) = rows.next().map_err(to_chain_error)? { + let height = BlockHeight::from_u32(row.get(0).map_err(to_chain_error)?); + let data: Vec = row.get(1).map_err(to_chain_error)?; + let block = CompactBlock::decode(&data[..]).map_err(to_chain_error)?; + if block.height() != height { + return Err(to_chain_error(SqliteClientError::CorruptedData(format!( "Block height {} did not match row's height field value {}", block.height(), - cbr.height - ))); + height + )))); } with_row(block)?; @@ -160,53 +158,65 @@ pub(crate) fn blockmetadb_get_max_cached_height( /// invoked with each block retrieved from the backing store. If the `limit` value provided is /// `None`, all blocks are traversed up to the maximum height for which metadata is available. #[cfg(feature = "unstable")] -pub(crate) fn fsblockdb_with_blocks( +pub(crate) fn fsblockdb_with_blocks( cache: &FsBlockDb, last_scanned_height: BlockHeight, limit: Option, mut with_block: F, -) -> Result<(), SqliteClientError> +) -> Result<(), Error> where - F: FnMut(CompactBlock) -> Result<(), SqliteClientError>, + F: FnMut(CompactBlock) -> Result<(), Error>, { + fn to_chain_error, N>(err: E) -> Error { + Error::BlockSource(err.into()) + } + // Fetch the CompactBlocks we need to scan - let mut stmt_blocks = cache.conn.prepare( - "SELECT height, blockhash, time, sapling_outputs_count, orchard_actions_count + let mut stmt_blocks = cache + .conn + .prepare( + "SELECT height, blockhash, time, sapling_outputs_count, orchard_actions_count FROM compactblocks_meta WHERE height > ? ORDER BY height ASC LIMIT ?", - )?; + ) + .map_err(to_chain_error)?; - let rows = stmt_blocks.query_map( - params![ - u32::from(last_scanned_height), - limit.unwrap_or(u32::max_value()), - ], - |row| { - Ok(BlockMeta { - height: BlockHeight::from_u32(row.get(0)?), - block_hash: BlockHash::from_slice(&row.get::<_, Vec<_>>(1)?), - block_time: row.get(2)?, - sapling_outputs_count: row.get(3)?, - orchard_actions_count: row.get(4)?, - }) - }, - )?; + let rows = stmt_blocks + .query_map( + params![ + u32::from(last_scanned_height), + limit.unwrap_or(u32::max_value()), + ], + |row| { + Ok(BlockMeta { + height: BlockHeight::from_u32(row.get(0)?), + block_hash: BlockHash::from_slice(&row.get::<_, Vec<_>>(1)?), + block_time: row.get(2)?, + sapling_outputs_count: row.get(3)?, + orchard_actions_count: row.get(4)?, + }) + }, + ) + .map_err(to_chain_error)?; for row_result in rows { - let cbr = row_result?; - let mut block_file = File::open(cbr.block_file_path(&cache.blocks_dir))?; + let cbr = row_result.map_err(to_chain_error)?; + let mut block_file = + File::open(cbr.block_file_path(&cache.blocks_dir)).map_err(to_chain_error)?; let mut block_data = vec![]; - block_file.read_to_end(&mut block_data)?; + block_file + .read_to_end(&mut block_data) + .map_err(to_chain_error)?; - let block = CompactBlock::decode(&block_data[..]).map_err(Error::from)?; + let block = CompactBlock::decode(&block_data[..]).map_err(to_chain_error)?; if block.height() != cbr.height { - return Err(SqliteClientError::CorruptedData(format!( + return Err(to_chain_error(FsBlockDbError::CorruptedData(format!( "Block height {} did not match row's height field value {}", block.height(), cbr.height - ))); + )))); } with_block(block)?; @@ -225,21 +235,20 @@ mod tests { block::BlockHash, transaction::components::Amount, zip32::ExtendedSpendingKey, }; - use zcash_client_backend::data_api::WalletRead; - use zcash_client_backend::data_api::{ - chain::{scan_cached_blocks, validate_chain}, - error::{ChainInvalid, Error}, + use zcash_client_backend::data_api::chain::{ + error::{Cause, Error}, + scan_cached_blocks, validate_chain, }; + use zcash_client_backend::data_api::WalletRead; use crate::{ chain::init::init_cache_database, - error::SqliteClientError, tests::{ self, fake_compact_block, fake_compact_block_spending, init_test_accounts_table, insert_into_cache, sapling_activation_height, AddressType, }, wallet::{get_balance, init::init_wallet_db, rewind_to_height}, - AccountId, BlockDb, NoteId, WalletDb, + AccountId, BlockDb, WalletDb, }; #[test] @@ -385,16 +394,13 @@ mod tests { insert_into_cache(&db_cache, &cb4); // Data+cache chain should be invalid at the data/cache boundary - match validate_chain( + let val_result = validate_chain( &tests::network(), &db_cache, db_data.get_max_height_hash().unwrap(), - ) { - Err(SqliteClientError::BackendError(Error::InvalidChain(lower_bound, _))) => { - assert_eq!(lower_bound, sapling_activation_height() + 2) - } - _ => panic!(), - } + ); + + assert_matches!(val_result, Err(Error::Chain(e)) if e.at_height() == sapling_activation_height() + 2); } #[test] @@ -459,16 +465,13 @@ mod tests { insert_into_cache(&db_cache, &cb4); // Data+cache chain should be invalid inside the cache - match validate_chain( + let val_result = validate_chain( &tests::network(), &db_cache, db_data.get_max_height_hash().unwrap(), - ) { - Err(SqliteClientError::BackendError(Error::InvalidChain(lower_bound, _))) => { - assert_eq!(lower_bound, sapling_activation_height() + 3) - } - _ => panic!(), - } + ); + + assert_matches!(val_result, Err(Error::Chain(e)) if e.at_height() == sapling_activation_height() + 3); } #[test] @@ -590,14 +593,11 @@ mod tests { ); insert_into_cache(&db_cache, &cb3); match scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None) { - Err(SqliteClientError::BackendError(e)) => { - assert_eq!( - e.to_string(), - ChainInvalid::block_height_discontinuity::( - sapling_activation_height() + 1, - sapling_activation_height() + 2 - ) - .to_string() + Err(Error::Chain(e)) => { + assert_matches!( + e.cause(), + Cause::BlockHeightDiscontinuity(h) if *h + == sapling_activation_height() + 2 ); } Ok(_) | Err(_) => panic!("Should have failed"), diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 1c7fb6873..29c09ec75 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -3,13 +3,10 @@ use std::error; use std::fmt; -use zcash_client_backend::{ - data_api, - encoding::{Bech32DecodeError, TransparentCodecError}, -}; +use zcash_client_backend::encoding::{Bech32DecodeError, TransparentCodecError}; use zcash_primitives::{consensus::BlockHeight, zip32::AccountId}; -use crate::{NoteId, PRUNING_HEIGHT}; +use crate::PRUNING_HEIGHT; #[cfg(feature = "transparent-inputs")] use zcash_primitives::legacy::TransparentAddress; @@ -23,8 +20,8 @@ pub enum SqliteClientError { /// Decoding of a stored value from its serialized form has failed. CorruptedData(String), - /// Decoding of the extended full viewing key has failed (for the specified network) - IncorrectHrpExtFvk, + /// An error occurred decoding a protobuf message. + Protobuf(prost::DecodeError), /// The rcm value for a note cannot be decoded to a valid JubJub point. InvalidNote, @@ -43,14 +40,12 @@ pub enum SqliteClientError { /// A Bech32-encoded key or address decoding error Bech32DecodeError(Bech32DecodeError), - /// Base58 decoding error - Base58(bs58::decode::Error), - - /// + /// An error produced in legacy transparent address derivation #[cfg(feature = "transparent-inputs")] HdwalletError(hdwallet::error::Error), - /// Base58 decoding error + /// An error encountered in decoding a transparent address from its + /// serialized form. TransparentAddress(TransparentCodecError), /// Wrapper for rusqlite errors. @@ -67,9 +62,6 @@ pub enum SqliteClientError { /// (safe rewind height, requested height). RequestedRewindInvalid(BlockHeight, BlockHeight), - /// Wrapper for errors from zcash_client_backend - BackendError(data_api::error::Error), - /// The space of allocatable diversifier indices has been exhausted for /// the given account. DiversifierIndexOutOfRange, @@ -109,14 +101,13 @@ impl fmt::Display for SqliteClientError { SqliteClientError::CorruptedData(reason) => { write!(f, "Data DB is corrupted: {}", reason) } - SqliteClientError::IncorrectHrpExtFvk => write!(f, "Incorrect HRP for extfvk"), + SqliteClientError::Protobuf(e) => write!(f, "Failed to parse protobuf-encoded record: {}", e), SqliteClientError::InvalidNote => write!(f, "Invalid note"), SqliteClientError::InvalidNoteId => write!(f, "The note ID associated with an inserted witness must correspond to a received note."), SqliteClientError::RequestedRewindInvalid(h, r) => write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet; the requested height was {}.", PRUNING_HEIGHT, h, r), SqliteClientError::Bech32DecodeError(e) => write!(f, "{}", e), - SqliteClientError::Base58(e) => write!(f, "{}", e), #[cfg(feature = "transparent-inputs")] SqliteClientError::HdwalletError(e) => write!(f, "{:?}", e), SqliteClientError::TransparentAddress(e) => write!(f, "{}", e), @@ -126,7 +117,6 @@ impl fmt::Display for SqliteClientError { SqliteClientError::DbError(e) => write!(f, "{}", e), SqliteClientError::Io(e) => write!(f, "{}", e), SqliteClientError::InvalidMemo(e) => write!(f, "{}", e), - SqliteClientError::BackendError(e) => write!(f, "{}", e), SqliteClientError::DiversifierIndexOutOfRange => write!(f, "The space of available diversifier indices is exhausted"), SqliteClientError::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {:?}", acct_id), SqliteClientError::AccountIdDiscontinuity => write!(f, "Wallet account identifiers must be sequential."), @@ -155,9 +145,9 @@ impl From for SqliteClientError { } } -impl From for SqliteClientError { - fn from(e: bs58::decode::Error) -> Self { - SqliteClientError::Base58(e) +impl From for SqliteClientError { + fn from(e: prost::DecodeError) -> Self { + SqliteClientError::Protobuf(e) } } @@ -179,9 +169,3 @@ impl From for SqliteClientError { SqliteClientError::InvalidMemo(e) } } - -impl From> for SqliteClientError { - fn from(e: data_api::error::Error) -> Self { - SqliteClientError::BackendError(e) - } -} diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 1c2fa0e52..e7a0ebebd 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -25,7 +25,7 @@ //! //! [`WalletRead`]: zcash_client_backend::data_api::WalletRead //! [`WalletWrite`]: zcash_client_backend::data_api::WalletWrite -//! [`BlockSource`]: zcash_client_backend::data_api::BlockSource +//! [`BlockSource`]: zcash_client_backend::data_api::chain::BlockSource //! [`CompactBlock`]: zcash_client_backend::proto::compact_formats::CompactBlock //! [`init_cache_database`]: crate::chain::init::init_cache_database @@ -45,15 +45,18 @@ use zcash_primitives::{ memo::Memo, merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::{Node, Nullifier}, - transaction::{components::Amount, Transaction, TxId}, + transaction::{ + components::{amount::Amount, OutPoint}, + Transaction, TxId, + }, zip32::{AccountId, DiversifierIndex, ExtendedFullViewingKey}, }; use zcash_client_backend::{ address::{AddressMetadata, UnifiedAddress}, data_api::{ - BlockSource, DecryptedTransaction, PoolType, PrunedBlock, Recipient, SentTransaction, - WalletRead, WalletWrite, + self, chain::BlockSource, DecryptedTransaction, PoolType, PrunedBlock, Recipient, + SentTransaction, WalletRead, WalletWrite, }, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, @@ -63,9 +66,6 @@ use zcash_client_backend::{ use crate::error::SqliteClientError; -#[cfg(not(feature = "transparent-inputs"))] -use zcash_client_backend::data_api::error::Error; - #[cfg(feature = "unstable")] use { crate::chain::{fsblockdb_with_blocks, BlockMeta}, @@ -87,7 +87,7 @@ pub(crate) const PRUNING_HEIGHT: u32 = 100; /// A newtype wrapper for sqlite primary key values for the notes /// table. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] pub enum NoteId { SentNoteId(i64), ReceivedNoteId(i64), @@ -116,7 +116,10 @@ pub struct WalletDb

{ impl WalletDb

{ /// Construct a connection to the wallet database stored at the specified path. pub fn for_path>(path: F, params: P) -> Result { - Connection::open(path).map(move |conn| WalletDb { conn, params }) + Connection::open(path).and_then(move |conn| { + rusqlite::vtab::array::load_module(&conn)?; + Ok(WalletDb { conn, params }) + }) } /// Given a wallet database connection, obtain a handle for the write operations @@ -230,9 +233,10 @@ impl WalletRead for WalletDb

{ &self, account: AccountId, anchor_height: BlockHeight, - ) -> Result, Self::Error> { + exclude: &[Self::NoteRef], + ) -> Result>, Self::Error> { #[allow(deprecated)] - wallet::transact::get_spendable_sapling_notes(self, account, anchor_height) + wallet::transact::get_spendable_sapling_notes(self, account, anchor_height, exclude) } fn select_spendable_sapling_notes( @@ -240,9 +244,16 @@ impl WalletRead for WalletDb

{ account: AccountId, target_value: Amount, anchor_height: BlockHeight, - ) -> Result, Self::Error> { + exclude: &[Self::NoteRef], + ) -> Result>, Self::Error> { #[allow(deprecated)] - wallet::transact::select_spendable_sapling_notes(self, account, target_value, anchor_height) + wallet::transact::select_spendable_sapling_notes( + self, + account, + target_value, + anchor_height, + exclude, + ) } fn get_transparent_receivers( @@ -253,23 +264,24 @@ impl WalletRead for WalletDb

{ return wallet::get_transparent_receivers(&self.params, &self.conn, _account); #[cfg(not(feature = "transparent-inputs"))] - return Err(SqliteClientError::BackendError( - Error::TransparentInputsNotSupported, - )); + panic!( + "The wallet must be compiled with the transparent-inputs feature to use this method." + ); } 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, _address, _max_height); + return wallet::get_unspent_transparent_outputs(self, _address, _max_height, _exclude); #[cfg(not(feature = "transparent-inputs"))] - return Err(SqliteClientError::BackendError( - Error::TransparentInputsNotSupported, - )); + panic!( + "The wallet must be compiled with the transparent-inputs feature to use this method." + ); } fn get_transparent_balances( @@ -281,9 +293,9 @@ impl WalletRead for WalletDb

{ return wallet::get_transparent_balances(self, _account, _max_height); #[cfg(not(feature = "transparent-inputs"))] - return Err(SqliteClientError::BackendError( - Error::TransparentInputsNotSupported, - )); + panic!( + "The wallet must be compiled with the transparent-inputs feature to use this method." + ); } } @@ -375,9 +387,10 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> { &self, account: AccountId, anchor_height: BlockHeight, - ) -> Result, Self::Error> { + exclude: &[Self::NoteRef], + ) -> Result>, Self::Error> { self.wallet_db - .get_spendable_sapling_notes(account, anchor_height) + .get_spendable_sapling_notes(account, anchor_height, exclude) } fn select_spendable_sapling_notes( @@ -385,9 +398,10 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> { account: AccountId, target_value: Amount, anchor_height: BlockHeight, - ) -> Result, Self::Error> { + exclude: &[Self::NoteRef], + ) -> Result>, Self::Error> { self.wallet_db - .select_spendable_sapling_notes(account, target_value, anchor_height) + .select_spendable_sapling_notes(account, target_value, anchor_height, exclude) } fn get_transparent_receivers( @@ -401,9 +415,10 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> { &self, address: &TransparentAddress, max_height: BlockHeight, + exclude: &[OutPoint], ) -> Result, Self::Error> { self.wallet_db - .get_unspent_transparent_outputs(address, max_height) + .get_unspent_transparent_outputs(address, max_height, exclude) } fn get_transparent_balances( @@ -745,9 +760,9 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> { return wallet::put_received_transparent_utxo(self, _output); #[cfg(not(feature = "transparent-inputs"))] - return Err(SqliteClientError::BackendError( - Error::TransparentInputsNotSupported, - )); + panic!( + "The wallet must be compiled with the transparent-inputs feature to use this method." + ); } } @@ -764,14 +779,17 @@ impl BlockDb { impl BlockSource for BlockDb { type Error = SqliteClientError; - fn with_blocks( + fn with_blocks( &self, from_height: BlockHeight, limit: Option, with_row: F, - ) -> Result<(), Self::Error> + ) -> Result<(), data_api::chain::error::Error> where - F: FnMut(CompactBlock) -> Result<(), Self::Error>, + F: FnMut( + CompactBlock, + ) + -> Result<(), data_api::chain::error::Error>, { chain::blockdb_with_blocks(self, from_height, limit, with_row) } @@ -788,7 +806,7 @@ impl BlockSource for BlockDb { /// /// where `` is the decimal value of the height at which the block was mined, and /// `` is the hexadecimal representation of the block hash, as produced by the -/// [`Display`] implementation for [`zcash_primitives::block::BlockHash`]. +/// [`fmt::Display`] implementation for [`zcash_primitives::block::BlockHash`]. /// /// This block source is intended to be used with the following data flow: /// * When the cache is being filled: @@ -828,6 +846,7 @@ pub struct FsBlockDb { pub enum FsBlockDbError { FsError(io::Error), DbError(rusqlite::Error), + Protobuf(prost::DecodeError), InvalidBlockstoreRoot(PathBuf), InvalidBlockPath(PathBuf), CorruptedData(String), @@ -847,6 +866,13 @@ impl From for FsBlockDbError { } } +#[cfg(feature = "unstable")] +impl From for FsBlockDbError { + fn from(e: prost::DecodeError) -> Self { + FsBlockDbError::Protobuf(e) + } +} + #[cfg(feature = "unstable")] impl FsBlockDb { /// Creates a filesystem-backed block store at the given path. @@ -896,21 +922,28 @@ impl FsBlockDb { #[cfg(feature = "unstable")] impl BlockSource for FsBlockDb { - type Error = SqliteClientError; + type Error = FsBlockDbError; - fn with_blocks( + fn with_blocks( &self, from_height: BlockHeight, limit: Option, with_row: F, - ) -> Result<(), Self::Error> + ) -> Result<(), data_api::chain::error::Error> where - F: FnMut(CompactBlock) -> Result<(), Self::Error>, + F: FnMut( + CompactBlock, + ) + -> Result<(), data_api::chain::error::Error>, { fsblockdb_with_blocks(self, from_height, limit, with_row) } } +#[cfg(test)] +#[macro_use] +extern crate assert_matches; + #[cfg(test)] #[allow(deprecated)] mod tests { diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 138a4154c..fa6044f89 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -27,7 +27,7 @@ use zcash_primitives::{ use zcash_client_backend::{ address::{RecipientAddress, UnifiedAddress}, - data_api::{error::Error, PoolType, Recipient, SentTransactionOutput}, + data_api::{PoolType, Recipient, SentTransactionOutput}, keys::UnifiedFullViewingKey, wallet::{WalletShieldedOutput, WalletTx}, DecryptedOutput, @@ -42,6 +42,7 @@ use crate::{ use { crate::UtxoId, rusqlite::{params, Connection}, + std::collections::BTreeSet, zcash_client_backend::{ address::AddressMetadata, encoding::AddressCodec, wallet::WalletTransparentOutput, }, @@ -745,7 +746,7 @@ pub(crate) fn rewind_to_height( let sapling_activation_height = wdb .params .activation_height(NetworkUpgrade::Sapling) - .ok_or(SqliteClientError::BackendError(Error::SaplingNotActive))?; + .expect("Sapling activation height mutst be available."); // Recall where we synced up to previously. let last_scanned_height = wdb @@ -955,6 +956,7 @@ pub(crate) fn get_unspent_transparent_outputs( wdb: &WalletDb

, address: &TransparentAddress, max_height: BlockHeight, + exclude: &[OutPoint], ) -> Result, SqliteClientError> { let mut stmt_blocks = wdb.conn.prepare( "SELECT u.prevout_txid, u.prevout_idx, u.script, @@ -971,6 +973,7 @@ pub(crate) fn get_unspent_transparent_outputs( let mut utxos = Vec::::new(); let mut rows = stmt_blocks.query(params![addr_str, u32::from(max_height)])?; + let excluded: BTreeSet = exclude.iter().cloned().collect(); while let Some(row) = rows.next()? { let txid: Vec = row.get(0)?; let mut txid_bytes = [0u8; 32]; @@ -981,8 +984,13 @@ pub(crate) fn get_unspent_transparent_outputs( let value = Amount::from_i64(row.get(3)?).unwrap(); let height: u32 = row.get(4)?; + let outpoint = OutPoint::new(txid_bytes, index); + if excluded.contains(&outpoint) { + continue; + } + let output = WalletTransparentOutput::from_parts( - OutPoint::new(txid_bytes, index), + outpoint, TxOut { value, script_pubkey, @@ -1394,7 +1402,8 @@ mod tests { super::get_unspent_transparent_outputs( &db_data, taddr, - BlockHeight::from_u32(12345) + BlockHeight::from_u32(12345), + &[] ), Ok(utxos) if utxos.is_empty() )); @@ -1403,7 +1412,8 @@ mod tests { super::get_unspent_transparent_outputs( &db_data, taddr, - BlockHeight::from_u32(34567) + BlockHeight::from_u32(34567), + &[] ), Ok(utxos) if { utxos.len() == 1 && diff --git a/zcash_client_sqlite/src/wallet/transact.rs b/zcash_client_sqlite/src/wallet/transact.rs index ba386bcd0..23e87c5de 100644 --- a/zcash_client_sqlite/src/wallet/transact.rs +++ b/zcash_client_sqlite/src/wallet/transact.rs @@ -1,6 +1,7 @@ //! Functions for creating transactions. //! -use rusqlite::{named_params, Row}; +use rusqlite::{named_params, types::Value, Row}; +use std::rc::Rc; use group::ff::PrimeField; @@ -14,11 +15,12 @@ use zcash_primitives::{ use zcash_client_backend::wallet::SpendableNote; -use crate::{error::SqliteClientError, WalletDb}; +use crate::{error::SqliteClientError, NoteId, WalletDb}; -fn to_spendable_note(row: &Row) -> Result { +fn to_spendable_note(row: &Row) -> Result, SqliteClientError> { + let note_id = NoteId::ReceivedNoteId(row.get(0)?); let diversifier = { - let d: Vec<_> = row.get(0)?; + let d: Vec<_> = row.get(1)?; if d.len() != 11 { return Err(SqliteClientError::CorruptedData( "Invalid diversifier length".to_string(), @@ -29,10 +31,10 @@ fn to_spendable_note(row: &Row) -> Result { Diversifier(tmp) }; - let note_value = Amount::from_i64(row.get(1)?).unwrap(); + let note_value = Amount::from_i64(row.get(2)?).unwrap(); let rseed = { - let rcm_bytes: Vec<_> = row.get(2)?; + let rcm_bytes: Vec<_> = row.get(3)?; // We store rcm directly in the data DB, regardless of whether the note // used a v1 or v2 note plaintext, so for the purposes of spending let's @@ -47,11 +49,12 @@ fn to_spendable_note(row: &Row) -> Result { }; let witness = { - let d: Vec<_> = row.get(3)?; + let d: Vec<_> = row.get(4)?; IncrementalWitness::read(&d[..])? }; Ok(SpendableNote { + note_id, diversifier, note_value, rseed, @@ -66,23 +69,34 @@ pub fn get_spendable_sapling_notes

( wdb: &WalletDb

, account: AccountId, anchor_height: BlockHeight, -) -> Result, SqliteClientError> { + exclude: &[NoteId], +) -> Result>, SqliteClientError> { let mut stmt_select_notes = wdb.conn.prepare( - "SELECT diversifier, value, rcm, witness + "SELECT id_note, diversifier, value, rcm, witness FROM received_notes INNER JOIN transactions ON transactions.id_tx = received_notes.tx INNER JOIN sapling_witnesses ON sapling_witnesses.note = received_notes.id_note WHERE account = :account AND spent IS NULL AND transactions.block <= :anchor_height - AND sapling_witnesses.block = :anchor_height", + AND sapling_witnesses.block = :anchor_height + AND id_note NOT IN rarray(:exclude)", )?; - // Select notes + let excluded: Vec = exclude + .iter() + .filter_map(|n| match n { + NoteId::ReceivedNoteId(i) => Some(Value::from(*i)), + NoteId::SentNoteId(_) => None, + }) + .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, ], to_spendable_note, )?; @@ -98,7 +112,8 @@ pub fn select_spendable_sapling_notes

( account: AccountId, target_value: Amount, anchor_height: BlockHeight, -) -> Result, SqliteClientError> { + exclude: &[NoteId], +) -> Result>, SqliteClientError> { // The goal of this SQL statement is to select the oldest notes until the required // value has been reached, and then fetch the witnesses at the desired height for the // selected notes. This is achieved in several steps: @@ -125,7 +140,10 @@ pub fn select_spendable_sapling_notes

( (PARTITION BY account, spent ORDER BY id_note) AS so_far FROM received_notes INNER JOIN transactions ON transactions.id_tx = received_notes.tx - WHERE account = :account AND spent IS NULL AND transactions.block <= :anchor_height + WHERE account = :account + AND spent IS NULL + AND transactions.block <= :anchor_height + AND id_note NOT IN rarray(:exclude) ) SELECT * FROM eligible WHERE so_far < :target_value UNION @@ -134,17 +152,26 @@ pub fn select_spendable_sapling_notes

( SELECT note, witness FROM sapling_witnesses WHERE block = :anchor_height ) - SELECT selected.diversifier, selected.value, selected.rcm, witnesses.witness + SELECT selected.id_note, selected.diversifier, selected.value, selected.rcm, witnesses.witness FROM selected INNER JOIN witnesses ON selected.id_note = witnesses.note", )?; - // Select notes + let excluded: Vec = exclude + .iter() + .filter_map(|n| match n { + NoteId::ReceivedNoteId(i) => Some(Value::from(*i)), + NoteId::SentNoteId(_) => None, + }) + .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), ":target_value": &i64::from(target_value), + ":exclude": &excluded_ptr ], to_spendable_note, )?; @@ -166,17 +193,23 @@ mod tests { consensus::{BlockHeight, BranchId}, legacy::TransparentAddress, sapling::{note_encryption::try_sapling_output_recovery, prover::TxProver}, - transaction::{components::Amount, Transaction}, + transaction::{components::Amount, fees::zip317::FeeRule as Zip317FeeRule, Transaction}, zip32::sapling::ExtendedSpendingKey, }; use zcash_client_backend::{ + address::RecipientAddress, data_api::{ - self, chain::scan_cached_blocks, wallet::create_spend_to_address, WalletRead, - WalletWrite, + self, + chain::scan_cached_blocks, + error::Error, + wallet::{create_spend_to_address, input_selection::GreedyInputSelector, spend}, + WalletRead, WalletWrite, }, + fees::{zip317, DustOutputPolicy}, keys::UnifiedSpendingKey, wallet::OvkPolicy, + zip321::{Payment, TransactionRequest}, }; use crate::{ @@ -220,7 +253,7 @@ mod tests { // Attempting to spend with a USK that is not in the wallet results in an error let mut db_write = db_data.get_update_ops().unwrap(); - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -232,10 +265,8 @@ mod tests { OvkPolicy::Sender, 10, ), - Err(crate::SqliteClientError::BackendError( - data_api::error::Error::KeyNotRecognized - )) - )); + Err(data_api::error::Error::KeyNotRecognized) + ); } #[test] @@ -253,7 +284,7 @@ mod tests { // We cannot do anything if we aren't synchronised let mut db_write = db_data.get_update_ops().unwrap(); - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -265,10 +296,8 @@ mod tests { OvkPolicy::Sender, 10, ), - Err(crate::SqliteClientError::BackendError( - data_api::error::Error::ScanRequired - )) - )); + Err(data_api::error::Error::ScanRequired) + ); } #[test] @@ -300,7 +329,7 @@ mod tests { // We cannot spend anything let mut db_write = db_data.get_update_ops().unwrap(); - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -312,14 +341,12 @@ mod tests { OvkPolicy::Sender, 10, ), - Err(crate::SqliteClientError::BackendError( - data_api::error::Error::InsufficientBalance( - available, - required - ) - )) + Err(data_api::error::Error::InsufficientFunds { + available, + required + }) if available == Amount::zero() && required == Amount::from_u64(1001).unwrap() - )); + ); } #[test] @@ -384,7 +411,7 @@ mod tests { // Spend fails because there are insufficient verified notes let extsk2 = ExtendedSpendingKey::master(&[]); let to = extsk2.default_address().1.into(); - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -396,15 +423,13 @@ mod tests { OvkPolicy::Sender, 10, ), - Err(crate::SqliteClientError::BackendError( - data_api::error::Error::InsufficientBalance( - available, - required - ) - )) + Err(data_api::error::Error::InsufficientFunds { + available, + required + }) if available == Amount::from_u64(50000).unwrap() && required == Amount::from_u64(71000).unwrap() - )); + ); // Mine blocks SAPLING_ACTIVATION_HEIGHT + 2 to 9 until just before the second // note is verified @@ -421,7 +446,7 @@ mod tests { scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); // Second spend still fails - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -433,15 +458,13 @@ mod tests { OvkPolicy::Sender, 10, ), - Err(crate::SqliteClientError::BackendError( - data_api::error::Error::InsufficientBalance( - available, - required - ) - )) + Err(data_api::error::Error::InsufficientFunds { + available, + required + }) if available == Amount::from_u64(50000).unwrap() && required == Amount::from_u64(71000).unwrap() - )); + ); // Mine block 11 so that the second note becomes verified let (cb, _) = fake_compact_block( @@ -455,7 +478,7 @@ mod tests { scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); // Second spend should now succeed - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -468,7 +491,7 @@ mod tests { 10, ), Ok(_) - )); + ); } #[test] @@ -504,7 +527,7 @@ mod tests { // Send some of the funds to another address let extsk2 = ExtendedSpendingKey::master(&[]); let to = extsk2.default_address().1.into(); - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -517,10 +540,10 @@ mod tests { 10, ), Ok(_) - )); + ); // A second spend fails because there are no usable notes - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -532,14 +555,12 @@ mod tests { OvkPolicy::Sender, 10, ), - Err(crate::SqliteClientError::BackendError( - data_api::error::Error::InsufficientBalance( - available, - required - ) - )) + Err(data_api::error::Error::InsufficientFunds { + available, + required + }) if available == Amount::zero() && required == Amount::from_u64(3000).unwrap() - )); + ); // Mine blocks SAPLING_ACTIVATION_HEIGHT + 1 to 21 (that don't send us funds) // until just before the first transaction expires @@ -556,7 +577,7 @@ mod tests { scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); // Second spend still fails - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -568,14 +589,12 @@ mod tests { OvkPolicy::Sender, 10, ), - Err(crate::SqliteClientError::BackendError( - data_api::error::Error::InsufficientBalance( - available, - required - ) - )) + Err(data_api::error::Error::InsufficientFunds { + available, + required + }) if available == Amount::zero() && required == Amount::from_u64(3000).unwrap() - )); + ); // Mine block SAPLING_ACTIVATION_HEIGHT + 22 so that the first transaction expires let (cb, _) = fake_compact_block( @@ -804,7 +823,7 @@ mod tests { ); let to = TransparentAddress::PublicKey([7; 20]).into(); - assert!(matches!( + assert_matches!( create_spend_to_address( &mut db_write, &tests::network(), @@ -817,6 +836,115 @@ mod tests { 10, ), Ok(_) - )); + ); + } + + #[test] + fn zip317_spend() { + let cache_file = NamedTempFile::new().unwrap(); + let db_cache = BlockDb(Connection::open(cache_file.path()).unwrap()); + init_cache_database(&db_cache).unwrap(); + + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); + init_wallet_db(&mut db_data, None).unwrap(); + + // Add an account to the wallet + let mut ops = db_data.get_update_ops().unwrap(); + let seed = Secret::new([0u8; 32].to_vec()); + let (_, usk) = ops.create_account(&seed).unwrap(); + let dfvk = usk.sapling().to_diversifiable_full_viewing_key(); + + // Add funds to the wallet + let (cb, _) = fake_compact_block( + sapling_activation_height(), + BlockHash([0; 32]), + &dfvk, + AddressType::Internal, + Amount::from_u64(50000).unwrap(), + ); + insert_into_cache(&db_cache, &cb); + + // Add 10 dust notes to the wallet + for i in 1..=10 { + let (cb, _) = fake_compact_block( + sapling_activation_height() + i, + cb.hash(), + &dfvk, + AddressType::DefaultExternal, + Amount::from_u64(1000).unwrap(), + ); + insert_into_cache(&db_cache, &cb); + } + + let mut db_write = db_data.get_update_ops().unwrap(); + scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); + + // Verified balance matches total balance + let total = Amount::from_u64(60000).unwrap(); + let (_, anchor_height) = db_data.get_target_and_anchor_heights(1).unwrap().unwrap(); + assert_eq!(get_balance(&db_data, AccountId::from(0)).unwrap(), total); + assert_eq!( + get_balance_at(&db_data, AccountId::from(0), anchor_height).unwrap(), + total + ); + + let input_selector = GreedyInputSelector::new( + zip317::SingleOutputChangeStrategy::new(Zip317FeeRule::standard()), + DustOutputPolicy::default(), + ); + + // This first request will fail due to insufficient non-dust funds + let req = TransactionRequest::new(vec![Payment { + recipient_address: RecipientAddress::Shielded(dfvk.default_address().1), + amount: Amount::from_u64(50000).unwrap(), + memo: None, + label: None, + message: None, + other_params: vec![], + }]) + .unwrap(); + + assert_matches!( + spend( + &mut db_write, + &tests::network(), + test_prover(), + &input_selector, + &usk, + req, + OvkPolicy::Sender, + 1, + ), + Err(Error::InsufficientFunds { available, required }) + if available == Amount::from_u64(51000).unwrap() + && required == Amount::from_u64(60000).unwrap() + ); + + // This request will succeed, spending a single dust input to pay the 10000 + // ZAT fee in addition to the 41000 ZAT output to the recipient + let req = TransactionRequest::new(vec![Payment { + recipient_address: RecipientAddress::Shielded(dfvk.default_address().1), + amount: Amount::from_u64(41000).unwrap(), + memo: None, + label: None, + message: None, + other_params: vec![], + }]) + .unwrap(); + + assert_matches!( + spend( + &mut db_write, + &tests::network(), + test_prover(), + &input_selector, + &usk, + req, + OvkPolicy::Sender, + 1, + ), + Ok(_) + ); } } diff --git a/zcash_extensions/src/transparent/demo.rs b/zcash_extensions/src/transparent/demo.rs index 764f4d408..bce8d0a21 100644 --- a/zcash_extensions/src/transparent/demo.rs +++ b/zcash_extensions/src/transparent/demo.rs @@ -492,10 +492,10 @@ mod tests { transaction::{ builder::Builder, components::{ - amount::{Amount, DEFAULT_FEE}, + amount::Amount, tze::{Authorized, Bundle, OutPoint, TzeIn, TzeOut}, }, - fees::FixedFeeRule, + fees::fixed, Transaction, TransactionData, TxVersion, }, zip32::ExtendedSpendingKey, @@ -809,7 +809,7 @@ mod tests { // let mut rng = OsRng; - let fee_rule = FixedFeeRule::new(DEFAULT_FEE); + let fee_rule = fixed::FeeRule::standard(); // create some inputs to spend let extsk = ExtendedSpendingKey::master(&[]); @@ -848,7 +848,7 @@ mod tests { let mut builder_b = demo_builder(tx_height + 1); let prevout_a = (OutPoint::new(tx_a.txid(), 0), tze_a.vout[0].clone()); - let value_xfr = (value - DEFAULT_FEE).unwrap(); + let value_xfr = (value - fee_rule.fixed_fee()).unwrap(); builder_b .demo_transfer_to_close(prevout_a, value_xfr, preimage_1, h2) .map_err(|e| format!("transfer failure: {:?}", e)) @@ -874,7 +874,7 @@ mod tests { builder_c .add_transparent_output( &TransparentAddress::PublicKey([0; 20]), - (value_xfr - DEFAULT_FEE).unwrap(), + (value_xfr - fee_rule.fixed_fee()).unwrap(), ) .unwrap(); diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 903bee6e1..5a9bae7ca 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -10,9 +10,12 @@ and this library adheres to Rust's notion of ### Added - Added in `zcash_primitives::zip32` - An implementation of `TryFrom` for `u32` +- `zcash_primitives::transaction::components::amount::NonNegativeAmount` - Added to `zcash_primitives::transaction::builder` - `Error::InsufficientFunds` - `Error::ChangeRequired` + - `Error::Balance` + - `Error::Fee` - `Builder` state accessor methods: - `Builder::params()` - `Builder::target_height()` @@ -24,6 +27,10 @@ and this library adheres to Rust's notion of and types related to fee calculations. - `FeeRule` a trait that describes how to compute the fee required for a transaction given inputs and outputs to the transaction. + - `fixed`, a new module containing an implementation of the existing fixed + fee rule. + - `zip317`, a new module containing an implementation of the ZIP 317 fee + rules. - Added to `zcash_primitives::transaction::components::sapling::builder` - `SaplingBuilder::{inputs, outputs}`: accessors for Sapling builder state. - `zcash_primitives::transaction::components::sapling::fees` @@ -33,14 +40,22 @@ and this library adheres to Rust's notion of - `zcash_primitives::sapling::Note::commitment` - Added to `zcash_primitives::zip32::sapling::DiversifiableFullViewingKey` - `DiversifiableFullViewingKey::{diversified_address, diversified_change_address}` +- `impl Eq for zcash_primitves::sapling::PaymentAddress` +- `impl {PartialOrd, Ord} for zcash_primitves::transaction::components::transparent::OutPoint` ### Changed - `zcash_primitives::transaction::builder::Builder::build` now takes a `FeeRule` argument which is used to compute the fee for the transaction as part of the build process. +- `zcash_primitives::transaction::builder::Builder::value_balance` now + returns `Result` instead of `Option`. - `zcash_primitives::transaction::builder::Builder::{new, new_with_rng}` no longer fixes the fee for transactions to 0.00001 ZEC; the builder instead computes the fee using a `FeeRule` implementation at build time. +- `zcash_primitives::transaction::builder::Error` now is parameterized by the + types that can now be produced by fee calculation. +- `zcash_primitives::transaction::components::tze::builder::Builder::value_balance` now + returns `Result` instead of `Option`. ### Deprecated - `zcash_primitives::zip32::sapling::to_extended_full_viewing_key` Use @@ -48,13 +63,14 @@ and this library adheres to Rust's notion of ### Removed - Removed from `zcash_primitives::transaction::builder::Builder` - - `Builder::{new_with_fee, new_with_rng_and_fee`} (use `Builder::{new, new_with_rng}` + - `Builder::{new_with_fee, new_with_rng_and_fee`} (use `Builder::{new, new_with_rng}` instead along with a `FeeRule` implementation passed to `Builder::build`.) - `Builder::send_change_to` has been removed. Change outputs must be added to the builder by the caller, just like any other output. - Removed from `zcash_primitives::transaction::builder::Error` - `Error::ChangeIsNegative` - `Error::NoChangeAddress` + - `Error::InvalidAmount` (replaced by `Error::BalanceError`) - `zcash_primitives::transaction::components::sapling::builder::SaplingBuilder::get_candidate_change_address` has been removed; change outputs must now be added by the caller. - The `From<&ExtendedSpendingKey>` instance for `ExtendedFullViewingKey` has been diff --git a/zcash_primitives/src/sapling.rs b/zcash_primitives/src/sapling.rs index 30a1afc4f..f4a47f008 100644 --- a/zcash_primitives/src/sapling.rs +++ b/zcash_primitives/src/sapling.rs @@ -280,6 +280,8 @@ impl PartialEq for PaymentAddress { } } +impl Eq for PaymentAddress {} + impl PaymentAddress { /// Constructs a PaymentAddress from a diversifier and a Jubjub point. /// diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 91dce2122..cf53cdbb7 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -1,7 +1,6 @@ //! Structs for building transactions. use std::cmp::Ordering; -use std::convert::Infallible; use std::error; use std::fmt; use std::sync::mpsc::Sender; @@ -20,7 +19,7 @@ use crate::{ sapling::{prover::TxProver, Diversifier, Node, Note, PaymentAddress}, transaction::{ components::{ - amount::Amount, + amount::{Amount, BalanceError}, sapling::{ self, builder::{SaplingBuilder, SaplingMetadata}, @@ -54,15 +53,17 @@ const DEFAULT_TX_EXPIRY_DELTA: u32 = 20; /// Errors that can occur during transaction construction. #[derive(Debug, PartialEq, Eq)] -pub enum Error { +pub enum Error { /// Insufficient funds were provided to the transaction builder; the given /// additional amount is required in order to construct the transaction. InsufficientFunds(Amount), /// The transaction has inputs in excess of outputs and fees; the user must /// add a change output. ChangeRequired(Amount), + /// An error occurred in computing the fees for a transaction. + Fee(FeeError), /// An overflow or underflow occurred when computing value balances - InvalidAmount, + Balance(BalanceError), /// An error occurred in constructing the transparent parts of a transaction. TransparentBuild(transparent::builder::Error), /// An error occurred in constructing the Sapling parts of a transaction. @@ -72,7 +73,7 @@ pub enum Error { TzeBuild(tze::builder::Error), } -impl fmt::Display for Error { +impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { Error::InsufficientFunds(amount) => write!( @@ -85,7 +86,8 @@ impl fmt::Display for Error { "The transaction requires an additional change output of {:?} zatoshis", amount ), - Error::InvalidAmount => write!(f, "Invalid amount (overflow or underflow)"), + Error::Balance(e) => write!(f, "Invalid amount {:?}", e), + Error::Fee(e) => write!(f, "An error occurred in fee calculation: {}", e), Error::TransparentBuild(err) => err.fmt(f), Error::SaplingBuild(err) => err.fmt(f), #[cfg(feature = "zfuture")] @@ -94,11 +96,11 @@ impl fmt::Display for Error { } } -impl error::Error for Error {} +impl error::Error for Error {} -impl From for Error { - fn from(_: Infallible) -> Error { - unreachable!() +impl From for Error { + fn from(e: BalanceError) -> Self { + Error::Balance(e) } } @@ -171,7 +173,7 @@ impl<'a, P, R> Builder<'a, P, R> { /// Returns the set of Sapling inputs currently committed to be consumed /// by the transaction. - pub fn sapling_inputs(&self) -> &[impl sapling::fees::InputView] { + pub fn sapling_inputs(&self) -> &[impl sapling::fees::InputView<()>] { self.sapling_builder.inputs() } @@ -239,10 +241,9 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { diversifier: Diversifier, note: Note, merkle_path: MerklePath, - ) -> Result<(), Error> { + ) -> Result<(), sapling::builder::Error> { self.sapling_builder .add_spend(&mut self.rng, extsk, diversifier, note, merkle_path) - .map_err(Error::SaplingBuild) } /// Adds a Sapling address to send funds to. @@ -252,10 +253,9 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { to: PaymentAddress, value: Amount, memo: MemoBytes, - ) -> Result<(), Error> { + ) -> Result<(), sapling::builder::Error> { self.sapling_builder .add_output(&mut self.rng, ovk, to, value, memo) - .map_err(Error::SaplingBuild) } /// Adds a transparent coin to be spent in this transaction. @@ -266,10 +266,8 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { sk: secp256k1::SecretKey, utxo: transparent::OutPoint, coin: TxOut, - ) -> Result<(), Error> { - self.transparent_builder - .add_input(sk, utxo, coin) - .map_err(Error::TransparentBuild) + ) -> Result<(), transparent::builder::Error> { + self.transparent_builder.add_input(sk, utxo, coin) } /// Adds a transparent address to send funds to. @@ -277,10 +275,8 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { &mut self, to: &TransparentAddress, value: Amount, - ) -> Result<(), Error> { - self.transparent_builder - .add_output(to, value) - .map_err(Error::TransparentBuild) + ) -> Result<(), transparent::builder::Error> { + self.transparent_builder.add_output(to, value) } /// Sets the notifier channel, where progress of building the transaction is sent. @@ -294,22 +290,18 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { } /// Returns the sum of the transparent, Sapling, and TZE value balances. - fn value_balance(&self) -> Result { + fn value_balance(&self) -> Result { let value_balances = [ - self.transparent_builder - .value_balance() - .ok_or(Error::InvalidAmount)?, + self.transparent_builder.value_balance()?, self.sapling_builder.value_balance(), #[cfg(feature = "zfuture")] - self.tze_builder - .value_balance() - .ok_or(Error::InvalidAmount)?, + self.tze_builder.value_balance()?, ]; value_balances .into_iter() .sum::>() - .ok_or(Error::InvalidAmount) + .ok_or(BalanceError::Overflow) } /// Builds a transaction from the configured spends and outputs. @@ -320,18 +312,17 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { self, prover: &impl TxProver, fee_rule: &FR, - ) -> Result<(Transaction, SaplingMetadata), Error> - where - Error: From, - { - let fee = fee_rule.fee_required( - &self.params, - self.target_height, - self.transparent_builder.inputs(), - self.transparent_builder.outputs(), - self.sapling_builder.inputs(), - self.sapling_builder.outputs(), - )?; + ) -> Result<(Transaction, SaplingMetadata), Error> { + let fee = fee_rule + .fee_required( + &self.params, + self.target_height, + self.transparent_builder.inputs(), + self.transparent_builder.outputs(), + self.sapling_builder.inputs().len(), + self.sapling_builder.outputs().len(), + ) + .map_err(Error::Fee)?; self.build_internal(prover, fee) } @@ -344,28 +335,28 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { self, prover: &impl TxProver, fee_rule: &FR, - ) -> Result<(Transaction, SaplingMetadata), Error> - where - Error: From, - { - let fee = fee_rule.fee_required_zfuture( - &self.params, - self.target_height, - self.transparent_builder.inputs(), - self.transparent_builder.outputs(), - self.sapling_builder.inputs(), - self.sapling_builder.outputs(), - self.tze_builder.inputs(), - self.tze_builder.outputs(), - )?; + ) -> Result<(Transaction, SaplingMetadata), Error> { + let fee = fee_rule + .fee_required_zfuture( + &self.params, + self.target_height, + self.transparent_builder.inputs(), + self.transparent_builder.outputs(), + self.sapling_builder.inputs().len(), + self.sapling_builder.outputs().len(), + self.tze_builder.inputs(), + self.tze_builder.outputs(), + ) + .map_err(Error::Fee)?; + self.build_internal(prover, fee) } - fn build_internal( + fn build_internal( self, prover: &impl TxProver, fee: Amount, - ) -> Result<(Transaction, SaplingMetadata), Error> { + ) -> Result<(Transaction, SaplingMetadata), Error> { let consensus_branch_id = BranchId::for_height(&self.params, self.target_height); // determine transaction version @@ -376,7 +367,7 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { // // After fees are accounted for, the value balance of the transaction must be zero. - let balance_after_fees = (self.value_balance()? - fee).ok_or(Error::InvalidAmount)?; + let balance_after_fees = (self.value_balance()? - fee).ok_or(BalanceError::Underflow)?; match balance_after_fees.cmp(&Amount::zero()) { Ordering::Less => { @@ -514,12 +505,13 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> ExtensionTxBuilder<'a #[cfg(any(test, feature = "test-dependencies"))] mod testing { use rand::RngCore; + use std::convert::Infallible; use super::{Builder, Error, SaplingMetadata}; use crate::{ consensus::{self, BlockHeight}, sapling::prover::mock::MockTxProver, - transaction::{components::amount::DEFAULT_FEE, fees::FixedFeeRule, Transaction}, + transaction::{fees::fixed, Transaction}, }; impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> { @@ -536,8 +528,8 @@ mod testing { Self::new_internal(params, rng, height) } - pub fn mock_build(self) -> Result<(Transaction, SaplingMetadata), Error> { - self.build(&MockTxProver, &FixedFeeRule::new(DEFAULT_FEE)) + pub fn mock_build(self) -> Result<(Transaction, SaplingMetadata), Error> { + self.build(&MockTxProver, &fixed::FeeRule::standard()) } } } @@ -599,7 +591,7 @@ mod tests { Amount::from_i64(-1).unwrap(), MemoBytes::empty() ), - Err(Error::SaplingBuild(build_s::Error::InvalidAmount)) + Err(build_s::Error::InvalidAmount) ); } @@ -714,7 +706,7 @@ mod tests { &TransparentAddress::PublicKey([0; 20]), Amount::from_i64(-1).unwrap(), ), - Err(Error::TransparentBuild(build_t::Error::InvalidAmount)) + Err(build_t::Error::InvalidAmount) ); } diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index 89f73dd5a..532e225f8 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -1,6 +1,6 @@ use std::convert::TryFrom; use std::iter::Sum; -use std::ops::{Add, AddAssign, Neg, Sub, SubAssign}; +use std::ops::{Add, AddAssign, Mul, Neg, Sub, SubAssign}; use memuse::DynamicUsage; use orchard::value as orchard; @@ -205,6 +205,17 @@ impl Neg for Amount { } } +impl Mul for Amount { + type Output = Option; + + fn mul(self, rhs: usize) -> Option { + let rhs: i64 = rhs.try_into().ok()?; + self.0 + .checked_mul(rhs) + .and_then(|i| Amount::try_from(i).ok()) + } +} + impl TryFrom for Amount { type Error = (); @@ -213,6 +224,35 @@ impl TryFrom for Amount { } } +/// A type-safe representation of some nonnegative amount of Zcash. +/// +/// A NonNegativeAmount can only be constructed from an integer that is within the valid monetary +/// range of `{0..MAX_MONEY}` (where `MAX_MONEY` = 21,000,000 × 10⁸ zatoshis). +#[derive(Clone, Copy, Debug, PartialEq, PartialOrd, Eq, Ord)] +pub struct NonNegativeAmount(Amount); + +impl NonNegativeAmount { + /// Creates a NonNegativeAmount from a u64. + /// + /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. + pub fn from_u64(amount: u64) -> Result { + Amount::from_u64(amount).map(NonNegativeAmount) + } + + /// Creates a NonNegativeAmount from an i64. + /// + /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. + pub fn from_nonnegative_i64(amount: i64) -> Result { + Amount::from_nonnegative_i64(amount).map(NonNegativeAmount) + } +} + +impl From for Amount { + fn from(n: NonNegativeAmount) -> Self { + n.0 + } +} + /// A type for balance violations in amount addition and subtraction /// (overflow and underflow of allowed ranges) #[derive(Copy, Clone, Debug, PartialEq, Eq)] diff --git a/zcash_primitives/src/transaction/components/sapling/builder.rs b/zcash_primitives/src/transaction/components/sapling/builder.rs index 9351af856..60bf7a835 100644 --- a/zcash_primitives/src/transaction/components/sapling/builder.rs +++ b/zcash_primitives/src/transaction/components/sapling/builder.rs @@ -69,10 +69,14 @@ pub struct SpendDescriptionInfo { merkle_path: MerklePath, } -impl fees::InputView for SpendDescriptionInfo { +impl fees::InputView<()> for SpendDescriptionInfo { + fn note_id(&self) -> &() { + // The builder does not make use of note identifiers, so we can just return the unit value. + &() + } + fn value(&self) -> Amount { - // An existing note to be spent must have a valid - // amount value. + // An existing note to be spent must have a valid amount value. Amount::from_u64(self.note.value).unwrap() } } @@ -243,7 +247,7 @@ impl

SaplingBuilder

{ /// Returns the list of Sapling inputs that will be consumed by the transaction being /// constructed. - pub fn inputs(&self) -> &[impl fees::InputView] { + pub fn inputs(&self) -> &[impl fees::InputView<()>] { &self.spends } diff --git a/zcash_primitives/src/transaction/components/sapling/fees.rs b/zcash_primitives/src/transaction/components/sapling/fees.rs index a1ad1659d..10d72adb6 100644 --- a/zcash_primitives/src/transaction/components/sapling/fees.rs +++ b/zcash_primitives/src/transaction/components/sapling/fees.rs @@ -5,7 +5,9 @@ use crate::transaction::components::amount::Amount; /// A trait that provides a minimized view of a Sapling input suitable for use in /// fee and change calculation. -pub trait InputView { +pub trait InputView { + /// An identifier for the input being spent. + fn note_id(&self) -> &NoteRef; /// The value of the input being spent. fn value(&self) -> Amount; } diff --git a/zcash_primitives/src/transaction/components/transparent.rs b/zcash_primitives/src/transaction/components/transparent.rs index ed1108259..988174e2f 100644 --- a/zcash_primitives/src/transaction/components/transparent.rs +++ b/zcash_primitives/src/transaction/components/transparent.rs @@ -90,7 +90,7 @@ impl Bundle { } } -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct OutPoint { hash: [u8; 32], n: u32, diff --git a/zcash_primitives/src/transaction/components/transparent/builder.rs b/zcash_primitives/src/transaction/components/transparent/builder.rs index 5d2780ad6..e32398072 100644 --- a/zcash_primitives/src/transaction/components/transparent/builder.rs +++ b/zcash_primitives/src/transaction/components/transparent/builder.rs @@ -6,7 +6,7 @@ use crate::{ legacy::{Script, TransparentAddress}, transaction::{ components::{ - amount::Amount, + amount::{Amount, BalanceError}, transparent::{self, fees, Authorization, Authorized, Bundle, TxIn, TxOut}, }, sighash::TransparentAuthorizingContext, @@ -176,23 +176,26 @@ impl TransparentBuilder { Ok(()) } - pub fn value_balance(&self) -> Option { + pub fn value_balance(&self) -> Result { #[cfg(feature = "transparent-inputs")] let input_sum = self .inputs .iter() .map(|input| input.coin.value) - .sum::>()?; + .sum::>() + .ok_or(BalanceError::Overflow)?; #[cfg(not(feature = "transparent-inputs"))] let input_sum = Amount::zero(); - input_sum - - self - .vout - .iter() - .map(|vo| vo.value) - .sum::>()? + let output_sum = self + .vout + .iter() + .map(|vo| vo.value) + .sum::>() + .ok_or(BalanceError::Overflow)?; + + (input_sum - output_sum).ok_or(BalanceError::Underflow) } pub fn build(self) -> Option> { diff --git a/zcash_primitives/src/transaction/components/tze/builder.rs b/zcash_primitives/src/transaction/components/tze/builder.rs index 8555a4870..82bd1f2e0 100644 --- a/zcash_primitives/src/transaction/components/tze/builder.rs +++ b/zcash_primitives/src/transaction/components/tze/builder.rs @@ -8,7 +8,7 @@ use crate::{ transaction::{ self as tx, components::{ - amount::Amount, + amount::{Amount, BalanceError}, tze::{fees, Authorization, Authorized, Bundle, OutPoint, TzeIn, TzeOut}, }, }, @@ -121,16 +121,22 @@ impl<'a, BuildCtx> TzeBuilder<'a, BuildCtx> { Ok(()) } - pub fn value_balance(&self) -> Option { - self.vin + pub fn value_balance(&self) -> Result { + let total_in = self + .vin .iter() .map(|tzi| tzi.coin.value) - .sum::>()? - - self - .vout - .iter() - .map(|tzo| tzo.value) - .sum::>()? + .sum::>() + .ok_or(BalanceError::Overflow)?; + + let total_out = self + .vout + .iter() + .map(|tzo| tzo.value) + .sum::>() + .ok_or(BalanceError::Overflow)?; + + (total_in - total_out).ok_or(BalanceError::Underflow) } pub fn build(self) -> (Option>, Vec>) { diff --git a/zcash_primitives/src/transaction/fees.rs b/zcash_primitives/src/transaction/fees.rs index 706ea9984..e29913acc 100644 --- a/zcash_primitives/src/transaction/fees.rs +++ b/zcash_primitives/src/transaction/fees.rs @@ -2,14 +2,15 @@ use crate::{ consensus::{self, BlockHeight}, - transaction::components::{ - amount::Amount, sapling::fees as sapling, transparent::fees as transparent, - }, + transaction::components::{amount::Amount, transparent::fees as transparent}, }; #[cfg(feature = "zfuture")] use crate::transaction::components::tze::fees as tze; +pub mod fixed; +pub mod zip317; + /// A trait that represents the ability to compute the fees that must be paid /// by a transaction having a specified set of inputs and outputs. pub trait FeeRule { @@ -26,8 +27,8 @@ pub trait FeeRule { target_height: BlockHeight, transparent_inputs: &[impl transparent::InputView], transparent_outputs: &[impl transparent::OutputView], - sapling_inputs: &[impl sapling::InputView], - sapling_outputs: &[impl sapling::OutputView], + sapling_input_count: usize, + sapling_output_count: usize, ) -> Result; } @@ -47,55 +48,9 @@ pub trait FutureFeeRule: FeeRule { target_height: BlockHeight, transparent_inputs: &[impl transparent::InputView], transparent_outputs: &[impl transparent::OutputView], - sapling_inputs: &[impl sapling::InputView], - sapling_outputs: &[impl sapling::OutputView], + sapling_input_count: usize, + sapling_output_count: usize, tze_inputs: &[impl tze::InputView], tze_outputs: &[impl tze::OutputView], ) -> Result; } - -/// A fee rule that always returns a fixed fee, irrespective of the structure of -/// the transaction being constructed. -pub struct FixedFeeRule { - fixed_fee: Amount, -} - -impl FixedFeeRule { - /// Creates a new fixed fee rule with the specified fixed fee. - pub fn new(fixed_fee: Amount) -> Self { - Self { fixed_fee } - } -} - -impl FeeRule for FixedFeeRule { - type Error = std::convert::Infallible; - - fn fee_required( - &self, - _params: &P, - _target_height: BlockHeight, - _transparent_inputs: &[impl transparent::InputView], - _transparent_outputs: &[impl transparent::OutputView], - _sapling_inputs: &[impl sapling::InputView], - _sapling_outputs: &[impl sapling::OutputView], - ) -> Result { - Ok(self.fixed_fee) - } -} - -#[cfg(feature = "zfuture")] -impl FutureFeeRule for FixedFeeRule { - fn fee_required_zfuture( - &self, - _params: &P, - _target_height: BlockHeight, - _transparent_inputs: &[impl transparent::InputView], - _transparent_outputs: &[impl transparent::OutputView], - _sapling_inputs: &[impl sapling::InputView], - _sapling_outputs: &[impl sapling::OutputView], - _tze_inputs: &[impl tze::InputView], - _tze_outputs: &[impl tze::OutputView], - ) -> Result { - Ok(self.fixed_fee) - } -} diff --git a/zcash_primitives/src/transaction/fees/fixed.rs b/zcash_primitives/src/transaction/fees/fixed.rs new file mode 100644 index 000000000..e9dfd7613 --- /dev/null +++ b/zcash_primitives/src/transaction/fees/fixed.rs @@ -0,0 +1,69 @@ +use crate::{ + consensus::{self, BlockHeight}, + transaction::components::{ + amount::{Amount, DEFAULT_FEE}, + transparent::fees as transparent, + }, +}; + +#[cfg(feature = "zfuture")] +use crate::transaction::components::tze::fees as tze; + +/// A fee rule that always returns a fixed fee, irrespective of the structure of +/// the transaction being constructed. +#[derive(Clone, Copy, Debug)] +pub struct FeeRule { + fixed_fee: Amount, +} + +impl FeeRule { + /// Creates a new nonstandard fixed fee rule with the specified fixed fee. + pub fn non_standard(fixed_fee: Amount) -> Self { + Self { fixed_fee } + } + + /// Creates a new fixed fee rule with the standard default fee. + pub fn standard() -> Self { + Self { + fixed_fee: DEFAULT_FEE, + } + } + + /// Returns the fixed fee amount which which this rule was configured. + pub fn fixed_fee(&self) -> Amount { + self.fixed_fee + } +} + +impl super::FeeRule for FeeRule { + type Error = std::convert::Infallible; + + fn fee_required( + &self, + _params: &P, + _target_height: BlockHeight, + _transparent_inputs: &[impl transparent::InputView], + _transparent_outputs: &[impl transparent::OutputView], + _sapling_input_count: usize, + _sapling_output_count: usize, + ) -> Result { + Ok(self.fixed_fee) + } +} + +#[cfg(feature = "zfuture")] +impl super::FutureFeeRule for FeeRule { + fn fee_required_zfuture( + &self, + _params: &P, + _target_height: BlockHeight, + _transparent_inputs: &[impl transparent::InputView], + _transparent_outputs: &[impl transparent::OutputView], + _sapling_input_count: usize, + _sapling_output_count: usize, + _tze_inputs: &[impl tze::InputView], + _tze_outputs: &[impl tze::OutputView], + ) -> Result { + Ok(self.fixed_fee) + } +} diff --git a/zcash_primitives/src/transaction/fees/zip317.rs b/zcash_primitives/src/transaction/fees/zip317.rs new file mode 100644 index 000000000..6f5218129 --- /dev/null +++ b/zcash_primitives/src/transaction/fees/zip317.rs @@ -0,0 +1,139 @@ +//! Types related to implementing a [`FeeRule`] provides [ZIP 317] fee calculation. +//! +//! [`FeeRule`]: crate::transaction::fees::FeeRule +//! [ZIP 317]: https//zips.z.cash/zip-0317 +use core::cmp::max; + +use crate::{ + consensus::{self, BlockHeight}, + legacy::TransparentAddress, + transaction::components::{ + amount::{Amount, BalanceError}, + transparent::{fees as transparent, OutPoint}, + }, +}; + +/// A [`FeeRule`] implementation that implements the [ZIP 317] fee rule. +/// +/// This fee rule supports only P2pkh transparent inputs; an error will be returned if a coin +/// containing a non-p2pkh script is provided as an input. This fee rule may slightly overestimate +/// fees in case where the user is attempting to spend more than ~150 transparent inputs. +/// +/// [`FeeRule`]: crate::transaction::fees::FeeRule +/// [ZIP 317]: https//zips.z.cash/zip-0317 +#[derive(Clone, Debug)] +pub struct FeeRule { + marginal_fee: Amount, + grace_actions: usize, + p2pkh_standard_input_size: usize, + p2pkh_standard_output_size: usize, +} + +impl FeeRule { + /// Construct a new FeeRule using the standard [ZIP 317] constants. + /// + /// [ZIP 317]: https//zips.z.cash/zip-0317 + pub fn standard() -> Self { + Self { + marginal_fee: Amount::from_u64(5000).unwrap(), + grace_actions: 2, + p2pkh_standard_input_size: 150, + p2pkh_standard_output_size: 34, + } + } + + /// Construct a new FeeRule instance with the specified parameter values. + /// + /// Returns `None` if either `p2pkh_standard_input_size` or `p2pkh_standard_output_size` are + /// zero. + pub fn non_standard( + marginal_fee: Amount, + grace_actions: usize, + p2pkh_standard_input_size: usize, + p2pkh_standard_output_size: usize, + ) -> Option { + if p2pkh_standard_input_size == 0 || p2pkh_standard_output_size == 0 { + None + } else { + Some(Self { + marginal_fee, + grace_actions, + p2pkh_standard_input_size, + p2pkh_standard_output_size, + }) + } + } + + /// Returns the ZIP 317 marginal fee. + pub fn marginal_fee(&self) -> Amount { + self.marginal_fee + } + /// Returns the ZIP 317 number of grace actions + pub fn grace_actions(&self) -> usize { + self.grace_actions + } + /// Returns the ZIP 317 standard P2PKH input size + pub fn p2pkh_standard_input_size(&self) -> usize { + self.p2pkh_standard_input_size + } + /// Returns the ZIP 317 standard P2PKH output size + pub fn p2pkh_standard_output_size(&self) -> usize { + self.p2pkh_standard_output_size + } +} + +/// Errors that can occur in ZIP 317 fee computation +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum FeeError { + /// An overflow or underflow of amount computation occurred. + Balance(BalanceError), + /// Transparent inputs provided to the fee calculation included coins that do not pay to + /// standard P2pkh scripts. + NonP2pkhInputs(Vec), +} + +impl From for FeeError { + fn from(err: BalanceError) -> Self { + FeeError::Balance(err) + } +} + +impl super::FeeRule for FeeRule { + type Error = FeeError; + + fn fee_required( + &self, + _params: &P, + _target_height: BlockHeight, + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling_input_count: usize, + sapling_output_count: usize, + ) -> Result { + let non_p2pkh_inputs: Vec<_> = transparent_inputs + .iter() + .filter_map(|t_in| match t_in.coin().script_pubkey.address() { + Some(TransparentAddress::PublicKey(_)) => None, + _ => Some(t_in.outpoint()), + }) + .cloned() + .collect(); + + if !non_p2pkh_inputs.is_empty() { + return Err(FeeError::NonP2pkhInputs(non_p2pkh_inputs)); + } + + let t_in_total_size = transparent_inputs.len() * 150; + let t_out_total_size = transparent_outputs.len() * 34; + + let ceildiv = |num: usize, den: usize| (num + den - 1) / den; + + let logical_actions = max( + ceildiv(t_in_total_size, self.p2pkh_standard_input_size), + ceildiv(t_out_total_size, self.p2pkh_standard_output_size), + ) + max(sapling_input_count, sapling_output_count); + + (self.marginal_fee * max(self.grace_actions, logical_actions)) + .ok_or_else(|| BalanceError::Overflow.into()) + } +}