Expand MASP hardware wallet support to other transaction types.

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<sapling::Authorized, sapling::Authorized>
 // 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<Item = &mut PseudoExtendedKey>,
 ) -> Result<HashMap<String, ExtendedViewingKey>, 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<Box<dyn BuildParams>, 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<N: Namada>(
     namada: &N,
-    args: args::TxIbcTransfer,
+    mut args: args::TxIbcTransfer,
 ) -> Result<(), error::Error>
 where
     <N::Client as namada_sdk::io::Client>::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,

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<Address> {
         match self {

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<MaspEpoch>)>
     {
-        tx::build_ibc_transfer(context, self).await
+        tx::build_ibc_transfer(context, self, bparams).await
     }
 }
 

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<MaspEpoch>)> {
     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<N: Namada>(
 pub async fn build_shielding_transfer<N: Namada>(
     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<N: Namada>(
         // 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<N: Namada>(
         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<N: Namada>(
         });
 
         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<N: Namada>(
 pub async fn build_unshielding_transfer<N: Namada>(
     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<N: Namada>(
         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<N: Namada>(
         });
 
         data.shielded_section_hash = Some(shielded_section_hash);
+        signing_data.shielded_hash = Some(shielded_section_hash);
         tracing::debug!("Transfer data {data:?}");
         Ok(())
     };