Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve dry run rebase #3781

Merged
merged 6 commits into from
Sep 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changelog/unreleased/improvements/3758-improve-dry-run.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Miscellaneous improvements and bug fixes to the dry run command.
([\#3758](https://github.com/anoma/namada/pull/3758))
4 changes: 3 additions & 1 deletion crates/gas/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion crates/governance/src/vp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
)));
}

Expand Down
84 changes: 27 additions & 57 deletions crates/node/src/dry_run_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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(),
Expand Down
11 changes: 6 additions & 5 deletions crates/node/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
Expand Down
2 changes: 1 addition & 1 deletion crates/shielded_token/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion crates/trans_token/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions crates/vm/src/host_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?)
Expand Down
Loading