Skip to content

intermediary(revm bump): re-enable Anvil tests, remove duplicate LogCollector, entire codebase builds #10412

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

Merged
merged 23 commits into from
Apr 30, 2025

Conversation

zerosnacks
Copy link
Member

@zerosnacks zerosnacks commented Apr 29, 2025

Solution

Related: #10183

Temporarily removes previous FoundryHandler / FoundryEvm implementation in favor of alloy-evm's

In a follow-up ticket we will make Foundry use the EitherEvm

Tests are still expected to fail with a panic

  • Re-enable Anvil tests
  • Make inspectors generic for dual use in Anvil, Foundry
  • Codebase now builds in its entirety
  • Fix issues raised by Clippy

@zerosnacks zerosnacks changed the title temp(revm bump): re-enable Anvil tests, remove duplicate LogCollector, entire codebase builds intermediary(revm bump): re-enable Anvil tests, remove duplicate LogCollector, entire codebase builds Apr 29, 2025
@zerosnacks zerosnacks added the L-ignore Log: ignore PR in changelog label Apr 29, 2025
@zerosnacks zerosnacks requested a review from yash-atreya April 29, 2025 15:02
@zerosnacks zerosnacks self-assigned this Apr 29, 2025
Copy link
Member

@yash-atreya yash-atreya left a comment

Choose a reason for hiding this comment

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

Tested anvil and forge locally.

All Anvil tests are passing.

Re: forge tests.

It seems the panic has gone. We can verify this by running the test_bind_json on the parent PR (panics) VS this one (doesn't panic but still fails).

This leads me to believe that the issue was with the FoundryEvm setup and configuration. Maybe the Handler implementation.

Copy link
Member

@yash-atreya yash-atreya left a comment

Choose a reason for hiding this comment

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

Re: Super large diff

It is from uncommenting the tests that were commented out/ignored in the parent PR and also clippy nits.

TL;DR

This PR just removes the Handler impl and replaces the FoundryEvm with alloy_evm::EthEvm

lgtm!

@zerosnacks zerosnacks marked this pull request as ready for review April 30, 2025 07:58
@zerosnacks zerosnacks merged commit c7d41af into zerosnacks/revm-bump-2 Apr 30, 2025
8 of 22 checks passed
@zerosnacks zerosnacks deleted the zerosnacks/temp-evm-refactor branch April 30, 2025 07:58
@github-project-automation github-project-automation bot moved this to Done in Foundry Apr 30, 2025
@grandizzy grandizzy moved this from Done to Completed in Foundry May 13, 2025
zerosnacks added a commit that referenced this pull request May 19, 2025
* start fixing imports

* continue fixing imports

* continue fixing imports

* continue fixing imports

* add alloy-evm

* fix known good changes

* more known good fixes

* more known good fixes

* more known good fixes

* more known good fixes, unclear how OptimismFields should be ported

* start introducing crate::Env

* continue introducing crate::Env

* fix cow types

* fix type

* add journaledstate types, fix env types

* fix JournaledState = JournalInner<JournalEntry>

* fix types

* fix merge type

* add odyssey precompile

* continue fixing type issues, handler abstraction

* start working on create2 handler

* revert to type instead of struct, investigating handlers

* comment out accesslistinspector for now, needs to be addressed

* fix imports, minor fixes

* imports, minor fixes, there is no equivalent of AuthorizationList - requires slight refactor

* more interpreter type fixes

* continue type fixes

* fix inspectorext

* start porting inspectors

* start adding custom evm

* continue adding custom FoundryEvm

* impl traits for FoundryEvm

* restructure, move out of utils into evm, precompiles and future handlers

* clean up

* clean up

* improve docs

* scaffold handler

* evaluate how to add handles

* prefer EnvRef over EnvMut

* address feedback of owned env

* revert get_or_insert_map workaround

* avoid changing types, leave mut where previously, avoid unnecessary mut

* start layout out handler registry connected to evm

* get create2 from frame inputs

* start adding create2 handler

* continue create2handler

* wrap up create2 handler

* clean up

* continue fixing types

* generalize precompiles

* clean up

* tag inline

* fix imports

* start fixing cheatcode types

* use `env` on handler

* clean up

* temp revert

* odyssey precompile was deprecated

* refix cheatcode types

* clean up

* still facing issues with borrow-checker, double mut

* open questions around passing around env

* minor fix

* for now work around mutability limitations by limited cloning, unclear performance impact or whether it will work with cheatcode macros

* continue fixing types, still issues around cheatcodes, inspector

* bump revm

* bump deps

* minor type fixes

* bump foundry-fork-db to handle c-kzg build issue

* bump rust version

* utilize Host, ContextTr, JournalTr to avoid double mutable borrows

* temp revert

* temp revert

* restore handler, improve types

* refactor types

* restore types

* restore, clean up

* continue fixing types

* clean up

* continue fixing types

* revert journal env cloning, still issues around double borrows

* fix core types per conversation, use EnvMut<'_>

* fix types

* more progress for foundry-evm

* mutate outcome in place

* temp revert exec_create

* some progress with porting with_evm core loop

* remove redundant types

* context -> test_context in Cheatcodes config

* construct new handler, wrapping evm context, imports Handler trait

* temporarily comment out exec_create section to unblock

* add replacement of EnvWithHandlerCfg

* minor fixes

* continue fixing types

* continue fixing types

* continue fixing types

* continue fixing types

* continue types

* fix cached_env

* remove possibly incorrect handling of CreateOutcome on methods like do_eofcreate_end as outcome is now mutated in place

* add custom_printer from revm19, porting for compatibility

* cast: fix types

* verify: fix types

* forge + script: fix types

* anvil: start fixing types

* anvil: continue porting types

* anvil: continue porting types

* anvil: continue porting types

* anvil: continue porting types, small fix in foundry-evm

* use AnvilEvm

* stash optimism hardfork specifics for now

* temp mute anvil use in forge

* apply apparant fixes, test still failing

* clean up

* revert to replay

* apply possible nonce 0/1 fixes, committed to proceed

* disable nonce check in local_evm_env

* undo is_odyssey remove

* always spawn evm with handler

* replay() -> inspect_replay()

* modify macro, comment out anvil related cast tests for the time being

* reapply state depth = 1

* something like this?

* introduce outer block for early return

* print debugging

* clean up

* fix merge

* migrate: anvil to revm 21 (#10361)

* downgrade op-revm to 2.0.0 to resolve dep conflict

* op-revm 3.0 uses revm 22

* add `as_mut_dyn` to trait `MaybeFullDatabase` as we now require mut db_ref access (

* Revert "add `as_mut_dyn` to trait `MaybeFullDatabase` as we now require mut db_ref access ("

This reverts commit 84d11f1.

* fix: Inspector should be generic over CTX not DB

* fixes helpers: new_evm_with_inspector_* to use CTX generic

* fix: pass TxEnv to evm.transact

* fix: inspector inference in TransactionExecutor and build_access_list_with_state

* workaround: dup LogCollector to use with AnvilEvmContext

* coz FoundryEvmContext is not generic over DB, instead hardoded to dyn DatabaseExt

* fix tests

* fix traces test

* fix: use default kzg settings in blob validation

* reintroduce OptimismHardfork

* fix: disable nonce check if nonce is None

* fix!: load state tests by addressing breaking changes in state files

* BlockEnv Breaking change:
- most fields now use `u64` instead of `U64` / `U256`
- coinbase renamed to beneficiary
- best_block_number is `u64`, prev `U64`

* fix: access_list test by using evm.inspect_with_tx

* fix: replace evm.transact with evm.inspect_with_tx

* fix: make impl Inspector for AnvilInspector generic over CTX

* fix: clone inspector in TransactionExecutor to enable evm.inspect_commit

* fix: remove cloned inspector from TransactionExecutor

* feat(`anvil`): op support revm 21 (#10407)

* enable OpHardforks in NodeConfig

* feat: add is_optimism flag to foundry_evm::Env

* feat(`anvil`): set is_optimism in Backend

* feat(`anvil`): introducing EvmContext enum holding Eth and Op variants.

* adds OpEnv to foundry_evm_core

* feat: EitherEvm

* impl Evm for EitherEvm

* integrate EitherEvm into RPC and executor

*Map OpHaltReason and OpTransactionError

* rm old evm helpers

* feat(`foundry_evm`): add deposit tx parts field to Env

* fix(`anvil`): set deposit tx parts in tx executor and backend.inspect_tx

* nit

* docs EitherEvm

* nit

* refac: return TxEnv and Deposit parts separately

* nits

* nit

* make anvil result aliases more generic

* nit

* intermediary(`revm bump`): re-enable Anvil tests, remove duplicate `LogCollector`, entire codebase builds (#10412)

* temp refactor, still facing issue

* clean up

* clean up

* temp cleanup, can later be refd

* clean up, refactor stack.rs to apply ecx restore from cache to outside lamba

* fix

* clean up

* clean up

* avoid borrowing mutably for clarity

* use EthEvmContext directly

* FoundryEvmContext -> EthEvmContext

* continue

* fix tests

* fix inspectors

* codebase now builds entirely

* fix clippy lints

* remove duplicate LogCollector in Anvil

* fmt

* fix clippy

* fix doctests

* disable nonce checks on forks, enforce setting of tx.nonce on set_nonce

* fix: use `transact` from alloy-evm (#10417)

* Patch revm to fix interpreter panic

* bump revm

* fix eof test

* fix bytecode hash

* fix fixture

* fix fixture

* fix fixture

* chore: mv EitherEvm to foundry_evm (#10445)

mv EitherEvm to foundry_evm_core

* remove unused JournalTr

* restore formatting, avoid diff

* remove leftover comment re: optimism support

* fix displays_chained_error test

* fix doc test

* remove optimism todo leftover

* avoid direct field assignment, prefer *current.

* create2 handler register

* fix patch

* fix test_broadcast_raw_create2_deployer

* fix gas meter test

* correctly reset env.tx to cached env, cfg and block, ref https://github.com/foundry-rs/foundry/blob/a34f4c989b94f572497631ff5c85909d674c23a6/crates/evm/evm/src/inspectors/stack.rs#L640-L649

* exec_create

* revert test_GasMeter, assert exact gas used

* fix arbitrum test

* doc test fixes

* fix clippy warnings

* remove leftover comment

* fix assert_can_detect_unlinked_target_with_libraries, ref: bluealloy/revm@fc54dd0

* fix gas metering tests

* restore unintended .wrap_err changes, ref: https://github.com/search?q=repo%3Afoundry-rs%2Ffoundry%20wrap_err(%22EVM%20error%22)&type=code

* fix test_cheats_local_default

* add CC0-1.0 license exception, has been previously approved in Reth: https://github.com/paradigmxyz/reth/blob/adb8bdc70758558d6122e87d78d73cc0f12d4dbb/deny.toml#L48

* usize depth

* repin foundry-fork-db, this aligns the revm and alloy version back

* fix clippy, after usize depth change

* allow foundry-fork-db as git exception

* fix: EitherEvm should work over OpTransaction

* fix fmt

* Env::from_with_spec_id -> Env::new_with_spec_id

* bump clippy msrv to align with foundry.toml

* chore: avoid leaking Anvil specific optimism fields into evm/core (#10466)

* start sketching

* maybe ?

* some kind of conversion still required

* continue porting

* clean up types

* pass op transaction in directly

* fixes

* restore setting of enveloped_tx

* refactor anvil Env and reduce changes in tx processing

* fix: correctly set txtype when setting up TxEnv

* update last commits from master to be u64 compatible

* fix clippy lint

* revert clippy changes, make sure lint-foundry uses nightly clippy version

* apply tx_type if set, upgrading from legacy to eip2930 if access_list is present and tx type is legacy

* restore #[ret] macro that was removed unintendedly

* replace redundant Env::new_with_spec_id(..) with default

* allow passing is_optimism into Env constructor specific to Anvil

* extract environment configuration into init.rs to make configuring the environment less error prone

* remove redundant debug derive

---------

Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
Co-authored-by: Yash Atreya <44857776+yash-atreya@users.noreply.github.com>
Co-authored-by: grandizzy <grandizzy.the.egg@gmail.com>
zerosnacks added a commit that referenced this pull request May 19, 2025
…s to latest (#10454)

* restructure, move out of utils into evm, precompiles and future handlers

* clean up

* clean up

* improve docs

* scaffold handler

* evaluate how to add handles

* prefer EnvRef over EnvMut

* address feedback of owned env

* revert get_or_insert_map workaround

* avoid changing types, leave mut where previously, avoid unnecessary mut

* start layout out handler registry connected to evm

* get create2 from frame inputs

* start adding create2 handler

* continue create2handler

* wrap up create2 handler

* clean up

* continue fixing types

* generalize precompiles

* clean up

* tag inline

* fix imports

* start fixing cheatcode types

* use `env` on handler

* clean up

* temp revert

* odyssey precompile was deprecated

* refix cheatcode types

* clean up

* still facing issues with borrow-checker, double mut

* open questions around passing around env

* minor fix

* for now work around mutability limitations by limited cloning, unclear performance impact or whether it will work with cheatcode macros

* continue fixing types, still issues around cheatcodes, inspector

* bump revm

* bump deps

* minor type fixes

* bump foundry-fork-db to handle c-kzg build issue

* bump rust version

* utilize Host, ContextTr, JournalTr to avoid double mutable borrows

* temp revert

* temp revert

* restore handler, improve types

* refactor types

* restore types

* restore, clean up

* continue fixing types

* clean up

* continue fixing types

* revert journal env cloning, still issues around double borrows

* fix core types per conversation, use EnvMut<'_>

* fix types

* more progress for foundry-evm

* mutate outcome in place

* temp revert exec_create

* some progress with porting with_evm core loop

* remove redundant types

* context -> test_context in Cheatcodes config

* construct new handler, wrapping evm context, imports Handler trait

* temporarily comment out exec_create section to unblock

* add replacement of EnvWithHandlerCfg

* minor fixes

* continue fixing types

* continue fixing types

* continue fixing types

* continue fixing types

* continue types

* fix cached_env

* remove possibly incorrect handling of CreateOutcome on methods like do_eofcreate_end as outcome is now mutated in place

* add custom_printer from revm19, porting for compatibility

* cast: fix types

* verify: fix types

* forge + script: fix types

* anvil: start fixing types

* anvil: continue porting types

* anvil: continue porting types

* anvil: continue porting types

* anvil: continue porting types, small fix in foundry-evm

* use AnvilEvm

* stash optimism hardfork specifics for now

* temp mute anvil use in forge

* apply apparant fixes, test still failing

* clean up

* revert to replay

* apply possible nonce 0/1 fixes, committed to proceed

* disable nonce check in local_evm_env

* undo is_odyssey remove

* always spawn evm with handler

* replay() -> inspect_replay()

* modify macro, comment out anvil related cast tests for the time being

* reapply state depth = 1

* something like this?

* introduce outer block for early return

* print debugging

* clean up

* fix merge

* migrate: anvil to revm 21 (#10361)

* downgrade op-revm to 2.0.0 to resolve dep conflict

* op-revm 3.0 uses revm 22

* add `as_mut_dyn` to trait `MaybeFullDatabase` as we now require mut db_ref access (

* Revert "add `as_mut_dyn` to trait `MaybeFullDatabase` as we now require mut db_ref access ("

This reverts commit 84d11f1.

* fix: Inspector should be generic over CTX not DB

* fixes helpers: new_evm_with_inspector_* to use CTX generic

* fix: pass TxEnv to evm.transact

* fix: inspector inference in TransactionExecutor and build_access_list_with_state

* workaround: dup LogCollector to use with AnvilEvmContext

* coz FoundryEvmContext is not generic over DB, instead hardoded to dyn DatabaseExt

* fix tests

* fix traces test

* fix: use default kzg settings in blob validation

* reintroduce OptimismHardfork

* fix: disable nonce check if nonce is None

* fix!: load state tests by addressing breaking changes in state files

* BlockEnv Breaking change:
- most fields now use `u64` instead of `U64` / `U256`
- coinbase renamed to beneficiary
- best_block_number is `u64`, prev `U64`

* fix: access_list test by using evm.inspect_with_tx

* fix: replace evm.transact with evm.inspect_with_tx

* fix: make impl Inspector for AnvilInspector generic over CTX

* fix: clone inspector in TransactionExecutor to enable evm.inspect_commit

* fix: remove cloned inspector from TransactionExecutor

* feat(`anvil`): op support revm 21 (#10407)

* enable OpHardforks in NodeConfig

* feat: add is_optimism flag to foundry_evm::Env

* feat(`anvil`): set is_optimism in Backend

* feat(`anvil`): introducing EvmContext enum holding Eth and Op variants.

* adds OpEnv to foundry_evm_core

* feat: EitherEvm

* impl Evm for EitherEvm

* integrate EitherEvm into RPC and executor

*Map OpHaltReason and OpTransactionError

* rm old evm helpers

* feat(`foundry_evm`): add deposit tx parts field to Env

* fix(`anvil`): set deposit tx parts in tx executor and backend.inspect_tx

* nit

* docs EitherEvm

* nit

* refac: return TxEnv and Deposit parts separately

* nits

* nit

* make anvil result aliases more generic

* nit

* intermediary(`revm bump`): re-enable Anvil tests, remove duplicate `LogCollector`, entire codebase builds (#10412)

* temp refactor, still facing issue

* clean up

* clean up

* temp cleanup, can later be refd

* clean up, refactor stack.rs to apply ecx restore from cache to outside lamba

* fix

* clean up

* clean up

* avoid borrowing mutably for clarity

* use EthEvmContext directly

* FoundryEvmContext -> EthEvmContext

* continue

* fix tests

* fix inspectors

* codebase now builds entirely

* fix clippy lints

* remove duplicate LogCollector in Anvil

* fmt

* fix clippy

* fix doctests

* disable nonce checks on forks, enforce setting of tx.nonce on set_nonce

* fix: use `transact` from alloy-evm (#10417)

* Patch revm to fix interpreter panic

* bump revm

* fix eof test

* fix bytecode hash

* fix fixture

* fix fixture

* fix fixture

* chore: mv EitherEvm to foundry_evm (#10445)

mv EitherEvm to foundry_evm_core

* remove unused JournalTr

* restore formatting, avoid diff

* remove leftover comment re: optimism support

* fix displays_chained_error test

* fix doc test

* remove optimism todo leftover

* avoid direct field assignment, prefer *current.

* create2 handler register

* fix patch

* fix test_broadcast_raw_create2_deployer

* bump alloy and related deps

apply patches for block-explorers and compilers

* fix: common

* fix gas meter test

* fix

* fix: ConsoleFmt proc_macro

* more fixes

* fix: validate bool removal from abi_decode_*

* fix: use take_slice instead of take_slice_unchecked in Decoder

* fix more validate bool removal

* correctly reset env.tx to cached env, cfg and block, ref https://github.com/foundry-rs/foundry/blob/a34f4c989b94f572497631ff5c85909d674c23a6/crates/evm/evm/src/inspectors/stack.rs#L640-L649

* address more alloy-core 1.0 breaking changes

* fix anvil

* exec_create

* fix cast

* bump gcloudsdk in wallets

* fix(`cheatcodes`): rand workaround
Use ChaChaRng as temporary measure since proptest is on rand 8

* revert test_GasMeter, assert exact gas used

* fix arbitrum test

* address deprecations

* doc test fixes

* fix clippy warnings

* remove leftover comment

* fix assert_can_detect_unlinked_target_with_libraries, ref: bluealloy/revm@fc54dd0

* fix gas metering tests

* restore unintended .wrap_err changes, ref: https://github.com/search?q=repo%3Afoundry-rs%2Ffoundry%20wrap_err(%22EVM%20error%22)&type=code

* fix test_cheats_local_default

* add CC0-1.0 license exception, has been previously approved in Reth: https://github.com/paradigmxyz/reth/blob/adb8bdc70758558d6122e87d78d73cc0f12d4dbb/deny.toml#L48

* usize depth

* repin foundry-fork-db, this aligns the revm and alloy version back

* fix clippy, after usize depth change

* allow foundry-fork-db as git exception

* revm 23

* fix: EitherEvm should work over OpTransaction

* bump compilers and explorers

* fix fmt

* Env::from_with_spec_id -> Env::new_with_spec_id

* bump clippy msrv to align with foundry.toml

* chore: avoid leaking Anvil specific optimism fields into evm/core (#10466)

* start sketching

* maybe ?

* some kind of conversion still required

* continue porting

* clean up types

* pass op transaction in directly

* fixes

* restore setting of enveloped_tx

* refactor anvil Env and reduce changes in tx processing

* apply revm bump fixes, solar fixes

* bump op-alloy-*

* bump to msrv 1.86 for solar, use 0.15.* for alloy instead of pinning to 0.15.0, use alloy-evm patch for .use_ref() issue

* fix: correctly set txtype when setting up TxEnv

* start upgrading to revm 23

* bump PR to be revm 23+ compatible

* fix: correctly set txtype when setting up TxEnv

* fix: correctly set txtype when setting up TxEnv

* clean up

* fix merge conflict, apply fixes from upstream

* bump to 0.7.2

* fix order

* update block-explorers and compilers

* fix clippy

* fix failing abi test

* empty

* integrate BlobParams into anvil

* fix tests

* fix cast decode-event

* fix tests

* fix colored_traces

* fix gas pausing

* fix tests

* fix test

* update last commits from master to be u64 compatible

* syn no longer implements PartialEq requiring us to use `matches!`

* temp comment out journal push loop

* fix clippy lint

* revert clippy changes, make sure lint-foundry uses nightly clippy version

* also assert that blob_count is less than the configured max_blob_count

* fix: only upgrade tx_type to eip-2930 (type 1) if it is a legacy tx

* optimistically remove previous workaround that was required for internal tracking, tests do not indicate it is longer required

* nit

* prefer using typed TransactionType over raw u8

* apply tx_type if set, upgrading from legacy to eip2930 if access_list is present and tx type is legacy

* restore #[ret] macro that was removed unintendedly

* replace redundant Env::new_with_spec_id(..) with default

* allow passing is_optimism into Env constructor specific to Anvil

* extract environment configuration into init.rs to make configuring the environment less error prone

* remove redundant debug derive

* restore #[cold] do hardhat log, previously preferred inline because of new context requirement for bytes but we refactor resolved this

* avoid code duplication, add documented `apply_accesslist`

* alloy 1.0 + fork-db 0.14 + op-alloy 0.16 + revm-insp 0.22 + block-explorers 0.17

* fix clippy

* Update crates/evm/evm/src/inspectors/logs.rs

Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>

* fix fmt

* set env tx type by deriving tx type from other fields if no transaction_type has been set

* use hardfork configured max_blob_count rather than hardcoded Dancun in assertion and error message

* add temporary workaround for failing StdChains test because eth.llamarpc.com is down

* bump(`revm`: step 3): reintroduce precompile injection (#10508)

* sketching

* sketch

* sketch

* restore test

* add echo precompile test

* pick a safe non precompile target outside of 0x00-0xff range

* add op evm test

* instead of activating all precompiles by default we activate selectively based on the spec defined

* add note for us pinning to OpSpecId::BEDROCK here, we should make this configurable

* bump deps to latest

---------

Co-authored-by: zerosnacks <zerosnacks@protonmail.com>
Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com>
Co-authored-by: grandizzy <grandizzy.the.egg@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L-ignore Log: ignore PR in changelog
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

2 participants