Merge pull request #852 from nuttycom/wallet/sapling_cleanup

Consolidate sapling functionality in zcash_client_sqlite in a dedicated module.

Cargo.toml

@@ -21,5 +21,5 @@ codegen-units = 1
 [patch.crates-io]
 zcash_encoding = { path = "components/zcash_encoding" }
 zcash_note_encryption = { path = "components/zcash_note_encryption" }
-incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree.git", rev = "62f0c9039b0bee94c16c40c272e19c5922290664" }
-orchard = { git = "https://github.com/zcash/orchard.git", rev = "2a4f27c937fbcbdb66163e1bb426ce1fcb5bc4f8" }
+incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree.git", rev = "2dbdd345670ea22337a0efa6734272d54551285f" }
+orchard = { git = "https://github.com/zcash/orchard.git", rev = "35054e85b85dc144b4572ed0fd57ea164f50c26a" }

zcash_client_backend/Cargo.toml

@@ -20,6 +20,7 @@ exclude = ["*.proto"]
 development = ["zcash_proofs"]
 
 [dependencies]
+incrementalmerkletree = { version = "0.3", features = ["legacy-api"] }
 zcash_address = { version = "0.2", path = "../components/zcash_address" }
 zcash_encoding = { version = "0.2", path = "../components/zcash_encoding" }
 zcash_note_encryption = "0.3"
@@ -94,6 +95,7 @@ test-dependencies = [
     "proptest",
     "orchard/test-dependencies",
     "zcash_primitives/test-dependencies",
+    "incrementalmerkletree/test-dependencies"
 ]
 unstable = ["byteorder"]
 

zcash_client_backend/src/data_api.rs

@@ -10,7 +10,7 @@ use zcash_primitives::{
     consensus::BlockHeight,
     legacy::TransparentAddress,
     memo::{Memo, MemoBytes},
-    sapling::{self, Nullifier, PaymentAddress},
+    sapling,
     transaction::{
         components::{amount::Amount, OutPoint},
         Transaction, TxId,
@@ -22,13 +22,18 @@ use crate::{
     address::{AddressMetadata, UnifiedAddress},
     decrypt::DecryptedOutput,
     keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
-    wallet::{SpendableNote, WalletTransparentOutput, WalletTx},
+    wallet::{ReceivedSaplingNote, WalletTransparentOutput, WalletTx},
 };
 
 pub mod chain;
 pub mod error;
 pub mod wallet;
 
+pub enum NullifierQuery {
+    Unspent,
+    All,
+}
+
 /// Read-only operations required for light wallet functions.
 ///
 /// This trait defines the read-only portion of the storage interface atop which
@@ -107,15 +112,15 @@ pub trait WalletRead {
             .map(|oo| oo.flatten())
     }
 
-    /// Returns the block height in which the specified transaction was mined,
-    /// or `Ok(None)` if the transaction is not mined in the main chain.
+    /// Returns the block height in which the specified transaction was mined, or `Ok(None)` if the
+    /// transaction is not in the main chain.
     fn get_tx_height(&self, txid: TxId) -> Result<Option<BlockHeight>, Self::Error>;
 
     /// Returns the most recently generated unified address for the specified account, if the
     /// account identifier specified refers to a valid account for this wallet.
     ///
-    /// This will return `Ok(None)` if the account identifier does not correspond
-    /// to a known account.
+    /// This will return `Ok(None)` if the account identifier does not correspond to a known
+    /// account.
     fn get_current_address(
         &self,
         account: AccountId,
@@ -126,26 +131,23 @@ pub trait WalletRead {
         &self,
     ) -> Result<HashMap<AccountId, UnifiedFullViewingKey>, Self::Error>;
 
-    /// Returns the account id corresponding to a given [`UnifiedFullViewingKey`],
-    /// if any.
+    /// Returns the account id corresponding to a given [`UnifiedFullViewingKey`], if any.
     fn get_account_for_ufvk(
         &self,
         ufvk: &UnifiedFullViewingKey,
     ) -> Result<Option<AccountId>, Self::Error>;
 
-    /// Checks whether the specified extended full viewing key is
-    /// associated with the account.
+    /// Checks whether the specified extended full viewing key is associated with the account.
     fn is_valid_account_extfvk(
         &self,
         account: AccountId,
         extfvk: &ExtendedFullViewingKey,
     ) -> Result<bool, Self::Error>;
 
-    /// Returns the wallet balance for an account as of the specified block
-    /// height.
+    /// Returns the wallet balance for an account as of the specified block height.
     ///
-    /// This may be used to obtain a balance that ignores notes that have been
-    /// received so recently that they are not yet deemed spendable.
+    /// This may be used to obtain a balance that ignores notes that have been received so recently
+    /// that they are not yet deemed spendable.
     fn get_balance_at(
         &self,
         account: AccountId,
@@ -176,33 +178,31 @@ pub trait WalletRead {
         block_height: BlockHeight,
     ) -> Result<Vec<(Self::NoteRef, sapling::IncrementalWitness)>, Self::Error>;
 
-    /// Returns the nullifiers for notes that the wallet is tracking, along with their
-    /// associated account IDs, that are either unspent or have not yet been confirmed as
-    /// spent (in that the spending transaction has not yet been included in a block).
-    fn get_nullifiers(&self) -> Result<Vec<(AccountId, Nullifier)>, Self::Error>;
-
-    /// Returns all nullifiers for notes that the wallet is tracking
-    /// (including those for notes that have been previously spent),
-    /// along with the account identifiers with which they are associated.
-    fn get_all_nullifiers(&self) -> Result<Vec<(AccountId, Nullifier)>, Self::Error>;
+    /// Returns the nullifiers for notes that the wallet is tracking, along with their associated
+    /// account IDs, that are either unspent or have not yet been confirmed as spent (in that a
+    /// spending transaction known to the wallet has not yet been included in a block).
+    fn get_sapling_nullifiers(
+        &self,
+        query: NullifierQuery,
+    ) -> Result<Vec<(AccountId, sapling::Nullifier)>, Self::Error>;
 
     /// Return all unspent Sapling notes.
     fn get_spendable_sapling_notes(
         &self,
         account: AccountId,
         anchor_height: BlockHeight,
         exclude: &[Self::NoteRef],
-    ) -> Result<Vec<SpendableNote<Self::NoteRef>>, Self::Error>;
+    ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error>;
 
-    /// Returns a list of spendable Sapling notes sufficient to cover the specified
-    /// target value, if possible.
+    /// Returns a list of spendable Sapling notes sufficient to cover the specified target value,
+    /// if possible.
     fn select_spendable_sapling_notes(
         &self,
         account: AccountId,
         target_value: Amount,
         anchor_height: BlockHeight,
         exclude: &[Self::NoteRef],
-    ) -> Result<Vec<SpendableNote<Self::NoteRef>>, Self::Error>;
+    ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error>;
 
     /// Returns the set of all transparent receivers associated with the given account.
     ///
@@ -241,7 +241,7 @@ pub struct PrunedBlock<'a> {
     pub block_hash: BlockHash,
     pub block_time: u32,
     pub commitment_tree: &'a sapling::CommitmentTree,
-    pub transactions: &'a Vec<WalletTx<Nullifier>>,
+    pub transactions: &'a Vec<WalletTx<sapling::Nullifier>>,
 }
 
 /// A transaction that was detected during scanning of the blockchain,
@@ -286,7 +286,7 @@ pub enum PoolType {
 #[derive(Debug, Clone)]
 pub enum Recipient {
     Transparent(TransparentAddress),
-    Sapling(PaymentAddress),
+    Sapling(sapling::PaymentAddress),
     Unified(UnifiedAddress, PoolType),
     InternalAccount(AccountId, PoolType),
 }
@@ -395,7 +395,7 @@ pub trait WalletWrite: WalletRead {
     /// Caches a decrypted transaction in the persistent wallet store.
     fn store_decrypted_tx(
         &mut self,
-        received_tx: &DecryptedTransaction,
+        received_tx: DecryptedTransaction,
     ) -> Result<Self::TxRef, Self::Error>;
 
     /// Saves information about a transaction that was constructed and sent by the wallet to the
@@ -434,7 +434,7 @@ pub mod testing {
         consensus::{BlockHeight, Network},
         legacy::TransparentAddress,
         memo::Memo,
-        sapling::{self, Nullifier},
+        sapling,
         transaction::{
             components::{Amount, OutPoint},
             Transaction, TxId,
@@ -445,10 +445,12 @@ pub mod testing {
     use crate::{
         address::{AddressMetadata, UnifiedAddress},
         keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
-        wallet::{SpendableNote, WalletTransparentOutput},
+        wallet::{ReceivedSaplingNote, WalletTransparentOutput},
     };
 
-    use super::{DecryptedTransaction, PrunedBlock, SentTransaction, WalletRead, WalletWrite};
+    use super::{
+        DecryptedTransaction, NullifierQuery, PrunedBlock, SentTransaction, WalletRead, WalletWrite,
+    };
 
     pub struct MockWalletDb {
         pub network: Network,
@@ -537,11 +539,10 @@ pub mod testing {
             Ok(Vec::new())
         }
 
-        fn get_nullifiers(&self) -> Result<Vec<(AccountId, Nullifier)>, Self::Error> {
-            Ok(Vec::new())
-        }
-
-        fn get_all_nullifiers(&self) -> Result<Vec<(AccountId, Nullifier)>, Self::Error> {
+        fn get_sapling_nullifiers(
+            &self,
+            _query: NullifierQuery,
+        ) -> Result<Vec<(AccountId, sapling::Nullifier)>, Self::Error> {
             Ok(Vec::new())
         }
 
@@ -550,7 +551,7 @@ pub mod testing {
             _account: AccountId,
             _anchor_height: BlockHeight,
             _exclude: &[Self::NoteRef],
-        ) -> Result<Vec<SpendableNote<Self::NoteRef>>, Self::Error> {
+        ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error> {
             Ok(Vec::new())
         }
 
@@ -560,7 +561,7 @@ pub mod testing {
             _target_value: Amount,
             _anchor_height: BlockHeight,
             _exclude: &[Self::NoteRef],
-        ) -> Result<Vec<SpendableNote<Self::NoteRef>>, Self::Error> {
+        ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error> {
             Ok(Vec::new())
         }
 
@@ -620,7 +621,7 @@ pub mod testing {
 
         fn store_decrypted_tx(
             &mut self,
-            _received_tx: &DecryptedTransaction,
+            _received_tx: DecryptedTransaction,
         ) -> Result<Self::TxRef, Self::Error> {
             Ok(TxId::from_bytes([0u8; 32]))
         }

zcash_client_backend/src/data_api/chain.rs

@@ -104,6 +104,8 @@ use crate::{
 pub mod error;
 use error::{ChainError, Error};
 
+use super::NullifierQuery;
+
 /// This trait provides sequential access to raw blockchain data via a callback-oriented
 /// API.
 pub trait BlockSource {
@@ -252,7 +254,9 @@ where
     )?;
 
     // Get the nullifiers for the notes we are tracking
-    let mut nullifiers = data_db.get_nullifiers().map_err(Error::Wallet)?;
+    let mut nullifiers = data_db
+        .get_sapling_nullifiers(NullifierQuery::Unspent)
+        .map_err(Error::Wallet)?;
 
     let mut batch_runner = BatchRunner::<_, _, _, ()>::new(
         100,

zcash_client_backend/src/data_api/wallet.rs

@@ -28,7 +28,7 @@ use crate::{
     decrypt_transaction,
     fees::{self, ChangeValue, DustOutputPolicy},
     keys::UnifiedSpendingKey,
-    wallet::{OvkPolicy, SpendableNote},
+    wallet::{OvkPolicy, ReceivedSaplingNote},
     zip321::{self, Payment},
 };
 
@@ -68,7 +68,7 @@ where
         .or_else(|| params.activation_height(NetworkUpgrade::Sapling))
         .expect("Sapling activation height must be known.");
 
-    data.store_decrypted_tx(&DecryptedTransaction {
+    data.store_decrypted_tx(DecryptedTransaction {
         tx,
         sapling_outputs: &decrypt_transaction(params, height, tx, &ufvks),
     })?;
@@ -701,7 +701,7 @@ where
 }
 
 fn select_key_for_note<N>(
-    selected: &SpendableNote<N>,
+    selected: &ReceivedSaplingNote<N>,
     extsk: &ExtendedSpendingKey,
     dfvk: &DiversifiableFullViewingKey,
 ) -> Option<(sapling::Note, ExtendedSpendingKey, sapling::MerklePath)> {

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -22,7 +22,7 @@ use crate::{
     address::{RecipientAddress, UnifiedAddress},
     data_api::WalletRead,
     fees::{ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance},
-    wallet::{SpendableNote, WalletTransparentOutput},
+    wallet::{ReceivedSaplingNote, WalletTransparentOutput},
     zip321::TransactionRequest,
 };
 
@@ -68,7 +68,7 @@ impl<DE: fmt::Display, SE: fmt::Display> fmt::Display for InputSelectorError<DE,
 pub struct Proposal<FeeRuleT, NoteRef> {
     transaction_request: TransactionRequest,
     transparent_inputs: Vec<WalletTransparentOutput>,
-    sapling_inputs: Vec<SpendableNote<NoteRef>>,
+    sapling_inputs: Vec<ReceivedSaplingNote<NoteRef>>,
     balance: TransactionBalance,
     fee_rule: FeeRuleT,
     target_height: BlockHeight,
@@ -85,7 +85,7 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
         &self.transparent_inputs
     }
     /// Returns the Sapling inputs that have been selected to fund the transaction.
-    pub fn sapling_inputs(&self) -> &[SpendableNote<NoteRef>] {
+    pub fn sapling_inputs(&self) -> &[ReceivedSaplingNote<NoteRef>] {
         &self.sapling_inputs
     }
     /// Returns the change outputs to be added to the transaction and the fee to be paid.
@@ -336,7 +336,7 @@ where
             }
         }
 
-        let mut sapling_inputs: Vec<SpendableNote<DbT::NoteRef>> = vec![];
+        let mut sapling_inputs: Vec<ReceivedSaplingNote<DbT::NoteRef>> = vec![];
         let mut prior_available = Amount::zero();
         let mut amount_required = Amount::zero();
         let mut exclude: Vec<DbT::NoteRef> = vec![];
@@ -425,7 +425,7 @@ where
             target_height,
             &transparent_inputs,
             &Vec::<TxOut>::new(),
-            &Vec::<SpendableNote<DbT::NoteRef>>::new(),
+            &Vec::<ReceivedSaplingNote<DbT::NoteRef>>::new(),
             &Vec::<SaplingPayment>::new(),
             &self.dust_output_policy,
         );
@@ -441,7 +441,7 @@ where
                     target_height,
                     &transparent_inputs,
                     &Vec::<TxOut>::new(),
-                    &Vec::<SpendableNote<DbT::NoteRef>>::new(),
+                    &Vec::<ReceivedSaplingNote<DbT::NoteRef>>::new(),
                     &Vec::<SaplingPayment>::new(),
                     &self.dust_output_policy,
                 )?

zcash_client_backend/src/wallet.rs

@@ -177,15 +177,15 @@ impl<N> WalletSaplingOutput<N> {
 
 /// Information about a note that is tracked by the wallet that is available for spending,
 /// with sufficient information for use in note selection.
-pub struct SpendableNote<NoteRef> {
+pub struct ReceivedSaplingNote<NoteRef> {
     pub note_id: NoteRef,
     pub diversifier: sapling::Diversifier,
     pub note_value: Amount,
     pub rseed: sapling::Rseed,
     pub witness: sapling::IncrementalWitness,
 }
 
-impl<NoteRef> sapling_fees::InputView<NoteRef> for SpendableNote<NoteRef> {
+impl<NoteRef> sapling_fees::InputView<NoteRef> for ReceivedSaplingNote<NoteRef> {
     fn note_id(&self) -> &NoteRef {
         &self.note_id
     }

zcash_client_backend/src/welding_rig.rs

@@ -91,7 +91,7 @@ impl ScanningKey for DiversifiableFullViewingKey {
     ) -> Self::Nf {
         note.nf(
             key,
-            u64::try_from(witness.position())
+            u64::try_from(witness.tip_position())
                 .expect("Sapling note commitment tree position must fit into a u64"),
         )
     }

zcash_client_sqlite/CHANGELOG.md

@@ -10,6 +10,9 @@ and this library adheres to Rust's notion of
 - MSRV is now 1.65.0.
 - Bumped dependencies to `hdwallet 0.4`.
 
+### Removed
+- The empty `wallet::transact` module has been removed.
+
 ## [0.7.1] - 2023-05-17
 
 ### Fixed