Skip to content

Commit

Permalink
fixup! Merge branch 'grarco/tx-batch' (#3103)
Browse files Browse the repository at this point in the history
  • Loading branch information
brentstone authored and tzemanovic committed May 20, 2024
1 parent d5dd8a3 commit 30bb889
Show file tree
Hide file tree
Showing 26 changed files with 131 additions and 84 deletions.
2 changes: 1 addition & 1 deletion crates/apps/src/lib/bench_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ impl BenchShell {
/// Execute the tx and return a set of verifiers inserted by the tx.
pub fn execute_tx(
&mut self,
batched_tx: &BatchedTxRef,
batched_tx: &BatchedTxRef<'_>,
) -> BTreeSet<Address> {
let gas_meter =
RefCell::new(TxGasMeter::new_from_sub_limit(u64::MAX.into()));
Expand Down
100 changes: 54 additions & 46 deletions crates/apps/src/lib/node/ledger/shell/finalize_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,29 +214,27 @@ where
continue;
}

let (mut tx_event, tx_gas_meter, mut wrapper_args) =
match &tx_header.tx_type {
TxType::Wrapper(wrapper) => {
stats.increment_wrapper_txs();
let tx_event = new_tx_event(&tx, height.0);
let gas_limit = match Gas::try_from(wrapper.gas_limit) {
Ok(value) => value,
Err(_) => {
response.events.emit(
new_tx_event(&tx, height.0)
.with(Code(ResultCode::InvalidTx))
.with(Info(
"The wrapper gas limit overflowed \
gas representation"
.to_owned(),
))
.with(GasUsed(0.into())),
);
continue;
}
};
let gas_meter = TxGasMeter::new(gas_limit);
for cmt in tx.commitments() {
let (tx_gas_meter, mut wrapper_args) = match &tx_header.tx_type {
TxType::Wrapper(wrapper) => {
stats.increment_wrapper_txs();
let gas_limit = match Gas::try_from(wrapper.gas_limit) {
Ok(value) => value,
Err(_) => {
response.events.emit(
new_tx_event(&tx, height.0)
.with(Code(ResultCode::InvalidTx))
.with(Info(
"The wrapper gas limit overflowed gas \
representation"
.to_owned(),
))
.with(GasUsed(0.into())),
);
continue;
}
};
let gas_meter = TxGasMeter::new(gas_limit);
for cmt in tx.commitments() {
if let Some(code_sec) = tx
.get_section(cmt.code_sechash())
.and_then(|x| Section::code_sec(x.as_ref()))
Expand All @@ -245,8 +243,8 @@ where
code_sec.code.hash().to_string(),
);
}
}
}

(
gas_meter,
Some(WrapperArgs {
Expand Down Expand Up @@ -544,8 +542,8 @@ where
namada::tx::data::TxResult<protocol::Error>,
DispatchError,
>,
tx_data: TxData,
mut tx_logs: TxLogs,
tx_data: TxData<'_>,
mut tx_logs: TxLogs<'_>,
) {
// Check the commitment of the fee unshielding regardless of the
// result, it could be committed even in case of errors
Expand Down Expand Up @@ -628,8 +626,8 @@ where
&mut self,
response: &mut shim::response::FinalizeBlock,
tx_result: namada::tx::data::TxResult<protocol::Error>,
tx_data: TxData,
tx_logs: &mut TxLogs,
tx_data: TxData<'_>,
tx_logs: &mut TxLogs<'_>,
) {
let mut temp_log = TempTxLogs::new_from_tx_logs(tx_logs);

Expand All @@ -646,8 +644,13 @@ where
if tx_data.is_atomic_batch && is_any_tx_invalid {
// Atomic batches need custom handling when even a single tx fails,
// since we need to drop everything
let unrun_txs = tx_data.commitments_len
- tx_result.batch_results.0.len() as u64;
let unrun_txs = tx_data
.commitments_len
.checked_sub(
u64::try_from(tx_result.batch_results.0.len())
.expect("Should be able to convert to u64"),
)
.expect("Shouldn't underflow");
temp_log.stats.set_failing_atomic_batch(unrun_txs);
temp_log.commit_stats_only(tx_logs);
self.state.write_log_mut().drop_batch();
Expand Down Expand Up @@ -687,8 +690,8 @@ where
response: &mut shim::response::FinalizeBlock,
msg: &Error,
tx_result: namada::tx::data::TxResult<protocol::Error>,
tx_data: TxData,
tx_logs: &mut TxLogs,
tx_data: TxData<'_>,
tx_logs: &mut TxLogs<'_>,
) {
let mut temp_log = TempTxLogs::new_from_tx_logs(tx_logs);

Expand All @@ -702,8 +705,13 @@ where
tx_data.height,
);

let unrun_txs =
tx_data.commitments_len - tx_result.batch_results.0.len() as u64;
let unrun_txs = tx_data
.commitments_len
.checked_sub(
u64::try_from(tx_result.batch_results.0.len())
.expect("Should be able to convert to u64"),
)
.expect("Shouldn't underflow");

if tx_data.is_atomic_batch {
tx_logs.stats.set_failing_atomic_batch(unrun_txs);
Expand Down Expand Up @@ -735,7 +743,7 @@ where
.extend(Batch(&tx_result.to_result_string()));
}

fn handle_batch_error_reprot(&mut self, err: &Error, tx_data: TxData) {
fn handle_batch_error_reprot(&mut self, err: &Error, tx_data: TxData<'_>) {
// If user transaction didn't fail because of out of gas nor
// invalid section commitment, commit its hash to prevent
// replays
Expand Down Expand Up @@ -801,7 +809,7 @@ struct TempTxLogs {
}

impl TempTxLogs {
fn new_from_tx_logs(tx_logs: &TxLogs) -> Self {
fn new_from_tx_logs(tx_logs: &TxLogs<'_>) -> Self {
Self {
tx_event: Event::new(
tx_logs.tx_event.kind().to_owned(),
Expand Down Expand Up @@ -3095,7 +3103,7 @@ mod test_finalize_block {
assert_eq!(*event[0].kind(), APPLIED_TX);
let code = event[0].read_attribute::<CodeAttr>().expect("Test failed");
assert_eq!(code, ResultCode::Ok);
let inner_tx_result = event[0].read_attribute::<Batch>().unwrap();
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let first_tx_result = inner_tx_result
.batch_results
.0
Expand Down Expand Up @@ -3246,7 +3254,7 @@ mod test_finalize_block {
assert_eq!(*event[1].kind(), APPLIED_TX);
let code = event[1].read_attribute::<CodeAttr>().expect("Test failed");
assert_eq!(code, ResultCode::Ok);
let inner_tx_result = event[1].read_attribute::<Batch>().unwrap();
let inner_tx_result = event[1].read_attribute::<Batch<'_>>().unwrap();
let inner_result = inner_tx_result
.batch_results
.0
Expand All @@ -3256,7 +3264,7 @@ mod test_finalize_block {
assert_eq!(*event[2].kind(), APPLIED_TX);
let code = event[2].read_attribute::<CodeAttr>().expect("Test failed");
assert_eq!(code, ResultCode::Ok);
let inner_tx_result = event[2].read_attribute::<Batch>().unwrap();
let inner_tx_result = event[2].read_attribute::<Batch<'_>>().unwrap();
let inner_result = inner_tx_result
.batch_results
.0
Expand All @@ -3271,7 +3279,7 @@ mod test_finalize_block {
assert_eq!(*event[3].kind(), APPLIED_TX);
let code = event[3].read_attribute::<CodeAttr>().expect("Test failed");
assert_eq!(code, ResultCode::Ok);
let inner_tx_result = event[3].read_attribute::<Batch>().unwrap();
let inner_tx_result = event[3].read_attribute::<Batch<'_>>().unwrap();
let inner_result = inner_tx_result
.batch_results
.0
Expand Down Expand Up @@ -3476,7 +3484,7 @@ mod test_finalize_block {
assert_eq!(*event.kind(), APPLIED_TX);
let code = event.read_attribute::<CodeAttr>().expect("Test failed");
assert_eq!(code, ResultCode::Ok);
let inner_tx_result = event.read_attribute::<Batch>().unwrap();
let inner_tx_result = event.read_attribute::<Batch<'_>>().unwrap();
let inner_result = inner_tx_result
.batch_results
.0
Expand Down Expand Up @@ -5399,7 +5407,7 @@ mod test_finalize_block {

let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::Ok);
let inner_tx_result = event[0].read_attribute::<Batch>().unwrap();
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;

for cmt in batch.commitments() {
Expand Down Expand Up @@ -5444,7 +5452,7 @@ mod test_finalize_block {

let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::WasmRuntimeError);
let inner_tx_result = event[0].read_attribute::<Batch>().unwrap();
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;

assert!(
Expand Down Expand Up @@ -5494,7 +5502,7 @@ mod test_finalize_block {

let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::Ok);
let inner_tx_result = event[0].read_attribute::<Batch>().unwrap();
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;

assert!(
Expand Down Expand Up @@ -5562,7 +5570,7 @@ mod test_finalize_block {

let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::WasmRuntimeError);
let inner_tx_result = event[0].read_attribute::<Batch>().unwrap();
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;

assert!(
Expand Down Expand Up @@ -5611,7 +5619,7 @@ mod test_finalize_block {

let code = event[0].read_attribute::<CodeAttr>().unwrap();
assert_eq!(code, ResultCode::WasmRuntimeError);
let inner_tx_result = event[0].read_attribute::<Batch>().unwrap();
let inner_tx_result = event[0].read_attribute::<Batch<'_>>().unwrap();
let inner_results = inner_tx_result.batch_results.0;

assert!(
Expand Down
7 changes: 5 additions & 2 deletions crates/namada/src/ledger/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ where

fn validate_tx(
&self,
tx_data: &BatchedTxRef,
tx_data: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
verifiers: &BTreeSet<Address>,
) -> Result<()> {
Expand Down Expand Up @@ -1119,7 +1119,10 @@ where
}

/// Validate a governance parameter
pub fn is_valid_parameter(&self, batched_tx: &BatchedTxRef) -> Result<()> {
pub fn is_valid_parameter(
&self,
batched_tx: &BatchedTxRef<'_>,
) -> Result<()> {
let BatchedTxRef { tx, cmt } = batched_tx;
tx.data(cmt).map_or_else(
|| {
Expand Down
1 change: 1 addition & 0 deletions crates/namada/src/ledger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub use {
mod dry_run_tx {
use std::cell::RefCell;

use namada_gas::Gas;
use namada_sdk::queries::{EncodedResponseQuery, RequestCtx, RequestQuery};
use namada_state::{DBIter, ResultExt, StorageHasher, DB};
use namada_tx::data::{GasLimit, TxResult};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ where

fn validate_tx(
&self,
batched_tx: &BatchedTxRef,
batched_tx: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
_verifiers: &BTreeSet<Address>,
) -> Result<(), Error> {
Expand Down
2 changes: 1 addition & 1 deletion crates/namada/src/ledger/native_vp/ethereum_bridge/nut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ where

fn validate_tx(
&self,
_: &BatchedTxRef,
_: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
verifiers: &BTreeSet<Address>,
) -> Result<(), Self::Error> {
Expand Down
2 changes: 1 addition & 1 deletion crates/namada/src/ledger/native_vp/ethereum_bridge/vp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ where
/// no wasm transactions should be able to modify those keys.
fn validate_tx(
&self,
_: &BatchedTxRef,
_: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
verifiers: &BTreeSet<Address>,
) -> Result<(), Self::Error> {
Expand Down
2 changes: 1 addition & 1 deletion crates/namada/src/ledger/native_vp/ibc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ where

fn validate_tx(
&self,
batched_tx: &BatchedTxRef,
batched_tx: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
_verifiers: &BTreeSet<Address>,
) -> VpResult<()> {
Expand Down
2 changes: 1 addition & 1 deletion crates/namada/src/ledger/native_vp/masp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ where

fn validate_tx(
&self,
tx_data: &BatchedTxRef,
tx_data: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
_verifiers: &BTreeSet<Address>,
) -> Result<()> {
Expand Down
2 changes: 1 addition & 1 deletion crates/namada/src/ledger/native_vp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub trait NativeVp {
/// Run the validity predicate
fn validate_tx(
&self,
batched_tx: &BatchedTxRef,
batched_tx: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
verifiers: &BTreeSet<Address>,
) -> std::result::Result<(), Self::Error>;
Expand Down
7 changes: 5 additions & 2 deletions crates/namada/src/ledger/native_vp/multitoken.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ where

fn validate_tx(
&self,
tx_data: &BatchedTxRef,
tx_data: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
verifiers: &BTreeSet<Address>,
) -> Result<()> {
Expand Down Expand Up @@ -280,7 +280,10 @@ where
}

/// Return if the parameter change was done via a governance proposal
pub fn is_valid_parameter(&self, batched_tx: &BatchedTxRef) -> Result<()> {
pub fn is_valid_parameter(
&self,
batched_tx: &BatchedTxRef<'_>,
) -> Result<()> {
batched_tx.tx.data(batched_tx.cmt).map_or_else(
|| {
Err(native_vp::Error::new_const(
Expand Down
2 changes: 1 addition & 1 deletion crates/namada/src/ledger/native_vp/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ where

fn validate_tx(
&self,
batched_tx: &BatchedTxRef,
batched_tx: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
_verifiers: &BTreeSet<Address>,
) -> Result<()> {
Expand Down
4 changes: 2 additions & 2 deletions crates/namada/src/ledger/pgf/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ where

fn validate_tx(
&self,
batched_tx: &BatchedTxRef,
batched_tx: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
verifiers: &BTreeSet<Address>,
) -> Result<()> {
Expand Down Expand Up @@ -202,7 +202,7 @@ where
/// Validate a governance parameter
pub fn is_valid_parameter_change(
&self,
batched_tx: &BatchedTxRef,
batched_tx: &BatchedTxRef<'_>,
) -> Result<()> {
batched_tx.tx.data(batched_tx.cmt).map_or_else(
|| {
Expand Down
2 changes: 1 addition & 1 deletion crates/namada/src/ledger/pos/vp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ where

fn validate_tx(
&self,
batched_tx: &BatchedTxRef,
batched_tx: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<Key>,
verifiers: &BTreeSet<Address>,
) -> Result<()> {
Expand Down
Loading

0 comments on commit 30bb889

Please sign in to comment.