From 44729a243f425820339c098658aeb3af2c9e3e19 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 23 Sep 2024 17:16:45 +0200 Subject: [PATCH 1/9] Adds masp batch integration tests --- crates/tests/src/integration/masp.rs | 443 ++++++++++++++++++++++++++- 1 file changed, 438 insertions(+), 5 deletions(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index f960552368..681a05ee75 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -3,7 +3,9 @@ use std::str::FromStr; use color_eyre::eyre::Result; use color_eyre::owo_colors::OwoColorize; -use namada_apps_lib::wallet::defaults::{albert_keypair, christel_keypair}; +use namada_apps_lib::wallet::defaults::{ + albert_keypair, bertha_keypair, christel_keypair, +}; use namada_core::dec::Dec; use namada_core::masp::TokenMap; use namada_node::shell::testing::client::run; @@ -3541,7 +3543,7 @@ fn masp_fee_payment_with_different_token() -> Result<()> { // An ouput description of the masp can be replayed (pushed to the commitment // tree more than once). The nullifiers and merkle paths will be unique. Test // that a batch containing two identical shielding txs can be executed correctly -// and the two identical notes can be spent with no issues. +// and the two identical notes can be spent (nullified) with no issues. #[test] fn identical_output_descriptions() -> Result<()> { // This address doesn't matter for tests. But an argument is required. @@ -3585,7 +3587,7 @@ fn identical_output_descriptions() -> Result<()> { "--gas-limit", "300000", "--gas-payer", - ALBERT_KEY, + BERTHA_KEY, "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-tx", @@ -3639,7 +3641,7 @@ fn identical_output_descriptions() -> Result<()> { ), None, ); - batched_tx.sign_wrapper(albert_keypair()); + batched_tx.sign_wrapper(bertha_keypair()); let wrapper_hash = batched_tx.wrapper_hash(); let inner_cmts = batched_tx.commitments(); @@ -3664,7 +3666,7 @@ fn identical_output_descriptions() -> Result<()> { assert_eq!(results.len(), 1); for result in results.iter() { - // The batch should contain a two inner tx + // The batch should contain two inner txs assert_eq!(result.0.len(), 2); for inner_cmt in inner_cmts { @@ -3714,6 +3716,24 @@ fn identical_output_descriptions() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains("nam: 2000")); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + ALBERT_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 1998000")); + let captured = CapturedOutput::of(|| { run( &node, @@ -3797,3 +3817,416 @@ fn identical_output_descriptions() -> Result<()> { Ok(()) } + +// Test MASP batched txs where one is failing and one is successful and check +// that both the protocol and the shielded sync command behave correctly. Since +// the batches are not atomic check that the valid transactions get committed +// and the balances are correctly updated +#[test] +fn masp_batch() -> Result<()> { + // This address doesn't matter for tests. But an argument is required. + let validator_one_rpc = "http://127.0.0.1:26567"; + // Download the shielded pool parameters before starting node + let _ = FsShieldedUtils::new(PathBuf::new()); + let (mut node, _services) = setup::setup()?; + _ = node.next_masp_epoch(); + let tempdir = tempfile::tempdir().unwrap(); + + // Generate txs for the batch to shield some tokens. Use two different + // sources + let mut batch = vec![]; + for source in [ALBERT_KEY, BERTHA_KEY] { + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + source, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1000", + "--gas-limit", + "300000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + )?; + node.assert_success(); + let file_path = tempdir + .path() + .read_dir() + .unwrap() + .next() + .unwrap() + .unwrap() + .path(); + batch.push(std::fs::read(&file_path).unwrap()); + std::fs::remove_file(&file_path).unwrap(); + } + + // Create the batch + let tx0: namada_sdk::tx::Tx = serde_json::from_slice(&batch[0]).unwrap(); + let tx1: namada_sdk::tx::Tx = serde_json::from_slice(&batch[1]).unwrap(); + + let signing_data = SigningTxData { + owner: None, + public_keys: vec![albert_keypair().to_public()], + threshold: 1, + account_public_keys_map: None, + fee_payer: albert_keypair().to_public(), + }; + + let mut txs = vec![]; + let mut inner_cmts = vec![]; + let mut wrapper_hashes = vec![]; + + // Try different tx orders and generate a single block with both batch + // combinations + for (tx0, tx1) in [(tx0.clone(), tx1.clone()), (tx1, tx0)] { + let (mut batched_tx, _signing_data) = + namada_sdk::tx::build_batch(vec![ + (tx0, signing_data.clone()), + (tx1, signing_data.clone()), + ]) + .unwrap(); + batched_tx.header.atomic = false; + + // Sign the batch with just the signer of one tx to force the failure of + // the other one + batched_tx.sign_raw( + vec![albert_keypair()], + AccountPublicKeysMap::from_iter( + vec![(albert_keypair().to_public())].into_iter(), + ), + None, + ); + batched_tx.sign_wrapper(christel_keypair()); + + wrapper_hashes.push(batched_tx.wrapper_hash()); + for cmt in batched_tx.commitments() { + inner_cmts.push(cmt.to_owned()); + } + + txs.push(batched_tx.to_bytes()); + } + + node.clear_results(); + node.submit_txs(txs); + + // Check the block result + { + let codes = node.tx_result_codes.lock().unwrap(); + // If empty than failed in process proposal + assert!(!codes.is_empty()); + + // Both batches must succeed + for code in codes.iter() { + assert!(matches!(code, NodeResults::Ok)) + } + + let results = node.tx_results.lock().unwrap(); + // We submitted two batches + assert_eq!(results.len(), 2); + + // Check inner tx results of first batch + let res0 = &results[0]; + assert_eq!(res0.len(), 2); + let inner_tx_result = res0 + .get_inner_tx_result( + wrapper_hashes[0].as_ref(), + itertools::Either::Right(&inner_cmts[0]), + ) + .expect("Missing expected tx result") + .as_ref() + .expect("Result is supposed to be Ok"); + assert!(inner_tx_result.is_accepted()); + let inner_tx_result = res0 + .get_inner_tx_result( + wrapper_hashes[0].as_ref(), + itertools::Either::Right(&inner_cmts[1]), + ) + .expect("Missing expected tx result") + .as_ref() + .expect("Result is supposed to be Ok"); + assert!(!inner_tx_result.is_accepted()); + + // Check inner tx results of second batch + let res1 = &results[1]; + assert_eq!(res1.len(), 2); + let inner_tx_result = res1 + .get_inner_tx_result( + wrapper_hashes[1].as_ref(), + itertools::Either::Right(&inner_cmts[2]), + ) + .expect("Missing expected tx result") + .as_ref() + .expect("Result is supposed to be Ok"); + assert!(!inner_tx_result.is_accepted()); + let inner_tx_result = res1 + .get_inner_tx_result( + wrapper_hashes[1].as_ref(), + itertools::Either::Right(&inner_cmts[3]), + ) + .expect("Missing expected tx result") + .as_ref() + .expect("Result is supposed to be Ok"); + assert!(inner_tx_result.is_accepted()); + } + + node.clear_results(); + + // sync the shielded context + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + validator_one_rpc, + ], + )?; + node.assert_success(); + + // Assert NAM balances at VK(A), Albert and Bertha + for (owner, balance) in [ + (AA_VIEWING_KEY, 2000), + (ALBERT_KEY, 1_998_000), + (BERTHA_KEY, 2_000_000), + ] { + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + owner, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("nam: {balance}"))); + } + + Ok(()) +} + +// Test MASP atomic batched txs where one is failing and one is successful and +// check that both the protocol and the shielded sync command behave correctly. +// Verify that since the batch is atomic both transactions are rejected and no +// storage modifications are committed. +#[test] +fn masp_atomic_batch() -> Result<()> { + // This address doesn't matter for tests. But an argument is required. + let validator_one_rpc = "http://127.0.0.1:26567"; + // Download the shielded pool parameters before starting node + let _ = FsShieldedUtils::new(PathBuf::new()); + let (mut node, _services) = setup::setup()?; + _ = node.next_masp_epoch(); + let tempdir = tempfile::tempdir().unwrap(); + + // Generate txs for the batch to shield some tokens. Use two different + // sources + let mut batch = vec![]; + for source in [ALBERT_KEY, BERTHA_KEY] { + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + source, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1000", + "--gas-limit", + "300000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + )?; + node.assert_success(); + let file_path = tempdir + .path() + .read_dir() + .unwrap() + .next() + .unwrap() + .unwrap() + .path(); + batch.push(std::fs::read(&file_path).unwrap()); + std::fs::remove_file(&file_path).unwrap(); + } + + // Create the batch + let tx0: namada_sdk::tx::Tx = serde_json::from_slice(&batch[0]).unwrap(); + let tx1: namada_sdk::tx::Tx = serde_json::from_slice(&batch[1]).unwrap(); + + let signing_data = SigningTxData { + owner: None, + public_keys: vec![albert_keypair().to_public()], + threshold: 1, + account_public_keys_map: None, + fee_payer: albert_keypair().to_public(), + }; + + let mut txs = vec![]; + let mut inner_cmts = vec![]; + let mut wrapper_hashes = vec![]; + + // Try different tx orders and generate a single block with both batch + // combinations + for (tx0, tx1) in [(tx0.clone(), tx1.clone()), (tx1, tx0)] { + let (mut batched_tx, _signing_data) = + namada_sdk::tx::build_batch(vec![ + (tx0, signing_data.clone()), + (tx1, signing_data.clone()), + ]) + .unwrap(); + batched_tx.header.atomic = true; + + // Sign the batch with just the signer of one tx to force the failure of + // the other one + batched_tx.sign_raw( + vec![albert_keypair()], + AccountPublicKeysMap::from_iter( + vec![(albert_keypair().to_public())].into_iter(), + ), + None, + ); + batched_tx.sign_wrapper(christel_keypair()); + + wrapper_hashes.push(batched_tx.wrapper_hash()); + for cmt in batched_tx.commitments() { + inner_cmts.push(cmt.to_owned()); + } + + txs.push(batched_tx.to_bytes()); + } + + node.clear_results(); + node.submit_txs(txs); + + // Check the block result + { + let codes = node.tx_result_codes.lock().unwrap(); + // If empty than failed in process proposal + assert!(!codes.is_empty()); + + // Both batches must fail + for code in codes.iter() { + assert!(matches!( + code, + NodeResults::Failed( + namada_node::shell::ResultCode::WasmRuntimeError + ) + )) + } + + let results = node.tx_results.lock().unwrap(); + // We submitted two batches + assert_eq!(results.len(), 2); + + // Check inner tx results of first batch + let res0 = &results[0]; + assert_eq!(res0.len(), 2); + let inner_tx_result = res0 + .get_inner_tx_result( + wrapper_hashes[0].as_ref(), + itertools::Either::Right(&inner_cmts[0]), + ) + .expect("Missing expected tx result") + .as_ref() + .expect("Result is supposed to be Ok"); + assert!(inner_tx_result.is_accepted()); + let inner_tx_result = res0 + .get_inner_tx_result( + wrapper_hashes[0].as_ref(), + itertools::Either::Right(&inner_cmts[1]), + ) + .expect("Missing expected tx result") + .as_ref() + .expect("Result is supposed to be Ok"); + assert!(!inner_tx_result.is_accepted()); + + // Check inner tx results of second batch, the second result is missing + // since the atomic batch gets short-circuited + let res1 = &results[1]; + assert_eq!(res1.len(), 1); + let inner_tx_result = res1 + .get_inner_tx_result( + wrapper_hashes[1].as_ref(), + itertools::Either::Right(&inner_cmts[2]), + ) + .expect("Missing expected tx result") + .as_ref() + .expect("Result is supposed to be Ok"); + assert!(!inner_tx_result.is_accepted()); + } + + node.clear_results(); + + // sync the shielded context + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + validator_one_rpc, + ], + )?; + node.assert_success(); + + // Assert NAM balances at VK(A), Albert and Bertha are unchanged + for (owner, balance) in [ + (AA_VIEWING_KEY, 0), + (ALBERT_KEY, 2_000_000), + (BERTHA_KEY, 2_000_000), + ] { + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + owner, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("nam: {balance}"))); + } + + Ok(()) +} From 29f423a4574f0f810a3d3cfdafec35f81695f689 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 23 Sep 2024 19:51:44 +0200 Subject: [PATCH 2/9] Adds masp tricky txs test --- crates/tests/src/integration/masp.rs | 330 ++++++++++++++++++++++++++- 1 file changed, 329 insertions(+), 1 deletion(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 681a05ee75..26edfbcbca 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -3832,6 +3832,31 @@ fn masp_batch() -> Result<()> { _ = node.next_masp_epoch(); let tempdir = tempfile::tempdir().unwrap(); + // Assert reference NAM balances at VK(A), Albert and Bertha + for (owner, balance) in [ + (AA_VIEWING_KEY, 0), + (ALBERT_KEY, 2_000_000), + (BERTHA_KEY, 2_000_000), + ] { + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + owner, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("nam: {balance}"))); + } + // Generate txs for the batch to shield some tokens. Use two different // sources let mut batch = vec![]; @@ -4000,7 +4025,7 @@ fn masp_batch() -> Result<()> { // Assert NAM balances at VK(A), Albert and Bertha for (owner, balance) in [ - (AA_VIEWING_KEY, 2000), + (AA_VIEWING_KEY, 2_000), (ALBERT_KEY, 1_998_000), (BERTHA_KEY, 2_000_000), ] { @@ -4040,6 +4065,31 @@ fn masp_atomic_batch() -> Result<()> { _ = node.next_masp_epoch(); let tempdir = tempfile::tempdir().unwrap(); + // Assert reference NAM balances at VK(A), Albert and Bertha are unchanged + for (owner, balance) in [ + (AA_VIEWING_KEY, 0), + (ALBERT_KEY, 2_000_000), + (BERTHA_KEY, 2_000_000), + ] { + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + owner, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("nam: {balance}"))); + } + // Generate txs for the batch to shield some tokens. Use two different // sources let mut batch = vec![]; @@ -4230,3 +4280,281 @@ fn masp_atomic_batch() -> Result<()> { Ok(()) } + +// Test some edge-case masp txs: +// 1. A non masp tx that carries a masp section (check that both the protocol +// and the shielded-sync command ignore this) +// 2. A masp tx that carries two masp sections (check that both the protocol +// and the shielded-sync command only pick the correct data) +#[test] +fn tricky_masp_txs() -> Result<()> { + // This address doesn't matter for tests. But an argument is required. + let validator_one_rpc = "http://127.0.0.1:26567"; + // Download the shielded pool parameters before starting node + let _ = FsShieldedUtils::new(PathBuf::new()); + let (mut node, _services) = setup::setup()?; + _ = node.next_masp_epoch(); + let tempdir = tempfile::tempdir().unwrap(); + + // Assert reference NAM balances at VK(A), Albert, Bertha and Christel + for (owner, balance) in [ + (AA_VIEWING_KEY, 0), + (ALBERT_KEY, 2_000_000), + (BERTHA_KEY, 2_000_000), + (ALBERT, 1_980_000), + (CHRISTEL, 2_000_000), + ] { + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + owner, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("nam: {balance}"))); + } + + // Generate masp tx to extract the section + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1000", + "--gas-limit", + "300000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + )?; + node.assert_success(); + let file_path = tempdir + .path() + .read_dir() + .unwrap() + .next() + .unwrap() + .unwrap() + .path(); + let masp_tx_bytes = std::fs::read(&file_path).unwrap(); + std::fs::remove_file(&file_path).unwrap(); + let masp_tx: namada_sdk::tx::Tx = + serde_json::from_slice(&masp_tx_bytes).unwrap(); + let masp_transaction = masp_tx + .sections + .into_iter() + .find_map(|sec| sec.masp_tx()) + .unwrap(); + + // Generate first tx + run( + &node, + Bin::Client, + vec![ + "transparent-transfer", + "--source", + ALBERT_KEY, + "--target", + CHRISTEL, + "--token", + NAM, + "--amount", + "1000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + )?; + node.assert_success(); + let file_path = tempdir + .path() + .read_dir() + .unwrap() + .next() + .unwrap() + .unwrap() + .path(); + let tx_bytes = std::fs::read(&file_path).unwrap(); + std::fs::remove_file(&file_path).unwrap(); + + // Attach useless masp section to tx + let mut tx0: namada_sdk::tx::Tx = + serde_json::from_slice(&tx_bytes).unwrap(); + tx0.add_masp_tx_section(masp_transaction.clone()); + + tx0.sign_raw( + vec![albert_keypair()], + AccountPublicKeysMap::from_iter( + vec![(albert_keypair().to_public())].into_iter(), + ), + None, + ); + tx0.sign_wrapper(christel_keypair()); + + // Generate second tx + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + BERTHA_KEY, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1000", + "--gas-limit", + "300000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + )?; + node.assert_success(); + let file_path = tempdir + .path() + .read_dir() + .unwrap() + .next() + .unwrap() + .unwrap() + .path(); + let tx_bytes = std::fs::read(&file_path).unwrap(); + std::fs::remove_file(&file_path).unwrap(); + + // Attach another useless masp section to tx + let mut tx1: namada_sdk::tx::Tx = + serde_json::from_slice(&tx_bytes).unwrap(); + tx1.add_masp_tx_section(masp_transaction); + + tx1.sign_raw( + vec![bertha_keypair()], + AccountPublicKeysMap::from_iter( + vec![(bertha_keypair().to_public())].into_iter(), + ), + None, + ); + tx1.sign_wrapper(christel_keypair()); + + let txs = vec![tx0.to_bytes(), tx1.to_bytes()]; + node.clear_results(); + node.submit_txs(txs); + + // Check the block result + { + let codes = node.tx_result_codes.lock().unwrap(); + // If empty than failed in process proposal + assert!(!codes.is_empty()); + + // Both batches must succeed + for code in codes.iter() { + assert!(matches!(code, NodeResults::Ok)) + } + + let results = node.tx_results.lock().unwrap(); + // We submitted two batches + assert_eq!(results.len(), 2); + + // Check inner tx results of first batch + let res0 = &results[0]; + assert_eq!(res0.len(), 1); + let inner_tx_result = res0 + .get_inner_tx_result( + tx0.wrapper_hash().as_ref(), + itertools::Either::Right(tx0.first_commitments().unwrap()), + ) + .expect("Missing expected tx result") + .as_ref() + .expect("Result is supposed to be Ok"); + assert!(inner_tx_result.is_accepted()); + + // Check inner tx results of second batch + let res1 = &results[1]; + assert_eq!(res1.len(), 1); + let inner_tx_result = res1 + .get_inner_tx_result( + tx1.wrapper_hash().as_ref(), + itertools::Either::Right(tx1.first_commitments().unwrap()), + ) + .expect("Missing expected tx result") + .as_ref() + .expect("Result is supposed to be Ok"); + assert!(inner_tx_result.is_accepted()); + } + + node.clear_results(); + + // sync the shielded context + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + validator_one_rpc, + ], + )?; + node.assert_success(); + + // Assert NAM balances at VK(A), Albert, Bertha and Christel + for (owner, balance) in [ + (AA_VIEWING_KEY, 1_000), + (ALBERT_KEY, 1_999_000), + (BERTHA_KEY, 1_999_000), + (ALBERT, 1_980_000), + (CHRISTEL, 2_001_000), + ] { + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + owner, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(&format!("nam: {balance}"))); + } + + Ok(()) +} From 74a53df4b9d2c6abb1076b9bb0904e9ef7c9b260 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 24 Sep 2024 12:07:32 +0200 Subject: [PATCH 3/9] Simplifies test assertion --- crates/tests/src/integration/masp.rs | 45 +--------------------------- 1 file changed, 1 insertion(+), 44 deletions(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 26edfbcbca..f00c12780f 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -4470,50 +4470,7 @@ fn tricky_masp_txs() -> Result<()> { let txs = vec![tx0.to_bytes(), tx1.to_bytes()]; node.clear_results(); node.submit_txs(txs); - - // Check the block result - { - let codes = node.tx_result_codes.lock().unwrap(); - // If empty than failed in process proposal - assert!(!codes.is_empty()); - - // Both batches must succeed - for code in codes.iter() { - assert!(matches!(code, NodeResults::Ok)) - } - - let results = node.tx_results.lock().unwrap(); - // We submitted two batches - assert_eq!(results.len(), 2); - - // Check inner tx results of first batch - let res0 = &results[0]; - assert_eq!(res0.len(), 1); - let inner_tx_result = res0 - .get_inner_tx_result( - tx0.wrapper_hash().as_ref(), - itertools::Either::Right(tx0.first_commitments().unwrap()), - ) - .expect("Missing expected tx result") - .as_ref() - .expect("Result is supposed to be Ok"); - assert!(inner_tx_result.is_accepted()); - - // Check inner tx results of second batch - let res1 = &results[1]; - assert_eq!(res1.len(), 1); - let inner_tx_result = res1 - .get_inner_tx_result( - tx1.wrapper_hash().as_ref(), - itertools::Either::Right(tx1.first_commitments().unwrap()), - ) - .expect("Missing expected tx result") - .as_ref() - .expect("Result is supposed to be Ok"); - assert!(inner_tx_result.is_accepted()); - } - - node.clear_results(); + node.assert_success(); // sync the shielded context run( From e01682b15999d531d5fab7c35db09cf6cdc9e769 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 24 Sep 2024 12:14:14 +0200 Subject: [PATCH 4/9] Removes useless calls to `assert_success` in integration tests --- crates/tests/src/integration/masp.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index f00c12780f..6ab7f3dae9 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -4021,7 +4021,6 @@ fn masp_batch() -> Result<()> { validator_one_rpc, ], )?; - node.assert_success(); // Assert NAM balances at VK(A), Albert and Bertha for (owner, balance) in [ @@ -4251,7 +4250,6 @@ fn masp_atomic_batch() -> Result<()> { validator_one_rpc, ], )?; - node.assert_success(); // Assert NAM balances at VK(A), Albert and Bertha are unchanged for (owner, balance) in [ @@ -4484,7 +4482,6 @@ fn tricky_masp_txs() -> Result<()> { validator_one_rpc, ], )?; - node.assert_success(); // Assert NAM balances at VK(A), Albert, Bertha and Christel for (owner, balance) in [ From f2afcac98379bf773fff83945be4877e6577586f Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 24 Sep 2024 14:32:23 +0200 Subject: [PATCH 5/9] More masp unit tests --- crates/shielded_token/src/vp.rs | 166 +++++++++++++++++++++++++++++++- 1 file changed, 165 insertions(+), 1 deletion(-) diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index ef8cdc2781..01c5106576 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -885,8 +885,9 @@ mod shielded_token_tests { use std::collections::BTreeSet; use namada_core::address::testing::nam; - use namada_core::address::MASP; + use namada_core::address::{IBC, MASP}; use namada_core::borsh::BorshSerializeExt; + use namada_core::masp::TokenMap; use namada_gas::{TxGasMeter, VpGasMeter}; use namada_state::testing::TestState; use namada_state::{StateRead, TxIndex}; @@ -898,6 +899,9 @@ mod shielded_token_tests { use namada_vm::wasm::VpCache; use namada_vm::WasmCacheRwAccess; use namada_vp::native_vp::{self, CtxPostStorageRead, CtxPreStorageRead}; + use namada_vp_env::Error; + + use crate::storage_key::masp_token_map_key; type CA = WasmCacheRwAccess; type Eval = VpEvalWasm<::D, ::H, CA>; @@ -967,6 +971,7 @@ mod shielded_token_tests { vp_vp_cache, ); + // We don't care about the specific error so long as it fails assert!( MaspVp::validate_tx( &ctx, @@ -978,4 +983,163 @@ mod shielded_token_tests { ); } } + + // FIXME: this should be prop test + // Changing no MASP keys at all is allowed + #[test] + fn test_no_masp_op_accepted() { + let mut state = TestState::default(); + namada_parameters::init_test_storage(&mut state).unwrap(); + let src_key = balance_key(&nam(), &IBC); + let keys_changed = BTreeSet::from([src_key.clone()]); + let verifiers = Default::default(); + + let tx_index = TxIndex::default(); + let mut tx = Tx::from_type(namada_tx::data::TxType::Raw); + tx.push_default_inner_tx(); + let BatchedTx { tx, cmt } = tx.batch_first_tx(); + + // Update the balance key + let amount = Amount::native_whole(100); + let _ = state + .write_log_mut() + .write(&src_key, amount.serialize_to_vec()) + .unwrap(); + + let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( + &TxGasMeter::new(u64::MAX), + )); + let (vp_vp_cache, _vp_cache_dir) = vp_cache(); + let ctx = Ctx::new( + &MASP, + &state, + &tx, + &cmt, + &tx_index, + &gas_meter, + &keys_changed, + &verifiers, + vp_vp_cache, + ); + + assert!( + MaspVp::validate_tx( + &ctx, + &tx.batch_ref_tx(&cmt), + &keys_changed, + &verifiers + ) + .is_ok() + ); + } + + // FIXME: proptest + // Changing keys for both a transfer and a governance proposal is not + // allowed + #[test] + fn test_mixed_keys_rejected() { + let mut state = TestState::default(); + namada_parameters::init_test_storage(&mut state).unwrap(); + let balance_key = balance_key(&nam(), &MASP); + let token_map_key = masp_token_map_key(); + let keys_changed = + BTreeSet::from([balance_key.clone(), token_map_key.clone()]); + let verifiers = Default::default(); + + let tx_index = TxIndex::default(); + let mut tx = Tx::from_type(namada_tx::data::TxType::Raw); + tx.push_default_inner_tx(); + let BatchedTx { tx, cmt } = tx.batch_first_tx(); + + // Write the conflicting keys + let amount = Amount::native_whole(100); + let _ = state + .write_log_mut() + .write(&balance_key, amount.serialize_to_vec()) + .unwrap(); + let token_map = TokenMap::new(); + let _ = state + .write_log_mut() + .write(&token_map_key, token_map.serialize_to_vec()) + .unwrap(); + + let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( + &TxGasMeter::new(u64::MAX), + )); + let (vp_vp_cache, _vp_cache_dir) = vp_cache(); + let ctx = Ctx::new( + &MASP, + &state, + &tx, + &cmt, + &tx_index, + &gas_meter, + &keys_changed, + &verifiers, + vp_vp_cache, + ); + + assert!(matches!( + MaspVp::validate_tx( + &ctx, + &tx.batch_ref_tx(&cmt), + &keys_changed, + &verifiers + ), + Err(Error::SimpleMessage( + "Cannot simultaneously do governance proposal and MASP \ + transfer" + )) + )); + } + + // FIXME: proptest + // Changing unknown masp keys must be rejected + #[test] + fn test_unallowed_masp_keys_rejected() { + let mut state = TestState::default(); + namada_parameters::init_test_storage(&mut state).unwrap(); + let verifiers = Default::default(); + + let tx_index = TxIndex::default(); + let mut tx = Tx::from_type(namada_tx::data::TxType::Raw); + tx.push_default_inner_tx(); + let BatchedTx { tx, cmt } = tx.batch_first_tx(); + + // Write the random masp key + let random_masp_key = format!("#{MASP}/random_key").parse().unwrap(); + let _ = state + .write_log_mut() + .write(&random_masp_key, "random_value".serialize_to_vec()) + .unwrap(); + let keys_changed = BTreeSet::from([random_masp_key.clone()]); + + let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( + &TxGasMeter::new(u64::MAX), + )); + let (vp_vp_cache, _vp_cache_dir) = vp_cache(); + let ctx = Ctx::new( + &MASP, + &state, + &tx, + &cmt, + &tx_index, + &gas_meter, + &keys_changed, + &verifiers, + vp_vp_cache, + ); + + assert!(matches!( + MaspVp::validate_tx( + &ctx, + &tx.batch_ref_tx(&cmt), + &keys_changed, + &verifiers + ), + Err(Error::SimpleMessage( + "Found modifications to non-allowed masp keys" + )) + )); + } } From 5b87eafb32a3dfd67c0c76ff9e42fd378975037e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 24 Sep 2024 15:28:32 +0200 Subject: [PATCH 6/9] Proptests masp --- crates/shielded_token/src/vp.rs | 191 ++++++++++++++++---------------- 1 file changed, 95 insertions(+), 96 deletions(-) diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 01c5106576..4f3d68a402 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -885,11 +885,11 @@ mod shielded_token_tests { use std::collections::BTreeSet; use namada_core::address::testing::nam; - use namada_core::address::{IBC, MASP}; + use namada_core::address::MASP; use namada_core::borsh::BorshSerializeExt; use namada_core::masp::TokenMap; use namada_gas::{TxGasMeter, VpGasMeter}; - use namada_state::testing::TestState; + use namada_state::testing::{arb_account_storage_key, arb_key, TestState}; use namada_state::{StateRead, TxIndex}; use namada_trans_token::storage_key::balance_key; use namada_trans_token::Amount; @@ -900,8 +900,13 @@ mod shielded_token_tests { use namada_vm::WasmCacheRwAccess; use namada_vp::native_vp::{self, CtxPostStorageRead, CtxPreStorageRead}; use namada_vp_env::Error; + use proptest::proptest; + use proptest::strategy::Strategy; - use crate::storage_key::masp_token_map_key; + use crate::storage_key::{ + is_masp_key, is_masp_token_map_key, is_masp_transfer_key, + masp_token_map_key, + }; type CA = WasmCacheRwAccess; type Eval = VpEvalWasm<::D, ::H, CA>; @@ -924,7 +929,7 @@ mod shielded_token_tests { (), >; - // Changing only the balance key of the MASP alone is invalid + // Changing only the balance key of the MASP is invalid #[test] fn test_balance_change() { let mut state = TestState::default(); @@ -984,56 +989,6 @@ mod shielded_token_tests { } } - // FIXME: this should be prop test - // Changing no MASP keys at all is allowed - #[test] - fn test_no_masp_op_accepted() { - let mut state = TestState::default(); - namada_parameters::init_test_storage(&mut state).unwrap(); - let src_key = balance_key(&nam(), &IBC); - let keys_changed = BTreeSet::from([src_key.clone()]); - let verifiers = Default::default(); - - let tx_index = TxIndex::default(); - let mut tx = Tx::from_type(namada_tx::data::TxType::Raw); - tx.push_default_inner_tx(); - let BatchedTx { tx, cmt } = tx.batch_first_tx(); - - // Update the balance key - let amount = Amount::native_whole(100); - let _ = state - .write_log_mut() - .write(&src_key, amount.serialize_to_vec()) - .unwrap(); - - let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( - &TxGasMeter::new(u64::MAX), - )); - let (vp_vp_cache, _vp_cache_dir) = vp_cache(); - let ctx = Ctx::new( - &MASP, - &state, - &tx, - &cmt, - &tx_index, - &gas_meter, - &keys_changed, - &verifiers, - vp_vp_cache, - ); - - assert!( - MaspVp::validate_tx( - &ctx, - &tx.batch_ref_tx(&cmt), - &keys_changed, - &verifiers - ) - .is_ok() - ); - } - - // FIXME: proptest // Changing keys for both a transfer and a governance proposal is not // allowed #[test] @@ -1093,53 +1048,97 @@ mod shielded_token_tests { )); } - // FIXME: proptest - // Changing unknown masp keys must be rejected - #[test] - fn test_unallowed_masp_keys_rejected() { - let mut state = TestState::default(); - namada_parameters::init_test_storage(&mut state).unwrap(); - let verifiers = Default::default(); - - let tx_index = TxIndex::default(); - let mut tx = Tx::from_type(namada_tx::data::TxType::Raw); - tx.push_default_inner_tx(); - let BatchedTx { tx, cmt } = tx.batch_first_tx(); - - // Write the random masp key - let random_masp_key = format!("#{MASP}/random_key").parse().unwrap(); - let _ = state - .write_log_mut() - .write(&random_masp_key, "random_value".serialize_to_vec()) - .unwrap(); - let keys_changed = BTreeSet::from([random_masp_key.clone()]); + proptest! { + // Changing no MASP keys at all is allowed + #[test] + fn test_no_masp_op_accepted(src_key in arb_key().prop_filter("MASP key", |key| !is_masp_key(key))) { + let mut state = TestState::default(); + namada_parameters::init_test_storage(&mut state).unwrap(); + let keys_changed = BTreeSet::from([src_key.clone()]); + let verifiers = Default::default(); + + let tx_index = TxIndex::default(); + let mut tx = Tx::from_type(namada_tx::data::TxType::Raw); + tx.push_default_inner_tx(); + let BatchedTx { tx, cmt } = tx.batch_first_tx(); + + // Write some random value + let _ = state + .write_log_mut() + .write(&src_key, "test".serialize_to_vec()) + .unwrap(); - let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( - &TxGasMeter::new(u64::MAX), - )); - let (vp_vp_cache, _vp_cache_dir) = vp_cache(); - let ctx = Ctx::new( - &MASP, - &state, - &tx, - &cmt, - &tx_index, - &gas_meter, - &keys_changed, - &verifiers, - vp_vp_cache, - ); + let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( + &TxGasMeter::new(u64::MAX), + )); + let (vp_vp_cache, _vp_cache_dir) = vp_cache(); + let ctx = Ctx::new( + &MASP, + &state, + &tx, + &cmt, + &tx_index, + &gas_meter, + &keys_changed, + &verifiers, + vp_vp_cache, + ); - assert!(matches!( - MaspVp::validate_tx( + assert!(MaspVp::validate_tx( &ctx, &tx.batch_ref_tx(&cmt), &keys_changed, &verifiers - ), - Err(Error::SimpleMessage( - "Found modifications to non-allowed masp keys" - )) - )); + ) + .is_ok()); + } + + // Changing unknown masp keys is not allowed + #[test] + fn test_unallowed_masp_keys_rejected(random_masp_key in arb_account_storage_key(MASP).prop_filter("MASP valid key", |key| !(is_masp_transfer_key(key) || is_masp_token_map_key(key) ))) { + let mut state = TestState::default(); + namada_parameters::init_test_storage(&mut state).unwrap(); + let verifiers = Default::default(); + + let tx_index = TxIndex::default(); + let mut tx = Tx::from_type(namada_tx::data::TxType::Raw); + tx.push_default_inner_tx(); + let BatchedTx { tx, cmt } = tx.batch_first_tx(); + + // Write the random masp key + let _ = state + .write_log_mut() + .write(&random_masp_key, "random_value".serialize_to_vec()) + .unwrap(); + let keys_changed = BTreeSet::from([random_masp_key.clone()]); + + let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( + &TxGasMeter::new(u64::MAX), + )); + let (vp_vp_cache, _vp_cache_dir) = vp_cache(); + let ctx = Ctx::new( + &MASP, + &state, + &tx, + &cmt, + &tx_index, + &gas_meter, + &keys_changed, + &verifiers, + vp_vp_cache, + ); + + assert!(matches!( + MaspVp::validate_tx( + &ctx, + &tx.batch_ref_tx(&cmt), + &keys_changed, + &verifiers + ), + Err(Error::SimpleMessage( + "Found modifications to non-allowed masp keys" + )) + )); + } } } From e05eb0b91b9b3132f04946ed9a233794eaf06285 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 27 Sep 2024 17:18:04 +0200 Subject: [PATCH 7/9] Fixes typo + fmt --- crates/shielded_token/src/vp.rs | 7 ++++++- crates/tests/src/integration/masp.rs | 10 +++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 4f3d68a402..43dfaa1328 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -1095,7 +1095,12 @@ mod shielded_token_tests { // Changing unknown masp keys is not allowed #[test] - fn test_unallowed_masp_keys_rejected(random_masp_key in arb_account_storage_key(MASP).prop_filter("MASP valid key", |key| !(is_masp_transfer_key(key) || is_masp_token_map_key(key) ))) { + fn test_unallowed_masp_keys_rejected( + random_masp_key in arb_account_storage_key(MASP).prop_filter( + "MASP valid key", + |key| !(is_masp_transfer_key(key) || is_masp_token_map_key(key) + )) + ) { let mut state = TestState::default(); namada_parameters::init_test_storage(&mut state).unwrap(); let verifiers = Default::default(); diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 6ab7f3dae9..4130370053 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -1543,7 +1543,7 @@ fn multiple_unfetched_txs_same_block() -> Result<()> { node.clear_results(); node.submit_txs(txs); - // If empty than failed in process proposal + // If empty then failed in process proposal assert!(!node.tx_result_codes.lock().unwrap().is_empty()); node.assert_success(); @@ -1691,7 +1691,7 @@ fn expired_masp_tx() -> Result<()> { node.submit_txs(vec![tx.to_bytes()]); { let codes = node.tx_result_codes.lock().unwrap(); - // If empty than failed in process proposal + // If empty then failed in process proposal assert!(!codes.is_empty()); for code in codes.iter() { @@ -3654,7 +3654,7 @@ fn identical_output_descriptions() -> Result<()> { // Check that the batch was successful { let codes = node.tx_result_codes.lock().unwrap(); - // If empty than failed in process proposal + // If empty then failed in process proposal assert!(!codes.is_empty()); for code in codes.iter() { @@ -3950,7 +3950,7 @@ fn masp_batch() -> Result<()> { // Check the block result { let codes = node.tx_result_codes.lock().unwrap(); - // If empty than failed in process proposal + // If empty then failed in process proposal assert!(!codes.is_empty()); // Both batches must succeed @@ -4182,7 +4182,7 @@ fn masp_atomic_batch() -> Result<()> { // Check the block result { let codes = node.tx_result_codes.lock().unwrap(); - // If empty than failed in process proposal + // If empty then failed in process proposal assert!(!codes.is_empty()); // Both batches must fail From 550c623a4a0dce746e1c23d0d9709add78fc2884 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 26 Sep 2024 16:33:28 +0200 Subject: [PATCH 8/9] Changelog #3840 --- .changelog/unreleased/testing/3840-masp-tests.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/testing/3840-masp-tests.md diff --git a/.changelog/unreleased/testing/3840-masp-tests.md b/.changelog/unreleased/testing/3840-masp-tests.md new file mode 100644 index 0000000000..15071ec44d --- /dev/null +++ b/.changelog/unreleased/testing/3840-masp-tests.md @@ -0,0 +1,2 @@ +- Added more unit and integration tests for the MASP, including tests with + transaction batches. ([\#3840](https://github.com/anoma/namada/pull/3840)) \ No newline at end of file From 16f3bb95cc5bf2351581d5ee18d6136f2d416f1d Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 14 Oct 2024 13:18:02 +0200 Subject: [PATCH 9/9] Fixes broken assertions in new integration tests --- crates/tests/src/integration/masp.rs | 257 ++++++++++++++------------- 1 file changed, 134 insertions(+), 123 deletions(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 4130370053..14a35a1bab 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -3861,31 +3861,34 @@ fn masp_batch() -> Result<()> { // sources let mut batch = vec![]; for source in [ALBERT_KEY, BERTHA_KEY] { - run( - &node, - Bin::Client, - vec![ - "shield", - "--source", - source, - "--target", - AA_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "1000", - "--gas-limit", - "300000", - "--gas-payer", - CHRISTEL_KEY, - "--output-folder-path", - tempdir.path().to_str().unwrap(), - "--dump-tx", - "--ledger-address", - validator_one_rpc, - ], - )?; - node.assert_success(); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + source, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1000", + "--gas-limit", + "300000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + let file_path = tempdir .path() .read_dir() @@ -4093,31 +4096,33 @@ fn masp_atomic_batch() -> Result<()> { // sources let mut batch = vec![]; for source in [ALBERT_KEY, BERTHA_KEY] { - run( - &node, - Bin::Client, - vec![ - "shield", - "--source", - source, - "--target", - AA_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "1000", - "--gas-limit", - "300000", - "--gas-payer", - CHRISTEL_KEY, - "--output-folder-path", - tempdir.path().to_str().unwrap(), - "--dump-tx", - "--ledger-address", - validator_one_rpc, - ], - )?; - node.assert_success(); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + source, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1000", + "--gas-limit", + "300000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); let file_path = tempdir .path() .read_dir() @@ -4322,31 +4327,33 @@ fn tricky_masp_txs() -> Result<()> { } // Generate masp tx to extract the section - run( - &node, - Bin::Client, - vec![ - "shield", - "--source", - ALBERT, - "--target", - AA_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "1000", - "--gas-limit", - "300000", - "--gas-payer", - CHRISTEL_KEY, - "--output-folder-path", - tempdir.path().to_str().unwrap(), - "--dump-tx", - "--ledger-address", - validator_one_rpc, - ], - )?; - node.assert_success(); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1000", + "--gas-limit", + "300000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); let file_path = tempdir .path() .read_dir() @@ -4366,29 +4373,31 @@ fn tricky_masp_txs() -> Result<()> { .unwrap(); // Generate first tx - run( - &node, - Bin::Client, - vec![ - "transparent-transfer", - "--source", - ALBERT_KEY, - "--target", - CHRISTEL, - "--token", - NAM, - "--amount", - "1000", - "--gas-payer", - CHRISTEL_KEY, - "--output-folder-path", - tempdir.path().to_str().unwrap(), - "--dump-tx", - "--ledger-address", - validator_one_rpc, - ], - )?; - node.assert_success(); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "transparent-transfer", + "--source", + ALBERT_KEY, + "--target", + CHRISTEL, + "--token", + NAM, + "--amount", + "1000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); let file_path = tempdir .path() .read_dir() @@ -4415,31 +4424,33 @@ fn tricky_masp_txs() -> Result<()> { tx0.sign_wrapper(christel_keypair()); // Generate second tx - run( - &node, - Bin::Client, - vec![ - "shield", - "--source", - BERTHA_KEY, - "--target", - AA_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "1000", - "--gas-limit", - "300000", - "--gas-payer", - CHRISTEL_KEY, - "--output-folder-path", - tempdir.path().to_str().unwrap(), - "--dump-tx", - "--ledger-address", - validator_one_rpc, - ], - )?; - node.assert_success(); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + BERTHA_KEY, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "1000", + "--gas-limit", + "300000", + "--gas-payer", + CHRISTEL_KEY, + "--output-folder-path", + tempdir.path().to_str().unwrap(), + "--dump-tx", + "--ledger-address", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); let file_path = tempdir .path() .read_dir()