diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index ca0463cde5..973197e36b 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -6,10 +6,31 @@ 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. This is intended to be used in conjunction with +`zcash_client_sqlite` 0.11 or later. + +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. + - `AddressTrackingError` + - `impl From for wallet::input_selection::InputSelectorError` + - `WalletAddressTracking` trait, implemented for types that implement the other + `Wallet*` traits. - `zcash_client_backend::scanning`: - `testing` module - `zcash_client_backend::sync` module, behind the `sync` feature flag. @@ -21,12 +42,15 @@ and this library adheres to Rust's notion of CHANGELOG for details. - `zcash_client_backend::data_api`: - `error::Error` has a new `Address` variant. - - `wallet::input_selection::InputSelectorError` has a new `Address` variant. + - `wallet::input_selection::InputSelectorError` has new `Address` and + `AddressTracking` variants. - `zcash_client_backend::proto::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 + - `Recipient` variants have changed. It has a new `EphemeralTransparent` + variant, and an extra generic parameter giving the type of metadata about + an ephemeral transparent outpoint. Instead of 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. @@ -127,6 +151,7 @@ and this library adheres to Rust's notion of - `fn with_orchard_tree_mut` - `fn put_orchard_subtree_roots` - Removed `Error::AccountNotFound` variant. + - Added `Error::AddressTracking` variant. - `WalletSummary::new` now takes an additional `next_orchard_subtree_index` argument when the `orchard` feature flag is enabled. - `zcash_client_backend::decrypt`: diff --git a/zcash_client_backend/proto/proposal.proto b/zcash_client_backend/proto/proposal.proto index 950bb3406c..ef1c0732bc 100644 --- a/zcash_client_backend/proto/proposal.proto +++ b/zcash_client_backend/proto/proposal.proto @@ -113,6 +113,8 @@ enum FeeRule { // The proposed change outputs and fee value. message TransactionBalance { // A list of change output values. + // Any `ChangeValue`s for the transparent value pool represent ephemeral + // outputs that will each be given a unique t-address. repeated ChangeValue proposedChange = 1; // The fee to be paid by the proposed transaction, in zatoshis. uint64 feeRequired = 2; diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 2721219ae5..635cc3b433 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -30,18 +30,19 @@ //! //! ## Core Traits //! -//! The utility functions described above depend upon four important traits defined in this +//! The utility functions described above depend upon five important traits defined in this //! module, which between them encompass the data storage requirements of a light wallet. -//! The relevant traits are [`InputSource`], [`WalletRead`], [`WalletWrite`], and -//! [`WalletCommitmentTrees`]. A complete implementation of the data storage layer for a wallet -//! will include an implementation of all four of these traits. See the [`zcash_client_sqlite`] -//! crate for a complete example of the implementation of these traits. +//! The relevant traits are [`InputSource`], [`WalletRead`], [`WalletWrite`], +//! [`WalletCommitmentTrees`], and [`WalletAddressTracking`]. A complete implementation of the +//! data storage layer for a wallet will include an implementation of all five of these traits. +//! See the [`zcash_client_sqlite`] crate for a complete example of the implementation of these +//! traits. //! //! ## Accounts //! -//! The operation of the [`InputSource`], [`WalletRead`] and [`WalletWrite`] traits is built around -//! the concept of a wallet having one or more accounts, with a unique `AccountId` for each -//! account. +//! The operation of the [`InputSource`], [`WalletRead`], [`WalletWrite`], and +//! [`WalletAddressTracking`] traits is built around the concept of a wallet having one or more +//! accounts, with a unique `AccountId` for each account. //! //! An account identifier corresponds to at most a single [`UnifiedSpendingKey`]'s worth of spend //! authority, with the received and spent notes of that account tracked via the corresponding @@ -57,7 +58,7 @@ use std::{ collections::HashMap, - fmt::Debug, + fmt::{self, Debug, Display}, hash::Hash, io, num::{NonZeroU32, TryFromIntError}, @@ -86,18 +87,19 @@ use crate::{ use zcash_primitives::{ block::BlockHash, consensus::BlockHeight, + legacy::TransparentAddress, 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}, -}; +use crate::wallet::TransparentAddressMetadata; #[cfg(any(test, feature = "test-dependencies"))] use zcash_primitives::consensus::NetworkUpgrade; @@ -1239,7 +1241,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, } @@ -1256,7 +1258,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 { @@ -1278,8 +1280,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. @@ -1514,8 +1516,11 @@ pub trait WalletWrite: WalletRead { received_tx: DecryptedTransaction, ) -> Result<(), Self::Error>; - /// Saves information about a transaction that was constructed and sent by the wallet to the - /// persistent wallet store. + /// Saves information about a transaction constructed by the wallet to the persistent + /// wallet store. + /// + /// The name `store_sent_tx` is somewhat misleading; this must be called *before* the + /// transaction is sent to the network. fn store_sent_tx( &mut self, sent_tx: &SentTransaction, @@ -1608,6 +1613,97 @@ pub trait WalletCommitmentTrees { ) -> Result<(), ShardTreeError>; } +/// An error related to tracking of ephemeral transparent addresses. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AddressTrackingError { + /// The account id could not be found. + AccountNotFound, + + /// The proposal cannot be constructed until transactions with previously reserved + /// ephemeral address outputs have been mined. + ReachedGapLimit, + + /// Internal error. + Internal(String), +} + +impl Display for AddressTrackingError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + AddressTrackingError::AccountNotFound => write!( + f, + "The account id could not be found." + ), + AddressTrackingError::ReachedGapLimit => write!( + f, + "The proposal cannot be constructed until transactions with previously reserved ephemeral address outputs have been mined." + ), + AddressTrackingError::Internal(e) => write!( + f, + "Internal address tracking error: {}", e + ), + } + } +} + +/// Wallet operations required for tracking of ephemeral transparent addresses. +/// +/// This trait serves to allow the corresponding wallet functions to be abstracted +/// away from any particular data storage substrate. +pub trait WalletAddressTracking { + /// The type of the account identifier. + type AccountId: Copy + Debug + Eq + Hash; + + /// Reserves the next available address. + /// + /// 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]. An implementation should record + /// the index of the first unmined address, and update it for addresses that have + /// been observed as outputs in mined transactions when `addresses_are_mined` is + /// called. + /// + /// Returns an error if all addresses within the gap limit have already been + /// reserved. + /// + /// [BIP 44]: https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#user-content-Address_gap_limit + fn reserve_next_address( + &self, + account_id: Self::AccountId, + ) -> Result; + + /// Frees previously reserved ephemeral transparent addresses. + /// + /// This should only be used in the case that an error occurs in transaction + /// construction after the address was reserved. It is sufficient for an + /// implementation to only be able to unreserve the addresses that were last + /// reserved in the given account. + /// + /// Returns an error if the account identifier does not correspond to a known + /// account. + fn unreserve_addresses( + &self, + account_id: Self::AccountId, + address: &[TransparentAddress], + ) -> Result<(), AddressTrackingError>; + + /// Mark addresses as having been used. + fn mark_addresses_as_used( + &self, + account_id: Self::AccountId, + address: &[TransparentAddress], + ) -> Result<(), AddressTrackingError>; + + /// Checks the set of ephemeral transparent addresses within the gap limit for the + /// given mined t-addresses, in order to update the first unmined ephemeral t-address + /// index if necessary. + fn mark_addresses_as_mined( + &self, + account_id: Self::AccountId, + addresses: &[TransparentAddress], + ) -> Result<(), AddressTrackingError>; +} + #[cfg(feature = "test-dependencies")] pub mod testing { use incrementalmerkletree::Address; @@ -1619,6 +1715,7 @@ pub mod testing { use zcash_primitives::{ block::BlockHash, consensus::{BlockHeight, Network}, + legacy::TransparentAddress, memo::Memo, transaction::{components::amount::NonNegativeAmount, Transaction, TxId}, }; @@ -1633,13 +1730,14 @@ pub mod testing { use super::{ chain::{ChainState, CommitmentTreeRoot}, scanning::ScanRange, - AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, - ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, WalletCommitmentTrees, - WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + AccountBirthday, AddressTrackingError, BlockMetadata, DecryptedTransaction, InputSource, + NullifierQuery, ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, + WalletAddressTracking, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, + SAPLING_SHARD_HEIGHT, }; #[cfg(feature = "transparent-inputs")] - use {crate::wallet::TransparentAddressMetadata, zcash_primitives::legacy::TransparentAddress}; + use crate::wallet::TransparentAddressMetadata; #[cfg(feature = "orchard")] use super::ORCHARD_SHARD_HEIGHT; @@ -1926,6 +2024,41 @@ pub mod testing { } } + impl WalletAddressTracking for MockWalletDb { + type AccountId = u32; + + fn reserve_next_address( + &self, + _account_id: Self::AccountId, + ) -> Result { + Err(AddressTrackingError::ReachedGapLimit) + } + + fn unreserve_addresses( + &self, + _account_id: Self::AccountId, + _addresses: &[TransparentAddress], + ) -> Result<(), AddressTrackingError> { + Ok(()) + } + + fn mark_addresses_as_used( + &self, + _account_id: Self::AccountId, + _addresses: &[TransparentAddress], + ) -> Result<(), AddressTrackingError> { + Ok(()) + } + + fn mark_addresses_as_mined( + &self, + _account_id: Self::AccountId, + _addresses: &[TransparentAddress], + ) -> Result<(), AddressTrackingError> { + Ok(()) + } + } + impl WalletCommitmentTrees for MockWalletDb { type Error = Infallible; type SaplingShardStore<'a> = MemoryShardStore; diff --git a/zcash_client_backend/src/data_api/error.rs b/zcash_client_backend/src/data_api/error.rs index 2c10db70ca..4a49441e30 100644 --- a/zcash_client_backend/src/data_api/error.rs +++ b/zcash_client_backend/src/data_api/error.rs @@ -21,6 +21,8 @@ use zcash_primitives::legacy::TransparentAddress; use crate::wallet::NoteId; +use super::AddressTrackingError; + /// Errors that can occur as a consequence of wallet operations. #[derive(Debug)] pub enum Error { @@ -87,6 +89,9 @@ pub enum Error { #[cfg(feature = "transparent-inputs")] AddressNotRecognized(TransparentAddress), + + /// An error related to tracking of ephemeral transparent addresses. + AddressTracking(AddressTrackingError), } impl fmt::Display for Error @@ -156,6 +161,9 @@ where Error::AddressNotRecognized(_) => { write!(f, "The specified transparent address was not recognized as belonging to the wallet.") } + Error::AddressTracking(e) => { + write!(f, "Error related to tracking of ephemeral transparent addresses: {}", e) + } } } } @@ -212,6 +220,7 @@ impl From> for Error }, InputSelectorError::SyncRequired => Error::ScanRequired, InputSelectorError::Address(e) => Error::Address(e), + InputSelectorError::AddressTracking(e) => Error::AddressTracking(e), } } } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 0bbdcbd913..529bc9a492 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -41,7 +41,7 @@ use sapling::{ }; use std::num::NonZeroU32; -use super::InputSource; +use super::{InputSource, WalletAddressTracking}; use crate::{ address::Address, data_api::{ @@ -58,7 +58,7 @@ use crate::{ }; use zcash_primitives::transaction::{ builder::{BuildConfig, BuildResult, Builder}, - components::{amount::NonNegativeAmount, sapling::zip212_enforcement}, + components::{amount::NonNegativeAmount, sapling::zip212_enforcement, OutPoint}, fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule}, Transaction, TxId, }; @@ -70,11 +70,9 @@ use zip32::Scope; #[cfg(feature = "transparent-inputs")] use { - input_selection::ShieldingSelector, - std::convert::Infallible, - zcash_keys::encoding::AddressCodec, - zcash_primitives::legacy::TransparentAddress, - zcash_primitives::transaction::components::{OutPoint, TxOut}, + input_selection::ShieldingSelector, std::convert::Infallible, + zcash_keys::encoding::AddressCodec, zcash_primitives::legacy::TransparentAddress, + zcash_primitives::transaction::components::TxOut, }; pub mod input_selection; @@ -258,6 +256,7 @@ where AccountId = ::AccountId, >, DbT: WalletCommitmentTrees, + DbT: WalletAddressTracking::AccountId>, { let account = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) @@ -369,6 +368,7 @@ where AccountId = ::AccountId, >, DbT: WalletCommitmentTrees, + DbT: WalletAddressTracking::AccountId>, ParamsT: consensus::Parameters + Clone, InputsT: InputSelector, { @@ -603,6 +603,7 @@ pub fn create_proposed_transactions( > where DbT: WalletWrite + WalletCommitmentTrees, + DbT: WalletAddressTracking::AccountId>, ParamsT: consensus::Parameters + Clone, FeeRuleT: FeeRule, { @@ -656,37 +657,40 @@ fn create_proposed_transaction( > where DbT: WalletWrite + WalletCommitmentTrees, + DbT: WalletAddressTracking::AccountId>, 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. + // Spending shielded outputs of prior multi-step transaction steps (either payments or change) + // is not supported. + // + // TODO: 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 + // 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(()) + |step| { + let prior_pool = match s_ref.output_index() { + proposal::StepOutputIndex::Payment(i) => { + step.0.payment_pools().get(&i).cloned() } + proposal::StepOutputIndex::Change(i) => step + .0 + .balance() + .proposed_change() + .get(i) + .map(|change| change.output_pool()), } - 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) + .ok_or(Error::Proposal(ProposalError::ReferenceError(*s_ref)))?; + + // Return an error on trying to spend a prior shielded output. + match prior_pool { + PoolType::Shielded(_) => Err(Error::ProposalNotSupported), + PoolType::Transparent => Ok(()), } }, )?; @@ -695,8 +699,8 @@ where let account = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) .map_err(Error::DataSource)? - .ok_or(Error::KeyNotRecognized)? - .id(); + .ok_or(Error::KeyNotRecognized)?; + let account_id = account.id(); let (sapling_anchor, sapling_inputs) = if proposal_step.involves(PoolType::Shielded(ShieldedProtocol::Sapling)) { @@ -800,8 +804,9 @@ where #[cfg(feature = "transparent-inputs")] let utxos_spent = { + // FIXME: We need this to include transparent ephemeral addresses. let known_addrs = wallet_db - .get_transparent_receivers(account) + .get_transparent_receivers(account_id) .map_err(Error::DataSource)?; let mut utxos_spent: Vec = vec![]; @@ -858,12 +863,15 @@ where .clone() .convert_if_network(params.network_type())?; + // Address::Tex should not be able to occur here for a proposal made by propose_transaction. + // They can occur for a deserialized proposal and are invalid in that case. 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( @@ -1052,44 +1060,26 @@ where } } - for change_value in proposal_step.balance().proposed_change() { - let memo = change_value - .memo() - .map_or_else(MemoBytes::empty, |m| m.clone()); - let output_pool = change_value.output_pool(); - match output_pool { - PoolType::Shielded(ShieldedProtocol::Sapling) => { - builder.add_sapling_output( - sapling_internal_ovk(), - sapling_dfvk.change_address().1, - change_value.value(), - memo.clone(), - )?; - sapling_output_meta.push(( - Recipient::InternalAccount { - receiving_account: account, - external_address: None, - note: output_pool, - }, - change_value.value(), - Some(memo), - )) - } - PoolType::Shielded(ShieldedProtocol::Orchard) => { - #[cfg(not(feature = "orchard"))] - return Err(Error::UnsupportedChangeType(output_pool)); - - #[cfg(feature = "orchard")] - { - builder.add_orchard_output( - orchard_internal_ovk(), - orchard_fvk.address_at(0u32, orchard::keys::Scope::Internal), - change_value.value().into(), + #[cfg(feature = "transparent-inputs")] + let mut ephemeral_addrs = vec![]; + + let finish = || { + for change_value in proposal_step.balance().proposed_change() { + let memo = change_value + .memo() + .map_or_else(MemoBytes::empty, |m| m.clone()); + let output_pool = change_value.output_pool(); + match output_pool { + PoolType::Shielded(ShieldedProtocol::Sapling) => { + builder.add_sapling_output( + sapling_internal_ovk(), + sapling_dfvk.change_address().1, + change_value.value(), memo.clone(), )?; - orchard_output_meta.push(( + sapling_output_meta.push(( Recipient::InternalAccount { - receiving_account: account, + receiving_account: account_id, external_address: None, note: output_pool, }, @@ -1097,121 +1087,181 @@ where Some(memo), )) } - } - PoolType::Transparent => { - return Err(Error::UnsupportedChangeType(output_pool)); + PoolType::Shielded(ShieldedProtocol::Orchard) => { + #[cfg(not(feature = "orchard"))] + return Err(Error::UnsupportedChangeType(output_pool)); + + #[cfg(feature = "orchard")] + { + builder.add_orchard_output( + orchard_internal_ovk(), + orchard_fvk.address_at(0u32, orchard::keys::Scope::Internal), + change_value.value().into(), + memo.clone(), + )?; + orchard_output_meta.push(( + Recipient::InternalAccount { + receiving_account: account_id, + external_address: None, + note: output_pool, + }, + change_value.value(), + Some(memo), + )) + } + } + PoolType::Transparent => { + #[cfg(not(feature = "transparent-inputs"))] + return Err(Error::UnsupportedChangeType(output_pool)); + + #[cfg(feature = "transparent-inputs")] + { + // This is intended for an ephemeral transparent output, rather than a + // non-ephemeral transparent change output. That is, there should be another + // request, although we cannot check that here. + let ephemeral_address = wallet_db + .reserve_next_address(account_id) + .map_err(InputSelectorError::from)?; + + ephemeral_addrs.push(ephemeral_address); + builder.add_transparent_output(&ephemeral_address, change_value.value())?; + transparent_output_meta.push(( + Recipient::EphemeralTransparent { + receiving_account: account_id, + ephemeral_address, + outpoint: output_pool, + }, + ephemeral_address, + change_value.value(), + )) + } + } } } - } - // Build the transaction with the specified fee rule - let build_result = builder.build(OsRng, spend_prover, output_prover, fee_rule)?; + // Build the transaction with the specified fee rule + let build_result = builder.build(OsRng, spend_prover, output_prover, fee_rule)?; + + #[cfg(feature = "orchard")] + let orchard_internal_ivk = orchard_fvk.to_ivk(orchard::keys::Scope::Internal); + #[cfg(feature = "orchard")] + let orchard_outputs = + orchard_output_meta + .into_iter() + .enumerate() + .map(|(i, (recipient, value, memo))| { + let output_index = build_result.orchard_meta().output_action_index(i).expect( + "An action should exist in the transaction for each Orchard output.", + ); - #[cfg(feature = "orchard")] - let orchard_internal_ivk = orchard_fvk.to_ivk(orchard::keys::Scope::Internal); - #[cfg(feature = "orchard")] - let orchard_outputs = - orchard_output_meta - .into_iter() - .enumerate() - .map(|(i, (recipient, value, memo))| { - let output_index = build_result - .orchard_meta() - .output_action_index(i) - .expect("An action should exist in the transaction for each Orchard output."); - - let recipient = recipient - .map_internal_account_note(|pool| { - assert!(pool == PoolType::Shielded(ShieldedProtocol::Orchard)); - build_result - .transaction() - .orchard_bundle() - .and_then(|bundle| { - bundle - .decrypt_output_with_key(output_index, &orchard_internal_ivk) - .map(|(note, _, _)| Note::Orchard(note)) - }) - }) - .internal_account_note_transpose_option() - .expect("Wallet-internal outputs must be decryptable with the wallet's IVK"); - - SentTransactionOutput::from_parts(output_index, recipient, value, memo) - }); - - let sapling_internal_ivk = - PreparedIncomingViewingKey::new(&sapling_dfvk.to_ivk(Scope::Internal)); - let sapling_outputs = - sapling_output_meta - .into_iter() - .enumerate() - .map(|(i, (recipient, value, memo))| { - let output_index = build_result - .sapling_meta() - .output_index(i) - .expect("An output should exist in the transaction for each Sapling payment."); - - let recipient = recipient - .map_internal_account_note(|pool| { - assert!(pool == PoolType::Shielded(ShieldedProtocol::Sapling)); - build_result - .transaction() - .sapling_bundle() - .and_then(|bundle| { - try_sapling_note_decryption( - &sapling_internal_ivk, - &bundle.shielded_outputs()[output_index], - zip212_enforcement(params, min_target_height), - ) - .map(|(note, _, _)| Note::Sapling(note)) - }) - }) - .internal_account_note_transpose_option() - .expect("Wallet-internal outputs must be decryptable with the wallet's IVK"); - - 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 recipient = recipient + .map_internal_account_note(|pool| { + assert!(pool == PoolType::Shielded(ShieldedProtocol::Orchard)); + build_result + .transaction() + .orchard_bundle() + .and_then(|bundle| { + bundle + .decrypt_output_with_key( + output_index, + &orchard_internal_ivk, + ) + .map(|(note, _, _)| Note::Orchard(note)) + }) + }) + .internal_account_note_transpose_option() + .expect( + "Wallet-internal outputs must be decryptable with the wallet's IVK", + ); + + SentTransactionOutput::from_parts(output_index, recipient, value, memo) + }); + + let sapling_internal_ivk = + PreparedIncomingViewingKey::new(&sapling_dfvk.to_ivk(Scope::Internal)); + let sapling_outputs = + sapling_output_meta + .into_iter() + .enumerate() + .map(|(i, (recipient, value, memo))| { + let output_index = build_result.sapling_meta().output_index(i).expect( + "An output should exist in the transaction for each Sapling payment.", ); + let recipient = recipient + .map_internal_account_note(|pool| { + assert!(pool == PoolType::Shielded(ShieldedProtocol::Sapling)); + build_result + .transaction() + .sapling_bundle() + .and_then(|bundle| { + try_sapling_note_decryption( + &sapling_internal_ivk, + &bundle.shielded_outputs()[output_index], + zip212_enforcement(params, min_target_height), + ) + .map(|(note, _, _)| Note::Sapling(note)) + }) + }) + .internal_account_note_transpose_option() + .expect( + "Wallet-internal outputs must be decryptable with the wallet's IVK", + ); + + SentTransactionOutput::from_parts(output_index, recipient, value, memo) + }); + + let txid: [u8; 32] = build_result.transaction().txid().into(); + let num_txouts = build_result + .transaction() + .transparent_bundle() + .map_or(0, |b| b.vout.len()); + assert!(num_txouts <= u32::MAX as usize); + + let transparent_outputs = transparent_output_meta.into_iter().enumerate().map( + |(output_index, (recipient, _addr, value))| { + assert!(output_index < num_txouts); + let recipient = recipient.map_ephemeral_transparent_outpoint(|pool: PoolType| { + assert!(pool == PoolType::Transparent); + OutPoint::new(txid, output_index as u32) + }); + SentTransactionOutput::from_parts(output_index, recipient, value, None) - }); + }, + ); + + let mut outputs: Vec::AccountId>> = vec![]; + #[cfg(feature = "orchard")] + outputs.extend(orchard_outputs); + outputs.extend(sapling_outputs); + outputs.extend(transparent_outputs); + + wallet_db + .store_sent_tx(&SentTransaction { + tx: build_result.transaction(), + created: time::OffsetDateTime::now_utc(), + account: account_id, + outputs, + fee_amount: proposal_step.balance().fee_required(), + #[cfg(feature = "transparent-inputs")] + utxos_spent, + }) + .map_err(Error::DataSource)?; - let mut outputs = vec![]; - #[cfg(feature = "orchard")] - outputs.extend(orchard_outputs); - outputs.extend(sapling_outputs); - outputs.extend(transparent_outputs); - - wallet_db - .store_sent_tx(&SentTransaction { - tx: build_result.transaction(), - created: time::OffsetDateTime::now_utc(), - account, - outputs, - fee_amount: proposal_step.balance().fee_required(), - #[cfg(feature = "transparent-inputs")] - utxos_spent, - }) - .map_err(Error::DataSource)?; + Ok(build_result) + }; + let res = finish(); - Ok(build_result) + #[cfg(feature = "transparent-inputs")] + if !ephemeral_addrs.is_empty() { + match res { + Ok(_) => wallet_db.mark_addresses_as_used(account_id, &ephemeral_addrs), + Err(_) => wallet_db.unreserve_addresses(account_id, &ephemeral_addrs), + } + .map_err(InputSelectorError::from)?; + } + + res } /// Constructs a transaction that consumes available transparent UTXOs belonging to the specified @@ -1271,7 +1321,11 @@ pub fn shield_transparent_funds( > where ParamsT: consensus::Parameters, - DbT: WalletWrite + WalletCommitmentTrees + InputSource::Error>, + DbT: WalletRead, + DbT: WalletWrite, + DbT: WalletCommitmentTrees, + DbT: InputSource::Error>, + DbT: WalletAddressTracking::AccountId>, InputsT: ShieldingSelector, { let proposal = propose_shielding( 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 43ad908b8a..c1d3e4c8c7 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -22,7 +22,7 @@ use zcash_primitives::{ use crate::{ address::{Address, UnifiedAddress}, - data_api::{InputSource, SimpleNoteRetention, SpendableNotes}, + data_api::{AddressTrackingError, InputSource, SimpleNoteRetention, SpendableNotes}, fees::{sapling, ChangeError, ChangeStrategy, DustOutputPolicy}, proposal::{Proposal, ProposalError, ShieldedInputs}, wallet::WalletTransparentOutput, @@ -32,9 +32,16 @@ use crate::{ #[cfg(feature = "transparent-inputs")] use { - std::collections::BTreeSet, std::convert::Infallible, - zcash_primitives::legacy::TransparentAddress, - zcash_primitives::transaction::components::OutPoint, + crate::{ + fees::{ChangeValue, TransactionBalance}, + proposal::{Step, StepOutput, StepOutputIndex}, + zip321::Payment, + }, + std::collections::BTreeSet, + std::convert::Infallible, + zcash_primitives::{ + legacy::TransparentAddress, transaction::components::OutPoint, transaction::fees::zip317, + }, }; #[cfg(feature = "orchard")] @@ -60,6 +67,8 @@ pub enum InputSelectorError { /// The data source does not have enough information to choose an expiry height /// for the transaction. SyncRequired, + /// An error related to tracking of ephemeral transparent addresses. + AddressTracking(AddressTrackingError), } impl From> for InputSelectorError { @@ -107,10 +116,23 @@ impl fmt::Display for InputSelectorError { write!(f, "Insufficient chain data is available, sync required.") } + InputSelectorError::AddressTracking(e) => { + write!( + f, + "Error related to tracking of ephemeral transparent addresses: {}", + e + ) + } } } } +impl From for InputSelectorError { + fn from(e: AddressTrackingError) -> Self { + InputSelectorError::AddressTracking(e) + } +} + impl error::Error for InputSelectorError where DE: Debug + Display + error::Error + 'static, @@ -364,6 +386,16 @@ where #[cfg(feature = "orchard")] let mut orchard_outputs = vec![]; let mut payment_pools = BTreeMap::new(); + + #[cfg(feature = "transparent-inputs")] + let mut tr2_transparent_outputs = vec![]; + #[cfg(feature = "transparent-inputs")] + let mut tr2_payments = vec![]; + #[cfg(feature = "transparent-inputs")] + let mut tr2_payment_pools = BTreeMap::new(); + #[cfg(feature = "transparent-inputs")] + let mut total_ephemeral_plus_fee = Some(NonNegativeAmount::ZERO); // None means overflow + for (idx, payment) in transaction_request.payments() { let recipient_address: Address = payment .recipient_address() @@ -378,6 +410,30 @@ where script_pubkey: addr.script(), }); } + #[cfg(feature = "transparent-inputs")] + Address::Tex(data) => { + let p2pkh_addr = TransparentAddress::PublicKeyHash(data); + + tr2_payment_pools.insert(*idx, PoolType::Transparent); + tr2_transparent_outputs.push(TxOut { + value: payment.amount(), + script_pubkey: p2pkh_addr.script(), + }); + tr2_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_plus_fee = + total_ephemeral_plus_fee + payment.amount() + zip317::MARGINAL_FEE; + } + #[cfg(not(feature = "transparent-inputs"))] Address::Tex(_) => { return Err(InputSelectorError::Selection( GreedyInputSelectorError::UnsupportedTexAddress, @@ -417,10 +473,30 @@ where } } + #[cfg(feature = "transparent-inputs")] + let total_ephemeral_plus_fee = + total_ephemeral_plus_fee.ok_or(InputSelectorError::Selection( + GreedyInputSelectorError::Balance(BalanceError::Overflow), + ))?; + + #[cfg(feature = "transparent-inputs")] + if !tr2_transparent_outputs.is_empty() { + // Push a fake output of total_ephemeral_plus_fee to an arbitrary t-address. + // This is *only* used to compute the balance and will not appear in any + // `TransactionRequest`, so it is fine to use a burn address. This assumes + // that the fake output will have the same effect on the fee for the first + // transaction as the real ephemeral output. + transparent_outputs.push(TxOut { + value: total_ephemeral_plus_fee, + script_pubkey: TransparentAddress::PublicKeyHash([0u8; 20]).script(), + }); + } + 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 +510,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) @@ -487,18 +563,92 @@ where match balance { Ok(balance) => { - return Proposal::single_step( - transaction_request, - payment_pools, - vec![], + let fee_rule = (*self.change_strategy.fee_rule()).clone(); + 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 !tr2_transparent_outputs.is_empty() { + // Construct two new `TransactionRequest`s: + // * `tr1` excludes the TEX outputs, and in their place includes + // a single additional "change" output to the transparent pool. + // * `tr2` spends from that change output to each TEX output. + + let tr1 = TransactionRequest::from_indexed( + transaction_request + .payments() + .iter() + .filter_map(|(idx, payment)| { + if tr2_payment_pools.contains_key(idx) { + None + } else { + Some((*idx, payment.clone())) + } + }) + .collect(), + ) + .expect( + "removing payments from a TransactionRequest preserves correctness", + ); + + // Create a TransactionBalance for `tr1` that adds the ephemeral output + // as an extra change output. + let mut tr1_change: Vec<_> = balance.proposed_change().into(); + let ephemeral_output = + StepOutput::new(0, StepOutputIndex::Change(tr1_change.len())); + tr1_change.push(ChangeValue::transparent(total_ephemeral_plus_fee)); + let tr1_balance = + TransactionBalance::new(tr1_change, balance.fee_required()).map_err( + |_| InputSelectorError::Proposal(ProposalError::Overflow), + )?; + + let tr2 = + TransactionRequest::new(tr2_payments).expect("correct by construction"); + + let step1 = Step::from_parts( + &[], + tr1, + payment_pools, + vec![], + shielded_inputs, + vec![], + tr1_balance, + false, + ) + .map_err(InputSelectorError::Proposal)?; + + let step2 = Step::from_parts( + &[step1.clone()], + tr2, + tr2_payment_pools, + vec![], + None, + vec![ephemeral_output], + balance, + false, + ) + .map_err(InputSelectorError::Proposal)?; + + return Proposal::multi_step( + fee_rule, + target_height, + NonEmpty::from((step1, vec![step2])), + ) + .map_err(InputSelectorError::Proposal); + } + + return Proposal::single_step( + transaction_request, + payment_pools, + vec![], + shielded_inputs, balance, - (*self.change_strategy.fee_rule()).clone(), + fee_rule, target_height, false, ) diff --git a/zcash_client_backend/src/proto/proposal.rs b/zcash_client_backend/src/proto/proposal.rs index a17b83bf8b..c94aada37e 100644 --- a/zcash_client_backend/src/proto/proposal.rs +++ b/zcash_client_backend/src/proto/proposal.rs @@ -116,6 +116,8 @@ pub mod proposed_input { #[derive(Clone, PartialEq, ::prost::Message)] pub struct TransactionBalance { /// A list of change output values. + /// Any `ChangeValue`s for the transparent value pool represent ephemeral + /// outputs that will each be given a unique t-address. #[prost(message, repeated, tag = "1")] pub proposed_change: ::prost::alloc::vec::Vec, /// The fee to be paid by the proposed transaction, in zatoshis. diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 53e0be8247..dcf2f6957f 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: O, + }, InternalAccount { receiving_account: AccountId, external_address: Option, @@ -76,10 +82,22 @@ pub enum Recipient { }, } -impl Recipient { - pub fn map_internal_account_note B>(self, f: F) -> Recipient { +impl Recipient { + 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, + } => Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint, + }, Recipient::InternalAccount { receiving_account, external_address, @@ -91,12 +109,48 @@ impl Recipient { }, } } + + 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, + } => Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint: f(outpoint), + }, + 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> { + 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, + } => Some(Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint, + }), Recipient::InternalAccount { receiving_account, external_address, diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index b15f86a721..647fdad93a 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -6,7 +6,30 @@ 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. + +### Added +- `zcash_client_sqlite::data_api::error`: + - `impl From for SqliteClientError` + ### Changed +- `zcash_client_sqlite::data_api::error`: + - `SqliteClientError` has a new `AddressTracking` variant. - The result of the `v_tx_outputs` SQL query could now include transparent outputs with unknown height. - MSRV is now 1.66.0. diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 2f961853c0..69db8c8e20 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -5,7 +5,7 @@ use std::fmt; use shardtree::error::ShardTreeError; use zcash_address::ParseError; -use zcash_client_backend::PoolType; +use zcash_client_backend::{data_api::AddressTrackingError, PoolType}; use zcash_keys::keys::AddressGenerationError; use zcash_primitives::zip32; use zcash_primitives::{consensus::BlockHeight, transaction::components::amount::BalanceError}; @@ -112,6 +112,9 @@ pub enum SqliteClientError { /// An error occurred in computing wallet balance BalanceError(BalanceError), + + /// Address tracking error + AddressTracking(AddressTrackingError), } impl error::Error for SqliteClientError { @@ -162,6 +165,7 @@ 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), + SqliteClientError::AddressTracking(e) => write!(f, "Error related to tracking of ephemeral transparent addresses: {}", e), } } } @@ -226,3 +230,9 @@ impl From for SqliteClientError { SqliteClientError::AddressGeneration(e) } } + +impl From for SqliteClientError { + fn from(e: AddressTrackingError) -> Self { + SqliteClientError::AddressTracking(e) + } +} diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 733c35973a..2339bffe46 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -42,8 +42,18 @@ use rusqlite::{self, Connection}; use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, ShardTree}; use std::{ - borrow::Borrow, collections::HashMap, convert::AsRef, fmt, num::NonZeroU32, ops::Range, + borrow::Borrow, + collections::{ + hash_map::Entry::{Occupied, Vacant}, + HashMap, + }, + convert::AsRef, + fmt, + num::NonZeroU32, + ops::Range, path::Path, + rc::Rc, + sync::Mutex, }; use subtle::ConditionallySelectable; use tracing::{debug, trace, warn}; @@ -54,9 +64,10 @@ use zcash_client_backend::{ self, chain::{BlockSource, ChainState, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, - Account, AccountBirthday, AccountSource, BlockMetadata, DecryptedTransaction, InputSource, - NullifierQuery, ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, - WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + Account, AccountBirthday, AccountSource, AddressTrackingError, BlockMetadata, + DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SeedRelevance, + SentTransaction, SpendableNotes, WalletAddressTracking, WalletCommitmentTrees, WalletRead, + WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }, keys::{ AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey, @@ -69,6 +80,7 @@ use zcash_keys::address::Receiver; use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight}, + legacy::TransparentAddress, memo::{Memo, MemoBytes}, transaction::{components::amount::NonNegativeAmount, Transaction, TxId}, zip32::{self, DiversifierIndex, Scope}, @@ -88,7 +100,7 @@ use { #[cfg(feature = "transparent-inputs")] use { zcash_client_backend::wallet::TransparentAddressMetadata, - zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}, + zcash_primitives::transaction::components::{OutPoint, TxOut}, }; #[cfg(feature = "unstable")] @@ -102,6 +114,7 @@ pub mod chain; pub mod error; pub mod wallet; use wallet::{ + address_tracker::AddressTracker, commitment_tree::{self, put_shard_roots}, SubtreeScanProgress, }; @@ -168,6 +181,7 @@ pub struct UtxoId(pub i64); pub struct WalletDb { conn: C, params: P, + address_trackers: Rc>>, } /// A wrapper for a SQLite transaction affecting the wallet database. @@ -184,7 +198,11 @@ impl WalletDb { pub fn for_path>(path: F, params: P) -> Result { Connection::open(path).and_then(move |conn| { rusqlite::vtab::array::load_module(&conn)?; - Ok(WalletDb { conn, params }) + Ok(WalletDb { + conn, + params, + address_trackers: Rc::new(Mutex::new(HashMap::new())), + }) }) } @@ -196,6 +214,7 @@ impl WalletDb { let mut wdb = WalletDb { conn: SqlTransaction(&tx), params: self.params.clone(), + address_trackers: self.address_trackers.clone(), }; let result = f(&mut wdb)?; tx.commit()?; @@ -785,7 +804,7 @@ impl WalletWrite for WalletDb } // Prune the nullifier map of entries we no longer need. - if let Some(meta) = wdb.block_fully_scanned()? { + if let Some(meta) = wallet::block_fully_scanned(wdb.conn.0, &wdb.params)? { wallet::prune_nullifier_map( wdb.conn.0, meta.block_height().saturating_sub(PRUNING_DEPTH), @@ -1079,6 +1098,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, *output.account(), tx_ref, output.index(), @@ -1098,6 +1118,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, *output.account(), tx_ref, output.index(), @@ -1128,6 +1149,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, account_id, tx_ref, output.index(), @@ -1160,6 +1182,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, *output.account(), tx_ref, output.index(), @@ -1179,6 +1202,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, *output.account(), tx_ref, output.index(), @@ -1210,6 +1234,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, account_id, tx_ref, output.index(), @@ -1254,6 +1279,7 @@ impl WalletWrite for WalletDb ) } + let mut output_addrs = vec![]; for (output_index, txout) in d_tx .tx() .transparent_bundle() @@ -1262,6 +1288,11 @@ impl WalletWrite for WalletDb .enumerate() { if let Some(address) = txout.recipient_address() { + // Mark this output address as mined. + // TODO: we really want to only mark outputs when a transaction has been + // *reliably* mined, e.g. it has 10 confirmations. + output_addrs.push(address); + let receiver = Receiver::Transparent(address); #[cfg(feature = "transparent-inputs")] @@ -1281,6 +1312,7 @@ impl WalletWrite for WalletDb wallet::put_sent_output( wdb.conn.0, + &wdb.params, account_id, tx_ref, output_index, @@ -1290,6 +1322,7 @@ impl WalletWrite for WalletDb )?; } } + wdb.mark_addresses_as_mined(account_id, &output_addrs).map_err(SqliteClientError::from)?; } } @@ -1343,7 +1376,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 { @@ -1387,6 +1426,27 @@ impl WalletWrite for WalletDb None, )?; } + #[cfg(feature = "transparent-inputs")] + Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint, + } => { + wallet::put_legacy_transparent_utxo( + wdb.conn.0, + &wdb.params, + &WalletTransparentOutput::from_parts( + outpoint.clone(), + TxOut { + value: output.value(), + script_pubkey: ephemeral_address.script(), + }, + None, + ) + .expect("txout is correct"), + *receiving_account, + )?; + } _ => (), } } @@ -1396,9 +1456,7 @@ impl WalletWrite for WalletDb } fn truncate_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error> { - self.transactionally(|wdb| { - wallet::truncate_to_height(wdb.conn.0, &wdb.params, block_height) - }) + self.transactionally(|wdb| wallet::truncate_to_height(wdb, block_height)) } } @@ -1594,6 +1652,82 @@ impl<'conn, P: consensus::Parameters> WalletCommitmentTrees for WalletDb WalletAddressTracking for WalletDb +where + C: Borrow, + P: consensus::Parameters, +{ + /// Backend-specific account identifier. + type AccountId = ::AccountId; + + fn reserve_next_address( + &self, + account_id: Self::AccountId, + ) -> Result { + let mut trackers = self + .address_trackers + .lock() + .map_err(|e| AddressTrackingError::Internal(format!("{:?}", e)))?; + + match trackers.entry(account_id) { + Occupied(e) => e.into_mut(), + Vacant(e) => e.insert(AddressTracker::new(self, account_id)?), + } + .reserve_next_address(self) + } + + fn unreserve_addresses( + &self, + account_id: Self::AccountId, + addresses: &[TransparentAddress], + ) -> Result<(), AddressTrackingError> { + let mut trackers = self + .address_trackers + .lock() + .map_err(|e| AddressTrackingError::Internal(format!("{:?}", e)))?; + + match trackers.entry(account_id) { + Occupied(e) => e.into_mut(), + Vacant(e) => e.insert(AddressTracker::new(self, account_id)?), + } + .unreserve_addresses(self, addresses) + } + + fn mark_addresses_as_used( + &self, + account_id: Self::AccountId, + addresses: &[TransparentAddress], + ) -> Result<(), AddressTrackingError> { + let mut trackers = self + .address_trackers + .lock() + .map_err(|e| AddressTrackingError::Internal(format!("{:?}", e)))?; + + match trackers.entry(account_id) { + Occupied(e) => e.into_mut(), + Vacant(e) => e.insert(AddressTracker::new(self, account_id)?), + } + .mark_addresses_as_used(self, addresses) + } + + fn mark_addresses_as_mined( + &self, + account_id: Self::AccountId, + addresses: &[TransparentAddress], + ) -> Result<(), AddressTrackingError> { + let mut trackers = self + .address_trackers + .lock() + .map_err(|e| AddressTrackingError::Internal(format!("{:?}", e)))?; + + match trackers.entry(account_id) { + Occupied(e) => e.into_mut(), + Vacant(e) => e.insert(AddressTracker::new(self, account_id)?), + } + .mark_addresses_as_mined(self, addresses) + } +} + /// A handle for the SQLite block source. pub struct BlockDb(Connection); diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 9006f7f600..7ed4a81e32 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -2020,7 +2020,7 @@ pub(crate) fn data_db_truncation() { // "Rewind" to height of last scanned block (this is a no-op) st.wallet_mut() - .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h + 1)) + .transactionally(|wdb| truncate_to_height(wdb, h + 1)) .unwrap(); // Spendable balance should be unaltered @@ -2031,7 +2031,7 @@ pub(crate) fn data_db_truncation() { // Rewind so that one block is dropped st.wallet_mut() - .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h)) + .transactionally(|wdb| truncate_to_height(wdb, h)) .unwrap(); // Spendable balance should only contain the first received note; diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 228ab20c97..7e80470dbe 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -68,6 +68,7 @@ use incrementalmerkletree::Retention; use rusqlite::{self, named_params, OptionalExtension}; use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; +use zcash_keys::encoding::encode_transparent_address_p; use zip32::fingerprint::SeedFingerprint; use std::collections::{HashMap, HashSet}; @@ -102,7 +103,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, }, }; @@ -132,10 +133,11 @@ use { keys::{IncomingViewingKey, NonHardenedChildIndex}, Script, TransparentAddress, }, - transaction::components::{OutPoint, TxOut}, + transaction::components::TxOut, }, }; +pub(crate) mod address_tracker; pub mod commitment_tree; pub(crate) mod common; pub mod init; @@ -1986,22 +1988,25 @@ pub(crate) fn get_min_unspent_height( /// /// This should only be executed inside a transactional context. pub(crate) fn truncate_to_height( - conn: &rusqlite::Transaction, - params: &P, + wdb: &mut WalletDb, P>, block_height: BlockHeight, ) -> Result<(), SqliteClientError> { - let sapling_activation_height = params + let sapling_activation_height = wdb + .params .activation_height(NetworkUpgrade::Sapling) .expect("Sapling activation height must be available."); // Recall where we synced up to previously. - let last_scanned_height = conn.query_row("SELECT MAX(height) FROM blocks", [], |row| { - row.get::<_, Option>(0) - .map(|opt| opt.map_or_else(|| sapling_activation_height - 1, BlockHeight::from)) - })?; + let last_scanned_height = + wdb.conn + .0 + .query_row("SELECT MAX(height) FROM blocks", [], |row| { + row.get::<_, Option>(0) + .map(|opt| opt.map_or_else(|| sapling_activation_height - 1, BlockHeight::from)) + })?; if block_height < last_scanned_height - PRUNING_DEPTH { - if let Some(h) = get_min_unspent_height(conn)? { + if let Some(h) = get_min_unspent_height(wdb.conn.0)? { if block_height > h { return Err(SqliteClientError::RequestedRewindInvalid(h, block_height)); } @@ -2012,12 +2017,12 @@ pub(crate) fn truncate_to_height( // truncation height, and then truncate any remaining range by setting the end // equal to the truncation height + 1. This sets our view of the chain tip back // to the retained height. - conn.execute( + wdb.conn.0.execute( "DELETE FROM scan_queue WHERE block_range_start >= :new_end_height", named_params![":new_end_height": u32::from(block_height + 1)], )?; - conn.execute( + wdb.conn.0.execute( "UPDATE scan_queue SET block_range_end = :new_end_height WHERE block_range_end > :new_end_height", @@ -2029,10 +2034,6 @@ pub(crate) fn truncate_to_height( // database. if block_height < last_scanned_height { // Truncate the note commitment trees - let mut wdb = WalletDb { - conn: SqlTransaction(conn), - params: params.clone(), - }; wdb.with_sapling_tree_mut(|tree| { tree.truncate_removing_checkpoint(&block_height).map(|_| ()) })?; @@ -2050,27 +2051,29 @@ pub(crate) fn truncate_to_height( // Rewind utxos. It is currently necessary to delete these because we do // not have the full transaction data for the received output. - conn.execute( + // FIXME: This does not delete utxos with an unknown height (sentinel -1); + // is that correct? + wdb.conn.0.execute( "DELETE FROM utxos WHERE height > ?", [u32::from(block_height)], )?; // Un-mine transactions. - conn.execute( + wdb.conn.0.execute( "UPDATE transactions SET block = NULL, tx_index = NULL WHERE block IS NOT NULL AND block > ?", [u32::from(block_height)], )?; // Now that they aren't depended on, delete un-mined blocks. - conn.execute( + wdb.conn.0.execute( "DELETE FROM blocks WHERE height > ?", [u32::from(block_height)], )?; // Delete from the nullifier map any entries with a locator referencing a block // height greater than the truncation height. - conn.execute( + wdb.conn.0.execute( "DELETE FROM tx_locator_map WHERE block_height > :block_height", named_params![":block_height": u32::from(block_height)], @@ -2491,6 +2494,32 @@ pub(crate) fn put_received_transparent_utxo( params: &P, output: &WalletTransparentOutput, ) -> Result { + if let Some(account) = find_account_for_transparent_output(conn, params, output)? { + let utxo_id = put_legacy_transparent_utxo(conn, params, output, account)?; + Ok(utxo_id) + } else { + // The UTXO was not for any of the legacy transparent addresses. + Err(SqliteClientError::AddressNotRecognized( + *output.recipient_address(), + )) + } +} + +/// Attempts to determine the account that received the given transparent output. +/// +/// The following two locations in the wallet's key tree are searched: +/// - Transparent receivers that have been generated as part of a Unified Address. +/// - "Legacy transparent addresses" (at BIP 44 address index 0 within an account). +/// FIXME: also look here in the set of ephemeral addresses we have generated. +/// +/// 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". +#[cfg(feature = "transparent-inputs")] +pub(crate) fn find_account_for_transparent_output( + conn: &rusqlite::Connection, + params: &P, + output: &WalletTransparentOutput, +) -> Result, SqliteClientError> { let address_str = output.recipient_address().encode(params); let account_id = conn .query_row( @@ -2501,7 +2530,7 @@ pub(crate) fn put_received_transparent_utxo( .optional()?; if let Some(account) = account_id { - Ok(put_legacy_transparent_utxo(conn, params, output, account)?) + Ok(Some(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 @@ -2514,21 +2543,13 @@ pub(crate) fn put_received_transparent_utxo( .find_map( |account| match get_legacy_transparent_address(params, conn, account) { Ok(Some((legacy_taddr, _))) if &legacy_taddr == output.recipient_address() => { - Some( - put_legacy_transparent_utxo(conn, params, output, account) - .map_err(SqliteClientError::from), - ) + Some(Ok(account)) } 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(), - )) - }) + .transpose() } } @@ -2573,11 +2594,21 @@ pub(crate) fn put_legacy_transparent_utxo( // 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(encode_transparent_address_p(params, ephemeral_address)), + Some(*receiving_account), + PoolType::Transparent, + ), Recipient::InternalAccount { receiving_account, external_address, @@ -2591,8 +2622,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, @@ -2606,7 +2638,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), @@ -2635,12 +2667,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> { @@ -2659,7 +2692,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/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 9fff7e3a3f..3dd508cd0c 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -174,6 +174,9 @@ 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") } + SqliteClientError::AddressTracking(_) => { + unreachable!("we don't call address tracking methods") + } } } @@ -379,6 +382,11 @@ mod tests { birthday_sapling_tree_size INTEGER, birthday_orchard_tree_size INTEGER, recover_until_height INTEGER, + first_unmined_ephemeral_taddr_index INTEGER NOT NULL DEFAULT 0, + first_unused_ephemeral_taddr_index INTEGER NOT NULL DEFAULT 0 + CONSTRAINT unused_gte_unmined CHECK ( + first_unused_ephemeral_taddr_index >= first_unmined_ephemeral_taddr_index + ), CHECK ( ( account_kind = 0 diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index 0cfba40e93..1d47a63792 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -1,6 +1,7 @@ mod add_account_birthdays; mod add_transaction_views; mod add_utxo_account; +mod address_tracking; mod addresses_table; mod ensure_orchard_ua_receiver; mod full_account_ids; @@ -61,10 +62,10 @@ pub(super) fn all_migrations( // \ \ | v_transactions_note_uniqueness // \ \ | / // -------------------- full_account_ids - // | - // orchard_received_notes - // | - // ensure_orchard_ua_receiver + // / \ + // orchard_received_notes address_tracking + // | + // ensure_orchard_ua_receiver vec![ Box::new(initial_setup::Migration {}), Box::new(utxos_table::Migration {}), @@ -114,5 +115,6 @@ pub(super) fn all_migrations( Box::new(ensure_orchard_ua_receiver::Migration { params: params.clone(), }), + Box::new(address_tracking::Migration), ] } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/address_tracking.rs b/zcash_client_sqlite/src/wallet/init/migrations/address_tracking.rs new file mode 100644 index 0000000000..fb2368ee68 --- /dev/null +++ b/zcash_client_sqlite/src/wallet/init/migrations/address_tracking.rs @@ -0,0 +1,57 @@ +//! The migration that records ephemeral addresses used beyond the last known mined address, for each account. +use std::collections::HashSet; + +use rusqlite; +use schemer; +use schemer_rusqlite::RusqliteMigration; +use uuid::Uuid; + +use crate::wallet::init::WalletMigrationError; + +use super::full_account_ids; + +pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0x0e1d4274_1f8e_44e2_909d_689a4bc2967b); + +pub(super) struct Migration; + +impl schemer::Migration for Migration { + fn id(&self) -> Uuid { + MIGRATION_ID + } + + fn dependencies(&self) -> HashSet { + [full_account_ids::MIGRATION_ID].into_iter().collect() + } + + fn description(&self) -> &'static str { + "Record indices of ephemeral addresses that have been used beyond the last known mined address, for each account." + } +} + +impl RusqliteMigration for Migration { + type Error = WalletMigrationError; + + fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + transaction.execute_batch( + r#" + ALTER TABLE accounts ADD first_unmined_ephemeral_taddr_index INTEGER NOT NULL DEFAULT 0; + ALTER TABLE accounts ADD first_unused_ephemeral_taddr_index INTEGER NOT NULL DEFAULT 0 + CONSTRAINT unused_gte_unmined CHECK ( + first_unused_ephemeral_taddr_index >= first_unmined_ephemeral_taddr_index + ); + "#, + )?; + Ok(()) + } + + fn down(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + // Dropping first_unused_ephemeral_taddr_index also drops its constraint. + transaction.execute_batch( + r#" + ALTER TABLE accounts DROP COLUMN first_unused_ephemeral_index; + ALTER TABLE accounts DROP COLUMN first_unmined_ephemeral_index; + "#, + )?; + Ok(()) + } +}