feat: `alloy-evm` and new revm integration by klkvr · Pull Request #14021 · paradigmxyz/reth

klkvr · 2025-01-27T21:06:54Z

Integrates Evm trait moved to alloy-evm. Core Evm construction logic is now implemented via EvmFactory and ConfigureEvm implementations are just required to hold one.

mattsse

this is pretty cool,
love that this now doesn't require any interface changes

mattsse · 2025-01-28T13:31:07Z

crates/optimism/evm/src/lib.rs

this is okay because this type is a ZST anyway

gakonst · 2025-01-31T22:34:20Z

Cargo.toml

 alloy-chains = { version = "0.1.32", default-features = false }
 alloy-dyn-abi = "0.8.15"
 alloy-eip2124 = { version = "0.1.0", default-features = false }
+alloy-evm = { path = "../evm/crates/evm" }


shoudl be able to use from git now

emhane · 2025-02-04T12:01:49Z

crates/evm/src/lib.rs

-/// Helper trait to bound [`revm::Database::Error`] with common requirements.
-pub trait Database: revm::Database<Error: core::error::Error + Send + Sync + 'static> {}
-impl<T> Database for T where T: revm::Database<Error: core::error::Error + Send + Sync + 'static> {}
+pub use alloy_evm::{Database, Evm, EvmEnv, EvmError, InvalidTxError};


wondering if it's actually more helpful to not re-export all of these so libraries update their deps to point to alloy directly instead

emhane · 2025-02-04T12:06:37Z

crates/evm/src/lib.rs

+        &self,
+        db: DB,
+        header: &Self::Header,
+    ) -> <Self::EvmFactory as EvmFactory<EvmEnv<Self::Spec>>>::Evm<'_, DB, ()> {


could be helpful to create a type alias as adapter for this and export it from crate root to improve devx with the trait, smthg like

pub type EvmTy<'a, T, DB> = <<T as ConfigureEvm>::EvmFactory<EvmEnv<T::Spec>>>::Evm<'a, DB, ()>;

emhane · 2025-02-04T12:08:33Z

crates/optimism/evm/src/lib.rs

 use alloc::sync::Arc;
 use alloy_consensus::{BlockHeader, Header};
 use alloy_eips::eip7840::BlobParams;
+use alloy_op_evm::OpEvmFactory;


is the plan to move this to op-alloy?

emhane · 2025-02-04T12:10:38Z

crates/rpc/rpc-eth-api/src/helpers/call.rs

            ExecutionResult::Halt { reason, gas_used } => {
                let error =
-                    Some(RpcInvalidTransactionError::halt(reason, tx_env.gas_limit()).to_string());
+                    Some(Self::Error::from_evm_halt(reason, tx_env.gas_limit()).to_string());


nice application of this pattern, good eye

emhane · 2025-02-04T12:13:41Z

crates/rpc/rpc-eth-api/src/helpers/estimate.rs

+        ExecutionResult::Revert { .. } | ExecutionResult::Halt { .. } => {
+            // We know that transaction succeeded with a higher gas limit before, so any failure
+            // means that we need to increase it.


do we want to add some trace level logging here right away to differentiate the two in debugging? does the migration to alloy not change so much code the we can still consider the sigp audit of the reth code to apply to the code in alloy?

mattsse

gg

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de> Co-authored-by: rakita <rakita@users.noreply.github.com>

mattsse reviewed Jan 28, 2025

View reviewed changes

crates/optimism/evm/src/lib.rs Outdated

Copy link

Collaborator

mattsse Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is okay because this type is a ZST anyway

mattsse mentioned this pull request Jan 29, 2025

Add abstraction around Evm in ConfigureEvm #8672

Closed

klkvr force-pushed the klkvr/alloy-evm branch from ecfc143 to 8cbe3c0 Compare January 30, 2025 18:11

klkvr changed the base branch from main to klkvr/generic-halt-reason January 30, 2025 18:13

Base automatically changed from klkvr/generic-halt-reason to main January 30, 2025 21:03

gakonst reviewed Jan 31, 2025

View reviewed changes

emhane added A-execution Related to the Execution and EVM A-dependencies Pull requests or issues that are about dependencies labels Feb 4, 2025

emhane reviewed Feb 4, 2025

View reviewed changes

klkvr mentioned this pull request Feb 7, 2025

feat: add error traits and ATs alloy-rs/evm#11

Merged

3 tasks

klkvr force-pushed the klkvr/alloy-evm branch 6 times, most recently from 25eb1b0 to 7e97984 Compare February 12, 2025 18:03

klkvr force-pushed the klkvr/alloy-evm branch from 837ba71 to 43d42e8 Compare February 14, 2025 14:53

klkvr mentioned this pull request Feb 14, 2025

[wip] feat: integrate new revm #14115

Merged

klkvr added 7 commits February 17, 2025 19:45

wip

827b821

wip

5d753dc

use git deps

25322e9

fix conflicts

e75ade7

bump alloy-evm

dd26784

rm OpEvm

3767a67

clippy

270ef66

klkvr force-pushed the klkvr/alloy-evm branch from 43d42e8 to 270ef66 Compare February 17, 2025 15:47

mattsse approved these changes Feb 17, 2025

View reviewed changes

klkvr added 2 commits February 17, 2025 21:18

crunchy

bb867e0

fix deny

ab9d7a7

mattsse marked this pull request as ready for review February 17, 2025 19:02

mattsse requested review from Rjected, fgimenez, joshieDo, rakita and shekhirin as code owners February 17, 2025 19:02

[wip] feat: integrate new revm (#14115)

9bf7709

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de> Co-authored-by: rakita <rakita@users.noreply.github.com>

klkvr requested review from onbjerg and rkrasiuk as code owners February 17, 2025 19:35

klkvr enabled auto-merge February 17, 2025 19:35

klkvr changed the title ~~[wip] feat: alloy-evm integration~~ feat: alloy-evm integration Feb 17, 2025

klkvr changed the title ~~feat: alloy-evm integration~~ feat: alloy-evm and new revm integration Feb 17, 2025

klkvr disabled auto-merge February 17, 2025 19:39

bump alloy-evm

986dbcc

klkvr enabled auto-merge February 17, 2025 19:43

klkvr added this pull request to the merge queue Feb 17, 2025

Merged via the queue into main with commit 336c3d1 Feb 17, 2025
43 checks passed

klkvr deleted the klkvr/alloy-evm branch February 17, 2025 20:15

mattsse mentioned this pull request Feb 18, 2025

fix: add clear tx l1 cost back #14564

Merged

mattsse mentioned this pull request Apr 9, 2025

Add Arsenii Kulikov protocolguild/documentation#343

Merged

futreall mentioned this pull request Oct 15, 2025

fix: add revm-state to dev-dependencies of chain-state crate #19044

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `alloy-evm` and new revm integration#14021

feat: `alloy-evm` and new revm integration#14021
klkvr merged 11 commits intomainfrom
klkvr/alloy-evm

klkvr commented Jan 27, 2025

Uh oh!

mattsse left a comment

Uh oh!

mattsse Jan 28, 2025

Uh oh!

gakonst Jan 31, 2025

Uh oh!

emhane Feb 4, 2025

Uh oh!

emhane Feb 4, 2025

Uh oh!

emhane Feb 4, 2025

Uh oh!

emhane Feb 4, 2025

Uh oh!

emhane Feb 4, 2025

Uh oh!

mattsse left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

klkvr commented Jan 27, 2025

Uh oh!

mattsse left a comment

Choose a reason for hiding this comment

Uh oh!

mattsse Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

gakonst Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

emhane Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

emhane Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

emhane Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

emhane Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

emhane Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

mattsse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants