Merge pull request #3781 from anoma/grarco/improve-dry-run-2-rebase

Improve dry run rebase

.changelog/unreleased/improvements/3758-improve-dry-run.md

@@ -0,0 +1,2 @@
+- Miscellaneous improvements and bug fixes to the dry run command.
+  ([\#3758](https://github.com/anoma/namada/pull/3758))

crates/gas/src/lib.rs

@@ -367,7 +367,7 @@ pub trait GasMetering {
         )
     }
 
-    /// Get the gas consumed by the tx alone
+    /// Get the gas consumed by the tx
     fn get_tx_consumed_gas(&self) -> Gas;
 
     /// Get the gas limit
@@ -434,6 +434,7 @@ impl GasMetering for TxGasMeter {
         Ok(())
     }
 
+    /// Get the entire gas used by the transaction up until this point
     fn get_tx_consumed_gas(&self) -> Gas {
         if !self.gas_overflow {
             self.transaction_gas
@@ -512,6 +513,7 @@ impl GasMetering for VpGasMeter {
         Ok(())
     }
 
+    /// Get the gas consumed by the tx alone before the vps were executed
     fn get_tx_consumed_gas(&self) -> Gas {
         if !self.gas_overflow {
             self.initial_gas

crates/governance/src/vp/mod.rs

@@ -566,7 +566,7 @@ where
         if !proposal_type.is_default_with_wasm() {
             return Err(Error::new_alloc(format!(
                 "Proposal with id {proposal_id} modified a proposal code key, \
-                 but its type is not default.",
+                 but its type is not allowed this change.",
             )));
         }
 

crates/node/src/dry_run_tx.rs

@@ -9,9 +9,7 @@ use namada_sdk::queries::{EncodedResponseQuery, RequestQuery};
 use namada_sdk::state::{
     DBIter, Result, ResultExt, StorageHasher, TxIndex, DB,
 };
-use namada_sdk::tx::data::{
-    DryRunResult, ExtendedTxResult, GasLimit, TxResult, TxType,
-};
+use namada_sdk::tx::data::{DryRunResult, GasLimit, TxResult, TxType};
 use namada_sdk::tx::Tx;
 use namada_vm::wasm::{TxCache, VpCache};
 use namada_vm::WasmCacheAccess;
@@ -36,8 +34,8 @@ where
 
     let gas_scale = parameters::get_gas_scale(&state)?;
 
-    // Wrapper dry run to allow estimating the gas cost of a transaction
-    let (wrapper_hash, extended_tx_result, tx_gas_meter, gas_used) =
+    // Wrapper dry run to allow estimating the entire gas cost of a transaction
+    let (wrapper_hash, extended_tx_result, tx_gas_meter) =
         match tx.header().tx_type {
             TxType::Wrapper(wrapper) => {
                 let gas_limit = wrapper
@@ -63,71 +61,43 @@ where
                 .into_storage_result()?;
 
                 state.write_log_mut().commit_tx_to_batch();
-                let available_gas = tx_gas_meter.borrow().get_available_gas();
-                let consumed_gas = tx_gas_meter.borrow().get_tx_consumed_gas();
-                (
-                    Some(tx.header_hash()),
-                    tx_result,
-                    TxGasMeter::new(available_gas),
-                    consumed_gas,
-                )
+                (Some(tx.header_hash()), tx_result, tx_gas_meter)
             }
             _ => {
-                // If dry run only the inner tx, use the max block gas as
-                // the gas limit
+                // When dry running only the inner tx(s), use the max block gas
+                // as the gas limit
                 let max_block_gas = parameters::get_max_block_gas(&state)?;
                 let gas_limit = GasLimit::from(max_block_gas)
                     .as_scaled_gas(gas_scale)
                     .into_storage_result()?;
                 (
                     None,
                     TxResult::default().to_extended_result(None),
-                    TxGasMeter::new(gas_limit),
-                    0.into(),
+                    RefCell::new(TxGasMeter::new(gas_limit)),
                 )
             }
         };
 
-    let ExtendedTxResult {
-        mut tx_result,
-        ref masp_tx_refs,
-        ibc_tx_data_refs,
-    } = extended_tx_result;
-    let tx_gas_meter = RefCell::new(tx_gas_meter);
-    for cmt in
-        protocol::get_batch_txs_to_execute(&tx, masp_tx_refs, &ibc_tx_data_refs)
-    {
-        let batched_tx = tx.batch_ref_tx(cmt);
-        let batched_tx_result = protocol::apply_wasm_tx(
-            &batched_tx,
-            &TxIndex(0),
-            ShellParams::new(
-                &tx_gas_meter,
-                &mut state,
-                &mut vp_wasm_cache,
-                &mut tx_wasm_cache,
-            ),
-        );
-        let is_accepted =
-            matches!(&batched_tx_result, Ok(result) if result.is_accepted());
-        if is_accepted {
-            state.write_log_mut().commit_tx_to_batch();
-        } else {
-            state.write_log_mut().drop_tx();
-        }
-        tx_result.insert_inner_tx_result(
-            wrapper_hash.as_ref(),
-            either::Right(cmt),
-            batched_tx_result,
-        );
-    }
-    // Account gas for both batch and wrapper
-    let gas_used = gas_used
-        .checked_add(tx_gas_meter.borrow().get_tx_consumed_gas())
-        .unwrap_or(u64::MAX.into())
-        .get_whole_gas_units(gas_scale);
-    let tx_result_string = tx_result.to_result_string();
-    let dry_run_result = DryRunResult(tx_result_string, gas_used);
+    let extended_tx_result = protocol::dispatch_inner_txs(
+        &tx,
+        wrapper_hash.as_ref(),
+        extended_tx_result,
+        TxIndex(0),
+        &tx_gas_meter,
+        &mut state,
+        &mut vp_wasm_cache,
+        &mut tx_wasm_cache,
+    )
+    .map_err(|err| err.error)
+    .into_storage_result()?;
+    let tx_result_string = extended_tx_result.tx_result.to_result_string();
+    let dry_run_result = DryRunResult(
+        tx_result_string,
+        tx_gas_meter
+            .borrow()
+            .get_tx_consumed_gas()
+            .get_whole_gas_units(gas_scale),
+    );
 
     Ok(EncodedResponseQuery {
         data: dry_run_result.serialize_to_vec(),

crates/node/src/protocol.rs

@@ -329,17 +329,18 @@ pub(crate) fn get_batch_txs_to_execute<'a>(
 }
 
 #[allow(clippy::too_many_arguments)]
-fn dispatch_inner_txs<'a, D, H, CA>(
+pub(crate) fn dispatch_inner_txs<'a, S, D, H, CA>(
     tx: &Tx,
     wrapper_hash: Option<&'a Hash>,
     mut extended_tx_result: ExtendedTxResult<Error>,
     tx_index: TxIndex,
     tx_gas_meter: &'a RefCell<TxGasMeter>,
-    state: &'a mut WlState<D, H>,
+    state: &'a mut S,
     vp_wasm_cache: &'a mut VpCache<CA>,
     tx_wasm_cache: &'a mut TxCache<CA>,
 ) -> std::result::Result<ExtendedTxResult<Error>, DispatchError>
 where
+    S: 'static + State<D = D, H = H> + Read<Err = state::Error> + Sync,
     D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
     H: 'static + StorageHasher + Sync,
     CA: 'static + WasmCacheAccess + Sync,
@@ -931,9 +932,9 @@ where
     }
 }
 
-/// Apply a transaction going via the wasm environment. Gas will be metered and
-/// validity predicates will be triggered in the normal way.
-pub fn apply_wasm_tx<S, D, H, CA>(
+// Apply a transaction going via the wasm environment. Gas will be metered and
+// validity predicates will be triggered in the normal way.
+fn apply_wasm_tx<S, D, H, CA>(
     batched_tx: &BatchedTxRef<'_>,
     tx_index: &TxIndex,
     shell_params: ShellParams<'_, S, D, H, CA>,

crates/shielded_token/src/utils.rs

@@ -29,7 +29,7 @@ fn reveal_nullifiers(
 /// Appends the note commitments of the provided transaction to the merkle tree
 /// and updates the anchor
 /// NOTE: this function is public as a temporary workaround because of an issue
-/// when running this function in WASM
+/// when running it in WASM (https://github.com/anoma/masp/issues/73)
 pub fn update_note_commitment_tree(
     ctx: &mut (impl StorageRead + StorageWrite),
     transaction: &Transaction,

crates/trans_token/src/storage.rs

@@ -310,7 +310,7 @@ where
                 )
                 .ok_or_else(underflow_err)?
         };
-        // Wite the new balance
+        // Write the new balance
         storage.write(&owner_key, new_owner_balance)?;
     }
     Ok(debited_accounts)

crates/vm/src/host_env.rs

@@ -2134,8 +2134,6 @@ where
     H: 'static + StorageHasher,
     CA: WasmCacheAccess,
 {
-    let _sentinel = unsafe { env.ctx.sentinel.get() };
-    let _gas_meter = unsafe { env.ctx.gas_meter.get() };
     let (serialized_transaction, gas) = env
         .memory
         .read_bytes(transaction_ptr, transaction_len.try_into()?)