Skip to content

Commit c0b9f68

Browse files
mergify[bot]mergify[bot]
authored
Merge pull request #4228 from anoma/murisi/fix-hw-tests-by-unbatching
Murisi/fix hw tests by unbatching
2 parents 9527ea9 + 672d99e commit c0b9f68

File tree

4 files changed

+57
-40
lines changed

4 files changed

+57
-40
lines changed

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

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

‎crates/apps_lib/src/client/tx.rs

+40-8
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,11 @@ where
146146
)));
147147
}
148148
// Get the Ledger to sign using our obtained derivation path
149+
println!(
150+
"Requesting that hardware wallet sign transaction with transparent \
151+
key at {}...",
152+
path.path
153+
);
149154
let response = app
150155
.sign(&path, &tx.serialize_to_vec())
151156
.await
@@ -258,7 +263,7 @@ async fn batch_opt_reveal_pk_and_submit<N: Namada>(
258263
namada: &N,
259264
args: &args::Tx,
260265
owners: &[&Address],
261-
tx_data: (Tx, SigningTxData),
266+
mut tx_data: (Tx, SigningTxData),
262267
) -> Result<ProcessTxResponse, error::Error>
263268
where
264269
<N::Client as namada_sdk::io::Client>::Error: std::fmt::Display,
@@ -272,15 +277,33 @@ where
272277
batched_tx_data.push(reveal_pk_tx_data);
273278
}
274279
}
275-
batched_tx_data.push(tx_data);
276280

277-
let (mut batched_tx, batched_signing_data) =
278-
namada_sdk::tx::build_batch(batched_tx_data)?;
279-
for sig_data in batched_signing_data {
280-
sign(namada, &mut batched_tx, args, sig_data).await?;
281+
// Since hardware wallets do not yet support batched transactions, use a
282+
// different logic in order to achieve compatibility
283+
if args.use_device {
284+
// Sign each transaction separately
285+
for (tx, sig_data) in &mut batched_tx_data {
286+
sign(namada, tx, args, sig_data.clone()).await?;
287+
}
288+
sign(namada, &mut tx_data.0, args, tx_data.1).await?;
289+
// Then submit each transaction separately
290+
for (tx, _sig_data) in batched_tx_data {
291+
namada.submit(tx, args).await?;
292+
}
293+
// The result of submitting this function's argument is what is returned
294+
namada.submit(tx_data.0, args).await
295+
} else {
296+
// Otherwise complete the batch with this function's argument
297+
batched_tx_data.push(tx_data);
298+
let (mut batched_tx, batched_signing_data) =
299+
namada_sdk::tx::build_batch(batched_tx_data)?;
300+
// Sign the batch with the union of the signers required for each part
301+
for sig_data in batched_signing_data {
302+
sign(namada, &mut batched_tx, args, sig_data).await?;
303+
}
304+
// Then finally submit everything in one go
305+
namada.submit(batched_tx, args).await
281306
}
282-
283-
namada.submit(batched_tx, args).await
284307
}
285308

286309
pub async fn submit_bridge_pool_tx<N: Namada>(
@@ -943,6 +966,10 @@ async fn augment_masp_hardware_keys(
943966
// Then confirm that the viewing key at this path in the
944967
// hardware wallet matches the viewing key in this pseudo
945968
// spending key
969+
println!(
970+
"Requesting viewing key at {} from hardware wallet...",
971+
path.path
972+
);
946973
let response = app
947974
.retrieve_keys(&path, NamadaKeys::ViewKey, true)
948975
.await
@@ -1120,6 +1147,11 @@ async fn masp_sign(
11201147
let path = BIP44Path {
11211148
path: path.to_string(),
11221149
};
1150+
println!(
1151+
"Requesting that hardware wallet sign shielded transfer with \
1152+
spending key at {}...",
1153+
path.path
1154+
);
11231155
app.sign_masp_spends(&path, &tx.serialize_to_vec())
11241156
.await
11251157
.map_err(|err| error::Error::Other(err.to_string()))?;

‎crates/tests/src/e2e/ibc_tests.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1403,7 +1403,7 @@ fn transfer_on_chain(
14031403
&rpc,
14041404
]);
14051405
tx_args.extend_from_slice(extra_args);
1406-
let mut client = run!(test, Bin::Client, tx_args, Some(120))?;
1406+
let mut client = run!(test, Bin::Client, tx_args, Some(240))?;
14071407
client.exp_string(TX_APPLIED_SUCCESS)?;
14081408
client.assert_success();
14091409

‎crates/tests/src/integration/ledger_tests.rs

+13-31
Original file line numberDiff line numberDiff line change
@@ -2381,12 +2381,7 @@ fn wrap_tx_by_elsewho() -> Result<()> {
23812381
run(
23822382
&node,
23832383
Bin::Wallet,
2384-
apply_use_device(vec![
2385-
"gen",
2386-
"--alias",
2387-
key_alias,
2388-
"--unsafe-dont-encrypt",
2389-
]),
2384+
vec!["gen", "--alias", key_alias, "--unsafe-dont-encrypt"],
23902385
)
23912386
});
23922387
assert!(captured.result.is_ok());
@@ -2510,7 +2505,7 @@ fn wrap_tx_by_elsewho() -> Result<()> {
25102505
run(
25112506
&node,
25122507
Bin::Client,
2513-
apply_use_device(vec![
2508+
vec![
25142509
"utils",
25152510
"sign-offline",
25162511
"--data-path",
@@ -2519,7 +2514,7 @@ fn wrap_tx_by_elsewho() -> Result<()> {
25192514
&key_alias,
25202515
"--output-folder-path",
25212516
&output_folder.to_str().unwrap(),
2522-
]),
2517+
],
25232518
)
25242519
});
25252520
assert!(captured.result.is_ok());
@@ -2594,12 +2589,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
25942589
run(
25952590
&node,
25962591
Bin::Wallet,
2597-
apply_use_device(vec![
2598-
"gen",
2599-
"--alias",
2600-
key_alias,
2601-
"--unsafe-dont-encrypt",
2602-
]),
2592+
vec!["gen", "--alias", key_alias, "--unsafe-dont-encrypt"],
26032593
)
26042594
});
26052595
assert!(captured.result.is_ok());
@@ -2721,7 +2711,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
27212711
run(
27222712
&node,
27232713
Bin::Client,
2724-
apply_use_device(vec![
2714+
vec![
27252715
"utils",
27262716
"sign-offline",
27272717
"--data-path",
@@ -2730,7 +2720,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
27302720
&key_alias,
27312721
"--output-folder-path",
27322722
&output_folder.to_str().unwrap(),
2733-
]),
2723+
],
27342724
)
27352725
});
27362726
assert!(captured.result.is_ok());
@@ -2774,7 +2764,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
27742764
run(
27752765
&node,
27762766
Bin::Client,
2777-
apply_use_device(vec![
2767+
vec![
27782768
"utils",
27792769
"sign-offline",
27802770
"--data-path",
@@ -2783,7 +2773,7 @@ fn offline_wrap_tx_by_elsewho() -> Result<()> {
27832773
CHRISTEL_KEY,
27842774
"--output-folder-path",
27852775
&output_folder.to_str().unwrap(),
2786-
]),
2776+
],
27872777
)
27882778
});
27892779
assert!(captured.result.is_ok());
@@ -2862,12 +2852,7 @@ fn offline_wrapper_tx() -> Result<()> {
28622852
run(
28632853
&node,
28642854
Bin::Wallet,
2865-
apply_use_device(vec![
2866-
"gen",
2867-
"--alias",
2868-
key_alias,
2869-
"--unsafe-dont-encrypt",
2870-
]),
2855+
vec!["gen", "--alias", key_alias, "--unsafe-dont-encrypt"],
28712856
)
28722857
});
28732858
assert!(captured.result.is_ok());
@@ -2990,7 +2975,7 @@ fn offline_wrapper_tx() -> Result<()> {
29902975
run(
29912976
&node,
29922977
Bin::Client,
2993-
apply_use_device(vec![
2978+
vec![
29942979
"utils",
29952980
"sign-offline",
29962981
"--data-path",
@@ -3001,7 +2986,7 @@ fn offline_wrapper_tx() -> Result<()> {
30012986
CHRISTEL_KEY,
30022987
"--output-folder-path",
30032988
&output_folder.to_str().unwrap(),
3004-
]),
2989+
],
30052990
)
30062991
});
30072992
assert!(captured.result.is_ok());
@@ -3107,11 +3092,8 @@ fn pos_validator_metadata_validation() -> Result<()> {
31073092
assert!(captured.contains(TX_APPLIED_SUCCESS));
31083093

31093094
// 3. Check that the metadata has changed.
3110-
let query_args = apply_use_device(vec![
3111-
"validator-metadata",
3112-
"--validator",
3113-
"validator-0-validator",
3114-
]);
3095+
let query_args =
3096+
vec!["validator-metadata", "--validator", "validator-0-validator"];
31153097
let captured =
31163098
CapturedOutput::of(|| run(&node, Bin::Client, query_args.clone()));
31173099
println!("{:?}", captured.result);

0 commit comments

Comments
 (0)