diff --git a/.changelog/unreleased/improvements/3736-refactor-token-transfer.md b/.changelog/unreleased/improvements/3736-refactor-token-transfer.md new file mode 100644 index 00000000000..af4ee62a070 --- /dev/null +++ b/.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)) \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index ffd306a1565..7856bb9d4c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5545,8 +5545,12 @@ dependencies = [ "namada_macros", "namada_migrations", "namada_shielded_token", + "namada_storage", "namada_systems", + "namada_tests", "namada_trans_token", + "namada_tx", + "namada_tx_env", "proptest", "serde", ] @@ -5556,6 +5560,7 @@ name = "namada_trans_token" version = "0.43.0" dependencies = [ "assert_matches", + "itertools 0.12.1", "konst", "linkme", "namada_core", @@ -5566,10 +5571,13 @@ dependencies = [ "namada_parameters", "namada_state", "namada_systems", + "namada_tests", "namada_tx", + "namada_tx_env", "namada_vm", "namada_vp", "namada_vp_env", + "proptest", "thiserror", "tracing", ] diff --git a/crates/core/src/address.rs b/crates/core/src/address.rs index 38bd4398467..a0a64e8b8f2 100644 --- a/crates/core/src/address.rs +++ b/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 { prop_oneof![ diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index ce9d7967c91..28d1ee5b615 100644 --- a/crates/core/src/masp.rs +++ b/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::*; diff --git a/crates/core/src/uint.rs b/crates/core/src/uint.rs index 18224342a37..aa260a52f39 100644 --- a/crates/core/src/uint.rs +++ b/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 for I256 { + fn sub_assign(&mut self, rhs: I256) { + *self = *self - rhs; + } + } + impl std::ops::Mul for I256 { type Output = Self; diff --git a/crates/shielded_token/src/lib.rs b/crates/shielded_token/src/lib.rs index c4512a3035d..cf9f837961e 100644 --- a/crates/shielded_token/src/lib.rs +++ b/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, diff --git a/crates/tests/src/vm_host_env/tx.rs b/crates/tests/src/vm_host_env/tx.rs index 5e39ead354c..bfdbe3f710f 100644 --- a/crates/tests/src/vm_host_env/tx.rs +++ b/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, diff --git a/crates/token/Cargo.toml b/crates/token/Cargo.toml index 827da0c9e7c..bb5cd91f74f 100644 --- a/crates/token/Cargo.toml +++ b/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 diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index dff43909bfd..17b5fdc0a25 100644 --- a/crates/token/src/lib.rs +++ b/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, } +/// References to the transparent sections of a [`Transfer`]. +#[derive(Debug, Clone)] +pub struct TransparentTransfersRef<'a> { + /// Sources of this transfer + pub sources: &'a BTreeMap, + /// Targets of this transfer + pub targets: &'a BTreeMap, +} + 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> { + 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::>() + } + + /// 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::>() + } } #[cfg(all(any(test, feature = "testing"), feature = "masp"))] diff --git a/crates/token/src/tx.rs b/crates/token/src/tx.rs new file mode 100644 index 00000000000..38e8fa81c84 --- /dev/null +++ b/crates/token/src/tx.rs @@ -0,0 +1,289 @@ +//! Token transaction + +use std::borrow::Cow; +use std::collections::BTreeSet; + +use namada_core::collections::HashSet; +use namada_core::masp; +use namada_events::EmitEvents; +use namada_shielded_token::{utils, MaspTxId}; +use namada_storage::{Error, OptionExt, ResultExt}; +pub use namada_trans_token::tx::transfer; +use namada_tx::action::{self, Action, MaspAction}; +use namada_tx::BatchedTx; +use namada_tx_env::{Address, Result, TxEnv}; + +use crate::{Transfer, TransparentTransfersRef}; + +/// Transparent and shielded token transfers that can be used in a transaction. +pub fn multi_transfer( + env: &mut ENV, + transfers: Transfer, + tx_data: &BatchedTx, + event_desc: Cow<'static, str>, +) -> Result<()> +where + ENV: TxEnv + EmitEvents + action::Write, +{ + // Effect the transparent multi transfer(s) + let debited_accounts = + if let Some(transparent) = transfers.transparent_part() { + apply_transparent_transfers(env, transparent, event_desc) + .wrap_err("Transparent token transfer failed")? + } else { + HashSet::new() + }; + + // Apply the shielded transfer if there is a link to one + if let Some(masp_section_ref) = transfers.shielded_section_hash { + apply_shielded_transfer( + env, + masp_section_ref, + debited_accounts, + tx_data, + ) + .wrap_err("Shielded token transfer failed")?; + } + Ok(()) +} + +/// Transfer tokens from `sources` to `targets` and submit a transfer event. +/// +/// Returns an `Err` if any source has insufficient balance or if the transfer +/// to any destination would overflow (This can only happen if the total supply +/// doesn't fit in `token::Amount`). Returns a set of debited accounts. +pub fn apply_transparent_transfers( + env: &mut ENV, + transfers: TransparentTransfersRef<'_>, + event_desc: Cow<'static, str>, +) -> Result> +where + ENV: TxEnv + EmitEvents, +{ + let sources = transfers.sources(); + let targets = transfers.targets(); + let debited_accounts = namada_trans_token::tx::multi_transfer( + env, sources, targets, event_desc, + )?; + Ok(debited_accounts) +} + +/// Apply a shielded transfer +pub fn apply_shielded_transfer( + env: &mut ENV, + masp_section_ref: MaspTxId, + debited_accounts: HashSet
, + tx_data: &BatchedTx, +) -> Result<()> +where + ENV: TxEnv + EmitEvents + action::Write, +{ + let shielded = tx_data + .tx + .get_masp_section(&masp_section_ref) + .cloned() + .ok_or_err_msg("Unable to find required shielded section in tx data") + .inspect_err(|_err| { + env.set_commitment_sentinel(); + })?; + utils::handle_masp_tx(env, &shielded) + .wrap_err("Encountered error while handling MASP transaction")?; + ENV::update_masp_note_commitment_tree(&shielded) + .wrap_err("Failed to update the MASP commitment tree")?; + + env.push_action(Action::Masp(MaspAction::MaspSectionRef( + masp_section_ref, + )))?; + // Extract the debited accounts for the masp part of the transfer and + // push the relative actions + let vin_addresses = + shielded + .transparent_bundle() + .map_or_else(Default::default, |bndl| { + bndl.vin + .iter() + .map(|vin| vin.address) + .collect::>() + }); + let masp_authorizers: Vec<_> = debited_accounts + .into_iter() + .filter(|account| { + vin_addresses.contains(&masp::addr_taddr(account.clone())) + }) + .collect(); + if masp_authorizers.len() != vin_addresses.len() { + return Err(Error::SimpleMessage( + "Transfer transaction does not debit all the expected accounts", + )); + } + + for authorizer in masp_authorizers { + env.push_action(Action::Masp(MaspAction::MaspAuthorizer(authorizer)))?; + } + + Ok(()) +} + +#[cfg(test)] +#[allow(clippy::arithmetic_side_effects, clippy::disallowed_types)] +mod test { + use std::collections::HashMap; + + use namada_core::address::testing::{ + arb_address, arb_non_internal_address, + }; + use namada_core::token; + use namada_tests::tx::{ctx, tx_host_env}; + use namada_trans_token::testing::arb_amount; + use namada_trans_token::{read_balance, Amount, DenominatedAmount}; + use namada_tx::{Tx, TxCommitments}; + use proptest::prelude::*; + + use super::*; + + const EVENT_DESC: Cow<'static, str> = Cow::Borrowed("event-desc"); + + proptest! { + #[test] + fn test_valid_trans_multi_transfer_tx( + transfers in prop::collection::vec(arb_trans_transfer(), 1..10) + ) { + test_valid_trans_multi_transfer_tx_aux(transfers) + } + } + + #[derive(Debug)] + struct SingleTransfer { + src: Address, + dest: Address, + token: Address, + amount: Amount, + } + + fn arb_trans_transfer() -> impl Strategy { + (( + arb_non_internal_address(), + arb_non_internal_address(), + arb_address(), + arb_amount(), + ) + .prop_filter( + "unique addresses", + |(src, dest, token, _amount)| { + src != dest && dest != token && src != token + }, + )) + .prop_map(|(src, dest, token, amount)| SingleTransfer { + src, + dest, + token, + amount, + }) + } + + fn test_valid_trans_multi_transfer_tx_aux(transfers: Vec) { + tx_host_env::init(); + + let mut genesis_balances = HashMap::< + // Token address + Address, + HashMap< + // Owner address + Address, + token::Amount, + >, + >::new(); + + for SingleTransfer { + src, + dest, + token, + amount, + } in &transfers + { + tx_host_env::with(|tx_env| { + tx_env.spawn_accounts([src, dest, token]); + tx_env.credit_tokens(src, token, *amount); + }); + // Store the credited token balances + *genesis_balances + .entry(token.clone()) + .or_default() + .entry(src.clone()) + .or_default() += *amount; + } + + let mut transfer = Transfer::default(); + for SingleTransfer { + src, + dest, + token, + amount, + } in &transfers + { + let denom = DenominatedAmount::native(*amount); + transfer = transfer + .transfer(src.clone(), dest.clone(), token.clone(), denom) + .unwrap(); + } + + let tx_data = BatchedTx { + tx: Tx::default(), + cmt: TxCommitments::default(), + }; + multi_transfer(ctx(), transfer, &tx_data, EVENT_DESC).unwrap(); + + let mut changes = HashMap::< + // Token address + Address, + HashMap< + // Owner address + Address, + token::Change, + >, + >::new(); + + for SingleTransfer { + src, + dest, + token, + amount, + } in &transfers + { + // Accumulate all token changes + let token_changes = changes.entry(token.clone()).or_default(); + let change = token::Change::from(*amount); + *token_changes.entry(src.clone()).or_default() -= change; + *token_changes.entry(dest.clone()).or_default() += change; + + // Every address has to be in the verifier set + tx_host_env::with(|tx_env| { + // Internal token address have to be part of the verifier set + assert!( + !token.is_internal() || tx_env.verifiers.contains(token) + ); + assert!(tx_env.verifiers.contains(src)); + assert!(tx_env.verifiers.contains(dest)); + }) + } + + // Check all the changed balances + for (token, changes) in changes { + for (owner, change) in changes { + let expected_balance = token::Change::from( + genesis_balances + .get(&token) + .and_then(|balances| balances.get(&owner)) + .cloned() + .unwrap_or_default(), + ) + change; + assert_eq!( + token::Change::from( + read_balance(ctx(), &token, &owner).unwrap() + ), + expected_balance + ); + } + } + } +} diff --git a/crates/trans_token/Cargo.toml b/crates/trans_token/Cargo.toml index acb178536ed..3991b3de2f8 100644 --- a/crates/trans_token/Cargo.toml +++ b/crates/trans_token/Cargo.toml @@ -24,6 +24,7 @@ namada_events = { path = "../events", default-features = false } namada_state = { path = "../state" } namada_systems = { path = "../systems" } namada_tx = { path = "../tx" } +namada_tx_env = { path = "../tx_env" } namada_vp_env = { path = "../vp_env" } konst.workspace = true @@ -38,11 +39,14 @@ namada_governance = { path = "../governance", features = ["testing"] } namada_ibc = { path = "../ibc", features = ["testing"] } namada_parameters = { path = "../parameters", features = ["testing"] } namada_state = { path = "../state", features = ["testing"] } +namada_tests = { path = "../tests" } namada_tx = { path = "../tx", features = ["testing"] } namada_vm = { path = "../vm", features = ["testing"] } namada_vp = { path = "../vp" } assert_matches.workspace = true +itertools.workspace = true +proptest.workspace = true [lints.rust] unexpected_cfgs = { level = "warn", check-cfg = ['cfg(fuzzing)'] } diff --git a/crates/trans_token/src/lib.rs b/crates/trans_token/src/lib.rs index 56513caee69..522a8b0ac59 100644 --- a/crates/trans_token/src/lib.rs +++ b/crates/trans_token/src/lib.rs @@ -20,6 +20,7 @@ pub mod event; mod storage; pub mod storage_key; +pub mod tx; pub mod vp; use std::collections::BTreeMap; diff --git a/crates/trans_token/src/storage.rs b/crates/trans_token/src/storage.rs index 51074bcc224..da3f6cb1927 100644 --- a/crates/trans_token/src/storage.rs +++ b/crates/trans_token/src/storage.rs @@ -1,7 +1,4 @@ -use std::collections::{BTreeMap, BTreeSet}; - use namada_core::address::{Address, InternalAddress}; -use namada_core::collections::HashSet; use namada_core::hints; pub use namada_core::storage::Key; use namada_core::token::{self, Amount, AmountError, DenominatedAmount}; @@ -219,11 +216,15 @@ where storage.write(&key, denom) } -/// Transfer `token` from `src` to `dest`. +/// Apply transfer of a `token` from `src` to `dest` in storage. /// /// Returns an `Err` if `src` has insufficient balance or if the transfer the /// `dest` would overflow (This can only happen if the total supply doesn't fit /// in `token::Amount`). +/// +/// For a regular token transfer in a transaction, use +/// [tx::transfer](crate::tx::transfer) instead that inserts a verifier expected +/// by the token VP and emits a transfer events. pub fn transfer( storage: &mut S, token: &Address, @@ -260,65 +261,6 @@ where } } -/// Transfer tokens from `sources` to `dests`. -/// -/// Returns an `Err` if any source has insufficient balance or if the transfer -/// to any destination would overflow (This can only happen if the total supply -/// doesn't fit in `token::Amount`). Returns the set of debited accounts. -pub fn multi_transfer( - storage: &mut S, - sources: &BTreeMap<(Address, Address), Amount>, - dests: &BTreeMap<(Address, Address), Amount>, -) -> Result> -where - S: StorageRead + StorageWrite, -{ - let mut debited_accounts = HashSet::new(); - // Collect all the accounts whose balance has changed - let mut accounts = BTreeSet::new(); - accounts.extend(sources.keys().cloned()); - accounts.extend(dests.keys().cloned()); - - let unexpected_err = || { - Error::new_const( - "Computing difference between amounts should never overflow", - ) - }; - // Apply the balance change for each account in turn - for ref account @ (ref owner, ref token) in accounts { - let overflow_err = || { - Error::new_alloc(format!( - "The transfer would overflow balance of {owner}" - )) - }; - let underflow_err = - || Error::new_alloc(format!("{owner} has insufficient balance")); - // Load account balances and deltas - let owner_key = balance_key(token, owner); - let owner_balance = read_balance(storage, token, owner)?; - let src_amt = sources.get(account).cloned().unwrap_or_default(); - let dest_amt = dests.get(account).cloned().unwrap_or_default(); - // Compute owner_balance + dest_amt - src_amt - let new_owner_balance = if src_amt <= dest_amt { - owner_balance - .checked_add( - dest_amt.checked_sub(src_amt).ok_or_else(unexpected_err)?, - ) - .ok_or_else(overflow_err)? - } else { - debited_accounts.insert(owner.to_owned()); - owner_balance - .checked_sub( - src_amt.checked_sub(dest_amt).ok_or_else(unexpected_err)?, - ) - .ok_or_else(underflow_err)? - }; - // Write the new balance - storage.write(&owner_key, new_owner_balance)?; - } - Ok(debited_accounts) -} - /// Mint `amount` of `token` as `minter` to `dest`. pub fn mint_tokens( storage: &mut S, @@ -423,53 +365,13 @@ pub fn denom_to_amount( #[cfg(test)] mod testing { - use std::collections::BTreeMap; - use namada_core::{address, token}; use namada_state::testing::TestStorage; use super::{ - burn_tokens, credit_tokens, multi_transfer, read_balance, - read_total_supply, transfer, + burn_tokens, credit_tokens, read_balance, read_total_supply, transfer, }; - #[test] - fn test_multi_transfer() { - let mut storage = TestStorage::default(); - let native_token = address::testing::nam(); - - // Get one account - let addr = address::testing::gen_implicit_address(); - - // Credit the account some balance - let pre_balance = token::Amount::native_whole(1); - credit_tokens(&mut storage, &native_token, &addr, pre_balance).unwrap(); - - let pre_balance_check = - read_balance(&storage, &native_token, &addr).unwrap(); - - assert_eq!(pre_balance_check, pre_balance); - - // sources - let sources = BTreeMap::from_iter([( - (addr.clone(), native_token.clone()), - pre_balance, - )]); - - // targets - let targets = BTreeMap::from_iter([( - (addr.clone(), native_token.clone()), - pre_balance, - )]); - - multi_transfer(&mut storage, &sources, &targets).unwrap(); - - let post_balance_check = - read_balance(&storage, &native_token, &addr).unwrap(); - - assert_eq!(post_balance_check, pre_balance); - } - #[test] fn test_credit() { let mut storage = TestStorage::default(); diff --git a/crates/trans_token/src/tx.rs b/crates/trans_token/src/tx.rs new file mode 100644 index 00000000000..bfcd51f7009 --- /dev/null +++ b/crates/trans_token/src/tx.rs @@ -0,0 +1,676 @@ +//! Token transfers + +use std::borrow::Cow; +use std::collections::{BTreeMap, BTreeSet}; + +use namada_core::address::Address; +use namada_core::collections::HashSet; +use namada_events::{EmitEvents, EventLevel}; +use namada_state::Error; +use namada_tx_env::{Result, TxEnv}; + +use crate::event::{TokenEvent, TokenOperation}; +use crate::storage_key::balance_key; +use crate::{read_balance, Amount, UserAccount}; + +/// Multi-transfer credit or debit amounts +pub trait CreditOrDebit { + /// Gets an iterator over the pair of credited or debited owners and token + /// addresses, in sorted order. + fn keys(&self) -> impl Iterator; + + /// Returns a reference to the value corresponding to pair of owner and + /// token address + fn get(&self, key: &(Address, Address)) -> Option<&Amount>; + + /// Gets an owning iterator over the pairs of credited or debited owners and + /// token addresses paired with the token amount, sorted by key. + fn into_iter( + self, + ) -> impl IntoIterator; +} + +impl CreditOrDebit for BTreeMap<(Address, Address), Amount> { + fn keys(&self) -> impl Iterator { + self.keys().cloned() + } + + fn get(&self, key: &(Address, Address)) -> Option<&Amount> { + self.get(key) + } + + fn into_iter( + self, + ) -> impl IntoIterator { + IntoIterator::into_iter(self) + } +} + +/// Transfer tokens from `sources` to `dests`. +/// +/// Returns an `Err` if any source has insufficient balance or if the transfer +/// to any destination would overflow (This can only happen if the total supply +/// doesn't fit in `token::Amount`). Returns the set of debited accounts. +pub fn multi_transfer( + env: &mut ENV, + sources: impl CreditOrDebit, + targets: impl CreditOrDebit, + event_desc: Cow<'static, str>, +) -> Result> +where + ENV: TxEnv + EmitEvents, +{ + let mut debited_accounts = HashSet::new(); + // Collect all the accounts whose balance has changed + let mut accounts = BTreeSet::new(); + accounts.extend(sources.keys()); + accounts.extend(targets.keys()); + + let unexpected_err = || { + Error::new_const( + "Computing difference between amounts should never overflow", + ) + }; + // Apply the balance change for each account in turn + let mut any_balance_changed = false; + for ref account @ (ref owner, ref token) in accounts { + let overflow_err = || { + Error::new_alloc(format!( + "The transfer would overflow balance of {owner}" + )) + }; + let underflow_err = + || Error::new_alloc(format!("{owner} has insufficient balance")); + // Load account balances and deltas + let owner_key = balance_key(token, owner); + let owner_balance = read_balance(env, token, owner)?; + let src_amt = sources.get(account).cloned().unwrap_or_default(); + let dest_amt = targets.get(account).cloned().unwrap_or_default(); + // Compute owner_balance + dest_amt - src_amt + let new_owner_balance = if src_amt <= dest_amt { + owner_balance + .checked_add( + dest_amt.checked_sub(src_amt).ok_or_else(unexpected_err)?, + ) + .ok_or_else(overflow_err)? + } else { + debited_accounts.insert(owner.to_owned()); + owner_balance + .checked_sub( + src_amt.checked_sub(dest_amt).ok_or_else(unexpected_err)?, + ) + .ok_or_else(underflow_err)? + }; + // Write the new balance + if new_owner_balance != owner_balance { + any_balance_changed = true; + env.write(&owner_key, new_owner_balance)?; + } + } + + if !any_balance_changed { + return Ok(debited_accounts); + } + + let mut evt_sources = BTreeMap::new(); + let mut evt_targets = BTreeMap::new(); + let mut post_balances = BTreeMap::new(); + + for ((src, token), amount) in sources.into_iter() { + // The tx must be authorized by the source address + env.insert_verifier(&src)?; + if token.is_internal() { + // Established address tokens do not have VPs themselves, their + // validation is handled by the `Multitoken` internal address, + // but internal token addresses have to verify + // the transfer + env.insert_verifier(&token)?; + } + evt_sources.insert( + (UserAccount::Internal(src.clone()), token.clone()), + amount.into(), + ); + post_balances.insert( + (UserAccount::Internal(src.clone()), token.clone()), + crate::read_balance(env, &token, &src)?.into(), + ); + } + + for ((target, token), amount) in targets.into_iter() { + // The tx must be authorized by the involved address + env.insert_verifier(&target)?; + if token.is_internal() { + // Established address tokens do not have VPs themselves, their + // validation is handled by the `Multitoken` internal address, + // but internal token addresses have to verify + // the transfer + env.insert_verifier(&token)?; + } + evt_targets.insert( + (UserAccount::Internal(target.clone()), token.clone()), + amount.into(), + ); + post_balances.insert( + (UserAccount::Internal(target.clone()), token.clone()), + crate::read_balance(env, &token, &target)?.into(), + ); + } + + env.emit(TokenEvent { + descriptor: event_desc, + level: EventLevel::Tx, + operation: TokenOperation::Transfer { + sources: evt_sources, + targets: evt_targets, + post_balances, + }, + }); + + Ok(debited_accounts) +} + +#[derive(Debug, Clone)] +struct SingleCreditOrDebit { + src_or_dest: Address, + token: Address, + amount: Amount, +} + +impl CreditOrDebit for SingleCreditOrDebit { + fn keys(&self) -> impl Iterator { + [(self.src_or_dest.clone(), self.token.clone())].into_iter() + } + + fn get( + &self, + (key_owner, key_token): &(Address, Address), + ) -> Option<&Amount> { + if key_token == &self.token && key_owner == &self.src_or_dest { + return Some(&self.amount); + } + None + } + + fn into_iter( + self, + ) -> impl IntoIterator { + [((self.src_or_dest.clone(), self.token.clone()), self.amount)] + } +} + +/// Transfer transparent token, insert the verifier expected by the VP and an +/// emit an event. +pub fn transfer( + env: &mut ENV, + source: &Address, + target: &Address, + token: &Address, + amount: Amount, + event_desc: Cow<'static, str>, +) -> Result<()> +where + ENV: TxEnv + EmitEvents, +{ + multi_transfer( + env, + SingleCreditOrDebit { + src_or_dest: source.clone(), + token: token.clone(), + amount, + }, + SingleCreditOrDebit { + src_or_dest: target.clone(), + token: token.clone(), + amount, + }, + event_desc, + )?; + + Ok(()) +} + +#[cfg(test)] +mod test { + use std::collections::BTreeMap; + + use namada_core::address::testing::{ + arb_address, arb_non_internal_address, + }; + use namada_core::token::testing::arb_amount; + use namada_core::uint::Uint; + use namada_core::{address, token}; + use namada_events::extend::EventAttributeEntry; + use namada_tests::tx::{ctx, tx_host_env}; + use proptest::prelude::*; + + use super::*; + use crate::event::{PostBalances, SourceAccounts, TargetAccounts}; + + const EVENT_DESC: Cow<'static, str> = Cow::Borrowed("event-desc"); + + proptest! { + #[test] + fn test_valid_transfer_tx( + (src, dest, token) in ( + arb_non_internal_address(), + arb_non_internal_address(), + arb_address() + ).prop_filter("unique addresses", |(src, dest, token)| + src != dest && dest != token && src != token), + + amount in arb_amount(), + ) { + // Test via `fn transfer` + test_valid_transfer_tx_aux(src.clone(), dest.clone(), token.clone(), amount, || { + transfer(ctx(), &src, &dest, &token, amount, EVENT_DESC).unwrap(); + }); + + // Clean-up tx env before running next test + let _old_env = tx_host_env::take(); + + // Test via `fn multi_transfer` + test_valid_transfer_tx_aux(src.clone(), dest.clone(), token.clone(), amount, || { + let sources = + BTreeMap::from_iter([((src.clone(), token.clone()), amount)]); + + let targets = + BTreeMap::from_iter([((dest.clone(), token.clone()), amount)]); + + let debited_accounts = + multi_transfer(ctx(), sources, targets, EVENT_DESC).unwrap(); + + if amount.is_zero() { + assert!(debited_accounts.is_empty()); + } else { + assert_eq!(debited_accounts.len(), 1); + assert!(debited_accounts.contains(&src)); + } + }); + } + } + + fn test_valid_transfer_tx_aux( + src: Address, + dest: Address, + token: Address, + amount: Amount, + apply_transfer: F, + ) { + tx_host_env::init(); + + tx_host_env::with(|tx_env| { + tx_env.spawn_accounts([&src, &dest, &token]); + tx_env.credit_tokens(&src, &token, amount); + }); + assert_eq!(read_balance(ctx(), &token, &src).unwrap(), amount); + + apply_transfer(); + + // Dest received the amount + assert_eq!(read_balance(ctx(), &token, &dest).unwrap(), amount); + + // Src spent the amount + assert_eq!( + read_balance(ctx(), &token, &src).unwrap(), + token::Amount::zero() + ); + + tx_host_env::with(|tx_env| { + // Internal token address have to be part of the verifier set + assert!(!token.is_internal() || tx_env.verifiers.contains(&token)); + // Src and dest should always verify + assert!(tx_env.verifiers.contains(&src)); + assert!(tx_env.verifiers.contains(&dest)); + }); + + // The transfer must emit an event + tx_host_env::with(|tx_env| { + let events: Vec<_> = tx_env + .state + .write_log() + .get_events_of::() + .collect(); + assert_eq!(events.len(), 1); + let event = events[0].clone(); + assert_eq!(event.level(), &EventLevel::Tx); + assert_eq!(event.kind(), &crate::event::types::TRANSFER); + let attrs = event.into_attributes(); + let amount_uint: Uint = amount.into(); + + let exp_balances = if src < dest { + format!( + "[[[\"internal-address/{src}\",\"{token}\"],\"0\"],[[\"\ + internal-address/{dest}\",\"{token}\"],\"{amount_uint}\"\ + ]]", + ) + } else { + format!( + "[[[\"internal-address/{dest}\",\"{token}\"],\"\ + {amount_uint}\"],[[\"internal-address/{src}\",\"{token}\"\ + ],\"0\"]]", + ) + }; + let exp_sources = format!( + "[[[\"internal-address/{src}\",\"{token}\"],\"{amount_uint}\"\ + ]]", + ); + let exp_targets = format!( + "[[[\"internal-address/{dest}\",\"{token}\"],\"{amount_uint}\"\ + ]]", + ); + + itertools::assert_equal( + attrs, + BTreeMap::from_iter([ + (PostBalances::KEY.to_string(), exp_balances), + (SourceAccounts::KEY.to_string(), exp_sources), + (TargetAccounts::KEY.to_string(), exp_targets), + ( + "token-event-descriptor".to_string(), + EVENT_DESC.to_string(), + ), + ]), + ); + }) + } + + #[test] + fn test_transfer_tx_zero_amount_is_noop() { + let src = address::testing::established_address_1(); + let dest = address::testing::established_address_2(); + let token = address::testing::established_address_3(); + let amount = token::Amount::zero(); + let src_balance = token::Amount::native_whole(1); + let dest_balance = token::Amount::native_whole(1); + + tx_host_env::init(); + + tx_host_env::with(|tx_env| { + tx_env.spawn_accounts([&src, &dest, &token]); + tx_env.credit_tokens(&src, &token, src_balance); + tx_env.credit_tokens(&dest, &token, src_balance); + }); + + transfer(ctx(), &src, &dest, &token, amount, EVENT_DESC).unwrap(); + + // Dest balance is still the same + assert_eq!(read_balance(ctx(), &token, &dest).unwrap(), dest_balance); + + // Src balance is still the same + assert_eq!(read_balance(ctx(), &token, &src).unwrap(), src_balance); + + // Verifiers set is empty + tx_host_env::with(|tx_env| { + assert!(tx_env.verifiers.is_empty()); + }); + + // Must no emit an event + tx_host_env::with(|tx_env| { + let events: Vec<_> = tx_env + .state + .write_log() + .get_events_of::() + .collect(); + assert!(events.is_empty()); + }); + } + + #[test] + fn test_transfer_tx_to_self_is_noop() { + let src = address::testing::established_address_1(); + let token = address::testing::established_address_2(); + let amount = token::Amount::zero(); + let src_balance = token::Amount::native_whole(1); + + tx_host_env::init(); + + tx_host_env::with(|tx_env| { + tx_env.spawn_accounts([&src, &token]); + tx_env.credit_tokens(&src, &token, src_balance); + }); + + transfer(ctx(), &src, &src, &token, amount, EVENT_DESC).unwrap(); + + // Src balance is still the same + assert_eq!(read_balance(ctx(), &token, &src).unwrap(), src_balance); + + // Verifiers set is empty + tx_host_env::with(|tx_env| { + assert!(tx_env.verifiers.is_empty()); + }); + + // Must no emit an event + tx_host_env::with(|tx_env| { + let events: Vec<_> = tx_env + .state + .write_log() + .get_events_of::() + .collect(); + assert!(events.is_empty()); + }); + } + + #[test] + fn test_transfer_tx_to_self_with_insufficient_balance() { + let src = address::testing::established_address_1(); + let token = address::testing::established_address_2(); + let amount = token::Amount::native_whole(10); + let src_balance = token::Amount::native_whole(1); + assert!(amount > src_balance); + + tx_host_env::init(); + + tx_host_env::with(|tx_env| { + tx_env.spawn_accounts([&src, &token]); + tx_env.credit_tokens(&src, &token, src_balance); + }); + + transfer(ctx(), &src, &src, &token, amount, EVENT_DESC).unwrap(); + + // Src balance is still the same + assert_eq!(read_balance(ctx(), &token, &src).unwrap(), src_balance); + + // Verifiers set is empty + tx_host_env::with(|tx_env| { + assert!(tx_env.verifiers.is_empty()); + }); + + // Must no emit an event + tx_host_env::with(|tx_env| { + let events: Vec<_> = tx_env + .state + .write_log() + .get_events_of::() + .collect(); + assert!(events.is_empty()); + }); + } + + /// Test a 3-way transfer between three participants: + /// + /// 1. (p1, token1, amount1) -> p2 + /// 2. (p2, token1, amount2) -> p3 + /// 3. (p3, token2, amount3) -> p1 + #[test] + fn test_three_way_multi_transfer_tx() { + tx_host_env::init(); + + let p1 = address::testing::established_address_1(); + let p2 = address::testing::established_address_2(); + let p3 = address::testing::established_address_3(); + let token1 = address::testing::established_address_4(); + let token2 = address::testing::established_address_5(); + let amount1 = token::Amount::native_whole(10); + let amount2 = token::Amount::native_whole(3); + let amount3 = token::Amount::native_whole(90); + + tx_host_env::with(|tx_env| { + tx_env.spawn_accounts([&p1, &p2, &p3, &token1, &token2]); + tx_env.credit_tokens(&p1, &token1, amount1); + tx_env.credit_tokens(&p3, &token2, amount3); + }); + assert_eq!(read_balance(ctx(), &token1, &p1).unwrap(), amount1); + assert_eq!(read_balance(ctx(), &token1, &p2).unwrap(), Amount::zero()); + assert_eq!(read_balance(ctx(), &token1, &p3).unwrap(), Amount::zero()); + assert_eq!(read_balance(ctx(), &token2, &p1).unwrap(), Amount::zero()); + assert_eq!(read_balance(ctx(), &token2, &p2).unwrap(), Amount::zero()); + assert_eq!(read_balance(ctx(), &token2, &p3).unwrap(), amount3); + + let sources = BTreeMap::from_iter([ + ((p1.clone(), token1.clone()), amount1), + ((p2.clone(), token1.clone()), amount2), + ((p3.clone(), token2.clone()), amount3), + ]); + + let targets = BTreeMap::from_iter([ + ((p2.clone(), token1.clone()), amount1), + ((p3.clone(), token1.clone()), amount2), + ((p1.clone(), token2.clone()), amount3), + ]); + + let debited_accounts = + multi_transfer(ctx(), sources, targets, EVENT_DESC).unwrap(); + + // p2 is not debited as it received more of token1 than it spent + assert_eq!(debited_accounts.len(), 2); + assert!(debited_accounts.contains(&p1)); + assert!(debited_accounts.contains(&p3)); + + // p1 spent all token1 + assert_eq!(read_balance(ctx(), &token1, &p1).unwrap(), Amount::zero()); + // p1 received token2 + assert_eq!(read_balance(ctx(), &token2, &p1).unwrap(), amount3); + + // p2 received amount1 and spent amount2 of token1 + assert_eq!( + read_balance(ctx(), &token1, &p2).unwrap(), + amount1 - amount2 + ); + // p2 doesn't have any token2 + assert_eq!(read_balance(ctx(), &token2, &p2).unwrap(), Amount::zero()); + + // p3 received token1 + assert_eq!(read_balance(ctx(), &token1, &p3).unwrap(), amount2); + // p3 spent token2 + assert_eq!(read_balance(ctx(), &token2, &p3).unwrap(), Amount::zero()); + + tx_host_env::with(|tx_env| { + // All parties should always verify + assert!(tx_env.verifiers.contains(&p1)); + assert!(tx_env.verifiers.contains(&p2)); + assert!(tx_env.verifiers.contains(&p3)); + }); + + // The transfer must emit an event + tx_host_env::with(|tx_env| { + let events: Vec<_> = tx_env + .state + .write_log() + .get_events_of::() + .collect(); + assert_eq!(events.len(), 1); + let event = events[0].clone(); + assert_eq!(event.level(), &EventLevel::Tx); + assert_eq!(event.kind(), &crate::event::types::TRANSFER); + + dbg!(event.into_attributes()); + }) + } + + #[test] + fn test_multi_transfer_to_self_is_no_op() { + tx_host_env::init(); + + let token = address::testing::nam(); + + // Get one account + let addr = address::testing::gen_implicit_address(); + + // Credit the account some balance + let pre_balance = token::Amount::native_whole(1); + tx_host_env::with(|tx_env| { + tx_env.credit_tokens(&addr, &token, pre_balance); + }); + + let pre_balance_check = read_balance(ctx(), &token, &addr).unwrap(); + + assert_eq!(pre_balance_check, pre_balance); + + let sources = + BTreeMap::from_iter([((addr.clone(), token.clone()), pre_balance)]); + + let targets = + BTreeMap::from_iter([((addr.clone(), token.clone()), pre_balance)]); + + let debited_accounts = + multi_transfer(ctx(), sources, targets, EVENT_DESC).unwrap(); + + // No account has been debited + assert!(debited_accounts.is_empty()); + + // Balance is the same + let post_balance_check = read_balance(ctx(), &token, &addr).unwrap(); + assert_eq!(post_balance_check, pre_balance); + + // Verifiers set is empty + tx_host_env::with(|tx_env| { + assert!(tx_env.verifiers.is_empty()); + }); + + // Must no emit an event + tx_host_env::with(|tx_env| { + let events: Vec<_> = tx_env + .state + .write_log() + .get_events_of::() + .collect(); + assert!(events.is_empty()); + }); + } + + #[test] + fn test_multi_transfer_tx_to_self_with_insufficient_balance() { + let src = address::testing::established_address_1(); + let token = address::testing::established_address_2(); + let amount = token::Amount::native_whole(10); + let src_balance = token::Amount::native_whole(1); + assert!(amount > src_balance); + + tx_host_env::init(); + + tx_host_env::with(|tx_env| { + tx_env.spawn_accounts([&src, &token]); + tx_env.credit_tokens(&src, &token, src_balance); + }); + + let sources = + BTreeMap::from_iter([((src.clone(), token.clone()), amount)]); + + let targets = + BTreeMap::from_iter([((src.clone(), token.clone()), amount)]); + + let debited_accounts = + multi_transfer(ctx(), sources, targets, EVENT_DESC).unwrap(); + + // No account has been debited + assert!(debited_accounts.is_empty()); + + // Src balance is still the same + assert_eq!(read_balance(ctx(), &token, &src).unwrap(), src_balance); + + // Verifiers set is empty + tx_host_env::with(|tx_env| { + assert!(tx_env.verifiers.is_empty()); + }); + + // Must no emit an event + tx_host_env::with(|tx_env| { + let events: Vec<_> = tx_env + .state + .write_log() + .get_events_of::() + .collect(); + assert!(events.is_empty()); + }); + } +} diff --git a/crates/tx_env/src/lib.rs b/crates/tx_env/src/lib.rs index 7eb99caa851..2bbb513f02e 100644 --- a/crates/tx_env/src/lib.rs +++ b/crates/tx_env/src/lib.rs @@ -22,6 +22,7 @@ pub use namada_core::address::Address; pub use namada_core::borsh::{ BorshDeserialize, BorshSerialize, BorshSerializeExt, }; +pub use namada_core::masp::MaspTransaction; pub use namada_core::storage; pub use namada_events::{Event, EventToEmit, EventType}; pub use namada_storage::{Result, ResultExt, StorageRead, StorageWrite}; @@ -103,4 +104,9 @@ pub trait TxEnv: StorageRead + StorageWrite { /// Set the sentinel for an invalid section commitment fn set_commitment_sentinel(&mut self); + + /// Update the masp note commitment tree in storage with the new notes + fn update_masp_note_commitment_tree( + transaction: &MaspTransaction, + ) -> Result; } diff --git a/crates/tx_prelude/src/ibc.rs b/crates/tx_prelude/src/ibc.rs index 4a20dcae177..283dc0f723e 100644 --- a/crates/tx_prelude/src/ibc.rs +++ b/crates/tx_prelude/src/ibc.rs @@ -19,22 +19,20 @@ pub use namada_ibc::{ }; use namada_tx_env::TxEnv; -use crate::token::transfer; -use crate::{Ctx, Result}; +use crate::{token, Ctx, Result}; /// IBC actions to handle an IBC message. The `verifiers` inserted into the set /// must be inserted into the tx context with `Ctx::insert_verifier` after tx /// execution. pub fn ibc_actions( ctx: &mut Ctx, -) -> IbcActions<'_, Ctx, crate::parameters::Store, crate::token::Store> -{ +) -> IbcActions<'_, Ctx, crate::parameters::Store, token::Store> { let ctx = Rc::new(RefCell::new(ctx.clone())); let verifiers = Rc::new(RefCell::new(BTreeSet::
::new())); let mut actions = IbcActions::new(ctx.clone(), verifiers.clone()); let module = TransferModule::new(ctx.clone(), verifiers); actions.add_transfer_module(module); - let module = NftTransferModule::>::new(ctx); + let module = NftTransferModule::>::new(ctx); actions.add_transfer_module(module); actions } @@ -65,7 +63,7 @@ impl IbcStorageContext for Ctx { token: &Address, amount: Amount, ) -> Result<()> { - transfer(self, src, dest, token, amount) + token::transfer(self, src, dest, token, amount) } fn mint_token( @@ -74,7 +72,7 @@ impl IbcStorageContext for Ctx { token: &Address, amount: Amount, ) -> Result<()> { - mint_tokens::<_, crate::token::Store<_>>(self, target, token, amount) + mint_tokens::<_, token::Store<_>>(self, target, token, amount) } fn burn_token( @@ -83,7 +81,7 @@ impl IbcStorageContext for Ctx { token: &Address, amount: Amount, ) -> Result<()> { - burn_tokens::<_, crate::token::Store<_>>(self, target, token, amount) + burn_tokens::<_, token::Store<_>>(self, target, token, amount) } fn insert_verifier(&mut self, addr: &Address) -> Result<()> { diff --git a/crates/tx_prelude/src/lib.rs b/crates/tx_prelude/src/lib.rs index 89ad1e80c86..edfb8a5a25b 100644 --- a/crates/tx_prelude/src/lib.rs +++ b/crates/tx_prelude/src/lib.rs @@ -403,6 +403,12 @@ impl TxEnv for Ctx { fn set_commitment_sentinel(&mut self) { unsafe { namada_tx_set_commitment_sentinel() } } + + fn update_masp_note_commitment_tree( + transaction: &MaspTransaction, + ) -> Result { + update_masp_note_commitment_tree(transaction) + } } impl namada_tx::action::Read for Ctx { diff --git a/crates/tx_prelude/src/token.rs b/crates/tx_prelude/src/token.rs index a1b753e39ea..b77ef598dfc 100644 --- a/crates/tx_prelude/src/token.rs +++ b/crates/tx_prelude/src/token.rs @@ -1,22 +1,22 @@ //! Shielded and transparent tokens related functions -use std::collections::BTreeMap; - -use namada_core::address::Address; use namada_core::collections::HashSet; -use namada_events::extend::UserAccount; -use namada_events::{EmitEvents, EventLevel}; -use namada_token::event::{TokenEvent, TokenOperation}; #[cfg(any(test, feature = "testing"))] pub use namada_token::testing; +pub use namada_token::tx::apply_shielded_transfer; +use namada_token::TransparentTransfersRef; pub use namada_token::{ storage_key, utils, Amount, DenominatedAmount, Store, Transfer, }; -use namada_tx_env::TxEnv; +use namada_tx::BatchedTx; +use namada_tx_env::Address; use crate::{Ctx, Result, TxResult}; -/// A transparent token transfer that can be used in a transaction. +const EVENT_DESC: &str = "transfer-from-wasm"; + +/// Transfer transparent token, insert the verifier expected by the VP and an +/// emit an event. pub fn transfer( ctx: &mut Ctx, src: &Address, @@ -24,94 +24,30 @@ pub fn transfer( token: &Address, amount: Amount, ) -> TxResult { - // The tx must be authorized by the source and destination addresses - ctx.insert_verifier(src)?; - ctx.insert_verifier(dest)?; - if token.is_internal() { - // Established address tokens do not have VPs themselves, their - // validation is handled by the `Multitoken` internal address, but - // internal token addresses have to verify the transfer - ctx.insert_verifier(token)?; - } - - namada_token::transfer(ctx, token, src, dest, amount)?; - - ctx.emit(TokenEvent { - descriptor: "transfer-from-wasm".into(), - level: EventLevel::Tx, - operation: TokenOperation::transfer( - UserAccount::Internal(src.clone()), - UserAccount::Internal(dest.clone()), - token.clone(), - amount.into(), - namada_token::read_balance(ctx, token, src)?.into(), - Some(namada_token::read_balance(ctx, token, dest)?.into()), - ), - }); - - Ok(()) + namada_token::tx::transfer(ctx, src, dest, token, amount, EVENT_DESC.into()) } -/// A transparent token transfer that can be used in a transaction. Returns the -/// set of debited accounts. +/// Transparent and shielded token transfers that can be used in a transaction. pub fn multi_transfer( ctx: &mut Ctx, - sources: &BTreeMap<(Address, Address), Amount>, - dests: &BTreeMap<(Address, Address), Amount>, -) -> Result> { - let debited_accounts = namada_token::multi_transfer(ctx, sources, dests)?; - - let mut evt_sources = BTreeMap::new(); - let mut evt_targets = BTreeMap::new(); - let mut post_balances = BTreeMap::new(); - - for ((src, token), amount) in sources { - // The tx must be authorized by the involved address - ctx.insert_verifier(src)?; - if token.is_internal() { - // Established address tokens do not have VPs themselves, their - // validation is handled by the `Multitoken` internal address, but - // internal token addresses have to verify the transfer - ctx.insert_verifier(token)?; - } - evt_sources.insert( - (UserAccount::Internal(src.clone()), token.clone()), - (*amount).into(), - ); - post_balances.insert( - (UserAccount::Internal(src.clone()), token.clone()), - namada_token::read_balance(ctx, token, src)?.into(), - ); - } - - for ((dest, token), amount) in dests { - // The tx must be authorized by the involved address - ctx.insert_verifier(dest)?; - if token.is_internal() { - // Established address tokens do not have VPs themselves, their - // validation is handled by the `Multitoken` internal address, but - // internal token addresses have to verify the transfer - ctx.insert_verifier(token)?; - } - evt_targets.insert( - (UserAccount::Internal(dest.clone()), token.clone()), - (*amount).into(), - ); - post_balances.insert( - (UserAccount::Internal(dest.clone()), token.clone()), - namada_token::read_balance(ctx, token, dest)?.into(), - ); - } - - ctx.emit(TokenEvent { - descriptor: "transfer-from-wasm".into(), - level: EventLevel::Tx, - operation: TokenOperation::Transfer { - sources: evt_sources, - targets: evt_targets, - post_balances, - }, - }); + transfers: Transfer, + tx_data: &BatchedTx, +) -> TxResult { + namada_token::tx::multi_transfer(ctx, transfers, tx_data, EVENT_DESC.into()) +} - Ok(debited_accounts) +/// Transfer tokens from `sources` to `targets` and submit a transfer event. +/// +/// Returns an `Err` if any source has insufficient balance or if the transfer +/// to any destination would overflow (This can only happen if the total supply +/// doesn't fit in `token::Amount`). Returns a set of debited accounts. +pub fn apply_transparent_transfers( + ctx: &mut Ctx, + transfers: TransparentTransfersRef<'_>, +) -> Result> { + namada_token::tx::apply_transparent_transfers( + ctx, + transfers, + EVENT_DESC.into(), + ) } diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 70b07f53401..20c9d6fae7c 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -4099,8 +4099,11 @@ dependencies = [ "namada_events", "namada_macros", "namada_shielded_token", + "namada_storage", "namada_systems", "namada_trans_token", + "namada_tx", + "namada_tx_env", "proptest", "serde", ] @@ -4115,6 +4118,7 @@ dependencies = [ "namada_state", "namada_systems", "namada_tx", + "namada_tx_env", "namada_vp_env", "thiserror", "tracing", diff --git a/wasm/tx_ibc/src/lib.rs b/wasm/tx_ibc/src/lib.rs index 964012ca671..c860be5f227 100644 --- a/wasm/tx_ibc/src/lib.rs +++ b/wasm/tx_ibc/src/lib.rs @@ -3,8 +3,6 @@ //! tx_data. This tx uses an IBC message wrapped inside //! `key::ed25519::SignedTxData` as its input as declared in `ibc` crate. -use std::collections::BTreeMap; - use namada_tx_prelude::action::{Action, MaspAction, Write}; use namada_tx_prelude::*; @@ -16,27 +14,11 @@ fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { .into_storage_result()?; let masp_section_ref = if let Some(transfers) = transfer { - // Prepare the sources of the multi-transfer - let sources = transfers - .sources - .into_iter() - .map(|(account, amount)| { - ((account.owner, account.token), amount.amount()) - }) - .collect::>(); - - // Prepare the target of the multi-transfer - let targets = transfers - .targets - .into_iter() - .map(|(account, amount)| { - ((account.owner, account.token), amount.amount()) - }) - .collect::>(); - - // Effect the multi transfer - token::multi_transfer(ctx, &sources, &targets) - .wrap_err("Token transfer failed")?; + if let Some(transparent) = transfers.transparent_part() { + let _debited_accounts = + token::apply_transparent_transfers(ctx, transparent) + .wrap_err("Transparent token transfer failed")?; + } transfers.shielded_section_hash } else { diff --git a/wasm/tx_transfer/src/lib.rs b/wasm/tx_transfer/src/lib.rs index aa1301e164d..ff933fc09d9 100644 --- a/wasm/tx_transfer/src/lib.rs +++ b/wasm/tx_transfer/src/lib.rs @@ -2,10 +2,6 @@ //! This tx uses `token::TransparentTransfer` wrapped inside `SignedTxData` //! as its input as declared in `namada` crate. -use std::collections::{BTreeMap, BTreeSet}; - -use namada_tx_prelude::action::{Action, MaspAction, Write}; -use namada_tx_prelude::masp::addr_taddr; use namada_tx_prelude::*; #[transaction] @@ -15,77 +11,5 @@ fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { .wrap_err("Failed to decode token::TransparentTransfer tx data")?; debug_log!("apply_tx called with transfer: {:#?}", transfers); - // Prepare the sources of the multi-transfer - let sources = transfers - .sources - .into_iter() - .map(|(account, amount)| { - ((account.owner, account.token), amount.amount()) - }) - .collect::>(); - - // Prepare the target of the multi-transfer - let targets = transfers - .targets - .into_iter() - .map(|(account, amount)| { - ((account.owner, account.token), amount.amount()) - }) - .collect::>(); - - // Effect the multi transfer - let debited_accounts = token::multi_transfer(ctx, &sources, &targets) - .wrap_err("Token transfer failed")?; - - // Apply the shielded transfer if there is a link to one - if let Some(masp_section_ref) = transfers.shielded_section_hash { - let shielded = tx_data - .tx - .get_masp_section(&masp_section_ref) - .cloned() - .ok_or_err_msg( - "Unable to find required shielded section in tx data", - ) - .inspect_err(|_| { - ctx.set_commitment_sentinel(); - })?; - token::utils::handle_masp_tx(ctx, &shielded) - .wrap_err("Encountered error while handling MASP transaction")?; - update_masp_note_commitment_tree(&shielded) - .wrap_err("Failed to update the MASP commitment tree")?; - - ctx.push_action(Action::Masp(MaspAction::MaspSectionRef( - masp_section_ref, - )))?; - // Extract the debited accounts for the masp part of the transfer and - // push the relative actions - let vin_addresses = shielded.transparent_bundle().map_or_else( - Default::default, - |bndl| { - bndl.vin - .iter() - .map(|vin| vin.address) - .collect::>() - }, - ); - let masp_authorizers: Vec<_> = debited_accounts - .into_iter() - .filter(|account| { - vin_addresses.contains(&addr_taddr(account.clone())) - }) - .collect(); - if masp_authorizers.len() != vin_addresses.len() { - return Err(Error::SimpleMessage( - "Transfer transaction does not debit all the expected accounts", - )); - } - - for authorizer in masp_authorizers { - ctx.push_action(Action::Masp(MaspAction::MaspAuthorizer( - authorizer, - )))?; - } - } - - Ok(()) + token::multi_transfer(ctx, transfers, &tx_data) } diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index b854ddb9982..a574f910fba 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -2262,8 +2262,11 @@ dependencies = [ "namada_events", "namada_macros", "namada_shielded_token", + "namada_storage", "namada_systems", "namada_trans_token", + "namada_tx", + "namada_tx_env", "serde", ] @@ -2277,6 +2280,7 @@ dependencies = [ "namada_state", "namada_systems", "namada_tx", + "namada_tx_env", "namada_vp_env", "thiserror", "tracing",