From 97728552ebd63aa356757117e5562f13186f2bce Mon Sep 17 00:00:00 2001 From: Erwan Or Date: Thu, 2 May 2024 10:53:57 -0400 Subject: [PATCH] planner: create memo from change address --- crates/bin/pcli/tests/network_integration.rs | 234 ++++++++++--------- crates/view/src/planner.rs | 26 +-- 2 files changed, 132 insertions(+), 128 deletions(-) diff --git a/crates/bin/pcli/tests/network_integration.rs b/crates/bin/pcli/tests/network_integration.rs index 6c0c1c30bf..fdec4ffe9e 100644 --- a/crates/bin/pcli/tests/network_integration.rs +++ b/crates/bin/pcli/tests/network_integration.rs @@ -112,6 +112,7 @@ fn get_validator(tmpdir: &TempDir) -> String { #[ignore] #[test] fn transaction_send_from_addr_0_to_addr_1() { + tracing_subscriber::fmt::try_init().ok(); let tmpdir = load_wallet_into_tmpdir(); // Create a memo that we can inspect later, to confirm transaction @@ -186,6 +187,8 @@ fn transaction_send_from_addr_0_to_addr_1() { .expect("can find MemoView in TransactionView"); match mv { penumbra_transaction::MemoView::Visible { plaintext, .. } => { + tracing::info!(?plaintext, "plaintext memo"); + tracing::info!(?memo_text, "expected memo text"); assert!(plaintext.text == memo_text); } penumbra_transaction::MemoView::Opaque { .. } => { @@ -328,6 +331,7 @@ fn delegate_and_undelegate() { if undelegation_result.is_ok() { break; } else { + tracing::error!(?undelegation_result, "undelegation failed"); num_attempts += 1; tracing::info!(num_attempts, max_attempts, "undelegation failed"); if num_attempts >= max_attempts { @@ -1238,118 +1242,118 @@ fn test_orders() { withdraw_cmd.assert().success(); } -#[ignore] -#[test] -fn delegate_submit_proposal_and_vote() { - let tmpdir = load_wallet_into_tmpdir(); - - // Get a validator from the testnet. - let validator = get_validator(&tmpdir); - - // Now undelegate. We attempt `max_attempts` times in case an epoch boundary passes - // while we prepare the delegation. See issues #1522, #2047. - let max_attempts = 5; - - let mut num_attempts = 0; - loop { - // Delegate a tiny bit of penumbra to the validator. - let mut delegate_cmd = Command::cargo_bin("pcli").unwrap(); - delegate_cmd - .args([ - "--home", - tmpdir.path().to_str().unwrap(), - "tx", - "delegate", - "1penumbra", - "--to", - validator.as_str(), - ]) - .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)); - let delegation_result = delegate_cmd.assert().try_success(); - - // If the undelegation command succeeded, we can exit this loop. - if delegation_result.is_ok() { - break; - } else { - num_attempts += 1; - if num_attempts >= max_attempts { - panic!("Exceeded max attempts for fallible command"); - } - } - } - - // Check we have some of the delegation token for that validator now. - let mut balance_cmd = Command::cargo_bin("pcli").unwrap(); - balance_cmd - .args(["--home", tmpdir.path().to_str().unwrap(), "view", "balance"]) - .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)); - balance_cmd - .assert() - .stdout(predicate::str::is_match(validator.as_str()).unwrap()); - - let mut proposal_template_cmd = Command::cargo_bin("pcli").unwrap(); - let template = proposal_template_cmd - .args([ - "--home", - tmpdir.path().to_str().unwrap(), - "tx", - "proposal", - "template", - "signaling", - ]) - .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)) - .assert() - .success() - .get_output() - .stdout - .clone(); - let template_str = String::from_utf8(template).unwrap(); - let template_file = load_string_to_file(template_str, &tmpdir); - let template_path = template_file.path().to_str().unwrap(); - - let mut submit_proposal_cmd = Command::cargo_bin("pcli").unwrap(); - submit_proposal_cmd - .args([ - "--home", - tmpdir.path().to_str().unwrap(), - "tx", - "proposal", - "submit", - "--file", - template_path, - "--deposit-amount", - "10000000", - ]) - .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)); - submit_proposal_cmd.assert().success(); - sync(&tmpdir); - - let mut proposals_cmd = Command::cargo_bin("pcli").unwrap(); - proposals_cmd - .args([ - "--home", - tmpdir.path().to_str().unwrap(), - "query", - "governance", - "list-proposals", - ]) - .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)) - .assert() - .success() - .stdout(predicate::str::is_match("A short title").unwrap()); - - let mut vote_cmd = Command::cargo_bin("pcli").unwrap(); - vote_cmd - .args([ - "--home", - tmpdir.path().to_str().unwrap(), - "tx", - "vote", - "yes", - "--on", - "0", - ]) - .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)) - .assert() - .success(); -} +// #[ignore] +// #[test] +// fn delegate_submit_proposal_and_vote() { +// let tmpdir = load_wallet_into_tmpdir(); +// +// // Get a validator from the testnet. +// let validator = get_validator(&tmpdir); +// +// // Now undelegate. We attempt `max_attempts` times in case an epoch boundary passes +// // while we prepare the delegation. See issues #1522, #2047. +// let max_attempts = 5; +// +// let mut num_attempts = 0; +// loop { +// // Delegate a tiny bit of penumbra to the validator. +// let mut delegate_cmd = Command::cargo_bin("pcli").unwrap(); +// delegate_cmd +// .args([ +// "--home", +// tmpdir.path().to_str().unwrap(), +// "tx", +// "delegate", +// "1penumbra", +// "--to", +// validator.as_str(), +// ]) +// .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)); +// let delegation_result = delegate_cmd.assert().try_success(); +// +// // If the undelegation command succeeded, we can exit this loop. +// if delegation_result.is_ok() { +// break; +// } else { +// num_attempts += 1; +// if num_attempts >= max_attempts { +// panic!("Exceeded max attempts for fallible command"); +// } +// } +// } +// +// // Check we have some of the delegation token for that validator now. +// let mut balance_cmd = Command::cargo_bin("pcli").unwrap(); +// balance_cmd +// .args(["--home", tmpdir.path().to_str().unwrap(), "view", "balance"]) +// .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)); +// balance_cmd +// .assert() +// .stdout(predicate::str::is_match(validator.as_str()).unwrap()); +// +// let mut proposal_template_cmd = Command::cargo_bin("pcli").unwrap(); +// let template = proposal_template_cmd +// .args([ +// "--home", +// tmpdir.path().to_str().unwrap(), +// "tx", +// "proposal", +// "template", +// "signaling", +// ]) +// .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)) +// .assert() +// .success() +// .get_output() +// .stdout +// .clone(); +// let template_str = String::from_utf8(template).unwrap(); +// let template_file = load_string_to_file(template_str, &tmpdir); +// let template_path = template_file.path().to_str().unwrap(); +// +// let mut submit_proposal_cmd = Command::cargo_bin("pcli").unwrap(); +// submit_proposal_cmd +// .args([ +// "--home", +// tmpdir.path().to_str().unwrap(), +// "tx", +// "proposal", +// "submit", +// "--file", +// template_path, +// "--deposit-amount", +// "10000000", +// ]) +// .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)); +// submit_proposal_cmd.assert().success(); +// sync(&tmpdir); +// +// let mut proposals_cmd = Command::cargo_bin("pcli").unwrap(); +// proposals_cmd +// .args([ +// "--home", +// tmpdir.path().to_str().unwrap(), +// "query", +// "governance", +// "list-proposals", +// ]) +// .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)) +// .assert() +// .success() +// .stdout(predicate::str::is_match("A short title").unwrap()); +// +// let mut vote_cmd = Command::cargo_bin("pcli").unwrap(); +// vote_cmd +// .args([ +// "--home", +// tmpdir.path().to_str().unwrap(), +// "tx", +// "vote", +// "yes", +// "--on", +// "0", +// ]) +// .timeout(std::time::Duration::from_secs(TIMEOUT_COMMAND_SECONDS)) +// .assert() +// .success(); +// } diff --git a/crates/view/src/planner.rs b/crates/view/src/planner.rs index d4e60cb96e..7c77099d0f 100644 --- a/crates/view/src/planner.rs +++ b/crates/view/src/planner.rs @@ -788,7 +788,7 @@ impl Planner { let expiry_height = self.plan.transaction_parameters.expiry_height; - let plan = TransactionPlan { + let mut plan = TransactionPlan { actions: self .actions .clone() @@ -804,8 +804,6 @@ impl Planner { memo: None, }; - self.plan = plan; - // All actions have now been added, so check to make sure that you don't build and submit an // empty transaction if self.plan.actions.is_empty() { @@ -820,18 +818,20 @@ impl Planner { ); } - // If there are outputs, we check that a memo has been added. If not, we add a blank memo. - if self.plan.num_outputs() > 0 && self.plan.memo.is_none() { - self.memo(MemoPlaintext::blank_memo(change_address.clone())) - .expect("empty string is a valid memo"); - } else if self.plan.num_outputs() == 0 && self.plan.memo.is_some() { - anyhow::bail!("if no outputs, no memo should be added"); + if let Some(memo_plan) = self.plan.memo.clone() { + plan.memo = Some(MemoPlan::new(&mut OsRng, memo_plan.plaintext)?); + } else if plan.output_plans().next().is_some() { + // If a memo was not provided, but is required (because we have outputs), + // auto-create one with the change address. + plan.memo = Some(MemoPlan::new( + &mut OsRng, + MemoPlaintext::new(change_address, String::new())?, + )?); } - // Add clue plans for `Output`s. - let precision_bits = fmd_params.precision_bits; - self.plan - .populate_detection_data(&mut self.rng, precision_bits.into()); + plan.populate_detection_data(&mut OsRng, fmd_params.precision_bits.into()); + + self.plan = plan; tracing::debug!(plan = ?self.plan, "finished balancing transaction");