From ade4cca3a31835efc55e8e74a93a3a86e7dee7fa Mon Sep 17 00:00:00 2001 From: crStiv Date: Fri, 19 Sep 2025 21:44:13 +0000 Subject: [PATCH 1/2] feat: add transaction index to batch execution error handling --- crates/context/interface/src/result.rs | 47 +++++++++ crates/handler/src/api.rs | 17 ++-- crates/handler/src/validation.rs | 136 ++++++++++++++++++++++++- 3 files changed, 188 insertions(+), 12 deletions(-) diff --git a/crates/context/interface/src/result.rs b/crates/context/interface/src/result.rs index d309ca13ca..71fd30a548 100644 --- a/crates/context/interface/src/result.rs +++ b/crates/context/interface/src/result.rs @@ -642,3 +642,50 @@ pub enum OutOfGasError { /// When performing SSTORE the gasleft is less than or equal to 2300 ReentrancySentry, } + +/// Error that includes transaction index for batch transaction processing. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct TransactionIndexedError { + /// The original error that occurred. + pub error: Error, + /// The index of the transaction that failed. + pub transaction_index: usize, +} + +impl TransactionIndexedError { + /// Create a new `TransactionIndexedError` with the given error and transaction index. + pub fn new(error: Error, transaction_index: usize) -> Self { + Self { + error, + transaction_index, + } + } + + /// Get the transaction index. + pub fn transaction_index(&self) -> usize { + self.transaction_index + } + + /// Get a reference to the underlying error. + pub fn error(&self) -> &Error { + &self.error + } + + /// Convert into the underlying error. + pub fn into_error(self) -> Error { + self.error + } +} + +impl fmt::Display for TransactionIndexedError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "transaction {} failed: {}", self.transaction_index, self.error) + } +} + +impl core::error::Error for TransactionIndexedError { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + Some(&self.error) + } +} diff --git a/crates/handler/src/api.rs b/crates/handler/src/api.rs index 020d03e4df..bab74cab28 100644 --- a/crates/handler/src/api.rs +++ b/crates/handler/src/api.rs @@ -4,7 +4,7 @@ use crate::{ use context::{ result::{ EVMError, ExecResultAndState, ExecutionResult, HaltReason, InvalidTransaction, - ResultAndState, ResultVecAndState, + ResultAndState, ResultVecAndState, TransactionIndexedError, }, Block, ContextSetters, ContextTr, Database, Evm, JournalTr, Transaction, }; @@ -79,19 +79,18 @@ pub trait ExecuteEvm { /// /// # Outcome of Error /// - /// If any transaction fails, the journal is finalized and the last error is returned. - /// - /// TODO add tx index to the error. + /// If any transaction fails, the journal is finalized and the error is returned with the + /// transaction index that failed. #[inline] fn transact_many( &mut self, txs: impl Iterator, - ) -> Result, Self::Error> { + ) -> Result, TransactionIndexedError> { let mut outputs = Vec::new(); - for tx in txs { + for (index, tx) in txs.enumerate() { outputs.push(self.transact_one(tx).inspect_err(|_| { let _ = self.finalize(); - })?); + }).map_err(|error| TransactionIndexedError::new(error, index))?); } Ok(outputs) } @@ -103,7 +102,7 @@ pub trait ExecuteEvm { fn transact_many_finalize( &mut self, txs: impl Iterator, - ) -> Result, Self::Error> { + ) -> Result, TransactionIndexedError> { // on error transact_multi will clear the journal let result = self.transact_many(txs)?; let state = self.finalize(); @@ -145,7 +144,7 @@ pub trait ExecuteCommitEvm: ExecuteEvm { fn transact_many_commit( &mut self, txs: impl Iterator, - ) -> Result, Self::Error> { + ) -> Result, TransactionIndexedError> { let outputs = self.transact_many(txs)?; self.commit_inner(); Ok(outputs) diff --git a/crates/handler/src/validation.rs b/crates/handler/src/validation.rs index 737f3111b0..3ae6fe4cab 100644 --- a/crates/handler/src/validation.rs +++ b/crates/handler/src/validation.rs @@ -254,14 +254,15 @@ pub fn validate_initial_tx_gas( #[cfg(test)] mod tests { - use crate::{ExecuteCommitEvm, MainBuilder, MainContext}; + use crate::{api::ExecuteEvm, ExecuteCommitEvm, MainBuilder, MainContext}; use bytecode::opcode; use context::{ result::{EVMError, ExecutionResult, HaltReason, InvalidTransaction, Output}, - Context, TxEnv, + Context, ContextTr, TxEnv, }; use database::{CacheDB, EmptyDB}; - use primitives::{address, eip3860, eip7907, hardfork::SpecId, Bytes, TxKind}; + use primitives::{address, eip3860, eip7907, hardfork::SpecId, Bytes, TxKind, U256, B256}; + use state::{AccountInfo, Bytecode}; fn deploy_contract( bytecode: Bytes, @@ -554,4 +555,133 @@ mod tests { _ => panic!("execution result is not Success"), } } + + #[test] + fn test_transact_many_with_transaction_index_error() { + use context::result::TransactionIndexedError; + + let ctx = Context::mainnet().with_db(CacheDB::::default()); + let mut evm = ctx.build_mainnet(); + + // Create a transaction that will fail (invalid gas limit) + let invalid_tx = TxEnv::builder() + .gas_limit(0) // This will cause a validation error + .build() + .unwrap(); + + // Create a valid transaction + let valid_tx = TxEnv::builder() + .gas_limit(100000) + .build() + .unwrap(); + + // Test that the first transaction fails with index 0 + let result = evm.transact_many([invalid_tx.clone()].into_iter()); + assert!(matches!( + result, + Err(TransactionIndexedError { transaction_index: 0, .. }) + )); + + // Test that the second transaction fails with index 1 + let result = evm.transact_many([valid_tx, invalid_tx].into_iter()); + assert!(matches!( + result, + Err(TransactionIndexedError { transaction_index: 1, .. }) + )); + } + + #[test] + fn test_transact_many_success() { + use primitives::{address, U256}; + + let ctx = Context::mainnet().with_db(CacheDB::::default()); + let mut evm = ctx.build_mainnet(); + + // Add balance to the caller account + let caller = address!("0x0000000000000000000000000000000000000001"); + evm.db_mut().insert_account_info(caller, AccountInfo::new( + U256::from(1000000000000000000u64), + 0, + B256::ZERO, + Bytecode::new() + )); + + // Create valid transactions with proper data + let tx1 = TxEnv::builder() + .caller(caller) + .gas_limit(100000) + .gas_price(20_000_000_000u128) + .nonce(0) + .build() + .unwrap(); + + let tx2 = TxEnv::builder() + .caller(caller) + .gas_limit(100000) + .gas_price(20_000_000_000u128) + .nonce(1) + .build() + .unwrap(); + + // Test that all transactions succeed + let result = evm.transact_many([tx1, tx2].into_iter()); + if let Err(e) = &result { + println!("Error: {:?}", e); + } + assert!(result.is_ok()); + let outputs = result.unwrap(); + assert_eq!(outputs.len(), 2); + } + + #[test] + fn test_transact_many_finalize_with_error() { + use context::result::TransactionIndexedError; + + let ctx = Context::mainnet().with_db(CacheDB::::default()); + let mut evm = ctx.build_mainnet(); + + // Create transactions where the second one fails + let valid_tx = TxEnv::builder() + .gas_limit(100000) + .build() + .unwrap(); + + let invalid_tx = TxEnv::builder() + .gas_limit(0) // This will cause a validation error + .build() + .unwrap(); + + // Test that transact_many_finalize returns the error with correct index + let result = evm.transact_many_finalize([valid_tx, invalid_tx].into_iter()); + assert!(matches!( + result, + Err(TransactionIndexedError { transaction_index: 1, .. }) + )); + } + + #[test] + fn test_transact_many_commit_with_error() { + use context::result::TransactionIndexedError; + + let ctx = Context::mainnet().with_db(CacheDB::::default()); + let mut evm = ctx.build_mainnet(); + + // Create transactions where the first one fails + let invalid_tx = TxEnv::builder() + .gas_limit(0) // This will cause a validation error + .build() + .unwrap(); + + let valid_tx = TxEnv::builder() + .gas_limit(100000) + .build() + .unwrap(); + + // Test that transact_many_commit returns the error with correct index + let result = evm.transact_many_commit([invalid_tx, valid_tx].into_iter()); + assert!(matches!( + result, + Err(TransactionIndexedError { transaction_index: 0, .. }) + )); + } } From b75c74438887e89a2abfc170d06d8713e4155f31 Mon Sep 17 00:00:00 2001 From: crStiv Date: Fri, 19 Sep 2025 21:47:47 +0000 Subject: [PATCH 2/2] fix: fmt and clippy --- crates/context/interface/src/result.rs | 13 +++--- crates/handler/src/api.rs | 16 +++++-- crates/handler/src/handler.rs | 2 +- crates/handler/src/validation.rs | 63 ++++++++++++++------------ 4 files changed, 54 insertions(+), 40 deletions(-) diff --git a/crates/context/interface/src/result.rs b/crates/context/interface/src/result.rs index 71fd30a548..1a79006a60 100644 --- a/crates/context/interface/src/result.rs +++ b/crates/context/interface/src/result.rs @@ -655,6 +655,7 @@ pub struct TransactionIndexedError { impl TransactionIndexedError { /// Create a new `TransactionIndexedError` with the given error and transaction index. + #[must_use] pub fn new(error: Error, transaction_index: usize) -> Self { Self { error, @@ -662,17 +663,13 @@ impl TransactionIndexedError { } } - /// Get the transaction index. - pub fn transaction_index(&self) -> usize { - self.transaction_index - } - /// Get a reference to the underlying error. pub fn error(&self) -> &Error { &self.error } /// Convert into the underlying error. + #[must_use] pub fn into_error(self) -> Error { self.error } @@ -680,7 +677,11 @@ impl TransactionIndexedError { impl fmt::Display for TransactionIndexedError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "transaction {} failed: {}", self.transaction_index, self.error) + write!( + f, + "transaction {} failed: {}", + self.transaction_index, self.error + ) } } diff --git a/crates/handler/src/api.rs b/crates/handler/src/api.rs index bab74cab28..a2faacf186 100644 --- a/crates/handler/src/api.rs +++ b/crates/handler/src/api.rs @@ -13,6 +13,10 @@ use interpreter::{interpreter::EthInterpreter, InterpreterResult}; use state::EvmState; use std::vec::Vec; +/// Type alias for the result of transact_many_finalize to reduce type complexity. +type TransactManyFinalizeResult = + Result, TransactionIndexedError>; + /// Execute EVM transactions. Main trait for transaction execution. pub trait ExecuteEvm { /// Output of transaction execution. @@ -88,9 +92,13 @@ pub trait ExecuteEvm { ) -> Result, TransactionIndexedError> { let mut outputs = Vec::new(); for (index, tx) in txs.enumerate() { - outputs.push(self.transact_one(tx).inspect_err(|_| { - let _ = self.finalize(); - }).map_err(|error| TransactionIndexedError::new(error, index))?); + outputs.push( + self.transact_one(tx) + .inspect_err(|_| { + let _ = self.finalize(); + }) + .map_err(|error| TransactionIndexedError::new(error, index))?, + ); } Ok(outputs) } @@ -102,7 +110,7 @@ pub trait ExecuteEvm { fn transact_many_finalize( &mut self, txs: impl Iterator, - ) -> Result, TransactionIndexedError> { + ) -> TransactManyFinalizeResult { // on error transact_multi will clear the journal let result = self.transact_many(txs)?; let state = self.finalize(); diff --git a/crates/handler/src/handler.rs b/crates/handler/src/handler.rs index e79141524a..8b1d34f676 100644 --- a/crates/handler/src/handler.rs +++ b/crates/handler/src/handler.rs @@ -457,7 +457,7 @@ pub trait Handler { match core::mem::replace(evm.ctx().error(), Ok(())) { Err(ContextError::Db(e)) => return Err(e.into()), Err(ContextError::Custom(e)) => return Err(Self::Error::from_string(e)), - Ok(_) => (), + Ok(()) => (), } let exec_result = post_execution::output(evm.ctx(), result); diff --git a/crates/handler/src/validation.rs b/crates/handler/src/validation.rs index 3ae6fe4cab..feb072f0c9 100644 --- a/crates/handler/src/validation.rs +++ b/crates/handler/src/validation.rs @@ -261,7 +261,7 @@ mod tests { Context, ContextTr, TxEnv, }; use database::{CacheDB, EmptyDB}; - use primitives::{address, eip3860, eip7907, hardfork::SpecId, Bytes, TxKind, U256, B256}; + use primitives::{address, eip3860, eip7907, hardfork::SpecId, Bytes, TxKind, B256}; use state::{AccountInfo, Bytecode}; fn deploy_contract( @@ -559,7 +559,7 @@ mod tests { #[test] fn test_transact_many_with_transaction_index_error() { use context::result::TransactionIndexedError; - + let ctx = Context::mainnet().with_db(CacheDB::::default()); let mut evm = ctx.build_mainnet(); @@ -570,41 +570,47 @@ mod tests { .unwrap(); // Create a valid transaction - let valid_tx = TxEnv::builder() - .gas_limit(100000) - .build() - .unwrap(); + let valid_tx = TxEnv::builder().gas_limit(100000).build().unwrap(); // Test that the first transaction fails with index 0 let result = evm.transact_many([invalid_tx.clone()].into_iter()); assert!(matches!( result, - Err(TransactionIndexedError { transaction_index: 0, .. }) + Err(TransactionIndexedError { + transaction_index: 0, + .. + }) )); // Test that the second transaction fails with index 1 let result = evm.transact_many([valid_tx, invalid_tx].into_iter()); assert!(matches!( result, - Err(TransactionIndexedError { transaction_index: 1, .. }) + Err(TransactionIndexedError { + transaction_index: 1, + .. + }) )); } #[test] fn test_transact_many_success() { use primitives::{address, U256}; - + let ctx = Context::mainnet().with_db(CacheDB::::default()); let mut evm = ctx.build_mainnet(); // Add balance to the caller account let caller = address!("0x0000000000000000000000000000000000000001"); - evm.db_mut().insert_account_info(caller, AccountInfo::new( - U256::from(1000000000000000000u64), - 0, - B256::ZERO, - Bytecode::new() - )); + evm.db_mut().insert_account_info( + caller, + AccountInfo::new( + U256::from(1000000000000000000u64), + 0, + B256::ZERO, + Bytecode::new(), + ), + ); // Create valid transactions with proper data let tx1 = TxEnv::builder() @@ -628,23 +634,19 @@ mod tests { if let Err(e) = &result { println!("Error: {:?}", e); } - assert!(result.is_ok()); - let outputs = result.unwrap(); + let outputs = result.expect("All transactions should succeed"); assert_eq!(outputs.len(), 2); } #[test] fn test_transact_many_finalize_with_error() { use context::result::TransactionIndexedError; - + let ctx = Context::mainnet().with_db(CacheDB::::default()); let mut evm = ctx.build_mainnet(); // Create transactions where the second one fails - let valid_tx = TxEnv::builder() - .gas_limit(100000) - .build() - .unwrap(); + let valid_tx = TxEnv::builder().gas_limit(100000).build().unwrap(); let invalid_tx = TxEnv::builder() .gas_limit(0) // This will cause a validation error @@ -655,14 +657,17 @@ mod tests { let result = evm.transact_many_finalize([valid_tx, invalid_tx].into_iter()); assert!(matches!( result, - Err(TransactionIndexedError { transaction_index: 1, .. }) + Err(TransactionIndexedError { + transaction_index: 1, + .. + }) )); } #[test] fn test_transact_many_commit_with_error() { use context::result::TransactionIndexedError; - + let ctx = Context::mainnet().with_db(CacheDB::::default()); let mut evm = ctx.build_mainnet(); @@ -672,16 +677,16 @@ mod tests { .build() .unwrap(); - let valid_tx = TxEnv::builder() - .gas_limit(100000) - .build() - .unwrap(); + let valid_tx = TxEnv::builder().gas_limit(100000).build().unwrap(); // Test that transact_many_commit returns the error with correct index let result = evm.transact_many_commit([invalid_tx, valid_tx].into_iter()); assert!(matches!( result, - Err(TransactionIndexedError { transaction_index: 0, .. }) + Err(TransactionIndexedError { + transaction_index: 0, + .. + }) )); } }