Skip to content
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

build: upgrade to latest REVM #669

Merged
merged 20 commits into from
Sep 18, 2024
Merged

build: upgrade to latest REVM #669

merged 20 commits into from
Sep 18, 2024

Conversation

Wodann
Copy link
Member

@Wodann Wodann commented Sep 16, 2024

This PR bumps our REVM dependency to Wodann/revm@31771cc, which is the latest commit of REVM with some minor modifications:

  • Revert of a yanked dependency
  • Splitting of the trait EvmWiring into an additional trait ChainSpec such that we can have a compile-time constant ChainSpec without depending on the Database and ExternalContext associated types.
  • Revert changes to REVM that require Rust v1.81.

NOTE
Support for Rust v1.81 is blocked by a bug introduced in Rust v1.81. Unfortunately, REVM requires v1.81 without the changes made in our fork and EDR, preventing us from reverting to an older version of Rust.

Compiler errors in REVM using Rust v1.80 - without changes ``` error[E0658]: use of unstable library feature 'error_in_core' --> crates/primitives/src/eip7702/bytecode.rs:96:6 | 96 | impl core::error::Error for Eip7702DecodeError {} | ^^^^^^^^^^^^^^^^^^ | = note: see issue #103765 for more information

error[E0658]: use of unstable library feature 'error_in_core'
--> crates/primitives/src/bytecode.rs:238:6
|
238 | impl core::error::Error for BytecodeDecodeError {}
| ^^^^^^^^^^^^^^^^^^
|
= note: see issue #103765 rust-lang/rust#103765 for more information

error[E0658]: use of unstable library feature 'error_in_core'
--> crates/primitives/src/bytecode/eof.rs:185:6
|
185 | impl core::error::Error for EofDecodeError {}
| ^^^^^^^^^^^^^^^^^^
|
= note: see issue #103765 rust-lang/rust#103765 for more information

error[E0658]: use of unstable library feature 'error_in_core'
--> crates/primitives/src/evm_wiring.rs:21:35
|
21 | type ValidationError: Debug + core::error::Error;
| ^^^^^^^^^^^^^^^^^^
|
= note: see issue #103765 rust-lang/rust#103765 for more information

error[E0658]: use of unstable library feature 'error_in_core'
--> crates/primitives/src/precompile.rs:143:6
|
143 | impl core::error::Error for PrecompileErrors {}
| ^^^^^^^^^^^^^^^^^^
|
= note: see issue #103765 rust-lang/rust#103765 for more information

error[E0658]: use of unstable library feature 'error_in_core'
--> crates/primitives/src/precompile.rs:198:6
|
198 | impl core::error::Error for PrecompileError {}
| ^^^^^^^^^^^^^^^^^^
|
= note: see issue #103765 rust-lang/rust#103765 for more information

error[E0658]: use of unstable library feature 'error_in_core'
--> crates/primitives/src/result.rs:316:6
|
316 | impl core::error::Error for InvalidTransaction {}
| ^^^^^^^^^^^^^^^^^^
|
= note: see issue #103765 rust-lang/rust#103765 for more information

error[E0658]: use of unstable library feature 'error_in_core'
--> crates/primitives/src/result.rs:392:6
|
392 | impl core::error::Error for InvalidHeader {}
| ^^^^^^^^^^^^^^^^^^
|
= note: see issue #103765 rust-lang/rust#103765 for more information


</details>

@Wodann Wodann added the no changeset needed This PR doesn't require a changeset label Sep 16, 2024
@Wodann Wodann self-assigned this Sep 16, 2024
Copy link

changeset-bot bot commented Sep 16, 2024

🦋 Changeset detected

Latest commit: 3fa7969

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@nomicfoundation/edr Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Wodann Wodann had a problem deploying to github-action-benchmark September 16, 2024 23:18 — with GitHub Actions Failure
Base automatically changed from refactor/single-npm-package to feat/multichain September 17, 2024 00:42
@Wodann Wodann removed the no changeset needed This PR doesn't require a changeset label Sep 17, 2024
@Wodann Wodann had a problem deploying to github-action-benchmark September 17, 2024 01:12 — with GitHub Actions Failure
@Wodann Wodann had a problem deploying to github-action-benchmark September 17, 2024 04:39 — with GitHub Actions Failure
@Wodann Wodann temporarily deployed to github-action-benchmark September 17, 2024 04:55 — with GitHub Actions Inactive
Copy link

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 62.07865% with 135 lines in your changes missing coverage. Please review.

Please upload report for BASE (feat/multichain@bcdcafa). Learn more about missing BASE report.

Files with missing lines Patch % Lines
crates/edr_napi_core/src/logger.rs 0.00% 29 Missing ⚠️
crates/edr_napi_core/src/spec.rs 0.00% 23 Missing ⚠️
crates/edr_provider/src/data.rs 73.68% 9 Missing and 6 partials ⚠️
crates/edr_evm/src/runtime.rs 30.76% 9 Missing ⚠️
crates/edr_optimism/src/spec.rs 42.85% 8 Missing ⚠️
crates/edr_provider/src/error.rs 0.00% 7 Missing ⚠️
crates/edr_provider/src/requests/eth/call.rs 16.66% 5 Missing ⚠️
crates/edr_evm/src/block/builder.rs 20.00% 4 Missing ⚠️
crates/edr_provider/src/data/gas.rs 42.85% 4 Missing ⚠️
crates/edr_provider/src/requests/eth/config.rs 20.00% 4 Missing ⚠️
... and 18 more
Additional details and impacted files
@@                Coverage Diff                 @@
##             feat/multichain     #669   +/-   ##
==================================================
  Coverage                   ?   63.04%           
==================================================
  Files                      ?      215           
  Lines                      ?    23677           
  Branches                   ?    23677           
==================================================
  Hits                       ?    14926           
  Misses                     ?     7837           
  Partials                   ?      914           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Wodann Wodann temporarily deployed to github-action-benchmark September 17, 2024 19:05 — with GitHub Actions Inactive
@Wodann Wodann temporarily deployed to github-action-benchmark September 17, 2024 21:50 — with GitHub Actions Inactive
@Wodann Wodann temporarily deployed to github-action-benchmark September 17, 2024 22:23 — with GitHub Actions Inactive
@Wodann Wodann marked this pull request as ready for review September 17, 2024 22:25
@Wodann Wodann requested a review from a team September 17, 2024 22:50
Copy link
Member

@agostbiro agostbiro left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

Copy link
Contributor

@Xanewok Xanewok left a comment

Choose a reason for hiding this comment

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

Thanks for simplifying some of the traits/ADTs! The changes make sense and strictly improve what we have, so I'm fine with landing this if that's possible, left one thread about the naming but that's non-blocking.

For the Rust 1.81, would it be possible to submit a patch that downgrades MSRV for revm to use the std Error types for now? I looked at the upstream rustc issue and I'm not sure that's going to be backported soon, so it looks like we're stuck on <1.81 for a while.

crates/edr_optimism/Cargo.toml Show resolved Hide resolved
crates/edr_provider/Cargo.toml Show resolved Hide resolved

const MIN_ETHASH_DIFFICULTY: u64 = 131072;
const MIN_ETHASH_DIFFICULTY: u64 = L1ChainSpec::MIN_ETHASH_DIFFICULTY;
Copy link
Contributor

Choose a reason for hiding this comment

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

That's better, thanks!

remainder => TransactionFailureReason::Inner(remainder),
}
fn cast_halt_reason(reason: Self::HaltReason) -> TransactionFailureReason<Self::HaltReason> {
<L1ChainSpec as ProviderSpec<TimerT>>::cast_halt_reason(reason)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for simplifying this!

crates/edr_evm/src/chain_spec.rs Outdated Show resolved Hide resolved
@Wodann Wodann temporarily deployed to github-action-benchmark September 18, 2024 18:38 — with GitHub Actions Inactive
@Wodann Wodann temporarily deployed to github-action-benchmark September 18, 2024 19:25 — with GitHub Actions Inactive
@Wodann Wodann temporarily deployed to github-action-benchmark September 18, 2024 20:19 — with GitHub Actions Inactive
@Wodann Wodann merged commit 618da93 into feat/multichain Sep 18, 2024
41 checks passed
@Wodann Wodann deleted the build/upgrade-revm branch September 18, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider updating chain specification traits based on findings from GenericChainSpec implementation
3 participants