diff --git a/.changelog/unreleased/bug-fixes/4055-wrap-tx-by-elsewho.md b/.changelog/unreleased/bug-fixes/4055-wrap-tx-by-elsewho.md new file mode 100644 index 0000000000..f363190c16 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/4055-wrap-tx-by-elsewho.md @@ -0,0 +1,2 @@ +- CLI: Allow to wrap a raw tx signed by someone else. + ([\#4055](https://github.com/anoma/namada/pull/4055)) \ No newline at end of file diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 79bdc318dc..17de45a2fa 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -7557,26 +7557,14 @@ pub mod args { )) .conflicts_with_all([EXPIRATION_OPT.name]), ) - .arg( - SIGNING_KEYS - .def() - .help(wrap!( - "Sign the transaction with the key for the given \ - public key, public key hash or alias from your \ - wallet." - )) - .conflicts_with_all([SIGNATURES.name]), - ) - .arg( - SIGNATURES - .def() - .help(wrap!( - "List of file paths containing a serialized signature \ - to be attached to a transaction. Requires to provide \ - a gas payer." - )) - .conflicts_with_all([SIGNING_KEYS.name]), - ) + .arg(SIGNING_KEYS.def().help(wrap!( + "Sign the transaction with the key for the given public key, \ + public key hash or alias from your wallet." + ))) + .arg(SIGNATURES.def().help(wrap!( + "List of file paths containing a serialized signature to be \ + attached to a transaction. Requires to provide a gas payer." + ))) .arg( WRAPPER_SIGNATURE_OPT .def() diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index c5f3da2b0f..b0ff3ff119 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -170,11 +170,15 @@ pub async fn tx_signers( args: &args::Tx, default: Option
, ) -> Result, Error> { - let signer = if !&args.signing_keys.is_empty() { + let signer = if !args.signing_keys.is_empty() { return Ok(args.signing_keys.clone()); - } else { + } else if args.signatures.is_empty() { // Otherwise use the signer determined by the caller default + } else { + // If explicit signature(s) are provided signing keys are not required + // anymore + return Ok(vec![]); }; // Now actually fetch the signing key and apply it diff --git a/crates/tests/src/integration/ledger_tests.rs b/crates/tests/src/integration/ledger_tests.rs index 7649c72413..0a5a39cc01 100644 --- a/crates/tests/src/integration/ledger_tests.rs +++ b/crates/tests/src/integration/ledger_tests.rs @@ -24,6 +24,7 @@ use namada_node::shell::SnapshotSync; use namada_node::storage::DbSnapshot; use namada_sdk::account::AccountPublicKeysMap; use namada_sdk::collections::HashMap; +use namada_sdk::error::TxSubmitError; use namada_sdk::migrations; use namada_sdk::queries::RPC; use namada_sdk::token::{self, DenominatedAmount}; @@ -2353,6 +2354,217 @@ fn scheduled_migration() -> Result<()> { Ok(()) } +/// Test that a raw transaction can be wrapped and signed by someone else who +/// can pay for the gas fees for this tx. +/// +/// 1. Create a new account +/// 2. Credit the new account with some tokens to reveal its PK and have tiny +/// amount left +/// 3. Reveal the PK of the new account +/// 4. Check that the new account doesn't have sufficient balance to submit a +/// transfer tx +/// 5. Dump a raw tx of a transfer from the new account +/// 6. Sign the raw transaction +/// 7. Wrap the raw transaction by another account and submit it +#[test] +fn wrap_tx_by_elsewho() -> Result<()> { + let (node, _services) = setup::setup()?; + + // 1. Create a new account + let key_alias = "new-account"; + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Wallet, + apply_use_device(vec![ + "gen", + "--alias", + key_alias, + "--unsafe-dont-encrypt", + ]), + ) + }); + assert!(captured.result.is_ok()); + + // 2. Credit the new account with some tokens to reveal its PK and have tiny + // amount left + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "transparent-transfer", + "--source", + ALBERT, + "--target", + key_alias, + "--token", + NAM, + "--amount", + // transfer enough to cover reveal-pk gas fees (0.5) and to + // have only 0.000001 left after + "0.500001", + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // 3. Reveal the PK of the new account + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec!["reveal-pk", "--public-key", key_alias]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // Assert that there's only the smallest possible non-zero amount left + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec!["balance", "--owner", key_alias, "--token", NAM], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 0.000001")); + + // 4. Check that the new account doesn't have sufficient balance to submit a + // transfer tx + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "transparent-transfer", + "--source", + key_alias, + "--target", + ALBERT, + "--token", + NAM, + "--amount", + "0.000001", + ]), + ) + }); + assert!(captured.result.is_err()); + assert_matches!( + captured + .result + .unwrap_err() + .downcast_ref::() + .unwrap(), + namada_sdk::error::Error::Tx(TxSubmitError::BalanceTooLowForFees( + _, + _, + _, + _ + )) + ); + + // 5. Dump a raw tx of a transfer from the new account + let output_folder = node.test_dir.path(); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "transparent-transfer", + "--source", + key_alias, + "--target", + ALBERT, + "--token", + NAM, + "--amount", + "0.000001", + "--gas-payer", + CHRISTEL_KEY, + "--dump-tx", + "--output-folder-path", + &output_folder.to_str().unwrap(), + ]), + ) + }); + assert!(captured.result.is_ok()); + + let tx = find_files_with_ext(output_folder, "tx") + .unwrap() + .first() + .expect("Offline tx should be found.") + .to_path_buf() + .display() + .to_string(); + + // 6. Sign the raw transaction + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "utils", + "sign-offline", + "--data-path", + &tx, + "--secret-keys", + &key_alias, + "--output-folder-path", + &output_folder.to_str().unwrap(), + ]), + ) + }); + assert!(captured.result.is_ok()); + + let sig_files = find_files_with_ext(output_folder, "sig").unwrap(); + assert_eq!(sig_files.len(), 1); + let offline_sig = sig_files.first().unwrap().to_str().unwrap(); + + // 7. Wrap the raw transaction by another account and submit it + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + apply_use_device(vec![ + "tx", + "--tx-path", + &tx, + "--signatures", + &offline_sig, + "--gas-payer", + CHRISTEL_KEY, + ]), + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // Assert changed balances + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec!["balance", "--owner", ALBERT, "--token", NAM], + ) + }); + assert!(captured.contains("nam: 1979999.5\n")); + + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec!["balance", "--owner", key_alias, "--token", NAM], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 0\n")); + + Ok(()) +} + fn make_migration_json() -> (Hash, tempfile::NamedTempFile) { let file = tempfile::Builder::new().tempfile().expect("Test failed"); let updates = [migrations::DbUpdateType::Add {