feat: simplify Transaction trait (#1959)

* chore: relax some impl and exec_with_tx * add helper created_address * add error to database error * ext journal with state * fix: handle inspector final return * call final_return * expose precompile addresses to Journal * precompile addresses fn for journal * feat: align Block trait * fix mul * feat: simplify transaction trait * wip impl for TxEnv * compiled simlified tx trait * tweek * add perf on access list load * rm debug print * op enabled. Rm Error from Tx trait * fix tests * rn old prague tests
bluealloy · Jan 3, 2025 · 12a790f · 12a790f
1 parent 223a2d6
commit 12a790f
Show file tree

Hide file tree

Showing 59 changed files with 658 additions and 1,047 deletions.
diff --git a/.github/workflows/ethereum-tests.yml b/.github/workflows/ethereum-tests.yml
@@ -51,7 +51,7 @@ jobs:
             ethtests/EIPTests/StateTests/stEOF \
             tests/eof_suite/eest/state_tests \
             tests/eof_suite/evmone/state_tests \
-            tests/prague_suite/state_tests
+      #      tests/prague_suite/state_tests
       # - name: Run EOF validation tests
       #   run: |
       #     cross run --target ${{matrix.target}} --profile ${{ matrix.profile }} -p revme -- eof-validation \

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -4,7 +4,6 @@ members = [
     "crates/revm",
     "crates/primitives",
     "crates/interpreter",
-    "crates/inspector",
     "crates/precompile",
     "crates/database",
     "crates/database/interface",
@@ -21,6 +20,7 @@ members = [
 
     # variants
     "crates/optimism",
+    "crates/inspector",
 
     # utility
     "crates/statetest-types",

diff --git a/bins/revme/src/cmd/bench/analysis.rs b/bins/revme/src/cmd/bench/analysis.rs
@@ -17,7 +17,7 @@ pub fn run() {
         .modify_tx_chained(|tx| {
             // Execution globals block hash/gas_limit/coinbase/timestamp..
             tx.caller = address!("1000000000000000000000000000000000000000");
-            tx.transact_to = TxKind::Call(address!("0000000000000000000000000000000000000000"));
+            tx.kind = TxKind::Call(address!("0000000000000000000000000000000000000000"));
             //evm.env.tx.data = Bytes::from(hex::decode("30627b7c").unwrap());
             tx.data = bytes!("8035F0CE");
         });

diff --git a/bins/revme/src/cmd/bench/burntpix.rs b/bins/revme/src/cmd/bench/burntpix.rs
@@ -39,7 +39,7 @@ pub fn run() {
 
     let context = Context::builder().with_db(db).modify_tx_chained(|tx| {
         tx.caller = address!("1000000000000000000000000000000000000000");
-        tx.transact_to = TxKind::Call(BURNTPIX_MAIN_ADDRESS);
+        tx.kind = TxKind::Call(BURNTPIX_MAIN_ADDRESS);
         tx.data = run_call_data.clone().into();
         tx.gas_limit = u64::MAX;
     });

diff --git a/bins/revme/src/cmd/bench/snailtracer.rs b/bins/revme/src/cmd/bench/snailtracer.rs
@@ -12,7 +12,7 @@ pub fn simple_example(bytecode: Bytecode) {
         .modify_tx_chained(|tx| {
             // Execution globals block hash/gas_limit/coinbase/timestamp..
             tx.caller = address!("1000000000000000000000000000000000000000");
-            tx.transact_to = TxKind::Call(address!("0000000000000000000000000000000000000000"));
+            tx.kind = TxKind::Call(address!("0000000000000000000000000000000000000000"));
             tx.data = bytes!("30627b7c");
             tx.gas_limit = 1_000_000_000;
         });

diff --git a/bins/revme/src/cmd/bench/transfer.rs b/bins/revme/src/cmd/bench/transfer.rs
@@ -15,7 +15,7 @@ pub fn run() {
                 .parse()
                 .unwrap();
             tx.value = U256::from(10);
-            tx.transact_to = TxKind::Call(
+            tx.kind = TxKind::Call(
                 "0x0000000000000000000000000000000000000000"
                     .parse()
                     .unwrap(),

diff --git a/bins/revme/src/cmd/evmrunner.rs b/bins/revme/src/cmd/evmrunner.rs
@@ -89,7 +89,7 @@ impl Cmd {
         let mut evm = MainEvm::new(
             Context::builder().with_db(db).modify_tx_chained(|tx| {
                 tx.caller = CALLER;
-                tx.transact_to = TxKind::Call(Address::ZERO);
+                tx.kind = TxKind::Call(Address::ZERO);
                 tx.data = input;
                 tx.nonce = nonce;
             }),

diff --git a/bins/revme/src/cmd/statetest/runner.rs b/bins/revme/src/cmd/statetest/runner.rs
@@ -19,7 +19,7 @@ use revm::{
     database_interface::EmptyDB,
     handler::EthHandler,
     primitives::{keccak256, Bytes, TxKind, B256},
-    specification::{eip7702::AuthorizationList, hardfork::SpecId},
+    specification::hardfork::SpecId,
     Context, DatabaseCommit, EvmCommit, MainEvm,
 };
 use serde_json::json;
@@ -329,11 +329,20 @@ pub fn execute_test_suite(
             .transaction
             .gas_price
             .or(unit.transaction.max_fee_per_gas)
-            .unwrap_or_default();
-        tx.gas_priority_fee = unit.transaction.max_priority_fee_per_gas;
+            .unwrap_or_default()
+            .try_into()
+            .unwrap_or(u128::MAX);
+        tx.gas_priority_fee = unit
+            .transaction
+            .max_priority_fee_per_gas
+            .map(|b| u128::try_from(b).expect("max priority fee less than u128::MAX"));
         // EIP-4844
         tx.blob_hashes = unit.transaction.blob_versioned_hashes.clone();
-        tx.max_fee_per_blob_gas = unit.transaction.max_fee_per_blob_gas;
+        tx.max_fee_per_blob_gas = unit
+            .transaction
+            .max_fee_per_blob_gas
+            .map(|b| u128::try_from(b).expect("max fee less than u128::MAX"))
+            .unwrap_or(u128::MAX);
 
         // Post and execution
         for (spec_name, tests) in unit.post {
@@ -366,7 +375,7 @@ pub fn execute_test_suite(
                     }
                 };
 
-                tx.tx_type = tx_type;
+                tx.tx_type = tx_type as u8;
 
                 tx.gas_limit = unit.transaction.gas_limit[test.indexes.gas].saturating_to();
 
@@ -385,26 +394,26 @@ pub fn execute_test_suite(
                     .access_lists
                     .get(test.indexes.data)
                     .and_then(Option::as_deref)
-                    .cloned()
-                    .unwrap_or_default()
-                    .into();
+                    .map(|access_list| {
+                        access_list
+                            .iter()
+                            .map(|item| (item.address, item.storage_keys.clone()))
+                            .collect::<Vec<_>>()
+                    })
+                    .unwrap_or_default();
 
                 tx.authorization_list = unit
                     .transaction
                     .authorization_list
-                    .as_ref()
-                    .map(|auth_list| {
-                        AuthorizationList::Recovered(
-                            auth_list.iter().map(|auth| auth.into_recovered()).collect(),
-                        )
-                    })
+                    .clone()
+                    .map(|auth_list| auth_list.into_iter().map(Into::into).collect::<Vec<_>>())
                     .unwrap_or_default();
 
                 let to = match unit.transaction.to {
                     Some(add) => TxKind::Call(add),
                     None => TxKind::Create,
                 };
-                tx.transact_to = to;
+                tx.kind = to;
 
                 let mut cache = cache_state.clone();
                 cache.set_state_clear_flag(cfg.spec.is_enabled_in(SpecId::SPURIOUS_DRAGON));

diff --git a/crates/context/interface/src/context.rs b/crates/context/interface/src/context.rs
@@ -0,0 +1,12 @@
+/// Some actions on top of context with just Getter traits would require borrowing the context
+/// with a both mutable and immutable reference.
+///
+/// To allow doing this action more efficiently, we introduce a new trait that does this directly.
+///
+/// Used for loading access list and applying EIP-7702 authorization list.
+pub trait PerformantContextAccess {
+    type Error;
+
+    /// Load access list
+    fn load_access_list(&mut self) -> Result<(), Self::Error>;
+}
diff --git a/crates/context/interface/src/lib.rs b/crates/context/interface/src/lib.rs
@@ -7,6 +7,7 @@ extern crate alloc as std;
 
 pub mod block;
 pub mod cfg;
+pub mod context;
 pub mod errors;
 pub mod host;
 pub mod journaled_state;
@@ -15,6 +16,7 @@ pub mod transaction;
 
 pub use block::{Block, BlockGetter};
 pub use cfg::{Cfg, CfgGetter, CreateScheme, TransactTo};
+pub use context::PerformantContextAccess;
 pub use database_interface::{DBErrorMarker, Database, DatabaseGetter};
 pub use errors::ErrorGetter;
 pub use journaled_state::{Journal, JournalDBError, JournalGetter};

diff --git a/crates/context/interface/src/result.rs b/crates/context/interface/src/result.rs
@@ -2,7 +2,6 @@ use crate::transaction::TransactionError;
 use core::fmt::{self, Debug};
 use database_interface::DBErrorMarker;
 use primitives::{Address, Bytes, Log, U256};
-use specification::eip7702::InvalidAuthorization;
 use state::EvmState;
 use std::{boxed::Box, string::String, vec::Vec};
 
@@ -318,8 +317,6 @@ pub enum InvalidTransaction {
     AuthorizationListInvalidFields,
     /// Empty Authorization List is not allowed.
     EmptyAuthorizationList,
-    /// Invalid EIP-7702 Authorization List
-    InvalidAuthorizationList(InvalidAuthorization),
     /// EIP-2930 is not supported.
     Eip2930NotSupported,
     /// EIP-1559 is not supported.
@@ -332,12 +329,6 @@ pub enum InvalidTransaction {
 
 impl TransactionError for InvalidTransaction {}
 
-impl From<InvalidAuthorization> for InvalidTransaction {
-    fn from(value: InvalidAuthorization) -> Self {
-        Self::InvalidAuthorizationList(value)
-    }
-}
-
 impl core::error::Error for InvalidTransaction {}
 
 impl fmt::Display for InvalidTransaction {
@@ -403,7 +394,6 @@ impl fmt::Display for InvalidTransaction {
             Self::Eip1559NotSupported => write!(f, "Eip1559 is not supported"),
             Self::Eip4844NotSupported => write!(f, "Eip4844 is not supported"),
             Self::Eip7702NotSupported => write!(f, "Eip7702 is not supported"),
-            Self::InvalidAuthorizationList(i) => fmt::Display::fmt(i, f),
         }
     }
 }