diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index ecffb7c028..2f186b97dd 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -12,7 +12,7 @@ use masp_primitives::transaction::components::sapling::builder::{ SpendBuildParams, StoredBuildParams, }; use masp_primitives::transaction::components::sapling::fees::InputView; -use masp_primitives::zip32::{ExtendedFullViewingKey, ExtendedKey}; +use masp_primitives::zip32::{ExtendedFullViewingKey, ExtendedKey, PseudoExtendedKey}; use namada_sdk::address::{Address, ImplicitAddress}; use namada_sdk::args::TxBecomeValidator; use namada_sdk::collections::{HashMap, HashSet}; @@ -43,6 +43,22 @@ use crate::wallet::{ gen_validator_keys, read_and_confirm_encryption_password, WalletTransport, }; +// Maximum number of spend description randomness parameters that can be +// generated on the hardware wallet. It is hard to compute the exact required +// number because a given MASP source could be distributed amongst several +// notes. +const MAX_HW_SPEND: usize = 10; +// Maximum number of convert description randomness parameters that can be +// generated on the hardware wallet. It is hard to compute the exact required +// number because the number of conversions that are used depends on the +// protocol's current state. +const MAX_HW_CONVERT: usize = 10; +// Maximum number of output description randomness parameters that can be +// generated on the hardware wallet. It is hard to compute the exact required +// number because the number of outputs depends on the number of dummy outputs +// introduced. +const MAX_HW_OUTPUT: usize = 10; + /// Wrapper around `signing::aux_signing_data` that stores the optional /// disposable address to the wallet pub async fn aux_signing_data( @@ -862,24 +878,25 @@ impl sapling::MapAuth // it does on the software client. async fn augment_masp_hardware_keys( namada: &impl Namada, - args: &mut args::TxShieldedTransfer, + args: &args::Tx, + sources: impl Iterator, ) -> Result, error::Error> { // Records the shielded keys that are on the hardware wallet let mut shielded_hw_keys = HashMap::new(); // Construct the build parameters that parameterized the Transaction // authorizations - if args.tx.use_device { - let transport = WalletTransport::from_arg(args.tx.device_transport); + if args.use_device { + let transport = WalletTransport::from_arg(args.device_transport); let app = NamadaApp::new(transport); let wallet = namada.wallet().await; // Augment the pseudo spending key with a proof authorization key - for data in &mut args.data { + for source in sources { // Only attempt an augmentation if proof authorization is not there - if data.source.to_spending_key().is_none() { + if source.to_spending_key().is_none() { // First find the derivation path corresponding to this viewing // key let viewing_key = - ExtendedViewingKey::from(data.source.to_viewing_key()); + ExtendedViewingKey::from(source.to_viewing_key()); let path = wallet .find_path_by_viewing_key(&viewing_key) .map_err(|err| { @@ -950,7 +967,7 @@ async fn augment_masp_hardware_keys( )) })?; // Augment the pseudo spending key - data.source.augment_proof_generation_key(pgk).map_err( + source.augment_proof_generation_key(pgk).map_err( |_| { error::Error::Other( "Proof generation key in response from the \ @@ -962,7 +979,7 @@ async fn augment_masp_hardware_keys( )?; // Finally, augment an incorrect spend authorization key just to // make sure that the Transaction is built. - data.source.augment_spend_authorizing_key_unchecked( + source.augment_spend_authorizing_key_unchecked( PrivateKey(jubjub::Fr::default()), ); shielded_hw_keys.insert(path.path, viewing_key); @@ -977,12 +994,15 @@ async fn augment_masp_hardware_keys( // If the hardware wallet is beig used, use it to generate the random build // parameters for the spend, convert, and output descriptions. async fn generate_masp_build_params( - args: &args::TxShieldedTransfer, + spend_len: usize, + convert_len: usize, + output_len: usize, + args: &args::Tx, ) -> Result, error::Error> { // Construct the build parameters that parameterized the Transaction // authorizations - if args.tx.use_device { - let transport = WalletTransport::from_arg(args.tx.device_transport); + if args.use_device { + let transport = WalletTransport::from_arg(args.device_transport); let app = NamadaApp::new(transport); // Clear hardware wallet randomness buffers app.clean_randomness_buffers().await.map_err(|err| { @@ -993,16 +1013,6 @@ async fn generate_masp_build_params( })?; // Get randomness to aid in construction of various descriptors let mut bparams = StoredBuildParams::default(); - // Number of spend descriptions is the number of transfers - let spend_len = args.data.len(); - // Number of convert description is assumed to be double the number of - // transfers. This is because each spend description might first be - // converted to epoch 0 before going to the intended epoch. - let convert_len = args.data.len() * 2; - // Number of output descriptions is assumed to be double the number of - // transfers. This is because there may be change from each output - // that's destined for the sender. - let output_len = args.data.len() * 2; for _ in 0..spend_len { let spend_randomness = app .get_spend_randomness() @@ -1143,9 +1153,19 @@ pub async fn submit_shielded_transfer( namada: &impl Namada, mut args: args::TxShieldedTransfer, ) -> Result<(), error::Error> { + let sources = args + .data + .iter_mut() + .map(|x| &mut x.source) + .chain(args.gas_spending_keys.iter_mut()); let shielded_hw_keys = - augment_masp_hardware_keys(namada, &mut args).await?; - let mut bparams = generate_masp_build_params(&args).await?; + augment_masp_hardware_keys(namada, &mut args.tx, sources).await?; + let mut bparams = generate_masp_build_params( + MAX_HW_SPEND, + MAX_HW_CONVERT, + MAX_HW_OUTPUT, + &args.tx, + ).await?; let (mut tx, signing_data) = args.clone().build(namada, &mut bparams).await?; @@ -1165,7 +1185,13 @@ pub async fn submit_shielding_transfer( ) -> Result<(), error::Error> { // Repeat once if the tx fails on a crossover of an epoch for _ in 0..2 { - let (tx, signing_data, tx_epoch) = args.clone().build(namada).await?; + let mut bparams = generate_masp_build_params( + MAX_HW_SPEND, + MAX_HW_CONVERT, + MAX_HW_OUTPUT, + &args.tx, + ).await?; + let (tx, signing_data, tx_epoch) = args.clone().build(namada, &mut bparams).await?; if args.tx.dump_tx { tx::dump_tx(namada.io(), &args.tx, tx); @@ -1217,13 +1243,24 @@ pub async fn submit_shielding_transfer( pub async fn submit_unshielding_transfer( namada: &impl Namada, - args: args::TxUnshieldingTransfer, + mut args: args::TxUnshieldingTransfer, ) -> Result<(), error::Error> { - let (mut tx, signing_data) = args.clone().build(namada).await?; + let sources = std::iter::once(&mut args.source) + .chain(args.gas_spending_keys.iter_mut()); + let shielded_hw_keys = + augment_masp_hardware_keys(namada, &mut args.tx, sources).await?; + let mut bparams = generate_masp_build_params( + MAX_HW_SPEND, + MAX_HW_CONVERT, + MAX_HW_OUTPUT, + &args.tx, + ).await?; + let (mut tx, signing_data) = args.clone().build(namada, &mut bparams).await?; if args.tx.dump_tx { tx::dump_tx(namada.io(), &args.tx, tx); } else { + masp_sign(&mut tx, &args.tx, &signing_data, shielded_hw_keys).await?; sign(namada, &mut tx, &args.tx, signing_data).await?; namada.submit(tx, &args.tx).await?; } @@ -1232,16 +1269,30 @@ pub async fn submit_unshielding_transfer( pub async fn submit_ibc_transfer( namada: &N, - args: args::TxIbcTransfer, + mut args: args::TxIbcTransfer, ) -> Result<(), error::Error> where ::Error: std::fmt::Display, { - let (tx, signing_data, _) = args.build(namada).await?; + let sources = args + .source + .spending_key_mut() + .into_iter() + .chain(args.gas_spending_keys.iter_mut()); + let shielded_hw_keys = + augment_masp_hardware_keys(namada, &args.tx, sources).await?; + let mut bparams = generate_masp_build_params( + MAX_HW_SPEND, + MAX_HW_CONVERT, + MAX_HW_OUTPUT, + &args.tx, + ).await?; + let (mut tx, signing_data, _) = args.build(namada, &mut bparams).await?; if args.tx.dump_tx { tx::dump_tx(namada.io(), &args.tx, tx); } else { + masp_sign(&mut tx, &args.tx, &signing_data, shielded_hw_keys).await?; batch_opt_reveal_pk_and_submit( namada, &args.tx, diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index ca00a4b7be..62ef4aa69b 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -547,6 +547,14 @@ impl TransferSource { } } + /// Get the contained ExtendedSpendingKey contained, if any + pub fn spending_key_mut(&mut self) -> Option<&mut PseudoExtendedKey> { + match self { + Self::ExtendedSpendingKey(x) => Some(x), + _ => None, + } + } + /// Get the contained Address, if any pub fn address(&self) -> Option
{ match self { diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 7ecfd966d5..4f8a158b13 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -430,8 +430,9 @@ impl TxShieldingTransfer { pub async fn build( &mut self, context: &impl Namada, + bparams: &mut impl BuildParams, ) -> crate::error::Result<(namada_tx::Tx, SigningTxData, MaspEpoch)> { - tx::build_shielding_transfer(context, self).await + tx::build_shielding_transfer(context, self, bparams).await } } @@ -469,8 +470,9 @@ impl TxUnshieldingTransfer { pub async fn build( &mut self, context: &impl Namada, + bparams: &mut impl BuildParams, ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { - tx::build_unshielding_transfer(context, self).await + tx::build_unshielding_transfer(context, self, bparams).await } } @@ -618,9 +620,10 @@ impl TxIbcTransfer { pub async fn build( &self, context: &impl Namada, + bparams: &mut impl BuildParams, ) -> crate::error::Result<(namada_tx::Tx, SigningTxData, Option)> { - tx::build_ibc_transfer(context, self).await + tx::build_ibc_transfer(context, self, bparams).await } } diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index a5e47be354..935b35b1c5 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2522,6 +2522,7 @@ pub async fn build_pgf_stewards_proposal( pub async fn build_ibc_transfer( context: &impl Namada, args: &args::TxIbcTransfer, + bparams: &mut impl BuildParams, ) -> Result<(Tx, SigningTxData, Option)> { if args.ibc_shielding_data.is_some() && args.ibc_memo.is_some() { return Err(Error::Other( @@ -2535,7 +2536,7 @@ pub async fn build_ibc_transfer( get_refund_target(context, &args.source, &args.refund_target).await?; let source = args.source.effective_address(); - let signing_data = signing::aux_signing_data( + let mut signing_data = signing::aux_signing_data( context, &args.tx, Some(source.clone()), @@ -2631,7 +2632,7 @@ pub async fn build_ibc_transfer( masp_fee_data, !(args.tx.dry_run || args.tx.dry_run_wrapper), args.tx.expiration.to_datetime(), - &mut RngBuildParams::new(OsRng), + bparams, ) .await?; let shielded_tx_epoch = shielded_parts.as_ref().map(|trans| trans.0.epoch); @@ -2682,6 +2683,7 @@ 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); tx.add_masp_builder(MaspBuilder { asset_types, metadata: shielded_transfer.metadata, @@ -3181,6 +3183,7 @@ async fn get_masp_fee_payment_amount( pub async fn build_shielding_transfer( context: &N, args: &mut args::TxShieldingTransfer, + bparams: &mut impl BuildParams, ) -> Result<(Tx, SigningTxData, MaspEpoch)> { let source = if args.data.len() == 1 { // If only one transfer take its source as the signer @@ -3192,7 +3195,7 @@ pub async fn build_shielding_transfer( // argument None }; - let signing_data = signing::aux_signing_data( + let mut signing_data = signing::aux_signing_data( context, &args.tx, source.clone(), @@ -3267,7 +3270,7 @@ pub async fn build_shielding_transfer( None, !(args.tx.dry_run || args.tx.dry_run_wrapper), args.tx.expiration.to_datetime(), - &mut RngBuildParams::new(OsRng), + bparams, ) .await? .expect("Shielding transfer must have shielded parts"); @@ -3298,6 +3301,7 @@ pub async fn build_shielding_transfer( }); data.shielded_section_hash = Some(shielded_section_hash); + signing_data.shielded_hash = Some(shielded_section_hash); tracing::debug!("Transfer data {data:?}"); Ok(()) }; @@ -3319,8 +3323,9 @@ pub async fn build_shielding_transfer( pub async fn build_unshielding_transfer( context: &N, args: &mut args::TxUnshieldingTransfer, + bparams: &mut impl BuildParams, ) -> Result<(Tx, SigningTxData)> { - let signing_data = signing::aux_signing_data( + let mut signing_data = signing::aux_signing_data( context, &args.tx, Some(MASP), @@ -3390,7 +3395,7 @@ pub async fn build_unshielding_transfer( masp_fee_data, !(args.tx.dry_run || args.tx.dry_run_wrapper), args.tx.expiration.to_datetime(), - &mut RngBuildParams::new(OsRng), + bparams, ) .await? .expect("Shielding transfer must have shielded parts"); @@ -3420,6 +3425,7 @@ pub async fn build_unshielding_transfer( }); data.shielded_section_hash = Some(shielded_section_hash); + signing_data.shielded_hash = Some(shielded_section_hash); tracing::debug!("Transfer data {data:?}"); Ok(()) };