diff --git a/Cargo.lock b/Cargo.lock index dff0bff627..1512161448 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3183,6 +3183,7 @@ dependencies = [ "schemer-rusqlite", "secrecy", "shardtree", + "static_assertions", "subtle", "tempfile", "time", diff --git a/Cargo.toml b/Cargo.toml index d6e6b09d53..1ba2dc6be2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -104,8 +104,9 @@ tonic-build = { version = "0.12", default-features = false } secrecy = "0.8" subtle = "2.2.3" -# Static constants +# Static constants and assertions lazy_static = "1" +static_assertions = "1" # Tests and benchmarks assert_matches = "1.5" diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 1f6fc7c0f8..492169acef 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -6,18 +6,39 @@ and this library adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Notable changes +`zcash_client_backend` now supports TEX (transparent-source-only) addresses as specified +in ZIP 320. Sending to one or more TEX addresses will automatically create a multi-step +proposal that uses two transactions. + +In order to take advantage of this support, client wallets will need to be able to send +multiple transactions created from `zcash_client_backend::data_api::wallet::create_proposed_transactions`. +This API was added in `zcash_client_backend` 0.11.0 but previously could only return a +single transaction. + +**Note:** This feature changes the use of transparent addresses in ways that are relevant +to security and access to funds, and that may interact with other wallet behaviour. In +particular it exposes new ephemeral transparent addresses belonging to the wallet, which +need to be scanned in order to recover funds if the first transaction of the proposal is +mined but the second is not, or if someone (e.g. the TEX-address recipient) sends back +funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for details. ### Added - `zcash_client_backend::data_api`: - `chain::BlockCache` trait, behind the `sync` feature flag. - `WalletRead::get_spendable_transparent_outputs`. - `zcash_client_backend::fees`: - - `ChangeValue::{transparent, shielded}` + - `EphemeralBalance` + - `ChangeValue::shielded, is_ephemeral` + - `ChangeValue::ephemeral_transparent` (when "transparent-inputs" is enabled) - `sapling::EmptyBundleView` - `orchard::EmptyBundleView` +- `zcash_client_backend::proposal`: + - `impl Hash for {StepOutput, StepOutputIndex}` - `zcash_client_backend::scanning`: - `testing` module -- `zcash_client_backend::sync` module, behind the `sync` feature flag +- `zcash_client_backend::sync` module, behind the `sync` feature flag. +- `zcash_client_backend::wallet::Recipient::map_ephemeral_transparent_outpoint` ### Changed - MSRV is now 1.70.0. @@ -31,36 +52,54 @@ and this library adheres to Rust's notion of `change_memo` is given, and defends against losing money by using `DustAction::AddDustToFee` with a too-high dust threshold. See [#1430](https://github.com/zcash/librustzcash/pull/1430) for details. -- `zcash_client_backend::zip321` has been extracted to, and is now a reexport +- `zcash_client_backend::zip321` has been extracted to, and is now a reexport of the root module of the `zip321` crate. Several of the APIs of this module have changed as a consequence of this extraction; please see the `zip321` CHANGELOG for details. - `zcash_client_backend::data_api`: - - `error::Error` has a new `Address` variant. + - `WalletRead` has new `get_known_ephemeral_addresses`, + `find_account_for_ephemeral_address`, and `get_transparent_address_metadata` + methods when the "transparent-inputs" feature is enabled. + - `WalletWrite` has a new `reserve_next_n_ephemeral_addresses` method when + the "transparent-inputs" feature is enabled. + - `error::Error` has new `Address` and (when the "transparent-inputs" feature + is enabled) `PaysEphemeralTransparentAddress` variants. - `wallet::input_selection::InputSelectorError` has a new `Address` variant. -- `zcash_client_backend::proto::proposal::Proposal::{from_standard_proposal, - try_into_standard_proposal}` each no longer require a `consensus::Parameters` - argument. - `zcash_client_backend::data_api::fees` - - The return type of `ChangeValue::output_pool`, and the type of the - `output_pool` argument to `ChangeValue::new`, have changed from - `ShieldedProtocol` to `zcash_protocol::PoolType`. - - The return type of `ChangeValue::new` is now optional; it returns `None` - if a memo is given for the transparent pool. Use `ChangeValue::shielded` - to avoid this error case when creating a `ChangeValue` known to be for a - shielded pool. + - When the "transparent-inputs" feature is enabled, `ChangeValue` can also + represent an ephemeral transparent output in a proposal. Accordingly, the + return type of `ChangeValue::output_pool` has (unconditionally) changed + from `ShieldedProtocol` to `zcash_protocol::PoolType`. + - `ChangeStrategy::compute_balance`: this trait method has an additional + `Option<&EphemeralBalance>` parameter. If the "transparent-inputs" feature is + enabled, this can be used to specify whether the change memo should be + ignored, and the amounts of additional transparent P2PKH inputs and + outputs. Passing `None` will retain the previous + behaviour (and is necessary when the "transparent-inputs" feature is + not enabled). - `zcash_client_backend::input_selection::GreedyInputSelectorError` has a new variant `UnsupportedTexAddress`. +- `zcash_client_backend::proposal::ProposalError` has new variants + `SpendsChange`, `EphemeralOutputLeftUnspent`, and `PaysTexFromShielded`. + (the last two are conditional on the "transparent-inputs" feature). +- `zcash_client_backend::proto`: + - `ProposalDecodingError` has a new variant `InvalidEphemeralRecipient`. + - `proposal::Proposal::{from_standard_proposal, try_into_standard_proposal}` + each no longer require a `consensus::Parameters` argument. - `zcash_client_backend::wallet::Recipient` variants have changed. Instead of - wrapping protocol-address types, the `Recipient` type now wraps a - `zcash_address::ZcashAddress`. This simplifies the process of tracking the - original address to which value was sent. + wrapping protocol-address types, the `External` and `InternalAccount` variants + now wrap a `zcash_address::ZcashAddress`. This simplifies the process of + tracking the original address to which value was sent. There is also a new + `EphemeralTransparent` variant, and an additional generic parameter for the + type of metadata associated with an ephemeral transparent outpoint. ### Removed - `zcash_client_backend::data_api`: - `WalletRead::get_unspent_transparent_outputs` has been removed because its semantics were unclear and could not be clarified. Use `WalletRead::get_spendable_transparent_outputs` instead. +- `zcash_client_backend::fees::ChangeValue::new`. Use `ChangeValue::shielded` + or `ChangeValue::ephemeral_transparent` instead. ## [0.12.1] - 2024-03-27 diff --git a/zcash_client_backend/proto/proposal.proto b/zcash_client_backend/proto/proposal.proto index 950bb3406c..db548c6e23 100644 --- a/zcash_client_backend/proto/proposal.proto +++ b/zcash_client_backend/proto/proposal.proto @@ -73,14 +73,14 @@ message ReceivedOutput { uint64 value = 4; } -// A reference a payment in a prior step of the proposal. This payment must +// A reference to a payment in a prior step of the proposal. This payment must // belong to the wallet. message PriorStepOutput { uint32 stepIndex = 1; uint32 paymentIndex = 2; } -// A reference a change output from a prior step of the proposal. +// A reference to a change or ephemeral output from a prior step of the proposal. message PriorStepChange { uint32 stepIndex = 1; uint32 changeIndex = 2; @@ -112,22 +112,29 @@ enum FeeRule { // The proposed change outputs and fee value. message TransactionBalance { - // A list of change output values. + // A list of change or ephemeral output values. repeated ChangeValue proposedChange = 1; // The fee to be paid by the proposed transaction, in zatoshis. uint64 feeRequired = 2; } -// A proposed change output. If the transparent value pool is selected, -// the `memo` field must be null. +// A proposed change or ephemeral output. If the transparent value pool is +// selected, the `memo` field must be null. +// +// When the `isEphemeral` field of a `ChangeValue` is set, it represents +// an ephemeral output, which must be spent by a subsequent step. This is +// only supported for transparent outputs. Each ephemeral output will be +// given a unique t-address. message ChangeValue { - // The value of a change output to be created, in zatoshis. + // The value of a change or ephemeral output to be created, in zatoshis. uint64 value = 1; - // The value pool in which the change output should be created. + // The value pool in which the change or ephemeral output should be created. ValuePool valuePool = 2; - // The optional memo that should be associated with the newly created change output. - // Memos must not be present for transparent change outputs. + // The optional memo that should be associated with the newly created output. + // Memos must not be present for transparent outputs. MemoBytes memo = 3; + // Whether this is to be an ephemeral output. + bool isEphemeral = 4; } // An object wrapper for memo bytes, to facilitate representing the diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index fc89170786..9c8e5a9326 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -88,15 +88,18 @@ use zcash_primitives::{ consensus::BlockHeight, memo::{Memo, MemoBytes}, transaction::{ - components::amount::{BalanceError, NonNegativeAmount}, + components::{ + amount::{BalanceError, NonNegativeAmount}, + OutPoint, + }, Transaction, TxId, }, }; #[cfg(feature = "transparent-inputs")] use { - crate::wallet::TransparentAddressMetadata, - zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}, + crate::wallet::TransparentAddressMetadata, std::ops::Range, + zcash_primitives::legacy::TransparentAddress, }; #[cfg(any(test, feature = "test-dependencies"))] @@ -677,12 +680,14 @@ pub trait InputSource { Ok(None) } - /// Returns the list of transparent outputs received at `address` such that: - /// * The transaction that produced these outputs is mined or mineable as of `max_height`. - /// * Each returned output is unspent as of the current chain tip. + /// Returns the list of spendable transparent outputs received by this wallet at `address` + /// such that, at height `target_height`: + /// * the transaction that produced the output had or will have at least `min_confirmations` + /// confirmations; and + /// * the output is unspent as of the current chain tip. /// - /// The caller should filter these outputs to ensure they respect the desired number of - /// confirmations before attempting to spend them. + /// An output that is potentially spent by an unmined transaction in the mempool is excluded + /// iff the spending transaction will not be expired at `target_height`. #[cfg(feature = "transparent-inputs")] fn get_spendable_transparent_outputs( &self, @@ -890,10 +895,14 @@ pub trait WalletRead { query: NullifierQuery, ) -> Result, Self::Error>; - /// Returns the set of all transparent receivers associated with the given account. + /// Returns the set of non-ephemeral transparent receivers associated with the given + /// account controlled by this wallet. /// - /// The set contains all transparent receivers that are known to have been derived - /// under this account. Wallets should scan the chain for UTXOs sent to these + /// The set contains all non-ephemeral transparent receivers that are known to have + /// been derived under this account. Wallets should scan the chain for UTXOs sent to + /// these receivers. + /// + /// Use [`Self::get_known_ephemeral_addresses`] to obtain the ephemeral transparent /// receivers. #[cfg(feature = "transparent-inputs")] fn get_transparent_receivers( @@ -903,8 +912,11 @@ pub trait WalletRead { Ok(HashMap::new()) } - /// Returns a mapping from transparent receiver to not-yet-shielded UTXO balance, - /// for each address associated with a nonzero balance. + /// Returns a mapping from each transparent receiver associated with the specified account + /// to its not-yet-shielded UTXO balance as of the end of the block at the provided + /// `max_height`, when that balance is non-zero. + /// + /// Balances of ephemeral transparent addresses will not be included. #[cfg(feature = "transparent-inputs")] fn get_transparent_balances( &self, @@ -913,6 +925,120 @@ pub trait WalletRead { ) -> Result, Self::Error> { Ok(HashMap::new()) } + + /// Returns the metadata associated with a given transparent receiver in an account + /// controlled by this wallet, if available. + /// + /// This is equivalent to (but may be implemented more efficiently than): + /// ```compile_fail + /// Ok( + /// if let Some(result) = self.get_transparent_receivers(account)?.get(address) { + /// result.clone() + /// } else { + /// self.get_known_ephemeral_addresses(account, None)? + /// .into_iter() + /// .find(|(known_addr, _)| known_addr == address) + /// .map(|(_, metadata)| metadata) + /// }, + /// ) + /// ``` + /// + /// Returns `Ok(None)` if the address is not recognized, or we do not have metadata for it. + /// Returns `Ok(Some(metadata))` if we have the metadata. + #[cfg(feature = "transparent-inputs")] + fn get_transparent_address_metadata( + &self, + account: Self::AccountId, + address: &TransparentAddress, + ) -> Result, Self::Error> { + // This should be overridden. + Ok( + if let Some(result) = self.get_transparent_receivers(account)?.get(address) { + result.clone() + } else { + self.get_known_ephemeral_addresses(account, None)? + .into_iter() + .find(|(known_addr, _)| known_addr == address) + .map(|(_, metadata)| metadata) + }, + ) + } + + /// Returns a vector of ephemeral transparent addresses associated with the given + /// account controlled by this wallet, along with their metadata. The result includes + /// reserved addresses, and addresses for `GAP_LIMIT` additional indices (capped to + /// the maximum index). + /// + /// If `index_range` is some `Range`, it limits the result to addresses with indices + /// in that range. + /// + /// Wallets should scan the chain for UTXOs sent to these ephemeral transparent + /// receivers, but do not need to do so regularly. Under expected usage, outputs + /// would only be detected with these receivers in the following situations: + /// + /// - This wallet created a payment to a ZIP 320 (TEX) address, but the second + /// transaction (that spent the output sent to the ephemeral address) did not get + /// mined before it expired. + /// - In this case the output will already be known to the wallet (because it + /// stores the transactions that it creates). + /// + /// - Another wallet app using the same seed phrase created a payment to a ZIP 320 + /// address, and this wallet queried for the ephemeral UTXOs after the first + /// transaction was mined but before the second transaction was mined. + /// - In this case, the output should not be considered unspent until the expiry + /// height of the transaction it was received in has passed. Wallets creating + /// payments to TEX addresses generally set the same expiry height for the first + /// and second transactions, meaning that this wallet does not need to observe + /// the second transaction to determine when it would have expired. + /// + /// - A TEX address recipient decided to return funds that the wallet had sent to + /// them. + /// + /// In all cases, the wallet should re-shield the unspent outputs, in a separate + /// transaction per ephemeral address, before re-spending the funds. + #[cfg(feature = "transparent-inputs")] + fn get_known_ephemeral_addresses( + &self, + _account: Self::AccountId, + _index_range: Option>, + ) -> Result, Self::Error> { + Ok(vec![]) + } + + /// If a given ephemeral address might have been reserved, i.e. would be included in + /// the map returned by `get_known_ephemeral_addresses(account_id, false)` for any + /// of the wallet's accounts, then return `Ok(Some(account_id))`. Otherwise return + /// `Ok(None)`. + /// + /// This is equivalent to (but may be implemented more efficiently than): + /// ```compile_fail + /// for account_id in self.get_account_ids()? { + /// if self + /// .get_known_ephemeral_addresses(account_id, None)? + /// .into_iter() + /// .any(|(known_addr, _)| &known_addr == address) + /// { + /// return Ok(Some(account_id)); + /// } + /// } + /// Ok(None) + /// ``` + #[cfg(feature = "transparent-inputs")] + fn find_account_for_ephemeral_address( + &self, + address: &TransparentAddress, + ) -> Result, Self::Error> { + for account_id in self.get_account_ids()? { + if self + .get_known_ephemeral_addresses(account_id, None)? + .into_iter() + .any(|(known_addr, _)| &known_addr == address) + { + return Ok(Some(account_id)); + } + } + Ok(None) + } } /// The relevance of a seed to a given wallet. @@ -1243,7 +1369,7 @@ impl<'a, AccountId> SentTransaction<'a, AccountId> { /// This type is capable of representing both shielded and transparent outputs. pub struct SentTransactionOutput { output_index: usize, - recipient: Recipient, + recipient: Recipient, value: NonNegativeAmount, memo: Option, } @@ -1260,7 +1386,7 @@ impl SentTransactionOutput { /// * `memo` - the memo that was sent with this output pub fn from_parts( output_index: usize, - recipient: Recipient, + recipient: Recipient, value: NonNegativeAmount, memo: Option, ) -> Self { @@ -1282,8 +1408,8 @@ impl SentTransactionOutput { self.output_index } /// Returns the recipient address of the transaction, or the account id and - /// resulting note for wallet-internal outputs. - pub fn recipient(&self) -> &Recipient { + /// resulting note/outpoint for wallet-internal outputs. + pub fn recipient(&self) -> &Recipient { &self.recipient } /// Returns the value of the newly created output. @@ -1463,6 +1589,8 @@ pub trait WalletWrite: WalletRead { /// funds have been received by the currently-available account (in order to enable automated /// account recovery). /// + /// # Panics + /// /// Panics if the length of the seed is not between 32 and 252 bytes inclusive. /// /// [ZIP 316]: https://zips.z.cash/zip-0316 @@ -1542,6 +1670,26 @@ pub trait WalletWrite: WalletRead { /// /// There may be restrictions on heights to which it is possible to truncate. fn truncate_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error>; + + /// Reserves the next `n` available ephemeral addresses for the given account. + /// This cannot be undone, so as far as possible, errors associated with transaction + /// construction should have been reported before calling this method. + /// + /// To ensure that sufficient information is stored on-chain to allow recovering + /// funds sent back to any of the used addresses, a "gap limit" of 20 addresses + /// should be observed as described in [BIP 44]. + /// + /// Returns an error if there is insufficient space within the gap limit to allocate + /// the given number of addresses, or if the account identifier does not correspond + /// to a known account. + /// + /// [BIP 44]: https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#user-content-Address_gap_limit + #[cfg(feature = "transparent-inputs")] + fn reserve_next_n_ephemeral_addresses( + &mut self, + account_id: Self::AccountId, + n: usize, + ) -> Result, Self::Error>; } /// This trait describes a capability for manipulating wallet note commitment trees. @@ -1646,7 +1794,10 @@ pub mod testing { }; #[cfg(feature = "transparent-inputs")] - use {crate::wallet::TransparentAddressMetadata, zcash_primitives::legacy::TransparentAddress}; + use { + crate::wallet::TransparentAddressMetadata, std::ops::Range, + zcash_primitives::legacy::TransparentAddress, + }; #[cfg(feature = "orchard")] use super::ORCHARD_SHARD_HEIGHT; @@ -1869,6 +2020,32 @@ pub mod testing { ) -> Result, Self::Error> { Ok(HashMap::new()) } + + #[cfg(feature = "transparent-inputs")] + fn get_transparent_address_metadata( + &self, + _account: Self::AccountId, + _address: &TransparentAddress, + ) -> Result, Self::Error> { + Ok(None) + } + + #[cfg(feature = "transparent-inputs")] + fn get_known_ephemeral_addresses( + &self, + _account: Self::AccountId, + _index_range: Option>, + ) -> Result, Self::Error> { + Ok(vec![]) + } + + #[cfg(feature = "transparent-inputs")] + fn find_account_for_ephemeral_address( + &self, + _address: &TransparentAddress, + ) -> Result, Self::Error> { + Ok(None) + } } impl WalletWrite for MockWalletDb { @@ -1931,6 +2108,15 @@ pub mod testing { ) -> Result { Ok(0) } + + #[cfg(feature = "transparent-inputs")] + fn reserve_next_n_ephemeral_addresses( + &mut self, + _account_id: Self::AccountId, + _n: usize, + ) -> Result, Self::Error> { + Err(()) + } } impl WalletCommitmentTrees for MockWalletDb { diff --git a/zcash_client_backend/src/data_api/error.rs b/zcash_client_backend/src/data_api/error.rs index 562045a2af..fd1d35aaae 100644 --- a/zcash_client_backend/src/data_api/error.rs +++ b/zcash_client_backend/src/data_api/error.rs @@ -36,8 +36,10 @@ pub enum Error { /// An error in transaction proposal construction Proposal(ProposalError), - /// The proposal was structurally valid, but spending shielded outputs of prior multi-step - /// transaction steps is not yet supported. + /// The proposal was structurally valid, but tried to do one of these unsupported things: + /// * spend a prior shielded output; + /// * pay to an output pool for which the corresponding feature is not enabled; + /// * pay to a TEX address if the "transparent-inputs" feature is not enabled. ProposalNotSupported, /// No account could be found corresponding to a provided spending key. @@ -89,6 +91,11 @@ pub enum Error { /// belonging to the wallet. #[cfg(feature = "transparent-inputs")] AddressNotRecognized(TransparentAddress), + + /// The wallet tried to pay to an ephemeral transparent address as a normal + /// output. + #[cfg(feature = "transparent-inputs")] + PaysEphemeralTransparentAddress(String), } impl fmt::Display for Error @@ -116,12 +123,12 @@ where Error::Proposal(e) => { write!(f, "Input selection attempted to construct an invalid proposal: {}", e) } - Error::ProposalNotSupported => { - write!( - f, - "The proposal was valid, but spending shielded outputs of prior transaction steps is not yet supported." - ) - } + Error::ProposalNotSupported => write!( + f, + "The proposal was valid but tried to do something that is not supported \ + (spend shielded outputs of prior transaction steps or use a feature that \ + is not enabled).", + ), Error::KeyNotRecognized => { write!( f, @@ -158,6 +165,10 @@ where Error::AddressNotRecognized(_) => { write!(f, "The specified transparent address was not recognized as belonging to the wallet.") } + #[cfg(feature = "transparent-inputs")] + Error::PaysEphemeralTransparentAddress(addr) => { + write!(f, "The wallet tried to pay to an ephemeral transparent address as a normal output: {}", addr) + } } } } @@ -187,6 +198,12 @@ impl From> for Error { } } +impl From for Error { + fn from(e: ProposalError) -> Self { + Error::Proposal(e) + } +} + impl From for Error { fn from(e: BalanceError) -> Self { Error::BalanceError(e) diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 2518998906..29486b54b4 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -51,16 +51,19 @@ use crate::{ decrypt_transaction, fees::{self, DustOutputPolicy}, keys::UnifiedSpendingKey, - proposal::{self, Proposal, ProposalError}, + proposal::{Proposal, ProposalError, Step, StepOutputIndex}, wallet::{Note, OvkPolicy, Recipient}, zip321::{self, Payment}, PoolType, ShieldedProtocol, }; -use zcash_primitives::transaction::{ - builder::{BuildConfig, BuildResult, Builder}, - components::{amount::NonNegativeAmount, sapling::zip212_enforcement}, - fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule}, - Transaction, TxId, +use zcash_primitives::{ + legacy::TransparentAddress, + transaction::{ + builder::{BuildConfig, BuildResult, Builder}, + components::{amount::NonNegativeAmount, sapling::zip212_enforcement, OutPoint}, + fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule}, + Transaction, TxId, + }, }; use zcash_protocol::{ consensus::{self, BlockHeight, NetworkUpgrade}, @@ -70,11 +73,12 @@ use zip32::Scope; #[cfg(feature = "transparent-inputs")] use { + crate::{fees::ChangeValue, proposal::StepOutput, wallet::TransparentAddressMetadata}, + core::convert::Infallible, input_selection::ShieldingSelector, - std::convert::Infallible, + std::collections::HashMap, zcash_keys::encoding::AddressCodec, - zcash_primitives::legacy::TransparentAddress, - zcash_primitives::transaction::components::{OutPoint, TxOut}, + zcash_primitives::transaction::components::TxOut, }; pub mod input_selection; @@ -597,6 +601,12 @@ where ParamsT: consensus::Parameters + Clone, FeeRuleT: FeeRule, { + // The set of transparent `StepOutput`s available and unused from prior steps. + // When a transparent `StepOutput` is created, it is added to the map. When it + // is consumed, it is removed from the map. + #[cfg(feature = "transparent-inputs")] + let mut unused_transparent_outputs = HashMap::new(); + let mut step_results = Vec::with_capacity(proposal.steps().len()); for step in proposal.steps() { let step_result = create_proposed_transaction( @@ -610,10 +620,23 @@ where proposal.min_target_height(), &step_results, step, + #[cfg(feature = "transparent-inputs")] + &mut unused_transparent_outputs, )?; step_results.push((step, step_result)); } + // Ephemeral outputs must be referenced exactly once. + #[cfg(feature = "transparent-inputs")] + for so in unused_transparent_outputs.into_keys() { + if let StepOutputIndex::Change(i) = so.output_index() { + // references have already been checked + if step_results[so.step_index()].0.balance().proposed_change()[i].is_ephemeral() { + return Err(ProposalError::EphemeralOutputLeftUnspent(so).into()); + } + } + } + Ok(NonEmpty::from_vec( step_results .iter() @@ -623,6 +646,9 @@ where .expect("proposal.steps is NonEmpty")) } +// `unused_transparent_outputs` maps `StepOutput`s for transparent outputs +// that have not been consumed so far, to the corresponding pair of +// `TransparentAddress` and `Outpoint`. #[allow(clippy::too_many_arguments)] #[allow(clippy::type_complexity)] fn create_proposed_transaction( @@ -634,48 +660,55 @@ fn create_proposed_transaction( ovk_policy: OvkPolicy, fee_rule: &FeeRuleT, min_target_height: BlockHeight, - prior_step_results: &[(&proposal::Step, BuildResult)], - proposal_step: &proposal::Step, + prior_step_results: &[(&Step, BuildResult)], + proposal_step: &Step, + #[cfg(feature = "transparent-inputs")] unused_transparent_outputs: &mut HashMap< + StepOutput, + (TransparentAddress, OutPoint), + >, ) -> Result> where DbT: WalletWrite + WalletCommitmentTrees, ParamsT: consensus::Parameters + Clone, FeeRuleT: FeeRule, { - // TODO: Spending shielded outputs of prior multi-step transaction steps is not yet - // supported. Maybe support this at some point? Doing so would require a higher-level - // approach in the wallet that waits for transactions with shielded outputs to be - // mined and only then attempts to perform the next step. - for s_ref in proposal_step.prior_step_inputs() { - prior_step_results.get(s_ref.step_index()).map_or_else( - || { - // Return an error in case the step index doesn't match up with a step - Err(Error::Proposal(ProposalError::ReferenceError(*s_ref))) - }, - |step| match s_ref.output_index() { - proposal::StepOutputIndex::Payment(i) => { - let prior_pool = step - .0 - .payment_pools() - .get(&i) - .ok_or(Error::Proposal(ProposalError::ReferenceError(*s_ref)))?; - - if matches!(prior_pool, PoolType::Shielded(_)) { - Err(Error::ProposalNotSupported) - } else { - Ok(()) - } - } - proposal::StepOutputIndex::Change(_) => { - // Only shielded change is supported by zcash_client_backend, so multi-step - // transactions cannot yet spend prior transactions' change outputs. - Err(Error::ProposalNotSupported) + #[cfg(feature = "transparent-inputs")] + let step_index = prior_step_results.len(); + + // We only support spending transparent payments or transparent ephemeral outputs from a + // prior step (when "transparent-inputs" is enabled). + // + // TODO: Maybe support spending prior shielded outputs at some point? Doing so would require + // a higher-level approach in the wallet that waits for transactions with shielded outputs to + // be mined and only then attempts to perform the next step. + #[allow(clippy::never_loop)] + for input_ref in proposal_step.prior_step_inputs() { + let (prior_step, _) = prior_step_results + .get(input_ref.step_index()) + .ok_or(ProposalError::ReferenceError(*input_ref))?; + + #[allow(unused_variables)] + let output_pool = match input_ref.output_index() { + StepOutputIndex::Payment(i) => prior_step.payment_pools().get(&i).cloned(), + StepOutputIndex::Change(i) => match prior_step.balance().proposed_change().get(i) { + Some(change) if !change.is_ephemeral() => { + return Err(ProposalError::SpendsChange(*input_ref).into()); } + other => other.map(|change| change.output_pool()), }, - )?; + } + .ok_or(ProposalError::ReferenceError(*input_ref))?; + + // Return an error on trying to spend a prior output that is not supported. + #[cfg(feature = "transparent-inputs")] + if output_pool != PoolType::TRANSPARENT { + return Err(Error::ProposalNotSupported); + } + #[cfg(not(feature = "transparent-inputs"))] + return Err(Error::ProposalNotSupported); } - let account = wallet_db + let account_id = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) .map_err(Error::DataSource)? .ok_or(Error::KeyNotRecognized)? @@ -762,7 +795,7 @@ where let orchard_anchor = None; // Create the transaction. The type of the proposal ensures that there - // are no possible transparent inputs, so we ignore those + // are no possible transparent inputs, so we ignore those here. let mut builder = Builder::new( params.clone(), min_target_height, @@ -772,6 +805,11 @@ where }, ); + #[cfg(all(feature = "transparent-inputs", not(feature = "orchard")))] + let has_shielded_inputs = !sapling_inputs.is_empty(); + #[cfg(all(feature = "transparent-inputs", feature = "orchard"))] + let has_shielded_inputs = !(sapling_inputs.is_empty() && orchard_inputs.is_empty()); + for (sapling_key, sapling_note, merkle_path) in sapling_inputs.into_iter() { builder.add_sapling_spend(&sapling_key, sapling_note.clone(), merkle_path)?; } @@ -782,94 +820,86 @@ where } #[cfg(feature = "transparent-inputs")] - let utxos_spent = { - let known_addrs = wallet_db - .get_transparent_receivers(account) - .map_err(Error::DataSource)?; + let mut cache = HashMap::::new(); - let mut utxos_spent: Vec = vec![]; - let mut add_transparent_input = |addr: &TransparentAddress, - outpoint: OutPoint, - utxo: TxOut| - -> Result< - (), - Error< - ::Error, - ::Error, - InputsErrT, - FeeRuleT::Error, - >, - > { - let address_metadata = known_addrs - .get(addr) - .ok_or(Error::AddressNotRecognized(*addr))? - .clone() - .ok_or_else(|| Error::NoSpendingKey(addr.encode(params)))?; + #[cfg(feature = "transparent-inputs")] + let mut metadata_from_address = |addr: TransparentAddress| -> Result< + TransparentAddressMetadata, + ErrorT, + > { + match cache.get(&addr) { + Some(result) => Ok(result.clone()), + None => { + // `wallet_db.get_transparent_address_metadata` includes reserved ephemeral + // addresses in its lookup. We don't need to include these in order to be + // able to construct ZIP 320 transactions, because in that case the ephemeral + // output is represented via a "change" reference to a previous step. However, + // we do need them in order to create a transaction from a proposal that + // explicitly spends an output from an ephemeral address (only for outputs + // already detected by this wallet instance). + + let result = wallet_db + .get_transparent_address_metadata(account_id, &addr) + .map_err(InputSelectorError::DataSource)? + .ok_or(Error::AddressNotRecognized(addr))?; + cache.insert(addr, result.clone()); + Ok(result) + } + } + }; + #[cfg(feature = "transparent-inputs")] + let utxos_spent = { + let mut utxos_spent: Vec = vec![]; + let add_transparent_input = |builder: &mut Builder<_, _>, + utxos_spent: &mut Vec<_>, + address_metadata: &TransparentAddressMetadata, + outpoint: OutPoint, + txout: TxOut| + -> Result<(), ErrorT> { let secret_key = usk .transparent() .derive_secret_key(address_metadata.scope(), address_metadata.address_index()) - .unwrap(); + .expect("spending key derivation should not fail"); utxos_spent.push(outpoint.clone()); - builder.add_transparent_input(secret_key, outpoint, utxo)?; + builder.add_transparent_input(secret_key, outpoint, txout)?; Ok(()) }; for utxo in proposal_step.transparent_inputs() { add_transparent_input( - utxo.recipient_address(), + &mut builder, + &mut utxos_spent, + &metadata_from_address(*utxo.recipient_address())?, utxo.outpoint().clone(), utxo.txout().clone(), )?; } for input_ref in proposal_step.prior_step_inputs() { - match input_ref.output_index() { - proposal::StepOutputIndex::Payment(i) => { - // We know based upon the earlier check that this must be a transparent input, - // We also know that transparent outputs for that previous step were added to - // the transaction in payment index order, so we can use dead reckoning to - // figure out which output it ended up being. - let (prior_step, result) = &prior_step_results[input_ref.step_index()]; - let recipient_address = &prior_step - .transaction_request() - .payments() - .get(&i) - .expect("Payment step references are checked at construction") - .recipient_address() - .clone() - .convert_if_network(params.network_type())?; - - let recipient_taddr = match recipient_address { - Address::Transparent(t) => Some(t), - Address::Unified(uaddr) => uaddr.transparent(), - _ => None, - } - .ok_or(Error::ProposalNotSupported)?; - let outpoint = OutPoint::new( - result.transaction().txid().into(), - u32::try_from( - prior_step - .payment_pools() - .iter() - .filter(|(_, pool)| pool == &&PoolType::Transparent) - .take_while(|(j, _)| j <= &&i) - .count() - - 1, - ) - .expect("Transparent output index fits into a u32"), - ); - let utxo = &result - .transaction() - .transparent_bundle() - .ok_or(Error::Proposal(ProposalError::ReferenceError(*input_ref)))? - .vout[outpoint.n() as usize]; - - add_transparent_input(recipient_taddr, outpoint, utxo.clone())?; - } - proposal::StepOutputIndex::Change(_) => unreachable!(), - } + // A referenced transparent step output must exist and be referenced *at most* once. + // (Exactly once in the case of ephemeral outputs.) + let (address, outpoint) = unused_transparent_outputs + .remove(input_ref) + .ok_or(Error::Proposal(ProposalError::ReferenceError(*input_ref)))?; + + let address_metadata = metadata_from_address(address)?; + + let txout = &prior_step_results[input_ref.step_index()] + .1 + .transaction() + .transparent_bundle() + .ok_or(ProposalError::ReferenceError(*input_ref))? + .vout[outpoint.n() as usize]; + + add_transparent_input( + &mut builder, + &mut utxos_spent, + &address_metadata, + outpoint, + txout.clone(), + )?; } utxos_spent }; @@ -923,107 +953,134 @@ where }; #[cfg(feature = "orchard")] - let mut orchard_output_meta = vec![]; - let mut sapling_output_meta = vec![]; - let mut transparent_output_meta = vec![]; - for (payment, output_pool) in proposal_step - .payment_pools() - .iter() - .map(|(idx, output_pool)| { - let payment = proposal_step - .transaction_request() - .payments() - .get(idx) - .expect( - "The mapping between payment index and payment is checked in step construction", - ); - (payment, output_pool) - }) - { - let recipient_address: Address = payment - .recipient_address() + let mut orchard_output_meta: Vec<( + Recipient<_, PoolType, _>, + NonNegativeAmount, + Option, + )> = vec![]; + let mut sapling_output_meta: Vec<( + Recipient<_, PoolType, _>, + NonNegativeAmount, + Option, + )> = vec![]; + let mut transparent_output_meta: Vec<( + Recipient<_, _, ()>, + TransparentAddress, + NonNegativeAmount, + StepOutputIndex, + )> = vec![]; + + for (&payment_index, output_pool) in proposal_step.payment_pools() { + let payment = proposal_step + .transaction_request() + .payments() + .get(&payment_index) + .expect( + "The mapping between payment index and payment is checked in step construction", + ); + let recipient_address = payment.recipient_address(); + + let add_sapling_output = |builder: &mut Builder<_, _>, + sapling_output_meta: &mut Vec<_>, + to: sapling::PaymentAddress| + -> Result<(), ErrorT> { + let memo = payment.memo().map_or_else(MemoBytes::empty, |m| m.clone()); + builder.add_sapling_output(sapling_external_ovk, to, payment.amount(), memo.clone())?; + sapling_output_meta.push(( + Recipient::External(recipient_address.clone(), PoolType::SAPLING), + payment.amount(), + Some(memo), + )); + Ok(()) + }; + + #[cfg(feature = "orchard")] + let add_orchard_output = |builder: &mut Builder<_, _>, + orchard_output_meta: &mut Vec<_>, + to: orchard::Address| + -> Result<(), ErrorT> { + let memo = payment.memo().map_or_else(MemoBytes::empty, |m| m.clone()); + builder.add_orchard_output( + orchard_external_ovk.clone(), + to, + payment.amount().into(), + memo.clone(), + )?; + orchard_output_meta.push(( + Recipient::External(recipient_address.clone(), PoolType::ORCHARD), + payment.amount(), + Some(memo), + )); + Ok(()) + }; + + let add_transparent_output = |builder: &mut Builder<_, _>, + transparent_output_meta: &mut Vec<_>, + to: TransparentAddress| + -> Result<(), ErrorT> { + // Always reject sending to one of our known ephemeral addresses. + #[cfg(feature = "transparent-inputs")] + if wallet_db + .find_account_for_ephemeral_address(&to) + .map_err(Error::DataSource)? + .is_some() + { + return Err(Error::PaysEphemeralTransparentAddress(to.encode(params))); + } + if payment.memo().is_some() { + return Err(Error::MemoForbidden); + } + builder.add_transparent_output(&to, payment.amount())?; + transparent_output_meta.push(( + Recipient::External(recipient_address.clone(), PoolType::TRANSPARENT), + to, + payment.amount(), + StepOutputIndex::Payment(payment_index), + )); + Ok(()) + }; + + match recipient_address .clone() - .convert_if_network(params.network_type())?; - - match recipient_address { - Address::Unified(ua) => { - let memo = payment.memo().map_or_else(MemoBytes::empty, |m| m.clone()); - - match output_pool { - #[cfg(not(feature = "orchard"))] - PoolType::Shielded(ShieldedProtocol::Orchard) => { - return Err(Error::ProposalNotSupported); - } - #[cfg(feature = "orchard")] - PoolType::Shielded(ShieldedProtocol::Orchard) => { - builder.add_orchard_output( - orchard_external_ovk.clone(), - *ua.orchard().expect("The mapping between payment pool and receiver is checked in step construction"), - payment.amount().into(), - memo.clone(), - )?; - orchard_output_meta.push(( - Recipient::External(payment.recipient_address().clone(), *output_pool), - payment.amount(), - Some(memo), - )); - } - - PoolType::Shielded(ShieldedProtocol::Sapling) => { - builder.add_sapling_output( - sapling_external_ovk, - *ua.sapling().expect("The mapping between payment pool and receiver is checked in step construction"), - payment.amount(), - memo.clone(), - )?; - sapling_output_meta.push(( - Recipient::External(payment.recipient_address().clone(), *output_pool), - payment.amount(), - Some(memo), - )); - } - - PoolType::Transparent => { - if payment.memo().is_some() { - return Err(Error::MemoForbidden); - } else { - builder.add_transparent_output( - ua.transparent().expect("The mapping between payment pool and receiver is checked in step construction."), - payment.amount() - )?; - } - } + .convert_if_network(params.network_type())? + { + Address::Unified(ua) => match output_pool { + #[cfg(not(feature = "orchard"))] + PoolType::Shielded(ShieldedProtocol::Orchard) => { + return Err(Error::ProposalNotSupported); } - } - Address::Sapling(addr) => { - let memo = payment.memo().map_or_else(MemoBytes::empty, |m| m.clone()); - builder.add_sapling_output( - sapling_external_ovk, - addr, - payment.amount(), - memo.clone(), - )?; - sapling_output_meta.push(( - Recipient::External(payment.recipient_address().clone(), PoolType::SAPLING), - payment.amount(), - Some(memo), - )); + #[cfg(feature = "orchard")] + PoolType::Shielded(ShieldedProtocol::Orchard) => { + let to = *ua.orchard().expect("The mapping between payment pool and receiver is checked in step construction"); + add_orchard_output(&mut builder, &mut orchard_output_meta, to)?; + } + PoolType::Shielded(ShieldedProtocol::Sapling) => { + let to = *ua.sapling().expect("The mapping between payment pool and receiver is checked in step construction"); + add_sapling_output(&mut builder, &mut sapling_output_meta, to)?; + } + PoolType::Transparent => { + let to = *ua.transparent().expect("The mapping between payment pool and receiver is checked in step construction"); + add_transparent_output(&mut builder, &mut transparent_output_meta, to)?; + } + }, + Address::Sapling(to) => { + add_sapling_output(&mut builder, &mut sapling_output_meta, to)?; } Address::Transparent(to) => { - if payment.memo().is_some() { - return Err(Error::MemoForbidden); - } else { - builder.add_transparent_output(&to, payment.amount())?; - } - transparent_output_meta.push(( - Recipient::External(payment.recipient_address().clone(), PoolType::TRANSPARENT), - to, - payment.amount(), - )); + add_transparent_output(&mut builder, &mut transparent_output_meta, to)?; } + #[cfg(not(feature = "transparent-inputs"))] Address::Tex(_) => { return Err(Error::ProposalNotSupported); } + #[cfg(feature = "transparent-inputs")] + Address::Tex(data) => { + if has_shielded_inputs { + return Err(ProposalError::PaysTexFromShielded.into()); + } + let to = TransparentAddress::PublicKeyHash(data); + add_transparent_output(&mut builder, &mut transparent_output_meta, to)?; + } } } @@ -1042,7 +1099,7 @@ where )?; sapling_output_meta.push(( Recipient::InternalAccount { - receiving_account: account, + receiving_account: account_id, external_address: None, note: output_pool, }, @@ -1064,7 +1121,7 @@ where )?; orchard_output_meta.push(( Recipient::InternalAccount { - receiving_account: account, + receiving_account: account_id, external_address: None, note: output_pool, }, @@ -1074,11 +1131,52 @@ where } } PoolType::Transparent => { + #[cfg(not(feature = "transparent-inputs"))] return Err(Error::UnsupportedChangeType(output_pool)); } } } + // This reserves the ephemeral addresses even if transaction construction fails. + // It is not worth the complexity of being able to unreserve them, because there + // are few failure modes after this point that would allow us to do so. + #[cfg(feature = "transparent-inputs")] + { + let ephemeral_outputs: Vec<(usize, &ChangeValue)> = proposal_step + .balance() + .proposed_change() + .iter() + .enumerate() + .filter(|(_, change_value)| { + change_value.is_ephemeral() && change_value.output_pool() == PoolType::Transparent + }) + .collect(); + + let addresses_and_metadata = wallet_db + .reserve_next_n_ephemeral_addresses(account_id, ephemeral_outputs.len()) + .map_err(Error::DataSource)?; + assert_eq!(addresses_and_metadata.len(), ephemeral_outputs.len()); + + // We don't need the TransparentAddressMetadata here; we can look it up from the data source later. + for ((change_index, change_value), (ephemeral_address, _)) in + ephemeral_outputs.iter().zip(addresses_and_metadata) + { + // This output is ephemeral; we will report an error in `create_proposed_transactions` + // if a later step does not consume it. + builder.add_transparent_output(&ephemeral_address, change_value.value())?; + transparent_output_meta.push(( + Recipient::EphemeralTransparent { + receiving_account: account_id, + ephemeral_address, + outpoint_metadata: (), + }, + ephemeral_address, + change_value.value(), + StepOutputIndex::Change(*change_index), + )) + } + } + // Build the transaction with the specified fee rule let build_result = builder.build(OsRng, spend_prover, output_prover, fee_rule)?; @@ -1146,29 +1244,35 @@ where SentTransactionOutput::from_parts(output_index, recipient, value, memo) }); - let transparent_outputs = - transparent_output_meta - .into_iter() - .map(|(recipient, addr, value)| { - let script = addr.script(); - let output_index = build_result - .transaction() - .transparent_bundle() - .and_then(|b| { - b.vout - .iter() - .enumerate() - .find(|(_, tx_out)| tx_out.script_pubkey == script) - }) - .map(|(index, _)| index) - .expect( - "An output should exist in the transaction for each transparent payment.", - ); + let txid: [u8; 32] = build_result.transaction().txid().into(); + assert_eq!( + transparent_output_meta.len(), + build_result + .transaction() + .transparent_bundle() + .map_or(0, |b| b.vout.len()), + ); - SentTransactionOutput::from_parts(output_index, recipient, value, None) - }); + #[allow(unused_variables)] + let transparent_outputs = transparent_output_meta.into_iter().enumerate().map( + |(n, (recipient, address, value, step_output_index))| { + // This assumes that transparent outputs are pushed onto `transparent_output_meta` + // with the same indices they have in the transaction's transparent outputs. + // We do not reorder transparent outputs; there is no reason to do so because it + // would not usefully improve privacy. + let outpoint = OutPoint::new(txid, n as u32); + + let recipient = recipient.map_ephemeral_transparent_outpoint(|()| outpoint.clone()); + #[cfg(feature = "transparent-inputs")] + unused_transparent_outputs.insert( + StepOutput::new(step_index, step_output_index), + (address, outpoint), + ); + SentTransactionOutput::from_parts(n, recipient, value, None) + }, + ); - let mut outputs = vec![]; + let mut outputs: Vec> = vec![]; #[cfg(feature = "orchard")] outputs.extend(orchard_outputs); outputs.extend(sapling_outputs); @@ -1178,7 +1282,7 @@ where .store_sent_tx(&SentTransaction { tx: build_result.transaction(), created: time::OffsetDateTime::now_utc(), - account, + account: account_id, outputs, fee_amount: proposal_step.balance().fee_required(), #[cfg(feature = "transparent-inputs")] diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index e6579aa031..ad9b56a03a 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -32,9 +32,14 @@ use crate::{ #[cfg(feature = "transparent-inputs")] use { - std::collections::BTreeSet, std::convert::Infallible, - zcash_primitives::legacy::TransparentAddress, - zcash_primitives::transaction::components::OutPoint, + crate::{ + fees::EphemeralBalance, + proposal::{Step, StepOutput, StepOutputIndex}, + zip321::Payment, + }, + std::collections::BTreeSet, + std::convert::Infallible, + zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}, }; #[cfg(feature = "orchard")] @@ -323,6 +328,11 @@ pub struct GreedyInputSelector { impl GreedyInputSelector { /// Constructs a new greedy input selector that uses the provided change strategy to determine /// change values and fee amounts. + /// + /// The [`ChangeStrategy`] provided must produce exactly one ephemeral change value when + /// computing a transaction balance if an [`EphemeralBalance::Output`] value is provided for + /// its ephemeral balance, or the resulting [`GreedyInputSelector`] will return an error when + /// attempting to construct a transaction proposal that requires such an output. pub fn new(change_strategy: ChangeT, dust_output_policy: DustOutputPolicy) -> Self { GreedyInputSelector { change_strategy, @@ -364,6 +374,22 @@ where #[cfg(feature = "orchard")] let mut orchard_outputs = vec![]; let mut payment_pools = BTreeMap::new(); + + // In a ZIP 320 pair, tr0 refers to the first transaction request that + // collects shielded value and sends it to an ephemeral address, and tr1 + // refers to the second transaction request that pays the TEX addresses. + #[cfg(feature = "transparent-inputs")] + let mut tr1_transparent_outputs = vec![]; + #[cfg(feature = "transparent-inputs")] + let mut tr1_payments = vec![]; + #[cfg(feature = "transparent-inputs")] + let mut tr1_payment_pools = BTreeMap::new(); + // This balance value is just used for overflow checking; the actual value of ephemeral + // outputs will be computed from the constructed `tr1_transparent_outputs` value + // constructed below. + #[cfg(feature = "transparent-inputs")] + let mut total_ephemeral = NonNegativeAmount::ZERO; + for (idx, payment) in transaction_request.payments() { let recipient_address: Address = payment .recipient_address() @@ -378,6 +404,30 @@ where script_pubkey: addr.script(), }); } + #[cfg(feature = "transparent-inputs")] + Address::Tex(data) => { + let p2pkh_addr = TransparentAddress::PublicKeyHash(data); + + tr1_payment_pools.insert(*idx, PoolType::TRANSPARENT); + tr1_transparent_outputs.push(TxOut { + value: payment.amount(), + script_pubkey: p2pkh_addr.script(), + }); + tr1_payments.push( + Payment::new( + payment.recipient_address().clone(), + payment.amount(), + None, + payment.label().cloned(), + payment.message().cloned(), + payment.other_params().to_vec(), + ) + .expect("cannot fail because memo is None"), + ); + total_ephemeral = (total_ephemeral + payment.amount()) + .ok_or_else(|| GreedyInputSelectorError::Balance(BalanceError::Overflow))?; + } + #[cfg(not(feature = "transparent-inputs"))] Address::Tex(_) => { return Err(InputSelectorError::Selection( GreedyInputSelectorError::UnsupportedTexAddress, @@ -417,10 +467,65 @@ where } } + #[cfg(not(feature = "transparent-inputs"))] + let ephemeral_balance = None; + + #[cfg(feature = "transparent-inputs")] + let (ephemeral_balance, tr1_balance_opt) = { + if tr1_transparent_outputs.is_empty() { + (None, None) + } else { + // The ephemeral input going into transaction 1 must be able to pay that + // transaction's fee, as well as the TEX address payments. + + // First compute the required total with an additional zero input, + // catching the `InsufficientFunds` error to obtain the required amount + // given the provided change strategy. Ignore the change memo in order + // to avoid adding a change output. + let tr1_required_input_value = + match self.change_strategy.compute_balance::<_, DbT::NoteRef>( + params, + target_height, + &[] as &[WalletTransparentOutput], + &tr1_transparent_outputs, + &sapling::EmptyBundleView, + #[cfg(feature = "orchard")] + &orchard_fees::EmptyBundleView, + &self.dust_output_policy, + Some(&EphemeralBalance::Input(NonNegativeAmount::ZERO)), + ) { + Err(ChangeError::InsufficientFunds { required, .. }) => required, + Ok(_) => NonNegativeAmount::ZERO, // shouldn't happen + Err(other) => return Err(other.into()), + }; + + // Now recompute to obtain the `TransactionBalance` and verify that it + // fully accounts for the required fees. + let tr1_balance = self.change_strategy.compute_balance::<_, DbT::NoteRef>( + params, + target_height, + &[] as &[WalletTransparentOutput], + &tr1_transparent_outputs, + &sapling::EmptyBundleView, + #[cfg(feature = "orchard")] + &orchard_fees::EmptyBundleView, + &self.dust_output_policy, + Some(&EphemeralBalance::Input(tr1_required_input_value)), + )?; + assert_eq!(tr1_balance.total(), tr1_balance.fee_required()); + + ( + Some(EphemeralBalance::Output(tr1_required_input_value)), + Some(tr1_balance), + ) + } + }; + let mut shielded_inputs = SpendableNotes::empty(); let mut prior_available = NonNegativeAmount::ZERO; let mut amount_required = NonNegativeAmount::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. @@ -434,12 +539,12 @@ where shielded_inputs.orchard_value()?, ); - // Use Sapling inputs if there are no Orchard outputs or there are not sufficient - // Orchard outputs to cover the amount required. + // Use Sapling inputs if there are no Orchard outputs or if there are insufficient + // funds from Orchard inputs to cover the amount required. let use_sapling = orchard_outputs.is_empty() || amount_required > orchard_input_total; - // Use Orchard inputs if there are insufficient Sapling funds to cover the amount - // reqiuired. + // Use Orchard inputs if there are insufficient funds from Sapling inputs to cover + // the amount required. let use_orchard = !use_sapling || amount_required > sapling_input_total; (use_sapling, use_orchard) @@ -466,10 +571,12 @@ where vec![] }; + // In the ZIP 320 case, this is the balance for transaction 0, taking into account + // the ephemeral output. let balance = self.change_strategy.compute_balance( params, target_height, - &Vec::::new(), + &[] as &[WalletTransparentOutput], &transparent_outputs, &( ::sapling::builder::BundleType::DEFAULT, @@ -483,22 +590,109 @@ where &orchard_outputs[..], ), &self.dust_output_policy, + ephemeral_balance.as_ref(), ); match balance { Ok(balance) => { - return Proposal::single_step( - transaction_request, - payment_pools, - vec![], + // At this point, we have enough input value to pay for everything, so we will + // return at the end of this block. + + let shielded_inputs = NonEmpty::from_vec(shielded_inputs.into_vec(&SimpleNoteRetention { sapling: use_sapling, #[cfg(feature = "orchard")] orchard: use_orchard, })) - .map(|notes| ShieldedInputs::from_parts(anchor_height, notes)), + .map(|notes| ShieldedInputs::from_parts(anchor_height, notes)); + + #[cfg(feature = "transparent-inputs")] + if let Some(tr1_balance) = tr1_balance_opt { + // Construct two new `TransactionRequest`s: + // * `tr0` excludes the TEX outputs, and in their place includes + // a single additional ephemeral output to the transparent pool. + // * `tr1` spends from that ephemeral output to each TEX output. + + // Find exactly one ephemeral change output. + let ephemeral_outputs = balance + .proposed_change() + .iter() + .enumerate() + .filter(|(_, c)| c.is_ephemeral()) + .collect::>(); + + let ephemeral_value = ephemeral_balance + .and_then(|b| b.ephemeral_output_amount()) + .expect("ephemeral output balance exists (constructed above)"); + + let ephemeral_output_index = match &ephemeral_outputs[..] { + [(i, change_value)] if change_value.value() == ephemeral_value => { + Ok(*i) + } + _ => Err(InputSelectorError::Proposal( + ProposalError::EphemeralOutputsInvalid, + )), + }?; + + let ephemeral_stepoutput = + StepOutput::new(0, StepOutputIndex::Change(ephemeral_output_index)); + + let tr0 = TransactionRequest::from_indexed( + transaction_request + .payments() + .iter() + .filter(|(idx, _payment)| !tr1_payment_pools.contains_key(idx)) + .map(|(k, v)| (*k, v.clone())) + .collect(), + ) + .expect("removing payments from a TransactionRequest preserves validity"); + + let mut steps = vec![]; + steps.push( + Step::from_parts( + &[], + tr0, + payment_pools, + vec![], + shielded_inputs, + vec![], + balance, + false, + ) + .map_err(InputSelectorError::Proposal)?, + ); + + let tr1 = + TransactionRequest::new(tr1_payments).expect("valid by construction"); + steps.push( + Step::from_parts( + &steps, + tr1, + tr1_payment_pools, + vec![], + None, + vec![ephemeral_stepoutput], + tr1_balance, + false, + ) + .map_err(InputSelectorError::Proposal)?, + ); + + return Proposal::multi_step( + self.change_strategy.fee_rule().clone(), + target_height, + NonEmpty::from_vec(steps).expect("steps is known to be nonempty"), + ) + .map_err(InputSelectorError::Proposal); + } + + return Proposal::single_step( + transaction_request, + payment_pools, + vec![], + shielded_inputs, balance, - (*self.change_strategy.fee_rule()).clone(), + self.change_strategy.fee_rule().clone(), target_height, false, ) @@ -592,11 +786,12 @@ where params, target_height, &transparent_inputs, - &Vec::::new(), + &[] as &[TxOut], &sapling::EmptyBundleView, #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, &self.dust_output_policy, + None, ); let balance = match trial_balance { @@ -609,11 +804,12 @@ where params, target_height, &transparent_inputs, - &Vec::::new(), + &[] as &[TxOut], &sapling::EmptyBundleView, #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, &self.dust_output_policy, + None, )? } Err(other) => { diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index f4fcfc2312..fea9424243 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -21,35 +21,28 @@ pub mod sapling; pub mod standard; pub mod zip317; -/// A proposed change amount and output pool. +/// `ChangeValue` represents either a proposed change output to a shielded pool +/// (with an optional change memo), or if the "transparent-inputs" feature is +/// enabled, an ephemeral output to the transparent pool. #[derive(Clone, Debug, PartialEq, Eq)] -pub struct ChangeValue { - output_pool: PoolType, - value: NonNegativeAmount, - memo: Option, -} +pub struct ChangeValue(ChangeValueInner); -impl ChangeValue { - /// Constructs a new change value from its constituent parts. - pub fn new( - output_pool: PoolType, +#[derive(Clone, Debug, PartialEq, Eq)] +enum ChangeValueInner { + Shielded { + protocol: ShieldedProtocol, value: NonNegativeAmount, memo: Option, - ) -> Option { - (matches!(output_pool, PoolType::Shielded(_)) || memo.is_none()).then_some(Self { - output_pool, - value, - memo, - }) - } + }, + #[cfg(feature = "transparent-inputs")] + EphemeralTransparent { value: NonNegativeAmount }, +} - /// Constructs a new change value that will be created as a transparent output. - pub fn transparent(value: NonNegativeAmount) -> Self { - Self { - output_pool: PoolType::TRANSPARENT, - value, - memo: None, - } +impl ChangeValue { + /// Constructs a new ephemeral transparent output value. + #[cfg(feature = "transparent-inputs")] + pub fn ephemeral_transparent(value: NonNegativeAmount) -> Self { + Self(ChangeValueInner::EphemeralTransparent { value }) } /// Constructs a new change value that will be created as a shielded output. @@ -58,11 +51,11 @@ impl ChangeValue { value: NonNegativeAmount, memo: Option, ) -> Self { - Self { - output_pool: PoolType::Shielded(protocol), + Self(ChangeValueInner::Shielded { + protocol, value, memo, - } + }) } /// Constructs a new change value that will be created as a Sapling output. @@ -76,19 +69,45 @@ impl ChangeValue { Self::shielded(ShieldedProtocol::Orchard, value, memo) } - /// Returns the pool to which the change output should be sent. + /// Returns the pool to which the change or ephemeral output should be sent. pub fn output_pool(&self) -> PoolType { - self.output_pool + match &self.0 { + ChangeValueInner::Shielded { protocol, .. } => PoolType::Shielded(*protocol), + #[cfg(feature = "transparent-inputs")] + ChangeValueInner::EphemeralTransparent { .. } => PoolType::Transparent, + } } - /// Returns the value of the change output to be created, in zatoshis. + /// Returns the value of the change or ephemeral output to be created, in zatoshis. pub fn value(&self) -> NonNegativeAmount { - self.value + match &self.0 { + ChangeValueInner::Shielded { value, .. } => *value, + #[cfg(feature = "transparent-inputs")] + ChangeValueInner::EphemeralTransparent { value } => *value, + } } - /// Returns the memo to be associated with the change output. + /// Returns the memo to be associated with the output. pub fn memo(&self) -> Option<&MemoBytes> { - self.memo.as_ref() + match &self.0 { + ChangeValueInner::Shielded { memo, .. } => memo.as_ref(), + #[cfg(feature = "transparent-inputs")] + ChangeValueInner::EphemeralTransparent { .. } => None, + } + } + + /// Whether this is to be an ephemeral output. + #[cfg_attr( + not(feature = "transparent-inputs"), + doc = "This is always false because the `transparent-inputs` feature is + not enabled." + )] + pub fn is_ephemeral(&self) -> bool { + match &self.0 { + ChangeValueInner::Shielded { .. } => false, + #[cfg(feature = "transparent-inputs")] + ChangeValueInner::EphemeralTransparent { .. } => true, + } } } @@ -154,15 +173,25 @@ pub enum ChangeError { /// including the required fees. required: NonNegativeAmount, }, - /// 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.) + /// Some of the inputs provided to the transaction have value less than the + /// marginal fee, and could not be determined to have any economic value in + /// the context of this input selection. + /// + /// This determination is potentially conservative in the sense that inputs + /// with value less than or equal to the marginal fee might be excluded, even + /// though in practice they would not cause the fee to increase. Inputs with + /// value greater than the marginal fee will never be excluded. + /// + /// The ordering of the inputs in each list is unspecified. DustInputs { - /// The outpoints corresponding to transparent inputs having no current economic value. + /// The outpoints for transparent inputs that could not be determined to + /// have economic value in the context of this input selection. transparent: Vec, - /// The identifiers for Sapling inputs having no current economic value + /// The identifiers for Sapling inputs that could not be determined to + /// have economic value in the context of this input selection. sapling: Vec, - /// The identifiers for Orchard inputs having no current economic value + /// The identifiers for Orchard inputs that could not be determined to + /// have economic value in the context of this input selection. #[cfg(feature = "orchard")] orchard: Vec, }, @@ -301,6 +330,41 @@ impl Default for DustOutputPolicy { } } +/// `EphemeralBalance` describes the ephemeral input or output value for a transaction. It is used +/// in fee computation for series of transactions that use an ephemeral transparent output in an +/// intermediate step, such as when sending from a shielded pool to a [ZIP 320] "TEX" address. +/// +/// [ZIP 320]: https://zips.z.cash/zip-0320 +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum EphemeralBalance { + Input(NonNegativeAmount), + Output(NonNegativeAmount), +} + +impl EphemeralBalance { + pub fn is_input(&self) -> bool { + matches!(self, EphemeralBalance::Input(_)) + } + + pub fn is_output(&self) -> bool { + matches!(self, EphemeralBalance::Output(_)) + } + + pub fn ephemeral_input_amount(&self) -> Option { + match self { + EphemeralBalance::Input(v) => Some(*v), + EphemeralBalance::Output(_) => None, + } + } + + pub fn ephemeral_output_amount(&self) -> Option { + match self { + EphemeralBalance::Input(_) => None, + EphemeralBalance::Output(v) => Some(*v), + } + } +} + /// 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 { @@ -318,6 +382,20 @@ 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`]. + /// + /// If the inputs include notes or UTXOs that are not economic to spend in the context + /// of this input selection, a [`ChangeError::DustInputs`] error can be returned + /// indicating inputs that should be removed from the selection (all of which will + /// have value less than or equal to the marginal fee). The caller should order the + /// inputs from most to least preferred to spend within each pool, so that the most + /// preferred ones are less likely to be indicated to remove. + /// + /// - `ephemeral_balance`: if the transaction is to be constructed with either an + /// ephemeral transparent input or an ephemeral transparent output this argument + /// may be used to provide the value of that input or output. The value of this + /// output should be `None` in the case that there are no such items. + /// + /// [ZIP 320]: https://zips.z.cash/zip-0320 #[allow(clippy::too_many_arguments)] fn compute_balance( &self, @@ -328,6 +406,7 @@ pub trait ChangeStrategy { sapling: &impl sapling::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard::BundleView, dust_output_policy: &DustOutputPolicy, + ephemeral_balance: Option<&EphemeralBalance>, ) -> Result>; } diff --git a/zcash_client_backend/src/fees/common.rs b/zcash_client_backend/src/fees/common.rs index 4079046b6e..f1b0b7319e 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/zcash_client_backend/src/fees/common.rs @@ -1,18 +1,18 @@ -use core::cmp::max; +use core::cmp::{max, min}; use zcash_primitives::{ consensus::{self, BlockHeight}, memo::MemoBytes, transaction::{ components::amount::{BalanceError, NonNegativeAmount}, - fees::{transparent, zip317::MINIMUM_FEE, FeeRule}, + fees::{transparent, zip317::MINIMUM_FEE, zip317::P2PKH_STANDARD_OUTPUT_SIZE, FeeRule}, }, }; use zcash_protocol::ShieldedProtocol; use super::{ sapling as sapling_fees, ChangeError, ChangeValue, DustAction, DustOutputPolicy, - TransactionBalance, + EphemeralBalance, TransactionBalance, }; #[cfg(feature = "orchard")] @@ -49,6 +49,7 @@ pub(crate) fn calculate_net_flows( transparent_outputs: &[impl transparent::OutputView], sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, + ephemeral_balance: Option<&EphemeralBalance>, ) -> Result> where E: From + From, @@ -58,11 +59,13 @@ where let t_in = transparent_inputs .iter() .map(|t_in| t_in.coin().value) + .chain(ephemeral_balance.and_then(|b| b.ephemeral_input_amount())) .sum::>() .ok_or_else(overflow)?; let t_out = transparent_outputs .iter() .map(|t_out| t_out.value()) + .chain(ephemeral_balance.and_then(|b| b.ephemeral_output_amount())) .sum::>() .ok_or_else(overflow)?; let sapling_in = sapling @@ -154,12 +157,20 @@ pub(crate) fn single_change_output_balance< #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, default_dust_threshold: NonNegativeAmount, - change_memo: Option, + change_memo: Option<&MemoBytes>, fallback_change_pool: ShieldedProtocol, + marginal_fee: NonNegativeAmount, + grace_actions: usize, + ephemeral_balance: Option<&EphemeralBalance>, ) -> Result> where E: From + From, { + // The change memo, if any, must be attached to the change in the intermediate step that + // produces the ephemeral output, and so it should be discarded in the ultimate step; this is + // distinguished by identifying that this transaction has ephemeral inputs. + let change_memo = change_memo.filter(|_| ephemeral_balance.map_or(true, |b| !b.is_input())); + let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow)); let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow)); @@ -169,7 +180,44 @@ where sapling, #[cfg(feature = "orchard")] orchard, + ephemeral_balance, )?; + + #[allow(unused_variables)] + let (change_pool, sapling_change, orchard_change) = + single_change_output_policy(&net_flows, fallback_change_pool); + + // We don't create a fully-transparent transaction if a change memo is used. + let transparent = net_flows.is_transparent() && change_memo.is_none(); + + // If we have a non-zero marginal fee, we need to check for uneconomic inputs. + // This is basically assuming that fee rules with non-zero marginal fee are + // "ZIP 317-like", but we can generalize later if needed. + if marginal_fee.is_positive() { + // Is it certain that there will be a change output? If it is not certain, + // we should call `check_for_uneconomic_inputs` with `possible_change` + // including both possibilities. + let possible_change = + // These are the situations where we might not have a change output. + if transparent || (dust_output_policy.action() == DustAction::AddDustToFee && change_memo.is_none()) { + vec![(0, 0, 0), (0, sapling_change, orchard_change)] + } else { + vec![(0, sapling_change, orchard_change)] + }; + + check_for_uneconomic_inputs( + transparent_inputs, + transparent_outputs, + sapling, + #[cfg(feature = "orchard")] + orchard, + marginal_fee, + grace_actions, + &possible_change[..], + ephemeral_balance, + )?; + } + let total_in = net_flows .total_in() .map_err(|e| ChangeError::StrategyError(E::from(e)))?; @@ -177,10 +225,6 @@ where .total_out() .map_err(|e| ChangeError::StrategyError(E::from(e)))?; - #[allow(unused_variables)] - let (change_pool, sapling_change, orchard_change) = - single_change_output_policy(&net_flows, fallback_change_pool); - let sapling_input_count = sapling .bundle_type() .num_spends(sapling.inputs().len()) @@ -235,17 +279,34 @@ where // and the sum of their outputs learns the sum of the inputs if no change // output is present); and // * we will then always have an shielded output in which to put change_memo, - // if one is given. + // if one is used. // // Note that using the `DustAction::AddDustToFee` policy inherently leaks // more information. + let transparent_input_sizes = transparent_inputs + .iter() + .map(|i| i.serialized_size()) + .chain( + ephemeral_balance + .and_then(|b| b.ephemeral_input_amount()) + .map(|_| transparent::InputSize::STANDARD_P2PKH), + ); + let transparent_output_sizes = transparent_outputs + .iter() + .map(|i| i.serialized_size()) + .chain( + ephemeral_balance + .and_then(|b| b.ephemeral_output_amount()) + .map(|_| P2PKH_STANDARD_OUTPUT_SIZE), + ); + let fee_without_change = fee_rule .fee_required( params, target_height, - transparent_inputs.iter().map(|i| i.serialized_size()), - transparent_outputs.iter().map(|i| i.serialized_size()), + transparent_input_sizes.clone(), + transparent_output_sizes.clone(), sapling_input_count, sapling_output_count, orchard_action_count, @@ -258,8 +319,8 @@ where .fee_required( params, target_height, - transparent_inputs.iter().map(|i| i.serialized_size()), - transparent_outputs.iter().map(|i| i.serialized_size()), + transparent_input_sizes, + transparent_output_sizes, sapling_input_count, sapling_output_count_with_change, orchard_action_count_with_change, @@ -267,14 +328,12 @@ where .map_err(|fee_error| ChangeError::StrategyError(E::from(fee_error)))?, ); - // We don't create a fully-transparent transaction if a change memo is requested. - let transparent = net_flows.is_transparent() && change_memo.is_none(); - let total_out_plus_fee_without_change = (total_out + fee_without_change).ok_or_else(overflow)?; let total_out_plus_fee_with_change = (total_out + fee_with_change).ok_or_else(overflow)?; - let (change, fee) = { + #[allow(unused_mut)] + let (mut change, fee) = { if transparent && total_in < total_out_plus_fee_without_change { // Case 1 for a tx with all transparent flows. return Err(ChangeError::InsufficientFunds { @@ -294,18 +353,22 @@ where // Case 3b or 3c. let proposed_change = (total_in - total_out_plus_fee_with_change).expect("checked above"); - let simple_case = |memo| { + let simple_case = || { ( - vec![ChangeValue::shielded(change_pool, proposed_change, memo)], + vec![ChangeValue::shielded( + change_pool, + proposed_change, + change_memo.cloned(), + )], fee_with_change, ) }; - let dust_threshold = dust_output_policy + let change_dust_threshold = dust_output_policy .dust_threshold() .unwrap_or(default_dust_threshold); - if proposed_change < dust_threshold { + if proposed_change < change_dust_threshold { match dust_output_policy.action() { DustAction::Reject => { // Always allow zero-valued change even for the `Reject` policy: @@ -315,10 +378,10 @@ where // * zero-valued notes do not require witness tracking; // * the effect on trial decryption overhead is small. if proposed_change.is_zero() { - simple_case(change_memo) + simple_case() } else { let shortfall = - (dust_threshold - proposed_change).ok_or_else(underflow)?; + (change_dust_threshold - proposed_change).ok_or_else(underflow)?; return Err(ChangeError::InsufficientFunds { available: total_in, @@ -326,7 +389,7 @@ where }); } } - DustAction::AllowDustChange => simple_case(change_memo), + DustAction::AllowDustChange => simple_case(), DustAction::AddDustToFee => { // Zero-valued change is also always allowed for this policy, but when // no change memo is given, we might omit the change output instead. @@ -342,13 +405,13 @@ where if fee_with_dust > reasonable_fee { // Defend against losing money by using AddDustToFee with a too-high // dust threshold. - simple_case(change_memo) + simple_case() } else if change_memo.is_some() { ( vec![ChangeValue::shielded( change_pool, NonNegativeAmount::ZERO, - change_memo, + change_memo.cloned(), )], fee_with_dust, ) @@ -358,10 +421,221 @@ where } } } else { - simple_case(change_memo) + simple_case() } } }; + #[cfg(feature = "transparent-inputs")] + change.extend( + ephemeral_balance + .and_then(|b| b.ephemeral_output_amount()) + .map(ChangeValue::ephemeral_transparent), + ); TransactionBalance::new(change, fee).map_err(|_| overflow()) } + +/// Returns a `[ChangeStrategy::DustInputs]` error if some of the inputs provided +/// to the transaction have value less than the marginal fee, and could not be +/// determined to have any economic value in the context of this input selection. +/// +/// This determination is potentially conservative in the sense that outputs +/// with value less than the marginal fee might be excluded, even though in +/// practice they would not cause the fee to increase. Outputs with value +/// greater than the marginal fee will never be excluded. +/// +/// `possible_change` is a slice of `(transparent_change, sapling_change, orchard_change)` +/// tuples indicating possible combinations of how many change outputs (0 or 1) +/// might be included in the transaction for each pool. The shape of the tuple +/// does not depend on which protocol features are enabled. +#[allow(clippy::too_many_arguments)] +pub(crate) fn check_for_uneconomic_inputs( + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling: &impl sapling_fees::BundleView, + #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, + marginal_fee: NonNegativeAmount, + grace_actions: usize, + possible_change: &[(usize, usize, usize)], + ephemeral_balance: Option<&EphemeralBalance>, +) -> Result<(), ChangeError> { + let mut t_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 <= marginal_fee { + Some(i.outpoint().clone()) + } else { + None + } + }) + .collect(); + + let mut s_dust: Vec<_> = sapling + .inputs() + .iter() + .filter_map(|i| { + if sapling_fees::InputView::::value(i) <= marginal_fee { + Some(sapling_fees::InputView::::note_id(i).clone()) + } else { + None + } + }) + .collect(); + + #[cfg(feature = "orchard")] + let mut o_dust: Vec = orchard + .inputs() + .iter() + .filter_map(|i| { + if orchard_fees::InputView::::value(i) <= marginal_fee { + Some(orchard_fees::InputView::::note_id(i).clone()) + } else { + None + } + }) + .collect(); + #[cfg(not(feature = "orchard"))] + let mut o_dust: Vec = vec![]; + + // If we don't have any dust inputs, there is nothing to check. + if t_dust.is_empty() && s_dust.is_empty() && o_dust.is_empty() { + return Ok(()); + } + + let (t_inputs_len, t_outputs_len) = ( + transparent_inputs.len() + usize::from(ephemeral_balance.map_or(false, |b| b.is_input())), + transparent_outputs.len() + usize::from(ephemeral_balance.map_or(false, |b| b.is_output())), + ); + let (s_inputs_len, s_outputs_len) = (sapling.inputs().len(), sapling.outputs().len()); + #[cfg(feature = "orchard")] + let (o_inputs_len, o_outputs_len) = (orchard.inputs().len(), orchard.outputs().len()); + #[cfg(not(feature = "orchard"))] + let (o_inputs_len, o_outputs_len) = (0usize, 0usize); + + let t_non_dust = t_inputs_len.checked_sub(t_dust.len()).unwrap(); + let s_non_dust = s_inputs_len.checked_sub(s_dust.len()).unwrap(); + let o_non_dust = o_inputs_len.checked_sub(o_dust.len()).unwrap(); + + // Return the number of allowed dust inputs from each pool. + let allowed_dust = |(t_change, s_change, o_change): &(usize, usize, usize)| { + // Here we assume a "ZIP 317-like" fee model in which the existence of an output + // to a given pool implies that a corresponding input from that pool can be + // provided without increasing the fee. (This is also likely to be true for + // future fee models if we do not want to penalize use of Orchard relative to + // other pools.) + // + // Under that assumption, we want to calculate the maximum number of dust inputs + // from each pool, out of the ones we actually have, that can be economically + // spent along with the non-dust inputs. Get an initial estimate by calculating + // the number of dust inputs in each pool that will be allowed regardless of + // padding or grace. + + let t_allowed = min( + t_dust.len(), + (t_outputs_len + t_change).saturating_sub(t_non_dust), + ); + let s_allowed = min( + s_dust.len(), + (s_outputs_len + s_change).saturating_sub(s_non_dust), + ); + let o_allowed = min( + o_dust.len(), + (o_outputs_len + o_change).saturating_sub(o_non_dust), + ); + + // We'll be spending the non-dust and allowed dust in each pool. + let t_req_inputs = t_non_dust + t_allowed; + let s_req_inputs = s_non_dust + s_allowed; + #[cfg(feature = "orchard")] + let o_req_inputs = o_non_dust + o_allowed; + + // This calculates the hypothetical number of actions with given extra inputs, + // for ZIP 317 and the padding rules in effect. The padding rules for each + // pool are subtle (they also depend on `bundle_required` for example), so we + // must actually call them rather than try to predict their effect. To tell + // whether we can freely add an extra input from a given pool, we need to call + // them both with and without that input; if the number of actions does not + // increase, then the input is free to add. + let hypothetical_actions = |t_extra, s_extra, _o_extra| { + let s_spend_count = sapling + .bundle_type() + .num_spends(s_req_inputs + s_extra) + .map_err(ChangeError::BundleError)?; + + let s_output_count = sapling + .bundle_type() + .num_outputs(s_req_inputs + s_extra, s_outputs_len + s_change) + .map_err(ChangeError::BundleError)?; + + #[cfg(feature = "orchard")] + let o_action_count = orchard + .bundle_type() + .num_actions(o_req_inputs + _o_extra, o_outputs_len + o_change) + .map_err(ChangeError::BundleError)?; + #[cfg(not(feature = "orchard"))] + let o_action_count = 0; + + // To calculate the number of unused actions, we assume that transparent inputs + // and outputs are P2PKH. + Ok(max(t_req_inputs + t_extra, t_outputs_len + t_change) + + max(s_spend_count, s_output_count) + + o_action_count) + }; + + // First calculate the baseline number of logical actions with only the definitely + // allowed inputs estimated above. If it is less than `grace_actions`, try to allocate + // a grace input first to transparent dust, then to Sapling dust, then to Orchard dust. + // If the number of actions increases, it was not possible to allocate that input for + // free. This approach is sufficient because at most one such input can be allocated, + // since `grace_actions` is at most 2 for ZIP 317 and there must be at least one + // logical action. (If `grace_actions` were greater than 2 then the code would still + // be correct, it would just not find all potential extra inputs.) + + let baseline = hypothetical_actions(0, 0, 0)?; + + let (t_extra, s_extra, o_extra) = if baseline >= grace_actions { + (0, 0, 0) + } else if t_dust.len() > t_allowed && hypothetical_actions(1, 0, 0)? <= baseline { + (1, 0, 0) + } else if s_dust.len() > s_allowed && hypothetical_actions(0, 1, 0)? <= baseline { + (0, 1, 0) + } else if o_dust.len() > o_allowed && hypothetical_actions(0, 0, 1)? <= baseline { + (0, 0, 1) + } else { + (0, 0, 0) + }; + Ok(( + t_allowed + t_extra, + s_allowed + s_extra, + o_allowed + o_extra, + )) + }; + + // Find the least number of allowed dust inputs for each pool for any `possible_change`. + let (t_allowed, s_allowed, o_allowed) = possible_change + .iter() + .map(allowed_dust) + .collect::, _>>()? + .into_iter() + .reduce(|(a, b, c), (x, y, z)| (min(a, x), min(b, y), min(c, z))) + .expect("possible_change is nonempty"); + + // The inputs in the tail of each list after the first `*_allowed` are returned as uneconomic. + // The caller should order the inputs from most to least preferred to spend. + let t_dust = t_dust.split_off(t_allowed); + let s_dust = s_dust.split_off(s_allowed); + let o_dust = o_dust.split_off(o_allowed); + + if t_dust.is_empty() && s_dust.is_empty() && o_dust.is_empty() { + Ok(()) + } else { + Err(ChangeError::DustInputs { + transparent: t_dust, + sapling: s_dust, + #[cfg(feature = "orchard")] + orchard: o_dust, + }) + } +} diff --git a/zcash_client_backend/src/fees/fixed.rs b/zcash_client_backend/src/fees/fixed.rs index ee032ab013..933b008207 100644 --- a/zcash_client_backend/src/fees/fixed.rs +++ b/zcash_client_backend/src/fees/fixed.rs @@ -4,7 +4,7 @@ use zcash_primitives::{ consensus::{self, BlockHeight}, memo::MemoBytes, transaction::{ - components::amount::BalanceError, + components::amount::{BalanceError, NonNegativeAmount}, fees::{fixed::FeeRule as FixedFeeRule, transparent}, }, }; @@ -13,7 +13,7 @@ use crate::ShieldedProtocol; use super::{ common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy, - DustOutputPolicy, TransactionBalance, + DustOutputPolicy, EphemeralBalance, TransactionBalance, }; #[cfg(feature = "orchard")] @@ -63,6 +63,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, + ephemeral_balance: Option<&EphemeralBalance>, ) -> Result> { single_change_output_balance( params, @@ -74,9 +75,12 @@ impl ChangeStrategy for SingleOutputChangeStrategy { #[cfg(feature = "orchard")] orchard, dust_output_policy, - self.fee_rule().fixed_fee(), - self.change_memo.clone(), + self.fee_rule.fixed_fee(), + self.change_memo.as_ref(), self.fallback_change_pool, + NonNegativeAmount::ZERO, + 0, + ephemeral_balance, ) } } @@ -132,6 +136,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), + None, ); assert_matches!( @@ -177,6 +182,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), + None, ); assert_matches!( diff --git a/zcash_client_backend/src/fees/standard.rs b/zcash_client_backend/src/fees/standard.rs index fd146b5105..5c08c7b1af 100644 --- a/zcash_client_backend/src/fees/standard.rs +++ b/zcash_client_backend/src/fees/standard.rs @@ -18,7 +18,7 @@ use crate::ShieldedProtocol; use super::{ fixed, sapling as sapling_fees, zip317, ChangeError, ChangeStrategy, DustOutputPolicy, - TransactionBalance, + EphemeralBalance, TransactionBalance, }; #[cfg(feature = "orchard")] @@ -68,6 +68,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, + ephemeral_balance: Option<&EphemeralBalance>, ) -> Result> { #[allow(deprecated)] match self.fee_rule() { @@ -85,6 +86,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { #[cfg(feature = "orchard")] orchard, dust_output_policy, + ephemeral_balance, ) .map_err(|e| e.map(Zip317FeeError::Balance)), StandardFeeRule::Zip313 => fixed::SingleOutputChangeStrategy::new( @@ -101,6 +103,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { #[cfg(feature = "orchard")] orchard, dust_output_policy, + ephemeral_balance, ) .map_err(|e| e.map(Zip317FeeError::Balance)), StandardFeeRule::Zip317 => zip317::SingleOutputChangeStrategy::new( @@ -117,6 +120,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { #[cfg(feature = "orchard")] orchard, dust_output_policy, + ephemeral_balance, ), } } diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index 9ef62fbca6..c5cd7d499a 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -16,8 +16,8 @@ use zcash_primitives::{ use crate::ShieldedProtocol; use super::{ - common::{calculate_net_flows, single_change_output_balance, single_change_output_policy}, - sapling as sapling_fees, ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance, + common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy, + DustOutputPolicy, EphemeralBalance, TransactionBalance, }; #[cfg(feature = "orchard")] @@ -67,132 +67,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, + ephemeral_balance: Option<&EphemeralBalance>, ) -> 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 sapling_fees::InputView::::value(i) < self.fee_rule.marginal_fee() { - Some(sapling_fees::InputView::::note_id(i).clone()) - } else { - None - } - }) - .collect(); - - #[cfg(feature = "orchard")] - let mut orchard_dust: Vec = orchard - .inputs() - .iter() - .filter_map(|i| { - if orchard_fees::InputView::::value(i) < self.fee_rule.marginal_fee() { - Some(orchard_fees::InputView::::note_id(i).clone()) - } else { - None - } - }) - .collect(); - #[cfg(not(feature = "orchard"))] - let mut orchard_dust: Vec = vec![]; - - // 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() && orchard_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 either the Sapling or Orchard outputs for the (single) - // change output. Note that this means that wallet-internal shielding - // transactions are an opportunity to spend a dust note. - let net_flows = calculate_net_flows::( - transparent_inputs, - transparent_outputs, - sapling, - #[cfg(feature = "orchard")] - orchard, - )?; - let (_, sapling_change, orchard_change) = - single_change_output_policy(&net_flows, self.fallback_change_pool); - - let s_non_dust = sapling.inputs().len() - sapling_dust.len(); - let s_allowed_dust = - (sapling.outputs().len() + sapling_change).saturating_sub(s_non_dust); - - #[cfg(feature = "orchard")] - let (orchard_inputs_len, orchard_outputs_len) = - (orchard.inputs().len(), orchard.outputs().len()); - #[cfg(not(feature = "orchard"))] - let (orchard_inputs_len, orchard_outputs_len) = (0, 0); - - let o_non_dust = orchard_inputs_len - orchard_dust.len(); - let o_allowed_dust = (orchard_outputs_len + orchard_change).saturating_sub(o_non_dust); - - let available_grace_inputs = self - .fee_rule - .grace_actions() - .saturating_sub(t_non_dust) - .saturating_sub(s_non_dust) - .saturating_sub(o_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); - let mut o_disallowed_dust = orchard_dust.len().saturating_sub(o_allowed_dust); - - if available_grace_inputs > 0 { - // If we have available grace inputs, allocate them first to transparent dust - // and then to Sapling dust followed by Orchard 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); - - let o_grace_dust = available_grace_inputs - .saturating_sub(t_grace_dust) - .saturating_sub(s_grace_dust) - .saturating_sub(o_disallowed_dust); - o_disallowed_dust = o_disallowed_dust.saturating_sub(o_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); - orchard_dust.reverse(); - orchard_dust.truncate(o_disallowed_dust); - - if !(transparent_dust.is_empty() && sapling_dust.is_empty() && orchard_dust.is_empty()) - { - return Err(ChangeError::DustInputs { - transparent: transparent_dust, - sapling: sapling_dust, - #[cfg(feature = "orchard")] - orchard: orchard_dust, - }); - } - } - single_change_output_balance( params, &self.fee_rule, @@ -204,8 +80,11 @@ impl ChangeStrategy for SingleOutputChangeStrategy { orchard, dust_output_policy, self.fee_rule.marginal_fee(), - self.change_memo.clone(), + self.change_memo.as_ref(), self.fallback_change_pool, + self.fee_rule.marginal_fee(), + self.fee_rule.grace_actions(), + ephemeral_balance, ) } } @@ -268,6 +147,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), + None, ); assert_matches!( @@ -311,6 +191,7 @@ mod tests { ))][..], ), &DustOutputPolicy::default(), + None, ); assert_matches!( @@ -363,6 +244,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, dust_output_policy, + None, ); assert_matches!( @@ -406,6 +288,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), + None, ); assert_matches!( @@ -449,6 +332,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), + None, ); assert_matches!( @@ -498,6 +382,7 @@ mod tests { DustAction::AllowDustChange, Some(NonNegativeAmount::const_from_u64(1000)), ), + None, ); assert_matches!( @@ -558,6 +443,7 @@ mod tests { #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, dust_output_policy, + None, ); assert_matches!( @@ -576,10 +462,8 @@ mod tests { ShieldedProtocol::Sapling, ); - // Attempt to spend three Sapling notes, one of them dust. Taking into account - // Sapling output padding, the dust note would be free to add to the transaction - // if there were only two notes (as in the `change_with_allowable_dust` test), but - // it is not free when there are three notes. + // Attempt to spend three Sapling notes, one of them dust. Adding the third + // note increases the number of actions, and so it is uneconomic to spend it. let result = change_strategy.compute_balance( &Network::TestNetwork, Network::TestNetwork @@ -604,15 +488,16 @@ mod tests { }, ][..], &[SaplingPayment::new(NonNegativeAmount::const_from_u64( - 40000, + 30000, ))][..], ), #[cfg(feature = "orchard")] &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), + None, ); - // We will get an error here, because the dust input now isn't free to add + // We will get an error here, because the dust input isn't free to add // to the transaction. assert_matches!( result, diff --git a/zcash_client_backend/src/proposal.rs b/zcash_client_backend/src/proposal.rs index 3c0c01215a..e88dbcf8d5 100644 --- a/zcash_client_backend/src/proposal.rs +++ b/zcash_client_backend/src/proposal.rs @@ -47,6 +47,18 @@ pub enum ProposalError { /// There was a mismatch between the payments in the proposal's transaction request /// and the payment pool selection values. PaymentPoolsMismatch, + /// The proposal tried to spend a change output. Mark the `ChangeValue` as ephemeral if this is intended. + SpendsChange(StepOutput), + /// A proposal step created an ephemeral output that was not spent in any later step. + #[cfg(feature = "transparent-inputs")] + EphemeralOutputLeftUnspent(StepOutput), + /// The proposal included a payment to a TEX address and a spend from a shielded input in the same step. + #[cfg(feature = "transparent-inputs")] + PaysTexFromShielded, + /// The change strategy provided to input selection failed to correctly generate an ephemeral + /// change output when needed for sending to a TEX address. + #[cfg(feature = "transparent-inputs")] + EphemeralOutputsInvalid, } impl Display for ProposalError { @@ -90,6 +102,27 @@ impl Display for ProposalError { f, "The chosen payment pools did not match the payments of the transaction request." ), + ProposalError::SpendsChange(r) => write!( + f, + "The proposal attempts to spends the change output created at step {:?}.", + r, + ), + #[cfg(feature = "transparent-inputs")] + ProposalError::EphemeralOutputLeftUnspent(r) => write!( + f, + "The proposal created an ephemeral output at step {:?} that was not spent in any later step.", + r, + ), + #[cfg(feature = "transparent-inputs")] + ProposalError::PaysTexFromShielded => write!( + f, + "The proposal included a payment to a TEX address and a spend from a shielded input in the same step.", + ), + #[cfg(feature = "transparent-inputs")] + ProposalError::EphemeralOutputsInvalid => write!( + f, + "The change strategy provided to input selection failed to correctly generate an ephemeral change output when needed for sending to a TEX address." + ), } } } @@ -292,14 +325,14 @@ impl Debug for Proposal { } /// A reference to either a payment or change output within a step. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum StepOutputIndex { Payment(usize), Change(usize), } /// A reference to the output of a step in a proposal. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct StepOutput { step_index: usize, output_index: StepOutputIndex, diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index 915d1d39e3..c832fb44f6 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -335,7 +335,7 @@ pub const PROPOSAL_SER_V1: u32 = 1; /// representation. #[derive(Debug, Clone)] pub enum ProposalDecodingError { - /// The encoded proposal contained no steps + /// The encoded proposal contained no steps. NoSteps, /// The ZIP 321 transaction request URI was invalid. Zip321(Zip321Error), @@ -366,6 +366,8 @@ pub enum ProposalDecodingError { TransparentMemo, /// Change outputs to the specified pool are not supported. InvalidChangeRecipient(PoolType), + /// Ephemeral outputs to the specified pool are not supported. + InvalidEphemeralRecipient(PoolType), } impl From for ProposalDecodingError { @@ -424,6 +426,11 @@ impl Display for ProposalDecodingError { "Change outputs to the {} pool are not supported.", pool_type ), + ProposalDecodingError::InvalidEphemeralRecipient(pool_type) => write!( + f, + "Ephemeral outputs to the {} pool are not supported.", + pool_type + ), } } } @@ -572,6 +579,7 @@ impl proposal::Proposal { memo: change.memo().map(|memo_bytes| proposal::MemoBytes { value: memo_bytes.as_slice().to_vec(), }), + is_ephemeral: change.is_ephemeral(), }) .collect(), fee_required: step.balance().fee_required().into(), @@ -641,9 +649,7 @@ impl proposal::Proposal { }) .collect::, ProposalDecodingError>>()?; - #[cfg(not(feature = "transparent-inputs"))] - let transparent_inputs = vec![]; - #[cfg(feature = "transparent-inputs")] + #[allow(unused_mut)] let mut transparent_inputs = vec![]; let mut received_notes = vec![]; let mut prior_step_inputs = vec![]; @@ -662,7 +668,7 @@ impl proposal::Proposal { PoolType::Transparent => { #[cfg(not(feature = "transparent-inputs"))] return Err(ProposalDecodingError::ValuePoolNotSupported( - 1, + out.value_pool, )); #[cfg(feature = "transparent-inputs")] @@ -751,18 +757,27 @@ impl proposal::Proposal { .map_err(ProposalDecodingError::MemoInvalid) }) .transpose()?; - match cv.pool_type()? { - PoolType::Shielded(ShieldedProtocol::Sapling) => { + match (cv.pool_type()?, cv.is_ephemeral) { + (PoolType::Shielded(ShieldedProtocol::Sapling), false) => { Ok(ChangeValue::sapling(value, memo)) } #[cfg(feature = "orchard")] - PoolType::Shielded(ShieldedProtocol::Orchard) => { + (PoolType::Shielded(ShieldedProtocol::Orchard), false) => { Ok(ChangeValue::orchard(value, memo)) } - PoolType::Transparent if memo.is_some() => { + (PoolType::Transparent, _) if memo.is_some() => { Err(ProposalDecodingError::TransparentMemo) } - t => Err(ProposalDecodingError::InvalidChangeRecipient(t)), + #[cfg(feature = "transparent-inputs")] + (PoolType::Transparent, true) => { + Ok(ChangeValue::ephemeral_transparent(value)) + } + (pool, false) => { + Err(ProposalDecodingError::InvalidChangeRecipient(pool)) + } + (pool, true) => { + Err(ProposalDecodingError::InvalidEphemeralRecipient(pool)) + } } }) .collect::, _>>()?, diff --git a/zcash_client_backend/src/proto/proposal.rs b/zcash_client_backend/src/proto/proposal.rs index a498b6eb12..d4fd13c1e4 100644 --- a/zcash_client_backend/src/proto/proposal.rs +++ b/zcash_client_backend/src/proto/proposal.rs @@ -73,7 +73,7 @@ pub struct ReceivedOutput { #[prost(uint64, tag = "4")] pub value: u64, } -/// A reference a payment in a prior step of the proposal. This payment must +/// A reference to a payment in a prior step of the proposal. This payment must /// belong to the wallet. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, Copy, PartialEq, ::prost::Message)] @@ -83,7 +83,7 @@ pub struct PriorStepOutput { #[prost(uint32, tag = "2")] pub payment_index: u32, } -/// A reference a change output from a prior step of the proposal. +/// A reference to a change or ephemeral output from a prior step of the proposal. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct PriorStepChange { @@ -116,28 +116,36 @@ pub mod proposed_input { #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct TransactionBalance { - /// A list of change output values. + /// A list of change or ephemeral output values. #[prost(message, repeated, tag = "1")] pub proposed_change: ::prost::alloc::vec::Vec, /// The fee to be paid by the proposed transaction, in zatoshis. #[prost(uint64, tag = "2")] pub fee_required: u64, } -/// A proposed change output. If the transparent value pool is selected, -/// the `memo` field must be null. +/// A proposed change or ephemeral output. If the transparent value pool is +/// selected, the `memo` field must be null. +/// +/// When the `isEphemeral` field of a `ChangeValue` is set, it represents +/// an ephemeral output, which must be spent by a subsequent step. This is +/// only supported for transparent outputs. Each ephemeral output will be +/// given a unique t-address. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct ChangeValue { - /// The value of a change output to be created, in zatoshis. + /// The value of a change or ephemeral output to be created, in zatoshis. #[prost(uint64, tag = "1")] pub value: u64, - /// The value pool in which the change output should be created. + /// The value pool in which the change or ephemeral output should be created. #[prost(enumeration = "ValuePool", tag = "2")] pub value_pool: i32, - /// The optional memo that should be associated with the newly created change output. - /// Memos must not be present for transparent change outputs. + /// The optional memo that should be associated with the newly created output. + /// Memos must not be present for transparent outputs. #[prost(message, optional, tag = "3")] pub memo: ::core::option::Option, + /// Whether this is to be an ephemeral output. + #[prost(bool, tag = "4")] + pub is_ephemeral: bool, } /// An object wrapper for memo bytes, to facilitate representing the /// `change_memo == None` case. diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 7d555b07f6..1bc4dae3fc 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -62,13 +62,19 @@ impl NoteId { } } -/// A type that represents the recipient of a transaction output: a recipient address (and, for -/// unified addresses, the pool to which the payment is sent) in the case of an outgoing output, or an -/// internal account ID and the pool to which funds were sent in the case of a wallet-internal -/// output. +/// A type that represents the recipient of a transaction output: +/// * a recipient address; +/// * for external unified addresses, the pool to which the payment is sent; +/// * for ephemeral transparent addresses, the internal account ID and metadata about the outpoint; +/// * for wallet-internal outputs, the internal account ID and metadata about the note. #[derive(Debug, Clone)] -pub enum Recipient { +pub enum Recipient { External(ZcashAddress, PoolType), + EphemeralTransparent { + receiving_account: AccountId, + ephemeral_address: TransparentAddress, + outpoint_metadata: O, + }, InternalAccount { receiving_account: AccountId, external_address: Option, @@ -76,10 +82,23 @@ pub enum Recipient { }, } -impl Recipient { - pub fn map_internal_account_note B>(self, f: F) -> Recipient { +impl Recipient { + /// Return a copy of this `Recipient` with `f` applied to the note metadata, if any. + pub fn map_internal_account_note B>( + self, + f: F, + ) -> Recipient { match self { Recipient::External(addr, pool) => Recipient::External(addr, pool), + Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint_metadata, + } => Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint_metadata, + }, Recipient::InternalAccount { receiving_account, external_address, @@ -91,12 +110,51 @@ impl Recipient { }, } } + + /// Return a copy of this `Recipient` with `f` applied to the output metadata, if any. + pub fn map_ephemeral_transparent_outpoint B>( + self, + f: F, + ) -> Recipient { + match self { + Recipient::External(addr, pool) => Recipient::External(addr, pool), + Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint_metadata, + } => Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint_metadata: f(outpoint_metadata), + }, + Recipient::InternalAccount { + receiving_account, + external_address, + note, + } => Recipient::InternalAccount { + receiving_account, + external_address, + note, + }, + } + } } -impl Recipient> { - pub fn internal_account_note_transpose_option(self) -> Option> { +impl Recipient, O> { + /// Return a copy of this `Recipient` with optional note metadata transposed to + /// an optional result. + pub fn internal_account_note_transpose_option(self) -> Option> { match self { Recipient::External(addr, pool) => Some(Recipient::External(addr, pool)), + Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint_metadata, + } => Some(Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint_metadata, + }), Recipient::InternalAccount { receiving_account, external_address, @@ -579,6 +637,7 @@ impl OvkPolicy { } /// Metadata related to the ZIP 32 derivation of a transparent address. +/// This is implicitly scoped to an account. #[derive(Debug, Clone, PartialEq, Eq)] #[cfg(feature = "transparent-inputs")] pub struct TransparentAddressMetadata { @@ -588,6 +647,8 @@ pub struct TransparentAddressMetadata { #[cfg(feature = "transparent-inputs")] impl TransparentAddressMetadata { + /// Returns a `TransparentAddressMetadata` in the given scope for the + /// given address index. pub fn new(scope: TransparentKeyScope, address_index: NonHardenedChildIndex) -> Self { Self { scope, diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 6c3e8df808..615e22aefc 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -6,7 +6,28 @@ and this library adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Notable changes +`zcash_client_sqlite` now supports TEX (transparent-source-only) addresses as specified +in ZIP 320. Sending to one or more TEX addresses will automatically create a multi-step +proposal that uses two transactions. + +In order to take advantage of this support, client wallets will need to be able to send +multiple transactions created from `zcash_client_backend::data_api::wallet::create_proposed_transactions`. +This API was added in `zcash_client_backend` 0.11.0 but previously could only return a +single transaction. + +**Note:** This feature changes the use of transparent addresses in ways that are relevant +to security and access to funds, and that may interact with other wallet behaviour. In +particular it exposes new ephemeral transparent addresses belonging to the wallet, which +need to be scanned in order to recover funds if the first transaction of the proposal is +mined but the second is not, or if someone (e.g. the TEX-address recipient) sends back +funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for details. + ### Changed +- `zcash_client_sqlite::error::SqliteClientError` has a new `ReachedGapLimit` and + `EphemeralAddressReuse` variants when the "transparent-inputs" feature is enabled. +- The result of the `v_tx_outputs` SQL query could now include transparent outputs + with unknown height. - MSRV is now 1.70.0. - `zcash_client_sqlite::error::SqliteClientError` has changed variants: - Removed `HdwalletError`. diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index a1903acb4a..1521d3a0e2 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -54,6 +54,9 @@ jubjub.workspace = true secrecy.workspace = true subtle.workspace = true +# - Static assertions +static_assertions.workspace = true + # - Shielded protocols orchard = { workspace = true, optional = true } sapling.workspace = true diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index c0a10cac4b..3e9ce57211 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -15,8 +15,9 @@ use crate::PRUNING_DEPTH; #[cfg(feature = "transparent-inputs")] use { + crate::AccountId, zcash_client_backend::encoding::TransparentCodecError, - zcash_primitives::legacy::TransparentAddress, + zcash_primitives::{legacy::TransparentAddress, transaction::TxId}, }; /// The primary error type for the SQLite wallet backend. @@ -112,6 +113,18 @@ pub enum SqliteClientError { /// An error occurred in computing wallet balance BalanceError(BalanceError), + + /// The proposal cannot be constructed until transactions with previously reserved + /// ephemeral address outputs have been mined. The parameters are the account id and + /// the index that could not safely be reserved. + #[cfg(feature = "transparent-inputs")] + ReachedGapLimit(AccountId, u32), + + /// An ephemeral address would be reused. The parameters are the address in string + /// form, and the txid of the earliest transaction in which it is known to have been + /// used. + #[cfg(feature = "transparent-inputs")] + EphemeralAddressReuse(String, TxId), } impl error::Error for SqliteClientError { @@ -162,6 +175,13 @@ impl fmt::Display for SqliteClientError { SqliteClientError::ChainHeightUnknown => write!(f, "Chain height unknown; please call `update_chain_tip`"), SqliteClientError::UnsupportedPoolType(t) => write!(f, "Pool type is not currently supported: {}", t), SqliteClientError::BalanceError(e) => write!(f, "Balance error: {}", e), + #[cfg(feature = "transparent-inputs")] + SqliteClientError::ReachedGapLimit(account_id, bad_index) => write!(f, + "The proposal cannot be constructed until transactions with previously reserved ephemeral address outputs have been mined. \ + The ephemeral address in account {account_id:?} at index {bad_index} could not be safely reserved.", + ), + #[cfg(feature = "transparent-inputs")] + SqliteClientError::EphemeralAddressReuse(address_str, txid) => write!(f, "The ephemeral address {address_str} previously used in txid {txid} would be reused."), } } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index a429eee12b..028124f2c7 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -88,7 +88,11 @@ use { #[cfg(feature = "transparent-inputs")] use { zcash_client_backend::wallet::TransparentAddressMetadata, - zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}, + zcash_keys::encoding::AddressCodec, + zcash_primitives::{ + legacy::TransparentAddress, + transaction::components::{OutPoint, TxOut}, + }, }; #[cfg(feature = "unstable")] @@ -298,7 +302,7 @@ impl, P: consensus::Parameters> WalletRead for W type Account = wallet::Account; fn get_account_ids(&self) -> Result, Self::Error> { - wallet::get_account_ids(self.conn.borrow()) + Ok(wallet::get_account_ids(self.conn.borrow())?) } fn get_account( @@ -523,13 +527,52 @@ impl, P: consensus::Parameters> WalletRead for W account: AccountId, max_height: BlockHeight, ) -> Result, Self::Error> { - wallet::transparent::get_transparent_address_balances( + wallet::transparent::get_transparent_balances( self.conn.borrow(), &self.params, account, max_height, ) } + + #[cfg(feature = "transparent-inputs")] + fn get_transparent_address_metadata( + &self, + account: Self::AccountId, + address: &TransparentAddress, + ) -> Result, Self::Error> { + wallet::transparent::get_transparent_address_metadata( + self.conn.borrow(), + &self.params, + account, + address, + ) + } + + #[cfg(feature = "transparent-inputs")] + fn get_known_ephemeral_addresses( + &self, + account: Self::AccountId, + index_range: Option>, + ) -> Result, Self::Error> { + wallet::transparent::ephemeral::get_known_ephemeral_addresses( + self.conn.borrow(), + &self.params, + account, + index_range, + ) + } + + #[cfg(feature = "transparent-inputs")] + fn find_account_for_ephemeral_address( + &self, + address: &TransparentAddress, + ) -> Result, Self::Error> { + wallet::transparent::ephemeral::find_account_for_ephemeral_address_str( + self.conn.borrow(), + &address.encode(&self.params), + ) + } } impl WalletWrite for WalletDb { @@ -1056,6 +1099,8 @@ impl WalletWrite for WalletDb self.transactionally(|wdb| { let tx_ref = wallet::put_tx_data(wdb.conn.0, d_tx.tx(), None, None)?; let funding_accounts = wallet::get_funding_accounts(wdb.conn.0, d_tx.tx())?; + + // TODO(#1305): Correctly track accounts that fund each transaction output. let funding_account = funding_accounts.iter().next().copied(); if funding_accounts.len() > 1 { warn!( @@ -1084,6 +1129,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, *output.account(), tx_ref, output.index(), @@ -1103,6 +1149,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, *output.account(), tx_ref, output.index(), @@ -1133,6 +1180,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, account_id, tx_ref, output.index(), @@ -1165,6 +1213,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, *output.account(), tx_ref, output.index(), @@ -1184,6 +1233,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, *output.account(), tx_ref, output.index(), @@ -1215,6 +1265,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, account_id, tx_ref, output.index(), @@ -1238,35 +1289,27 @@ impl WalletWrite for WalletDb wallet::transparent::mark_transparent_utxo_spent(wdb.conn.0, tx_ref, &txin.prevout)?; } - // If we have some transparent outputs: - if d_tx - .tx() - .transparent_bundle() - .iter() - .any(|b| !b.vout.is_empty()) - { - // If the transaction contains spends from our wallet, we will store z->t - // transactions we observe in the same way they would be stored by - // create_spend_to_address. - let funding_accounts = wallet::get_funding_accounts(wdb.conn.0, d_tx.tx())?; - let funding_account = funding_accounts.iter().next().copied(); - if let Some(account_id) = funding_account { - if funding_accounts.len() > 1 { - warn!( - "More than one wallet account detected as funding transaction {:?}, selecting {:?}", - d_tx.tx().txid(), - account_id - ) - } - - for (output_index, txout) in d_tx - .tx() - .transparent_bundle() - .iter() - .flat_map(|b| b.vout.iter()) - .enumerate() - { - if let Some(address) = txout.recipient_address() { + // This `if` is just an optimization for cases where we would do nothing in the loop. + if funding_account.is_some() || cfg!(feature = "transparent-inputs") { + for (output_index, txout) in d_tx + .tx() + .transparent_bundle() + .iter() + .flat_map(|b| b.vout.iter()) + .enumerate() + { + if let Some(address) = txout.recipient_address() { + // The transaction is not necessarily mined yet, but we want to record + // that an output to the address was seen in this tx anyway. This will + // advance the gap regardless of whether it is mined, but an output in + // an unmined transaction won't advance the range of safe indices. + #[cfg(feature = "transparent-inputs")] + wallet::transparent::ephemeral::mark_ephemeral_address_as_seen(wdb, &address, tx_ref)?; + + // If a transaction we observe contains spends from our wallet, we will + // store its transparent outputs in the same way they would be stored by + // create_spend_to_address. + if let Some(account_id) = funding_account { let receiver = Receiver::Transparent(address); #[cfg(feature = "transparent-inputs")] @@ -1286,6 +1329,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, account_id, tx_ref, output_index, @@ -1352,7 +1396,13 @@ impl WalletWrite for WalletDb } for output in sent_tx.outputs() { - wallet::insert_sent_output(wdb.conn.0, tx_ref, *sent_tx.account_id(), output)?; + wallet::insert_sent_output( + wdb.conn.0, + &wdb.params, + tx_ref, + *sent_tx.account_id(), + output, + )?; match output.recipient() { Recipient::InternalAccount { @@ -1396,7 +1446,31 @@ impl WalletWrite for WalletDb None, )?; } - _ => (), + #[cfg(feature = "transparent-inputs")] + Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint_metadata, + } => { + wallet::transparent::put_transparent_output( + wdb.conn.0, + &wdb.params, + outpoint_metadata, + &TxOut { + value: output.value(), + script_pubkey: ephemeral_address.script(), + }, + None, + ephemeral_address, + *receiving_account, + )?; + wallet::transparent::ephemeral::mark_ephemeral_address_as_used( + wdb, + ephemeral_address, + tx_ref, + )?; + } + _ => {} } } @@ -1409,6 +1483,17 @@ impl WalletWrite for WalletDb wallet::truncate_to_height(wdb.conn.0, &wdb.params, block_height) }) } + + #[cfg(feature = "transparent-inputs")] + fn reserve_next_n_ephemeral_addresses( + &mut self, + account_id: Self::AccountId, + n: usize, + ) -> Result, Self::Error> { + self.transactionally(|wdb| { + wallet::transparent::ephemeral::reserve_next_n_ephemeral_addresses(wdb, account_id, n) + }) + } } impl WalletCommitmentTrees for WalletDb { diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 833eb52b00..5e64a06eb2 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -62,13 +62,14 @@ use crate::{ #[cfg(feature = "transparent-inputs")] use { - zcash_client_backend::{ - fees::TransactionBalance, proposal::Step, wallet::WalletTransparentOutput, + zcash_client_backend::wallet::WalletTransparentOutput, + zcash_primitives::transaction::{ + components::{OutPoint, TxOut}, + fees::zip317, }, - zcash_primitives::transaction::components::{OutPoint, TxOut}, }; -#[cfg(any(feature = "transparent-inputs", feature = "orchard"))] +#[cfg(feature = "orchard")] use zcash_client_backend::PoolType; pub(crate) type OutputRecoveryError = Error< @@ -301,14 +302,19 @@ pub(crate) fn send_single_step_proposed_transfer() { #[cfg(feature = "transparent-inputs")] pub(crate) fn send_multi_step_proposed_transfer() { - use std::collections::BTreeSet; + use std::{collections::HashSet, str::FromStr}; - use nonempty::NonEmpty; + use rand_core::OsRng; use zcash_client_backend::{ fees::ChangeValue, - proposal::{Proposal, StepOutput, StepOutputIndex}, + wallet::{TransparentAddressMetadata, WalletTx}, + }; + use zcash_primitives::{ + legacy::keys::{NonHardenedChildIndex, TransparentKeyScope}, + transaction::builder::{BuildConfig, Builder}, }; - use zcash_primitives::{legacy::keys::IncomingViewingKey, transaction::TxId}; + + use crate::wallet::{sapling::tests::test_prover, GAP_LIMIT}; let mut st = TestBuilder::new() .with_block_cache() @@ -316,152 +322,436 @@ pub(crate) fn send_multi_step_proposed_transfer() { .build(); let account = st.test_account().cloned().unwrap(); + let account_id = account.account_id(); + let (default_addr, default_index) = account.usk().default_transparent_address(); let dfvk = T::test_account_fvk(&st); - // Add funds to the wallet in a single note + let add_funds = |st: &mut TestState<_>, value| { + let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + st.scan_cached_blocks(h, 1); + + assert_eq!( + block_max_scanned(&st.wallet().conn, &st.wallet().params) + .unwrap() + .unwrap() + .block_height(), + h + ); + assert_eq!(st.get_spendable_balance(account_id, 1), value); + h + }; + let value = NonNegativeAmount::const_from_u64(100000); - let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); - st.scan_cached_blocks(h, 1); + let transfer_amount = NonNegativeAmount::const_from_u64(50000); + + let run_test = |st: &mut TestState<_>, expected_index| { + // Add funds to the wallet. + add_funds(st, value); + + let expected_step0_fee = (zip317::MARGINAL_FEE * 3).unwrap(); + let expected_step1_fee = zip317::MINIMUM_FEE; + let expected_ephemeral = (transfer_amount + expected_step1_fee).unwrap(); + let expected_step0_change = + (value - expected_ephemeral - expected_step0_fee).expect("sufficient funds"); + assert!(expected_step0_change.is_positive()); + + // Generate a ZIP 320 proposal, sending to the wallet's default transparent address + // expressed as a TEX address. + let tex_addr = match default_addr { + TransparentAddress::PublicKeyHash(data) => Address::Tex(data), + _ => unreachable!(), + }; + let change_memo = Some(Memo::from_str("change").expect("valid memo").encode()); + + // We use `st.propose_standard_transfer` here in order to also test round-trip + // serialization of the proposal. + let proposal = st + .propose_standard_transfer::( + account_id, + StandardFeeRule::Zip317, + NonZeroU32::new(1).unwrap(), + &tex_addr, + transfer_amount, + None, + change_memo.clone(), + T::SHIELDED_PROTOCOL, + ) + .unwrap(); - // Spendable balance matches total balance - assert_eq!(st.get_total_balance(account.account_id()), value); - assert_eq!(st.get_spendable_balance(account.account_id(), 1), value); + let steps: Vec<_> = proposal.steps().iter().cloned().collect(); + assert_eq!(steps.len(), 2); + + assert_eq!(steps[0].balance().fee_required(), expected_step0_fee); + assert_eq!(steps[1].balance().fee_required(), expected_step1_fee); + assert_eq!( + steps[0].balance().proposed_change(), + [ + ChangeValue::shielded(T::SHIELDED_PROTOCOL, expected_step0_change, change_memo), + ChangeValue::ephemeral_transparent(expected_ephemeral), + ] + ); + assert_eq!(steps[1].balance().proposed_change(), []); - assert_eq!( - block_max_scanned(&st.wallet().conn, &st.wallet().params) - .unwrap() - .unwrap() - .block_height(), - h - ); + let create_proposed_result = st.create_proposed_transactions::( + account.usk(), + OvkPolicy::Sender, + &proposal, + ); + assert_matches!(&create_proposed_result, Ok(txids) if txids.len() == 2); + let txids = create_proposed_result.unwrap(); - // Generate a single-step proposal. Then, instead of executing that proposal, - // we will use its only step as the first step in a multi-step proposal that - // spends the first step's output. + // Verify that the stored sent outputs match what we're expecting. + let mut stmt_sent = st + .wallet() + .conn + .prepare( + "SELECT value, to_address, ephemeral_addresses.address, ephemeral_addresses.address_index + FROM sent_notes + JOIN transactions ON transactions.id_tx = sent_notes.tx + LEFT JOIN ephemeral_addresses ON ephemeral_addresses.used_in_tx = sent_notes.tx + WHERE transactions.txid = ? + ORDER BY value", + ) + .unwrap(); - // The first step will deshield to the wallet's default transparent address - let to0 = Address::Transparent(account.usk().default_transparent_address().0); - let request0 = zip321::TransactionRequest::new(vec![Payment::without_memo( - to0.to_zcash_address(&st.network()), - NonNegativeAmount::const_from_u64(50000), - )]) - .unwrap(); + // Check that there are sent outputs with the correct values. + let confirmed_sent: Vec> = txids + .iter() + .map(|sent_txid| { + stmt_sent + .query(rusqlite::params![sent_txid.as_ref()]) + .unwrap() + .mapped(|row| { + let v: u32 = row.get(0)?; + let to_address: Option = row.get(1)?; + let ephemeral_address: Option = row.get(2)?; + let address_index: Option = row.get(3)?; + Ok((u64::from(v), to_address, ephemeral_address, address_index)) + }) + .collect::, _>>() + .unwrap() + }) + .collect(); + + assert!(expected_step0_change < expected_ephemeral); + assert_eq!(confirmed_sent.len(), 2); + assert_eq!(confirmed_sent[0].len(), 2); + assert_eq!( + confirmed_sent[0][0].0, + u64::try_from(expected_step0_change).unwrap() + ); + let (ephemeral_v, to_addr, ephemeral_addr, index) = confirmed_sent[0][1].clone(); + assert_eq!(ephemeral_v, u64::try_from(expected_ephemeral).unwrap()); + assert!(to_addr.is_some()); + assert_eq!(ephemeral_addr, to_addr); + assert_eq!(index, Some(expected_index)); + + assert_eq!(confirmed_sent[1].len(), 1); + assert_matches!( + confirmed_sent[1][0].clone(), + (sent_v, sent_to_addr, None, None) + if sent_v == u64::try_from(transfer_amount).unwrap() && sent_to_addr == Some(tex_addr.encode(&st.wallet().params))); + + (ephemeral_addr.unwrap(), txids) + }; - let fee_rule = StandardFeeRule::Zip317; - let input_selector = GreedyInputSelector::new( - standard::SingleOutputChangeStrategy::new(fee_rule, None, T::SHIELDED_PROTOCOL), - DustOutputPolicy::default(), + // Each transfer should use a different ephemeral address. + let (ephemeral0, txids0) = run_test(&mut st, 0); + let (ephemeral1, txids1) = run_test(&mut st, 1); + assert_ne!(ephemeral0, ephemeral1); + + let height = add_funds(&mut st, value); + + let ephemeral_taddr = Address::decode(&st.wallet().params, &ephemeral0).expect("valid address"); + assert_matches!( + ephemeral_taddr, + Address::Transparent(TransparentAddress::PublicKeyHash(_)) ); - let proposal0 = st - .propose_transfer( - account.account_id(), - &input_selector, - request0, + + // Attempting to pay to an ephemeral address should cause an error. + let proposal = st + .propose_standard_transfer::( + account_id, + StandardFeeRule::Zip317, NonZeroU32::new(1).unwrap(), + &ephemeral_taddr, + transfer_amount, + None, + None, + T::SHIELDED_PROTOCOL, ) .unwrap(); - let min_target_height = proposal0.min_target_height(); - let step0 = &proposal0.steps().head; + let create_proposed_result = st.create_proposed_transactions::( + account.usk(), + OvkPolicy::Sender, + &proposal, + ); + assert_matches!( + &create_proposed_result, + Err(Error::PaysEphemeralTransparentAddress(address_str)) if address_str == &ephemeral0); + + // Simulate another wallet sending to an ephemeral address with an index + // within the current gap limit. The `PaysEphemeralTransparentAddress` error + // prevents us from doing so straightforwardly, so we'll do it by building + // a transaction and calling `store_decrypted_tx` with it. + let known_addrs = st + .wallet() + .get_known_ephemeral_addresses(account_id, None) + .unwrap(); + assert_eq!(known_addrs.len(), (GAP_LIMIT as usize) + 2); + + // Check that the addresses are all distinct. + let known_set: HashSet<_> = known_addrs.iter().map(|(addr, _)| addr).collect(); + assert_eq!(known_set.len(), known_addrs.len()); + // Check that the metadata is as expected. + for (i, (_, meta)) in known_addrs.iter().enumerate() { + assert_eq!( + meta, + &TransparentAddressMetadata::new( + TransparentKeyScope::EPHEMERAL, + NonHardenedChildIndex::from_index(i.try_into().unwrap()).unwrap() + ) + ); + } - assert_eq!( - step0.balance().proposed_change(), - [ChangeValue::shielded( - T::SHIELDED_PROTOCOL, - NonNegativeAmount::const_from_u64(35000), - None - )] + let mut builder = Builder::new( + st.wallet().params, + height + 1, + BuildConfig::Standard { + sapling_anchor: None, + orchard_anchor: None, + }, ); - assert_eq!( - step0.balance().fee_required(), - NonNegativeAmount::const_from_u64(15000) - ); - - // We'll use an internal transparent address that hasn't been added to the wallet - // to simulate an external transparent recipient. - let to1 = Address::Transparent( - account - .usk() - .transparent() - .to_account_pubkey() - .derive_internal_ivk() - .unwrap() - .default_address() - .0, + let (colliding_addr, _) = &known_addrs[10]; + assert_matches!( + builder.add_transparent_output(colliding_addr, (value - zip317::MINIMUM_FEE).unwrap()), + Ok(_) ); - let request1 = zip321::TransactionRequest::new(vec![Payment::without_memo( - to1.to_zcash_address(&st.network()), - NonNegativeAmount::const_from_u64(40000), - )]) - .unwrap(); - - let step1 = Step::from_parts( - &[step0.clone()], - request1, - [(0, PoolType::TRANSPARENT)].into_iter().collect(), + let sk = account + .usk() + .transparent() + .derive_secret_key(Scope::External.into(), default_index) + .unwrap(); + let outpoint = OutPoint::fake(); + let txout = TxOut { + script_pubkey: default_addr.script(), + value, + }; + assert_matches!(builder.add_transparent_input(sk, outpoint, txout), Ok(_)); + let test_prover = test_prover(); + let build_result = builder + .build( + OsRng, + &test_prover, + &test_prover, + &zip317::FeeRule::standard(), + ) + .unwrap(); + let decrypted_tx = DecryptedTransaction::::new( + build_result.transaction(), vec![], - None, - vec![StepOutput::new(0, StepOutputIndex::Payment(0))], - TransactionBalance::new(vec![], NonNegativeAmount::const_from_u64(10000)).unwrap(), - false, - ) - .unwrap(); + #[cfg(feature = "orchard")] + vec![], + ); + st.wallet_mut().store_decrypted_tx(decrypted_tx).unwrap(); - let proposal = Proposal::multi_step( - fee_rule, - min_target_height, - NonEmpty::from_vec(vec![step0.clone(), step1]).unwrap(), + // That should have advanced the start of the gap to index 11. + let new_known_addrs = st + .wallet() + .get_known_ephemeral_addresses(account_id, None) + .unwrap(); + assert_eq!(new_known_addrs.len(), (GAP_LIMIT as usize) + 11); + assert!(new_known_addrs.starts_with(&known_addrs)); + + let reservation_should_succeed = |st: &mut TestState<_>, n| { + let reserved = st + .wallet_mut() + .reserve_next_n_ephemeral_addresses(account_id, n) + .unwrap(); + assert_eq!(reserved.len(), n); + reserved + }; + let reservation_should_fail = |st: &mut TestState<_>, n, expected_bad_index| { + assert_matches!(st + .wallet_mut() + .reserve_next_n_ephemeral_addresses(account_id, n), + Err(SqliteClientError::ReachedGapLimit(acct, bad_index)) + if acct == account_id && bad_index == expected_bad_index); + }; + + let next_reserved = reservation_should_succeed(&mut st, 1); + assert_eq!(next_reserved[0], known_addrs[11]); + + // Calling `reserve_next_n_ephemeral_addresses(account_id, 1)` will have advanced + // the start of the gap to index 12. This also tests the `index_range` parameter. + let newer_known_addrs = st + .wallet() + .get_known_ephemeral_addresses(account_id, Some(5..100)) + .unwrap(); + assert_eq!(newer_known_addrs.len(), (GAP_LIMIT as usize) + 12 - 5); + assert!(newer_known_addrs.starts_with(&new_known_addrs[5..])); + + // None of the five transactions created above (two from each proposal and the + // one built manually) have been mined yet. So, the range of address indices + // that are safe to reserve is still 0..20, and we have already reserved 12 + // addresses, so trying to reserve another 9 should fail. + reservation_should_fail(&mut st, 9, 20); + reservation_should_succeed(&mut st, 8); + reservation_should_fail(&mut st, 1, 20); + + // Now mine the transaction with the ephemeral output at index 1. + // We already reserved 20 addresses, so this should allow 2 more (..22). + // It does not matter that the transaction with ephemeral output at index 0 + // remains unmined. + let (h, _) = st.generate_next_block_including(txids1.head); + st.scan_cached_blocks(h, 1); + reservation_should_succeed(&mut st, 2); + reservation_should_fail(&mut st, 1, 22); + + // Mining the transaction with the ephemeral output at index 0 at this point + // should make no difference. + let (h, _) = st.generate_next_block_including(txids0.head); + st.scan_cached_blocks(h, 1); + reservation_should_fail(&mut st, 1, 22); + + // Now mine the transaction with the ephemeral output at index 10. + let tx = build_result.transaction(); + let tx_index = 1; + let (h, _) = st.generate_next_block_from_tx(tx_index, tx); + st.scan_cached_blocks(h, 1); + + // The rest of this test would currently fail without the explicit call to + // `put_tx_meta` below. Ideally the above `scan_cached_blocks` would be + // sufficient, but it does not detect the transaction as interesting to the + // wallet. If a transaction is in the database with a null `mined_height`, + // as in this case, its `mined_height` will remain null unless `put_tx_meta` + // is called on it. Normally `put_tx_meta` would be called via `put_blocks` + // as a result of scanning, but that won't happen for any fully transparent + // transaction, and currently it also will not happen for a partially shielded + // transaction unless it is interesting to the wallet for another reason. + // Therefore we will not currently detect either collisions with uses of + // ephemeral outputs by other wallets, or refunds of funds sent to TEX + // addresses. (#1354, #1379) + + // Check that what we say in the above paragraph remains true, so that we + // don't accidentally fix it without updating this test. + reservation_should_fail(&mut st, 1, 22); + + // For now, we demonstrate that this problem is the only obstacle to the rest + // of the ZIP 320 code doing the right thing, by manually calling `put_tx_meta`: + crate::wallet::put_tx_meta( + &st.wallet_mut().conn, + &WalletTx::new( + tx.txid(), + tx_index, + vec![], + vec![], + #[cfg(feature = "orchard")] + vec![], + #[cfg(feature = "orchard")] + vec![], + ), + h, ) .unwrap(); - let create_proposed_result = st.create_proposed_transactions::( - account.usk(), - OvkPolicy::Sender, - &proposal, - ); - assert_matches!(&create_proposed_result, Ok(txids) if txids.len() == 2); - let txids = create_proposed_result.unwrap(); + // We already reserved 22 addresses, so mining the transaction with the + // ephemeral output at index 10 should allow 9 more (..31). + reservation_should_succeed(&mut st, 9); + reservation_should_fail(&mut st, 1, 31); - // Verify that the stored sent outputs match what we're expecting - let mut stmt_sent = st + let newest_known_addrs = st .wallet() - .conn - .prepare( - "SELECT value - FROM sent_notes - JOIN transactions ON transactions.id_tx = sent_notes.tx - WHERE transactions.txid = ?", - ) + .get_known_ephemeral_addresses(account_id, None) .unwrap(); + assert_eq!(newest_known_addrs.len(), (GAP_LIMIT as usize) + 31); + assert!(newest_known_addrs.starts_with(&known_addrs)); + assert!(newest_known_addrs[5..].starts_with(&newer_known_addrs)); +} + +#[cfg(feature = "transparent-inputs")] +pub(crate) fn proposal_fails_if_not_all_ephemeral_outputs_consumed() { + use nonempty::NonEmpty; + use zcash_client_backend::proposal::{Proposal, ProposalError, StepOutput, StepOutputIndex}; + + let mut st = TestBuilder::new() + .with_block_cache() + .with_account_from_sapling_activation(BlockHash([0; 32])) + .build(); + + let account = st.test_account().cloned().unwrap(); + let account_id = account.account_id(); + let dfvk = T::test_account_fvk(&st); - // Check that there are sent outputs with the correct values for each transaction. - let confirmed_sent: Vec> = txids - .iter() - .map(|sent_txid| { - stmt_sent - .query(rusqlite::params![sent_txid.as_ref()]) + let add_funds = |st: &mut TestState<_>, value| { + let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + st.scan_cached_blocks(h, 1); + + assert_eq!( + block_max_scanned(&st.wallet().conn, &st.wallet().params) .unwrap() - .mapped(|row| { - let value: u32 = row.get(0)?; - Ok((sent_txid, value)) - }) - .collect::, _>>() .unwrap() - }) - .collect(); + .block_height(), + h + ); + assert_eq!(st.get_spendable_balance(account_id, 1), value); + }; - assert_eq!( - confirmed_sent.get(0), - Some( - &[(&txids[0], 35000), (&txids[0], 50000)] - .iter() - .cloned() - .collect() - ), + let value = NonNegativeAmount::const_from_u64(100000); + let transfer_amount = NonNegativeAmount::const_from_u64(50000); + + // Add funds to the wallet. + add_funds(&mut st, value); + + // Generate a ZIP 320 proposal, sending to the wallet's default transparent address + // expressed as a TEX address. + let tex_addr = match account.usk().default_transparent_address().0 { + TransparentAddress::PublicKeyHash(data) => Address::Tex(data), + _ => unreachable!(), + }; + + let proposal = st + .propose_standard_transfer::( + account_id, + StandardFeeRule::Zip317, + NonZeroU32::new(1).unwrap(), + &tex_addr, + transfer_amount, + None, + None, + T::SHIELDED_PROTOCOL, + ) + .unwrap(); + + // This is somewhat redundant with `send_multi_step_proposed_transfer`, + // but tests the case with no change memo and ensures we haven't messed + // up the test setup. + let create_proposed_result = st.create_proposed_transactions::( + account.usk(), + OvkPolicy::Sender, + &proposal, ); - assert_eq!( - confirmed_sent.get(1), - Some(&[(&txids[1], 40000)].iter().cloned().collect()), + assert_matches!(create_proposed_result, Ok(_)); + + // Frobnicate the proposal to make it invalid because it does not consume + // the ephemeral output, by truncating it to the first step. + let frobbed_proposal = Proposal::multi_step( + *proposal.fee_rule(), + proposal.min_target_height(), + NonEmpty::singleton(proposal.steps().first().clone()), + ) + .unwrap(); + + let create_proposed_result = st.create_proposed_transactions::( + account.usk(), + OvkPolicy::Sender, + &frobbed_proposal, + ); + assert_matches!( + create_proposed_result, + Err(Error::Proposal(ProposalError::EphemeralOutputLeftUnspent(so))) + if so == StepOutput::new(0, StepOutputIndex::Change(1)) ); } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index d0cf7faf45..b7d0b9230f 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -102,7 +102,7 @@ use zcash_primitives::{ memo::{Memo, MemoBytes}, merkle_tree::read_commitment_tree, transaction::{ - components::{amount::NonNegativeAmount, Amount}, + components::{amount::NonNegativeAmount, Amount, OutPoint}, Transaction, TransactionData, TxId, }, }; @@ -133,6 +133,10 @@ pub(crate) mod transparent; pub(crate) const BLOCK_SAPLING_FRONTIER_ABSENT: &[u8] = &[0x0]; +/// The number of ephemeral addresses that can be safely reserved without observing any +/// of them to be mined. This is the same as the gap limit in Bitcoin. +pub(crate) const GAP_LIMIT: u32 = 20; + fn parse_account_source( account_kind: u32, hd_seed_fingerprint: Option<[u8; 32]>, @@ -509,6 +513,10 @@ pub(crate) fn add_account( let (address, d_idx) = account.default_address(DEFAULT_UA_REQUEST)?; insert_address(conn, params, account_id, d_idx, &address)?; + // Initialize the `ephemeral_addresses` table. + #[cfg(feature = "transparent-inputs")] + transparent::ephemeral::init_account(conn, params, account_id)?; + Ok(account_id) } @@ -1914,9 +1922,11 @@ pub(crate) fn truncate_to_height( } /// Returns a vector with the IDs of all accounts known to this wallet. +/// +/// Note that this is called from db migration code. pub(crate) fn get_account_ids( conn: &rusqlite::Connection, -) -> Result, SqliteClientError> { +) -> Result, rusqlite::Error> { let mut stmt = conn.prepare("SELECT id FROM accounts")?; let mut rows = stmt.query([])?; let mut result = Vec::new(); @@ -2136,11 +2146,21 @@ pub(crate) fn put_tx_data( // A utility function for creation of parameters for use in `insert_sent_output` // and `put_sent_output` -fn recipient_params( - to: &Recipient, +fn recipient_params( + params: &P, + to: &Recipient, ) -> (Option, Option, PoolType) { match to { Recipient::External(addr, pool) => (Some(addr.encode()), None, *pool), + Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + .. + } => ( + Some(ephemeral_address.encode(params)), + Some(*receiving_account), + PoolType::TRANSPARENT, + ), Recipient::InternalAccount { receiving_account, external_address, @@ -2154,8 +2174,9 @@ fn recipient_params( } /// Records information about a transaction output that your wallet created. -pub(crate) fn insert_sent_output( +pub(crate) fn insert_sent_output( conn: &rusqlite::Connection, + params: &P, tx_ref: i64, from_account: AccountId, output: &SentTransactionOutput, @@ -2169,7 +2190,7 @@ pub(crate) fn insert_sent_output( :to_address, :to_account_id, :value, :memo)", )?; - let (to_address, to_account_id, pool_type) = recipient_params(output.recipient()); + let (to_address, to_account_id, pool_type) = recipient_params(params, output.recipient()); let sql_args = named_params![ ":tx": &tx_ref, ":output_pool": &pool_code(pool_type), @@ -2198,12 +2219,13 @@ pub(crate) fn insert_sent_output( /// - If `recipient` is an internal account, `output_index` is an index into the Sapling outputs of /// the transaction. #[allow(clippy::too_many_arguments)] -pub(crate) fn put_sent_output( +pub(crate) fn put_sent_output( conn: &rusqlite::Connection, + params: &P, from_account: AccountId, tx_ref: i64, output_index: usize, - recipient: &Recipient, + recipient: &Recipient, value: NonNegativeAmount, memo: Option<&MemoBytes>, ) -> Result<(), SqliteClientError> { @@ -2222,7 +2244,7 @@ pub(crate) fn put_sent_output( memo = IFNULL(:memo, memo)", )?; - let (to_address, to_account_id, pool_type) = recipient_params(recipient); + let (to_address, to_account_id, pool_type) = recipient_params(params, recipient); let sql_args = named_params![ ":tx": &tx_ref, ":output_pool": &pool_code(pool_type), diff --git a/zcash_client_sqlite/src/wallet/common.rs b/zcash_client_sqlite/src/wallet/common.rs index 800d7f660d..3bcdbc97f5 100644 --- a/zcash_client_sqlite/src/wallet/common.rs +++ b/zcash_client_sqlite/src/wallet/common.rs @@ -158,7 +158,7 @@ where INNER JOIN transactions ON transactions.id_tx = {table_prefix}_received_notes.tx WHERE {table_prefix}_received_notes.account_id = :account - AND value >= 5000 -- FIXME #1016, allow selection of a dust inputs + AND value > 5000 -- FIXME #1316, allow selection of dust inputs AND accounts.ufvk IS NOT NULL AND recipient_key_scope IS NOT NULL AND nf IS NOT NULL diff --git a/zcash_client_sqlite/src/wallet/db.rs b/zcash_client_sqlite/src/wallet/db.rs index 6e54eb3dcf..65ce3b148e 100644 --- a/zcash_client_sqlite/src/wallet/db.rs +++ b/zcash_client_sqlite/src/wallet/db.rs @@ -13,10 +13,12 @@ // from showing up in `cargo doc --document-private-items`. #![allow(dead_code)] +use static_assertions::const_assert_eq; + use zcash_client_backend::data_api::scanning::ScanPriority; use zcash_protocol::consensus::{NetworkUpgrade, Parameters}; -use crate::wallet::scanning::priority_code; +use crate::wallet::{scanning::priority_code, GAP_LIMIT}; /// Stores information about the accounts that the wallet is tracking. pub(super) const TABLE_ACCOUNTS: &str = r#" @@ -76,6 +78,66 @@ CREATE INDEX "addresses_accounts" ON "addresses" ( "account_id" ASC )"#; +/// Stores ephemeral transparent addresses used for ZIP 320. +/// +/// For each account, these addresses are allocated sequentially by address index under scope 2 +/// (`TransparentKeyScope::EPHEMERAL`) at the "change" level of the BIP 32 address hierarchy. +/// The ephemeral addresses stored in the table are exactly the "reserved" ephemeral addresses +/// (that is addresses that have been allocated for use in a ZIP 320 transaction proposal), plus +/// the addresses at the next `GAP_LIMIT` indices. +/// +/// Addresses are never removed. New ones should only be reserved via the +/// `WalletWrite::reserve_next_n_ephemeral_addresses` API. All of the addresses in the table +/// should be scanned for incoming funds. +/// +/// ### Columns +/// - `address` contains the string (Base58Check) encoding of a transparent P2PKH address. +/// - `used_in_tx` indicates that the address has been used by this wallet in a transaction (which +/// has not necessarily been mined yet). This should only be set once, when the txid is known. +/// - `seen_in_tx` is non-null iff an output to the address has been seed in a transaction observed +/// on the network and passed to `store_decrypted_tx`. The transaction may have been sent by this +// wallet or another one using the same seed, or by a TEX address recipient sending back the +/// funds. This is used to advance the "gap", as well as to heuristically reduce the chance of +/// address reuse collisions with another wallet using the same seed. +/// +/// It is an external invariant that within each account: +/// - the address indices are contiguous and start from 0; +/// - the last `GAP_LIMIT` addresses have `used_in_tx` and `seen_in_tx` both NULL. +/// +/// All but the last `GAP_LIMIT` addresses are defined to be "reserved" addresses. Since the next +/// index to reserve is determined by dead reckoning from the last stored address, we use dummy +/// entries having `NULL` for the value of the `address` column after the maximum valid index in +/// order to allow the last `GAP_LIMIT` addresses at the end of the index range to be used. +/// +/// Note that the fact that `used_in_tx` references a specific transaction is just a debugging aid. +/// The same is mostly true of `seen_in_tx`, but we also take into account whether the referenced +/// transaction is unmined in order to determine the last index that is safe to reserve. +pub(super) const TABLE_EPHEMERAL_ADDRESSES: &str = r#" +CREATE TABLE ephemeral_addresses ( + account_id INTEGER NOT NULL, + address_index INTEGER NOT NULL, + -- nullability of this column is controlled by the index_range_and_address_nullity check + address TEXT, + used_in_tx INTEGER, + seen_in_tx INTEGER, + FOREIGN KEY (account_id) REFERENCES accounts(id), + FOREIGN KEY (used_in_tx) REFERENCES transactions(id_tx), + FOREIGN KEY (seen_in_tx) REFERENCES transactions(id_tx), + PRIMARY KEY (account_id, address_index), + CONSTRAINT ephemeral_addr_uniq UNIQUE (address), + CONSTRAINT used_implies_seen CHECK ( + used_in_tx IS NULL OR seen_in_tx IS NOT NULL + ), + CONSTRAINT index_range_and_address_nullity CHECK ( + (address_index BETWEEN 0 AND 0x7FFFFFFF AND address IS NOT NULL) OR + (address_index BETWEEN 0x80000000 AND 0x7FFFFFFF + 20 AND address IS NULL AND used_in_tx IS NULL AND seen_in_tx IS NULL) + ) +) WITHOUT ROWID"#; +// Hexadecimal integer literals were added in SQLite version 3.8.6 (2014-08-15). +// libsqlite3-sys requires at least version 3.14.0. +// "WITHOUT ROWID" tells SQLite to use a clustered index on the (composite) primary key. +const_assert_eq!(GAP_LIMIT, 20); + /// Stores information about every block that the wallet has scanned. /// /// Note that this table does not contain any rows for blocks that the wallet might have diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 08838d0486..7c5a27f73e 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -176,6 +176,14 @@ fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> Wallet SqliteClientError::ChainHeightUnknown => { unreachable!("we don't call methods that require a known chain height") } + #[cfg(feature = "transparent-inputs")] + SqliteClientError::ReachedGapLimit(_, _) => { + unreachable!("we don't do ephemeral address tracking") + } + #[cfg(feature = "transparent-inputs")] + SqliteClientError::EphemeralAddressReuse(_, _) => { + unreachable!("we don't do ephemeral address tracking") + } } } @@ -376,6 +384,7 @@ mod tests { db::TABLE_ACCOUNTS, db::TABLE_ADDRESSES, db::TABLE_BLOCKS, + db::TABLE_EPHEMERAL_ADDRESSES, db::TABLE_NULLIFIER_MAP, db::TABLE_ORCHARD_RECEIVED_NOTE_SPENDS, db::TABLE_ORCHARD_RECEIVED_NOTES, diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index 1f4befd996..3e6b056b9e 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -3,6 +3,7 @@ mod add_transaction_views; mod add_utxo_account; mod addresses_table; mod ensure_orchard_ua_receiver; +mod ephemeral_addresses; mod full_account_ids; mod initial_setup; mod nullifier_map; @@ -66,6 +67,8 @@ pub(super) fn all_migrations( // orchard_received_notes // / \ // ensure_orchard_ua_receiver utxos_to_txos + // | + // ephemeral_addresses vec![ Box::new(initial_setup::Migration {}), Box::new(utxos_table::Migration {}), @@ -116,6 +119,9 @@ pub(super) fn all_migrations( params: params.clone(), }), Box::new(utxos_to_txos::Migration), + Box::new(ephemeral_addresses::Migration { + params: params.clone(), + }), ] } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs new file mode 100644 index 0000000000..d9dffaf89b --- /dev/null +++ b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs @@ -0,0 +1,201 @@ +//! The migration that records ephemeral addresses for each account. +use std::collections::HashSet; + +use rusqlite; +use schemer; +use schemer_rusqlite::RusqliteMigration; +use uuid::Uuid; +use zcash_protocol::consensus; + +use crate::wallet::init::WalletMigrationError; + +#[cfg(feature = "transparent-inputs")] +use crate::wallet::{self, init, transparent::ephemeral}; + +use super::utxos_to_txos; + +pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0x0e1d4274_1f8e_44e2_909d_689a4bc2967b); + +const DEPENDENCIES: [Uuid; 1] = [utxos_to_txos::MIGRATION_ID]; + +#[allow(dead_code)] +pub(super) struct Migration

{ + pub(super) params: P, +} + +impl

schemer::Migration for Migration

{ + fn id(&self) -> Uuid { + MIGRATION_ID + } + + fn dependencies(&self) -> HashSet { + DEPENDENCIES.into_iter().collect() + } + + fn description(&self) -> &'static str { + "Record ephemeral addresses for each account." + } +} + +impl RusqliteMigration for Migration

{ + type Error = WalletMigrationError; + + fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + transaction.execute_batch( + "CREATE TABLE ephemeral_addresses ( + account_id INTEGER NOT NULL, + address_index INTEGER NOT NULL, + -- nullability of this column is controlled by the index_range_and_address_nullity check + address TEXT, + used_in_tx INTEGER, + seen_in_tx INTEGER, + FOREIGN KEY (account_id) REFERENCES accounts(id), + FOREIGN KEY (used_in_tx) REFERENCES transactions(id_tx), + FOREIGN KEY (seen_in_tx) REFERENCES transactions(id_tx), + PRIMARY KEY (account_id, address_index), + CONSTRAINT ephemeral_addr_uniq UNIQUE (address), + CONSTRAINT used_implies_seen CHECK ( + used_in_tx IS NULL OR seen_in_tx IS NOT NULL + ), + CONSTRAINT index_range_and_address_nullity CHECK ( + (address_index BETWEEN 0 AND 0x7FFFFFFF AND address IS NOT NULL) OR + (address_index BETWEEN 0x80000000 AND 0x7FFFFFFF + 20 AND address IS NULL AND used_in_tx IS NULL AND seen_in_tx IS NULL) + ) + ) WITHOUT ROWID;" + )?; + + // Make sure that at least `GAP_LIMIT` ephemeral transparent addresses are + // stored in each account. + #[cfg(feature = "transparent-inputs")] + for account_id in wallet::get_account_ids(transaction)? { + ephemeral::init_account(transaction, &self.params, account_id) + .map_err(init::sqlite_client_error_to_wallet_migration_error)?; + } + Ok(()) + } + + fn down(&self, _transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + Err(WalletMigrationError::CannotRevert(MIGRATION_ID)) + } +} + +#[cfg(test)] +mod tests { + use crate::wallet::init::migrations::tests::test_migrate; + + #[test] + fn migrate() { + test_migrate(&[super::MIGRATION_ID]); + } + + #[test] + #[cfg(feature = "transparent-inputs")] + fn initialize_table() { + use rusqlite::named_params; + use secrecy::Secret; + use tempfile::NamedTempFile; + use zcash_client_backend::{ + data_api::{AccountBirthday, AccountSource, WalletWrite}, + wallet::TransparentAddressMetadata, + }; + use zcash_keys::keys::UnifiedSpendingKey; + use zcash_primitives::{block::BlockHash, legacy::keys::NonHardenedChildIndex}; + use zcash_protocol::consensus::Network; + use zip32::{fingerprint::SeedFingerprint, AccountId as Zip32AccountId}; + + use crate::{ + error::SqliteClientError, + wallet::{ + account_kind_code, init::init_wallet_db_internal, transparent::ephemeral, GAP_LIMIT, + }, + WalletDb, + }; + + let network = Network::TestNetwork; + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); + + let seed0 = vec![0x00; 32]; + init_wallet_db_internal( + &mut db_data, + Some(Secret::new(seed0.clone())), + &super::DEPENDENCIES, + false, + ) + .unwrap(); + + let birthday = AccountBirthday::from_sapling_activation(&network, BlockHash([0; 32])); + + // Simulate creating an account prior to this migration. + let account0_index = Zip32AccountId::ZERO; + let account0_seed_fp = [0u8; 32]; + let account0_kind = account_kind_code(AccountSource::Derived { + seed_fingerprint: SeedFingerprint::from_seed(&account0_seed_fp).unwrap(), + account_index: account0_index, + }); + assert_eq!(u32::from(account0_index), 0); + let account0_id = crate::AccountId(0); + + let usk0 = UnifiedSpendingKey::from_seed(&network, &seed0, account0_index).unwrap(); + let ufvk0 = usk0.to_unified_full_viewing_key(); + let uivk0 = ufvk0.to_unified_incoming_viewing_key(); + + db_data + .conn + .execute( + "INSERT INTO accounts (id, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, birthday_height) + VALUES (:id, :account_kind, :hd_seed_fingerprint, :hd_account_index, :ufvk, :uivk, :birthday_height)", + named_params![ + ":id": account0_id.0, + ":account_kind": account0_kind, + ":hd_seed_fingerprint": account0_seed_fp, + ":hd_account_index": u32::from(account0_index), + ":ufvk": ufvk0.encode(&network), + ":uivk": uivk0.encode(&network), + ":birthday_height": u32::from(birthday.height()), + ], + ) + .unwrap(); + + // The `ephemeral_addresses` table is expected not to exist before migration. + assert_matches!( + ephemeral::first_unstored_index(&db_data.conn, account0_id), + Err(SqliteClientError::DbError(_)) + ); + + let check = |db: &WalletDb<_, _>, account_id| { + eprintln!("checking {account_id:?}"); + assert_matches!(ephemeral::first_unstored_index(&db.conn, account_id), Ok(addr_index) if addr_index == GAP_LIMIT); + assert_matches!(ephemeral::first_unreserved_index(&db.conn, account_id), Ok(addr_index) if addr_index == 0); + + let known_addrs = + ephemeral::get_known_ephemeral_addresses(&db.conn, &db.params, account_id, None) + .unwrap(); + + let expected_metadata: Vec = (0..GAP_LIMIT) + .map(|i| ephemeral::metadata(NonHardenedChildIndex::from_index(i).unwrap())) + .collect(); + let actual_metadata: Vec = + known_addrs.into_iter().map(|(_, meta)| meta).collect(); + assert_eq!(actual_metadata, expected_metadata); + }; + + // The migration should initialize `ephemeral_addresses`. + init_wallet_db_internal( + &mut db_data, + Some(Secret::new(seed0)), + &[super::MIGRATION_ID], + false, + ) + .unwrap(); + check(&db_data, account0_id); + + // Creating a new account should initialize `ephemeral_addresses` for that account. + let seed1 = vec![0x01; 32]; + let (account1_id, _usk) = db_data + .create_account(&Secret::new(seed1), &birthday) + .unwrap(); + assert_ne!(account0_id, account1_id); + check(&db_data, account1_id); + } +} diff --git a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs index cd1f768455..722c609bb5 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs @@ -367,7 +367,7 @@ mod tests { usk0.transparent() .derive_external_secret_key(NonHardenedChildIndex::ZERO) .unwrap(), - transparent::OutPoint::new([1; 32], 0), + transparent::OutPoint::fake(), transparent::TxOut { value: NonNegativeAmount::const_from_u64(EXTERNAL_VALUE + INTERNAL_VALUE), script_pubkey: usk0 diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 6f28719d50..88e7f66f75 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -575,6 +575,12 @@ pub(crate) mod tests { testing::pool::send_multi_step_proposed_transfer::() } + #[test] + #[cfg(feature = "transparent-inputs")] + fn proposal_fails_if_not_all_ephemeral_outputs_consumed() { + testing::pool::proposal_fails_if_not_all_ephemeral_outputs_consumed::() + } + #[test] #[allow(deprecated)] fn create_to_address_fails_on_incorrect_usk() { diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 10834346c6..b43ce4a908 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -581,6 +581,12 @@ pub(crate) mod tests { testing::pool::send_multi_step_proposed_transfer::() } + #[test] + #[cfg(feature = "transparent-inputs")] + fn proposal_fails_if_not_all_ephemeral_outputs_consumed() { + testing::pool::proposal_fails_if_not_all_ephemeral_outputs_consumed::() + } + #[test] #[allow(deprecated)] fn create_to_address_fails_on_incorrect_usk() { diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs index 181bc30c8b..000c515e2b 100644 --- a/zcash_client_sqlite/src/wallet/transparent.rs +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -1,15 +1,16 @@ //! Functions for transparent input support in the wallet. +use std::collections::{HashMap, HashSet}; + use rusqlite::OptionalExtension; use rusqlite::{named_params, Connection, Row}; -use std::collections::HashMap; -use std::collections::HashSet; -use zcash_client_backend::data_api::AccountBalance; -use zcash_keys::address::Address; use zip32::{DiversifierIndex, Scope}; use zcash_address::unified::{Encoding, Ivk, Uivk}; -use zcash_client_backend::wallet::{TransparentAddressMetadata, WalletTransparentOutput}; -use zcash_keys::encoding::AddressCodec; +use zcash_client_backend::{ + data_api::AccountBalance, + wallet::{TransparentAddressMetadata, WalletTransparentOutput}, +}; +use zcash_keys::{address::Address, encoding::AddressCodec}; use zcash_primitives::{ legacy::{ keys::{IncomingViewingKey, NonHardenedChildIndex}, @@ -23,6 +24,8 @@ use crate::{error::SqliteClientError, AccountId, UtxoId}; use super::{chain_tip_height, get_account_ids}; +pub(crate) mod ephemeral; + pub(crate) fn detect_spending_accounts<'a>( conn: &Connection, spent: impl Iterator, @@ -51,6 +54,28 @@ pub(crate) fn detect_spending_accounts<'a>( Ok(acc) } +/// Returns the `NonHardenedChildIndex` corresponding to a diversifier index +/// given as bytes in big-endian order (the reverse of the usual order). +fn address_index_from_diversifier_index_be( + diversifier_index_be: &[u8], +) -> Result { + let mut di: [u8; 11] = diversifier_index_be.try_into().map_err(|_| { + SqliteClientError::CorruptedData("Diversifier index is not an 11-byte value".to_owned()) + })?; + di.reverse(); // BE -> LE conversion + + NonHardenedChildIndex::from_index(DiversifierIndex::from(di).try_into().map_err(|_| { + SqliteClientError::CorruptedData( + "Unable to get diversifier for transparent address.".to_string(), + ) + })?) + .ok_or_else(|| { + SqliteClientError::CorruptedData( + "Unexpected hardened index for transparent address.".to_string(), + ) + }) +} + pub(crate) fn get_transparent_receivers( conn: &rusqlite::Connection, params: &P, @@ -67,10 +92,6 @@ pub(crate) fn get_transparent_receivers( while let Some(row) = rows.next()? { let ua_str: String = row.get(0)?; let di_vec: Vec = row.get(1)?; - let mut di: [u8; 11] = di_vec.try_into().map_err(|_| { - SqliteClientError::CorruptedData("Diversifier index is not an 11-byte value".to_owned()) - })?; - di.reverse(); // BE -> LE conversion let ua = Address::decode(params, &ua_str) .ok_or_else(|| { @@ -85,37 +106,15 @@ pub(crate) fn get_transparent_receivers( })?; if let Some(taddr) = ua.transparent() { - let index = NonHardenedChildIndex::from_index( - DiversifierIndex::from(di).try_into().map_err(|_| { - SqliteClientError::CorruptedData( - "Unable to get diversifier for transparent address.".to_string(), - ) - })?, - ) - .ok_or_else(|| { - SqliteClientError::CorruptedData( - "Unexpected hardened index for transparent address.".to_string(), - ) - })?; - - ret.insert( - *taddr, - Some(TransparentAddressMetadata::new( - Scope::External.into(), - index, - )), - ); + let address_index = address_index_from_diversifier_index_be(&di_vec)?; + let metadata = TransparentAddressMetadata::new(Scope::External.into(), address_index); + ret.insert(*taddr, Some(metadata)); } } if let Some((taddr, address_index)) = get_legacy_transparent_address(params, conn, account)? { - ret.insert( - taddr, - Some(TransparentAddressMetadata::new( - Scope::External.into(), - address_index, - )), - ); + let metadata = TransparentAddressMetadata::new(Scope::External.into(), address_index); + ret.insert(taddr, Some(metadata)); } Ok(ret) @@ -239,6 +238,19 @@ pub(crate) fn get_unspent_transparent_output( result } +/// Returns the list of spendable transparent outputs received by this wallet at `address` +/// such that, at height `target_height`: +/// * the transaction that produced the output had or will have at least `min_confirmations` +/// confirmations; and +/// * the output is unspent as of the current chain tip. +/// +/// An output that is potentially spent by an unmined transaction in the mempool is excluded +/// iff the spending transaction will not be expired at `target_height`. +/// +/// This could, in very rare circumstances, return as unspent outputs that are actually not +/// spendable, if they are the outputs of deshielding transactions where the spend anchors have +/// been invalidated by a rewind. There isn't a way to detect this circumstance at present, but +/// it should be vanishingly rare as the vast majority of rewinds are of a single block. pub(crate) fn get_spendable_transparent_outputs( conn: &rusqlite::Connection, params: &P, @@ -248,10 +260,6 @@ pub(crate) fn get_spendable_transparent_outputs( ) -> Result, SqliteClientError> { let confirmed_height = target_height - min_confirmations; - // This could, in very rare circumstances, return as unspent outputs that are actually not - // spendable, if they are the outputs of deshielding transactions where the spend anchors have - // been invalidated by a rewind. There isn't a way to detect this circumstance at present, but - // it should be vanishingly rare as the vast majority of rewinds are of a single block. let mut stmt_utxos = conn.prepare( "SELECT t.txid, u.output_index, u.script, u.value_zat, t.mined_height AS received_height @@ -301,10 +309,13 @@ pub(crate) fn get_spendable_transparent_outputs( Ok(utxos) } -/// Returns the unspent balance for each transparent address associated with the specified account, -/// such that the block that included the transaction was mined at a height less than or equal to -/// the provided `summary_height`. -pub(crate) fn get_transparent_address_balances( +/// Returns a mapping from each transparent receiver associated with the specified account +/// to its not-yet-shielded UTXO balance, including only the effects of transactions mined +/// at a block height less than or equal to `summary_height`. +/// +/// Only non-ephemeral transparent receivers with a non-zero balance at the summary height +/// will be included. +pub(crate) fn get_transparent_balances( conn: &rusqlite::Connection, params: &P, account: AccountId, @@ -438,63 +449,145 @@ pub(crate) fn put_received_transparent_utxo( params: &P, output: &WalletTransparentOutput, ) -> Result { - let address_str = output.recipient_address().encode(params); - let account_id = conn + let address = output.recipient_address(); + if let Some(receiving_account) = find_account_for_transparent_address(conn, params, address)? { + put_transparent_output( + conn, + params, + output.outpoint(), + output.txout(), + Some(output.height()), + address, + receiving_account, + ) + } else { + // The UTXO was not for any of our transparent addresses. + Err(SqliteClientError::AddressNotRecognized(*address)) + } +} + +pub(crate) fn get_transparent_address_metadata( + conn: &rusqlite::Connection, + params: &P, + account_id: AccountId, + address: &TransparentAddress, +) -> Result, SqliteClientError> { + let address_str = address.encode(params); + + if let Some(di_vec) = conn + .query_row( + "SELECT diversifier_index_be FROM addresses + WHERE account_id = :account_id AND cached_transparent_receiver_address = :address", + named_params![":account_id": account_id.0, ":address": &address_str], + |row| row.get::<_, Vec>(0), + ) + .optional()? + { + let address_index = address_index_from_diversifier_index_be(&di_vec)?; + let metadata = TransparentAddressMetadata::new(Scope::External.into(), address_index); + return Ok(Some(metadata)); + } + + if let Some((legacy_taddr, address_index)) = + get_legacy_transparent_address(params, conn, account_id)? + { + if &legacy_taddr == address { + let metadata = TransparentAddressMetadata::new(Scope::External.into(), address_index); + return Ok(Some(metadata)); + } + } + + // Search known ephemeral addresses. + if let Some(address_index) = + ephemeral::find_index_for_ephemeral_address_str(conn, account_id, &address_str)? + { + return Ok(Some(ephemeral::metadata(address_index))); + } + + Ok(None) +} + +/// Attempts to determine the account that received the given transparent output. +/// +/// The following three locations in the wallet's key tree are searched: +/// - Transparent receivers that have been generated as part of a Unified Address. +/// - Transparent ephemeral addresses that have been reserved or are within +/// the gap limit from the last reserved address. +/// - "Legacy transparent addresses" (at BIP 44 address index 0 within an account). +/// +/// Returns `Ok(None)` if the transparent output's recipient address is not in any of the +/// above locations. This means the wallet considers the output "not interesting". +pub(crate) fn find_account_for_transparent_address( + conn: &rusqlite::Connection, + params: &P, + address: &TransparentAddress, +) -> Result, SqliteClientError> { + let address_str = address.encode(params); + + if let Some(account_id) = conn .query_row( "SELECT account_id FROM addresses WHERE cached_transparent_receiver_address = :address", named_params![":address": &address_str], |row| Ok(AccountId(row.get(0)?)), ) - .optional()?; + .optional()? + { + return Ok(Some(account_id)); + } - if let Some(account) = account_id { - Ok(put_transparent_output(conn, params, output, account)?) - } else { - // If the UTXO is received at the legacy transparent address (at BIP 44 address - // index 0 within its particular account, which we specifically ensure is returned - // from `get_transparent_receivers`), there may be no entry in the addresses table - // that can be used to tie the address to a particular account. In this case, we - // look up the legacy address for each account in the wallet, and check whether it - // matches the address for the received UTXO; if so, insert/update it directly. - get_account_ids(conn)? - .into_iter() - .find_map( - |account| match get_legacy_transparent_address(params, conn, account) { - Ok(Some((legacy_taddr, _))) if &legacy_taddr == output.recipient_address() => { - Some( - put_transparent_output(conn, params, output, account) - .map_err(SqliteClientError::from), - ) - } - Ok(_) => None, - Err(e) => Some(Err(e)), - }, - ) - // The UTXO was not for any of the legacy transparent addresses. - .unwrap_or_else(|| { - Err(SqliteClientError::AddressNotRecognized( - *output.recipient_address(), - )) - }) + // Search known ephemeral addresses. + if let Some(account_id) = ephemeral::find_account_for_ephemeral_address_str(conn, &address_str)? + { + return Ok(Some(account_id)); } + + let account_ids = get_account_ids(conn)?; + + // If the UTXO is received at the legacy transparent address (at BIP 44 address + // index 0 within its particular account, which we specifically ensure is returned + // from `get_transparent_receivers`), there may be no entry in the addresses table + // that can be used to tie the address to a particular account. In this case, we + // look up the legacy address for each account in the wallet, and check whether it + // matches the address for the received UTXO. + for &account_id in account_ids.iter() { + if let Some((legacy_taddr, _)) = get_legacy_transparent_address(params, conn, account_id)? { + if &legacy_taddr == address { + return Ok(Some(account_id)); + } + } + } + + Ok(None) } +/// Add a transparent output relevant to this wallet to the database. +/// +/// `output_height` may be None if this is an ephemeral output from a +/// transaction we created, that we do not yet know to have been mined. pub(crate) fn put_transparent_output( conn: &rusqlite::Connection, params: &P, - output: &WalletTransparentOutput, - received_by_account: AccountId, -) -> Result { + outpoint: &OutPoint, + txout: &TxOut, + output_height: Option, + address: &TransparentAddress, + receiving_account: AccountId, +) -> Result { + let output_height = output_height.map(u32::from); + // Check whether we have an entry in the blocks table for the output height; // if not, the transaction will be updated with its mined height when the // associated block is scanned. - let block = conn - .query_row( - "SELECT height FROM blocks WHERE height = :height", - named_params![":height": &u32::from(output.height())], - |row| row.get::<_, u32>(0), - ) - .optional()?; + let block = match output_height { + Some(height) => conn + .query_row( + "SELECT height FROM blocks WHERE height = :height", + named_params![":height": height], + |row| row.get::<_, u32>(0), + ) + .optional()?, + None => None, + }; let id_tx = conn.query_row( "INSERT INTO transactions (txid, block, mined_height) @@ -504,9 +597,9 @@ pub(crate) fn put_transparent_output( mined_height = :mined_height RETURNING id_tx", named_params![ - ":txid": &output.outpoint().hash().to_vec(), + ":txid": &outpoint.hash().to_vec(), ":block": block, - ":mined_height": u32::from(output.height()) + ":mined_height": output_height ], |row| row.get::<_, i64>(0), )?; @@ -533,15 +626,17 @@ pub(crate) fn put_transparent_output( let sql_args = named_params![ ":transaction_id": id_tx, - ":output_index": &output.outpoint().n(), - ":account_id": received_by_account.0, - ":address": &output.recipient_address().encode(params), - ":script": &output.txout().script_pubkey.0, - ":value_zat": &i64::from(Amount::from(output.txout().value)), - ":height": &u32::from(output.height()), + ":output_index": &outpoint.n(), + ":account_id": receiving_account.0, + ":address": &address.encode(params), + ":script": &txout.script_pubkey.0, + ":value_zat": &i64::from(Amount::from(txout.value)), + ":height": output_height, ]; - stmt_upsert_transparent_output.query_row(sql_args, |row| row.get::<_, i64>(0).map(UtxoId)) + let utxo_id = stmt_upsert_transparent_output + .query_row(sql_args, |row| row.get::<_, i64>(0).map(UtxoId))?; + Ok(utxo_id) } #[cfg(test)] diff --git a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs new file mode 100644 index 0000000000..017ad002f9 --- /dev/null +++ b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs @@ -0,0 +1,445 @@ +//! Functions for wallet support of ephemeral transparent addresses. +use std::cmp::{max, min}; +use std::ops::Range; + +use rusqlite::{named_params, OptionalExtension}; + +use zcash_client_backend::{data_api::Account, wallet::TransparentAddressMetadata}; +use zcash_keys::{encoding::AddressCodec, keys::AddressGenerationError}; +use zcash_primitives::{ + legacy::{ + keys::{EphemeralIvk, NonHardenedChildIndex, TransparentKeyScope}, + TransparentAddress, + }, + transaction::TxId, +}; +use zcash_protocol::consensus; + +use crate::{ + error::SqliteClientError, + wallet::{get_account, GAP_LIMIT}, + AccountId, SqlTransaction, WalletDb, +}; + +// Returns `TransparentAddressMetadata` in the ephemeral scope for the +// given address index. +pub(crate) fn metadata(address_index: NonHardenedChildIndex) -> TransparentAddressMetadata { + TransparentAddressMetadata::new(TransparentKeyScope::EPHEMERAL, address_index) +} + +/// Returns the first unstored ephemeral address index in the given account. +pub(crate) fn first_unstored_index( + conn: &rusqlite::Connection, + account_id: AccountId, +) -> Result { + match conn + .query_row( + "SELECT address_index FROM ephemeral_addresses + WHERE account_id = :account_id + ORDER BY address_index DESC + LIMIT 1", + named_params![":account_id": account_id.0], + |row| row.get::<_, u32>(0), + ) + .optional()? + { + Some(i) if i >= (1 << 31) + GAP_LIMIT => { + unreachable!("violates constraint index_range_and_address_nullity") + } + Some(i) => Ok(i.checked_add(1).unwrap()), + None => Ok(0), + } +} + +/// Returns the first unreserved ephemeral address index in the given account. +pub(crate) fn first_unreserved_index( + conn: &rusqlite::Connection, + account_id: AccountId, +) -> Result { + first_unstored_index(conn, account_id)? + .checked_sub(GAP_LIMIT) + .ok_or(SqliteClientError::CorruptedData( + "ephemeral_addresses table has not been initialized".to_owned(), + )) +} + +/// Returns the first ephemeral address index in the given account that +/// would violate the gap invariant if used. +pub(crate) fn first_unsafe_index( + conn: &rusqlite::Connection, + account_id: AccountId, +) -> Result { + // The inner join with `transactions` excludes addresses for which + // `seen_in_tx` is NULL. The query also excludes addresses observed + // to have been mined in a transaction that we currently see as unmined. + // This is conservative in terms of avoiding violation of the gap + // invariant: it can only cause us to get to the end of the gap sooner. + // + // TODO: do we want to only consider transactions with a minimum number + // of confirmations here? + let first_unmined_index: u32 = match conn + .query_row( + "SELECT address_index FROM ephemeral_addresses + JOIN transactions t ON t.id_tx = seen_in_tx + WHERE account_id = :account_id AND t.mined_height IS NOT NULL + ORDER BY address_index DESC + LIMIT 1", + named_params![":account_id": account_id.0], + |row| row.get::<_, u32>(0), + ) + .optional()? + { + Some(i) if i >= 1 << 31 => { + unreachable!("violates constraint index_range_and_address_nullity") + } + Some(i) => i.checked_add(1).unwrap(), + None => 0, + }; + Ok(min( + 1 << 31, + first_unmined_index.checked_add(GAP_LIMIT).unwrap(), + )) +} + +/// Utility function to return an `Range` that starts at `i` +/// and is of length up to `n`. The range is truncated if necessary +/// so that it contains no elements beyond the maximum valid address +/// index, `(1 << 31) - 1`. +pub(crate) fn range_from(i: u32, n: u32) -> Range { + let first = min(1 << 31, i); + let last = min(1 << 31, i.saturating_add(n)); + first..last +} + +/// Returns the ephemeral transparent IVK for a given account ID. +pub(crate) fn get_ephemeral_ivk( + conn: &rusqlite::Connection, + params: &P, + account_id: AccountId, +) -> Result { + Ok(get_account(conn, params, account_id)? + .ok_or(SqliteClientError::AccountUnknown)? + .ufvk() + .and_then(|ufvk| ufvk.transparent()) + .ok_or(SqliteClientError::UnknownZip32Derivation)? + .derive_ephemeral_ivk()?) +} + +/// Returns a vector of ephemeral transparent addresses associated with the given +/// account controlled by this wallet, along with their metadata. The result includes +/// reserved addresses, and addresses for `GAP_LIMIT` additional indices (capped to +/// the maximum index). +/// +/// If `index_range` is some `Range`, it limits the result to addresses with indices +/// in that range. +pub(crate) fn get_known_ephemeral_addresses( + conn: &rusqlite::Connection, + params: &P, + account_id: AccountId, + index_range: Option>, +) -> Result, SqliteClientError> { + let index_range = index_range.unwrap_or(0..(1 << 31)); + + let mut stmt = conn.prepare( + "SELECT address, address_index FROM ephemeral_addresses + WHERE account_id = :account AND address_index >= :start AND address_index < :end + ORDER BY address_index", + )?; + let mut rows = stmt.query(named_params![ + ":account": account_id.0, + ":start": index_range.start, + ":end": min(1 << 31, index_range.end), + ])?; + + let mut result = vec![]; + + while let Some(row) = rows.next()? { + let addr_str: String = row.get(0)?; + let raw_index: u32 = row.get(1)?; + let address_index = NonHardenedChildIndex::from_index(raw_index) + .expect("where clause ensures this is in range"); + let address = TransparentAddress::decode(params, &addr_str)?; + result.push((address, metadata(address_index))); + } + Ok(result) +} + +/// If this is a known ephemeral address in any account, return its account id. +pub(crate) fn find_account_for_ephemeral_address_str( + conn: &rusqlite::Connection, + address_str: &str, +) -> Result, SqliteClientError> { + Ok(conn + .query_row( + "SELECT account_id FROM ephemeral_addresses WHERE address = :address", + named_params![":address": &address_str], + |row| Ok(AccountId(row.get(0)?)), + ) + .optional()?) +} + +/// If this is a known ephemeral address in the given account, return its index. +pub(crate) fn find_index_for_ephemeral_address_str( + conn: &rusqlite::Connection, + account_id: AccountId, + address_str: &str, +) -> Result, SqliteClientError> { + Ok(conn + .query_row( + "SELECT address_index FROM ephemeral_addresses + WHERE account_id = :account_id AND address = :address", + named_params![":account_id": account_id.0, ":address": &address_str], + |row| row.get::<_, u32>(0), + ) + .optional()? + .map(|index| { + NonHardenedChildIndex::from_index(index) + .expect("valid by constraint index_range_and_address_nullity") + })) +} + +/// Returns a vector with the next `n` previously unreserved ephemeral addresses for +/// the given account. +/// +/// # Errors +/// +/// * `SqliteClientError::AccountUnknown`, if there is no account with the given id. +/// * `SqliteClientError::UnknownZip32Derivation`, if the account is imported and +/// it is not possible to derive new addresses for it. +/// * `SqliteClientError::ReachedGapLimit`, if it is not possible to reserve `n` addresses +/// within the gap limit after the last address in this account that is known to have an +/// output in a mined transaction. +/// * `SqliteClientError::AddressGeneration(AddressGenerationError::DiversifierSpaceExhausted)`, +/// if the limit on transparent address indices has been reached. +pub(crate) fn reserve_next_n_ephemeral_addresses( + wdb: &mut WalletDb, P>, + account_id: AccountId, + n: usize, +) -> Result, SqliteClientError> { + if n == 0 { + return Ok(vec![]); + } + + let first_unreserved = first_unreserved_index(wdb.conn.0, account_id)?; + let first_unsafe = first_unsafe_index(wdb.conn.0, account_id)?; + let allocation = range_from( + first_unreserved, + u32::try_from(n).map_err(|_| AddressGenerationError::DiversifierSpaceExhausted)?, + ); + + if allocation.len() < n { + return Err(AddressGenerationError::DiversifierSpaceExhausted.into()); + } + if allocation.end > first_unsafe { + return Err(SqliteClientError::ReachedGapLimit( + account_id, + max(first_unreserved, first_unsafe), + )); + } + reserve_until(wdb.conn.0, &wdb.params, account_id, allocation.end)?; + get_known_ephemeral_addresses(wdb.conn.0, &wdb.params, account_id, Some(allocation)) +} + +/// Initialize the `ephemeral_addresses` table. This must be called when +/// creating or migrating an account. +pub(crate) fn init_account( + conn: &rusqlite::Transaction, + params: &P, + account_id: AccountId, +) -> Result<(), SqliteClientError> { + reserve_until(conn, params, account_id, 0) +} + +/// Extend the range of stored addresses in an account if necessary so that the +/// index of the next address to reserve will be *at least* `next_to_reserve`. +/// If it would already have been at least `next_to_reserve`, then do nothing. +/// +/// Note that this is called from db migration code. +/// +/// # Panics +/// +/// Panics if the precondition `next_to_reserve <= (1 << 31)` does not hold. +fn reserve_until( + conn: &rusqlite::Transaction, + params: &P, + account_id: AccountId, + next_to_reserve: u32, +) -> Result<(), SqliteClientError> { + assert!(next_to_reserve <= 1 << 31); + + let first_unstored = first_unstored_index(conn, account_id)?; + let range_to_store = first_unstored..(next_to_reserve.checked_add(GAP_LIMIT).unwrap()); + if range_to_store.is_empty() { + return Ok(()); + } + + let ephemeral_ivk = get_ephemeral_ivk(conn, params, account_id)?; + + // used_in_tx and seen_in_tx are initially NULL + let mut stmt_insert_ephemeral_address = conn.prepare_cached( + "INSERT INTO ephemeral_addresses (account_id, address_index, address) + VALUES (:account_id, :address_index, :address)", + )?; + + for raw_index in range_to_store { + // The range to store may contain indicies that are out of the valid range of non hardened + // child indices; we still store explicit rows in the ephemeral_addresses table for these + // so that it's possible to find the first unused address using dead reckoning with the gap + // limit. + let address_str_opt = NonHardenedChildIndex::from_index(raw_index) + .map(|address_index| { + ephemeral_ivk + .derive_ephemeral_address(address_index) + .map(|addr| addr.encode(params)) + }) + .transpose()?; + + stmt_insert_ephemeral_address.execute(named_params![ + ":account_id": account_id.0, + ":address_index": raw_index, + ":address": address_str_opt, + ])?; + } + Ok(()) +} + +/// Returns a `SqliteClientError::EphemeralAddressReuse` error if the address was +/// already used. +fn ephemeral_address_reuse_check( + wdb: &mut WalletDb, P>, + address_str: &str, +) -> Result<(), SqliteClientError> { + // It is intentional that we don't require `t.mined_height` to be non-null. + // That is, we conservatively treat an ephemeral address as potentially + // reused even if we think that the transaction where we had evidence of + // its use is at present unmined. This should never occur in supported + // situations where only a single correctly operating wallet instance is + // using a given seed, because such a wallet will not reuse an address that + // it ever reserved. + // + // `COALESCE(used_in_tx, seen_in_tx)` can only differ from `used_in_tx` + // if the address was reserved, an error occurred in transaction creation + // before calling `mark_ephemeral_address_as_used`, and then we saw the + // address in another transaction (presumably created by another wallet + // instance, or as a result of a bug) anyway. + let res = wdb + .conn + .0 + .query_row( + "SELECT t.txid FROM ephemeral_addresses + LEFT OUTER JOIN transactions t + ON t.id_tx = COALESCE(used_in_tx, seen_in_tx) + WHERE address = :address", + named_params![":address": address_str], + |row| row.get::<_, Option>>(0), + ) + .optional()? + .flatten(); + + if let Some(txid_bytes) = res { + let txid = TxId::from_bytes( + txid_bytes + .try_into() + .map_err(|_| SqliteClientError::CorruptedData("invalid txid".to_owned()))?, + ); + Err(SqliteClientError::EphemeralAddressReuse( + address_str.to_owned(), + txid, + )) + } else { + Ok(()) + } +} + +/// If `address` is one of our ephemeral addresses, mark it as having an output +/// in a transaction that we have just created. This has no effect if `address` is +/// not one of our ephemeral addresses. +/// +/// Returns a `SqliteClientError::EphemeralAddressReuse` error if the address was +/// already used. +pub(crate) fn mark_ephemeral_address_as_used( + wdb: &mut WalletDb, P>, + ephemeral_address: &TransparentAddress, + tx_ref: i64, +) -> Result<(), SqliteClientError> { + let address_str = ephemeral_address.encode(&wdb.params); + ephemeral_address_reuse_check(wdb, &address_str)?; + + // We update both `used_in_tx` and `seen_in_tx` here, because a used address has + // necessarily been seen in a transaction. We will not treat this as extending the + // range of addresses that are safe to reserve unless and until the transaction is + // observed as mined. + let update_result = wdb + .conn + .0 + .query_row( + "UPDATE ephemeral_addresses + SET used_in_tx = :tx_ref, seen_in_tx = :tx_ref + WHERE address = :address + RETURNING account_id, address_index", + named_params![":tx_ref": &tx_ref, ":address": address_str], + |row| Ok((AccountId(row.get::<_, u32>(0)?), row.get::<_, u32>(1)?)), + ) + .optional()?; + + // Maintain the invariant that the last `GAP_LIMIT` addresses are unused and unseen. + if let Some((account_id, address_index)) = update_result { + let next_to_reserve = address_index.checked_add(1).expect("ensured by constraint"); + reserve_until(wdb.conn.0, &wdb.params, account_id, next_to_reserve)?; + } + Ok(()) +} + +/// If `address` is one of our ephemeral addresses, mark it as having an output +/// in the given mined transaction (which may or may not be a transaction we sent). +/// +/// `tx_ref` must be a valid transaction reference. This call has no effect if +/// `address` is not one of our ephemeral addresses. +pub(crate) fn mark_ephemeral_address_as_seen( + wdb: &mut WalletDb, P>, + address: &TransparentAddress, + tx_ref: i64, +) -> Result<(), SqliteClientError> { + let address_str = address.encode(&wdb.params); + + // Figure out which transaction was mined earlier: `tx_ref`, or any existing + // tx referenced by `seen_in_tx` for the given address. Prefer the existing + // reference in case of a tie or if both transactions are unmined. + // This slightly reduces the chance of unnecessarily reaching the gap limit + // too early in some corner cases (because the earlier transaction is less + // likely to be unmined). + // + // The query should always return a value if `tx_ref` is valid. + let earlier_ref = wdb.conn.0.query_row( + "SELECT id_tx FROM transactions + LEFT OUTER JOIN ephemeral_addresses e + ON id_tx = e.seen_in_tx + WHERE id_tx = :tx_ref OR e.address = :address + ORDER BY mined_height ASC NULLS LAST, + tx_index ASC NULLS LAST, + e.seen_in_tx ASC NULLS LAST + LIMIT 1", + named_params![":tx_ref": &tx_ref, ":address": address_str], + |row| row.get::<_, i64>(0), + )?; + + let update_result = wdb + .conn + .0 + .query_row( + "UPDATE ephemeral_addresses + SET seen_in_tx = :seen_in_tx + WHERE address = :address + RETURNING account_id, address_index", + named_params![":seen_in_tx": &earlier_ref, ":address": address_str], + |row| Ok((AccountId(row.get::<_, u32>(0)?), row.get::<_, u32>(1)?)), + ) + .optional()?; + + // Maintain the invariant that the last `GAP_LIMIT` addresses are unused and unseen. + if let Some((account_id, address_index)) = update_result { + let next_to_reserve = address_index.checked_add(1).expect("ensured by constraint"); + reserve_until(wdb.conn.0, &wdb.params, account_id, next_to_reserve)?; + } + Ok(()) +} diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 9e2d78a6f8..ddd5850c71 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -11,6 +11,10 @@ and this library adheres to Rust's notion of - `impl From for bip32::ChildNumber` - `impl From for bip32::ChildNumber` - `impl TryFrom for NonHardenedChildIndex` + - `EphemeralIvk` + - `AccountPubKey::derive_ephemeral_ivk` + - `TransparentKeyScope::custom` is now `const`. + - `TransparentKeyScope::{EXTERNAL, INTERNAL, EPHEMERAL}` - `zcash_primitives::legacy::Script::serialized_size` - `zcash_primitives::transaction::fees::transparent`: - `InputSize` @@ -41,6 +45,14 @@ and this library adheres to Rust's notion of (expressed as iterators of `InputSize` for inputs and `usize` for outputs) rather than a slice of `InputView` or `OutputView`. +### Deprecated +- `zcash_primitives::transaction::fees::zip317::FeeRule::non_standard` has been + deprecated, because in general it can calculate fees that violate ZIP 317, which + might cause transactions built with it to fail. Maintaining the generality of the + current implementation imposes ongoing maintenance costs, and so it is likely to + be removed in the near future. Use `transaction::fees::zip317::FeeRule::standard()` + instead to comply with ZIP 317. + ### Removed - The `zcash_primitives::zip339` module, which reexported parts of the API of the `bip0039` crate, has been removed. Use the `bip0039` crate directly diff --git a/zcash_primitives/src/legacy/keys.rs b/zcash_primitives/src/legacy/keys.rs index 8357762cc1..25a20eed1a 100644 --- a/zcash_primitives/src/legacy/keys.rs +++ b/zcash_primitives/src/legacy/keys.rs @@ -21,20 +21,36 @@ use super::TransparentAddress; pub struct TransparentKeyScope(u32); impl TransparentKeyScope { - pub fn custom(i: u32) -> Option { + /// Returns an arbitrary custom `TransparentKeyScope`. + /// + /// This should be used with care: funds associated with keys derived under a custom + /// scope may not be recoverable if the wallet seed is restored in another wallet. It + /// is usually preferable to use standardized key scopes. + pub const fn custom(i: u32) -> Option { if i < (1 << 31) { Some(TransparentKeyScope(i)) } else { None } } + + /// The scope used to derive keys for external transparent addresses, + /// intended to be used to send funds to this wallet. + pub const EXTERNAL: Self = TransparentKeyScope(0); + + /// The scope used to derive keys for internal wallet operations, e.g. + /// change or UTXO management. + pub const INTERNAL: Self = TransparentKeyScope(1); + + /// The scope used to derive keys for ephemeral transparent addresses. + pub const EPHEMERAL: Self = TransparentKeyScope(2); } impl From for TransparentKeyScope { fn from(value: zip32::Scope) -> Self { match value { - zip32::Scope::External => TransparentKeyScope(0), - zip32::Scope::Internal => TransparentKeyScope(1), + zip32::Scope::External => TransparentKeyScope::EXTERNAL, + zip32::Scope::Internal => TransparentKeyScope::INTERNAL, } } } @@ -223,6 +239,14 @@ impl AccountPubKey { .map(InternalIvk) } + /// Derives the public key at the "ephemeral" path + /// `m/44'/'/'/2`. + pub fn derive_ephemeral_ivk(&self) -> Result { + self.0 + .derive_child(ChildNumber::new(2, false)?) + .map(EphemeralIvk) + } + /// Derives the internal ovk and external ovk corresponding to this /// transparent fvk. As specified in [ZIP 316][transparent-ovk]. /// @@ -405,6 +429,26 @@ impl private::SealedChangeLevelKey for InternalIvk { impl IncomingViewingKey for InternalIvk {} +/// An incoming viewing key at the "ephemeral" path +/// `m/44'/'/'/2`. +/// +/// This allows derivation of ephemeral addresses for use within the wallet. +#[derive(Clone, Debug)] +pub struct EphemeralIvk(ExtendedPublicKey); + +#[cfg(feature = "transparent-inputs")] +impl EphemeralIvk { + /// Derives a transparent address at the provided child index. + pub fn derive_ephemeral_address( + &self, + address_index: NonHardenedChildIndex, + ) -> Result { + let child_key = self.0.derive_child(address_index.into())?; + #[allow(deprecated)] + Ok(pubkey_to_address(child_key.public_key())) + } +} + /// Internal outgoing viewing key used for autoshielding. pub struct InternalOvk([u8; 32]); diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 8304c19f80..47c0995028 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -1003,7 +1003,7 @@ mod tests { .add_transparent_input( tsk.derive_external_secret_key(NonHardenedChildIndex::ZERO) .unwrap(), - OutPoint::new([0u8; 32], 1), + OutPoint::fake(), prev_coin, ) .unwrap(); diff --git a/zcash_primitives/src/transaction/fees/transparent.rs b/zcash_primitives/src/transaction/fees/transparent.rs index 8beb394b54..a2fbfd4526 100644 --- a/zcash_primitives/src/transaction/fees/transparent.rs +++ b/zcash_primitives/src/transaction/fees/transparent.rs @@ -16,6 +16,7 @@ use crate::transaction::components::transparent::builder::TransparentInputInfo; /// The size of a transparent input, or the outpoint corresponding to the input /// if the size of the script required to spend that input is unknown. +#[derive(Clone, Debug, PartialEq, Eq)] pub enum InputSize { /// The txin size is known. Known(usize), diff --git a/zcash_primitives/src/transaction/fees/zip317.rs b/zcash_primitives/src/transaction/fees/zip317.rs index 1d5d7513ac..48457b8007 100644 --- a/zcash_primitives/src/transaction/fees/zip317.rs +++ b/zcash_primitives/src/transaction/fees/zip317.rs @@ -78,6 +78,13 @@ impl FeeRule { /// /// Returns `None` if either `p2pkh_standard_input_size` or `p2pkh_standard_output_size` are /// zero. + #[deprecated( + note = "Using this fee rule with `marginal_fee < 5000 || grace_actions < 2 \ + || p2pkh_standard_input_size > P2PKH_STANDARD_INPUT_SIZE \ + || p2pkh_standard_output_size > P2PKH_STANDARD_OUTPUT_SIZE` \ + violates ZIP 317, and might cause transactions built with it to fail. \ + This API is likely to be removed. Use `[FeeRule::standard]` instead." + )] pub fn non_standard( marginal_fee: NonNegativeAmount, grace_actions: usize,