Merge branch 'tiago/uniquely-identify-inner-txs' (#3416)

* origin/tiago/uniquely-identify-inner-txs:
  Changelog for #3416
  Re-export `either` from `namada_tx`
  Derive inner tx hash using wrapper hash

.changelog/unreleased/improvements/3416-uniquely-identify-inner-txs.md

@@ -0,0 +1,2 @@
+- Index batched txs via their wrapper and commitment hashes.
+  ([\#3416](https://github.com/anoma/namada/pull/3416))

Cargo.toml

@@ -96,6 +96,7 @@ directories = "4.0.1"
 drain_filter_polyfill = "0.1.3"
 duration-str = "0.10.0"
 ed25519-consensus = "2.1.0"
+either = "1.12.0"
 escargot = "0.5.7"
 ethabi = "18.0.0"
 ethbridge-bridge-contract = {git = "https://github.com/heliaxdev/ethbridge-rs", tag = "v0.24.0"}

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

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."
@@ -806,11 +818,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
@@ -1126,9 +1145,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?;
         }

crates/namada/Cargo.toml

@@ -105,6 +105,7 @@ async-trait = { version = "0.1.51", optional = true }
 borsh.workspace = true
 borsh-ext.workspace = true
 clru.workspace = true
+either.workspace = true
 ethers.workspace = true
 eyre.workspace = true
 itertools.workspace = true

crates/namada/src/ledger/mod.rs

@@ -55,7 +55,10 @@ mod dry_run_tx {
         tx.validate_tx().into_storage_result()?;
 
         // Wrapper dry run to allow estimating the gas cost of a transaction
-        let (mut tx_result, tx_gas_meter) = match tx.header().tx_type {
+        let (wrapper_hash, mut tx_result, tx_gas_meter) = match tx
+            .header()
+            .tx_type
+        {
             TxType::Wrapper(wrapper) => {
                 let gas_limit =
                     Gas::try_from(wrapper.gas_limit).into_storage_result()?;
@@ -72,7 +75,11 @@ mod dry_run_tx {
 
                 temp_state.write_log_mut().commit_tx();
                 let available_gas = tx_gas_meter.borrow().get_available_gas();
-                (tx_result, TxGasMeter::new_from_sub_limit(available_gas))
+                (
+                    Some(tx.header_hash()),
+                    tx_result,
+                    TxGasMeter::new_from_sub_limit(available_gas),
+                )
             }
             _ => {
                 // If dry run only the inner tx, use the max block gas as
@@ -81,7 +88,7 @@ mod dry_run_tx {
                     namada_parameters::get_max_block_gas(ctx.state)?;
                 let gas_limit = Gas::try_from(GasLimit::from(max_block_gas))
                     .into_storage_result()?;
-                (TxResult::default(), TxGasMeter::new(gas_limit))
+                (None, TxResult::default(), TxGasMeter::new(gas_limit))
             }
         };
 
@@ -104,10 +111,11 @@ mod dry_run_tx {
             } else {
                 temp_state.write_log_mut().drop_tx();
             }
-            tx_result
-                .batch_results
-                .0
-                .insert(cmt.get_hash(), batched_tx_result);
+            tx_result.batch_results.insert_inner_tx_result(
+                wrapper_hash.as_ref(),
+                either::Right(cmt),
+                batched_tx_result,
+            );
         }
         // Account gas for both batch and wrapper
         tx_result.gas_used = tx_gas_meter.borrow().get_tx_consumed_gas();
@@ -297,8 +305,7 @@ mod test {
             result
                 .data
                 .batch_results
-                .0
-                .get(&cmt.get_hash())
+                .get_inner_tx_result(None, either::Right(cmt))
                 .unwrap()
                 .as_ref()
                 .unwrap()

crates/namada/src/ledger/protocol/mod.rs

@@ -183,6 +183,9 @@ pub enum DispatchArgs<'a, CA: 'static + WasmCacheAccess + Sync> {
     Raw {
         /// The tx index
         tx_index: TxIndex,
+        /// Hash of the header of the wrapper tx containing
+        /// this raw tx
+        wrapper_hash: Option<&'a Hash>,
         /// The result of the corresponding wrapper tx (missing if governance
         /// transaction)
         wrapper_tx_result: Option<TxResult<Error>>,
@@ -219,6 +222,7 @@ where
     match dispatch_args {
         DispatchArgs::Raw {
             tx_index,
+            wrapper_hash,
             wrapper_tx_result,
             vp_wasm_cache,
             tx_wasm_cache,
@@ -239,6 +243,7 @@ where
 
                 dispatch_inner_txs(
                     tx,
+                    wrapper_hash,
                     tx_result,
                     tx_index,
                     tx_gas_meter,
@@ -263,11 +268,15 @@ where
                 )?;
                 Ok(TxResult {
                     gas_used: tx_gas_meter.borrow().get_tx_consumed_gas(),
-                    batch_results: BatchResults(
-                        [(cmt.get_hash(), Ok(batched_tx_result))]
-                            .into_iter()
-                            .collect(),
-                    ),
+                    batch_results: {
+                        let mut batch_results = BatchResults::new();
+                        batch_results.insert_inner_tx_result(
+                            wrapper_hash,
+                            either::Right(cmt),
+                            Ok(batched_tx_result),
+                        );
+                        batch_results
+                    },
                 }
                 .to_extended_result(None))
             }
@@ -279,11 +288,15 @@ where
                 apply_protocol_tx(protocol_tx.tx, tx.data(cmt), state)?;
 
             Ok(TxResult {
-                batch_results: BatchResults(
-                    [(cmt.get_hash(), Ok(batched_tx_result))]
-                        .into_iter()
-                        .collect(),
-                ),
+                batch_results: {
+                    let mut batch_results = BatchResults::new();
+                    batch_results.insert_inner_tx_result(
+                        None,
+                        either::Right(cmt),
+                        Ok(batched_tx_result),
+                    );
+                    batch_results
+                },
                 ..Default::default()
             }
             .to_extended_result(None))
@@ -308,8 +321,10 @@ where
     }
 }
 
+#[allow(clippy::too_many_arguments)]
 fn dispatch_inner_txs<'a, D, H, CA>(
     tx: &Tx,
+    wrapper_hash: Option<&'a Hash>,
     tx_result: TxResult<Error>,
     tx_index: TxIndex,
     tx_gas_meter: &'a RefCell<TxGasMeter>,
@@ -341,10 +356,14 @@ where
                 // Gas error aborts the execution of the entire batch
                 extended_tx_result.tx_result.gas_used =
                     tx_gas_meter.borrow().get_tx_consumed_gas();
-                extended_tx_result.tx_result.batch_results.0.insert(
-                    cmt.get_hash(),
-                    Err(Error::GasError(msg.to_owned())),
-                );
+                extended_tx_result
+                    .tx_result
+                    .batch_results
+                    .insert_inner_tx_result(
+                        wrapper_hash,
+                        either::Right(cmt),
+                        Err(Error::GasError(msg.to_owned())),
+                    );
                 state.write_log_mut().drop_tx();
                 return Err(DispatchError {
                     error: Error::GasError(msg.to_owned()),
@@ -358,8 +377,11 @@ where
                 extended_tx_result
                     .tx_result
                     .batch_results
-                    .0
-                    .insert(cmt.get_hash(), res);
+                    .insert_inner_tx_result(
+                        wrapper_hash,
+                        either::Right(cmt),
+                        res,
+                    );
                 extended_tx_result.tx_result.gas_used =
                     tx_gas_meter.borrow().get_tx_consumed_gas();
                 if is_accepted {

crates/node/Cargo.toml

@@ -61,6 +61,7 @@ clap = { workspace = true, optional = true }
 color-eyre.workspace = true
 data-encoding.workspace = true
 drain_filter_polyfill.workspace = true
+either.workspace = true
 ethabi.workspace = true
 ethbridge-bridge-events.workspace = true
 ethbridge-events.workspace = true

crates/node/src/bench_utils.rs

@@ -914,19 +914,22 @@ impl Client for BenchShell {
                     .map(|(idx, (tx, changed_keys))| {
                         let tx_result = TxResult::<String> {
                             gas_used: 0.into(),
-                            batch_results: BatchResults(
-                                [(
-                                    tx.first_commitments().unwrap().get_hash(),
+                            batch_results: {
+                                let mut batch_results = BatchResults::new();
+                                batch_results.insert_inner_tx_result(
+                                    tx.wrapper_hash().as_ref(),
+                                    either::Right(
+                                        tx.first_commitments().unwrap(),
+                                    ),
                                     Ok(BatchedTxResult {
                                         changed_keys: changed_keys.to_owned(),
                                         vps_result: VpsResult::default(),
                                         initialized_accounts: vec![],
                                         events: BTreeSet::default(),
                                     }),
-                                )]
-                                .into_iter()
-                                .collect(),
-                            ),
+                                );
+                                batch_results
+                            },
                         };
                         let event: Event = new_tx_event(tx, height.value())
                             .with(Batch(&tx_result))