From 15477e00a056b351250d039d0bbc8eb76f1cce89 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 17 Jun 2024 11:00:51 +0100 Subject: [PATCH] fixup! Derive inner tx hash using wrapper hash --- Cargo.lock | 2 ++ crates/apps_lib/Cargo.toml | 1 + crates/apps_lib/src/client/tx.rs | 34 +++++++++++++++++++++++++++----- crates/sdk/Cargo.toml | 1 + crates/sdk/src/tx.rs | 24 +++++++++++++++------- wasm/Cargo.lock | 1 + wasm_for_tests/Cargo.lock | 1 + 7 files changed, 52 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 87ca9dad769..426ce0b488c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4785,6 +4785,7 @@ dependencies = [ "data-encoding", "derivative", "directories", + "either", "eyre", "fd-lock", "flate2", @@ -5241,6 +5242,7 @@ dependencies = [ "data-encoding", "derivation-path", "duration-str", + "either", "ethbridge-bridge-contract", "ethers", "eyre", diff --git a/crates/apps_lib/Cargo.toml b/crates/apps_lib/Cargo.toml index c769cff7fe0..2eb8e919e89 100644 --- a/crates/apps_lib/Cargo.toml +++ b/crates/apps_lib/Cargo.toml @@ -49,6 +49,7 @@ config.workspace = true data-encoding.workspace = true derivative.workspace = true directories.workspace = true +either.workspace = true eyre.workspace = true fd-lock.workspace = true flate2.workspace = true diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 80e6e52e092..537d0ec63bf 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -13,6 +13,7 @@ use namada::governance::cli::onchain::{ }; use namada::io::Io; use namada::state::EPOCH_SWITCH_BLOCKS_DELAY; +use namada::tx::data::compute_inner_tx_hash; use namada::tx::{CompressedAuthorization, Section, Signer, Tx}; use namada_sdk::args::TxBecomeValidator; use namada_sdk::rpc::{InnerTxResult, TxBroadcastData, TxResponse}; @@ -298,8 +299,11 @@ where sign(namada, &mut tx, &args.tx, signing_data).await?; let cmt = tx.first_commitments().unwrap().to_owned(); + let wrapper_hash = tx.wrapper_hash(); let response = namada.submit(tx, &args.tx).await?; - if let Some(result) = response.is_applied_and_valid(&cmt) { + if let Some(result) = + response.is_applied_and_valid(wrapper_hash.as_ref(), &cmt) + { return Ok(result.initialized_accounts.first().cloned()); } } @@ -377,10 +381,14 @@ pub async fn submit_change_consensus_key( } else { sign(namada, &mut tx, &args.tx, signing_data).await?; let cmt = tx.first_commitments().unwrap().to_owned(); + let wrapper_hash = tx.wrapper_hash(); let resp = namada.submit(tx, &args.tx).await?; if !args.tx.dry_run { - if resp.is_applied_and_valid(&cmt).is_some() { + if resp + .is_applied_and_valid(wrapper_hash.as_ref(), &cmt) + .is_some() + { namada.wallet_mut().await.save().unwrap_or_else(|err| { edisplay_line!(namada.io(), "{}", err) }); @@ -571,6 +579,7 @@ pub async fn submit_become_validator( } else { sign(namada, &mut tx, &args.tx, signing_data).await?; let cmt = tx.first_commitments().unwrap().to_owned(); + let wrapper_hash = tx.wrapper_hash(); let resp = namada.submit(tx, &args.tx).await?; if args.tx.dry_run { @@ -581,7 +590,10 @@ pub async fn submit_become_validator( safe_exit(0) } - if resp.is_applied_and_valid(&cmt).is_none() { + if resp + .is_applied_and_valid(wrapper_hash.as_ref(), &cmt) + .is_none() + { display_line!( namada.io(), "Transaction failed. No key or addresses have been saved." @@ -787,11 +799,18 @@ pub async fn submit_shielding_transfer( } else { sign(namada, &mut tx, &args.tx, signing_data).await?; let cmt_hash = tx.first_commitments().unwrap().get_hash(); + let wrapper_hash = tx.wrapper_hash(); let result = namada.submit(tx, &args.tx).await?; match result { ProcessTxResponse::Applied(resp) if // If a transaction is rejected by a VP - matches!(resp.batch_result().get(&cmt_hash), Some(InnerTxResult::VpsRejected(_))) => + matches!( + resp.batch_result().get(&compute_inner_tx_hash( + wrapper_hash.as_ref(), + either::Left(&cmt_hash) + )), + Some(InnerTxResult::VpsRejected(_)) + ) => { let submission_masp_epoch = rpc::query_and_print_masp_epoch(namada).await; // And its submission epoch doesn't match construction epoch @@ -1107,9 +1126,14 @@ where } else { sign(namada, &mut tx, &args.tx, signing_data).await?; let cmt = tx.first_commitments().unwrap().to_owned(); + let wrapper_hash = tx.wrapper_hash(); let resp = namada.submit(tx, &args.tx).await?; - if !args.tx.dry_run && resp.is_applied_and_valid(&cmt).is_some() { + if !args.tx.dry_run + && resp + .is_applied_and_valid(wrapper_hash.as_ref(), &cmt) + .is_some() + { tx::query_unbonds(namada, args.clone(), latest_withdrawal_pre) .await?; } diff --git a/crates/sdk/Cargo.toml b/crates/sdk/Cargo.toml index 6a3244a914b..2b589e10617 100644 --- a/crates/sdk/Cargo.toml +++ b/crates/sdk/Cargo.toml @@ -99,6 +99,7 @@ circular-queue.workspace = true data-encoding.workspace = true derivation-path.workspace = true duration-str.workspace = true +either.workspace = true ethbridge-bridge-contract.workspace = true ethers.workspace = true eyre.workspace = true diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index b9daabeca20..79485f006d3 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -61,7 +61,9 @@ use namada_token::storage_key::balance_key; use namada_token::DenominatedAmount; use namada_tx::data::pgf::UpdateStewardCommission; use namada_tx::data::pos::{BecomeValidator, ConsensusKeyChange}; -use namada_tx::data::{pos, BatchedTxResult, ResultCode, TxResult}; +use namada_tx::data::{ + compute_inner_tx_hash, pos, BatchedTxResult, ResultCode, TxResult, +}; pub use namada_tx::{Authorization, *}; use num_traits::Zero; use rand_core::{OsRng, RngCore}; @@ -159,13 +161,17 @@ impl ProcessTxResponse { /// all VPs. Note that this always returns false for dry-run transactions. pub fn is_applied_and_valid( &self, + wrapper_hash: Option<&Hash>, cmt: &TxCommitments, ) -> Option<&BatchedTxResult> { match self { ProcessTxResponse::Applied(resp) => { if resp.code == ResultCode::Ok { if let Some(InnerTxResult::Success(result)) = - resp.batch_result().get(&cmt.get_hash()) + resp.batch_result().get(&compute_inner_tx_hash( + wrapper_hash, + either::Right(cmt), + )) { return Some(result); } @@ -241,6 +247,7 @@ pub async fn process_tx( // We use this to determine when the wrapper tx makes it on-chain let tx_hash = tx.header_hash().to_string(); let cmts = tx.commitments().clone(); + let wrapper_hash = tx.wrapper_hash(); // We use this to determine when the decrypted inner tx makes it // on-chain let to_broadcast = TxBroadcastData::Live { tx, tx_hash }; @@ -253,7 +260,10 @@ pub async fn process_tx( Ok(resp) => { for cmt in cmts { if let Some(InnerTxResult::Success(result)) = - resp.batch_result().get(&cmt.get_hash()) + resp.batch_result().get(&compute_inner_tx_hash( + wrapper_hash.as_ref(), + either::Right(&cmt), + )) { save_initialized_accounts( context, @@ -400,13 +410,13 @@ pub async fn submit_tx( /// Display a result of a tx batch. pub fn display_batch_resp(context: &impl Namada, resp: &TxResponse) { - for (cmt_hash, result) in resp.batch_result() { + for (inner_hash, result) in resp.batch_result() { match result { InnerTxResult::Success(_) => { display_line!( context.io(), "Transaction {} was successfully applied at height {}.", - cmt_hash, + inner_hash, resp.height, ); } @@ -420,7 +430,7 @@ pub fn display_batch_resp(context: &impl Namada, resp: &TxResponse) { context.io(), "Transaction {} was rejected by VPs: {}\nErrors: \ {}\nChanged keys: {}", - cmt_hash, + inner_hash, serde_json::to_string_pretty( &inner.vps_result.rejected_vps ) @@ -434,7 +444,7 @@ pub fn display_batch_resp(context: &impl Namada, resp: &TxResponse) { edisplay_line!( context.io(), "Transaction {} failed.\nDetails: {}", - cmt_hash, + inner_hash, serde_json::to_string_pretty(&resp).unwrap() ); } diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 6d50ea4b1e1..50a48a6dd30 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3985,6 +3985,7 @@ dependencies = [ "data-encoding", "derivation-path", "duration-str", + "either", "ethbridge-bridge-contract", "ethers", "eyre", diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index 9af3852c743..9b4d37c7a5c 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -4041,6 +4041,7 @@ dependencies = [ "data-encoding", "derivation-path", "duration-str", + "either", "ethbridge-bridge-contract", "ethers", "eyre",