From e9f5a4607763dfe0ded5696159eb438d8d13d0e2 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sun, 2 Jun 2024 19:51:41 +0200 Subject: [PATCH 1/9] Vectorizes transparent transfers --- crates/apps_lib/src/cli.rs | 23 ++- crates/apps_lib/src/client/tx.rs | 13 +- crates/benches/host_env.rs | 6 +- crates/benches/native_vps.rs | 6 +- crates/benches/process_wrapper.rs | 8 +- crates/light_sdk/src/transaction/transfer.rs | 2 +- crates/sdk/src/args.rs | 19 +- crates/sdk/src/lib.rs | 19 +- crates/sdk/src/signing.rs | 173 ++++++++++++++----- crates/sdk/src/tx.rs | 119 ++++++++----- crates/tests/src/integration/ledger_tests.rs | 21 +-- crates/token/src/lib.rs | 33 +++- wasm/tx_transparent_transfer/src/lib.rs | 24 +-- 13 files changed, 317 insertions(+), 149 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index eadf78063a..2e59687eb1 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -4338,14 +4338,21 @@ pub mod args { ctx: &mut Context, ) -> Result, Self::Error> { let tx = self.tx.to_sdk(ctx)?; + let mut data = vec![]; let chain_ctx = ctx.borrow_mut_chain_or_exit(); + for transfer_data in self.data { + data.push(TxTransparentTransferData { + source: chain_ctx.get(&transfer_data.source), + target: chain_ctx.get(&transfer_data.target), + token: chain_ctx.get(&transfer_data.token), + amount: transfer_data.amount, + }); + } + Ok(TxTransparentTransfer:: { tx, - source: chain_ctx.get(&self.source), - target: chain_ctx.get(&self.target), - token: chain_ctx.get(&self.token), - amount: self.amount, + data, tx_code_path: self.tx_code_path.to_path_buf(), }) } @@ -4359,12 +4366,16 @@ pub mod args { let token = TOKEN.parse(matches); let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); let tx_code_path = PathBuf::from(TX_TRANSPARENT_TRANSFER_WASM); - Self { - tx, + + let data = vec![TxTransparentTransferData { source, target, token, amount, + }]; + Self { + tx, + data, tx_code_path, } } diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 80e6e52e09..944e204279 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -743,7 +743,18 @@ pub async fn submit_transparent_transfer( namada: &impl Namada, args: args::TxTransparentTransfer, ) -> Result<(), error::Error> { - submit_reveal_aux(namada, args.tx.clone(), &args.source).await?; + submit_reveal_aux( + namada, + args.tx.clone(), + &args + .data + .first() + .ok_or_else(|| { + error::Error::Other("Missing transfer data".to_string()) + })? + .source, + ) + .await?; let (mut tx, signing_data) = args.clone().build(namada).await?; diff --git a/crates/benches/host_env.rs b/crates/benches/host_env.rs index 1eee561163..48e521cfd3 100644 --- a/crates/benches/host_env.rs +++ b/crates/benches/host_env.rs @@ -3,7 +3,7 @@ use namada::core::account::AccountPublicKeysMap; use namada::core::address; use namada::core::collections::{HashMap, HashSet}; use namada::ledger::storage::DB; -use namada::token::{Amount, TransparentTransfer}; +use namada::token::{Amount, TransparentTransfer, TransparentTransferData}; use namada::tx::Authorization; use namada::vm::wasm::TxCache; use namada_apps_lib::wallet::defaults; @@ -18,12 +18,12 @@ use namada_node::bench_utils::{ // transaction fn tx_section_signature_validation(c: &mut Criterion) { let shell = BenchShell::default(); - let transfer_data = TransparentTransfer { + let transfer_data = TransparentTransfer(vec![TransparentTransferData { source: defaults::albert_address(), target: defaults::bertha_address(), token: address::testing::nam(), amount: Amount::native_whole(500).native_denominated(), - }; + }]); let tx = shell.generate_tx( TX_TRANSPARENT_TRANSFER_WASM, transfer_data, diff --git a/crates/benches/native_vps.rs b/crates/benches/native_vps.rs index 298c37fcf5..aba3cb9802 100644 --- a/crates/benches/native_vps.rs +++ b/crates/benches/native_vps.rs @@ -55,7 +55,7 @@ use namada::sdk::masp_primitives::merkle_tree::CommitmentTree; use namada::sdk::masp_primitives::transaction::Transaction; use namada::sdk::masp_proofs::sapling::SaplingVerificationContextInner; use namada::state::{Epoch, StorageRead, StorageWrite, TxIndex}; -use namada::token::{Amount, TransparentTransfer}; +use namada::token::{Amount, TransparentTransfer, TransparentTransferData}; use namada::tx::{BatchedTx, Code, Section, Tx}; use namada_apps_lib::wallet::defaults; use namada_node::bench_utils::{ @@ -476,12 +476,12 @@ fn vp_multitoken(c: &mut Criterion) { let transfer = shell.generate_tx( TX_TRANSPARENT_TRANSFER_WASM, - TransparentTransfer { + TransparentTransfer(vec![TransparentTransferData { source: defaults::albert_address(), target: defaults::bertha_address(), token: address::testing::nam(), amount: Amount::native_whole(1000).native_denominated(), - }, + }]), None, None, vec![&defaults::albert_keypair()], diff --git a/crates/benches/process_wrapper.rs b/crates/benches/process_wrapper.rs index 9510ee3125..65edb93467 100644 --- a/crates/benches/process_wrapper.rs +++ b/crates/benches/process_wrapper.rs @@ -3,7 +3,9 @@ use namada::core::address; use namada::core::key::RefTo; use namada::core::storage::BlockHeight; use namada::core::time::DateTimeUtc; -use namada::token::{Amount, DenominatedAmount, TransparentTransfer}; +use namada::token::{ + Amount, DenominatedAmount, TransparentTransfer, TransparentTransferData, +}; use namada::tx::data::{Fee, WrapperTx}; use namada::tx::Authorization; use namada_apps_lib::wallet::defaults; @@ -19,12 +21,12 @@ fn process_tx(c: &mut Criterion) { let mut batched_tx = shell.generate_tx( TX_TRANSPARENT_TRANSFER_WASM, - TransparentTransfer { + TransparentTransfer(vec![TransparentTransferData { source: defaults::albert_address(), target: defaults::bertha_address(), token: address::testing::nam(), amount: Amount::native_whole(1).native_denominated(), - }, + }]), None, None, vec![&defaults::albert_keypair()], diff --git a/crates/light_sdk/src/transaction/transfer.rs b/crates/light_sdk/src/transaction/transfer.rs index 84475660ea..29846ab7e0 100644 --- a/crates/light_sdk/src/transaction/transfer.rs +++ b/crates/light_sdk/src/transaction/transfer.rs @@ -25,7 +25,7 @@ impl Transfer { amount: DenominatedAmount, args: GlobalArgs, ) -> Self { - let data = namada_sdk::token::TransparentTransfer { + let data = namada_sdk::token::TransparentTransferData { source, target, token, diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 8db6155e6f..6847bf0694 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -229,11 +229,9 @@ impl From for InputAmount { } } -/// Transparent transfer transaction arguments +/// Transparent transfer-specific arguments #[derive(Clone, Debug)] -pub struct TxTransparentTransfer { - /// Common tx arguments - pub tx: Tx, +pub struct TxTransparentTransferData { /// Transfer source address pub source: C::Address, /// Transfer target address @@ -242,6 +240,15 @@ pub struct TxTransparentTransfer { pub token: C::Address, /// Transferred token amount pub amount: InputAmount, +} + +/// Transparent transfer transaction arguments +#[derive(Clone, Debug)] +pub struct TxTransparentTransfer { + /// Common tx arguments + pub tx: Tx, + /// The transfer specific data + pub data: Vec>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -258,7 +265,7 @@ impl TxBuilder for TxTransparentTransfer { } } -impl TxTransparentTransfer { +impl TxTransparentTransferData { /// Transfer source address pub fn source(self, source: C::Address) -> Self { Self { source, ..self } @@ -278,7 +285,9 @@ impl TxTransparentTransfer { pub fn amount(self, amount: InputAmount) -> Self { Self { amount, ..self } } +} +impl TxTransparentTransfer { /// Path to the TX WASM code file pub fn tx_code_path(self, tx_code_path: PathBuf) -> Self { Self { diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 7832b23d33..f08c87f2e3 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -175,16 +175,10 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { /// arguments fn new_transparent_transfer( &self, - source: Address, - target: Address, - token: Address, - amount: InputAmount, + data: Vec, ) -> args::TxTransparentTransfer { args::TxTransparentTransfer { - source, - target, - token, - amount, + data, tx_code_path: PathBuf::from(TX_TRANSPARENT_TRANSFER_WASM), tx: self.tx_builder(), } @@ -872,9 +866,7 @@ pub mod testing { }; use namada_governance::{InitProposalData, VoteProposalData}; use namada_ibc::testing::arb_ibc_any; - use namada_token::testing::{ - arb_denominated_amount, arb_transparent_transfer, - }; + use namada_token::testing::arb_denominated_amount; use namada_token::{ ShieldedTransfer, ShieldingTransfer, TransparentTransfer, UnshieldingTransfer, @@ -890,6 +882,9 @@ pub mod testing { use prost::Message; use ripemd::Digest as RipemdDigest; use sha2::Digest; + use token::testing::{ + arb_transparent_transfer, arb_vectorized_transparent_transfer, + }; use super::*; use crate::account::tests::{arb_init_account, arb_update_account}; @@ -1093,7 +1088,7 @@ pub mod testing { pub fn arb_transparent_transfer_tx()( mut header in arb_header(), wrapper in arb_wrapper_tx(), - transfer in arb_transparent_transfer(), + transfer in arb_vectorized_transparent_transfer(10), code_hash in arb_hash(), ) -> (Tx, TxData) { header.tx_type = TxType::Wrapper(Box::new(wrapper)); diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index a6e1ef5f44..1f652ff544 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -702,6 +702,11 @@ fn format_outputs(output: &mut Vec) { } } +enum TransferSide<'a> { + Source(&'a Address), + Target(&'a Address), +} + enum TokenTransfer<'a> { Transparent(&'a token::TransparentTransfer), Shielded, @@ -710,53 +715,128 @@ enum TokenTransfer<'a> { } impl TokenTransfer<'_> { - fn source(&self) -> Option<&Address> { + fn sources(&self) -> Vec<&Address> { match self { - TokenTransfer::Transparent(transfer) => Some(&transfer.source), - TokenTransfer::Shielded => None, - TokenTransfer::Shielding(transfer) => Some(&transfer.source), - TokenTransfer::Unshielding(_) => None, + TokenTransfer::Transparent(transfers) => transfers + .0 + .iter() + .map(|transfer| &transfer.source) + .collect(), + + TokenTransfer::Shielded => Default::default(), + TokenTransfer::Shielding(transfer) => vec![&transfer.source], + TokenTransfer::Unshielding(_) => Default::default(), } } - fn target(&self) -> Option<&Address> { + fn targets(&self) -> Vec<&Address> { match self { - TokenTransfer::Transparent(transfer) => Some(&transfer.target), - TokenTransfer::Shielded => None, - TokenTransfer::Shielding(_) => None, - TokenTransfer::Unshielding(transfer) => Some(&transfer.target), + TokenTransfer::Transparent(transfers) => transfers + .0 + .iter() + .map(|transfer| &transfer.target) + .collect(), + + TokenTransfer::Shielded => Default::default(), + TokenTransfer::Shielding(_) => Default::default(), + TokenTransfer::Unshielding(transfer) => { + vec![&transfer.target] + } } } - fn token_and_amount(&self) -> Option<(&Address, DenominatedAmount)> { - match self { - TokenTransfer::Transparent(transfer) => { - Some((&transfer.token, transfer.amount)) + fn tokens_and_amounts( + &self, + address: TransferSide<'_>, + ) -> Result, Error> { + Ok(match self { + TokenTransfer::Transparent(transfers) => { + let mut map: HashMap<&Address, DenominatedAmount> = + HashMap::new(); + + for transfer in &transfers.0 { + match address { + TransferSide::Source(source) + if source == &transfer.source => + { + match map.get_mut(&transfer.token) { + Some(amount) => { + *amount = amount + .checked_add(transfer.amount) + .ok_or_else(|| { + Error::Other( + "Overflow in amount" + .to_string(), + ) + })?; + } + None => { + map.insert( + &transfer.token, + transfer.amount, + ); + } + } + } + TransferSide::Target(target) + if target == &transfer.target => + { + match map.get_mut(&transfer.token) { + Some(amount) => { + *amount = amount + .checked_add(transfer.amount) + .ok_or_else(|| { + Error::Other( + "Overflow in amount" + .to_string(), + ) + })?; + } + None => { + map.insert( + &transfer.token, + transfer.amount, + ); + } + } + } + _ => (), + } + } + + map } - TokenTransfer::Shielded => None, + TokenTransfer::Shielded => Default::default(), TokenTransfer::Shielding(transfer) => { - Some((&transfer.token, transfer.amount)) + [(&transfer.token, transfer.amount)].into_iter().collect() } TokenTransfer::Unshielding(transfer) => { - Some((&transfer.token, transfer.amount)) + [(&transfer.token, transfer.amount)].into_iter().collect() } - } + }) } } -/// Adds a Ledger output for the sender and destination for transparent and MASP -/// transactions +/// Adds a Ledger output for the senders and destinations for transparent and +/// MASP transactions async fn make_ledger_token_transfer_endpoints( tokens: &HashMap, output: &mut Vec, transfer: TokenTransfer<'_>, builder: Option<&MaspBuilder>, assets: &HashMap, -) { - if let Some(source) = transfer.source() { - output.push(format!("Sender : {}", source)); - if let Some((token, amount)) = transfer.token_and_amount() { - make_ledger_amount_addr(tokens, output, amount, token, "Sending "); +) -> Result<(), Error> { + let sources = transfer.sources(); + if !sources.is_empty() { + for source in transfer.sources() { + output.push(format!("Sender : {}", source)); + for (token, amount) in + transfer.tokens_and_amounts(TransferSide::Source(source))? + { + make_ledger_amount_addr( + tokens, output, amount, token, "Sending ", + ); + } } } else if let Some(builder) = builder { for sapling_input in builder.builder.sapling_inputs() { @@ -773,16 +853,21 @@ async fn make_ledger_token_transfer_endpoints( .await; } } - if let Some(target) = transfer.target() { - output.push(format!("Destination : {}", target)); - if let Some((token, amount)) = transfer.token_and_amount() { - make_ledger_amount_addr( - tokens, - output, - amount, - token, - "Receiving ", - ); + let targets = transfer.targets(); + if !targets.is_empty() { + for target in targets { + output.push(format!("Destination : {}", target)); + for (token, amount) in + transfer.tokens_and_amounts(TransferSide::Target(target))? + { + make_ledger_amount_addr( + tokens, + output, + amount, + token, + "Receiving ", + ); + } } } else if let Some(builder) = builder { for sapling_output in builder.builder.sapling_outputs() { @@ -799,6 +884,8 @@ async fn make_ledger_token_transfer_endpoints( .await; } } + + Ok(()) } /// Convert decimal numbers into the format used by Ledger. Specifically remove @@ -1294,7 +1381,7 @@ pub async fn to_ledger_vector( None, &HashMap::default(), ) - .await; + .await?; make_ledger_token_transfer_endpoints( &tokens, &mut tv.output_expert, @@ -1302,7 +1389,7 @@ pub async fn to_ledger_vector( None, &HashMap::default(), ) - .await; + .await?; } else if code_sec.tag == Some(TX_SHIELDED_TRANSFER_WASM.to_string()) { let transfer = token::ShieldedTransfer::try_from_slice( &tx.data(cmt) @@ -1341,7 +1428,7 @@ pub async fn to_ledger_vector( builder, &asset_types, ) - .await; + .await?; make_ledger_token_transfer_endpoints( &tokens, &mut tv.output_expert, @@ -1349,7 +1436,7 @@ pub async fn to_ledger_vector( builder, &asset_types, ) - .await; + .await?; } else if code_sec.tag == Some(TX_SHIELDING_TRANSFER_WASM.to_string()) { let transfer = token::ShieldingTransfer::try_from_slice( &tx.data(cmt) @@ -1388,7 +1475,7 @@ pub async fn to_ledger_vector( builder, &asset_types, ) - .await; + .await?; make_ledger_token_transfer_endpoints( &tokens, &mut tv.output_expert, @@ -1396,7 +1483,7 @@ pub async fn to_ledger_vector( builder, &asset_types, ) - .await; + .await?; } else if code_sec.tag == Some(TX_UNSHIELDING_TRANSFER_WASM.to_string()) { let transfer = token::UnshieldingTransfer::try_from_slice( @@ -1436,7 +1523,7 @@ pub async fn to_ledger_vector( builder, &asset_types, ) - .await; + .await?; make_ledger_token_transfer_endpoints( &tokens, &mut tv.output_expert, @@ -1444,7 +1531,7 @@ pub async fn to_ledger_vector( builder, &asset_types, ) - .await; + .await?; } else if code_sec.tag == Some(TX_IBC_WASM.to_string()) { let any_msg = Any::decode( tx.data(cmt) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index b9daabeca2..17014589a6 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -66,6 +66,7 @@ pub use namada_tx::{Authorization, *}; use num_traits::Zero; use rand_core::{OsRng, RngCore}; +use crate::args::TxTransparentTransferData; use crate::control_flow::time; use crate::error::{EncodingError, Error, QueryError, Result, TxSubmitError}; use crate::io::Io; @@ -2831,64 +2832,86 @@ pub async fn build_transparent_transfer( context: &N, args: &mut args::TxTransparentTransfer, ) -> Result<(Tx, SigningTxData)> { - let source = &args.source; - let target = &args.target; + let mut transfers = vec![]; - let default_signer = Some(source.clone()); - let signing_data = signing::aux_signing_data( - context, - &args.tx, - Some(source.clone()), - default_signer, - ) - .await?; + // Evaluate signer and fees + let (signing_data, fee_amount, updated_balance) = { + let signing_data = signing::aux_signing_data( + context, + &args.tx, + None, + // If signing keys arg is not provided assume a single transfer and + // take the source + args.data + .first() + .map(|transfer_data| transfer_data.source.clone()), + ) + .await?; - // Transparent fee payment - let (fee_amount, updated_balance) = - validate_transparent_fee(context, &args.tx, &signing_data.fee_payer) - .await - .map(|(fee_amount, updated_balance)| { - (fee_amount, Some(updated_balance)) - })?; + // Transparent fee payment + let (fee_amount, updated_balance) = validate_transparent_fee( + context, + &args.tx, + &signing_data.fee_payer, + ) + .await + .map(|(fee_amount, updated_balance)| { + (fee_amount, Some(updated_balance)) + })?; - // Check that the source address exists on chain - source_exists_or_err(source.clone(), args.tx.force, context).await?; - // Check that the target address exists on chain - target_exists_or_err(target.clone(), args.tx.force, context).await?; + (signing_data, fee_amount, updated_balance) + }; - // Validate the amount given - let validated_amount = - validate_amount(context, args.amount, &args.token, args.tx.force) + for TxTransparentTransferData { + source, + target, + token, + amount, + } in &args.data + { + // Check that the source address exists on chain + source_exists_or_err(source.clone(), args.tx.force, context).await?; + // Check that the target address exists on chain + target_exists_or_err(target.clone(), args.tx.force, context).await?; + + // Validate the amount given + let validated_amount = + validate_amount(context, amount.to_owned(), token, args.tx.force) + .await?; + + // Check the balance of the source + if let Some(updated_balance) = &updated_balance { + let check_balance = if &updated_balance.source == source + && &updated_balance.token == token + { + CheckBalance::Balance(updated_balance.post_balance) + } else { + CheckBalance::Query(balance_key(token, source)) + }; + + check_balance_too_low_err( + token, + source, + validated_amount.amount(), + check_balance, + args.tx.force, + context, + ) .await?; + } - // Check the balance of the source - if let Some(updated_balance) = updated_balance { - let check_balance = if &updated_balance.source == source - && updated_balance.token == args.token - { - CheckBalance::Balance(updated_balance.post_balance) - } else { - CheckBalance::Query(balance_key(&args.token, source)) + // Construct the corresponding transparent Transfer object + let transfer_data = token::TransparentTransferData { + source: source.to_owned(), + target: target.to_owned(), + token: token.to_owned(), + amount: validated_amount, }; - check_balance_too_low_err( - &args.token, - source, - validated_amount.amount(), - check_balance, - args.tx.force, - context, - ) - .await?; + transfers.push(transfer_data); } - // Construct the corresponding transparent Transfer object - let transfer = token::TransparentTransfer { - source: source.clone(), - target: target.clone(), - token: args.token.clone(), - amount: validated_amount, - }; + let transfer = token::TransparentTransfer(transfers); let tx = build_pow_flag( context, diff --git a/crates/tests/src/integration/ledger_tests.rs b/crates/tests/src/integration/ledger_tests.rs index 214ab32a1d..3ec0da9b0e 100644 --- a/crates/tests/src/integration/ledger_tests.rs +++ b/crates/tests/src/integration/ledger_tests.rs @@ -59,16 +59,17 @@ fn ledger_txs_and_queries() -> Result<()> { let validator_one_rpc = "http://127.0.0.1:26567"; let (node, _services) = setup::setup()?; - let transfer = token::TransparentTransfer { - source: defaults::bertha_address(), - target: defaults::albert_address(), - token: node.native_token(), - amount: token::DenominatedAmount::new( - token::Amount::native_whole(10), - token::NATIVE_MAX_DECIMAL_PLACES.into(), - ), - } - .serialize_to_vec(); + let transfer = + token::TransparentTransfer(vec![token::TransparentTransferData { + source: defaults::bertha_address(), + target: defaults::albert_address(), + token: node.native_token(), + amount: token::DenominatedAmount::new( + token::Amount::native_whole(10), + token::NATIVE_MAX_DECIMAL_PLACES.into(), + ), + }]) + .serialize_to_vec(); let tx_data_path = node.test_dir.path().join("tx.data"); std::fs::write(&tx_data_path, transfer).unwrap(); let tx_data_path = tx_data_path.to_string_lossy(); diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index c14d93f430..4a67f36518 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -67,6 +67,23 @@ where Ok(()) } +/// Arguments for a multi-party transparent token transfer +#[derive( + Debug, + Clone, + PartialEq, + BorshSerialize, + BorshDeserialize, + BorshDeserializer, + BorshSchema, + Hash, + Eq, + PartialOrd, + Serialize, + Deserialize, +)] +pub struct TransparentTransfer(pub Vec); + /// Arguments for a transparent token transfer #[derive( Debug, @@ -82,7 +99,7 @@ where Serialize, Deserialize, )] -pub struct TransparentTransfer { +pub struct TransparentTransferData { /// Source address will spend the tokens pub source: Address, /// Target address will receive the tokens @@ -178,7 +195,7 @@ pub mod testing { pub use namada_trans_token::testing::*; use proptest::prelude::*; - use super::TransparentTransfer; + use super::{TransparentTransfer, TransparentTransferData}; prop_compose! { /// Generate a transparent transfer @@ -187,8 +204,8 @@ pub mod testing { target in arb_non_internal_address(), token in arb_established_address().prop_map(Address::Established), amount in arb_denominated_amount(), - ) -> TransparentTransfer { - TransparentTransfer { + ) -> TransparentTransferData{ + TransparentTransferData { source, target, token, @@ -196,4 +213,12 @@ pub mod testing { } } } + + /// Generate a vectorized transparent transfer + pub fn arb_vectorized_transparent_transfer( + number_of_txs: usize, + ) -> impl Strategy { + proptest::collection::vec(arb_transparent_transfer(), 0..number_of_txs) + .prop_map(TransparentTransfer) + } } diff --git a/wasm/tx_transparent_transfer/src/lib.rs b/wasm/tx_transparent_transfer/src/lib.rs index 76aacc3891..0da40600d8 100644 --- a/wasm/tx_transparent_transfer/src/lib.rs +++ b/wasm/tx_transparent_transfer/src/lib.rs @@ -7,16 +7,20 @@ use namada_tx_prelude::*; #[transaction] fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { let data = ctx.get_tx_data(&tx_data)?; - let transfer = token::TransparentTransfer::try_from_slice(&data[..]) + let transfers = token::TransparentTransfer::try_from_slice(&data[..]) .wrap_err("Failed to decode token::TransparentTransfer tx data")?; - debug_log!("apply_tx called with transfer: {:#?}", transfer); + debug_log!("apply_tx called with transfer: {:#?}", transfers); - token::transfer( - ctx, - &transfer.source, - &transfer.target, - &transfer.token, - transfer.amount.amount(), - ) - .wrap_err("Token transfer failed") + for transfer in transfers.0 { + token::transfer( + ctx, + &transfer.source, + &transfer.target, + &transfer.token, + transfer.amount.amount(), + ) + .wrap_err("Token transfer failed")?; + } + + Ok(()) } From 36e215bb9ad00d333715f0293c0d449454cfca79 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 3 Jun 2024 17:58:09 +0200 Subject: [PATCH 2/9] Vectorizes masp transfers --- crates/apps_lib/src/cli.rs | 71 ++- crates/light_sdk/src/transaction/transfer.rs | 69 +-- crates/node/src/bench_utils.rs | 31 +- crates/sdk/src/args.rs | 53 +- crates/sdk/src/lib.rs | 76 +-- crates/sdk/src/masp.rs | 540 ++++++++++--------- crates/sdk/src/signing.rs | 127 +++-- crates/sdk/src/tx.rs | 249 +++++---- crates/token/src/lib.rs | 94 +++- crates/tx_prelude/src/token.rs | 3 +- wasm/tx_shielding_transfer/src/lib.rs | 24 +- wasm/tx_unshielding_transfer/src/lib.rs | 24 +- 12 files changed, 833 insertions(+), 528 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 2e59687eb1..ff0745ffc1 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -4366,13 +4366,13 @@ pub mod args { let token = TOKEN.parse(matches); let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); let tx_code_path = PathBuf::from(TX_TRANSPARENT_TRANSFER_WASM); - let data = vec![TxTransparentTransferData { source, target, token, amount, }]; + Self { tx, data, @@ -4404,14 +4404,21 @@ pub mod args { ctx: &mut Context, ) -> Result, Self::Error> { let tx = self.tx.to_sdk(ctx)?; + let mut data = vec![]; let chain_ctx = ctx.borrow_mut_chain_or_exit(); + for transfer_data in self.data { + data.push(TxShieldedTransferData { + source: chain_ctx.get_cached(&transfer_data.source), + target: chain_ctx.get(&transfer_data.target), + token: chain_ctx.get(&transfer_data.token), + amount: transfer_data.amount, + }); + } + Ok(TxShieldedTransfer:: { tx, - source: chain_ctx.get_cached(&self.source), - target: chain_ctx.get(&self.target), - token: chain_ctx.get(&self.token), - amount: self.amount, + data, tx_code_path: self.tx_code_path.to_path_buf(), }) } @@ -4425,12 +4432,16 @@ pub mod args { let token = TOKEN.parse(matches); let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); let tx_code_path = PathBuf::from(TX_SHIELDED_TRANSFER_WASM); - Self { - tx, + let data = vec![TxShieldedTransferData { source, target, token, amount, + }]; + + Self { + tx, + data, tx_code_path, } } @@ -4464,14 +4475,21 @@ pub mod args { ctx: &mut Context, ) -> Result, Self::Error> { let tx = self.tx.to_sdk(ctx)?; + let mut data = vec![]; let chain_ctx = ctx.borrow_mut_chain_or_exit(); + for transfer_data in self.data { + data.push(TxShieldingTransferData { + source: chain_ctx.get(&transfer_data.source), + token: chain_ctx.get(&transfer_data.token), + amount: transfer_data.amount, + }); + } + Ok(TxShieldingTransfer:: { tx, - source: chain_ctx.get(&self.source), + data, target: chain_ctx.get(&self.target), - token: chain_ctx.get(&self.token), - amount: self.amount, tx_code_path: self.tx_code_path.to_path_buf(), }) } @@ -4485,12 +4503,16 @@ pub mod args { let token = TOKEN.parse(matches); let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); let tx_code_path = PathBuf::from(TX_SHIELDING_TRANSFER_WASM); - Self { - tx, + let data = vec![TxShieldingTransferData { source, - target, token, amount, + }]; + + Self { + tx, + data, + target, tx_code_path, } } @@ -4525,14 +4547,21 @@ pub mod args { ctx: &mut Context, ) -> Result, Self::Error> { let tx = self.tx.to_sdk(ctx)?; + let mut data = vec![]; let chain_ctx = ctx.borrow_mut_chain_or_exit(); + for transfer_data in self.data { + data.push(TxUnshieldingTransferData { + target: chain_ctx.get(&transfer_data.target), + token: chain_ctx.get(&transfer_data.token), + amount: transfer_data.amount, + }); + } + Ok(TxUnshieldingTransfer:: { tx, + data, source: chain_ctx.get_cached(&self.source), - target: chain_ctx.get(&self.target), - token: chain_ctx.get(&self.token), - amount: self.amount, tx_code_path: self.tx_code_path.to_path_buf(), }) } @@ -4546,12 +4575,16 @@ pub mod args { let token = TOKEN.parse(matches); let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); let tx_code_path = PathBuf::from(TX_UNSHIELDING_TRANSFER_WASM); - Self { - tx, - source, + let data = vec![TxUnshieldingTransferData { target, token, amount, + }]; + + Self { + tx, + source, + data, tx_code_path, } } diff --git a/crates/light_sdk/src/transaction/transfer.rs b/crates/light_sdk/src/transaction/transfer.rs index 29846ab7e0..9dc7aa8e45 100644 --- a/crates/light_sdk/src/transaction/transfer.rs +++ b/crates/light_sdk/src/transaction/transfer.rs @@ -1,7 +1,11 @@ use namada_sdk::address::Address; use namada_sdk::hash::Hash; use namada_sdk::key::common; -use namada_sdk::token::DenominatedAmount; +use namada_sdk::token::transaction::Transaction; +use namada_sdk::token::ShieldingTransferData; +pub use namada_sdk::token::{ + DenominatedAmount, TransparentTransfer, UnshieldingTransferData, +}; use namada_sdk::tx::data::GasLimit; use namada_sdk::tx::{ Authorization, Tx, TxError, TX_SHIELDED_TRANSFER_WASM, @@ -19,81 +23,78 @@ pub struct Transfer(Tx); impl Transfer { /// Build a transparent transfer transaction from the given parameters pub fn transparent( - source: Address, - target: Address, - token: Address, - amount: DenominatedAmount, + transfers: TransparentTransfer, args: GlobalArgs, ) -> Self { - let data = namada_sdk::token::TransparentTransferData { - source, - target, - token, - amount, - }; - Self(transaction::build_tx( args, - data, + transfers, TX_TRANSPARENT_TRANSFER_WASM.to_string(), )) } /// Build a shielded transfer transaction from the given parameters - pub fn shielded(shielded_section_hash: Hash, args: GlobalArgs) -> Self { + pub fn shielded( + shielded_section_hash: Hash, + transaction: Transaction, + args: GlobalArgs, + ) -> Self { let data = namada_sdk::token::ShieldedTransfer { section_hash: shielded_section_hash, }; - Self(transaction::build_tx( + let mut tx = transaction::build_tx( args, data, TX_SHIELDED_TRANSFER_WASM.to_string(), - )) + ); + tx.add_masp_tx_section(transaction); + + Self(tx) } /// Build a shielding transfer transaction from the given parameters pub fn shielding( - source: Address, - token: Address, - amount: DenominatedAmount, + transfers: Vec, shielded_section_hash: Hash, + transaction: Transaction, args: GlobalArgs, ) -> Self { - let data = namada_sdk::token::ShieldingTransfer { - source, - token, - amount, + let data = namada_sdk::token::ShieldingMultiTransfer { + data: transfers, shielded_section_hash, }; - Self(transaction::build_tx( + let mut tx = transaction::build_tx( args, data, TX_SHIELDING_TRANSFER_WASM.to_string(), - )) + ); + tx.add_masp_tx_section(transaction); + + Self(tx) } /// Build an unshielding transfer transaction from the given parameters pub fn unshielding( - target: Address, - token: Address, - amount: DenominatedAmount, + transfers: Vec, shielded_section_hash: Hash, + transaction: Transaction, args: GlobalArgs, ) -> Self { - let data = namada_sdk::token::UnshieldingTransfer { - target, - token, - amount, + let data = namada_sdk::token::UnshieldingMultiTransfer { + data: transfers, shielded_section_hash, }; - Self(transaction::build_tx( + let mut tx = transaction::build_tx( args, data, TX_UNSHIELDING_TRANSFER_WASM.to_string(), - )) + ); + tx.add_masp_tx_section(transaction); + + Self(tx) } /// Get the bytes to sign for the given transaction diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index ef80036715..9a910df04e 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -77,7 +77,7 @@ use namada::masp::MaspTxRefs; use namada::state::StorageRead; use namada::token::{ Amount, DenominatedAmount, ShieldedTransfer, ShieldingTransfer, - UnshieldingTransfer, + ShieldingTransferData, UnshieldingTransfer, UnshieldingTransferData, }; use namada::tx::data::pos::Bond; use namada::tx::data::{ @@ -94,7 +94,7 @@ use namada_apps_lib::cli::context::FromContext; use namada_apps_lib::cli::Context; use namada_apps_lib::wallet::{defaults, CliWalletUtils}; use namada_sdk::masp::{ - self, ContextSyncStatus, ShieldedContext, ShieldedUtils, + self, ContextSyncStatus, MaspTransferData, ShieldedContext, ShieldedUtils, }; pub use namada_sdk::tx::{ TX_BECOME_VALIDATOR_WASM, TX_BOND_WASM, TX_BRIDGE_POOL_WASM, @@ -1080,14 +1080,17 @@ impl BenchShieldedCtx { StdIo, native_token, ); + let masp_transfer_data = MaspTransferData { + source: source.clone(), + target: target.clone(), + token: address::testing::nam(), + amount: denominated_amount, + }; let shielded = async_runtime .block_on( ShieldedContext::::gen_shielded_transfer( &namada, - &source, - &target, - &address::testing::nam(), - denominated_amount, + vec![masp_transfer_data], true, ), ) @@ -1125,9 +1128,11 @@ impl BenchShieldedCtx { namada.client().generate_tx( TX_SHIELDING_TRANSFER_WASM, ShieldingTransfer { - source: source.effective_address(), - token: address::testing::nam(), - amount: DenominatedAmount::native(amount), + data: ShieldingTransferData { + source: source.effective_address(), + token: address::testing::nam(), + amount: DenominatedAmount::native(amount), + }, shielded_section_hash, }, Some(shielded), @@ -1138,9 +1143,11 @@ impl BenchShieldedCtx { namada.client().generate_tx( TX_UNSHIELDING_TRANSFER_WASM, UnshieldingTransfer { - target: target.effective_address(), - token: address::testing::nam(), - amount: DenominatedAmount::native(amount), + data: UnshieldingTransferData { + target: target.effective_address(), + token: address::testing::nam(), + amount: DenominatedAmount::native(amount), + }, shielded_section_hash, }, Some(shielded), diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 6847bf0694..7cc775829a 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -307,11 +307,9 @@ impl TxTransparentTransfer { } } -/// Shielded transfer transaction arguments +/// Shielded transfer-specific arguments #[derive(Clone, Debug)] -pub struct TxShieldedTransfer { - /// Common tx arguments - pub tx: Tx, +pub struct TxShieldedTransferData { /// Transfer source spending key pub source: C::SpendingKey, /// Transfer target address @@ -320,6 +318,15 @@ pub struct TxShieldedTransfer { pub token: C::Address, /// Transferred token amount pub amount: InputAmount, +} + +/// Shielded transfer transaction arguments +#[derive(Clone, Debug)] +pub struct TxShieldedTransfer { + /// Common tx arguments + pub tx: Tx, + /// Transfer-specific data + pub data: Vec>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -334,19 +341,26 @@ impl TxShieldedTransfer { } } +/// Shielded transfer-specific arguments +#[derive(Clone, Debug)] +pub struct TxShieldingTransferData { + /// Transfer source spending key + pub source: C::Address, + /// Transferred token address + pub token: C::Address, + /// Transferred token amount + pub amount: InputAmount, +} + /// Shielding transfer transaction arguments #[derive(Clone, Debug)] pub struct TxShieldingTransfer { /// Common tx arguments pub tx: Tx, - /// Transfer source address - pub source: C::Address, /// Transfer target address pub target: C::PaymentAddress, - /// Transferred token address - pub token: C::Address, - /// Transferred token amount - pub amount: InputAmount, + /// Transfer-specific data + pub data: Vec>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } @@ -361,19 +375,26 @@ impl TxShieldingTransfer { } } -/// Unshielding transfer transaction arguments +/// Unshielding transfer-specific arguments #[derive(Clone, Debug)] -pub struct TxUnshieldingTransfer { - /// Common tx arguments - pub tx: Tx, - /// Transfer source spending key - pub source: C::SpendingKey, +pub struct TxUnshieldingTransferData { /// Transfer target address pub target: C::Address, /// Transferred token address pub token: C::Address, /// Transferred token amount pub amount: InputAmount, +} + +/// Unshielding transfer transaction arguments +#[derive(Clone, Debug)] +pub struct TxUnshieldingTransfer { + /// Common tx arguments + pub tx: Tx, + /// Transfer source spending key + pub source: C::SpendingKey, + /// Transfer-specific data + pub data: Vec>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, } diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index f08c87f2e3..c4778e285d 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -188,16 +188,10 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { /// arguments fn new_shielded_transfer( &self, - source: ExtendedSpendingKey, - target: PaymentAddress, - token: Address, - amount: InputAmount, + data: Vec, ) -> args::TxShieldedTransfer { args::TxShieldedTransfer { - source, - target, - token, - amount, + data, tx_code_path: PathBuf::from(TX_SHIELDED_TRANSFER_WASM), tx: self.tx_builder(), } @@ -207,16 +201,12 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { /// arguments fn new_shielding_transfer( &self, - source: Address, target: PaymentAddress, - token: Address, - amount: InputAmount, + data: Vec, ) -> args::TxShieldingTransfer { args::TxShieldingTransfer { - source, + data, target, - token, - amount, tx_code_path: PathBuf::from(TX_SHIELDING_TRANSFER_WASM), tx: self.tx_builder(), } @@ -227,15 +217,11 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { fn new_unshielding_transfer( &self, source: ExtendedSpendingKey, - target: Address, - token: Address, - amount: InputAmount, + data: Vec, ) -> args::TxUnshieldingTransfer { args::TxUnshieldingTransfer { source, - target, - token, - amount, + data, tx_code_path: PathBuf::from(TX_UNSHIELDING_TRANSFER_WASM), tx: self.tx_builder(), } @@ -627,8 +613,8 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { args: &args::Tx, signing_data: SigningTxData, with: impl Fn(Tx, common::PublicKey, HashSet, D) -> F - + MaybeSend - + MaybeSync, + + MaybeSend + + MaybeSync, user_data: D, ) -> crate::error::Result<()> where @@ -867,10 +853,7 @@ pub mod testing { use namada_governance::{InitProposalData, VoteProposalData}; use namada_ibc::testing::arb_ibc_any; use namada_token::testing::arb_denominated_amount; - use namada_token::{ - ShieldedTransfer, ShieldingTransfer, TransparentTransfer, - UnshieldingTransfer, - }; + use namada_token::{ShieldedTransfer, TransparentTransfer}; use namada_tx::data::pgf::UpdateStewardCommission; use namada_tx::data::pos::{ BecomeValidator, Bond, CommissionChange, ConsensusKeyChange, @@ -882,8 +865,10 @@ pub mod testing { use prost::Message; use ripemd::Digest as RipemdDigest; use sha2::Digest; - use token::testing::{ - arb_transparent_transfer, arb_vectorized_transparent_transfer, + use token::testing::arb_vectorized_transparent_transfer; + use token::{ + ShieldingMultiTransfer, ShieldingTransferData, + UnshieldingMultiTransfer, UnshieldingTransferData, }; use super::*; @@ -928,8 +913,11 @@ pub mod testing { Withdraw(Withdraw), TransparentTransfer(TransparentTransfer), ShieldedTransfer(ShieldedTransfer, (StoredBuildParams, String)), - ShieldingTransfer(ShieldingTransfer, (StoredBuildParams, String)), - UnshieldingTransfer(UnshieldingTransfer, (StoredBuildParams, String)), + ShieldingTransfer(ShieldingMultiTransfer, (StoredBuildParams, String)), + UnshieldingTransfer( + UnshieldingMultiTransfer, + (StoredBuildParams, String), + ), Bond(Bond), Redelegation(Redelegation), UpdateStewardCommission(UpdateStewardCommission), @@ -1122,17 +1110,17 @@ pub mod testing { } prop_compose! { - /// Generate an arbitrary transfer transaction - pub fn arb_masp_transfer_tx()(transfer in arb_transparent_transfer())( + /// Generate an arbitrary masp transfer transaction + pub fn arb_masp_transfer_tx()(transfers in arb_vectorized_transparent_transfer(5))( mut header in arb_header(), wrapper in arb_wrapper_tx(), code_hash in arb_hash(), (masp_tx_type, (shielded_transfer, asset_types, build_params)) in prop_oneof![ (Just(MaspTxType::Shielded), arb_shielded_transfer(0..MAX_ASSETS)), - (Just(MaspTxType::Shielding), arb_shielding_transfer(encode_address(&transfer.source), 1)), - (Just(MaspTxType::Unshielding), arb_deshielding_transfer(encode_address(&transfer.target), 1)), + (Just(MaspTxType::Shielding), arb_shielding_transfer(encode_address(&transfers.0.first().unwrap().source), 1)), + (Just(MaspTxType::Unshielding), arb_deshielding_transfer(encode_address(&transfers.0.first().unwrap().target), 1)), ], - transfer in Just(transfer), + transfers in Just(transfers), ) -> (Tx, TxData) { header.tx_type = TxType::Wrapper(Box::new(wrapper)); let mut tx = Tx { header, sections: vec![] }; @@ -1155,7 +1143,14 @@ pub mod testing { decoded.denom, ); tx.add_code_from_hash(code_hash, Some(TX_SHIELDING_TRANSFER_WASM.to_owned())); - let data = ShieldingTransfer {source: transfer.source, token, amount, shielded_section_hash }; + let data = transfers.0.into_iter().map(|transfer| + ShieldingTransferData{ + source: transfer.source, + token: token.clone(), + amount + } + ).collect(); + let data = ShieldingMultiTransfer{data, shielded_section_hash }; tx.add_data(data.clone()); TxData::ShieldingTransfer(data, (build_params, build_param_bytes)) }, @@ -1168,7 +1163,14 @@ pub mod testing { decoded.denom, ); tx.add_code_from_hash(code_hash, Some(TX_UNSHIELDING_TRANSFER_WASM.to_owned())); - let data = UnshieldingTransfer {target: transfer.target, token, amount, shielded_section_hash }; + let data = transfers.0.into_iter().map(|transfer| + UnshieldingTransferData{ + target: transfer.target, + token: token.clone(), + amount + } + ).collect(); + let data = UnshieldingMultiTransfer{data, shielded_section_hash }; tx.add_data(data.clone()); TxData::UnshieldingTransfer(data, (build_params, build_param_bytes)) }, diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index 516a7ad611..0ae8d00e3b 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -121,6 +121,16 @@ pub struct ShieldedTransfer { pub epoch: MaspEpoch, } +/// The data for a single masp transfer +#[allow(missing_docs)] +#[derive(Debug)] +pub struct MaspTransferData { + pub source: TransferSource, + pub target: TransferTarget, + pub token: Address, + pub amount: token::DenominatedAmount, +} + /// Shielded pool data for a token #[allow(missing_docs)] #[derive(Debug, BorshSerialize, BorshDeserialize, BorshDeserializer)] @@ -134,11 +144,17 @@ pub struct MaspTokenRewardData { } /// A return type for gen_shielded_transfer +#[allow(clippy::large_enum_variant)] #[derive(Error, Debug)] pub enum TransferErr { /// Build error for masp errors - #[error("{0}")] - Build(#[from] builder::Error), + #[error("{error}")] + Build { + /// The error + error: builder::Error, + /// The optional associated transfer data for logging purposes + data: Option, + }, /// errors #[error("{0}")] General(#[from] Error), @@ -1515,36 +1531,9 @@ impl ShieldedContext { /// amounts and signatures specified by the containing Transfer object. pub async fn gen_shielded_transfer( context: &impl Namada, - source: &TransferSource, - target: &TransferTarget, - token: &Address, - amount: token::DenominatedAmount, + data: Vec, update_ctx: bool, ) -> Result, TransferErr> { - // No shielded components are needed when neither source nor destination - // are shielded - - let spending_key = source.spending_key(); - let payment_address = target.payment_address(); - // No shielded components are needed when neither source nor - // destination are shielded - if spending_key.is_none() && payment_address.is_none() { - return Ok(None); - } - // We want to fund our transaction solely from supplied spending key - let spending_key = spending_key.map(|x| x.into()); - { - // Load the current shielded context given the spending key we - // possess - let mut shielded = context.shielded_mut().await; - let _ = shielded.load().await; - } - // Determine epoch in which to submit potential shielded transaction - let epoch = rpc::query_masp_epoch(context.client()).await?; - // Context required for storing which notes are in the source's - // possession - let memo = MemoBytes::empty(); - // Try to get a seed from env var, if any. #[allow(unused_mut)] let mut rng = StdRng::from_rng(OsRng).unwrap(); @@ -1567,7 +1556,6 @@ impl ShieldedContext { rng }; - // Now we build up the transaction within this object // TODO: if the user requested the default expiration, there might be a // small discrepancy between the datetime we calculate here and the one // we set for the transaction. This should be small enough to not cause @@ -1621,235 +1609,305 @@ impl ShieldedContext { // use from the masp crate to specify the expiration better expiration_height.into(), ); + // Determine epoch in which to submit potential shielded transaction + let epoch = rpc::query_masp_epoch(context.client()).await?; - // Convert transaction amount into MASP types - let Some(denom) = query_denom(context.client(), token).await else { - return Err(TransferErr::General(Error::from( - QueryError::General(format!("denomination for token {token}")), - ))); - }; - let (asset_types, masp_amount) = { - let mut shielded = context.shielded_mut().await; - // Do the actual conversion to an asset type - let amount = shielded - .convert_amount( - context.client(), - epoch, - token, - denom, - amount.amount(), - ) - .await?; - // Make sure to save any decodings of the asset types used so that - // balance queries involving them are successful - let _ = shielded.save().await; - amount - }; - - // If there are shielded inputs - if let Some(sk) = spending_key { - // Locate unspent notes that can help us meet the transaction amount - let (_, unspent_notes, used_convs) = context - .shielded_mut() - .await - .collect_unspent_notes( - context, - &to_viewing_key(&sk).vk, - I128Sum::from_sum(masp_amount), - epoch, - ) - .await?; - // Commit the notes found to our transaction - for (diversifier, note, merkle_path) in unspent_notes { - builder - .add_sapling_spend(sk, diversifier, note, merkle_path) - .map_err(builder::Error::SaplingBuild)?; - } - // Commit the conversion notes used during summation - for (conv, wit, value) in used_convs.values() { - if value.is_positive() { - builder - .add_sapling_convert( - conv.clone(), - *value as u64, - wit.clone(), - ) - .map_err(builder::Error::SaplingBuild)?; - } + for MaspTransferData { + source, + target, + token, + amount, + } in data + { + let spending_key = source.spending_key(); + let payment_address = target.payment_address(); + // No shielded components are needed when neither source nor + // destination are shielded + if spending_key.is_none() && payment_address.is_none() { + return Ok(None); } - } else { - // We add a dummy UTXO to our transaction, but only the source of - // the parent Transfer object is used to validate fund - // availability - let source_enc = source - .address() - .ok_or_else(|| { - Error::Other( - "source address should be transparent".to_string(), - ) - })? - .serialize_to_vec(); - - let hash = ripemd::Ripemd160::digest(sha2::Sha256::digest( - source_enc.as_ref(), - )); - let script = TransparentAddress(hash.into()); - for (digit, asset_type) in - MaspDigitPos::iter().zip(asset_types.iter()) + // We want to fund our transaction solely from supplied spending key + let spending_key = spending_key.map(|x| x.into()); { - let amount_part = digit.denominate(&amount.amount()); - // Skip adding an input if its value is 0 - if amount_part != 0 { - builder - .add_transparent_input(TxOut { - asset_type: *asset_type, - value: amount_part, - address: script, - }) - .map_err(builder::Error::TransparentBuild)?; - } + // Load the current shielded context given the spending key we + // possess + let mut shielded = context.shielded_mut().await; + let _ = shielded.load().await; } - } + // Context required for storing which notes are in the source's + // possession + let memo = MemoBytes::empty(); - // Anotate the asset type in the value balance with its decoding in - // order to facilitate cross-epoch computations - let value_balance = builder.value_balance(); - let value_balance = context - .shielded_mut() - .await - .decode_sum(context.client(), value_balance) - .await; + // Now we build up the transaction within this object - // If we are sending to a transparent output, then we will need to embed - // the transparent target address into the shielded transaction so that - // it can be signed - let transparent_target_hash = if payment_address.is_none() { - let target_enc = target - .address() - .ok_or_else(|| { - Error::Other( - "target address should be transparent".to_string(), + // Convert transaction amount into MASP types + let Some(denom) = query_denom(context.client(), &token).await + else { + return Err(TransferErr::General(Error::from( + QueryError::General(format!( + "denomination for token {token}" + )), + ))); + }; + let (asset_types, masp_amount) = { + let mut shielded = context.shielded_mut().await; + // Do the actual conversion to an asset type + let amount = shielded + .convert_amount( + context.client(), + epoch, + &token, + denom, + amount.amount(), ) - })? - .serialize_to_vec(); - Some(ripemd::Ripemd160::digest(sha2::Sha256::digest( - target_enc.as_ref(), - ))) - } else { - None - }; - // This indicates how many more assets need to be sent to the receiver - // in order to satisfy the requested transfer amount. - let mut rem_amount = amount.amount().raw_amount().0; - // If we are sending to a shielded address, we may need the outgoing - // viewing key in the following computations. - let ovk_opt = spending_key.map(|x| x.expsk.ovk); - - // Now handle the outputs of this transaction - // Loop through the value balance components and see which - // ones can be given to the receiver - for ((asset_type, decoded), val) in value_balance.components() { - let rem_amount = &mut rem_amount[decoded.position as usize]; - // Only asset types with the correct token can contribute. But - // there must be a demonstrated need for it. - if decoded.token == *token - && decoded.denom == denom - && decoded.epoch.map_or(true, |vbal_epoch| vbal_epoch <= epoch) - && *rem_amount > 0 - { - let val = u128::try_from(*val).expect( - "value balance in absence of output descriptors should be \ - non-negative", - ); - // We want to take at most the remaining quota for the - // current denomination to the receiver - let contr = std::cmp::min(*rem_amount as u128, val) as u64; - // Make transaction output tied to the current token, - // denomination, and epoch. - if let Some(pa) = payment_address { - // If there is a shielded output - builder - .add_sapling_output( - ovk_opt, - pa.into(), - *asset_type, - contr, - memo.clone(), - ) - .map_err(builder::Error::SaplingBuild)?; - } else { - // If there is a transparent output - let hash = transparent_target_hash - .expect( - "transparent target hash should have been \ - computed already", - ) - .into(); + .await?; + // Make sure to save any decodings of the asset types used so + // that balance queries involving them are + // successful + let _ = shielded.save().await; + amount + }; + + // If there are shielded inputs + if let Some(sk) = spending_key { + // Locate unspent notes that can help us meet the transaction + // amount + let (_, unspent_notes, used_convs) = context + .shielded_mut() + .await + .collect_unspent_notes( + context, + &to_viewing_key(&sk).vk, + I128Sum::from_sum(masp_amount), + epoch, + ) + .await?; + // Commit the notes found to our transaction + for (diversifier, note, merkle_path) in unspent_notes { builder - .add_transparent_output( - &TransparentAddress(hash), - *asset_type, - contr, + .add_sapling_spend(sk, diversifier, note, merkle_path) + .map_err(|e| TransferErr::Build { + error: builder::Error::SaplingBuild(e), + data: None, + })?; + } + // Commit the conversion notes used during summation + for (conv, wit, value) in used_convs.values() { + if value.is_positive() { + builder + .add_sapling_convert( + conv.clone(), + *value as u64, + wit.clone(), + ) + .map_err(|e| TransferErr::Build { + error: builder::Error::SaplingBuild(e), + data: None, + })?; + } + } + } else { + // We add a dummy UTXO to our transaction, but only the source + // of the parent Transfer object is used to + // validate fund availability + let source_enc = source + .address() + .ok_or_else(|| { + Error::Other( + "source address should be transparent".to_string(), ) - .map_err(builder::Error::TransparentBuild)?; + })? + .serialize_to_vec(); + + let hash = ripemd::Ripemd160::digest(sha2::Sha256::digest( + source_enc.as_ref(), + )); + let script = TransparentAddress(hash.into()); + for (digit, asset_type) in + MaspDigitPos::iter().zip(asset_types.iter()) + { + let amount_part = digit.denominate(&amount.amount()); + // Skip adding an input if its value is 0 + if amount_part != 0 { + builder + .add_transparent_input(TxOut { + asset_type: *asset_type, + value: amount_part, + address: script, + }) + .map_err(|e| TransferErr::Build { + error: builder::Error::TransparentBuild(e), + data: None, + })?; + } } - // Lower what is required of the remaining contribution - *rem_amount -= contr; } - } - // Nothing must remain to be included in output - if rem_amount != [0; 4] { - // Convert the shortfall into a I128Sum - let mut shortfall = I128Sum::zero(); - for (asset_type, val) in asset_types.iter().zip(rem_amount) { - shortfall += I128Sum::from_pair(*asset_type, val.into()); - } - // Return an insufficient ffunds error - return Result::Err(TransferErr::from( - builder::Error::InsufficientFunds(shortfall), - )); - } + // Anotate the asset type in the value balance with its decoding in + // order to facilitate cross-epoch computations + let value_balance = builder.value_balance(); + let value_balance = context + .shielded_mut() + .await + .decode_sum(context.client(), value_balance) + .await; - // Now add outputs representing the change from this payment - if let Some(sk) = spending_key { - // Represents the amount of inputs we are short by - let mut additional = I128Sum::zero(); - for (asset_type, amt) in builder.value_balance().components() { - match amt.cmp(&0) { - Ordering::Greater => { - // Send the change in this asset type back to the sender + // If we are sending to a transparent output, then we will need to + // embed the transparent target address into the + // shielded transaction so that it can be signed + let transparent_target_hash = if payment_address.is_none() { + let target_enc = target + .address() + .ok_or_else(|| { + Error::Other( + "target address should be transparent".to_string(), + ) + })? + .serialize_to_vec(); + Some(ripemd::Ripemd160::digest(sha2::Sha256::digest( + target_enc.as_ref(), + ))) + } else { + None + }; + // This indicates how many more assets need to be sent to the + // receiver in order to satisfy the requested transfer + // amount. + let mut rem_amount = amount.amount().raw_amount().0; + // If we are sending to a shielded address, we may need the outgoing + // viewing key in the following computations. + let ovk_opt = spending_key.map(|x| x.expsk.ovk); + + // Now handle the outputs of this transaction + // Loop through the value balance components and see which + // ones can be given to the receiver + for ((asset_type, decoded), val) in value_balance.components() { + let rem_amount = &mut rem_amount[decoded.position as usize]; + // Only asset types with the correct token can contribute. But + // there must be a demonstrated need for it. + if decoded.token == token + && decoded.denom == denom + && decoded + .epoch + .map_or(true, |vbal_epoch| vbal_epoch <= epoch) + && *rem_amount > 0 + { + let val = u128::try_from(*val).expect( + "value balance in absence of output descriptors \ + should be non-negative", + ); + // We want to take at most the remaining quota for the + // current denomination to the receiver + let contr = std::cmp::min(*rem_amount as u128, val) as u64; + // Make transaction output tied to the current token, + // denomination, and epoch. + if let Some(pa) = payment_address { + // If there is a shielded output builder .add_sapling_output( - Some(sk.expsk.ovk), - sk.default_address().1, + ovk_opt, + pa.into(), *asset_type, - *amt as u64, + contr, memo.clone(), ) - .map_err(builder::Error::SaplingBuild)?; + .map_err(|e| TransferErr::Build { + error: builder::Error::SaplingBuild(e), + data: None, + })?; + } else { + // If there is a transparent output + let hash = transparent_target_hash + .expect( + "transparent target hash should have been \ + computed already", + ) + .into(); + builder + .add_transparent_output( + &TransparentAddress(hash), + *asset_type, + contr, + ) + .map_err(|e| TransferErr::Build { + error: builder::Error::TransparentBuild(e), + data: None, + })?; } - Ordering::Less => { - // Record how much of the current asset type we are - // short by - additional += - I128Sum::from_nonnegative(*asset_type, -*amt) - .map_err(|()| { + // Lower what is required of the remaining contribution + *rem_amount -= contr; + } + } + + // Nothing must remain to be included in output + if rem_amount != [0; 4] { + // Convert the shortfall into a I128Sum + let mut shortfall = I128Sum::zero(); + for (asset_type, val) in asset_types.iter().zip(rem_amount) { + shortfall += I128Sum::from_pair(*asset_type, val.into()); + } + // Return an insufficient funds error + return Result::Err(TransferErr::Build { + error: builder::Error::InsufficientFunds(shortfall), + data: Some(MaspTransferData { + source, + target, + token, + amount, + }), + }); + } + + // Now add outputs representing the change from this payment + if let Some(sk) = spending_key { + // Represents the amount of inputs we are short by + let mut additional = I128Sum::zero(); + for (asset_type, amt) in builder.value_balance().components() { + match amt.cmp(&0) { + Ordering::Greater => { + // Send the change in this asset type back to the + // sender + builder + .add_sapling_output( + Some(sk.expsk.ovk), + sk.default_address().1, + *asset_type, + *amt as u64, + memo.clone(), + ) + .map_err(|e| TransferErr::Build { + error: builder::Error::SaplingBuild(e), + data: None, + })?; + } + Ordering::Less => { + // Record how much of the current asset type we are + // short by + additional += + I128Sum::from_nonnegative(*asset_type, -*amt) + .map_err(|()| { Error::Other(format!( "from non negative conversion: {}", line!() )) })?; + } + Ordering::Equal => {} } - Ordering::Equal => {} } - } - // If we are short by a non-zero amount, then we have insufficient - // funds - if !additional.is_zero() { - return Err(TransferErr::from( - builder::Error::InsufficientFunds(additional), - )); + // If we are short by a non-zero amount, then we have + // insufficient funds + if !additional.is_zero() { + return Result::Err(TransferErr::Build { + error: builder::Error::InsufficientFunds(additional), + data: Some(MaspTransferData { + source, + target, + token, + amount, + }), + }); + } } } @@ -1859,12 +1917,14 @@ impl ShieldedContext { let prover = context.shielded().await.utils.local_tx_prover(); #[cfg(feature = "testing")] let prover = testing::MockTxProver(std::sync::Mutex::new(OsRng)); - let (masp_tx, metadata) = builder.build( - &prover, - &FeeRule::non_standard(U64Sum::zero()), - &mut rng, - &mut RngBuildParams::new(OsRng), - )?; + let (masp_tx, metadata) = builder + .build( + &prover, + &FeeRule::non_standard(U64Sum::zero()), + &mut rng, + &mut RngBuildParams::new(OsRng), + ) + .map_err(|error| TransferErr::Build { error, data: None })?; if update_ctx { // Cache the generated transfer diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index 1f652ff544..a92370a3b3 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -710,8 +710,8 @@ enum TransferSide<'a> { enum TokenTransfer<'a> { Transparent(&'a token::TransparentTransfer), Shielded, - Shielding(&'a token::ShieldingTransfer), - Unshielding(&'a token::UnshieldingTransfer), + Shielding(&'a token::ShieldingMultiTransfer), + Unshielding(&'a token::UnshieldingMultiTransfer), } impl TokenTransfer<'_> { @@ -722,9 +722,12 @@ impl TokenTransfer<'_> { .iter() .map(|transfer| &transfer.source) .collect(), - TokenTransfer::Shielded => Default::default(), - TokenTransfer::Shielding(transfer) => vec![&transfer.source], + TokenTransfer::Shielding(transfers) => transfers + .data + .iter() + .map(|transfer| &transfer.source) + .collect(), TokenTransfer::Unshielding(_) => Default::default(), } } @@ -739,9 +742,11 @@ impl TokenTransfer<'_> { TokenTransfer::Shielded => Default::default(), TokenTransfer::Shielding(_) => Default::default(), - TokenTransfer::Unshielding(transfer) => { - vec![&transfer.target] - } + TokenTransfer::Unshielding(transfers) => transfers + .data + .iter() + .map(|transfer| &transfer.target) + .collect(), } } @@ -759,46 +764,20 @@ impl TokenTransfer<'_> { TransferSide::Source(source) if source == &transfer.source => { - match map.get_mut(&transfer.token) { - Some(amount) => { - *amount = amount - .checked_add(transfer.amount) - .ok_or_else(|| { - Error::Other( - "Overflow in amount" - .to_string(), - ) - })?; - } - None => { - map.insert( - &transfer.token, - transfer.amount, - ); - } - } + Self::update_token_amount_map( + &mut map, + &transfer.token, + transfer.amount, + )?; } TransferSide::Target(target) if target == &transfer.target => { - match map.get_mut(&transfer.token) { - Some(amount) => { - *amount = amount - .checked_add(transfer.amount) - .ok_or_else(|| { - Error::Other( - "Overflow in amount" - .to_string(), - ) - })?; - } - None => { - map.insert( - &transfer.token, - transfer.amount, - ); - } - } + Self::update_token_amount_map( + &mut map, + &transfer.token, + transfer.amount, + )?; } _ => (), } @@ -807,14 +786,64 @@ impl TokenTransfer<'_> { map } TokenTransfer::Shielded => Default::default(), - TokenTransfer::Shielding(transfer) => { - [(&transfer.token, transfer.amount)].into_iter().collect() + TokenTransfer::Shielding(transfers) => { + let mut map: HashMap<&Address, DenominatedAmount> = + HashMap::new(); + + if let TransferSide::Source(source_addr) = address { + for transfer in &transfers.data { + if &transfer.source == source_addr { + Self::update_token_amount_map( + &mut map, + &transfer.token, + transfer.amount, + )?; + } + } + } + + map } - TokenTransfer::Unshielding(transfer) => { - [(&transfer.token, transfer.amount)].into_iter().collect() + TokenTransfer::Unshielding(transfers) => { + let mut map: HashMap<&Address, DenominatedAmount> = + HashMap::new(); + + if let TransferSide::Target(target_addr) = address { + for transfer in &transfers.data { + if &transfer.target == target_addr { + Self::update_token_amount_map( + &mut map, + &transfer.token, + transfer.amount, + )?; + } + } + } + + map } }) } + + fn update_token_amount_map<'a>( + map: &mut HashMap<&'a Address, DenominatedAmount>, + token: &'a Address, + amount: DenominatedAmount, + ) -> Result<(), Error> { + match map.get_mut(token) { + Some(prev_amount) => { + *prev_amount = + prev_amount.checked_add(amount).ok_or_else(|| { + Error::Other("Overflow in amount".to_string()) + })?; + } + None => { + map.insert(token, amount); + } + } + + Ok(()) + } } /// Adds a Ledger output for the senders and destinations for transparent and @@ -1438,7 +1467,7 @@ pub async fn to_ledger_vector( ) .await?; } else if code_sec.tag == Some(TX_SHIELDING_TRANSFER_WASM.to_string()) { - let transfer = token::ShieldingTransfer::try_from_slice( + let transfer = token::ShieldingMultiTransfer::try_from_slice( &tx.data(cmt) .ok_or_else(|| Error::Other("Invalid Data".to_string()))?, ) @@ -1486,7 +1515,7 @@ pub async fn to_ledger_vector( .await?; } else if code_sec.tag == Some(TX_UNSHIELDING_TRANSFER_WASM.to_string()) { - let transfer = token::UnshieldingTransfer::try_from_slice( + let transfer = token::UnshieldingMultiTransfer::try_from_slice( &tx.data(cmt) .ok_or_else(|| Error::Other("Invalid Data".to_string()))?, ) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 17014589a6..4100911fb3 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -65,13 +65,17 @@ use namada_tx::data::{pos, BatchedTxResult, ResultCode, TxResult}; pub use namada_tx::{Authorization, *}; use num_traits::Zero; use rand_core::{OsRng, RngCore}; +use token::ShieldingTransferData; -use crate::args::TxTransparentTransferData; +use crate::args::{ + TxShieldedTransferData, TxShieldingTransferData, TxTransparentTransferData, + TxUnshieldingTransferData, +}; use crate::control_flow::time; use crate::error::{EncodingError, Error, QueryError, Result, TxSubmitError}; use crate::io::Io; use crate::masp::TransferErr::Build; -use crate::masp::{ShieldedContext, ShieldedTransfer}; +use crate::masp::{MaspTransferData, ShieldedContext, ShieldedTransfer}; use crate::queries::Client; use crate::rpc::{ self, get_validator_stake, query_wasm_code_hash, validate_amount, @@ -2514,15 +2518,19 @@ pub async fn build_ibc_transfer( query_wasm_code_hash(context, args.tx_code_path.to_str().unwrap()) .await .map_err(|e| Error::from(QueryError::Wasm(e.to_string())))?; + let masp_transfer_data = MaspTransferData { + source: args.source.clone(), + target: TransferTarget::Address(Address::Internal( + InternalAddress::Ibc, + )), + token: args.token.clone(), + amount: validated_amount, + }; // For transfer from a spending key let shielded_parts = construct_shielded_parts( context, - &args.source, - // The token will be escrowed to IBC address - &TransferTarget::Address(Address::Internal(InternalAddress::Ibc)), - &args.token, - validated_amount, + vec![masp_transfer_data], !(args.tx.dry_run || args.tx.dry_run_wrapper), ) .await?; @@ -2569,10 +2577,12 @@ pub async fn build_ibc_transfer( let masp_tx_hash = tx.add_masp_tx_section(shielded_transfer.masp_tx.clone()).1; let transfer = token::ShieldingTransfer { - // The token will be escrowed to IBC address - source: source.clone(), - token: args.token.clone(), - amount: validated_amount, + data: ShieldingTransferData { + // The token will be escrowed to IBC address + source: source.clone(), + token: args.token.clone(), + amount: validated_amount, + }, // Link the Transfer to the MASP Transaction by hash code shielded_section_hash: masp_tx_hash, }; @@ -2931,31 +2941,39 @@ pub async fn build_shielded_transfer( context: &N, args: &mut args::TxShieldedTransfer, ) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(MASP); - let signing_data = signing::aux_signing_data( - context, - &args.tx, - Some(MASP), - default_signer, - ) - .await?; + let signing_data = + signing::aux_signing_data(context, &args.tx, Some(MASP), Some(MASP)) + .await?; // Shielded fee payment let fee_amount = validate_fee(context, &args.tx).await?; - // Validate the amount given - let validated_amount = - validate_amount(context, args.amount, &args.token, args.tx.force) - .await?; + let mut transfer_data = vec![]; + for TxShieldedTransferData { + source, + target, + token, + amount, + } in &args.data + { + // Validate the amount given + let validated_amount = + validate_amount(context, amount.to_owned(), token, args.tx.force) + .await?; + + transfer_data.push(MaspTransferData { + source: TransferSource::ExtendedSpendingKey(source.to_owned()), + target: TransferTarget::PaymentAddress(target.to_owned()), + token: token.to_owned(), + amount: validated_amount, + }); + } // TODO(namada#2597): this function should also take another arg as the fees // token and amount let shielded_parts = construct_shielded_parts( context, - &TransferSource::ExtendedSpendingKey(args.source), - &TransferTarget::PaymentAddress(args.target), - &args.token, - validated_amount, + transfer_data, !(args.tx.dry_run || args.tx.dry_run_wrapper), ) .await? @@ -3013,13 +3031,13 @@ pub async fn build_shielding_transfer( context: &N, args: &mut args::TxShieldingTransfer, ) -> Result<(Tx, SigningTxData, MaspEpoch)> { - let source = &args.source; - let default_signer = Some(source.clone()); let signing_data = signing::aux_signing_data( context, &args.tx, - Some(source.clone()), - default_signer, + None, + args.data + .first() + .map(|transfer_data| transfer_data.source.clone()), ) .await?; @@ -3031,38 +3049,57 @@ pub async fn build_shielding_transfer( (fee_amount, Some(updated_balance)) })?; - // Validate the amount given - let validated_amount = - validate_amount(context, args.amount, &args.token, args.tx.force) + let mut transfer_data = vec![]; + let mut data = vec![]; + for TxShieldingTransferData { + source, + token, + amount, + } in &args.data + { + // Validate the amount given + let validated_amount = + validate_amount(context, amount.to_owned(), token, args.tx.force) + .await?; + + // Check the balance of the source + if let Some(updated_balance) = &updated_balance { + let check_balance = if &updated_balance.source == source + && &updated_balance.token == token + { + CheckBalance::Balance(updated_balance.post_balance) + } else { + CheckBalance::Query(balance_key(token, source)) + }; + + check_balance_too_low_err( + token, + source, + validated_amount.amount(), + check_balance, + args.tx.force, + context, + ) .await?; + } - // Check the balance of the source - if let Some(updated_balance) = updated_balance { - let check_balance = if &updated_balance.source == source - && updated_balance.token == args.token - { - CheckBalance::Balance(updated_balance.post_balance) - } else { - CheckBalance::Query(balance_key(&args.token, source)) - }; + transfer_data.push(MaspTransferData { + source: TransferSource::Address(source.to_owned()), + target: TransferTarget::PaymentAddress(args.target), + token: token.to_owned(), + amount: validated_amount, + }); - check_balance_too_low_err( - &args.token, - source, - validated_amount.amount(), - check_balance, - args.tx.force, - context, - ) - .await?; + data.push(token::ShieldingTransferData { + source: source.to_owned(), + token: token.to_owned(), + amount: validated_amount, + }); } let shielded_parts = construct_shielded_parts( context, - &TransferSource::Address(source.clone()), - &TransferTarget::PaymentAddress(args.target), - &args.token, - validated_amount, + transfer_data, !(args.tx.dry_run || args.tx.dry_run_wrapper), ) .await? @@ -3070,7 +3107,7 @@ pub async fn build_shielding_transfer( let shielded_tx_epoch = shielded_parts.0.epoch; let add_shielded_parts = - |tx: &mut Tx, data: &mut token::ShieldingTransfer| { + |tx: &mut Tx, data: &mut token::ShieldingMultiTransfer| { // Add the MASP Transaction and its Builder to facilitate validation let ( ShieldedTransfer { @@ -3100,10 +3137,8 @@ pub async fn build_shielding_transfer( }; // Construct the tx data with a placeholder shielded section hash - let data = token::ShieldingTransfer { - source: source.clone(), - token: args.token.clone(), - amount: validated_amount, + let data = token::ShieldingMultiTransfer { + data, shielded_section_hash: Hash::zero(), }; @@ -3125,38 +3160,52 @@ pub async fn build_unshielding_transfer( context: &N, args: &mut args::TxUnshieldingTransfer, ) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(MASP); - let signing_data = signing::aux_signing_data( - context, - &args.tx, - Some(MASP), - default_signer, - ) - .await?; + let signing_data = + signing::aux_signing_data(context, &args.tx, Some(MASP), Some(MASP)) + .await?; // Shielded fee payment let fee_amount = validate_fee(context, &args.tx).await?; - // Validate the amount given - let validated_amount = - validate_amount(context, args.amount, &args.token, args.tx.force) - .await?; + let mut transfer_data = vec![]; + let mut data = vec![]; + for TxUnshieldingTransferData { + target, + token, + amount, + } in &args.data + { + // Validate the amount given + let validated_amount = + validate_amount(context, amount.to_owned(), token, args.tx.force) + .await?; + + transfer_data.push(MaspTransferData { + source: TransferSource::ExtendedSpendingKey(args.source), + target: TransferTarget::Address(target.to_owned()), + token: token.to_owned(), + amount: validated_amount, + }); + + data.push(token::UnshieldingTransferData { + target: target.to_owned(), + token: token.to_owned(), + amount: validated_amount, + }); + } // TODO(namada#2597): this function should also take another arg as the fees // token and amount let shielded_parts = construct_shielded_parts( context, - &TransferSource::ExtendedSpendingKey(args.source), - &TransferTarget::Address(args.target.clone()), - &args.token, - validated_amount, + transfer_data, !(args.tx.dry_run || args.tx.dry_run_wrapper), ) .await? .expect("Shielding transfer must have shielded parts"); let add_shielded_parts = - |tx: &mut Tx, data: &mut token::UnshieldingTransfer| { + |tx: &mut Tx, data: &mut token::UnshieldingMultiTransfer| { // Add the MASP Transaction and its Builder to facilitate validation let ( ShieldedTransfer { @@ -3186,12 +3235,11 @@ pub async fn build_unshielding_transfer( }; // Construct the tx data with a placeholder shielded section hash - let data = token::UnshieldingTransfer { - target: args.target.clone(), - token: args.token.clone(), - amount: validated_amount, + let data = token::UnshieldingMultiTransfer { + data, shielded_section_hash: Hash::zero(), }; + let tx = build_pow_flag( context, &args.tx, @@ -3208,10 +3256,7 @@ pub async fn build_unshielding_transfer( // Construct the shielded part of the transaction, if any async fn construct_shielded_parts( context: &N, - source: &TransferSource, - target: &TransferTarget, - token: &Address, - amount: token::DenominatedAmount, + data: Vec, update_ctx: bool, ) -> Result)>> { // Precompute asset types to increase chances of success in decoding @@ -3224,14 +3269,23 @@ async fn construct_shielded_parts( .await; let stx_result = ShieldedContext::::gen_shielded_transfer( - context, source, target, token, amount, update_ctx, + context, data, update_ctx, ) .await; let shielded_parts = match stx_result { Ok(Some(stx)) => stx, Ok(None) => return Ok(None), - Err(Build(builder::Error::InsufficientFunds(_))) => { + Err(Build { + error: builder::Error::InsufficientFunds(_), + data, + }) => { + let MaspTransferData { + source, + token, + amount, + .. + } = data.unwrap(); return Err(TxSubmitError::NegativeBalanceAfterTransfer( Box::new(source.effective_address()), amount.to_string(), @@ -3549,13 +3603,16 @@ pub async fn gen_ibc_shielding_transfer( .precompute_asset_types(context.client(), tokens) .await; + let masp_transfer_data = MaspTransferData { + source: TransferSource::Address(source.clone()), + target: args.target, + token: token.clone(), + amount: validated_amount, + }; let shielded_transfer = ShieldedContext::::gen_shielded_transfer( context, - &TransferSource::Address(source.clone()), - &args.target, - &token, - validated_amount, + vec![masp_transfer_data], true, ) .await @@ -3565,9 +3622,11 @@ pub async fn gen_ibc_shielding_transfer( let masp_tx_hash = Section::MaspTx(shielded_transfer.masp_tx.clone()).get_hash(); let transfer = token::ShieldingTransfer { - source: source.clone(), - token: token.clone(), - amount: validated_amount, + data: ShieldingTransferData { + source: source.clone(), + token: token.clone(), + amount: validated_amount, + }, shielded_section_hash: masp_tx_hash, }; Ok(Some((transfer, shielded_transfer.masp_tx))) diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index 4a67f36518..4470d08925 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -146,13 +146,57 @@ pub struct ShieldedTransfer { Serialize, Deserialize, )] -pub struct ShieldingTransfer { +pub struct ShieldingTransferData { /// Source address will spend the tokens pub source: Address, /// Token's address pub token: Address, /// The amount of tokens pub amount: DenominatedAmount, +} + +/// Arguments for a shielding transfer (from a transparent token to a shielded +/// token) +#[derive( + Debug, + Clone, + PartialEq, + BorshSerialize, + BorshDeserialize, + BorshDeserializer, + BorshSchema, + Hash, + Eq, + PartialOrd, + Serialize, + Deserialize, +)] +pub struct ShieldingTransfer { + /// Transfer-specific data + pub data: ShieldingTransferData, + /// Hash of tx section that contains the MASP transaction + pub shielded_section_hash: Hash, +} + +/// Arguments for a shielding transfer (from a transparent token to a shielded +/// token) +#[derive( + Debug, + Clone, + PartialEq, + BorshSerialize, + BorshDeserialize, + BorshDeserializer, + BorshSchema, + Hash, + Eq, + PartialOrd, + Serialize, + Deserialize, +)] +pub struct ShieldingMultiTransfer { + /// Transfer-specific data + pub data: Vec, /// Hash of tx section that contains the MASP transaction pub shielded_section_hash: Hash, } @@ -173,13 +217,57 @@ pub struct ShieldingTransfer { Serialize, Deserialize, )] -pub struct UnshieldingTransfer { +pub struct UnshieldingTransferData { /// Target address will receive the tokens pub target: Address, /// Token's address pub token: Address, /// The amount of tokens pub amount: DenominatedAmount, +} + +/// Arguments for an unshielding transfer (from a shielded token to a +/// transparent token) +#[derive( + Debug, + Clone, + PartialEq, + BorshSerialize, + BorshDeserialize, + BorshDeserializer, + BorshSchema, + Hash, + Eq, + PartialOrd, + Serialize, + Deserialize, +)] +pub struct UnshieldingTransfer { + /// Transfer-specific data + pub data: UnshieldingTransferData, + /// Hash of tx section that contains the MASP transaction + pub shielded_section_hash: Hash, +} + +/// Arguments for a multi-source unshielding transfer (from a shielded token to +/// a transparent token) +#[derive( + Debug, + Clone, + PartialEq, + BorshSerialize, + BorshDeserialize, + BorshDeserializer, + BorshSchema, + Hash, + Eq, + PartialOrd, + Serialize, + Deserialize, +)] +pub struct UnshieldingMultiTransfer { + /// Transfer-specific data + pub data: Vec, /// Hash of tx section that contains the MASP transaction pub shielded_section_hash: Hash, } @@ -199,7 +287,7 @@ pub mod testing { prop_compose! { /// Generate a transparent transfer - pub fn arb_transparent_transfer()( + fn arb_transparent_transfer()( source in arb_non_internal_address(), target in arb_non_internal_address(), token in arb_established_address().prop_map(Address::Established), diff --git a/crates/tx_prelude/src/token.rs b/crates/tx_prelude/src/token.rs index ab9ecaf571..cd59dcdcd4 100644 --- a/crates/tx_prelude/src/token.rs +++ b/crates/tx_prelude/src/token.rs @@ -6,7 +6,8 @@ use namada_events::{EmitEvents, EventLevel}; pub use namada_token::testing; pub use namada_token::{ storage_key, utils, Amount, DenominatedAmount, ShieldedTransfer, - ShieldingTransfer, TransparentTransfer, UnshieldingTransfer, + ShieldingMultiTransfer, ShieldingTransfer, TransparentTransfer, + UnshieldingMultiTransfer, UnshieldingTransfer, }; use namada_tx_env::TxEnv; diff --git a/wasm/tx_shielding_transfer/src/lib.rs b/wasm/tx_shielding_transfer/src/lib.rs index 389942686b..499f983b3c 100644 --- a/wasm/tx_shielding_transfer/src/lib.rs +++ b/wasm/tx_shielding_transfer/src/lib.rs @@ -6,20 +6,22 @@ use namada_tx_prelude::*; #[transaction] fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { let data = ctx.get_tx_data(&tx_data)?; - let transfer = token::ShieldingTransfer::try_from_slice(&data[..]) + let transfers = token::ShieldingMultiTransfer::try_from_slice(&data[..]) .wrap_err("Failed to decode token::ShieldingTransfer tx data")?; - debug_log!("apply_tx called with transfer: {:#?}", transfer); + debug_log!("apply_tx called with transfer: {:#?}", transfers); - token::transfer( - ctx, - &transfer.source, - &address::MASP, - &transfer.token, - transfer.amount.amount(), - ) - .wrap_err("Token transfer failed")?; + for transfer in transfers.data { + token::transfer( + ctx, + &transfer.source, + &address::MASP, + &transfer.token, + transfer.amount.amount(), + ) + .wrap_err("Token transfer failed")?; + } - let masp_section_ref = transfer.shielded_section_hash; + let masp_section_ref = transfers.shielded_section_hash; let shielded = tx_data .tx .get_section(&masp_section_ref) diff --git a/wasm/tx_unshielding_transfer/src/lib.rs b/wasm/tx_unshielding_transfer/src/lib.rs index 79bdac0757..f0bf375c73 100644 --- a/wasm/tx_unshielding_transfer/src/lib.rs +++ b/wasm/tx_unshielding_transfer/src/lib.rs @@ -6,20 +6,22 @@ use namada_tx_prelude::*; #[transaction] fn apply_tx(ctx: &mut Ctx, tx_data: BatchedTx) -> TxResult { let data = ctx.get_tx_data(&tx_data)?; - let transfer = token::UnshieldingTransfer::try_from_slice(&data[..]) + let transfers = token::UnshieldingMultiTransfer::try_from_slice(&data[..]) .wrap_err("Failed to decode token::UnshieldingTransfer tx data")?; - debug_log!("apply_tx called with transfer: {:#?}", transfer); + debug_log!("apply_tx called with transfer: {:#?}", transfers); - token::transfer( - ctx, - &address::MASP, - &transfer.target, - &transfer.token, - transfer.amount.amount(), - ) - .wrap_err("Token transfer failed")?; + for transfer in transfers.data { + token::transfer( + ctx, + &address::MASP, + &transfer.target, + &transfer.token, + transfer.amount.amount(), + ) + .wrap_err("Token transfer failed")?; + } - let masp_section_ref = transfer.shielded_section_hash; + let masp_section_ref = transfers.shielded_section_hash; let shielded = tx_data .tx .get_section(&masp_section_ref) From 3eff3b4fbd2f5a57940b4b3a1d83b134e9aae9c7 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 3 Jun 2024 20:57:58 +0200 Subject: [PATCH 3/9] Fixes benchmarks --- crates/node/src/bench_utils.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index 9a910df04e..bd5503f316 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -76,8 +76,9 @@ use namada::ledger::queries::{ use namada::masp::MaspTxRefs; use namada::state::StorageRead; use namada::token::{ - Amount, DenominatedAmount, ShieldedTransfer, ShieldingTransfer, - ShieldingTransferData, UnshieldingTransfer, UnshieldingTransferData, + Amount, DenominatedAmount, ShieldedTransfer, ShieldingMultiTransfer, + ShieldingTransfer, ShieldingTransferData, UnshieldingMultiTransfer, + UnshieldingTransferData, }; use namada::tx::data::pos::Bond; use namada::tx::data::{ @@ -1127,12 +1128,12 @@ impl BenchShieldedCtx { } else if target.effective_address() == MASP { namada.client().generate_tx( TX_SHIELDING_TRANSFER_WASM, - ShieldingTransfer { - data: ShieldingTransferData { + ShieldingMultiTransfer { + data: vec![ShieldingTransferData { source: source.effective_address(), token: address::testing::nam(), amount: DenominatedAmount::native(amount), - }, + }], shielded_section_hash, }, Some(shielded), @@ -1142,12 +1143,12 @@ impl BenchShieldedCtx { } else { namada.client().generate_tx( TX_UNSHIELDING_TRANSFER_WASM, - UnshieldingTransfer { - data: UnshieldingTransferData { + UnshieldingMultiTransfer { + data: vec![UnshieldingTransferData { target: target.effective_address(), token: address::testing::nam(), amount: DenominatedAmount::native(amount), - }, + }], shielded_section_hash, }, Some(shielded), @@ -1213,10 +1214,14 @@ impl BenchShieldedCtx { timeout_timestamp_on_b: timeout_timestamp, }; - let transfer = ShieldingTransfer::deserialize( + let vectorized_transfer = ShieldingMultiTransfer::deserialize( &mut tx.tx.data(&tx.cmt).unwrap().as_slice(), ) .unwrap(); + let transfer = ShieldingTransfer { + data: vectorized_transfer.data.first().unwrap().to_owned(), + shielded_section_hash: vectorized_transfer.shielded_section_hash, + }; let masp_tx = tx .tx .get_section(&transfer.shielded_section_hash) From 453b21196dea8ed7a96c91f614d9351966b39647 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 4 Jun 2024 11:01:44 +0200 Subject: [PATCH 4/9] Fixes signature generation for vectorized transfers --- crates/sdk/src/tx.rs | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 4100911fb3..60e6c19074 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2846,15 +2846,22 @@ pub async fn build_transparent_transfer( // Evaluate signer and fees let (signing_data, fee_amount, updated_balance) = { + let source = if args.data.len() == 1 { + // If only one transfer take its source as the signer + args.data + .first() + .map(|transfer_data| transfer_data.source.clone()) + } else { + // Otherwise the caller is required to pass the public keys in the + // argument + None + }; + let signing_data = signing::aux_signing_data( context, &args.tx, - None, - // If signing keys arg is not provided assume a single transfer and - // take the source - args.data - .first() - .map(|transfer_data| transfer_data.source.clone()), + source.clone(), + source, ) .await?; @@ -3031,15 +3038,19 @@ pub async fn build_shielding_transfer( context: &N, args: &mut args::TxShieldingTransfer, ) -> Result<(Tx, SigningTxData, MaspEpoch)> { - let signing_data = signing::aux_signing_data( - context, - &args.tx, - None, + let source = if args.data.len() == 1 { + // If only one transfer take its source as the signer args.data .first() - .map(|transfer_data| transfer_data.source.clone()), - ) - .await?; + .map(|transfer_data| transfer_data.source.clone()) + } else { + // Otherwise the caller is required to pass the public keys in the + // argument + None + }; + let signing_data = + signing::aux_signing_data(context, &args.tx, source.clone(), source) + .await?; // Transparent fee payment let (fee_amount, updated_balance) = From 4a94e0e99ee300833776caeba4073c660f5f0f4d Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 6 Jun 2024 10:48:41 +0200 Subject: [PATCH 5/9] Check no vectorized transfers in cli --- crates/apps_lib/src/client/tx.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 944e204279..12424f5b5c 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -743,6 +743,14 @@ pub async fn submit_transparent_transfer( namada: &impl Namada, args: args::TxTransparentTransfer, ) -> Result<(), error::Error> { + if args.data.len() > 1 { + // TODO(namada#3379): Vectorized transfers are not yet supported in the + // CLI + return Err(error::Error::Other( + "Unexpected vectorized transparent transfer".to_string(), + )); + } + submit_reveal_aux( namada, args.tx.clone(), From 9d6c19d1cb421b2bf8f06ebc2f8cbfe50220718c Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 6 Jun 2024 10:49:46 +0200 Subject: [PATCH 6/9] Misc improvements to signing for vectorized transfers --- crates/sdk/src/signing.rs | 44 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index a92370a3b3..aa9417c36f 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -759,27 +759,28 @@ impl TokenTransfer<'_> { let mut map: HashMap<&Address, DenominatedAmount> = HashMap::new(); - for transfer in &transfers.0 { - match address { - TransferSide::Source(source) - if source == &transfer.source => - { - Self::update_token_amount_map( - &mut map, - &transfer.token, - transfer.amount, - )?; + match address { + TransferSide::Source(source) => { + for transfer in &transfers.0 { + if source == &transfer.source { + Self::update_token_amount_map( + &mut map, + &transfer.token, + transfer.amount, + )?; + } } - TransferSide::Target(target) - if target == &transfer.target => - { - Self::update_token_amount_map( - &mut map, - &transfer.token, - transfer.amount, - )?; + } + TransferSide::Target(target) => { + for transfer in &transfers.0 { + if target == &transfer.target { + Self::update_token_amount_map( + &mut map, + &transfer.token, + transfer.amount, + )?; + } } - _ => (), } } @@ -832,10 +833,7 @@ impl TokenTransfer<'_> { ) -> Result<(), Error> { match map.get_mut(token) { Some(prev_amount) => { - *prev_amount = - prev_amount.checked_add(amount).ok_or_else(|| { - Error::Other("Overflow in amount".to_string()) - })?; + *prev_amount = checked!(prev_amount + amount)?; } None => { map.insert(token, amount); From 561e4e89e8ed9b7a843c5b26f6a6bde3e2d35b75 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 6 Jun 2024 10:52:46 +0200 Subject: [PATCH 7/9] Avoids reloading shielded context --- crates/sdk/src/masp.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index 0ae8d00e3b..bed3d1cf52 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -1612,6 +1612,7 @@ impl ShieldedContext { // Determine epoch in which to submit potential shielded transaction let epoch = rpc::query_masp_epoch(context.client()).await?; + let mut is_context_loaded = false; for MaspTransferData { source, target, @@ -1629,10 +1630,13 @@ impl ShieldedContext { // We want to fund our transaction solely from supplied spending key let spending_key = spending_key.map(|x| x.into()); { - // Load the current shielded context given the spending key we - // possess - let mut shielded = context.shielded_mut().await; - let _ = shielded.load().await; + if !is_context_loaded { + // Load the current shielded context (at most once) given + // the spending key we possess + let mut shielded = context.shielded_mut().await; + let _ = shielded.load().await; + is_context_loaded = true; + } } // Context required for storing which notes are in the source's // possession From 3a77b4a600481e32275ab696158e53826c9a2f24 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 6 Jun 2024 11:28:24 +0200 Subject: [PATCH 8/9] Fmt --- crates/sdk/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index c4778e285d..7d83a93895 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -613,8 +613,8 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { args: &args::Tx, signing_data: SigningTxData, with: impl Fn(Tx, common::PublicKey, HashSet, D) -> F - + MaybeSend - + MaybeSync, + + MaybeSend + + MaybeSync, user_data: D, ) -> crate::error::Result<()> where From 1e98403560a297f56f38ff67397a4bb9182068aa Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 4 Jun 2024 11:05:19 +0200 Subject: [PATCH 9/9] Changelog #3356 --- .changelog/unreleased/features/3356-vectorize-transfers.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/features/3356-vectorize-transfers.md diff --git a/.changelog/unreleased/features/3356-vectorize-transfers.md b/.changelog/unreleased/features/3356-vectorize-transfers.md new file mode 100644 index 0000000000..3b7f8e66da --- /dev/null +++ b/.changelog/unreleased/features/3356-vectorize-transfers.md @@ -0,0 +1,2 @@ +- Reworked transparent and masp transfers to allow for multiple sources, targets, + tokens and amounts. ([\#3356](https://github.com/anoma/namada/pull/3356)) \ No newline at end of file