Murisi/fix hw tests by unbatching

.changelog/unreleased/improvements/4228-fix-hw-tests-by-unbatching.md

@@ -0,0 +1,3 @@
+- Stop batching transactions that need to be signed on
+  the hardware wallet because these are not supported yet.
+  ([\#4228](https://github.com/anoma/namada/pull/4228))

crates/apps_lib/src/client/tx.rs

@@ -146,6 +146,11 @@ where
         )));
     }
     // Get the Ledger to sign using our obtained derivation path
+    println!(
+        "Requesting that hardware wallet sign transaction with transparent \
+         key at {}...",
+        path.path
+    );
     let response = app
         .sign(&path, &tx.serialize_to_vec())
         .await
@@ -258,7 +263,7 @@ async fn batch_opt_reveal_pk_and_submit<N: Namada>(
     namada: &N,
     args: &args::Tx,
     owners: &[&Address],
-    tx_data: (Tx, SigningTxData),
+    mut tx_data: (Tx, SigningTxData),
 ) -> Result<ProcessTxResponse, error::Error>
 where
     <N::Client as namada_sdk::io::Client>::Error: std::fmt::Display,
@@ -272,15 +277,33 @@ where
             batched_tx_data.push(reveal_pk_tx_data);
         }
     }
-    batched_tx_data.push(tx_data);
 
-    let (mut batched_tx, batched_signing_data) =
-        namada_sdk::tx::build_batch(batched_tx_data)?;
-    for sig_data in batched_signing_data {
-        sign(namada, &mut batched_tx, args, sig_data).await?;
+    // Since hardware wallets do not yet support batched transactions, use a
+    // different logic in order to achieve compatibility
+    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, &mut tx_data.0, args, tx_data.1).await?;
+        // Then submit each transaction separately
+        for (tx, _sig_data) in batched_tx_data {
+            namada.submit(tx, args).await?;
+        }
+        // The result of submitting this function's argument is what is returned
+        namada.submit(tx_data.0, args).await
+    } else {
+        // Otherwise complete the batch with this function's argument
+        batched_tx_data.push(tx_data);
+        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?;
+        }
+        // Then finally submit everything in one go
+        namada.submit(batched_tx, args).await
     }
-
-    namada.submit(batched_tx, args).await
 }
 
 pub async fn submit_bridge_pool_tx<N: Namada>(
@@ -943,6 +966,10 @@ async fn augment_masp_hardware_keys(
                 // Then confirm that the viewing key at this path in the
                 // hardware wallet matches the viewing key in this pseudo
                 // spending key
+                println!(
+                    "Requesting viewing key at {} from hardware wallet...",
+                    path.path
+                );
                 let response = app
                     .retrieve_keys(&path, NamadaKeys::ViewKey, true)
                     .await
@@ -1120,6 +1147,11 @@ async fn masp_sign(
             let path = BIP44Path {
                 path: path.to_string(),
             };
+            println!(
+                "Requesting that hardware wallet sign shielded transfer with \
+                 spending key at {}...",
+                path.path
+            );
             app.sign_masp_spends(&path, &tx.serialize_to_vec())
                 .await
                 .map_err(|err| error::Error::Other(err.to_string()))?;

crates/tests/src/e2e/ibc_tests.rs

@@ -1403,7 +1403,7 @@ fn transfer_on_chain(
         &rpc,
     ]);
     tx_args.extend_from_slice(extra_args);
-    let mut client = run!(test, Bin::Client, tx_args, Some(120))?;
+    let mut client = run!(test, Bin::Client, tx_args, Some(240))?;
     client.exp_string(TX_APPLIED_SUCCESS)?;
     client.assert_success();
 

crates/tests/src/integration/ledger_tests.rs

@@ -2381,12 +2381,7 @@ fn wrap_tx_by_elsewho() -> Result<()> {
         run(
             &node,
             Bin::Wallet,
-            apply_use_device(vec![
-                "gen",
-                "--alias",
-                key_alias,
-                "--unsafe-dont-encrypt",
-            ]),
+            vec!["gen", "--alias", key_alias, "--unsafe-dont-encrypt"],
         )
     });
     assert!(captured.result.is_ok());
@@ -2510,7 +2505,7 @@ fn wrap_tx_by_elsewho() -> Result<()> {
         run(
             &node,
             Bin::Client,
-            apply_use_device(vec![
+            vec![
                 "utils",
                 "sign-offline",
                 "--data-path",
@@ -2519,7 +2514,7 @@ fn wrap_tx_by_elsewho() -> Result<()> {
                 &key_alias,
                 "--output-folder-path",
                 &output_folder.to_str().unwrap(),
-            ]),
+            ],
         )
     });
     assert!(captured.result.is_ok());
@@ -2594,12 +2589,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
         run(
             &node,
             Bin::Wallet,
-            apply_use_device(vec![
-                "gen",
-                "--alias",
-                key_alias,
-                "--unsafe-dont-encrypt",
-            ]),
+            vec!["gen", "--alias", key_alias, "--unsafe-dont-encrypt"],
         )
     });
     assert!(captured.result.is_ok());
@@ -2721,7 +2711,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
         run(
             &node,
             Bin::Client,
-            apply_use_device(vec![
+            vec![
                 "utils",
                 "sign-offline",
                 "--data-path",
@@ -2730,7 +2720,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
                 &key_alias,
                 "--output-folder-path",
                 &output_folder.to_str().unwrap(),
-            ]),
+            ],
         )
     });
     assert!(captured.result.is_ok());
@@ -2774,7 +2764,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
         run(
             &node,
             Bin::Client,
-            apply_use_device(vec![
+            vec![
                 "utils",
                 "sign-offline",
                 "--data-path",
@@ -2783,7 +2773,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
                 CHRISTEL_KEY,
                 "--output-folder-path",
                 &output_folder.to_str().unwrap(),
-            ]),
+            ],
         )
     });
     assert!(captured.result.is_ok());
@@ -2862,12 +2852,7 @@ fn offline_wrapper_tx() -> Result<()> {
         run(
             &node,
             Bin::Wallet,
-            apply_use_device(vec![
-                "gen",
-                "--alias",
-                key_alias,
-                "--unsafe-dont-encrypt",
-            ]),
+            vec!["gen", "--alias", key_alias, "--unsafe-dont-encrypt"],
         )
     });
     assert!(captured.result.is_ok());
@@ -2990,7 +2975,7 @@ fn offline_wrapper_tx() -> Result<()> {
         run(
             &node,
             Bin::Client,
-            apply_use_device(vec![
+            vec![
                 "utils",
                 "sign-offline",
                 "--data-path",
@@ -3001,7 +2986,7 @@ fn offline_wrapper_tx() -> Result<()> {
                 CHRISTEL_KEY,
                 "--output-folder-path",
                 &output_folder.to_str().unwrap(),
-            ]),
+            ],
         )
     });
     assert!(captured.result.is_ok());
@@ -3107,11 +3092,8 @@ fn pos_validator_metadata_validation() -> Result<()> {
     assert!(captured.contains(TX_APPLIED_SUCCESS));
 
     // 3. Check that the metadata has changed.
-    let query_args = apply_use_device(vec![
-        "validator-metadata",
-        "--validator",
-        "validator-0-validator",
-    ]);
+    let query_args =
+        vec!["validator-metadata", "--validator", "validator-0-validator"];
     let captured =
         CapturedOutput::of(|| run(&node, Bin::Client, query_args.clone()));
     println!("{:?}", captured.result);