diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 1bd9f181a..fd2a182e7 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -12,8 +12,16 @@ and this library adheres to Rust's notion of - `BlockMetadata::orchard_tree_size`. - `TransparentInputSource` - `SaplingInputSource` - - `ScannedBlock::sapling_tree_size`. - - `ScannedBlock::orchard_tree_size`. + - `ScannedBlock::{ + sapling_tree_size, orchard_tree_size, orchard_nullifier_map, + orchard_commitments, into_commitments + }` + - `Balance::{add_spendable_value, add_pending_change_value, add_pending_spendable_value}` + - `AccountBalance::{ + with_sapling_balance_mut, + with_orchard_balance_mut, + add_unshielded_value + }` - `wallet::propose_standard_transfer_to_address` - `wallet::input_selection::Proposal::from_parts` - `wallet::input_selection::SaplingInputs` @@ -52,6 +60,9 @@ and this library adheres to Rust's notion of - `ShieldedProtocol` has a new variant for `Orchard`, allowing for better reporting to callers trying to perform actions using `Orchard` before it is fully supported. + - Fields of `Balance` and `AccountBalance` have been made private and the values + of these fields have been made available via methods having the same names + as the previously-public fields. - `chain::scan_cached_blocks` now returns a `ScanSummary` containing metadata about the scanned blocks on success. - `error::Error` enum changes: @@ -164,6 +175,9 @@ and this library adheres to Rust's notion of removed without replacement as it was unused, and its functionality will be fully reproduced by `SaplingInputSource::select_spendable_sapling_notes` in a future change. +- `zcash_client_backend::data_api::ScannedBlock::into_sapling_commitments` has been + replaced by `into_commitments` which returns both Sapling and Orchard note commitments + and associated note commitment retention information for the block. ## [0.10.0] - 2023-09-25 diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 65ce0f238..bce56e1b7 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -18,7 +18,7 @@ use zcash_primitives::{ sapling::{self, Node, NOTE_COMMITMENT_TREE_DEPTH}, transaction::{ components::{ - amount::{Amount, NonNegativeAmount}, + amount::{Amount, BalanceError, NonNegativeAmount}, OutPoint, }, Transaction, TxId, @@ -58,19 +58,9 @@ pub enum NullifierQuery { /// Balance information for a value within a single pool in an account. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Balance { - /// The value in the account that may currently be spent; it is possible to compute witnesses - /// for all the notes that comprise this value, and all of this value is confirmed to the - /// required confirmation depth. - pub spendable_value: NonNegativeAmount, - - /// The value in the account of shielded change notes that do not yet have sufficient - /// confirmations to be spendable. - pub change_pending_confirmation: NonNegativeAmount, - - /// The value in the account of all remaining received notes that either do not have sufficient - /// confirmations to be spendable, or for which witnesses cannot yet be constructed without - /// additional scanning. - pub value_pending_spendability: NonNegativeAmount, + spendable_value: NonNegativeAmount, + change_pending_confirmation: NonNegativeAmount, + value_pending_spendability: NonNegativeAmount, } impl Balance { @@ -81,6 +71,64 @@ impl Balance { value_pending_spendability: NonNegativeAmount::ZERO, }; + fn check_total_adding( + &self, + value: NonNegativeAmount, + ) -> Result { + (self.spendable_value + + self.change_pending_confirmation + + self.value_pending_spendability + + value) + .ok_or(BalanceError::Overflow) + } + + /// Returns the value in the account that may currently be spent; it is possible to compute + /// witnesses for all the notes that comprise this value, and all of this value is confirmed to + /// the required confirmation depth. + pub fn spendable_value(&self) -> NonNegativeAmount { + self.spendable_value + } + + /// Adds the specified value to the spendable total, checking for overflow. + pub fn add_spendable_value(&mut self, value: NonNegativeAmount) -> Result<(), BalanceError> { + self.check_total_adding(value)?; + self.spendable_value = (self.spendable_value + value).unwrap(); + Ok(()) + } + + /// Returns the value in the account of shielded change notes that do not yet have sufficient + /// confirmations to be spendable. + pub fn change_pending_confirmation(&self) -> NonNegativeAmount { + self.change_pending_confirmation + } + + /// Adds the specified value to the pending change total, checking for overflow. + pub fn add_pending_change_value( + &mut self, + value: NonNegativeAmount, + ) -> Result<(), BalanceError> { + self.check_total_adding(value)?; + self.change_pending_confirmation = (self.change_pending_confirmation + value).unwrap(); + Ok(()) + } + + /// Returns the value in the account of all remaining received notes that either do not have + /// sufficient confirmations to be spendable, or for which witnesses cannot yet be constructed + /// without additional scanning. + pub fn value_pending_spendability(&self) -> NonNegativeAmount { + self.value_pending_spendability + } + + /// Adds the specified value to the pending spendable total, checking for overflow. + pub fn add_pending_spendable_value( + &mut self, + value: NonNegativeAmount, + ) -> Result<(), BalanceError> { + self.check_total_adding(value)?; + self.value_pending_spendability = (self.value_pending_spendability + value).unwrap(); + Ok(()) + } + /// Returns the total value of funds represented by this [`Balance`]. pub fn total(&self) -> NonNegativeAmount { (self.spendable_value + self.change_pending_confirmation + self.value_pending_spendability) @@ -93,7 +141,10 @@ impl Balance { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct AccountBalance { /// The value of unspent Sapling outputs belonging to the account. - pub sapling_balance: Balance, + sapling_balance: Balance, + + /// The value of unspent Orchard outputs belonging to the account. + orchard_balance: Balance, /// The value of all unspent transparent outputs belonging to the account, irrespective of /// confirmation depth. @@ -102,19 +153,95 @@ pub struct AccountBalance { /// possible operation on a transparent balance is to shield it, it is possible to create a /// zero-conf transaction to perform that shielding, and the resulting shielded notes will be /// subject to normal confirmation rules. - pub unshielded: NonNegativeAmount, + unshielded: NonNegativeAmount, } impl AccountBalance { /// The [`Balance`] value having zero values for all its fields. pub const ZERO: Self = Self { sapling_balance: Balance::ZERO, + orchard_balance: Balance::ZERO, unshielded: NonNegativeAmount::ZERO, }; + fn check_total(&self) -> Result { + (self.sapling_balance.total() + self.orchard_balance.total() + self.unshielded) + .ok_or(BalanceError::Overflow) + } + + /// Returns the [`Balance`] of Sapling funds in the account. + pub fn sapling_balance(&self) -> &Balance { + &self.sapling_balance + } + + /// Provides a `mutable reference to the [`Balance`] of Sapling funds in the account + /// to the specified callback, checking invariants after the callback's action has been + /// evaluated. + pub fn with_sapling_balance_mut>( + &mut self, + f: impl FnOnce(&mut Balance) -> Result, + ) -> Result { + let result = f(&mut self.sapling_balance)?; + self.check_total()?; + Ok(result) + } + + /// Returns the [`Balance`] of Orchard funds in the account. + pub fn orchard_balance(&self) -> &Balance { + &self.orchard_balance + } + + /// Provides a `mutable reference to the [`Balance`] of Orchard funds in the account + /// to the specified callback, checking invariants after the callback's action has been + /// evaluated. + pub fn with_orchard_balance_mut>( + &mut self, + f: impl FnOnce(&mut Balance) -> Result, + ) -> Result { + let result = f(&mut self.orchard_balance)?; + self.check_total()?; + Ok(result) + } + + /// Returns the total value of unspent transparent transaction outputs belonging to the wallet. + pub fn unshielded(&self) -> NonNegativeAmount { + self.unshielded + } + + /// Adds the specified value to the unshielded total, checking for overflow of + /// the total account balance. + pub fn add_unshielded_value(&mut self, value: NonNegativeAmount) -> Result<(), BalanceError> { + self.unshielded = (self.unshielded + value).ok_or(BalanceError::Overflow)?; + self.check_total()?; + Ok(()) + } + /// Returns the total value of funds belonging to the account. pub fn total(&self) -> NonNegativeAmount { - (self.sapling_balance.total() + self.unshielded) + (self.sapling_balance.total() + self.orchard_balance.total() + self.unshielded) + .expect("Account balance cannot overflow MAX_MONEY") + } + + /// Returns the total value of shielded (Sapling and Orchard) funds that may immediately be + /// spent. + pub fn spendable_value(&self) -> NonNegativeAmount { + (self.sapling_balance.spendable_value + self.orchard_balance.spendable_value) + .expect("Account balance cannot overflow MAX_MONEY") + } + + /// Returns the total value of change and/or shielding transaction outputs that are awaiting + /// sufficient confirmations for spendability. + pub fn change_pending_confirmation(&self) -> NonNegativeAmount { + (self.sapling_balance.change_pending_confirmation + + self.orchard_balance.change_pending_confirmation) + .expect("Account balance cannot overflow MAX_MONEY") + } + + /// Returns the value of shielded funds that are not yet spendable because additional scanning + /// is required before it will be possible to derive witnesses for the associated notes. + pub fn value_pending_spendability(&self) -> NonNegativeAmount { + (self.sapling_balance.value_pending_spendability + + self.orchard_balance.value_pending_spendability) .expect("Account balance cannot overflow MAX_MONEY") } } @@ -484,6 +611,8 @@ pub struct ScannedBlock { transactions: Vec>, sapling_nullifier_map: Vec<(TxId, u16, Vec)>, sapling_commitments: Vec<(sapling::Node, Retention)>, + orchard_nullifier_map: Vec<(TxId, u16, Vec)>, + orchard_commitments: Vec<(orchard::note::NoteCommitment, Retention)>, } impl ScannedBlock { @@ -498,6 +627,8 @@ impl ScannedBlock { transactions: Vec>, sapling_nullifier_map: Vec<(TxId, u16, Vec)>, sapling_commitments: Vec<(sapling::Node, Retention)>, + orchard_nullifier_map: Vec<(TxId, u16, Vec)>, + orchard_commitments: Vec<(orchard::note::NoteCommitment, Retention)>, ) -> Self { Self { block_height, @@ -508,6 +639,8 @@ impl ScannedBlock { transactions, sapling_nullifier_map, sapling_commitments, + orchard_nullifier_map, + orchard_commitments, } } @@ -557,10 +690,34 @@ impl ScannedBlock { &self.sapling_commitments } - /// Consumes `self` and returns the list of Sapling note commitments associated with the - /// scanned block as an owned value. - pub fn into_sapling_commitments(self) -> Vec<(sapling::Node, Retention)> { - self.sapling_commitments + /// Returns the vector of Orchard nullifiers for each transaction in the block. + /// + /// The returned tuple is keyed by both transaction ID and the index of the transaction within + /// the block, so that either the txid or the combination of the block hash available from + /// [`Self::block_hash`] and returned transaction index may be used to uniquely identify the + /// transaction, depending upon the needs of the caller. + pub fn orchard_nullifier_map(&self) -> &[(TxId, u16, Vec)] { + &self.orchard_nullifier_map + } + + /// Returns the ordered list of Orchard note commitments to be added to the note commitment + /// tree. + pub fn orchard_commitments( + &self, + ) -> &[(orchard::note::NoteCommitment, Retention)] { + &self.orchard_commitments + } + + /// Consumes `self` and returns the lists of Sapling and Orchard note commitments associated + /// with the scanned block as an owned value. + #[allow(clippy::type_complexity)] + pub fn into_commitments( + self, + ) -> ( + Vec<(sapling::Node, Retention)>, + Vec<(orchard::note::NoteCommitment, Retention)>, + ) { + (self.sapling_commitments, self.orchard_commitments) } /// Returns the [`BlockMetadata`] corresponding to the scanned block. diff --git a/zcash_client_backend/src/scanning.rs b/zcash_client_backend/src/scanning.rs index 073056b76..5f778e487 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -644,6 +644,8 @@ pub(crate) fn scan_block_with_runner< wtxs, sapling_nullifier_map, sapling_note_commitments, + vec![], // FIXME: collect the Orchard nullifiers + vec![], // FIXME: collect the Orchard note commitments )) } diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index eb577b7c7..a2ded0be8 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -8,8 +8,9 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Changed -- `zcash_client_sqlite::error::SqliteClientError` has new error variant: +- `zcash_client_sqlite::error::SqliteClientError` has new error variants: - `SqliteClientError::UnsupportedPoolType` + - `SqliteClientError::BalanceError` ## [0.8.1] - 2023-10-18 diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 82104d285..952b0f93c 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -6,6 +6,7 @@ use std::fmt; use shardtree::error::ShardTreeError; use zcash_client_backend::data_api::PoolType; use zcash_client_backend::encoding::{Bech32DecodeError, TransparentCodecError}; +use zcash_primitives::transaction::components::amount::BalanceError; use zcash_primitives::{consensus::BlockHeight, zip32::AccountId}; use crate::wallet::commitment_tree; @@ -102,6 +103,9 @@ pub enum SqliteClientError { /// Unsupported pool type UnsupportedPoolType(PoolType), + + /// An error occurred in computing wallet balance + BalanceError(BalanceError), } impl error::Error for SqliteClientError { @@ -111,6 +115,7 @@ impl error::Error for SqliteClientError { SqliteClientError::Bech32DecodeError(Bech32DecodeError::Bech32Error(e)) => Some(e), SqliteClientError::DbError(e) => Some(e), SqliteClientError::Io(e) => Some(e), + SqliteClientError::BalanceError(e) => Some(e), _ => None, } } @@ -149,7 +154,8 @@ impl fmt::Display for SqliteClientError { SqliteClientError::CommitmentTree(err) => write!(f, "An error occurred accessing or updating note commitment tree data: {}.", err), SqliteClientError::CacheMiss(height) => write!(f, "Requested height {} does not exist in the block cache.", height), 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::UnsupportedPoolType(t) => write!(f, "Pool type is not currently supported: {}", t), + SqliteClientError::BalanceError(e) => write!(f, "Balance error: {}", e), } } } @@ -202,3 +208,9 @@ impl From> for SqliteClientError { SqliteClientError::CommitmentTree(e) } } + +impl From for SqliteClientError { + fn from(e: BalanceError) -> Self { + SqliteClientError::BalanceError(e) + } +} diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 596db68fc..39dae79fc 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -511,7 +511,8 @@ impl WalletWrite for WalletDb })); last_scanned_height = Some(block.height()); - sapling_commitments.extend(block.into_sapling_commitments().into_iter().map(Some)); + let (block_sapling_commitments, _) = block.into_commitments(); + sapling_commitments.extend(block_sapling_commitments.into_iter().map(Some)); } // Prune the nullifier map of entries we no longer need. diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 149f7fc45..3a9b77ec9 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -708,7 +708,7 @@ impl TestState { min_confirmations: u32, ) -> NonNegativeAmount { self.with_account_balance(account, min_confirmations, |balance| { - balance.sapling_balance.spendable_value + balance.sapling_balance().spendable_value() }) } @@ -718,8 +718,8 @@ impl TestState { min_confirmations: u32, ) -> NonNegativeAmount { self.with_account_balance(account, min_confirmations, |balance| { - balance.sapling_balance.value_pending_spendability - + balance.sapling_balance.change_pending_confirmation + balance.sapling_balance().value_pending_spendability() + + balance.sapling_balance().change_pending_confirmation() }) .unwrap() } @@ -731,7 +731,7 @@ impl TestState { min_confirmations: u32, ) -> NonNegativeAmount { self.with_account_balance(account, min_confirmations, |balance| { - balance.sapling_balance.change_pending_confirmation + balance.sapling_balance().change_pending_confirmation() }) } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 32f6ac999..3458b5132 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -666,17 +666,14 @@ pub(crate) fn get_wallet_summary( } }; - account_balances.entry(account).and_modify(|bal| { - bal.sapling_balance.spendable_value = (bal.sapling_balance.spendable_value - + spendable_value) - .expect("Spendable value cannot overflow"); - bal.sapling_balance.change_pending_confirmation = - (bal.sapling_balance.change_pending_confirmation + change_pending_confirmation) - .expect("Pending change value cannot overflow"); - bal.sapling_balance.value_pending_spendability = - (bal.sapling_balance.value_pending_spendability + value_pending_spendability) - .expect("Value pending spendability cannot overflow"); - }); + if let Some(balances) = account_balances.get_mut(&account) { + balances.with_sapling_balance_mut::<_, SqliteClientError>(|bal| { + bal.add_spendable_value(spendable_value)?; + bal.add_pending_change_value(change_pending_confirmation)?; + bal.add_pending_spendable_value(value_pending_spendability)?; + Ok(()) + })?; + } } #[cfg(feature = "transparent-inputs")] @@ -705,9 +702,9 @@ pub(crate) fn get_wallet_summary( SqliteClientError::CorruptedData(format!("Negative UTXO value {:?}", raw_value)) })?; - account_balances.entry(account).and_modify(|bal| { - bal.unshielded = (bal.unshielded + value).expect("Unshielded value cannot overflow") - }); + if let Some(balances) = account_balances.get_mut(&account) { + balances.add_unshielded_value(value)?; + } } } @@ -2138,7 +2135,7 @@ mod tests { .unwrap() .unwrap(); let balance = summary.account_balances().get(&account_id).unwrap(); - assert_eq!(balance.unshielded, expected); + assert_eq!(balance.unshielded(), expected); // Check the older APIs for consistency. let max_height = st.wallet().chain_height().unwrap().unwrap() + 1 - min_confirmations; diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index 1f4977ab6..0a6a701a0 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -1,4 +1,5 @@ use std::convert::TryFrom; +use std::error; use std::iter::Sum; use std::ops::{Add, AddAssign, Mul, Neg, Sub, SubAssign}; @@ -394,6 +395,8 @@ pub enum BalanceError { Underflow, } +impl error::Error for BalanceError {} + impl std::fmt::Display for BalanceError { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match &self {