Extend SignedAmount to hold MASP value balance.

anoma · May 8, 2024 · 06f4da1 · 06f4da1
1 parent 0380f7c
commit 06f4da1
Show file tree

Hide file tree

Showing 3 changed files with 212 additions and 83 deletions.
diff --git a/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs b/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs
@@ -58,11 +58,8 @@ struct AmountDelta {
 impl AmountDelta {
     /// Resolve the updated amount by applying the delta value.
     #[inline]
-    fn resolve(self) -> Amount {
-        match self.delta {
-            SignedAmount::Positive(delta) => self.base + delta,
-            SignedAmount::Negative(delta) => self.base - delta,
-        }
+    fn resolve(self) -> SignedAmount {
+        self.delta + SignedAmount::from(self.base)
     }
 }
 
@@ -108,11 +105,7 @@ where
             })?;
         Some(AmountDelta {
             base: before,
-            delta: if before > after {
-                SignedAmount::Negative(before - after)
-            } else {
-                SignedAmount::Positive(after - before)
-            },
+            delta: SignedAmount::from(after) - SignedAmount::from(before),
         })
     }
 
@@ -148,40 +141,22 @@ where
         match (debit, credit) {
             // success case
             (
-                Some(AmountDelta {
-                    delta: SignedAmount::Negative(debit),
-                    ..
-                }),
-                Some(
-                    escrow_balance @ AmountDelta {
-                        delta: SignedAmount::Positive(credit),
-                        ..
-                    },
-                ),
-            ) => Ok((debit == expected_debit && credit == expected_credit)
-                .then_some(escrow_balance)),
+                Some(AmountDelta { delta: debit, .. }),
+                Some(escrow_balance @ AmountDelta { delta: credit, .. }),
+            ) if !debit.is_positive() && !credit.is_negative() => Ok((debit
+                == -SignedAmount::from(expected_debit)
+                && credit == SignedAmount::from(expected_credit))
+            .then_some(escrow_balance)),
             // user did not debit from their account
-            (
-                Some(AmountDelta {
-                    delta: SignedAmount::Positive(_),
-                    ..
-                }),
-                _,
-            ) => {
+            (Some(AmountDelta { delta, .. }), _) if !delta.is_negative() => {
                 tracing::debug!(
                     "The account {} was not debited.",
                     payer_account
                 );
                 Ok(None)
             }
             // user did not credit escrow account
-            (
-                _,
-                Some(AmountDelta {
-                    delta: SignedAmount::Negative(_),
-                    ..
-                }),
-            ) => {
+            (_, Some(AmountDelta { delta, .. })) if !delta.is_positive() => {
                 tracing::debug!(
                     "The Ethereum bridge pool's escrow was not credited from \
                      account {}.",
@@ -191,13 +166,11 @@ where
             }
             // some other error occurred while calculating
             // balance deltas
-            (None, _) | (_, None) => {
-                Err(native_vp::Error::AllocMessage(format!(
-                    "Could not calculate the balance delta for {}",
-                    payer_account
-                ))
-                .into())
-            }
+            _ => Err(native_vp::Error::AllocMessage(format!(
+                "Could not calculate the balance delta for {}",
+                payer_account
+            ))
+            .into()),
         }
     }
 
@@ -290,7 +263,7 @@ where
                 None => return Ok(false),
             };
 
-        let wnam_cap = {
+        let wnam_cap: Amount = {
             let key = whitelist::Key {
                 asset: wnam_address,
                 suffix: whitelist::KeyType::Cap,
@@ -301,7 +274,7 @@ where
                 .map_err(Error)?
                 .unwrap_or_default()
         };
-        if escrowed_balance > wnam_cap {
+        if escrowed_balance > SignedAmount::from(wnam_cap) {
             tracing::debug!(
                 ?transfer,
                 escrowed_nam = %escrowed_balance.to_string_native(),

diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs
@@ -38,7 +38,7 @@ use token::storage_key::{
 use token::Amount;
 
 use crate::ledger::native_vp;
-use crate::ledger::native_vp::{Ctx, NativeVp};
+use crate::ledger::native_vp::{Ctx, NativeVp, SignedAmount};
 use crate::token;
 use crate::token::MaspDigitPos;
 use crate::vm::WasmCacheAccess;
@@ -557,38 +557,46 @@ fn verify_sapling_balancing_value(
     tokens: &BTreeMap<AssetType, (Address, token::Denomination, MaspDigitPos)>,
     conversion_state: &ConversionState,
 ) -> Result<()> {
-    let mut acc = pre.clone();
+    let mut acc = ValueSum::<Address, SignedAmount>::from_sum(pre.clone());
     for (asset_type, val) in sapling_value_balance.components() {
         // Only assets with at most the target timestamp count
         match conversion_state.assets.get(asset_type) {
             Some(((address, _, digit), asset_epoch, _, _))
                 if *asset_epoch <= target_epoch =>
             {
-                let decoded_change = token::Amount::from_masp_denominated(
-                    val.unsigned_abs() as u64,
-                    *digit,
-                );
+                let decoded_change = SignedAmount::from_masp_denominated(
+                    *val, *digit,
+                )
+                .map_err(|_| {
+                    Error::NativeVpError(native_vp::Error::SimpleMessage(
+                        "Overflow in MASP value balance",
+                    ))
+                })?;
                 let decoded_change =
                     ValueSum::from_pair(address.clone(), decoded_change);
-                if *val < 0 {
-                    acc += decoded_change;
-                } else {
-                    acc -= decoded_change;
-                }
+                acc = acc.checked_sub(&decoded_change).ok_or_else(|| {
+                    Error::NativeVpError(native_vp::Error::SimpleMessage(
+                        "Overflow in MASP value balance",
+                    ))
+                })?;
             }
             None if tokens.contains_key(asset_type) => {
                 let (token, _denom, digit) = &tokens[asset_type];
-                let decoded_change = token::Amount::from_masp_denominated(
-                    val.unsigned_abs() as u64,
-                    *digit,
-                );
+                let decoded_change = SignedAmount::from_masp_denominated(
+                    *val, *digit,
+                )
+                .map_err(|_| {
+                    Error::NativeVpError(native_vp::Error::SimpleMessage(
+                        "Overflow in MASP value balance",
+                    ))
+                })?;
                 let decoded_change =
                     ValueSum::from_pair(token.clone(), decoded_change);
-                if *val < 0 {
-                    acc += decoded_change;
-                } else {
-                    acc -= decoded_change;
-                }
+                acc = acc.checked_sub(&decoded_change).ok_or_else(|| {
+                    Error::NativeVpError(native_vp::Error::SimpleMessage(
+                        "Overflow in MASP value balance",
+                    ))
+                })?;
             }
             _ => {
                 let error =
@@ -600,7 +608,7 @@ fn verify_sapling_balancing_value(
             }
         }
     }
-    if acc == *post {
+    if acc == ValueSum::from_sum(post.clone()) {
         Ok(())
     } else {
         let error = Error::NativeVpError(native_vp::Error::SimpleMessage(