diff --git a/.changelog/unreleased/improvements/3758-improve-dry-run.md b/.changelog/unreleased/improvements/3758-improve-dry-run.md new file mode 100644 index 0000000000..910bef0f32 --- /dev/null +++ b/.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)) \ No newline at end of file diff --git a/crates/gas/src/lib.rs b/crates/gas/src/lib.rs index dbbb22a13d..e97c8abc09 100644 --- a/crates/gas/src/lib.rs +++ b/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 diff --git a/crates/governance/src/vp/mod.rs b/crates/governance/src/vp/mod.rs index 2ec68c3076..2471d2ef21 100644 --- a/crates/governance/src/vp/mod.rs +++ b/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.", ))); } diff --git a/crates/node/src/dry_run_tx.rs b/crates/node/src/dry_run_tx.rs index 678373daf6..17d3f9f64f 100644 --- a/crates/node/src/dry_run_tx.rs +++ b/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,18 +61,11 @@ 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) @@ -82,52 +73,31 @@ where ( 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(), diff --git a/crates/node/src/protocol.rs b/crates/node/src/protocol.rs index 8a09a3b873..66bd751e66 100644 --- a/crates/node/src/protocol.rs +++ b/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, tx_index: TxIndex, tx_gas_meter: &'a RefCell, - state: &'a mut WlState, + state: &'a mut S, vp_wasm_cache: &'a mut VpCache, tx_wasm_cache: &'a mut TxCache, ) -> std::result::Result, DispatchError> where + S: 'static + State + Read + 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( +// 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( batched_tx: &BatchedTxRef<'_>, tx_index: &TxIndex, shell_params: ShellParams<'_, S, D, H, CA>, diff --git a/crates/shielded_token/src/utils.rs b/crates/shielded_token/src/utils.rs index 9593c93a20..5202e39350 100644 --- a/crates/shielded_token/src/utils.rs +++ b/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, diff --git a/crates/trans_token/src/storage.rs b/crates/trans_token/src/storage.rs index 02281fec91..fb497f3675 100644 --- a/crates/trans_token/src/storage.rs +++ b/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) diff --git a/crates/vm/src/host_env.rs b/crates/vm/src/host_env.rs index 02288e7779..77f2b1dcb0 100644 --- a/crates/vm/src/host_env.rs +++ b/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()?)