From 47a0d0d2b789c816525aa1d07c0ebbfd67758cf1 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 13 Oct 2022 20:58:43 -0600 Subject: [PATCH] Remove the `WalletReadTransparent` and `WalletWriteTransparent` extension traits. These traits introduce a problem, in that constraints on a method cannot be conditionally required based upon the presence or absence of a feature flag. Instead, we make the methods previously introduced by the removed traits present in all cases on the `WalletRead` and `WalletWrite` traits, but ensure that their implementations return an error if the caller attempts to use them in a wallet that has not been configured with support for transparent inputs functionality. --- zcash_client_backend/CHANGELOG.md | 7 +-- zcash_client_backend/src/data_api.rs | 37 ++++++------ zcash_client_backend/src/data_api/error.rs | 10 +++ zcash_client_backend/src/data_api/wallet.rs | 5 +- zcash_client_backend/src/wallet.rs | 19 +++--- zcash_client_sqlite/src/lib.rs | 67 ++++++++++----------- 6 files changed, 73 insertions(+), 72 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 214bfced1..ed16fcf50 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -13,10 +13,6 @@ and this library adheres to Rust's notion of - A new `data_api::wallet::shield_transparent_funds` method has been added to facilitate the automatic shielding of transparent funds received by the wallet. - - `zcash_client_backend::data_api::WalletReadTransparent` read-only operations - related to information the wallet maintains about transparent funds. - - `zcash_client_backend::data_api::WalletWriteTransparent` operations - related persisting information the wallet maintains about transparent funds. - A `zcash_client_backend::wallet::WalletTransparentOutput` type has been added under the `transparent-inputs` feature flag in support of autoshielding functionality. @@ -38,9 +34,12 @@ and this library adheres to Rust's notion of - `WalletRead::get_account_for_ufvk` - `WalletRead::get_current_address` - `WalletRead::get_all_nullifiers` + - `WalletRead::get_transparent_receivers` + - `WalletRead::get_unspent_transparent_outputs` - `WalletWrite::create_account` - `WalletWrite::remove_unmined_tx` (behind the `unstable` feature flag). - `WalletWrite::get_next_available_address` + - `WalletWrite::put_received_transparent_utxo` - `zcash_client_backend::decrypt`: - `TransferType` - `zcash_client_backend::proto`: diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 52e75a64d..94b63acfd 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1,12 +1,9 @@ //! Interfaces for wallet data persistence & low-level wallet utilities. use std::cmp; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fmt::Debug; -#[cfg(feature = "transparent-inputs")] -use std::collections::HashSet; - use secrecy::SecretVec; use zcash_primitives::{ block::BlockHash, @@ -24,11 +21,11 @@ use crate::{ decrypt::DecryptedOutput, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, - wallet::{SpendableNote, WalletTx}, + wallet::{SpendableNote, WalletTransparentOutput, WalletTx}, }; #[cfg(feature = "transparent-inputs")] -use {crate::wallet::WalletTransparentOutput, zcash_primitives::transaction::components::OutPoint}; +use zcash_primitives::transaction::components::transparent::OutPoint; pub mod chain; pub mod error; @@ -201,10 +198,7 @@ pub trait WalletRead { target_value: Amount, anchor_height: BlockHeight, ) -> Result, Self::Error>; -} -#[cfg(feature = "transparent-inputs")] -pub trait WalletReadTransparent: WalletRead { /// Returns the set of all transparent receivers associated with the given account. /// /// The set contains all transparent receivers that are known to have been derived @@ -300,6 +294,9 @@ pub struct SentTransactionOutput { /// This trait encapsulates the write capabilities required to update stored /// wallet data. pub trait WalletWrite: WalletRead { + /// The type of identifiers used to look up transparent UTXOs. + type UtxoRef; + /// Tells the wallet to track the next available account-level spend authority, given /// the current set of [ZIP 316] account identifiers known to the wallet database. /// @@ -373,12 +370,8 @@ pub trait WalletWrite: WalletRead { /// /// There may be restrictions on how far it is possible to rewind. fn rewind_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error>; -} - -#[cfg(feature = "transparent-inputs")] -pub trait WalletWriteTransparent: WalletWrite + WalletReadTransparent { - type UtxoRef; + /// Adds a transparent UTXO received by the wallet to the data store. fn put_received_transparent_utxo( &mut self, output: &WalletTransparentOutput, @@ -433,9 +426,6 @@ pub mod testing { WalletWrite, }; - #[cfg(feature = "transparent-inputs")] - use super::WalletReadTransparent; - pub struct MockBlockSource {} impl BlockSource for MockBlockSource { @@ -561,10 +551,7 @@ pub mod testing { ) -> Result, Self::Error> { Ok(Vec::new()) } - } - #[cfg(feature = "transparent-inputs")] - impl WalletReadTransparent for MockWalletDb { fn get_transparent_receivers( &self, _account: AccountId, @@ -582,6 +569,8 @@ pub mod testing { } impl WalletWrite for MockWalletDb { + type UtxoRef = u32; + fn create_account( &mut self, seed: &SecretVec, @@ -630,5 +619,13 @@ pub mod testing { fn rewind_to_height(&mut self, _block_height: BlockHeight) -> Result<(), Self::Error> { Ok(()) } + + /// Adds a transparent UTXO received by the wallet to the data store. + fn put_received_transparent_utxo( + &mut self, + _output: &WalletTransparentOutput, + ) -> Result { + Ok(0) + } } } diff --git a/zcash_client_backend/src/data_api/error.rs b/zcash_client_backend/src/data_api/error.rs index cc06149fe..1c6cbf7a1 100644 --- a/zcash_client_backend/src/data_api/error.rs +++ b/zcash_client_backend/src/data_api/error.rs @@ -78,6 +78,11 @@ pub enum Error { /// An error occurred deriving a spending key from a seed and an account /// identifier. KeyDerivationError(AccountId), + + /// An error indicating that a call was attempted to a method providing + /// support + #[cfg(not(feature = "transparent-inputs"))] + TransparentInputsNotSupported, } impl ChainInvalid { @@ -133,6 +138,11 @@ impl fmt::Display for Error { Error::SaplingNotActive => write!(f, "Could not determine Sapling upgrade activation height."), Error::MemoForbidden => write!(f, "It is not possible to send a memo to a transparent address."), Error::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {:?}", acct_id), + + #[cfg(not(feature = "transparent-inputs"))] + Error::TransparentInputsNotSupported => { + write!(f, "This wallet does not support spending or manipulating transparent UTXOs.") + } } } } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index a63fb240e..226fa85b7 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -25,9 +25,6 @@ use crate::{ zip321::{Payment, TransactionRequest}, }; -#[cfg(feature = "transparent-inputs")] -use crate::data_api::WalletWriteTransparent; - /// Scans a [`Transaction`] for any information that can be decrypted by the accounts in /// the wallet, and saves it to the wallet. pub fn decrypt_and_store_transaction( @@ -454,7 +451,7 @@ where E: From>, P: consensus::Parameters, R: Copy + Debug, - D: WalletWrite + WalletWriteTransparent, + D: WalletWrite, { let account = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key())? diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 6f7eb441f..5c6c6c53a 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -3,20 +3,21 @@ use zcash_note_encryption::EphemeralKeyBytes; use zcash_primitives::{ + consensus::BlockHeight, keys::OutgoingViewingKey, + legacy::TransparentAddress, merkle_tree::IncrementalWitness, sapling::{Diversifier, Node, Note, Nullifier, PaymentAddress, Rseed}, - transaction::{components::Amount, TxId}, + transaction::{ + components::{ + transparent::{OutPoint, TxOut}, + Amount, + }, + TxId, + }, zip32::AccountId, }; -#[cfg(feature = "transparent-inputs")] -use zcash_primitives::{ - consensus::BlockHeight, - legacy::TransparentAddress, - transaction::components::{OutPoint, TxOut}, -}; - /// A subset of a [`Transaction`] relevant to wallets and light clients. /// /// [`Transaction`]: zcash_primitives::transaction::Transaction @@ -29,14 +30,12 @@ pub struct WalletTx { pub shielded_outputs: Vec>, } -#[cfg(feature = "transparent-inputs")] pub struct WalletTransparentOutput { pub outpoint: OutPoint, pub txout: TxOut, pub height: BlockHeight, } -#[cfg(feature = "transparent-inputs")] impl WalletTransparentOutput { pub fn address(&self) -> TransparentAddress { self.txout.script_pubkey.address().unwrap() diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 23c9494cb..21369bb95 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -32,19 +32,16 @@ // Catch documentation errors caused by code changes. #![deny(rustdoc::broken_intra_doc_links)] +use rusqlite::Connection; use secrecy::{ExposeSecret, SecretVec}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fmt; use std::path::Path; -#[cfg(feature = "transparent-inputs")] -use std::collections::HashSet; - -use rusqlite::Connection; - use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight}, + legacy::TransparentAddress, memo::Memo, merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::{Node, Nullifier}, @@ -60,20 +57,14 @@ use zcash_client_backend::{ }, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, - wallet::SpendableNote, + wallet::{SpendableNote, WalletTransparentOutput}, TransferType, }; use crate::error::SqliteClientError; -#[cfg(feature = "transparent-inputs")] -use { - zcash_client_backend::{ - data_api::{WalletReadTransparent, WalletWriteTransparent}, - wallet::WalletTransparentOutput, - }, - zcash_primitives::legacy::TransparentAddress, -}; +#[cfg(not(feature = "transparent-inputs"))] +use zcash_client_backend::data_api::error::Error; #[cfg(feature = "unstable")] use { @@ -253,23 +244,32 @@ impl WalletRead for WalletDb

{ #[allow(deprecated)] wallet::transact::select_spendable_sapling_notes(self, account, target_value, anchor_height) } -} -#[cfg(feature = "transparent-inputs")] -impl WalletReadTransparent for WalletDb

{ fn get_transparent_receivers( &self, - account: AccountId, + _account: AccountId, ) -> Result, Self::Error> { - wallet::get_transparent_receivers(&self.params, &self.conn, account) + #[cfg(feature = "transparent-inputs")] + return wallet::get_transparent_receivers(&self.params, &self.conn, _account); + + #[cfg(not(feature = "transparent-inputs"))] + return Err(SqliteClientError::BackendError( + Error::TransparentInputsNotSupported, + )); } fn get_unspent_transparent_outputs( &self, - address: &TransparentAddress, - max_height: BlockHeight, + _address: &TransparentAddress, + _max_height: BlockHeight, ) -> Result, Self::Error> { - wallet::get_unspent_transparent_outputs(self, address, max_height) + #[cfg(feature = "transparent-inputs")] + return wallet::get_unspent_transparent_outputs(self, _address, _max_height); + + #[cfg(not(feature = "transparent-inputs"))] + return Err(SqliteClientError::BackendError( + Error::TransparentInputsNotSupported, + )); } } @@ -375,10 +375,7 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> { self.wallet_db .select_spendable_sapling_notes(account, target_value, anchor_height) } -} -#[cfg(feature = "transparent-inputs")] -impl<'a, P: consensus::Parameters> WalletReadTransparent for DataConnStmtCache<'a, P> { fn get_transparent_receivers( &self, account: AccountId, @@ -426,6 +423,8 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> { #[allow(deprecated)] impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> { + type UtxoRef = UtxoId; + fn create_account( &mut self, seed: &SecretVec, @@ -708,17 +707,18 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> { fn rewind_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error> { wallet::rewind_to_height(self.wallet_db, block_height) } -} - -#[cfg(feature = "transparent-inputs")] -impl<'a, P: consensus::Parameters> WalletWriteTransparent for DataConnStmtCache<'a, P> { - type UtxoRef = UtxoId; fn put_received_transparent_utxo( &mut self, - output: &WalletTransparentOutput, + _output: &WalletTransparentOutput, ) -> Result { - wallet::put_received_transparent_utxo(self, output) + #[cfg(feature = "transparent-inputs")] + return wallet::put_received_transparent_utxo(self, _output); + + #[cfg(not(feature = "transparent-inputs"))] + return Err(SqliteClientError::BackendError( + Error::TransparentInputsNotSupported, + )); } } @@ -1162,7 +1162,6 @@ mod tests { fn transparent_receivers() { use secrecy::Secret; use tempfile::NamedTempFile; - use zcash_client_backend::data_api::WalletReadTransparent; use crate::{chain::init::init_cache_database, wallet::init::init_wallet_db};