From 3734cda925611c1aea7127e60563fbb20d7e6f19 Mon Sep 17 00:00:00 2001
From: Francisco Gindre <francisco.gindre@gmail.com>
Date: Sun, 20 Oct 2024 17:20:33 -0300
Subject: [PATCH] Add PR Suggestions:

- remove method instead of deprecating it.
- Formatting
- CHANGELOG entries
---
 zcash_client_backend/src/data_api.rs          | 12 +----------
 zcash_client_sqlite/src/wallet/transparent.rs | 10 ++-------
 zcash_primitives/CHANGELOG.md                 | 21 ++++++++++++-------
 3 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs
index 651e060b69..47d2314e79 100644
--- a/zcash_client_backend/src/data_api.rs
+++ b/zcash_client_backend/src/data_api.rs
@@ -287,7 +287,7 @@ impl AccountBalance {
 
     /// Returns the total value of unspent transparent transaction outputs belonging to the wallet.
     #[deprecated(
-        note = "this function is deprecated. Please use AccountBalance::unshielded_balance instead."
+        note = "this function is deprecated. Please use [`AccountBalance::unshielded_balance`] instead."
     )]
     pub fn unshielded(&self) -> NonNegativeAmount {
         self.unshielded_balance.total()
@@ -310,16 +310,6 @@ impl AccountBalance {
         Ok(result)
     }
 
-    /// Adds the specified value to the unshielded total, checking for overflow of
-    /// the total account balance.
-    #[deprecated(
-        note = "this function is deprecated. Please use the `Balance::add_spendable_value` on the unshielded field instead instead."
-    )]
-    pub fn add_unshielded_value(&mut self, value: NonNegativeAmount) -> Result<(), BalanceError> {
-        self.unshielded_balance.add_pending_spendable_value(value)?;
-        Ok(())
-    }
-
     /// Returns the total value of funds belonging to the account.
     pub fn total(&self) -> NonNegativeAmount {
         (self.sapling_balance.total()
diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs
index 4f4b537e44..89181ea87b 100644
--- a/zcash_client_sqlite/src/wallet/transparent.rs
+++ b/zcash_client_sqlite/src/wallet/transparent.rs
@@ -423,10 +423,7 @@ pub(crate) fn add_transparent_account_balances(
         account_balances
             .entry(account)
             .or_insert(AccountBalance::ZERO)
-            .with_unshielded_balance_mut::<_, SqliteClientError>(|bal| {
-                bal.add_spendable_value(value)?;
-                Ok(())
-            })?;
+            .with_unshielded_balance_mut(|bal| bal.add_spendable_value(value))?;
     }
 
     let mut stmt_account_unconfirmed_balances = conn.prepare(
@@ -469,10 +466,7 @@ pub(crate) fn add_transparent_account_balances(
         account_balances
             .entry(account)
             .or_insert(AccountBalance::ZERO)
-            .with_unshielded_balance_mut::<_, SqliteClientError>(|bal| {
-                bal.add_pending_spendable_value(value)?;
-                Ok(())
-            })?;
+            .with_unshielded_balance_mut(|bal| bal.add_pending_spendable_value(value))?;
     }
     Ok(())
 }
diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md
index 4e243c549c..283d1418fe 100644
--- a/zcash_primitives/CHANGELOG.md
+++ b/zcash_primitives/CHANGELOG.md
@@ -10,17 +10,24 @@ and this library adheres to Rust's notion of
 - `zcash_client_backend::AccountBalance::with_unshielded_balance_mut`
 
 ### Deprecated
-- `zcash_client_backend::AccountBalance::add_unshielded_value`
-- `zcash_client_backend::AccountBalance::unshielded`
+- `zcash_client_backend::AccountBalance::unshielded`. Instead use
+  `account_balance.unshielded_balance().total()`.
+  
+### Removed
+- `zcash_client_backend::AccountBalance::add_unshielded_value`. Instead use
+  `AccountBalance::with_unshielded_balance_mut` with a closure that calls
+  the appropriate `add_*_value` method(s) of `Balance` on its argument.
+  Note that the appropriate method(s) depend on whether the funds are
+  spendable, pending change, or pending non-change (previously, only the
+  total unshielded value was tracked).
 ## [0.19.0] - 2024-10-02
 
 ### Changed
 - Migrated to `zcash_address 0.6`.
-- Implemented changes to refactor AccountBalance to use Balance for transparent funds
-(see issue #1411). `AccountBalance` now has an `unshielded` value that uses Balance.
-The current implementation maintains retrocompatibility with the `unshielded` value
-represeted with a `NonNegativeAmount`. There are values that are pending to be 
-implemented such as change tracking.
+- Refactored `AccountBalance` to use `Balance` for transparent funds (issue #1411).
+  `AccountBalance` now has an `unshielded_balance()` that uses `Balance`. This does
+  not currently distinguish between pending change and non-change; the pending value
+  is all counted as non-change.
   
 
 ### Fixed