merge queue: embarking main (0960277) and #4003 together

.changelog/unreleased/improvements/4003-better-masp-balance-errors.md

@@ -0,0 +1,2 @@
+- Improve MASP insufficient balance errors.
+  ([\#4003](https://github.com/anoma/namada/pull/4003))

crates/sdk/src/tx.rs

@@ -17,7 +17,7 @@ use masp_primitives::transaction::components::transparent::fees::{
     InputView as TransparentInputView, OutputView as TransparentOutputView,
 };
 use masp_primitives::transaction::components::I128Sum;
-use masp_primitives::transaction::{builder, Transaction as MaspTransaction};
+use masp_primitives::transaction::Transaction as MaspTransaction;
 use namada_account::{InitAccount, UpdateAccount};
 use namada_core::address::{Address, IBC, MASP};
 use namada_core::arith::checked;
@@ -61,10 +61,7 @@ use namada_proof_of_stake::parameters::{
 use namada_proof_of_stake::types::{CommissionPair, ValidatorState};
 use namada_token as token;
 use namada_token::masp::shielded_wallet::ShieldedApi;
-use namada_token::masp::TransferErr::Build;
-use namada_token::masp::{
-    MaspDataLog, MaspFeeData, MaspTransferData, ShieldedTransfer,
-};
+use namada_token::masp::{MaspFeeData, MaspTransferData, ShieldedTransfer};
 use namada_token::storage_key::balance_key;
 use namada_token::DenominatedAmount;
 use namada_tx::data::pgf::UpdateStewardCommission;
@@ -3469,37 +3466,11 @@ async fn construct_shielded_parts<N: Namada>(
     let shielded_parts = match stx_result {
         Ok(Some(stx)) => stx,
         Ok(None) => return Ok(None),
-        Err(Build {
-            error: builder::Error::InsufficientFunds(_),
-            data,
-        }) => {
-            if let Some(MaspDataLog {
-                source,
-                token,
-                amount,
-            }) = data
-            {
-                if let Some(source) = source {
-                    return Err(TxSubmitError::NegativeBalanceAfterTransfer(
-                        Box::new(source.effective_address()),
-                        amount.to_string(),
-                        Box::new(token.clone()),
-                    )
-                    .into());
-                }
-                return Err(TxSubmitError::MaspError(format!(
-                    "Insufficient funds: Could not collect enough funds to \
-                     pay for fees: token {token}, amount: {amount}"
-                ))
-                .into());
-            }
-            return Err(TxSubmitError::MaspError(
-                "Insufficient funds".to_string(),
-            )
-            .into());
-        }
         Err(err) => {
-            return Err(TxSubmitError::MaspError(err.to_string()).into());
+            return Err(TxSubmitError::MaspError(format!(
+                "Failed to construct MASP transaction shielded parts: {err}"
+            ))
+            .into());
         }
     };
 

crates/shielded_token/src/masp.rs

@@ -8,7 +8,7 @@ pub mod shielded_wallet;
 mod test_utils;
 
 use std::collections::BTreeMap;
-use std::fmt::Debug;
+use std::fmt::{self, Debug};
 
 use borsh::{BorshDeserialize, BorshSerialize};
 use masp_primitives::asset_type::AssetType;
@@ -110,13 +110,52 @@ pub struct MaspTargetTransferData {
     token: Address,
 }
 
-/// Data to log masp transactions' errors
-#[allow(missing_docs)]
+/// Data to log the error of a single masp transaction
 #[derive(Debug)]
-pub struct MaspDataLog {
-    pub source: Option<TransferSource>,
+pub struct MaspDataLogEntry {
+    /// Token to be spent.
     pub token: Address,
-    pub amount: token::DenominatedAmount,
+    /// How many tokens are missing.
+    pub shortfall: token::DenominatedAmount,
+}
+
+impl fmt::Display for MaspDataLogEntry {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let Self { token, shortfall } = self;
+        write!(f, "{shortfall} {token} missing")
+    }
+}
+
+/// Data to log the error of a batch of masp transactions
+#[derive(Debug)]
+pub struct MaspDataLog {
+    /// The error batch
+    pub batch: Vec<MaspDataLogEntry>,
+}
+
+impl From<Vec<MaspDataLogEntry>> for MaspDataLog {
+    #[inline]
+    fn from(batch: Vec<MaspDataLogEntry>) -> Self {
+        Self { batch }
+    }
+}
+
+impl fmt::Display for MaspDataLog {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let Self { batch } = self;
+
+        if let Some(err) = batch.first() {
+            write!(f, "{err}")?;
+        } else {
+            return Ok(());
+        }
+
+        for err in &batch[1..] {
+            write!(f, ", {err}")?;
+        }
+
+        Ok(())
+    }
 }
 
 #[allow(missing_docs)]
@@ -143,14 +182,15 @@ pub struct MaspTokenRewardData {
 #[derive(Error, Debug)]
 pub enum TransferErr {
     /// Build error for masp errors
-    #[error("{error}")]
+    #[error("Transaction builder error: {error}")]
     Build {
-        /// The error
+        /// Builder error returned from the masp library
         error: builder::Error<std::convert::Infallible>,
-        /// The optional associated transfer data for logging purposes
-        data: Option<MaspDataLog>,
     },
-    /// errors
+    /// Insufficient funds error
+    #[error("Insufficient funds: {0}")]
+    InsufficientFunds(MaspDataLog),
+    /// Generic error
     #[error("{0}")]
     General(String),
 }