diff --git a/.changelog/unreleased/SDK/4816-wrapper-sigs.md b/.changelog/unreleased/SDK/4816-wrapper-sigs.md new file mode 100644 index 00000000000..4b1fc3de1e4 --- /dev/null +++ b/.changelog/unreleased/SDK/4816-wrapper-sigs.md @@ -0,0 +1,15 @@ +- Reworked SDK wrapping and signatures, including some breaking changes. + More specifically: + - The wrapper arguments have been extracted into a separate type and their + presence signals the need to wrap the tx + - The dump and dry-run arguments have been turned into enumerations + - The wrapper signer data has been removed from SigningTxData and moved into + SigningWrapperData + - Simplified the interface of aux_signing_data + - Removed redundant dispatcher functions + - Prevent casting from a wrapped to a raw transaction type + - Prevent submitting an unwrapped transaction + - Avoided passing the MASP internal address as a transaction's owner + - Updated the interface of build_batch + - Removed the owner for reveal_pk + ([\#4816](https://github.com/anoma/namada/pull/4816)) \ No newline at end of file diff --git a/.github/workflows/scripts/check-changelog.sh b/.github/workflows/scripts/check-changelog.sh index 9579d9bbb39..2c9946ee7b1 100755 --- a/.github/workflows/scripts/check-changelog.sh +++ b/.github/workflows/scripts/check-changelog.sh @@ -8,7 +8,7 @@ check_changelog_added_in_subfolders() { fi echo "Using sha: $head_commit" - subfolders=("ci" "bug-fixes" "improvements" "miscellaneous" "features" "testing" "docs") + subfolders=("ci" "bug-fixes" "improvements" "miscellaneous" "features" "testing" "docs" "SDK") subfolder_pattern=$(printf "|%s" "${subfolders[@]}") subfolder_pattern=${subfolder_pattern:1} # Remove the leading '|' @@ -26,4 +26,4 @@ check_changelog_added_in_subfolders() { fi } -check_changelog_added_in_subfolders \ No newline at end of file +check_changelog_added_in_subfolders diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index c71552360a5..32ec3e07b05 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -7813,21 +7813,24 @@ pub mod args { ctx: &mut Context, ) -> Result, Self::Error> { let ctx = ctx.borrow_mut_chain_or_exit(); + let wrapper = self.wrap_tx.map(|wrapper| Wrapper { + broadcast_only: wrapper.broadcast_only, + fee_amount: wrapper.fee_amount, + fee_token: ctx.get(&wrapper.fee_token).into(), + gas_limit: wrapper.gas_limit, + wrapper_fee_payer: wrapper + .wrapper_fee_payer + .map(|x| ctx.get(&x)), + }); Ok(Tx:: { dry_run: self.dry_run, - dry_run_wrapper: self.dry_run_wrapper, dump_tx: self.dump_tx, - dump_wrapper_tx: self.dump_wrapper_tx, output_folder: self.output_folder, force: self.force, - broadcast_only: self.broadcast_only, ledger_address: ctx.get(&self.ledger_address), initialized_account_alias: self.initialized_account_alias, wallet_alias_force: self.wallet_alias_force, - fee_amount: self.fee_amount, - fee_token: ctx.get(&self.fee_token).into(), - gas_limit: self.gas_limit, signing_keys: self .signing_keys .iter() @@ -7839,7 +7842,7 @@ pub mod args { chain_id: self .chain_id .or_else(|| Some(ctx.config.ledger.chain_id.clone())), - wrapper_fee_payer: self.wrapper_fee_payer.map(|x| ctx.get(&x)), + wrap_tx: wrapper, memo: self.memo, use_device: self.use_device, device_transport: self.device_transport, @@ -7966,10 +7969,20 @@ pub mod args { } fn parse(matches: &ArgMatches) -> Self { - let dry_run = DRY_RUN_TX.parse(matches); - let dry_run_wrapper = DRY_RUN_WRAPPER_TX.parse(matches); - let dump_tx = DUMP_TX.parse(matches); - let dump_wrapper_tx = DUMP_WRAPPER_TX.parse(matches); + let dry_run = if DRY_RUN_TX.parse(matches) { + Some(DryRun::Inner) + } else if DRY_RUN_WRAPPER_TX.parse(matches) { + Some(DryRun::Wrapper) + } else { + None + }; + let dump_tx = if DUMP_TX.parse(matches) { + Some(DumpTx::Inner) + } else if DUMP_WRAPPER_TX.parse(matches) { + Some(DumpTx::Wrapper) + } else { + None + }; let force = FORCE.parse(matches); let broadcast_only = BROADCAST_ONLY.parse(matches); let ledger_address = CONFIG_RPC_LEDGER_ADDRESS.parse(matches); @@ -7998,25 +8011,30 @@ pub mod args { } }; let device_transport = DEVICE_TRANSPORT.parse(matches); + // Wrap the transaction unless we want to dump or dry-run the raw tx + let wrap_tx = match (&dump_tx, &dry_run) { + (_, Some(DryRun::Inner)) | (Some(DumpTx::Inner), _) => None, + _ => Some(Wrapper { + broadcast_only, + fee_amount, + wrapper_fee_payer, + fee_token, + gas_limit, + }), + }; Self { dry_run, - dry_run_wrapper, dump_tx, - dump_wrapper_tx, force, - broadcast_only, ledger_address, initialized_account_alias, wallet_alias_force, - fee_amount, - fee_token, - gas_limit, expiration, signing_keys, tx_reveal_code_path, password, chain_id, - wrapper_fee_payer, + wrap_tx, output_folder, memo, use_device, diff --git a/crates/apps_lib/src/cli/client.rs b/crates/apps_lib/src/cli/client.rs index bc24b60adef..c55dac91df3 100644 --- a/crates/apps_lib/src/cli/client.rs +++ b/crates/apps_lib/src/cli/client.rs @@ -37,8 +37,7 @@ impl CliApi { client.wait_until_node_is_synced(&io).await?; let args = args.to_sdk(&mut ctx)?; let namada = ctx.to_sdk(client, io); - let dry_run = - args.tx.dry_run || args.tx.dry_run_wrapper; + let dry_run = args.tx.dry_run.is_some(); tx::submit_custom(&namada, args).await?; if !dry_run { namada @@ -193,8 +192,7 @@ impl CliApi { client.wait_until_node_is_synced(&io).await?; let args = args.to_sdk(&mut ctx)?; let namada = ctx.to_sdk(client, io); - let dry_run = - args.tx.dry_run || args.tx.dry_run_wrapper; + let dry_run = args.tx.dry_run.is_some(); tx::submit_init_account(&namada, args).await?; if !dry_run { namada diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 41cf8712d96..364f041ea4b 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -26,6 +26,7 @@ use namada_sdk::ibc::convert_masp_tx_to_ibc_memo; use namada_sdk::io::{Io, display_line, edisplay_line}; use namada_sdk::key::*; use namada_sdk::rpc::{InnerTxResult, TxBroadcastData, TxResponse}; +use namada_sdk::signing::SigningData; use namada_sdk::state::EPOCH_SWITCH_BLOCKS_DELAY; use namada_sdk::tx::data::compute_inner_tx_hash; use namada_sdk::tx::{CompressedAuthorization, Section, Signer, Tx}; @@ -62,32 +63,6 @@ const MAX_HW_CONVERT: usize = 15; // introduced. const MAX_HW_OUTPUT: usize = 15; -/// Wrapper around `signing::aux_signing_data` that stores the optional -/// disposable address to the wallet -pub async fn aux_signing_data( - context: &impl Namada, - args: &args::Tx, - owner: Option
, - default_signer: Option
, - disposable_signing_key: bool, - signatures: Vec>, - wrapper_signature: Option>, -) -> Result { - let signing_data = signing::aux_signing_data( - context, - args, - owner, - default_signer, - vec![], - disposable_signing_key, - signatures, - wrapper_signature, - ) - .await?; - - Ok(signing_data) -} - pub async fn with_hardware_wallet( mut tx: Tx, pubkey: common::PublicKey, @@ -188,7 +163,7 @@ pub async fn sign( context: &N, tx: &mut Tx, args: &args::Tx, - signing_data: SigningTxData, + signing_data: SigningData, ) -> Result<(), error::Error> { // Setup a reusable context for signing transactions using the Ledger if args.use_device { @@ -219,11 +194,7 @@ pub async fn submit_reveal_aux( context: &impl Namada, args: &args::Tx, address: &Address, -) -> Result, error::Error> { - if args.dump_tx || args.dump_wrapper_tx { - return Ok(None); - } - +) -> Result, error::Error> { if let Address::Implicit(ImplicitAddress(pkh)) = address { let public_key = context .wallet_mut() @@ -250,7 +221,7 @@ async fn batch_opt_reveal_pk_and_submit( namada: &N, args: &args::Tx, owners: &[&Address], - mut tx_data: (Tx, SigningTxData), + mut tx_data: (Tx, SigningData), ) -> Result where ::Error: std::fmt::Display, @@ -270,7 +241,7 @@ where if args.use_device { // Sign each transaction separately for (tx, sig_data) in &mut batched_tx_data { - sign(namada, tx, args, sig_data.clone()).await?; + sign(namada, tx, args, sig_data.to_owned()).await?; } sign(namada, &mut tx_data.0, args, tx_data.1).await?; // Then submit each transaction separately @@ -285,9 +256,13 @@ where let (mut batched_tx, batched_signing_data) = namada_sdk::tx::build_batch(batched_tx_data)?; // Sign the batch with the union of the signers required for each part - for sig_data in batched_signing_data { - sign(namada, &mut batched_tx, args, sig_data).await?; - } + sign( + namada, + &mut batched_tx, + args, + SigningData::Wrapper(batched_signing_data), + ) + .await?; // Then finally submit everything in one go namada.submit(batched_tx, args).await } @@ -299,8 +274,13 @@ pub async fn submit_bridge_pool_tx( ) -> Result<(), error::Error> { let bridge_pool_tx_data = args.clone().build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, bridge_pool_tx_data.0)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx( + namada.io(), + dump_tx, + args.tx.output_folder, + bridge_pool_tx_data.0, + )?; } else { batch_opt_reveal_pk_and_submit( namada, @@ -323,10 +303,7 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx { - return tx::dump_tx(namada.io(), &args.tx, tx); - } - if args.tx.dump_wrapper_tx { + if let Some(dump_tx) = args.tx.dump_tx { // Attach the provided inner signatures to the tx (if any) let signatures = args .signatures @@ -340,26 +317,15 @@ where }) .collect::>>()?; tx.add_signatures(signatures); - - return tx::dump_tx(namada.io(), &args.tx, tx); + return tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx); } - if let Some(signing_data) = signing_data { - let owners = args - .owner - .map_or_else(Default::default, |owner| vec![owner]); - let refs: Vec<&Address> = owners.iter().collect(); - batch_opt_reveal_pk_and_submit( - namada, - &args.tx, - &refs, - (tx, signing_data), - ) + let owners = args + .owner + .map_or_else(Default::default, |owner| vec![owner]); + let refs: Vec<&Address> = owners.iter().collect(); + batch_opt_reveal_pk_and_submit(namada, &args.tx, &refs, (tx, signing_data)) .await?; - } else { - // Just submit without the need for signing - namada.submit(tx, &args.tx).await?; - } Ok(()) } @@ -373,8 +339,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -393,8 +359,8 @@ where { let (mut tx, signing_data) = tx::build_init_account(namada, &args).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -476,15 +442,15 @@ pub async fn submit_change_consensus_key( let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)? + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; let cmt = tx.first_commitments().unwrap().to_owned(); let wrapper_hash = tx.wrapper_hash(); let resp = namada.submit(tx, &args.tx).await?; - if !(args.tx.dry_run || args.tx.dry_run_wrapper) { + if args.tx.dry_run.is_none() { if resp .is_applied_and_valid(wrapper_hash.as_ref(), &cmt) .is_some() @@ -674,15 +640,15 @@ pub async fn submit_become_validator( let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; let cmt = tx.first_commitments().unwrap().to_owned(); let wrapper_hash = tx.wrapper_hash(); let resp = namada.submit(tx, &args.tx).await?; - if args.tx.dry_run || args.tx.dry_run_wrapper { + if args.tx.dry_run.is_some() { display_line!( namada.io(), "Transaction dry run. No key or addresses have been saved." @@ -808,7 +774,7 @@ pub async fn submit_init_validator( ) .await?; - if tx_args.dry_run || tx_args.dry_run_wrapper { + if tx_args.dry_run.is_some() { eprintln!( "Cannot proceed to become validator in dry-run as no account has \ been created" @@ -864,8 +830,13 @@ pub async fn submit_transparent_transfer( let transfer_data = args.clone().build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, transfer_data.0)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx( + namada.io(), + dump_tx, + args.tx.output_folder, + transfer_data.0, + )?; } else { let reveal_pks: Vec<_> = args.sources.iter().map(|datum| &datum.source).collect(); @@ -1246,12 +1217,14 @@ pub async fn submit_shielded_transfer( ) .await?; let (mut tx, signing_data) = - args.clone().build(namada, &mut bparams, false).await?; - let disposable_fee_payer = match signing_data.fee_payer { - either::Either::Left((_, disposable_fee_payer)) => disposable_fee_payer, - either::Either::Right(_) => unreachable!(), + args.clone().build(namada, &mut bparams).await?; + let disposable_fee_payer = match signing_data { + SigningData::Inner(_) => None, + SigningData::Wrapper(ref signing_wrapper_data) => { + Some(signing_wrapper_data.disposable_fee_payer()) + } }; - if !disposable_fee_payer { + if let Some(false) = disposable_fee_payer { display_line!( namada.io(), "{}: {}\n", @@ -1261,7 +1234,16 @@ pub async fn submit_shielded_transfer( fees via the MASP with a disposable gas payer.", ); } - masp_sign(&mut tx, &args.tx, &signing_data, shielded_hw_keys).await?; + masp_sign( + &mut tx, + &args.tx, + signing_data + .signing_tx_data() + .first() + .expect("Missing expected signing data"), + shielded_hw_keys, + ) + .await?; let masp_section = tx .sections @@ -1272,20 +1254,20 @@ pub async fn submit_shielded_transfer( "Missing MASP section in shielded transaction".to_string(), ) })?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - if disposable_fee_payer { + if let Some(dump_tx) = args.tx.dump_tx { + if let Some(true) = disposable_fee_payer { display_line!( namada.io(), "Transaction dry run. The disposable address will not be \ saved to wallet." ) } - tx::dump_tx(namada.io(), &args.tx, tx)?; + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; pre_cache_masp_data(namada, &masp_section).await; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; // Store the generated disposable signing key to wallet in case of need - if disposable_fee_payer { + if let Some(true) = disposable_fee_payer { namada.wallet().await.save().map_err(|_| { error::Error::Other( "Failed to save disposable address to wallet".to_string(), @@ -1314,8 +1296,8 @@ pub async fn submit_shielding_transfer( let (tx, signing_data, tx_epoch) = args.clone().build(namada, &mut bparams).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; break; } @@ -1409,12 +1391,14 @@ pub async fn submit_unshielding_transfer( ) .await?; let (mut tx, signing_data) = - args.clone().build(namada, &mut bparams, false).await?; - let disposable_fee_payer = match signing_data.fee_payer { - either::Either::Left((_, disposable_fee_payer)) => disposable_fee_payer, - either::Either::Right(_) => unreachable!(), + args.clone().build(namada, &mut bparams).await?; + let disposable_fee_payer = match signing_data { + SigningData::Inner(_) => None, + SigningData::Wrapper(ref signing_wrapper_data) => { + Some(signing_wrapper_data.disposable_fee_payer()) + } }; - if !disposable_fee_payer { + if let Some(false) = disposable_fee_payer { display_line!( namada.io(), "{}: {}\n", @@ -1424,7 +1408,16 @@ pub async fn submit_unshielding_transfer( gas fees via the MASP with a disposable gas payer.", ); } - masp_sign(&mut tx, &args.tx, &signing_data, shielded_hw_keys).await?; + masp_sign( + &mut tx, + &args.tx, + signing_data + .signing_tx_data() + .first() + .expect("Missing signing data"), + shielded_hw_keys, + ) + .await?; let masp_section = tx .sections @@ -1435,20 +1428,20 @@ pub async fn submit_unshielding_transfer( "Missing MASP section in shielded transaction".to_string(), ) })?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - if disposable_fee_payer { + if let Some(dump_tx) = args.tx.dump_tx { + if let Some(true) = disposable_fee_payer { display_line!( namada.io(), "Transaction dry run. The disposable address will not be \ saved to wallet." ) } - tx::dump_tx(namada.io(), &args.tx, tx)?; + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; pre_cache_masp_data(namada, &masp_section).await; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; // Store the generated disposable signing key to wallet in case of need - if disposable_fee_payer { + if let Some(true) = disposable_fee_payer { namada.wallet().await.save().map_err(|_| { error::Error::Other( "Failed to save disposable address to wallet".to_string(), @@ -1493,14 +1486,16 @@ where // If transaction building fails for any reason, then abort the process // blaming MASP build parameter generation if that had also failed. let (mut tx, signing_data, _) = args - .build(namada, &mut bparams, false) + .build(namada, &mut bparams) .await .map_err(|e| bparams_err.unwrap_or(e))?; - let disposable_fee_payer = match signing_data.fee_payer { - either::Either::Left((_, disposable_fee_payer)) => disposable_fee_payer, - either::Either::Right(_) => unreachable!(), + let disposable_fee_payer = match signing_data { + SigningData::Inner(_) => None, + SigningData::Wrapper(ref signing_wrapper_data) => { + Some(signing_wrapper_data.disposable_fee_payer()) + } }; - if args.source.spending_key().is_some() && !disposable_fee_payer { + if let Some(false) = disposable_fee_payer { display_line!( namada.io(), "{}: {}\n", @@ -1513,13 +1508,22 @@ where // Any effects of a MASP build parameter generation failure would have // manifested during transaction building. So we discount that as a root // cause from now on. - masp_sign(&mut tx, &args.tx, &signing_data, shielded_hw_keys).await?; + masp_sign( + &mut tx, + &args.tx, + signing_data + .signing_tx_data() + .first() + .expect("Missing expected signing data"), + shielded_hw_keys, + ) + .await?; let opt_masp_section = tx.sections.iter().find_map(|section| section.masp_tx()); - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; - if disposable_fee_payer { + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; + if let Some(true) = disposable_fee_payer { display_line!( namada.io(), "Transaction dry run. The disposable address will not be \ @@ -1531,7 +1535,7 @@ where } } else { // Store the generated disposable signing key to wallet in case of need - if disposable_fee_payer { + if let Some(true) = disposable_fee_payer { namada.wallet().await.save().map_err(|_| { error::Error::Other( "Failed to save disposable address to wallet".to_string(), @@ -1650,8 +1654,13 @@ where ) }; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, proposal_tx_data.0)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx( + namada.io(), + dump_tx, + args.tx.output_folder, + proposal_tx_data.0, + )?; } else { batch_opt_reveal_pk_and_submit( namada, @@ -1674,8 +1683,13 @@ where { let submit_vote_proposal_data = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, submit_vote_proposal_data.0)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx( + namada.io(), + dump_tx, + args.tx.output_folder, + submit_vote_proposal_data.0, + )?; } else { batch_opt_reveal_pk_and_submit( namada, @@ -1696,10 +1710,15 @@ pub async fn submit_reveal_pk( where ::Error: std::fmt::Display, { - if args.tx.dump_tx || args.tx.dump_wrapper_tx { + if let Some(dump_tx) = &args.tx.dump_tx { let tx_data = tx::build_reveal_pk(namada, &args.tx, &args.public_key).await?; - tx::dump_tx(namada.io(), &args.tx, tx_data.0.clone())?; + tx::dump_tx( + namada.io(), + dump_tx.to_owned(), + args.tx.output_folder, + tx_data.0.clone(), + )?; } else { let tx_data = submit_reveal_aux(namada, &args.tx, &(&args.public_key).into()) @@ -1723,8 +1742,13 @@ where { let submit_bond_tx_data = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, submit_bond_tx_data.0)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx( + namada.io(), + dump_tx, + args.tx.output_folder, + submit_bond_tx_data.0, + )?; } else { let default_address = args.source.as_ref().unwrap_or(&args.validator); batch_opt_reveal_pk_and_submit( @@ -1749,15 +1773,15 @@ where let (mut tx, signing_data, latest_withdrawal_pre) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; let cmt = tx.first_commitments().unwrap().to_owned(); let wrapper_hash = tx.wrapper_hash(); let resp = namada.submit(tx, &args.tx).await?; - if !(args.tx.dry_run || args.tx.dry_run_wrapper) + if args.tx.dry_run.is_none() && resp .is_applied_and_valid(wrapper_hash.as_ref(), &cmt) .is_some() @@ -1779,8 +1803,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -1799,8 +1823,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -1819,8 +1843,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -1839,8 +1863,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -1859,8 +1883,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -1879,8 +1903,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -1899,8 +1923,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -1919,8 +1943,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -1939,8 +1963,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; @@ -1959,8 +1983,8 @@ where { let (mut tx, signing_data) = args.build(namada).await?; - if args.tx.dump_tx || args.tx.dump_wrapper_tx { - tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(dump_tx) = args.tx.dump_tx { + tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; diff --git a/crates/apps_lib/src/config/genesis/transactions.rs b/crates/apps_lib/src/config/genesis/transactions.rs index 6fc5eca6133..33c42bdf4dd 100644 --- a/crates/apps_lib/src/config/genesis/transactions.rs +++ b/crates/apps_lib/src/config/genesis/transactions.rs @@ -20,7 +20,7 @@ use namada_sdk::dec::Dec; use namada_sdk::key::common::PublicKey; use namada_sdk::key::{RefTo, SerializeWithBorsh, SigScheme, common, ed25519}; use namada_sdk::proof_of_stake::types::ValidatorMetaData; -use namada_sdk::signing::{SigningTxData, sign_tx}; +use namada_sdk::signing::{SigningTxData, SigningWrapperData, sign_tx}; use namada_sdk::string_encoding::StringEncoded; use namada_sdk::time::DateTimeUtc; use namada_sdk::token; @@ -77,21 +77,15 @@ fn get_tx_args(use_device: bool) -> TxArgs { use std::str::FromStr; TxArgs { - dry_run: false, - dry_run_wrapper: false, - dump_tx: false, - dump_wrapper_tx: false, + dry_run: None, + dump_tx: None, output_folder: None, force: false, - broadcast_only: false, ledger_address: tendermint_rpc::Url::from_str("http://127.0.0.1:26657") .unwrap(), initialized_account_alias: None, wallet_alias_force: false, - fee_amount: None, - wrapper_fee_payer: None, - fee_token: genesis_fee_token_address(), - gas_limit: 0.into(), + wrap_tx: None, expiration: Default::default(), chain_id: None, signing_keys: vec![], @@ -764,15 +758,23 @@ impl Signed { { let (pks, threshold) = self.data.get_pks(established_accounts); let owner = self.data.get_owner().address(); - let signing_data = SigningTxData { - owner: Some(owner), - account_public_keys_map: Some(pks.iter().cloned().collect()), - public_keys: pks.clone(), - threshold, - fee_payer: Either::Left((genesis_fee_payer_pk(), false)), - shielded_hash: None, - signatures: vec![], - }; + let signing_data = + namada_sdk::signing::SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + owner: Some(owner), + account_public_keys_map: Some( + pks.iter().cloned().collect(), + ), + public_keys: pks.clone(), + threshold, + shielded_hash: None, + signatures: vec![], + }], + fee_auth: namada_sdk::signing::FeeAuthorization::Signer { + pubkey: genesis_fee_payer_pk(), + disposable_fee_payer: false, + }, + }); let mut tx = self.data.tx_to_sign(); diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 4cc00334a5b..d70e035bc58 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -40,7 +40,7 @@ use crate::rpc::{ get_registry_from_xcs_osmosis_contract, osmosis_denom_from_namada_denom, query_ibc_denom, query_osmosis_route_and_min_out, }; -use crate::signing::{SigningTxData, gen_disposable_signing_key}; +use crate::signing::{SigningData, gen_disposable_signing_key}; use crate::wallet::{DatedSpendingKey, DatedViewingKey}; use crate::{Namada, rpc, tx}; @@ -231,7 +231,7 @@ impl TxCustom { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, Option)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_custom(context, self).await } } @@ -302,7 +302,7 @@ impl TxTransparentTransfer { pub async fn build( &mut self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_transparent_transfer(context, self).await } } @@ -362,10 +362,8 @@ impl TxShieldedTransfer { &mut self, context: &impl Namada, bparams: &mut impl BuildParams, - skip_fee_handling: bool, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { - tx::build_shielded_transfer(context, self, bparams, skip_fee_handling) - .await + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { + tx::build_shielded_transfer(context, self, bparams).await } } @@ -413,7 +411,7 @@ impl TxShieldingTransfer { &mut self, context: &impl Namada, bparams: &mut impl BuildParams, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData, MaspEpoch)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData, MaspEpoch)> { tx::build_shielding_transfer(context, self, bparams).await } } @@ -464,15 +462,8 @@ impl TxUnshieldingTransfer { &mut self, context: &impl Namada, bparams: &mut impl BuildParams, - skip_fee_handling: bool, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { - tx::build_unshielding_transfer( - context, - self, - bparams, - skip_fee_handling, - ) - .await + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { + tx::build_unshielding_transfer(context, self, bparams).await } } @@ -938,10 +929,9 @@ impl TxIbcTransfer { &self, context: &impl Namada, bparams: &mut impl BuildParams, - skip_fee_handling: bool, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData, Option)> + ) -> crate::error::Result<(namada_tx::Tx, SigningData, Option)> { - tx::build_ibc_transfer(context, self, bparams, skip_fee_handling).await + tx::build_ibc_transfer(context, self, bparams).await } } @@ -1011,7 +1001,7 @@ impl InitProposal { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { let current_epoch = rpc::query_epoch(context.client()).await?; let governance_parameters = rpc::query_governance_parameters(context.client()).await; @@ -1155,7 +1145,7 @@ impl VoteProposal { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { let current_epoch = rpc::query_epoch(context.client()).await?; tx::build_vote_proposal(context, self, current_epoch).await } @@ -1227,7 +1217,7 @@ impl TxInitAccount { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_init_account(context, self).await } } @@ -1327,7 +1317,7 @@ impl TxBecomeValidator { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_become_validator(context, self).await } } @@ -1450,7 +1440,7 @@ impl TxUpdateAccount { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_update_account(context, self).await } } @@ -1517,7 +1507,7 @@ impl Bond { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_bond(context, self).await } } @@ -1545,7 +1535,7 @@ impl Unbond { context: &impl Namada, ) -> crate::error::Result<( namada_tx::Tx, - SigningTxData, + SigningData, Option<(Epoch, token::Amount)>, )> { tx::build_unbond(context, self).await @@ -1615,7 +1605,7 @@ impl Redelegate { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_redelegation(context, self).await } } @@ -1696,7 +1686,7 @@ impl RevealPk { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_reveal_pk(context, &self.tx, &self.public_key).await } } @@ -1861,7 +1851,7 @@ impl Withdraw { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_withdraw(context, self).await } } @@ -1897,7 +1887,7 @@ impl ClaimRewards { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_claim_rewards(context, self).await } } @@ -2043,7 +2033,7 @@ impl CommissionRateChange { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_validator_commission_change(context, self).await } } @@ -2103,7 +2093,7 @@ impl ConsensusKeyChange { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_change_consensus_key(context, self).await } } @@ -2221,7 +2211,7 @@ impl MetaDataChange { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_validator_metadata_change(context, self).await } } @@ -2276,7 +2266,7 @@ impl UpdateStewardCommission { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_update_steward_commission(context, self).await } } @@ -2324,7 +2314,7 @@ impl ResignSteward { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_resign_steward(context, self).await } } @@ -2372,7 +2362,7 @@ impl TxUnjailValidator { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_unjail_validator(context, self).await } } @@ -2420,7 +2410,7 @@ impl TxDeactivateValidator { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_deactivate_validator(context, self).await } } @@ -2468,7 +2458,7 @@ impl TxReactivateValidator { pub async fn build( &self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { tx::build_reactivate_validator(context, self).await } } @@ -2632,23 +2622,51 @@ impl TxExpiration { } } +/// Argument for dry-runs +#[derive(Clone, Debug)] +pub enum DryRun { + /// Request a dry run of the inner transaction(s) only + Inner, + /// Request a dry run of both the inner transaction(s) and the wrapper + Wrapper, +} + +/// Argument for dumping transactions +#[derive(Clone, Debug)] +pub enum DumpTx { + /// Dump the raw transaction + Inner, + /// Dump the wrapper transaction + Wrapper, +} + +/// Arguments to request wrapping a transaction +#[derive(Clone, Debug)] +pub struct Wrapper { + /// Do not wait for the transaction to be added to the blockchain + pub broadcast_only: bool, + /// The amount being paid (for gas unit) to include the transaction + pub fee_amount: Option, + /// The fee payer signing key + pub wrapper_fee_payer: Option, + /// The token in which the fee is being paid + pub fee_token: C::AddrOrNativeToken, + /// The max amount of gas used to process tx + pub gas_limit: GasLimit, +} + /// Common transaction arguments #[derive(Clone, Debug)] pub struct Tx { - /// Simulate applying the transaction - pub dry_run: bool, - /// Simulate applying both the wrapper and inner transactions - pub dry_run_wrapper: bool, - /// Dump the raw transaction bytes to file - pub dump_tx: bool, - /// Dump the wrapper transaction bytes to file - pub dump_wrapper_tx: bool, + /// Simulate applying the transaction (possibly the wrapper too) + pub dry_run: Option, + /// Dump the transaction bytes to file, either the raw or the whole wrapper + /// transaction + pub dump_tx: Option, /// The output directory path to where serialize the data pub output_folder: Option, - /// Submit the transaction even if it doesn't pass client checks + /// Build the transaction even if it doesn't pass client checks pub force: bool, - /// Do not wait for the transaction to be added to the blockchain - pub broadcast_only: bool, /// The address of the ledger node as host:port pub ledger_address: C::ConfigRpcTendermintAddress, /// If any new account is initialized by the tx, use the given alias to @@ -2657,14 +2675,9 @@ pub struct Tx { /// Whether to force overwrite the above alias, if it is provided, in the /// wallet. pub wallet_alias_force: bool, - /// The amount being paid (for gas unit) to include the transaction - pub fee_amount: Option, - /// The fee payer signing key - pub wrapper_fee_payer: Option, - /// The token in which the fee is being paid - pub fee_token: C::AddrOrNativeToken, - /// The max amount of gas used to process tx - pub gas_limit: GasLimit, + /// Requests wrapping the transaction, optional in case only the raw tx + /// should be built + pub wrap_tx: Option>, /// The optional expiration of the transaction pub expiration: TxExpiration, /// The chain id for which the transaction is intended @@ -2715,18 +2728,11 @@ pub trait TxBuilder: Sized { where F: FnOnce(Tx) -> Tx; /// Simulate applying the transaction - fn dry_run(self, dry_run: bool) -> Self { + fn dry_run(self, dry_run: Option) -> Self { self.tx(|x| Tx { dry_run, ..x }) } - /// Simulate applying both the wrapper and inner transactions - fn dry_run_wrapper(self, dry_run_wrapper: bool) -> Self { - self.tx(|x| Tx { - dry_run_wrapper, - ..x - }) - } /// Dump the transaction bytes to file - fn dump_tx(self, dump_tx: bool) -> Self { + fn dump_tx(self, dump_tx: Option) -> Self { self.tx(|x| Tx { dump_tx, ..x }) } /// The output directory path to where serialize the data @@ -2740,10 +2746,10 @@ pub trait TxBuilder: Sized { fn force(self, force: bool) -> Self { self.tx(|x| Tx { force, ..x }) } - /// Do not wait for the transaction to be added to the blockchain - fn broadcast_only(self, broadcast_only: bool) -> Self { + /// Wrap the transaction + fn wrap_it(self, wrapper: Option>) -> Self { self.tx(|x| Tx { - broadcast_only, + wrap_tx: wrapper, ..x }) } @@ -2773,35 +2779,6 @@ pub trait TxBuilder: Sized { ..x }) } - /// The amount being paid (for gas unit) to include the transaction - fn fee_amount(self, fee_amount: InputAmount) -> Self { - self.tx(|x| Tx { - fee_amount: Some(fee_amount), - ..x - }) - } - /// The fee payer signing key - fn wrapper_fee_payer(self, wrapper_fee_payer: C::PublicKey) -> Self { - self.tx(|x| Tx { - wrapper_fee_payer: Some(wrapper_fee_payer), - ..x - }) - } - /// The token in which the fee is being paid - fn fee_token(self, fee_token: C::Address) -> Self { - self.tx(|x| Tx { - fee_token: fee_token.into(), - ..x - }) - } - /// The max amount of gas used to process tx - fn gas_limit(self, gas_limit: GasLimit) -> Self { - self.tx(|x| Tx { gas_limit, ..x }) - } - /// The optional expiration of the transaction - fn expiration(self, expiration: TxExpiration) -> Self { - self.tx(|x| Tx { expiration, ..x }) - } /// The chain id for which the transaction is intended fn chain_id(self, chain_id: ChainId) -> Self { self.tx(|x| Tx { @@ -3124,7 +3101,7 @@ impl EthereumBridgePool { pub async fn build( self, context: &impl Namada, - ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + ) -> crate::error::Result<(namada_tx::Tx, SigningData)> { bridge_pool::build_bridge_pool_tx(context, self).await } } diff --git a/crates/sdk/src/eth_bridge/bridge_pool.rs b/crates/sdk/src/eth_bridge/bridge_pool.rs index 0bce46d14a5..4bbb8c5c934 100644 --- a/crates/sdk/src/eth_bridge/bridge_pool.rs +++ b/crates/sdk/src/eth_bridge/bridge_pool.rs @@ -25,6 +25,7 @@ use namada_io::{Client, Io, display, display_line, edisplay_line}; use namada_token::Amount; use namada_token::storage_key::balance_key; use namada_tx::Tx; +use namada_tx::data::Fee; use owo_colors::OwoColorize; use serde::Serialize; @@ -41,9 +42,9 @@ use crate::queries::{ TransferToEthereumStatus, }; use crate::rpc::{query_storage_value, query_wasm_code_hash, validate_amount}; -use crate::signing::{aux_signing_data, validate_transparent_fee}; -use crate::tx::prepare_tx; -use crate::{MaybeSync, Namada, SigningTxData, args}; +use crate::signing::SigningData; +use crate::tx::{ExtendedWrapperArgs, WrapArgs, derive_build_data}; +use crate::{MaybeSync, Namada, args}; /// Craft a transaction that adds a transfer to the Ethereum bridge pool. pub async fn build_bridge_pool_tx( @@ -60,43 +61,42 @@ pub async fn build_bridge_pool_tx( fee_token, code_path, }: args::EthereumBridgePool, -) -> Result<(Tx, SigningTxData), Error> { +) -> Result<(Tx, SigningData), Error> { let sender_ = sender.clone(); - let (transfer, tx_code_hash, signing_data) = futures::try_join!( - validate_bridge_pool_tx( - context, - tx_args.force, - nut, - asset, - recipient, - sender, - amount, - fee_amount, - fee_payer, - fee_token, - ), - query_wasm_code_hash(context, code_path.to_string_lossy()), - aux_signing_data( - context, - &tx_args, - // token owner - Some(sender_.clone()), - // tx signer - Some(sender_), - vec![], - false, - vec![], - None - ), - )?; - let fee_payer = signing_data - .fee_payer - .clone() - .left() - .ok_or_else(|| Error::Other("Missing gas payer argument".to_string()))? - .0; - let (fee_amount, _) = - validate_transparent_fee(context, &tx_args, &fee_payer).await?; + let (signing_data, wrap_args, transfer, tx_code_hash) = { + let (transfer, tx_code_hash, (signing_data, wrap_args, _)) = futures::try_join!( + validate_bridge_pool_tx( + context, + tx_args.force, + nut, + asset, + recipient, + sender, + amount, + fee_amount, + fee_payer, + fee_token, + ), + query_wasm_code_hash(context, code_path.to_string_lossy()), + derive_build_data( + context, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false + }), + tx_args.force, + // tx signer + Some(sender_), + vec![], + vec![], + ), + )?; + + (signing_data, wrap_args, transfer, tx_code_hash) + }; let chain_id = tx_args .chain_id @@ -113,7 +113,22 @@ pub async fn build_bridge_pool_tx( ) .add_data(transfer); - prepare_tx(&tx_args, &mut tx, fee_amount, fee_payer).await?; + if let Some(WrapArgs { + fee_amount, + fee_payer, + fee_token, + gas_limit, + }) = wrap_args + { + tx.add_wrapper( + Fee { + amount_per_gas_unit: fee_amount, + token: fee_token, + }, + fee_payer, + gas_limit, + ); + } Ok((tx, signing_data)) } diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 9d2282f1336..677f0bdffe3 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -60,9 +60,7 @@ use namada_io::{Client, Io, NamadaIo}; pub use namada_io::{MaybeSend, MaybeSync}; pub use namada_token::masp::{ShieldedUtils, ShieldedWallet}; use namada_tx::Tx; -use namada_tx::data::wrapper::GasLimit; use rpc::{denominate_amount, format_denominated_amount, query_native_token}; -use signing::SigningTxData; use token::{DenominatedAmount, NATIVE_MAX_DECIMAL_PLACES}; use tokio::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use tx::{ @@ -125,23 +123,17 @@ pub trait Namada: NamadaIo { /// Make a tx builder using no arguments fn tx_builder(&self) -> args::Tx { args::Tx { - dry_run: false, - dry_run_wrapper: false, - dump_tx: false, - dump_wrapper_tx: false, + dry_run: None, + dump_tx: None, output_folder: None, force: false, - broadcast_only: false, ledger_address: tendermint_rpc::Url::from_str( "http://127.0.0.1:26657", ) .unwrap(), initialized_account_alias: None, wallet_alias_force: false, - fee_amount: None, - wrapper_fee_payer: None, - fee_token: self.native_token(), - gas_limit: GasLimit::from(DEFAULT_GAS_LIMIT), + wrap_tx: None, expiration: Default::default(), chain_id: None, signing_keys: vec![], @@ -611,7 +603,7 @@ pub trait Namada: NamadaIo { &self, tx: &mut Tx, args: &args::Tx, - signing_data: SigningTxData, + signing_data: signing::SigningData, with: impl Fn(Tx, common::PublicKey, signing::Signable, D) -> F + MaybeSend + MaybeSync, @@ -708,23 +700,17 @@ where io, native_token: native_token.clone(), prototype: args::Tx { - dry_run: false, - dry_run_wrapper: false, - dump_tx: false, - dump_wrapper_tx: false, + dry_run: None, + dump_tx: None, output_folder: None, force: false, - broadcast_only: false, ledger_address: tendermint_rpc::Url::from_str( "http://127.0.0.1:26657", ) .unwrap(), initialized_account_alias: None, wallet_alias_force: false, - fee_amount: None, - wrapper_fee_payer: None, - fee_token: native_token, - gas_limit: GasLimit::from(DEFAULT_GAS_LIMIT), + wrap_tx: None, expiration: Default::default(), chain_id: None, signing_keys: vec![], @@ -881,6 +867,7 @@ pub mod testing { BecomeValidator, Bond, CommissionChange, ConsensusKeyChange, MetaDataChange, Redelegation, Unbond, Withdraw, }; + use namada_tx::data::wrapper::GasLimit; use namada_tx::data::{Fee, TxType, WrapperTx}; use proptest::prelude::{Just, Strategy}; use proptest::sample::SizeRange; diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index d0a4074c453..f8ca8722fb4 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -13,7 +13,7 @@ use masp_primitives::transaction::components::sapling::fees::{ InputView, OutputView, }; use namada_account::{AccountPublicKeysMap, InitAccount, UpdateAccount}; -use namada_core::address::{Address, ImplicitAddress, InternalAddress, MASP}; +use namada_core::address::{Address, ImplicitAddress, MASP}; use namada_core::arith::checked; use namada_core::collections::{HashMap, HashSet}; use namada_core::ibc::primitives::IntoHostTime; @@ -36,8 +36,8 @@ use namada_parameters::storage as parameter_storage; use namada_token as token; use namada_token::storage_key::balance_key; use namada_tx::data::pgf::UpdateStewardCommission; +use namada_tx::data::pos; use namada_tx::data::pos::BecomeValidator; -use namada_tx::data::{Fee, pos}; use namada_tx::{ Authorization, MaspBuilder, Section, SignatureIndex, Signer, Tx, }; @@ -67,7 +67,7 @@ use crate::wallet::{Wallet, WalletIo}; use crate::{Namada, args, rpc}; /// A structure holding the signing data to craft a transaction -#[derive(Clone, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub struct SigningTxData { /// The address owning the transaction pub owner: Option
, @@ -77,25 +77,89 @@ pub struct SigningTxData { pub threshold: u8, /// The public keys to index map associated to an account pub account_public_keys_map: Option, - /// The fee payer, either a tuple with the public key to sign the wrapper - /// tx and a flag for the generation of a disposable fee payer or a - /// serialized signature - pub fee_payer: either::Either<(common::PublicKey, bool), Vec>, /// ID of the Transaction needing signing pub shielded_hash: Option, /// List of serialized signatures to attach to the transaction pub signatures: Vec>, } -impl SigningTxData { +/// The fee's authorization +#[derive(Clone, Debug)] +pub enum FeeAuthorization { + /// A wrapper signer + Signer { + /// The wrapper signer + pubkey: common::PublicKey, + /// Flag to mark a disposable fee payer + disposable_fee_payer: bool, + }, + /// A serialized signature + Signature(Vec), +} + +/// A structure holding the signing data for a wrapper transaction +#[derive(Clone, Debug)] +pub struct SigningWrapperData { + /// The signing data for each one of the inner transactions of this batch + pub signing_data: Vec, + /// The signing data for the wrapper itself + pub fee_auth: FeeAuthorization, +} + +impl SigningWrapperData { /// Returns the fee payer's public key if provided, otherwise returns an /// error. pub fn fee_payer_or_err(&self) -> Result<&common::PublicKey, Error> { - self.fee_payer - .as_ref() - .left() - .map(|(fee_payer, _)| fee_payer) - .ok_or_else(|| Error::Other("Missing gas payer".to_string())) + match &self.fee_auth { + FeeAuthorization::Signer { pubkey, .. } => Ok(pubkey), + FeeAuthorization::Signature(_) => Err(Error::Other( + "Expected wrapper signer, found wrapper signature".to_string(), + )), + } + } + + /// Returns whether fees will be paid from a disposable address + pub fn disposable_fee_payer(&self) -> bool { + match self.fee_auth { + FeeAuthorization::Signer { + pubkey: _, + disposable_fee_payer, + } => disposable_fee_payer, + FeeAuthorization::Signature(_) => false, + } + } +} + +#[allow(missing_docs)] +#[derive(Clone, Debug)] +pub enum SigningData { + Inner(SigningTxData), + Wrapper(SigningWrapperData), +} + +impl SigningData { + pub(crate) fn into_components( + self, + ) -> (Vec, Option) { + match self { + SigningData::Inner(signing_tx_data) => { + (vec![signing_tx_data], None) + } + SigningData::Wrapper(signing_wrapper_data) => ( + signing_wrapper_data.signing_data, + Some(signing_wrapper_data.fee_auth), + ), + } + } + + /// Returns a reference to the inner [`SigningTxData`] + pub fn signing_tx_data(&self) -> Vec<&SigningTxData> { + match self { + SigningData::Inner(signing_tx_data) => vec![signing_tx_data], + SigningData::Wrapper(signing_wrapper_data) => { + signing_wrapper_data.signing_data.iter().collect() + } + } } } @@ -165,37 +229,29 @@ pub fn find_key_by_pk( /// Given CLI arguments and some defaults, determine the rightful transaction /// signer. /// -/// Return the given signing key or public key of the given signer if -/// possible. If no explicit signer given, use the `default`. If no `default` -/// is given, an `Error` is returned. -pub async fn tx_signers( +/// When `signing_keys` are provided, these will be returned. Otherwise, if a +/// `default_signer` is provided, its key will be returned (provided that it can +/// be found in the wallet). Finally, if neither of the previous cases, an empty +/// set of signers will be returned. +async fn inner_tx_signers( context: &impl Namada, - args: &args::Tx, - default: Option
, - signatures: &[Vec], + signing_keys: Vec, + default_signer: Option<&Address>, ) -> Result, Error> { - let signer = if !args.signing_keys.is_empty() { - return Ok(args.signing_keys.clone().into_iter().collect()); - } else if signatures.is_empty() { - // Otherwise use the signer determined by the caller - default + let keys = if signing_keys.is_empty() { + match default_signer { + // When provided, use the signer determined by the caller, fetch the + // public key and apply it + Some(signer) => vec![find_pk(context, signer).await?], + // In this case return no signer + None => Default::default(), + } } else { - // If explicit signature(s) are provided signing keys are not required - // anymore - return Ok(Default::default()); + // Otherwise just use the provided public keys + signing_keys }; - // Now actually fetch the signing key and apply it - match signer { - // No signature needed if the source is MASP - Some(MASP) => Ok(Default::default()), - Some(signer) => Ok([find_pk(context, &signer).await?].into()), - None => other_err( - "All transactions must be signed; please either specify the key \ - or the address from which to look up the signing key." - .to_string(), - ), - } + Ok(keys.into_iter().collect()) } /// The different parts of a transaction that can be signed. Note that it's @@ -239,8 +295,7 @@ fn mock_hw_sig(tx: &mut Tx, pk: common::PublicKey, signable: Signable) { } CommonPublicKey::Secp256k1(_) => { // this key has no effect other than to satisfy api constraints - const SECRET_KEY_HEX: &str = - "c2c72dfbff11dfb4e9d5b0a20c620c58b15bb7552753601f043db91331b0db15"; + const SECRET_KEY_HEX: &str = "c2c72dfbff11dfb4e9d5b0a20c620c58b15bb7552753601f043db91331b0db15"; let sk_bytes = HEXLOWER.decode(SECRET_KEY_HEX.as_bytes()).unwrap(); let kp = secp256k1::SecretKey::try_from_slice(&sk_bytes[..]).unwrap(); @@ -262,8 +317,7 @@ fn mock_hw_sig(tx: &mut Tx, pk: common::PublicKey, signable: Signable) { /// If no explicit signer given, use the `default`. If no `default` is given, /// Error. /// -/// It also takes a second, optional keypair to sign the wrapper header -/// separately. +/// Optionally sign the wrapper header separately. /// /// If this is not a dry run, the tx is put in a wrapper and returned along with /// hashes needed for monitoring the tx on chain. @@ -273,7 +327,7 @@ pub async fn sign_tx( wallet: &RwLock>, args: &args::Tx, tx: &mut Tx, - signing_data: SigningTxData, + signing_data: SigningData, sign: impl Fn(Tx, common::PublicKey, Signable, D) -> F, user_data: D, ) -> Result<(), Error> @@ -282,11 +336,12 @@ where U: WalletIo, F: std::future::Future>, { - let mut used_pubkeys = HashSet::new(); + let (signing_data, fee_auth) = signing_data.into_components(); + for signing_tx_data in signing_data { + let mut used_pubkeys = HashSet::new(); - // First try to sign the raw header with the supplied signatures - if !signing_data.signatures.is_empty() { - let signatures = signing_data + // First try to sign the raw header with the supplied signatures + let signatures: Vec<_> = signing_tx_data .signatures .iter() .map(|bytes| { @@ -296,161 +351,170 @@ where sigidx }) .collect(); - tx.add_signatures(signatures); - } + if !signatures.is_empty() { + tx.add_signatures(signatures); + } - // Then try to sign the raw header with private keys in the software wallet - if let Some(account_public_keys_map) = signing_data.account_public_keys_map - { - let mut wallet = wallet.write().await; - let mut signing_tx_keypairs = vec![]; - - for public_key in &signing_data.public_keys { - if !used_pubkeys.contains(public_key) { - let Ok(secret_key) = - find_key_by_pk(&mut wallet, args, public_key) - else { - // If the secret key is not found, continue because the - // hardware wallet may still be able to sign this - continue; - }; - used_pubkeys.insert(public_key.clone()); - signing_tx_keypairs.push(secret_key); + // Then try to sign the raw header with private keys in the software + // wallet + if let Some(account_public_keys_map) = + signing_tx_data.account_public_keys_map + { + let mut wallet = wallet.write().await; + let mut signing_tx_keypairs = vec![]; + + for public_key in &signing_tx_data.public_keys { + if !used_pubkeys.contains(public_key) { + let Ok(secret_key) = + find_key_by_pk(&mut wallet, args, public_key) + else { + // If the secret key is not found, continue because the + // hardware wallet may still be able to sign this + continue; + }; + used_pubkeys.insert(public_key.clone()); + signing_tx_keypairs.push(secret_key); + } } - } - if !signing_tx_keypairs.is_empty() { - if args.dry_run || args.dry_run_wrapper { - tx.mock( - signing_tx_keypairs, - account_public_keys_map, - signing_data.owner, - ) - } else { - tx.sign_raw( - signing_tx_keypairs, - account_public_keys_map, - signing_data.owner, - ) - }; + if !signing_tx_keypairs.is_empty() { + if args.dry_run.is_some() { + tx.mock( + signing_tx_keypairs, + account_public_keys_map, + signing_tx_data.owner, + ) + } else { + tx.sign_raw( + signing_tx_keypairs, + account_public_keys_map, + signing_tx_data.owner, + ) + }; + } } - } - // Then try to sign the raw header using the hardware wallet - for pubkey in &signing_data.public_keys { - if !used_pubkeys.contains(pubkey) { - match &signing_data.fee_payer { - either::Either::Left((fee_payer, _)) if pubkey == fee_payer => { - } - _ => { - if args.dry_run || args.dry_run_wrapper { - mock_hw_sig(tx, pubkey.clone(), Signable::RawHeader); - used_pubkeys.insert(pubkey.clone()); - } else if let Ok(ntx) = sign( - tx.clone(), - pubkey.clone(), - Signable::RawHeader, - user_data.clone(), - ) - .await - { - *tx = ntx; + // Then try to sign the raw header using the hardware wallet + for pubkey in &signing_tx_data.public_keys { + if !used_pubkeys.contains(pubkey) { + match &fee_auth { + Some(FeeAuthorization::Signer { + pubkey: fee_payer, + .. + }) if pubkey == fee_payer => { + // We will sign the inner tx together with the wrapper, + // we can anticipate the accounting of this signature used_pubkeys.insert(pubkey.clone()); } + _ => { + if args.dry_run.is_some() { + mock_hw_sig( + tx, + pubkey.clone(), + Signable::RawHeader, + ); + used_pubkeys.insert(pubkey.clone()); + } else if let Ok(ntx) = sign( + tx.clone(), + pubkey.clone(), + Signable::RawHeader, + user_data.clone(), + ) + .await + { + *tx = ntx; + used_pubkeys.insert(pubkey.clone()); + } + } } } } + + // Then make sure that the number of public keys used is greater than or + // equal to the threshold + let used_pubkeys_len = used_pubkeys + .len() + .try_into() + .expect("Public keys associated with account exceed 127"); + if used_pubkeys_len < signing_tx_data.threshold { + return Err(Error::from(TxSubmitError::MissingSigningKeys( + signing_tx_data.threshold, + used_pubkeys_len, + ))); + } } // Before signing the wrapper tx prune all the possible duplicated sections // (including duplicated raw signatures) tx.prune_duplicated_sections(); - // Then try signing the wrapper header (fee payer). Check if there's a - // provided wrapper signature, otherwise sign with the software wallet or - // use the fallback - match &signing_data.fee_payer { - either::Either::Left((fee_payer, _)) => { + // Then try signing the wrapper header (fee payer) if requested. Check if + // there's a provided wrapper signature, otherwise sign with the + // software wallet or use the fallback + match &fee_auth { + Some(FeeAuthorization::Signer { + pubkey, + disposable_fee_payer: _, + }) => { let key = { // Lock the wallet just long enough to extract a key from it // without interfering with the sign closure // call let mut wallet = wallet.write().await; - find_key_by_pk(&mut *wallet, args, fee_payer) + find_key_by_pk(&mut *wallet, args, pubkey) }; match key { Ok(fee_payer_keypair) => { - if args.dry_run || args.dry_run_wrapper { + if args.dry_run.is_some() { tx.mock_sign_wrapper(fee_payer_keypair); } else { tx.sign_wrapper(fee_payer_keypair); } } Err(_) => { - if args.dry_run || args.dry_run_wrapper { - mock_hw_sig( - tx, - fee_payer.clone(), - Signable::FeeRawHeader, - ); + if args.dry_run.is_some() { + mock_hw_sig(tx, pubkey.clone(), Signable::FeeRawHeader); } else { *tx = sign( tx.clone(), - fee_payer.clone(), + pubkey.clone(), Signable::FeeRawHeader, user_data, ) .await?; } - if signing_data.public_keys.contains(fee_payer) { - used_pubkeys.insert(fee_payer.clone()); - } } } } - either::Either::Right(sig_bytes) => { + Some(FeeAuthorization::Signature(sig_bytes)) => { let auth = serde_json::from_slice(sig_bytes).map_err(|e| { Error::Encode(EncodingError::Serde(e.to_string())) })?; tx.add_section(Section::Authorization(auth)); } + _ => (), } // Remove redundant sections now that the signing process is complete. // Though this call might be redundant in circumstances, it is placed here // as a safeguard to prevent the transmission of private data to the // network. tx.protocol_filter(); - // Then make sure that the number of public keys used exceeds the threshold - let used_pubkeys_len = used_pubkeys - .len() - .try_into() - .expect("Public keys associated with account exceed 127"); - if used_pubkeys_len < signing_data.threshold { - Err(Error::from(TxSubmitError::MissingSigningKeys( - signing_data.threshold, - used_pubkeys_len, - ))) - } else { - Ok(()) - } + + Ok(()) } -/// Return the necessary data regarding an account to be able to generate a -/// signature section -#[allow(clippy::too_many_arguments)] +/// Return the necessary data regarding an account to be able to generate +/// signature sections for both the inner and the wrapper transaction pub async fn aux_signing_data( context: &impl Namada, - args: &args::Tx, + wrap_tx: &args::Wrapper, owner: Option
, - default_signer: Option
, - extra_public_keys: Vec, - is_shielded_source: bool, + signing_keys: Vec, + disposable_gas_payer: bool, signatures: Vec>, - wrapper_signature: Option>, -) -> Result { - let mut public_keys = - tx_signers(context, args, default_signer.clone(), &signatures).await?; - public_keys.extend(extra_public_keys.clone()); +) -> Result { + let public_keys = + inner_tx_signers(context, signing_keys, owner.as_ref()).await?; let (account_public_keys_map, threshold) = match &owner { Some(owner @ Address::Established(_)) => { @@ -468,45 +532,101 @@ pub async fn aux_signing_data( Some(AccountPublicKeysMap::from_iter(public_keys.clone())), 1u8, ), - Some(owner @ Address::Internal(internal)) => match internal { - InternalAddress::Masp => (None, 0u8), - _ => { + Some(owner @ Address::Internal(_)) => { + return Err(Error::from(TxSubmitError::InvalidAccount( + owner.encode(), + ))); + } + None => ( + Some(AccountPublicKeysMap::from_iter(public_keys.clone())), + 0u8, + ), + }; + let fee_auth = match wrap_tx { + args::Wrapper { + wrapper_fee_payer: Some(pubkey), + .. + } => FeeAuthorization::Signer { + pubkey: pubkey.to_owned(), + disposable_fee_payer: false, + }, + _ => { + if let Some(pubkey) = public_keys.first() { + FeeAuthorization::Signer { + pubkey: pubkey.to_owned(), + disposable_fee_payer: false, + } + // NOTE: the disposable gas_payer is always overridden by + // signing inputs + } else if disposable_gas_payer { + FeeAuthorization::Signer { + pubkey: gen_disposable_signing_key(context).await, + disposable_fee_payer: true, + } + } else { + return Err(Error::Tx(TxSubmitError::InvalidFeePayer)); + } + } + }; + + let signing_inner_data = SigningTxData { + owner, + public_keys, + threshold, + account_public_keys_map, + shielded_hash: None, + signatures, + }; + + Ok(SigningWrapperData { + signing_data: vec![signing_inner_data], + fee_auth, + }) +} + +/// Return the necessary data regarding an account to be able to generate a +/// signature section for the inner transaction only +pub async fn aux_inner_signing_data( + context: &impl Namada, + signing_keys: Vec, + owner: Option
, + signatures: Vec>, +) -> Result { + let public_keys = + inner_tx_signers(context, signing_keys, owner.as_ref()).await?; + + let (account_public_keys_map, threshold) = match &owner { + Some(owner @ Address::Established(_)) => { + let account = + rpc::get_account_info(context.client(), owner).await?; + if let Some(account) = account { + (Some(account.clone().public_keys_map), account.threshold) + } else { return Err(Error::from(TxSubmitError::InvalidAccount( owner.encode(), ))); } - }, + } + Some(Address::Implicit(_)) => ( + Some(AccountPublicKeysMap::from_iter(public_keys.clone())), + 1u8, + ), + Some(owner @ Address::Internal(_)) => { + return Err(Error::from(TxSubmitError::InvalidAccount( + owner.encode(), + ))); + } None => ( Some(AccountPublicKeysMap::from_iter(public_keys.clone())), 0u8, ), }; - let fee_payer = match &wrapper_signature { - Some(signature) => either::Right(signature.to_owned()), - None => match &args.wrapper_fee_payer { - Some(pubkey) => either::Left((pubkey.clone(), false)), - None => { - if let Some(pubkey) = public_keys.first() { - either::Left((pubkey.to_owned(), false)) - } else if is_shielded_source { - either::Left(( - gen_disposable_signing_key(context).await, - true, - )) - } else { - return Err(Error::Tx(TxSubmitError::InvalidFeePayer)); - } - } - }, - }; - Ok(SigningTxData { owner, public_keys, threshold, account_public_keys_map, - fee_payer, shielded_hash: None, signatures, }) @@ -600,7 +720,12 @@ pub struct TxSourcePostBalance { /// Validate the fee amount and token pub async fn validate_fee( context: &N, - args: &args::Tx, + args::Wrapper { + fee_amount, + fee_token, + .. + }: &args::Wrapper, + force: bool, ) -> Result { let gas_cost_key = parameter_storage::get_gas_cost_key(); let minimum_fee = match rpc::query_storage_value::< @@ -609,38 +734,35 @@ pub async fn validate_fee( >(context.client(), &gas_cost_key) .await .and_then(|map| { - map.get(&args.fee_token) - .map(ToOwned::to_owned) - .ok_or_else(|| { - Error::Other(format!( - "Could not retrieve from storage the gas cost for token {}", - args.fee_token - )) - }) + map.get(fee_token).map(ToOwned::to_owned).ok_or_else(|| { + Error::Other(format!( + "Could not retrieve from storage the gas cost for token {}", + fee_token + )) + }) }) { Ok(amount) => amount, Err(e) => { - if !args.force { + if !force { return Err(e); } else { token::Amount::zero() } } }; - let validated_minimum_fee = context - .denominate_amount(&args.fee_token, minimum_fee) - .await; + let validated_minimum_fee = + context.denominate_amount(fee_token, minimum_fee).await; - let fee_amount = match args.fee_amount { + let fee_amount = match fee_amount { Some(amount) => { let validated_fee_amount = - validate_amount(context, amount, &args.fee_token, args.force) + validate_amount(context, *amount, fee_token, force) .await .expect("Expected to be able to validate fee"); if validated_fee_amount >= validated_minimum_fee { validated_fee_amount - } else if !args.force { + } else if !force { // Update the fee amount if it's not enough display_line!( context.io(), @@ -664,13 +786,19 @@ pub async fn validate_fee( /// computing the updated post balance pub async fn validate_transparent_fee( context: &N, - args: &args::Tx, + args: &args::Wrapper, + force: bool, fee_payer: &common::PublicKey, ) -> Result<(DenominatedAmount, TxSourcePostBalance), Error> { - let fee_amount = validate_fee(context, args).await?; + let fee_amount = validate_fee(context, args, force).await?; + let args::Wrapper { + fee_token, + gas_limit, + .. + } = args; let fee_payer_address = Address::from(fee_payer); - let balance_key = balance_key(&args.fee_token, &fee_payer_address); + let balance_key = balance_key(fee_token, &fee_payer_address); #[allow(clippy::disallowed_methods)] let balance = rpc::query_storage_value::<_, token::Amount>( context.client(), @@ -679,17 +807,18 @@ pub async fn validate_transparent_fee( .await .unwrap_or_default(); - let total_fee = checked!(fee_amount.amount() * u64::from(args.gas_limit))?; + let total_fee = + checked!(fee_amount.amount() * u64::from(gas_limit.to_owned()))?; let mut updated_balance = TxSourcePostBalance { post_balance: balance, source: fee_payer_address.clone(), - token: args.fee_token.clone(), + token: fee_token.clone(), }; match total_fee.checked_sub(balance) { Some(diff) if !diff.is_zero() => { - let token_addr = args.fee_token.clone(); - if !args.force { + let token_addr = fee_token.clone(); + if !force { let fee_amount = context.format_amount(&token_addr, total_fee).await; @@ -713,28 +842,6 @@ pub async fn validate_transparent_fee( Ok((fee_amount, updated_balance)) } -/// Create a wrapper tx from a normal tx. Get the hash of the -/// wrapper and its payload which is needed for monitoring its -/// progress on chain. -pub async fn wrap_tx( - tx: &mut Tx, - args: &args::Tx, - fee_amount: DenominatedAmount, - fee_payer: common::PublicKey, -) -> Result<(), Error> { - tx.add_wrapper( - Fee { - amount_per_gas_unit: fee_amount, - token: args.fee_token.clone(), - }, - fee_payer, - // TODO(namada#1625): partially validate the gas limit in client - args.gas_limit, - ); - - Ok(()) -} - #[allow(clippy::result_large_err)] fn other_err(string: String) -> Result { Err(Error::Other(string)) @@ -2286,6 +2393,7 @@ mod test_signing { use assert_matches::assert_matches; use masp_primitives::consensus::BlockHeight; use masp_primitives::transaction::components::sapling::builder::SaplingMetadata; + use namada_core::address::InternalAddress; use namada_core::chain::ChainId; use namada_core::hash::Hash; use namada_core::ibc::PGFIbcTarget; @@ -2308,23 +2416,23 @@ mod test_signing { fn arbitrary_args() -> args::Tx { args::Tx { - dry_run: false, - dry_run_wrapper: false, - dump_tx: false, - dump_wrapper_tx: false, + dry_run: None, + dump_tx: None, output_folder: None, force: false, - broadcast_only: false, ledger_address: tendermint_rpc::Url::from_str( "http://127.0.0.1:42", ) .expect("Test failed"), initialized_account_alias: None, wallet_alias_force: false, - fee_amount: None, - wrapper_fee_payer: None, - fee_token: Address::Internal(InternalAddress::Governance), - gas_limit: namada_tx::data::GasLimit::from(2), + wrap_tx: Some(args::Wrapper { + broadcast_only: false, + fee_amount: None, + wrapper_fee_payer: None, + fee_token: Address::Internal(InternalAddress::Governance), + gas_limit: namada_tx::data::GasLimit::from(2), + }), expiration: Default::default(), chain_id: None, signing_keys: vec![], @@ -2506,14 +2614,6 @@ mod test_signing { .expect_err("Test failed"); } - #[tokio::test] - async fn test_tx_signers_failure() { - let args = arbitrary_args(); - tx_signers(&TestNamadaImpl::new(None).0, &args, None, &[]) - .await - .expect_err("Test failed"); - } - /// Test the unhappy flows in trying to validate /// the fee token and amounts, both with and without /// the force argument set. @@ -2529,7 +2629,7 @@ mod test_signing { // we should fail to validate the fee due to an unresponsive client client_handle.send(None).expect("Test failed"); let Error::Query(crate::error::QueryError::NoResponse(msg)) = - validate_fee(&context, &args) + validate_fee(&context, args.wrap_tx.as_ref().unwrap(), args.force) .await .expect_err("Test failed") else { @@ -2541,51 +2641,79 @@ mod test_signing { // client is unresponsive client_handle.send(None).expect("Test failed"); args.force = true; - let fee = validate_fee(&context, &args).await.expect("Test failed"); + let fee = + validate_fee(&context, args.wrap_tx.as_ref().unwrap(), args.force) + .await + .expect("Test failed"); assert_eq!(fee, DenominatedAmount::new(Amount::zero(), 0.into())); + let args::Wrapper { + broadcast_only, + wrapper_fee_payer, + fee_token, + gas_limit, + .. + } = args.wrap_tx.as_ref().unwrap().to_owned(); // now validation should the minimum fee from the client instead of // the args as force is false args.force = false; client_handle .send(Some(EncodedResponseQuery { - data: BTreeMap::from([( - args.fee_token.clone(), - Amount::from(100), - )]) - .serialize_to_vec(), + data: BTreeMap::from([(fee_token.clone(), Amount::from(100))]) + .serialize_to_vec(), info: "".to_string(), proof: None, height: Default::default(), })) .expect("Test failed"); - args.fee_amount = Some(InputAmount::Validated(DenominatedAmount::new( - Amount::from_u64(1), - 0.into(), - ))); - let fee = validate_fee(&context, &args).await.expect("Test failed"); + args.wrap_tx = Some(args::Wrapper { + broadcast_only, + wrapper_fee_payer, + fee_token, + gas_limit, + fee_amount: Some(InputAmount::Validated(DenominatedAmount::new( + Amount::from_u64(1), + 0.into(), + ))), + }); + let fee = + validate_fee(&context, args.wrap_tx.as_ref().unwrap(), args.force) + .await + .expect("Test failed"); assert_eq!(fee, DenominatedAmount::new(Amount::from(100), 0.into())); // now validation should ignore the minimum fee from the client // as force is true args.force = true; + let args::Wrapper { + broadcast_only, + wrapper_fee_payer, + fee_token, + gas_limit, + .. + } = args.wrap_tx.as_ref().unwrap().to_owned(); client_handle .send(Some(EncodedResponseQuery { - data: BTreeMap::from([( - args.fee_token.clone(), - Amount::from(100), - )]) - .serialize_to_vec(), + data: BTreeMap::from([(fee_token.clone(), Amount::from(100))]) + .serialize_to_vec(), info: "".to_string(), proof: None, height: Default::default(), })) .expect("Test failed"); - args.fee_amount = Some(InputAmount::Validated(DenominatedAmount::new( - Amount::from_u64(1), - 0.into(), - ))); - let fee = validate_fee(&context, &args).await.expect("Test failed"); + args.wrap_tx = Some(args::Wrapper { + broadcast_only, + wrapper_fee_payer, + fee_token, + gas_limit, + fee_amount: Some(InputAmount::Validated(DenominatedAmount::new( + Amount::from_u64(1), + 0.into(), + ))), + }); + let fee = validate_fee(&context, &args.wrap_tx.unwrap(), args.force) + .await + .expect("Test failed"); assert_eq!(fee, DenominatedAmount::new(Amount::from(1), 0.into())); } @@ -2603,7 +2731,7 @@ mod test_signing { client_handle .send(Some(EncodedResponseQuery { data: BTreeMap::from([( - args.fee_token.clone(), + args.wrap_tx.as_ref().unwrap().fee_token.clone(), Amount::from(100), )]) .serialize_to_vec(), @@ -2618,7 +2746,13 @@ mod test_signing { let public_key = secret_key.to_public(); assert_matches!( - validate_transparent_fee(&context, &args, &public_key).await, + validate_transparent_fee( + &context, + &args.wrap_tx.unwrap(), + args.force, + &public_key + ) + .await, Err(Error::Tx(TxSubmitError::BalanceTooLowForFees(_, _, _, _))) ); } @@ -2641,21 +2775,26 @@ mod test_signing { >()); let public_key_fee = secret_key_fee.to_public(); let mut tx = Tx::new(ChainId::default(), None); - let signing_data = SigningTxData { - owner: None, - public_keys: [public_key.clone()].into(), - threshold: 1, - account_public_keys_map: Some(Default::default()), - fee_payer: either::Either::Left((public_key_fee.clone(), false)), - shielded_hash: None, - signatures: vec![], + let signing_data = SigningWrapperData { + signing_data: vec![SigningTxData { + owner: None, + public_keys: [public_key.clone()].into(), + threshold: 1, + account_public_keys_map: Some(Default::default()), + shielded_hash: None, + signatures: vec![], + }], + fee_auth: FeeAuthorization::Signer { + pubkey: public_key_fee.clone(), + disposable_fee_payer: false, + }, }; let Error::Tx(TxSubmitError::MissingSigningKeys(1, 0)) = sign_tx( &RwLock::new(wallet), &args, &mut tx, - signing_data, + SigningData::Wrapper(signing_data), |tx, pk, _, _| { let pkf = public_key_fee.clone(); async move { @@ -2678,20 +2817,25 @@ mod test_signing { // This should now work let wallet = Wallet::::new(TestWalletUtils, Default::default()); - let signing_data = SigningTxData { - owner: None, - public_keys: [public_key.clone()].into(), - threshold: 1, - account_public_keys_map: Some(Default::default()), - fee_payer: either::Left((public_key.clone(), false)), - shielded_hash: None, - signatures: vec![], + let signing_data = SigningWrapperData { + signing_data: vec![SigningTxData { + owner: None, + public_keys: [public_key.clone()].into(), + threshold: 1, + account_public_keys_map: Some(Default::default()), + shielded_hash: None, + signatures: vec![], + }], + fee_auth: FeeAuthorization::Signer { + pubkey: public_key.clone(), + disposable_fee_payer: false, + }, }; sign_tx( &RwLock::new(wallet), &args, &mut tx, - signing_data, + SigningData::Wrapper(signing_data), |tx, _, _, _| async { Ok(tx) }, (), ) diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index e502da71d6f..6eec4e17925 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; use std::time::Duration; use borsh::BorshSerialize; +use data::{Fee, GasLimit}; use masp_primitives::asset_type::AssetType; use masp_primitives::transaction::Transaction as MaspTransaction; use masp_primitives::transaction::builder::Builder; @@ -75,7 +76,7 @@ pub use namada_tx::{Authorization, *}; use num_traits::Zero; use rand_core::{OsRng, RngCore}; -use crate::args::{SdkTypes, TxTransparentSource, TxTransparentTarget}; +use crate::args::{TxTransparentSource, TxTransparentTarget, Wrapper}; use crate::borsh::BorshSerializeExt; use crate::control_flow::time; use crate::error::{EncodingError, Error, QueryError, Result, TxSubmitError}; @@ -84,7 +85,8 @@ use crate::rpc::{ query_wasm_code_hash, validate_amount, }; use crate::signing::{ - self, SigningTxData, validate_fee, validate_transparent_fee, + self, FeeAuthorization, SigningData, SigningTxData, SigningWrapperData, + TxSourcePostBalance, validate_fee, validate_transparent_fee, }; use crate::tendermint_rpc::endpoint::broadcast::tx_sync::Response; use crate::tendermint_rpc::error::Error as RpcError; @@ -189,12 +191,20 @@ impl ProcessTxResponse { } /// Build and dump a transaction either to file or to screen -pub fn dump_tx(io: &IO, args: &args::Tx, mut tx: Tx) -> Result<()> { - if args.dump_tx { - tx.update_header(data::TxType::Raw); +pub fn dump_tx( + io: &IO, + dump_tx: args::DumpTx, + output_folder: Option, + mut tx: Tx, +) -> Result<()> { + let is_wrapper_tx = tx.header.wrapper().is_some(); + if matches!(dump_tx, args::DumpTx::Inner) && is_wrapper_tx { + return Err(Error::Other( + "Requested tx-dump on a tx which is a wrapper".to_string(), + )); }; - if args.dump_wrapper_tx && tx.header.wrapper().is_none() { + if matches!(dump_tx, args::DumpTx::Wrapper) && !is_wrapper_tx { return Err(Error::Other( "Requested wrapper-dump on a tx which is not a wrapper".to_string(), )); @@ -204,7 +214,7 @@ pub fn dump_tx(io: &IO, args: &args::Tx, mut tx: Tx) -> Result<()> { // dumped tx needed to be signed offline tx.prune_duplicated_sections(); - match args.output_folder.clone() { + match output_folder { Some(path) => { let tx_path = path.join(format!( "{}.tx", @@ -231,21 +241,6 @@ pub fn dump_tx(io: &IO, args: &args::Tx, mut tx: Tx) -> Result<()> { Ok(()) } -/// Prepare a transaction for signing and submission by adding a wrapper header -/// to it. -pub async fn prepare_tx( - args: &args::Tx, - tx: &mut Tx, - fee_amount: DenominatedAmount, - fee_payer: common::PublicKey, -) -> Result<()> { - if args.dry_run || args.dump_tx { - Ok(()) - } else { - signing::wrap_tx(tx, args, fee_amount, fee_payer).await - } -} - /// Submit transaction and wait for result. Returns a list of addresses /// initialized in the transaction if any. In dry run, this is always empty. pub async fn process_tx( @@ -263,17 +258,39 @@ pub async fn process_tx( // let request_body = request.into_json(); // println!("HTTP request body: {}", request_body); - if args.dry_run || args.dry_run_wrapper { + if let Some(dry_run) = &args.dry_run { + let is_wrapper_tx = tx.header.wrapper().is_some(); + if matches!(dry_run, args::DryRun::Inner) && is_wrapper_tx { + return Err(Error::Other( + "Requested tx-dry-run on a tx which is a wrapper".to_string(), + )); + }; + + if matches!(dry_run, args::DryRun::Wrapper) && !is_wrapper_tx { + return Err(Error::Other( + "Requested wrapper-dry-run on a tx which is not a wrapper" + .to_string(), + )); + } expect_dry_broadcast(TxBroadcastData::DryRun(tx), context).await } else { // We use this to determine when the wrapper tx makes it on-chain let tx_hash = tx.header_hash().to_string(); let cmts = tx.commitments().clone(); let wrapper_hash = tx.wrapper_hash(); + if wrapper_hash.is_none() { + return Err(Error::Other( + "Can't submit a non-wrapper transaction".to_string(), + )); + } // We use this to determine when the inner tx makes it // on-chain let to_broadcast = TxBroadcastData::Live { tx, tx_hash }; - if args.broadcast_only { + if args + .wrap_tx + .as_ref() + .is_some_and(|wrap_tx| wrap_tx.broadcast_only) + { broadcast_tx(context, &to_broadcast) .await .map(ProcessTxResponse::Broadcast) @@ -325,21 +342,19 @@ pub async fn build_reveal_pk( context: &impl Namada, args: &args::Tx, public_key: &common::PublicKey, -) -> Result<(Tx, SigningTxData)> { - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - args, + args.wrap_tx.as_ref().map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + args.force, None, - Some(public_key.into()), - vec![], - false, + args.signing_keys.to_owned(), vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, args, fee_payer).await?; build( context, @@ -347,8 +362,7 @@ pub async fn build_reveal_pk( args.tx_reveal_code_path.clone(), public_key, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -669,7 +683,7 @@ pub async fn build_change_consensus_key( tx_code_path, unsafe_dont_encrypt: _, }: &args::ConsensusKeyChange, -) -> Result<(Tx, SigningTxData)> { +) -> Result<(Tx, SigningData)> { let consensus_key = if let Some(consensus_key) = consensus_key { consensus_key } else { @@ -695,30 +709,34 @@ pub async fn build_change_consensus_key( consensus_key: consensus_key.clone(), }; - let signing_data = signing::aux_signing_data( + let signing_keys = [ + tx_args.signing_keys.to_owned(), + vec![consensus_key.to_owned()], + ] + .concat(); + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, - None, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, None, - vec![consensus_key.clone()], - false, + signing_keys, vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _updated_balance) = - validate_transparent_fee(context, tx_args, fee_payer).await?; - build( context, tx_args, tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -733,22 +751,22 @@ pub async fn build_validator_commission_change( rate, tx_code_path, }: &args::CommissionRateChange, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(validator.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, Some(validator.clone()), - default_signer, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; let epoch = rpc::query_epoch(context.client()).await?; @@ -856,8 +874,7 @@ pub async fn build_validator_commission_change( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -878,22 +895,22 @@ pub async fn build_validator_metadata_change( commission_rate, tx_code_path, }: &args::MetaDataChange, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(validator.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, Some(validator.clone()), - default_signer, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; let epoch = rpc::query_epoch(context.client()).await?; @@ -1090,8 +1107,7 @@ pub async fn build_validator_metadata_change( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -1106,22 +1122,22 @@ pub async fn build_update_steward_commission( commission, tx_code_path, }: &args::UpdateStewardCommission, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(steward.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, Some(steward.clone()), - default_signer, - vec![], - false, + tx_args.signing_keys.to_owned(), vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; if !rpc::is_steward(context.client(), steward).await { edisplay_line!( @@ -1162,8 +1178,7 @@ pub async fn build_update_steward_commission( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -1177,22 +1192,22 @@ pub async fn build_resign_steward( steward, tx_code_path, }: &args::ResignSteward, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(steward.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, Some(steward.clone()), - default_signer, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; if !rpc::is_steward(context.client(), steward).await { edisplay_line!( @@ -1213,8 +1228,7 @@ pub async fn build_resign_steward( tx_code_path.clone(), steward.clone(), do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -1228,22 +1242,22 @@ pub async fn build_unjail_validator( validator, tx_code_path, }: &args::TxUnjailValidator, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(validator.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, Some(validator.clone()), - default_signer, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; if !rpc::is_validator(context.client(), validator).await? { edisplay_line!( @@ -1320,8 +1334,7 @@ pub async fn build_unjail_validator( tx_code_path.clone(), validator.clone(), do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -1335,22 +1348,22 @@ pub async fn build_deactivate_validator( validator, tx_code_path, }: &args::TxDeactivateValidator, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(validator.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, Some(validator.clone()), - default_signer, - vec![], - false, + tx_args.signing_keys.to_owned(), vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; // Check if the validator address is actually a validator if !rpc::is_validator(context.client(), validator).await? { @@ -1398,8 +1411,7 @@ pub async fn build_deactivate_validator( tx_code_path.clone(), validator.clone(), do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -1413,22 +1425,22 @@ pub async fn build_reactivate_validator( validator, tx_code_path, }: &args::TxReactivateValidator, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(validator.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, Some(validator.clone()), - default_signer, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; // Check if the validator address is actually a validator if !rpc::is_validator(context.client(), validator).await? { @@ -1475,8 +1487,7 @@ pub async fn build_reactivate_validator( tx_code_path.clone(), validator.clone(), do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -1493,7 +1504,7 @@ pub async fn build_redelegation( amount: redel_amount, tx_code_path, }: &args::Redelegate, -) -> Result<(Tx, SigningTxData)> { +) -> Result<(Tx, SigningData)> { // Require a positive amount of tokens to be redelegated if redel_amount.is_zero() { edisplay_line!( @@ -1643,22 +1654,21 @@ pub async fn build_redelegation( ); } - let default_address = owner.clone(); - let default_signer = Some(default_address.clone()); - let signing_data = signing::aux_signing_data( + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, - Some(default_address), - default_signer, - vec![], - false, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, + Some(owner.clone()), + tx_args.signing_keys.to_owned(), vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; let data = pos::Redelegation { src_validator, @@ -1673,8 +1683,7 @@ pub async fn build_redelegation( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -1689,23 +1698,23 @@ pub async fn build_withdraw( source, tx_code_path, }: &args::Withdraw, -) -> Result<(Tx, SigningTxData)> { - let default_address = source.clone().unwrap_or(validator.clone()); - let default_signer = Some(default_address.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let default_signer = Some(source.clone().unwrap_or(validator.clone())); + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, - Some(default_address), + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, default_signer, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; let epoch = rpc::query_epoch(context.client()).await?; @@ -1763,8 +1772,7 @@ pub async fn build_withdraw( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -1779,23 +1787,23 @@ pub async fn build_claim_rewards( source, tx_code_path, }: &args::ClaimRewards, -) -> Result<(Tx, SigningTxData)> { - let default_address = source.clone().unwrap_or(validator.clone()); - let default_signer = Some(default_address.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let default_signer = Some(source.clone().unwrap_or(validator.clone())); + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, - Some(default_address), + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, default_signer, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; // Check that the validator address is actually a validator let validator = @@ -1818,8 +1826,7 @@ pub async fn build_claim_rewards( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -1835,7 +1842,7 @@ pub async fn build_unbond( source, tx_code_path, }: &args::Unbond, -) -> Result<(Tx, SigningTxData, Option<(Epoch, token::Amount)>)> { +) -> Result<(Tx, SigningData, Option<(Epoch, token::Amount)>)> { // Require a positive amount of tokens to be bonded if amount.is_zero() { edisplay_line!( @@ -1887,22 +1894,22 @@ pub async fn build_unbond( } } - let default_address = source.clone().unwrap_or(validator.clone()); - let default_signer = Some(default_address.clone()); - let signing_data = signing::aux_signing_data( + let default_signer = Some(source.clone().unwrap_or(validator.clone())); + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, - Some(default_address), + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, default_signer, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; // Check the source's current bond amount let bond_source = source.clone().unwrap_or_else(|| validator.clone()); @@ -1960,8 +1967,7 @@ pub async fn build_unbond( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await?; Ok((tx, signing_data, latest_withdrawal_pre)) @@ -2041,6 +2047,74 @@ pub async fn query_unbonds( Ok(()) } +pub(crate) struct ExtendedWrapperArgs<'args> { + pub(crate) wrap_args: &'args Wrapper, + pub(crate) disposable_gas_payer: bool, +} + +// Given the SDK arguments, extracts the necessary data to properly build the +// transaction, which are: the [`SigningData`] and the optional [`WrapArgs`] and +// updated balance +pub(crate) async fn derive_build_data<'args>( + context: &impl Namada, + wrap_args: Option>, + force: bool, + default_signer: Option
, + signing_keys: Vec, + signatures: Vec>, +) -> Result<(SigningData, Option, Option)> { + match wrap_args { + Some(ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer, + }) => { + let signing_data = signing::aux_signing_data( + context, + wrap_args, + default_signer, + signing_keys, + disposable_gas_payer, + signatures, + ) + .await?; + let fee_payer = signing_data.fee_payer_or_err()?.to_owned(); + let (fee_amount, updated_balance) = if disposable_gas_payer { + // MASP fee payment + (validate_fee(context, wrap_args, force).await?, None) + } else { + // Transparent fee payment + validate_transparent_fee(context, wrap_args, force, &fee_payer) + .await + .map(|(fee_amount, updated_balance)| { + (fee_amount, Some(updated_balance)) + })? + }; + + Ok(( + SigningData::Wrapper(signing_data), + Some(WrapArgs { + fee_amount, + fee_payer, + fee_token: wrap_args.fee_token.to_owned(), + gas_limit: wrap_args.gas_limit, + }), + updated_balance, + )) + } + None => { + let signing_data = signing::aux_inner_signing_data( + context, + signing_keys, + default_signer, + signatures, + ) + .await?; + + Ok((SigningData::Inner(signing_data), None, None)) + } + } +} + /// Submit a transaction to bond pub async fn build_bond( context: &impl Namada, @@ -2051,7 +2125,7 @@ pub async fn build_bond( source, tx_code_path, }: &args::Bond, -) -> Result<(Tx, SigningTxData)> { +) -> Result<(Tx, SigningData)> { // Require a positive amount of tokens to be bonded if amount.is_zero() { edisplay_line!( @@ -2125,33 +2199,35 @@ pub async fn build_bond( } } - let default_address = source.clone().unwrap_or(validator.clone()); - let default_signer = Some(default_address.clone()); - let signing_data = signing::aux_signing_data( + let default_signer = Some(source.clone().unwrap_or(validator.clone())); + let (signing_data, wrap_args, updated_balance) = derive_build_data( context, - tx_args, - Some(default_address.clone()), + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, default_signer, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, updated_balance) = - validate_transparent_fee(context, tx_args, fee_payer).await?; // Check bond's source (source for delegation or validator for self-bonds) // balance let bond_source = source.as_ref().unwrap_or(&validator); let native_token = context.native_token(); - let check_balance = if &updated_balance.source == bond_source - && updated_balance.token == native_token - { - CheckBalance::Balance(updated_balance.post_balance) - } else { - CheckBalance::Query(balance_key(&native_token, bond_source)) + let check_balance = match updated_balance { + Some(updated_balance) + if &updated_balance.source == bond_source + && updated_balance.token == native_token => + { + CheckBalance::Balance(updated_balance.post_balance) + } + _ => CheckBalance::Query(balance_key(&native_token, bond_source)), }; check_balance_too_low_err( &native_token, @@ -2175,8 +2251,7 @@ pub async fn build_bond( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -2193,22 +2268,19 @@ pub async fn build_default_proposal( tx_code_path, }: &args::InitProposal, proposal: DefaultProposal, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(proposal.proposal.author.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx, + tx.wrap_tx.as_ref().map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx.force, Some(proposal.proposal.author.clone()), - default_signer, - vec![], - false, + tx.signing_keys.to_owned(), vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _updated_balance) = - validate_transparent_fee(context, tx, fee_payer).await?; let init_proposal_data = InitProposalData::try_from(proposal.clone()) .map_err(|e| TxSubmitError::InvalidProposal(e.to_string()))?; @@ -2239,8 +2311,7 @@ pub async fn build_default_proposal( tx_code_path.clone(), init_proposal_data, push_data, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -2257,22 +2328,19 @@ pub async fn build_vote_proposal( tx_code_path, }: &args::VoteProposal, current_epoch: Epoch, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(voter_address.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx, - default_signer.clone(), - default_signer.clone(), - vec![], - false, + tx.wrap_tx.as_ref().map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx.force, + Some(voter_address.clone()), + tx.signing_keys.to_owned(), vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx, fee_payer).await?; let proposal_vote = ProposalVote::try_from(vote.clone()) .map_err(|_| TxSubmitError::InvalidProposalVote)?; @@ -2397,8 +2465,7 @@ pub async fn build_vote_proposal( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -2426,7 +2493,7 @@ pub async fn build_become_validator( unsafe_dont_encrypt: _, tx_code_path, }: &args::TxBecomeValidator, -) -> Result<(Tx, SigningTxData)> { +) -> Result<(Tx, SigningData)> { // Check that the address is established if !address.is_established() { edisplay_line!( @@ -2565,27 +2632,52 @@ pub async fn build_become_validator( return Err(Error::Other("Invalid address".to_string())); }; - let mut all_pks = account.get_all_public_keys(); + let mut all_pks = [ + tx_args.signing_keys.to_owned(), + account.get_all_public_keys(), + ] + .concat(); all_pks.push(consensus_key.clone().unwrap().clone()); all_pks.push(eth_cold_key.clone().unwrap()); all_pks.push(eth_hot_key.clone().unwrap()); all_pks.push(protocol_key.clone().unwrap().clone()); - let signing_data = signing::aux_signing_data( - context, - tx_args, - None, - None, - all_pks, - false, - vec![], - None, - ) - .await?; + let (signing_data, wrap_args) = if let Some(wrap_tx) = &tx_args.wrap_tx { + let signing_data = signing::aux_signing_data( + context, + wrap_tx, + None, + all_pks, + false, + vec![], + ) + .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _updated_balance) = - validate_transparent_fee(context, tx_args, fee_payer).await?; + let fee_payer = signing_data.fee_payer_or_err()?.to_owned(); + let (fee_amount, _updated_balance) = validate_transparent_fee( + context, + wrap_tx, + tx_args.force, + &fee_payer, + ) + .await?; + + ( + SigningData::Wrapper(signing_data), + Some(WrapArgs { + fee_amount, + fee_payer, + fee_token: wrap_tx.fee_token.to_owned(), + gas_limit: wrap_tx.gas_limit, + }), + ) + } else { + let signing_data = + signing::aux_inner_signing_data(context, all_pks, None, vec![]) + .await?; + + (SigningData::Inner(signing_data), None) + }; build( context, @@ -2593,8 +2685,7 @@ pub async fn build_become_validator( tx_code_path.clone(), data, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -2611,22 +2702,19 @@ pub async fn build_pgf_funding_proposal( tx_code_path, }: &args::InitProposal, proposal: PgfFundingProposal, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(proposal.proposal.author.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx, + tx.wrap_tx.as_ref().map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx.force, Some(proposal.proposal.author.clone()), - default_signer, + tx.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _updated_balance) = - validate_transparent_fee(context, tx, fee_payer).await?; let init_proposal_data = InitProposalData::try_from(proposal.clone()) .map_err(|e| TxSubmitError::InvalidProposal(e.to_string()))?; @@ -2643,8 +2731,7 @@ pub async fn build_pgf_funding_proposal( tx_code_path.clone(), init_proposal_data, add_section, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -2661,22 +2748,19 @@ pub async fn build_pgf_stewards_proposal( tx_code_path, }: &args::InitProposal, proposal: PgfStewardProposal, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(proposal.proposal.author.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx, + tx.wrap_tx.as_ref().map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx.force, Some(proposal.proposal.author.clone()), - default_signer, + tx.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _updated_balance) = - validate_transparent_fee(context, tx, fee_payer).await?; let init_proposal_data = InitProposalData::try_from(proposal.clone()) .map_err(|e| TxSubmitError::InvalidProposal(e.to_string()))?; @@ -2694,8 +2778,7 @@ pub async fn build_pgf_stewards_proposal( tx_code_path.clone(), init_proposal_data, add_section, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -2706,8 +2789,7 @@ pub async fn build_ibc_transfer( context: &impl Namada, args: &args::TxIbcTransfer, bparams: &mut impl BuildParams, - skip_fee_handling: bool, -) -> Result<(Tx, SigningTxData, Option)> { +) -> Result<(Tx, SigningData, Option)> { if args.ibc_shielding_data.is_some() && args.ibc_memo.is_some() { return Err(Error::Other( "The memo field of the IBC packet can't be used for both \ @@ -2719,33 +2801,24 @@ pub async fn build_ibc_transfer( let refund_target = get_refund_target(context, &args.source, &args.refund_target).await?; - let source = args.source.effective_address(); - let mut signing_data = signing::aux_signing_data( + let (mut signing_data, wrap_args, updated_balance) = derive_build_data( context, - &args.tx, - Some(source.clone()), - Some(source.clone()), - vec![], - args.source.spending_key().is_some(), + args.tx + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: args.source.spending_key().is_some(), + }), + args.tx.force, + args.source.address(), + args.tx.signing_keys.to_owned(), vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_per_gas_unit, updated_balance) = - if let TransferSource::ExtendedKey(_) = args.source { - // MASP fee payment - (validate_fee(context, &args.tx).await?, None) - } else { - // Transparent fee payment - validate_transparent_fee(context, &args.tx, fee_payer) - .await - .map(|(fee_amount, updated_balance)| { - (fee_amount, Some(updated_balance)) - })? - }; // Check that the source address exists on chain + let source = args.source.effective_address(); let source = source_exists_or_err(source.clone(), args.tx.force, context).await?; // We cannot check the receiver @@ -2758,13 +2831,15 @@ pub async fn build_ibc_transfer( // If source is transparent check the balance (MASP balance is checked when // constructing the shielded part) - 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)) + if source != MASP { + let check_balance = match updated_balance { + Some(updated_balance) + if updated_balance.source == source + && updated_balance.token == args.token => + { + CheckBalance::Balance(updated_balance.post_balance) + } + _ => CheckBalance::Query(balance_key(&args.token, &source)), }; check_balance_too_low_err( @@ -2799,15 +2874,11 @@ pub async fn build_ibc_transfer( let mut transfer = token::Transfer::default(); // Add masp fee payment if necessary - let masp_fee_data = if skip_fee_handling { - None - } else { + let masp_fee_data = if let Some(wrap_tx) = &wrap_args { let masp_fee_data = get_masp_fee_payment_amount( context, - &args.tx, - fee_per_gas_unit, + wrap_tx, // If no custom gas spending key is provided default to the source - fee_payer, args.gas_spending_key.or(args.source.spending_key()), ) .await?; @@ -2825,6 +2896,8 @@ pub async fn build_ibc_transfer( } masp_fee_data + } else { + None }; if let Some((target, percentage)) = &args.frontend_sus_fee { @@ -2939,7 +3012,18 @@ pub async fn build_ibc_transfer( let masp_tx_hash = tx.add_masp_tx_section(shielded_transfer.masp_tx.clone()).1; transfer.shielded_section_hash = Some(masp_tx_hash); - signing_data.shielded_hash = Some(masp_tx_hash); + match signing_data { + SigningData::Inner(ref mut signing_tx_data) => { + signing_tx_data.shielded_hash = Some(masp_tx_hash); + } + SigningData::Wrapper(ref mut signing_wrapper_data) => { + signing_wrapper_data + .signing_data + .first_mut() + .expect("Missing expected inner IBC transaction") + .shielded_hash = Some(masp_tx_hash); + } + }; tx.add_masp_builder(MaspBuilder { asset_types, metadata: shielded_transfer.metadata, @@ -3037,13 +3121,36 @@ pub async fn build_ibc_transfer( Some(args.tx_code_path.to_string_lossy().into_owned()), ) .add_serialized_data(data); - let fee_payer = signing_data.fee_payer_or_err()?.to_owned(); - prepare_tx(&args.tx, &mut tx, fee_per_gas_unit, fee_payer).await?; + if let Some(WrapArgs { + fee_amount, + fee_payer, + fee_token, + gas_limit, + }) = wrap_args + { + tx.add_wrapper( + Fee { + amount_per_gas_unit: fee_amount, + token: fee_token, + }, + fee_payer, + gas_limit, + ); + } Ok((tx, signing_data, shielded_tx_epoch)) } -/// Abstraction for helping build transactions +pub(crate) struct WrapArgs { + pub(crate) fee_amount: DenominatedAmount, + pub(crate) fee_payer: common::PublicKey, + pub(crate) fee_token: Address, + pub(crate) gas_limit: GasLimit, +} + +/// Abstraction for helping build transactions. This function will build either +/// a Raw or a Wrapper transaction depending on the presence of the [`WrapArgs`] +/// argument. #[allow(clippy::too_many_arguments)] async fn build( context: &impl Namada, @@ -3051,8 +3158,7 @@ async fn build( path: PathBuf, mut data: D, on_tx: F, - fee_amount: DenominatedAmount, - gas_payer: &common::PublicKey, + wrap_args: Option, ) -> Result where F: FnOnce(&mut Tx, &mut D) -> Result<()>, @@ -3060,26 +3166,42 @@ where { let chain_id = tx_args.chain_id.clone().unwrap(); - let mut tx_builder = Tx::new(chain_id, tx_args.expiration.to_datetime()); + let mut tx = Tx::new(chain_id, tx_args.expiration.to_datetime()); if let Some(memo) = &tx_args.memo { - tx_builder.add_memo(memo); + tx.add_memo(memo); } let tx_code_hash = query_wasm_code_hash(context, path.to_string_lossy()) .await .map_err(|e| Error::from(QueryError::Wasm(e.to_string())))?; - on_tx(&mut tx_builder, &mut data)?; + on_tx(&mut tx, &mut data)?; - tx_builder - .add_code_from_hash( - tx_code_hash, - Some(path.to_string_lossy().into_owned()), - ) - .add_data(data); + tx.add_code_from_hash( + tx_code_hash, + Some(path.to_string_lossy().into_owned()), + ) + .add_data(data); + + // Wrap the transaction if requested + if let Some(WrapArgs { + fee_amount, + fee_payer, + fee_token, + gas_limit, + }) = wrap_args + { + tx.add_wrapper( + Fee { + amount_per_gas_unit: fee_amount, + token: fee_token, + }, + fee_payer, + gas_limit, + ); + } - prepare_tx(tx_args, &mut tx_builder, fee_amount, gas_payer.clone()).await?; - Ok(tx_builder) + Ok(tx) } /// Try to decode the given asset type and add its decoding to the supplied set. @@ -3137,87 +3259,93 @@ async fn used_asset_types( Ok(asset_types) } -/// Constructs the batched tx from the provided list. Returns also the data for -/// signing +/// Constructs the wrapped batched tx from the provided list. Returns the batch +/// and the data for signing. +/// +/// # Arguments +/// +/// * `txs` - The list of transactions for the batch. The first transaction in +/// the list must be the wrapper transaction that will be used as the first +/// transaction of the batch and whose `Header` will be used as the whole +/// batche's header. If MASP fee payment is required that should be included +/// in this first transaction. The remaining ones are supposed to be raw +/// transactions. They can also be wrapped but in that case their wrapper data +/// will simply be discarded pub fn build_batch( - mut txs: Vec<(Tx, SigningTxData)>, -) -> Result<(Tx, Vec)> { + mut txs: Vec<(Tx, SigningData)>, +) -> Result<(Tx, SigningWrapperData)> { if txs.is_empty() { return Err(Error::Other( "No transactions provided for the batch".to_string(), )); } - let (mut batched_tx, sig_data) = txs.remove(0); - let mut signing_data = vec![sig_data]; + let (mut batched_tx, signing_data) = txs.remove(0); + if batched_tx.header.wrapper().is_none() { + return Err(Error::Other( + "The first transaction of the list is expected to be wrapped" + .to_string(), + )); + } + let SigningData::Wrapper(mut signing_wrapper_data) = signing_data else { + return Err(Error::Other( + "The first signing data of the list is missing fee authorization" + .to_string(), + )); + }; for (tx, sig_data) in txs { - if tx.commitments().len() != 1 { - return Err(Error::Other(format!( - "Inner tx did not contain exactly one transaction, \ - transaction length: {}", - tx.commitments().len() - ))); - } - - let cmt = tx.first_commitments().unwrap().to_owned(); - if !batched_tx.add_inner_tx(tx, cmt.clone()) { - return Err(Error::Other(format!( - "The transaction batch already contains inner tx: {}", - cmt.get_hash() - ))); - } + batched_tx = Tx::merge_transactions(batched_tx, tx).map_err(|_| { + Error::Other( + "Found duplicated tx commitments when building the batch" + .to_string(), + ) + })?; // Avoid redundant signing data - if !signing_data.iter().any(|sig| sig == &sig_data) { - signing_data.push(sig_data); + for signing_tx_data in sig_data.signing_tx_data() { + if !signing_wrapper_data.signing_data.contains(signing_tx_data) { + signing_wrapper_data + .signing_data + .push(signing_tx_data.to_owned()); + } } } - Ok((batched_tx, signing_data)) + Ok((batched_tx, signing_wrapper_data)) } /// Build a transparent transfer pub async fn build_transparent_transfer( context: &N, args: &mut args::TxTransparentTransfer, -) -> Result<(Tx, SigningTxData)> { +) -> Result<(Tx, SigningData)> { let mut transfers = token::Transfer::default(); // Evaluate signer and fees - let (signing_data, fee_amount, updated_balance) = { - let source = if args.sources.len() == 1 { - // If only one transfer take its source as the signer - args.sources - .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, - source.clone(), - source, - vec![], - false, - vec![], - None, - ) - .await?; - - // Transparent fee payment - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, updated_balance) = - validate_transparent_fee(context, &args.tx, fee_payer) - .await - .map(|(fee_amount, updated_balance)| { - (fee_amount, Some(updated_balance)) - })?; - - (signing_data, fee_amount, updated_balance) + let source = if args.sources.len() == 1 { + // If only one transfer take its source as the signer + args.sources + .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, wrap_args, updated_balance) = derive_build_data( + context, + args.tx + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + args.tx.force, + source, + args.tx.signing_keys.to_owned(), + vec![], + ) + .await?; for TxTransparentSource { source, @@ -3280,17 +3408,16 @@ pub async fn build_transparent_transfer( .ok_or(Error::Other("Combined transfer overflows".to_string()))?; } - let fee_payer = signing_data.fee_payer_or_err()?; let tx = build( context, &args.tx, args.tx_code_path.clone(), transfers, do_nothing, - fee_amount, - fee_payer, + wrap_args, ) .await?; + Ok((tx, signing_data)) } @@ -3299,23 +3426,23 @@ pub async fn build_shielded_transfer( context: &N, args: &mut args::TxShieldedTransfer, bparams: &mut impl BuildParams, - skip_fee_handling: bool, -) -> Result<(Tx, SigningTxData)> { - let mut signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (mut signing_data, wrap_args, _) = derive_build_data( context, - &args.tx, - Some(MASP), - Some(MASP), - vec![], - true, - vec![], + args.tx + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: true, + }), + args.tx.force, None, + args.tx.signing_keys.to_owned(), + vec![], ) .await?; - // Shielded fee payment - let fee_per_gas_unit = validate_fee(context, &args.tx).await?; - let mut transfer_data = MaspTransferData::default(); for args::TxShieldedSource { source, @@ -3355,16 +3482,11 @@ pub async fn build_shielded_transfer( // Construct the tx data with a placeholder shielded section hash let mut data = token::Transfer::default(); - let masp_fee_data = if skip_fee_handling { - None - } else { - let fee_payer = signing_data.fee_payer_or_err()?; - // Add masp fee payment if necessary + // Add masp fee payment if necessary + let masp_fee_data = if let Some(wrap_tx) = &wrap_args { let masp_fee_data = get_masp_fee_payment_amount( context, - &args.tx, - fee_per_gas_unit, - fee_payer, + wrap_tx, // If no custom gas spending key is provided default to the first // source args.gas_spending_key @@ -3385,6 +3507,8 @@ pub async fn build_shielded_transfer( } masp_fee_data + } else { + None }; let shielded_parts = construct_shielded_parts( @@ -3397,7 +3521,6 @@ pub async fn build_shielded_transfer( .await? .expect("Shielded transfer must have shielded parts"); - let fee_payer = signing_data.fee_payer_or_err()?.to_owned(); let add_shielded_parts = |tx: &mut Tx, data: &mut token::Transfer| { // Add the MASP Transaction and its Builder to facilitate validation let ( @@ -3423,7 +3546,18 @@ pub async fn build_shielded_transfer( }); data.shielded_section_hash = Some(section_hash); - signing_data.shielded_hash = Some(section_hash); + match signing_data { + SigningData::Inner(ref mut signing_tx_data) => { + signing_tx_data.shielded_hash = Some(section_hash); + } + SigningData::Wrapper(ref mut signing_wrapper_data) => { + signing_wrapper_data + .signing_data + .first_mut() + .expect("Missing expected inner shielded transaction") + .shielded_hash = Some(section_hash); + } + }; tracing::debug!("Transfer data {data:?}"); Ok(()) }; @@ -3434,8 +3568,7 @@ pub async fn build_shielded_transfer( args.tx_code_path.clone(), data, add_shielded_parts, - fee_per_gas_unit, - &fee_payer, + wrap_args, ) .await?; Ok((tx, signing_data)) @@ -3445,13 +3578,16 @@ pub async fn build_shielded_transfer( // right masp data async fn get_masp_fee_payment_amount( context: &N, - args: &args::Tx, - fee_amount: DenominatedAmount, - fee_payer: &common::PublicKey, + WrapArgs { + fee_amount, + fee_payer, + fee_token, + gas_limit, + }: &WrapArgs, gas_spending_key: Option, ) -> Result> { let fee_payer_address = Address::from(fee_payer); - let balance_key = balance_key(&args.fee_token, &fee_payer_address); + let balance_key = balance_key(fee_token, &fee_payer_address); #[allow(clippy::disallowed_methods)] let balance = rpc::query_storage_value::<_, token::Amount>( context.client(), @@ -3459,7 +3595,7 @@ async fn get_masp_fee_payment_amount( ) .await .unwrap_or_default(); - let total_fee = checked!(fee_amount.amount() * u64::from(args.gas_limit))?; + let total_fee = checked!(fee_amount.amount() * u64::from(*gas_limit))?; Ok(match total_fee.checked_sub(balance) { Some(diff) if !diff.is_zero() => Some(MaspFeeData { @@ -3469,7 +3605,7 @@ async fn get_masp_fee_payment_amount( .to_string(), ))?, target: fee_payer_address, - token: args.fee_token.clone(), + token: fee_token.to_owned(), amount: DenominatedAmount::new(diff, fee_amount.denom()), }), _ => None, @@ -3520,7 +3656,7 @@ pub async fn build_shielding_transfer( context: &N, args: &args::TxShieldingTransfer, bparams: &mut impl BuildParams, -) -> Result<(Tx, SigningTxData, MaspEpoch)> { +) -> Result<(Tx, SigningData, MaspEpoch)> { let source = if args.sources.len() == 1 { // If only one transfer take its source as the signer args.sources @@ -3531,27 +3667,22 @@ pub async fn build_shielding_transfer( // argument None }; - let mut signing_data = signing::aux_signing_data( + let (mut signing_data, wrap_args, updated_balance) = derive_build_data( context, - &args.tx, - source.clone(), + args.tx + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + args.tx.force, source, + args.tx.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - // Transparent fee payment - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, updated_balance) = - validate_transparent_fee(context, &args.tx, fee_payer) - .await - .map(|(fee_amount, updated_balance)| { - (fee_amount, Some(updated_balance)) - })?; - let mut transfer_data = MaspTransferData::default(); let mut data = token::Transfer::default(); for TxTransparentSource { @@ -3685,7 +3816,6 @@ pub async fn build_shielding_transfer( .expect("Shielding transfer must have shielded parts"); let shielded_tx_epoch = shielded_parts.0.epoch; - let fee_payer = signing_data.fee_payer_or_err()?.to_owned(); let add_shielded_parts = |tx: &mut Tx, data: &mut token::Transfer| { // Add the MASP Transaction and its Builder to facilitate validation let ( @@ -3711,7 +3841,18 @@ pub async fn build_shielding_transfer( }); data.shielded_section_hash = Some(shielded_section_hash); - signing_data.shielded_hash = Some(shielded_section_hash); + match signing_data { + SigningData::Inner(ref mut signing_tx_data) => { + signing_tx_data.shielded_hash = Some(shielded_section_hash); + } + SigningData::Wrapper(ref mut signing_wrapper_data) => { + signing_wrapper_data + .signing_data + .first_mut() + .expect("Missing expected inner shielding transaction") + .shielded_hash = Some(shielded_section_hash); + } + }; tracing::debug!("Transfer data {data:?}"); Ok(()) }; @@ -3722,8 +3863,7 @@ pub async fn build_shielding_transfer( args.tx_code_path.clone(), data, add_shielded_parts, - fee_amount, - &fee_payer, + wrap_args, ) .await?; Ok((tx, signing_data, shielded_tx_epoch)) @@ -3734,23 +3874,23 @@ pub async fn build_unshielding_transfer( context: &N, args: &mut args::TxUnshieldingTransfer, bparams: &mut impl BuildParams, - skip_fee_handling: bool, -) -> Result<(Tx, SigningTxData)> { - let mut signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (mut signing_data, wrap_args, _) = derive_build_data( context, - &args.tx, - Some(MASP), - Some(MASP), - vec![], - true, - vec![], + args.tx + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: true, + }), + args.tx.force, None, + args.tx.signing_keys.to_owned(), + vec![], ) .await?; - // Shielded fee payment - let fee_per_gas_unit = validate_fee(context, &args.tx).await?; - let mut transfer_data = MaspTransferData::default(); let mut data = token::Transfer::default(); for TxTransparentTarget { @@ -3834,15 +3974,10 @@ pub async fn build_unshielding_transfer( } // Add masp fee payment if necessary - let masp_fee_data = if skip_fee_handling { - None - } else { - let fee_payer = signing_data.fee_payer_or_err()?; + let masp_fee_data = if let Some(wrap_tx) = &wrap_args { let masp_fee_data = get_masp_fee_payment_amount( context, - &args.tx, - fee_per_gas_unit, - fee_payer, + wrap_tx, // If no custom gas spending key is provided default to the source args.gas_spending_key .or(args.sources.first().map(|x| x.source)), @@ -3863,6 +3998,8 @@ pub async fn build_unshielding_transfer( } masp_fee_data + } else { + None }; let shielded_parts = construct_shielded_parts( @@ -3875,7 +4012,6 @@ pub async fn build_unshielding_transfer( .await? .expect("Shielding transfer must have shielded parts"); - let fee_payer = signing_data.fee_payer_or_err()?.to_owned(); let add_shielded_parts = |tx: &mut Tx, data: &mut token::Transfer| { // Add the MASP Transaction and its Builder to facilitate validation let ( @@ -3901,7 +4037,18 @@ pub async fn build_unshielding_transfer( }); data.shielded_section_hash = Some(shielded_section_hash); - signing_data.shielded_hash = Some(shielded_section_hash); + match signing_data { + SigningData::Inner(ref mut signing_tx_data) => { + signing_tx_data.shielded_hash = Some(shielded_section_hash); + } + SigningData::Wrapper(ref mut signing_wrapper_data) => { + signing_wrapper_data + .signing_data + .first_mut() + .expect("Missing expected inner unshielding transaction") + .shielded_hash = Some(shielded_section_hash); + } + }; tracing::debug!("Transfer data {data:?}"); Ok(()) }; @@ -3912,8 +4059,7 @@ pub async fn build_unshielding_transfer( args.tx_code_path.clone(), data, add_shielded_parts, - fee_per_gas_unit, - &fee_payer, + wrap_args, ) .await?; Ok((tx, signing_data)) @@ -3973,21 +4119,22 @@ pub async fn build_init_account( public_keys, threshold, }: &args::TxInitAccount, -) -> Result<(Tx, SigningTxData)> { - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, - None, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, None, + tx_args.signing_keys.to_owned(), vec![], - false, - vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; let vp_code_hash = query_wasm_code_hash_buf(context, vp_code_path).await?; @@ -4045,8 +4192,7 @@ pub async fn build_init_account( tx_code_path.clone(), data, add_code_hash, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -4063,22 +4209,22 @@ pub async fn build_update_account( public_keys, threshold, }: &args::TxUpdateAccount, -) -> Result<(Tx, SigningTxData)> { - let default_signer = Some(addr.clone()); - let signing_data = signing::aux_signing_data( +) -> Result<(Tx, SigningData)> { + let (signing_data, wrap_args, _) = derive_build_data( context, - tx_args, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, Some(addr.clone()), - default_signer, - vec![], - false, + tx_args.signing_keys.to_owned(), vec![], - None, ) .await?; - let fee_payer = signing_data.fee_payer_or_err()?; - let (fee_amount, _) = - validate_transparent_fee(context, tx_args, fee_payer).await?; let account = if let Some(account) = rpc::get_account_info(context.client(), addr).await? @@ -4174,8 +4320,7 @@ pub async fn build_update_account( tx_code_path.clone(), data, add_code_hash, - fee_amount, - fee_payer, + wrap_args, ) .await .map(|tx| (tx, signing_data)) @@ -4193,7 +4338,7 @@ pub async fn build_custom( signatures, wrapper_signature, }: &args::TxCustom, -) -> Result<(Tx, Option)> { +) -> Result<(Tx, SigningData)> { let mut tx = if let Some(serialized_tx) = serialized_tx { Tx::try_from_json_bytes(serialized_tx.as_ref()).map_err(|_| { Error::Other( @@ -4227,67 +4372,107 @@ pub async fn build_custom( // 2. The user also provided the offline signatures for the inner // transaction(s) // The workflow is the following: - // 1. If no signatures were provided we generate a SigningTxData to sign + // 1. If no signatures were provided we generate a SigningData to sign // the tx - // 2. If only the inner sigs were provided we generate a SigningTxData - // that will attach them and then sign the wrapper online + // 2. If only the inner sigs were provided we generate a SigningData that + // will attach them and then sign the wrapper online // 3. If the wrapper signature was provided then we also expect the inner - // signature(s) to have been provided, in this case we attach all the - // signatures here and return no SigningTxData - let signing_data = if let Some(wrapper_signature) = &wrapper_signature { - if tx.header.wrapper().is_none() { + // signature(s) to have been provided, in this case we generate a + // SigningData to attach all these signatures + let (signing_data, wrap_tx) = if tx.header.wrapper().is_some() { + match (wrapper_signature, &tx_args.wrap_tx) { + (None, None) => { + return Err(Error::Other( + "A wrapper signature or a wrapper signer must be provided \ + when loading a wrapped custom transaction" + .to_string(), + )); + } + (None, Some(wrap_args)) => { + let (signing_data, wrap_tx, _) = derive_build_data( + context, + Some(ExtendedWrapperArgs { + wrap_args, + // The optional masp fee paying transaction has + // already been + // produced so no need to generate a disposable + // address anyway + disposable_gas_payer: false, + }), + tx_args.force, + owner.to_owned(), + tx_args.signing_keys.to_owned(), + signatures.to_owned(), + ) + .await?; + + (signing_data, wrap_tx) + } + // If both a serialized wrapper signature and a request to + // wrap the tx are passed, the serialized signature takes + // precedence + (Some(wrapper_sig), _) => ( + SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + owner: None, + public_keys: Default::default(), + threshold: 0, + account_public_keys_map: Default::default(), + shielded_hash: None, + signatures: signatures.to_owned(), + }], + fee_auth: FeeAuthorization::Signature( + wrapper_sig.to_owned(), + ), + }), + None, + ), + } + } else { + if wrapper_signature.is_some() { return Err(Error::Other( "A wrapper signature was provided but the transaction is not \ a wrapper" .to_string(), )); } - // Attach the provided signatures to the tx without the need to produce - // any more signatures - let signatures = signatures.iter().try_fold( - vec![], - |mut acc, bytes| -> Result> { - let sig = SignatureIndex::try_from_json_bytes(bytes).map_err( - |err| Error::Encode(EncodingError::Serde(err.to_string())), - )?; - acc.push(sig); - Ok(acc) - }, - )?; - tx.add_signatures(signatures) - .add_section(Section::Authorization( - serde_json::from_slice(wrapper_signature).map_err(|err| { - Error::Encode(EncodingError::Serde(err.to_string())) - })?, - )); - None - } else { - let default_signer = owner.clone(); - let fee_amount = validate_fee(context, tx_args).await?; - let signing_data = signing::aux_signing_data( + let (signing_data, wrap_tx, _) = derive_build_data( context, - tx_args, + tx_args + .wrap_tx + .as_ref() + .map(|wrap_args| ExtendedWrapperArgs { + wrap_args, + disposable_gas_payer: false, + }), + tx_args.force, owner.clone(), - default_signer, - vec![], - false, + tx_args.signing_keys.to_owned(), signatures.to_owned(), - None, ) .await?; - let (fee_payer, _) = signing_data - .fee_payer - .as_ref() - .left() - .ok_or_else(|| { - Error::Other("Missing gas payer argument".to_string()) - })? - .to_owned(); - prepare_tx(tx_args, &mut tx, fee_amount, fee_payer).await?; - Some(signing_data) + + (signing_data, wrap_tx) }; + if let Some(WrapArgs { + fee_amount, + fee_payer, + fee_token, + gas_limit, + }) = wrap_tx + { + tx.add_wrapper( + Fee { + amount_per_gas_unit: fee_amount, + token: fee_token, + }, + fee_payer, + gas_limit, + ); + } + Ok((tx, signing_data)) } diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index 9c1f33ebefd..a2d3360e2ee 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -1816,7 +1816,6 @@ fn test_invalid_validator_txs() -> Result<()> { client.exp_string(TX_REJECTED)?; client.assert_success(); - // Try to deactivate validator-1 as validator-0 let tx_args = vec![ "deactivate-validator", "--validator", @@ -2620,7 +2619,7 @@ fn masp_txs_and_queries() -> Result<()> { || tx_args[0] == "shield" || tx_args[0] == "unshield") { - [tx_args.clone(), vec!["--dry-run"]].concat() + [tx_args.clone(), vec!["--dry-run-wrapper"]].concat() } else { tx_args.clone() }; diff --git a/crates/tests/src/integration/ledger_tests.rs b/crates/tests/src/integration/ledger_tests.rs index 2c46f26df94..ac4dfdc4fed 100644 --- a/crates/tests/src/integration/ledger_tests.rs +++ b/crates/tests/src/integration/ledger_tests.rs @@ -133,6 +133,8 @@ fn ledger_txs_and_queries() -> Result<()> { NAM, "--amount", "10.1", + "--signing-keys", + ESTER, "--node", &validator_one_rpc, ]), @@ -204,7 +206,7 @@ fn ledger_txs_and_queries() -> Result<()> { { continue; } else if dry_run { - [tx_args.clone(), vec!["--dry-run"]].concat() + [tx_args.clone(), vec!["--dry-run-wrapper"]].concat() } else { tx_args.clone() }; @@ -458,7 +460,7 @@ fn test_dry_run_transaction() -> Result<()> { NAM, "--amount", "10.1", - "--dry-run", + "--dry-run-wrapper", "--signing-keys", HARDWARE_WALLET_PK, "--gas-payer", @@ -476,7 +478,6 @@ fn test_dry_run_transaction() -> Result<()> { /// 1. Run the ledger node /// 2. Submit an invalid transaction (disallowed by state machine) /// 3. Check that the state was changed -/// 5. Submit and invalid transactions (malformed) #[test] fn invalid_transactions() -> Result<()> { // This address doesn't matter for tests. But an argument is required. @@ -485,38 +486,7 @@ fn invalid_transactions() -> Result<()> { let (node, _services) = setup::setup()?; // 2. Submit an invalid transaction (trying to transfer tokens should fail - // in the user's VP due to the wrong signer) - let tx_args = apply_use_device(vec![ - "transparent-transfer", - "--source", - BERTHA, - "--target", - ALBERT, - "--token", - NAM, - "--amount", - "1", - "--signing-keys", - ALBERT_KEY, - "--node", - &validator_one_rpc, - "--force", - ]); - - let captured = CapturedOutput::of(|| run(&node, Bin::Client, tx_args)); - assert_matches!(captured.result, Ok(_)); - assert!(captured.contains(TX_REJECTED)); - - node.finalize_and_commit(None); - // There should be state now - { - let locked = node.shell.lock().unwrap(); - assert_ne!( - locked.last_state("").last_block_app_hash, - Default::default() - ); - } - + // in the user's VP due to the insufficient balance) let daewon_lower = DAEWON.to_lowercase(); let tx_args = apply_use_device(vec![ "transparent-transfer", @@ -539,6 +509,16 @@ fn invalid_transactions() -> Result<()> { let captured = CapturedOutput::of(|| run(&node, Bin::Client, tx_args)); assert!(captured.contains(TX_INSUFFICIENT_BALANCE)); + node.finalize_and_commit(None); + // There should be state now + { + let locked = node.shell.lock().unwrap(); + assert_ne!( + locked.last_state("").last_block_app_hash, + Default::default() + ); + } + Ok(()) } @@ -1935,6 +1915,7 @@ fn offline_sign() -> Result<()> { ) }); assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); // 5. Assert changed balances let captured = CapturedOutput::of(|| { @@ -2538,7 +2519,13 @@ fn wrap_tx_by_elsewho() -> Result<()> { run( &node, Bin::Client, - apply_use_device(vec!["reveal-pk", "--public-key", key_alias]), + apply_use_device(vec![ + "reveal-pk", + "--public-key", + key_alias, + "--gas-payer", + key_alias, + ]), ) }); assert!(captured.result.is_ok()); @@ -2746,7 +2733,13 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> { run( &node, Bin::Client, - apply_use_device(vec!["reveal-pk", "--public-key", key_alias]), + apply_use_device(vec![ + "reveal-pk", + "--public-key", + key_alias, + "--gas-payer", + key_alias, + ]), ) }); assert!(captured.result.is_ok()); @@ -3009,7 +3002,13 @@ fn offline_wrapper_tx() -> Result<()> { run( &node, Bin::Client, - apply_use_device(vec!["reveal-pk", "--public-key", key_alias]), + apply_use_device(vec![ + "reveal-pk", + "--public-key", + key_alias, + "--gas-payer", + key_alias, + ]), ) }); assert!(captured.result.is_ok()); diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 3d416ee0697..e2e325dbdbc 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -4,7 +4,6 @@ use std::str::FromStr; use color_eyre::eyre::Result; use color_eyre::owo_colors::OwoColorize; -use itertools::Either; use masp_primitives::convert::AllowedConversion; use masp_primitives::merkle_tree::CommitmentTree; use masp_primitives::sapling::Node; @@ -23,7 +22,9 @@ use namada_sdk::account::AccountPublicKeysMap; #[cfg(feature = "historic-masp")] use namada_sdk::collections::HashMap; use namada_sdk::masp::fs::FsShieldedUtils; -use namada_sdk::signing::SigningTxData; +use namada_sdk::signing::{ + FeeAuthorization, SigningData, SigningTxData, SigningWrapperData, +}; use namada_sdk::state::{StorageRead, StorageWrite}; use namada_sdk::time::DateTimeUtc; use namada_sdk::token::storage_key::{ @@ -749,7 +750,7 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { NAM, "--amount", &UNSHIELD_REWARDS_AMT.to_string(), - "--signing-keys", + "--gas-payer", BERTHA_KEY, "--node", RPC, @@ -4295,7 +4296,7 @@ fn masp_txs_and_queries() -> Result<()> { let tx_args = if dry_run && is_use_device() { continue; } else if dry_run { - [tx_args.clone(), vec!["--dry-run"]].concat() + [tx_args.clone(), vec!["--dry-run-wrapper"]].concat() } else { tx_args.clone() }; @@ -4470,8 +4471,6 @@ fn multiple_unfetched_txs_same_block() -> Result<()> { NAM, "--amount", "50", - "--gas-payer", - ALBERT_KEY, "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-tx", @@ -5194,8 +5193,6 @@ fn cross_epoch_unshield() -> Result<()> { NAM, "--amount", "100", - "--gas-payer", - ALBERT_KEY, "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-tx", @@ -6955,15 +6952,21 @@ fn identical_output_descriptions() -> Result<()> { let mut tx_clone = tx.clone(); tx_clone.add_memo(&[1, 2, 3]); - let signing_data = SigningTxData { - owner: None, - public_keys: [adam_key.to_public()].into(), - threshold: 1, - account_public_keys_map: None, - fee_payer: Either::Left((adam_key.to_public(), false)), - shielded_hash: None, - signatures: vec![], - }; + let signing_data = SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + owner: None, + public_keys: [adam_key.to_public()].into(), + threshold: 1, + account_public_keys_map: None, + shielded_hash: None, + signatures: vec![], + }], + + fee_auth: FeeAuthorization::Signer { + pubkey: adam_key.to_public(), + disposable_fee_payer: false, + }, + }); let (mut batched_tx, _signing_data) = namada_sdk::tx::build_batch(vec![ (tx, signing_data.clone()), @@ -7265,7 +7268,6 @@ fn masp_batch() -> Result<()> { public_keys: [adam_key.to_public()].into(), threshold: 1, account_public_keys_map: None, - fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, signatures: vec![], }; @@ -7281,17 +7283,29 @@ fn masp_batch() -> Result<()> { namada_sdk::tx::build_batch(vec![ ( tx0.clone(), - SigningTxData { - shielded_hash: get_shielded_hash(&tx0), - ..signing_data.clone() - }, + SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + shielded_hash: get_shielded_hash(&tx0), + ..signing_data.clone() + }], + fee_auth: FeeAuthorization::Signer { + pubkey: adam_key.to_public(), + disposable_fee_payer: false, + }, + }), ), ( tx1.clone(), - SigningTxData { - shielded_hash: get_shielded_hash(&tx1), - ..signing_data.clone() - }, + SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + shielded_hash: get_shielded_hash(&tx1), + ..signing_data.clone() + }], + fee_auth: FeeAuthorization::Signer { + pubkey: adam_key.to_public(), + disposable_fee_payer: false, + }, + }), ), ]) .unwrap(); @@ -7521,7 +7535,6 @@ fn masp_atomic_batch() -> Result<()> { public_keys: [adam_key.to_public()].into(), threshold: 1, account_public_keys_map: None, - fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, signatures: vec![], }; @@ -7537,17 +7550,29 @@ fn masp_atomic_batch() -> Result<()> { namada_sdk::tx::build_batch(vec![ ( tx0.clone(), - SigningTxData { - shielded_hash: get_shielded_hash(&tx0), - ..signing_data.clone() - }, + SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + shielded_hash: get_shielded_hash(&tx0), + ..signing_data.clone() + }], + fee_auth: FeeAuthorization::Signer { + pubkey: adam_key.to_public(), + disposable_fee_payer: false, + }, + }), ), ( tx1.clone(), - SigningTxData { - shielded_hash: get_shielded_hash(&tx1), - ..signing_data.clone() - }, + SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + shielded_hash: get_shielded_hash(&tx1), + ..signing_data.clone() + }], + fee_auth: FeeAuthorization::Signer { + pubkey: adam_key.to_public(), + disposable_fee_payer: false, + }, + }), ), ]) .unwrap(); @@ -7865,7 +7890,6 @@ fn masp_failing_atomic_batch() -> Result<()> { public_keys: [adam_key.to_public()].into(), threshold: 1, account_public_keys_map: None, - fee_payer: Either::Left((adam_key.to_public(), false)), shielded_hash: None, signatures: vec![], }; @@ -7873,17 +7897,29 @@ fn masp_failing_atomic_batch() -> Result<()> { let (mut batched_tx, _signing_data) = namada_sdk::tx::build_batch(vec![ ( tx0.clone(), - SigningTxData { - shielded_hash: get_shielded_hash(&tx0), - ..signing_data.clone() - }, + SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + shielded_hash: get_shielded_hash(&tx0), + ..signing_data.clone() + }], + fee_auth: FeeAuthorization::Signer { + pubkey: adam_key.to_public(), + disposable_fee_payer: false, + }, + }), ), ( tx1.clone(), - SigningTxData { - shielded_hash: get_shielded_hash(&tx1), - ..signing_data.clone() - }, + SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + shielded_hash: get_shielded_hash(&tx1), + ..signing_data.clone() + }], + fee_auth: FeeAuthorization::Signer { + pubkey: adam_key.to_public(), + disposable_fee_payer: false, + }, + }), ), ]) .unwrap(); @@ -8649,22 +8685,12 @@ fn masp_events() -> Result<()> { let (mut node, _services) = setup::setup()?; _ = node.next_masp_epoch(); - let native_token = node - .shell - .lock() - .unwrap() - .state - .in_mem() - .native_token - .clone(); - // 0. Initialize accounts we can access the secret keys of let (adam_alias, adam_key) = make_temp_account(&node, validator_one_rpc, "Adam", NAM, 100_000)?; let adam_pk = adam_key.to_public(); let (bradley_alias, bradley_key) = make_temp_account(&node, validator_one_rpc, "Bradley", NAM, 0)?; - let bradley_pk = bradley_key.to_public(); let (cooper_alias, cooper_key) = make_temp_account(&node, validator_one_rpc, "Cooper", NAM, 0)?; let cooper_pk = cooper_key.to_public(); @@ -8768,7 +8794,7 @@ fn masp_events() -> Result<()> { "1000", "--output-folder-path", tempdir.path().to_str().unwrap(), - "--dump-tx", + "--dump-wrapper-tx", "--ledger-address", validator_one_rpc, ]), @@ -8824,7 +8850,7 @@ fn masp_events() -> Result<()> { C_SPENDING_KEY, "--output-folder-path", tempdir.path().to_str().unwrap(), - "--dump-tx", + "--dump-wrapper-tx", "--ledger-address", validator_one_rpc, ]), @@ -8857,10 +8883,6 @@ fn masp_events() -> Result<()> { NAM, "--amount", "1", - // Fake a transparent gas payer, fees will actually be paid by - // the first tx of this batch - "--gas-payer", - CHRISTEL_KEY, "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-tx", @@ -8910,10 +8932,10 @@ fn masp_events() -> Result<()> { let signing_data = SigningTxData { owner: None, - public_keys: [cooper_pk.clone()].into(), + // No need to sign the raw tx, only the masp section + public_keys: Default::default(), threshold: 1, account_public_keys_map: None, - fee_payer: Either::Left((cooper_pk.clone(), false)), shielded_hash: None, signatures: vec![], }; @@ -8921,17 +8943,23 @@ fn masp_events() -> Result<()> { let (batched_tx, _signing_data) = namada_sdk::tx::build_batch(vec![ ( tx0.clone(), - SigningTxData { - shielded_hash: get_shielded_hash(&tx0), - ..signing_data.clone() - }, + SigningData::Wrapper(SigningWrapperData { + signing_data: vec![SigningTxData { + shielded_hash: get_shielded_hash(&tx0), + ..signing_data.clone() + }], + fee_auth: FeeAuthorization::Signer { + pubkey: cooper_pk.clone(), + disposable_fee_payer: false, + }, + }), ), ( tx1.clone(), - SigningTxData { + SigningData::Inner(SigningTxData { shielded_hash: get_shielded_hash(&tx1), ..signing_data.clone() - }, + }), ), ]) .unwrap(); @@ -8963,7 +8991,7 @@ fn masp_events() -> Result<()> { "0.00001", "--output-folder-path", tempdir.path().to_str().unwrap(), - "--dump-tx", + "--dump-wrapper-tx", "--ledger-address", validator_one_rpc, ]), @@ -8996,29 +9024,22 @@ fn masp_events() -> Result<()> { let mut txs = vec![]; for (idx, bytes) in txs_bytes.iter().enumerate() { - let (sk, pk) = if idx == 0 { - (adam_key.clone(), adam_pk.clone()) + let mut tx = Tx::try_from_json_bytes(bytes).unwrap(); + let sk = if idx == 0 { + tx.sign_raw( + vec![adam_key.clone()], + AccountPublicKeysMap::from_iter( + vec![(adam_pk.clone())].into_iter(), + ), + None, + ); + adam_key.clone() } else if idx == 1 { - (cooper_key.clone(), cooper_pk.clone()) + cooper_key.clone() } else { - (bradley_key.clone(), bradley_pk.clone()) + bradley_key.clone() }; - let mut tx = Tx::try_from_json_bytes(bytes).unwrap(); - tx.add_wrapper( - tx::data::wrapper::Fee { - amount_per_gas_unit: DenominatedAmount::native(10.into()), - token: native_token.clone(), - }, - pk.clone(), - 100_000.into(), - ); - tx.sign_raw( - vec![sk.clone()], - AccountPublicKeysMap::from_iter(vec![(pk)].into_iter()), - None, - ); tx.sign_wrapper(sk); - txs.push(tx.to_bytes()); } diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index 51642ff1fb4..7a3539642f5 100644 --- a/crates/tx/src/types.rs +++ b/crates/tx/src/types.rs @@ -145,51 +145,58 @@ impl Tx { self.header.batch.insert(TxCommitments::default()) } - /// Add a new inner tx to the transaction. Returns `false` if the - /// commitments already existed in the collection. This function expects a - /// transaction carrying a single inner tx as input and the provided - /// commitment is assumed to be present in the transaction without further - /// validation - pub fn add_inner_tx(&mut self, other: Tx, mut cmt: TxCommitments) -> bool { - if self.header.batch.contains(&cmt) { - return false; - } + /// Takes to [`Tx`]s and merges them together. `lhs` is kept as the base + /// layer on top of which the [`TxCommitments`] and [`Section`]s of `rhs` + /// will be merged. The [`Header`] of the resulting transactions will be + /// that of `lhs` plus the commitments of `rhs`. + /// + /// Returns an error if `lhs` contains any of the commitments of `rhs` + pub fn merge_transactions(mut lhs: Tx, rhs: Tx) -> Result { + for mut cmt in rhs.header.batch { + if lhs.header.batch.contains(&cmt) { + return Err(TxError::RepeatedSections); + } - for section in other.sections { - // PartialEq implementation of Section relies on an implementation - // on the inner types that doesn't account for the possible salt - // which is the correct behavior for this logic - if let Some(duplicate) = - self.sections.iter().find(|&sec| sec == §ion) - { - // Avoid pushing a duplicated section. Adjust the commitment of - // this inner tx for the different section's salt if needed - match duplicate { - Section::Code(_) => { - if cmt.code_hash == section.get_hash() { - cmt.code_hash = duplicate.get_hash(); + for section in rhs.sections.iter() { + // PartialEq implementation of Section relies on an + // implementation on the inner types that + // doesn't account for the possible salt + // which is the correct behavior for this logic + if let Some(duplicate) = + lhs.sections.iter().find(|&sec| sec == section) + { + // Avoid pushing a duplicated section. Adjust the commitment + // of this inner tx for the different + // section's salt if needed + match section { + Section::Code(_) => { + if cmt.code_hash == section.get_hash() { + cmt.code_hash = duplicate.get_hash(); + } } - } - Section::Data(_) => { - if cmt.data_hash == section.get_hash() { - cmt.data_hash = duplicate.get_hash(); + Section::Data(_) => { + if cmt.data_hash == section.get_hash() { + cmt.data_hash = duplicate.get_hash(); + } } - } - Section::ExtraData(_) => { - if cmt.memo_hash == section.get_hash() { - cmt.memo_hash = duplicate.get_hash(); + Section::ExtraData(_) => { + if cmt.memo_hash == section.get_hash() { + cmt.memo_hash = duplicate.get_hash(); + } } + // Other sections don't have a direct commitment in the + // header, so we can skip this section + _ => (), } - // Other sections don't have a direct commitment in the - // header - _ => (), + } else { + lhs.sections.push(section.to_owned()); } - } else { - self.sections.push(section); } + + lhs.header.batch.insert(cmt); } - self.header.batch.insert(cmt) + Ok(lhs) } /// Remove duplicated sections from the transaction @@ -1698,6 +1705,10 @@ mod test { let data_bytes3 = data_bytes2; let memo_bytes3 = memo_bytes1; + let code_bytes4 = "code four".as_bytes(); + let data_bytes4 = "bingbongfour".as_bytes(); + let memo_bytes4 = memo_bytes1; + let inner_tx1 = { let mut tx = Tx::default(); @@ -1726,9 +1737,11 @@ mod test { tx }; - let inner_tx3 = { + // This is actually a batch + let batch_tx3 = { let mut tx = Tx::default(); + // Push the first inner transaction let code = Code::new(code_bytes3.to_owned(), None); tx.set_code(code); @@ -1737,87 +1750,96 @@ mod test { tx.add_memo(memo_bytes3); + // Push the second inner transaction + tx.push_default_inner_tx(); + let code = Code::new(code_bytes4.to_owned(), None); + tx.set_code(code); + + let data = Data::new(data_bytes4.to_owned()); + tx.set_data(data); + + tx.add_memo(memo_bytes4); + tx }; let cmt1 = inner_tx1.first_commitments().unwrap().to_owned(); let mut cmt2 = inner_tx2.first_commitments().unwrap().to_owned(); - let mut cmt3 = inner_tx3.first_commitments().unwrap().to_owned(); + let mut cmt3 = batch_tx3.commitments().get_index(0).unwrap().to_owned(); + let mut cmt4 = batch_tx3.commitments().get_index(1).unwrap().to_owned(); - // Batch `inner_tx1`, `inner_tx2` and `inner_tx3` into `tx` + // Batch `inner_tx1`, `inner_tx2` and `batch_tx3` into `tx` let tx = { - let mut tx = Tx::default(); - - tx.add_inner_tx(inner_tx1, cmt1.clone()); + let tx = Tx::merge_transactions(Tx::default(), inner_tx1).unwrap(); assert_eq!(tx.first_commitments().unwrap(), &cmt1); assert_eq!(tx.header.batch.len(), 1); - tx.add_inner_tx(inner_tx2, cmt2.clone()); + let tx = Tx::merge_transactions(tx, inner_tx2).unwrap(); // Update cmt2 with the hash of cmt1 code section cmt2.code_hash = cmt1.code_hash; assert_eq!(tx.first_commitments().unwrap(), &cmt1); assert_eq!(tx.header.batch.len(), 2); assert_eq!(tx.header.batch.get_index(1).unwrap(), &cmt2); - tx.add_inner_tx(inner_tx3, cmt3.clone()); + let tx = Tx::merge_transactions(tx, batch_tx3).unwrap(); // Update cmt3 with the hash of cmt1 code and memo sections and the // hash of cmt2 data section cmt3.code_hash = cmt1.code_hash; cmt3.data_hash = cmt2.data_hash; cmt3.memo_hash = cmt1.memo_hash; assert_eq!(tx.first_commitments().unwrap(), &cmt1); - assert_eq!(tx.header.batch.len(), 3); + assert_eq!(tx.header.batch.len(), 4); assert_eq!(tx.header.batch.get_index(2).unwrap(), &cmt3); + // Update cmt4 with the hash of cmt1 memo sections + cmt4.memo_hash = cmt1.memo_hash; + assert_eq!(tx.header.batch.get_index(3).unwrap(), &cmt4); tx }; // Check sections of `inner_tx1` - assert!(tx.code(&cmt1).is_some()); assert_eq!(tx.code(&cmt1).unwrap(), code_bytes1); - assert!(tx.data(&cmt1).is_some()); assert_eq!(tx.data(&cmt1).unwrap(), data_bytes1); - assert!(tx.memo(&cmt1).is_some()); assert_eq!(tx.memo(&cmt1).unwrap(), memo_bytes1); // Check sections of `inner_tx2` - assert!(tx.code(&cmt2).is_some()); assert_eq!(tx.code(&cmt2).unwrap(), code_bytes2); - assert!(tx.data(&cmt2).is_some()); assert_eq!(tx.data(&cmt2).unwrap(), data_bytes2); - assert!(tx.memo(&cmt2).is_some()); assert_eq!(tx.memo(&cmt2).unwrap(), memo_bytes2); - // Check sections of `inner_tx3` - assert!(tx.code(&cmt3).is_some()); + // Check sections of `batch_tx3` assert_eq!(tx.code(&cmt3).unwrap(), code_bytes3); - assert!(tx.data(&cmt3).is_some()); assert_eq!(tx.data(&cmt3).unwrap(), data_bytes3); - assert!(tx.memo(&cmt3).is_some()); assert_eq!(tx.memo(&cmt3).unwrap(), memo_bytes3); + assert_eq!(tx.code(&cmt4).unwrap(), code_bytes4); + + assert_eq!(tx.data(&cmt4).unwrap(), data_bytes4); + + assert_eq!(tx.memo(&cmt4).unwrap(), memo_bytes4); + // Check that the redundant sections have been included only once in the // batch - assert_eq!(tx.sections.len(), 5); + assert_eq!(tx.sections.len(), 7); assert_eq!( tx.sections .iter() .filter(|section| section.code_sec().is_some()) .count(), - 1 + 2 ); assert_eq!( tx.sections .iter() .filter(|section| section.data().is_some()) .count(), - 2 + 3 ); assert_eq!( tx.sections