refactor token transfer and add more tests

.changelog/unreleased/improvements/3736-refactor-token-transfer.md

@@ -0,0 +1,2 @@
+- Refactored token transfer functions and fixed checks for no-op conditions.
+  ([\#3736](https://github.com/anoma/namada/pull/3736))

crates/core/src/address.rs

@@ -779,6 +779,12 @@ pub mod testing {
             .expect("The token address decoding shouldn't fail")
     }
 
+    /// A sampled established address for tests
+    pub fn established_address_5() -> Address {
+        Address::decode("tnam1qyftuue8fq25ezm0s8vj75d3qz759r2225ug7hll")
+            .expect("The token address decoding shouldn't fail")
+    }
+
     /// Generate an arbitrary [`Address`] (established or implicit).
     pub fn arb_non_internal_address() -> impl Strategy<Value = Address> {
         prop_oneof![

crates/core/src/masp.rs

@@ -10,7 +10,9 @@ use borsh_ext::BorshSerializeExt;
 use masp_primitives::asset_type::AssetType;
 use masp_primitives::sapling::ViewingKey;
 use masp_primitives::transaction::TransparentAddress;
-pub use masp_primitives::transaction::TxId as TxIdInner;
+pub use masp_primitives::transaction::{
+    Transaction as MaspTransaction, TxId as TxIdInner,
+};
 use namada_macros::BorshDeserializer;
 #[cfg(feature = "migrations")]
 use namada_migrations::*;

crates/core/src/uint.rs

@@ -1136,8 +1136,9 @@ impl PartialOrd for I320 {
     }
 }
 
-#[cfg(any(test, feature = "testing"))]
 /// Testing helpers
+#[cfg(any(test, feature = "testing"))]
+#[allow(clippy::arithmetic_side_effects)]
 pub mod testing {
     use super::*;
 
@@ -1166,6 +1167,12 @@ pub mod testing {
         }
     }
 
+    impl std::ops::SubAssign<I256> for I256 {
+        fn sub_assign(&mut self, rhs: I256) {
+            *self = *self - rhs;
+        }
+    }
+
     impl std::ops::Mul<I256> for I256 {
         type Output = Self;
 

crates/shielded_token/src/lib.rs

@@ -29,10 +29,9 @@ pub mod vp;
 
 use std::str::FromStr;
 
-pub use masp_primitives::transaction::Transaction as MaspTransaction;
 use namada_core::borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
 pub use namada_core::dec::Dec;
-pub use namada_core::masp::{MaspEpoch, MaspTxId, MaspValue};
+pub use namada_core::masp::{MaspEpoch, MaspTransaction, MaspTxId, MaspValue};
 pub use namada_state::{
     ConversionLeaf, ConversionState, Error, Key, OptionExt, Result, ResultExt,
     StorageRead, StorageWrite, WithConversionState,

crates/tests/src/vm_host_env/tx.rs

@@ -528,6 +528,10 @@ mod native_tx_host_env {
         public_keys_map_len: u64,
         threshold: u8,
     ) -> i64);
+    native_host_fn!(tx_update_masp_note_commitment_tree(
+        transaction_ptr: u64,
+        transaction_len: u64,
+    ) -> i64);
     native_host_fn!(tx_yield_value(
         buf_ptr: u64,
         buf_len: u64,

crates/token/Cargo.toml

@@ -30,8 +30,11 @@ namada_events = { path = "../events", default-features = false }
 namada_macros = { path = "../macros" }
 namada_migrations = { path = "../migrations", optional = true }
 namada_shielded_token = { path = "../shielded_token" }
+namada_storage = { path = "../storage" }
 namada_systems = { path = "../systems" }
 namada_trans_token = { path = "../trans_token" }
+namada_tx = { path = "../tx" }
+namada_tx_env = { path = "../tx_env" }
 
 arbitrary = { workspace = true, optional = true }
 borsh.workspace = true
@@ -42,6 +45,7 @@ serde.workspace = true
 [dev-dependencies]
 namada_core = { path = "../core", features = ["testing"] }
 namada_shielded_token = { path = "../shielded_token", features = ["testing"] }
+namada_tests = { path = "../tests" }
 
 masp_primitives.workspace = true
 

crates/token/src/lib.rs

@@ -30,6 +30,8 @@ pub use namada_shielded_token::*;
 use namada_systems::parameters;
 pub use namada_trans_token::*;
 
+pub mod tx;
+
 /// Validity predicates
 pub mod vp {
     pub use namada_shielded_token::vp::MaspVp;
@@ -38,6 +40,7 @@ pub mod vp {
     pub use namada_shielded_token::{Error, Result};
     pub use namada_trans_token::vp::MultitokenVp;
 }
+
 use serde::{Deserialize, Serialize};
 
 /// Token storage keys
@@ -172,6 +175,15 @@ pub struct Transfer {
     pub shielded_section_hash: Option<MaspTxId>,
 }
 
+/// References to the transparent sections of a [`Transfer`].
+#[derive(Debug, Clone)]
+pub struct TransparentTransfersRef<'a> {
+    /// Sources of this transfer
+    pub sources: &'a BTreeMap<Account, DenominatedAmount>,
+    /// Targets of this transfer
+    pub targets: &'a BTreeMap<Account, DenominatedAmount>,
+}
+
 impl Transfer {
     /// Create a MASP transaction
     pub fn masp(hash: MaspTxId) -> Self {
@@ -264,6 +276,46 @@ impl Transfer {
         self.debit(source, token.clone(), amount)?
             .credit(target, token, amount)
     }
+
+    /// Get references to the transparent sections.
+    pub fn transparent_part(&self) -> Option<TransparentTransfersRef<'_>> {
+        if self.sources.is_empty() && self.targets.is_empty() {
+            None
+        } else {
+            Some(TransparentTransfersRef {
+                sources: &self.sources,
+                targets: &self.targets,
+            })
+        }
+    }
+}
+
+impl TransparentTransfersRef<'_> {
+    /// Construct pairs of source address and token with a debited amount
+    pub fn sources(&self) -> BTreeMap<(Address, Address), Amount> {
+        self.sources
+            .iter()
+            .map(|(account, amount)| {
+                (
+                    (account.owner.clone(), account.token.clone()),
+                    amount.amount(),
+                )
+            })
+            .collect::<BTreeMap<_, _>>()
+    }
+
+    /// Construct pairs of target address and token with a credited amount
+    pub fn targets(&self) -> BTreeMap<(Address, Address), Amount> {
+        self.targets
+            .iter()
+            .map(|(account, amount)| {
+                (
+                    (account.owner.clone(), account.token.clone()),
+                    amount.amount(),
+                )
+            })
+            .collect::<BTreeMap<_, _>>()
+    }
 }
 
 #[cfg(all(any(test, feature = "testing"), feature = "masp"))]