Merge branch 'main' into 741-sapling-builder-refactor

zcash_client_backend/CHANGELOG.md

@@ -27,8 +27,12 @@ and this library adheres to Rust's notion of
   - The `NoteMismatch` variant of `data_api::error::Error` now wraps a
     `data_api::NoteId` instead of a backend-specific note identifier. The
     related `NoteRef` type parameter has been removed from `data_api::error::Error`.
-  - `wallet::create_spend_to_address` now takes a `NonNegativeAmount` rather than
-    an `Amount`.
+  - `SentTransactionOutput::value` is now represented as `NonNegativeAmount` instead of
+    `Amount`, and constructors and accessors have been updated accordingly.
+  - The `available` and `required` fields of `data_api::error::Error::InsufficientFunds`
+    are now represented as `NonNegativeAmount` instead of `Amount`.
+  - `data_api::wallet::create_spend_to_address` now takes its `amount` argument as
+    as `NonNegativeAmount` instead of `Amount`.
   - All uses of `Amount` in `data_api::wallet::input_selection` have been replaced
     with `NonNegativeAmount`.
   - `wallet::shield_transparent_funds` no longer
@@ -50,6 +54,14 @@ and this library adheres to Rust's notion of
   accept an additional `change_memo` argument.
 - All uses of `Amount` in `zcash_client_backend::fees` have been replaced
   with `NonNegativeAmount`.
+- `zcash_client_backend::wallet::WalletTransparentOutput::value` is now represented
+  as `NonNegativeAmount` instead of `Amount`, and constructors and accessors have been
+  updated accordingly.
+- `zcash_client_backend::wallet::ReceivedSaplingNote::value` is now represented
+  as `NonNegativeAmount` instead of `Amount`, and constructors and accessors have been
+  updated accordingly.
+- Almost all uses of `Amount` in `zcash_client_backend::zip321` have been replaced
+  with `NonNegativeAmount`.
 
 ## [0.10.0] - 2023-09-25
 

zcash_client_backend/src/data_api.rs

@@ -630,7 +630,7 @@ pub enum Recipient {
 pub struct SentTransactionOutput {
     output_index: usize,
     recipient: Recipient,
-    value: Amount,
+    value: NonNegativeAmount,
     memo: Option<MemoBytes>,
     sapling_change_to: Option<(AccountId, sapling::Note)>,
 }
@@ -639,7 +639,7 @@ impl SentTransactionOutput {
     pub fn from_parts(
         output_index: usize,
         recipient: Recipient,
-        value: Amount,
+        value: NonNegativeAmount,
         memo: Option<MemoBytes>,
         sapling_change_to: Option<(AccountId, sapling::Note)>,
     ) -> Self {
@@ -667,7 +667,7 @@ impl SentTransactionOutput {
         &self.recipient
     }
     /// Returns the value of the newly created output.
-    pub fn value(&self) -> Amount {
+    pub fn value(&self) -> NonNegativeAmount {
         self.value
     }
     /// Returns the memo that was attached to the output, if any. This will only be `None`

zcash_client_backend/src/data_api/error.rs

@@ -4,13 +4,11 @@ use std::error;
 use std::fmt::{self, Debug, Display};
 
 use shardtree::error::ShardTreeError;
+use zcash_primitives::transaction::components::amount::NonNegativeAmount;
 use zcash_primitives::{
     transaction::{
         builder,
-        components::{
-            amount::{Amount, BalanceError},
-            sapling, transparent,
-        },
+        components::{amount::BalanceError, sapling, transparent},
     },
     zip32::AccountId,
 };
@@ -45,7 +43,10 @@ pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError> {
     BalanceError(BalanceError),
 
     /// Unable to create a new spend because the wallet balance is not sufficient.
-    InsufficientFunds { available: Amount, required: Amount },
+    InsufficientFunds {
+        available: NonNegativeAmount,
+        required: NonNegativeAmount,
+    },
 
     /// The wallet must first perform a scan of the blockchain before other
     /// operations can be performed.
@@ -110,8 +111,8 @@ where
             Error::InsufficientFunds { available, required } => write!(
                 f,
                 "Insufficient balance (have {}, need {} including fee)",
-                i64::from(*available),
-                i64::from(*required)
+                u64::from(*available),
+                u64::from(*required)
             ),
             Error::ScanRequired => write!(f, "Must scan blocks first"),
             Error::Builder(e) => write!(f, "An error occurred building the transaction: {}", e),

zcash_client_backend/src/data_api/wallet.rs

@@ -216,7 +216,7 @@ where
 {
     let req = zip321::TransactionRequest::new(vec![Payment {
         recipient_address: to.clone(),
-        amount: amount.into(),
+        amount,
         memo,
         label: None,
         message: None,
@@ -596,15 +596,15 @@ where
                 builder.add_sapling_output(
                     internal_ovk(),
                     dfvk.change_address().1,
-                    value.into(),
+                    *value,
                     memo.clone(),
                 )?;
                 sapling_output_meta.push((
                     Recipient::InternalAccount(
                         account,
                         PoolType::Shielded(ShieldedProtocol::Sapling),
                     ),
-                    value.into(),
+                    *value,
                     Some(memo),
                 ))
             }

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -10,7 +10,7 @@ use zcash_primitives::{
     legacy::TransparentAddress,
     transaction::{
         components::{
-            amount::{Amount, BalanceError, NonNegativeAmount},
+            amount::{BalanceError, NonNegativeAmount},
             sapling::fees as sapling,
             OutPoint, TxOut,
         },
@@ -35,7 +35,10 @@ pub enum InputSelectorError<DbErrT, SelectorErrT> {
     Selection(SelectorErrT),
     /// Insufficient funds were available to satisfy the payment request that inputs were being
     /// selected to attempt to satisfy.
-    InsufficientFunds { available: Amount, required: Amount },
+    InsufficientFunds {
+        available: NonNegativeAmount,
+        required: NonNegativeAmount,
+    },
     /// The data source does not have enough information to choose an expiry height
     /// for the transaction.
     SyncRequired,
@@ -60,8 +63,8 @@ impl<DE: fmt::Display, SE: fmt::Display> fmt::Display for InputSelectorError<DE,
             } => write!(
                 f,
                 "Insufficient balance (have {}, need {} including fee)",
-                i64::from(*available),
-                i64::from(*required)
+                u64::from(*available),
+                u64::from(*required)
             ),
             InputSelectorError::SyncRequired => {
                 write!(f, "Insufficient chain data is available, sync required.")
@@ -270,17 +273,17 @@ impl<DbErrT, ChangeStrategyErrT, NoteRefT> From<BalanceError>
     }
 }
 
-pub(crate) struct SaplingPayment(Amount);
+pub(crate) struct SaplingPayment(NonNegativeAmount);
 
 #[cfg(test)]
 impl SaplingPayment {
-    pub(crate) fn new(amount: Amount) -> Self {
+    pub(crate) fn new(amount: NonNegativeAmount) -> Self {
         SaplingPayment(amount)
     }
 }
 
 impl sapling::OutputView for SaplingPayment {
-    fn value(&self) -> Amount {
+    fn value(&self) -> NonNegativeAmount {
         self.0
     }
 }
@@ -338,10 +341,7 @@ where
 
         let mut transparent_outputs = vec![];
         let mut sapling_outputs = vec![];
-        let mut output_total = Amount::zero();
         for payment in transaction_request.payments() {
-            output_total = (output_total + payment.amount).ok_or(BalanceError::Overflow)?;
-
             let mut push_transparent = |taddr: TransparentAddress| {
                 transparent_outputs.push(TxOut {
                     value: payment.amount,
@@ -374,8 +374,8 @@ where
         }
 
         let mut sapling_inputs: Vec<ReceivedSaplingNote<DbT::NoteRef>> = vec![];
-        let mut prior_available = Amount::zero();
-        let mut amount_required = Amount::zero();
+        let mut prior_available = NonNegativeAmount::ZERO;
+        let mut amount_required = NonNegativeAmount::ZERO;
         let mut exclude: Vec<DbT::NoteRef> = vec![];
         // This loop is guaranteed to terminate because on each iteration we check that the amount
         // of funds selected is strictly increasing. The loop will either return a successful
@@ -414,13 +414,18 @@ where
             }
 
             sapling_inputs = wallet_db
-                .select_spendable_sapling_notes(account, amount_required, anchor_height, &exclude)
+                .select_spendable_sapling_notes(
+                    account,
+                    amount_required.into(),
+                    anchor_height,
+                    &exclude,
+                )
                 .map_err(InputSelectorError::DataSource)?;
 
             let new_available = sapling_inputs
                 .iter()
                 .map(|n| n.value())
-                .sum::<Option<Amount>>()
+                .sum::<Option<NonNegativeAmount>>()
                 .ok_or(BalanceError::Overflow)?;
 
             if new_available <= prior_available {
@@ -506,8 +511,8 @@ where
             })
         } else {
             Err(InputSelectorError::InsufficientFunds {
-                available: balance.total().into(),
-                required: shielding_threshold.into(),
+                available: balance.total(),
+                required: shielding_threshold,
             })
         }
     }

zcash_client_backend/src/fees.rs

@@ -5,7 +5,7 @@ use zcash_primitives::{
     memo::MemoBytes,
     transaction::{
         components::{
-            amount::{Amount, BalanceError, NonNegativeAmount},
+            amount::{BalanceError, NonNegativeAmount},
             sapling::fees as sapling,
             transparent::fees as transparent,
             OutPoint,
@@ -100,10 +100,10 @@ pub enum ChangeError<E, NoteRefT> {
     /// required outputs and fees.
     InsufficientFunds {
         /// The total of the inputs provided to change selection
-        available: Amount,
+        available: NonNegativeAmount,
         /// The total amount of input value required to fund the requested outputs,
         /// including the required fees.
-        required: Amount,
+        required: NonNegativeAmount,
     },
     /// Some of the inputs provided to the transaction were determined to currently have no
     /// economic value (i.e. their inclusion in a transaction causes fees to rise in an amount
@@ -127,8 +127,8 @@ impl<CE: fmt::Display, N: fmt::Display> fmt::Display for ChangeError<CE, N> {
             } => write!(
                 f,
                 "Insufficient funds: required {} zatoshis, but only {} zatoshis were available.",
-                i64::from(required),
-                i64::from(available)
+                u64::from(*required),
+                u64::from(*available)
             ),
             ChangeError::DustInputs {
                 transparent,
@@ -235,7 +235,7 @@ pub trait ChangeStrategy {
 #[cfg(test)]
 pub(crate) mod tests {
     use zcash_primitives::transaction::components::{
-        amount::Amount,
+        amount::NonNegativeAmount,
         sapling::fees as sapling,
         transparent::{fees as transparent, OutPoint, TxOut},
     };
@@ -257,14 +257,14 @@ pub(crate) mod tests {
 
     pub(crate) struct TestSaplingInput {
         pub note_id: u32,
-        pub value: Amount,
+        pub value: NonNegativeAmount,
     }
 
     impl sapling::InputView<u32> for TestSaplingInput {
         fn note_id(&self) -> &u32 {
             &self.note_id
         }
-        fn value(&self) -> Amount {
+        fn value(&self) -> NonNegativeAmount {
             self.value
         }
     }