zcash_client_backend: Make `Balance` and `AccountBalance` fields private.

Public methods for mutation of these fields have been provided that
perform checking for overflow of the valid monetary range as part
of their operation.

zcash_client_backend/CHANGELOG.md

@@ -16,6 +16,12 @@ and this library adheres to Rust's notion of
       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`
@@ -54,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:

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<NonNegativeAmount, BalanceError> {
+        (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,10 +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.
-    pub orchard_balance: Balance,
+    orchard_balance: Balance,
 
     /// The value of all unspent transparent outputs belonging to the account, irrespective of
     /// confirmation depth.
@@ -105,7 +153,7 @@ 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 {
@@ -116,6 +164,58 @@ impl AccountBalance {
         unshielded: NonNegativeAmount::ZERO,
     };
 
+    fn check_total(&self) -> Result<NonNegativeAmount, BalanceError> {
+        (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<A, E: From<BalanceError>>(
+        &mut self,
+        f: impl FnOnce(&mut Balance) -> Result<A, E>,
+    ) -> Result<A, E> {
+        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<A, E: From<BalanceError>>(
+        &mut self,
+        f: impl FnOnce(&mut Balance) -> Result<A, E>,
+    ) -> Result<A, E> {
+        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.orchard_balance.total() + self.unshielded)
@@ -144,11 +244,6 @@ impl AccountBalance {
             + self.orchard_balance.value_pending_spendability)
             .expect("Account balance cannot overflow MAX_MONEY")
     }
-
-    /// Returns the total value of unspent transparent transaction outputs belonging to the wallet.
-    pub fn unshielded(&self) -> NonNegativeAmount {
-        self.unshielded
-    }
 }
 
 /// A polymorphic ratio type, usually used for rational numbers.

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
 

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<ShardTreeError<commitment_tree::Error>> for SqliteClientError {
         SqliteClientError::CommitmentTree(e)
     }
 }
+
+impl From<BalanceError> for SqliteClientError {
+    fn from(e: BalanceError) -> Self {
+        SqliteClientError::BalanceError(e)
+    }
+}

zcash_client_sqlite/src/testing.rs

@@ -708,7 +708,7 @@ impl<Cache> TestState<Cache> {
         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<Cache> TestState<Cache> {
         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<Cache> TestState<Cache> {
         min_confirmations: u32,
     ) -> NonNegativeAmount {
         self.with_account_balance(account, min_confirmations, |balance| {
-            balance.sapling_balance.change_pending_confirmation
+            balance.sapling_balance().change_pending_confirmation()
         })
     }
 

zcash_client_sqlite/src/wallet.rs

@@ -666,17 +666,14 @@ pub(crate) fn get_wallet_summary<P: consensus::Parameters>(
             }
         };
 
-        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<P: consensus::Parameters>(
                 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;

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 {