-
Notifications
You must be signed in to change notification settings - Fork 86
refactor(l1, l2): decouple L2 metrics logic from L1's & remove l2 feature flag from ethrex-blockchain
#3371
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TODO: Reject blob transactions in L2
…eature_flag_from_levm
…lass/ethrex into remove_l2_feature_flag_from_blockchain
MegaRedHand
approved these changes
Jun 30, 2025
…eature_flag_from_levm
…lass/ethrex into remove_l2_feature_flag_from_blockchain
tomip01
reviewed
Jul 2, 2025
crates/vm/backends/mod.rs
Outdated
Comment on lines
93
to
98
pub fn new_for_l2(engine: EvmEngine, db: impl VmDatabase + 'static) -> Result<Self, EvmError> { | ||
if let EvmEngine::REVM = engine { | ||
return Err(EvmError::InvalidEVM( | ||
"REVM is not supported for L2".to_string(), | ||
)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better if we only support LEVM for the L2 just create using LEVM.
In the case a Blockchain is created using REVM
and BlockchainType::L2
that should return an Error
…eature_flag_from_blockchain
jrchatruc
approved these changes
Jul 2, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 4, 2025
…e binary for both L1 and L2 (#3381) > [!WARNING] > Merge after #3371 **Motivation** We want `ethrex` to be a single binary for both running L1 and L2. For this, we need to remove the `l2` feature flag from `cmd/ethrex`, a work that includes removing the same feature flag from the following crates: 1. `ethrex-vm`. 2. `ethrex-levm`. 3. `ethrex-blockchain`. 1 and 2 are removed in #3367, and 3 is removed in #3371. **Description** Removes `l2` feature flag from `cmd/ethrex`
d-roak
pushed a commit
to 1sixtech/ethrex
that referenced
this pull request
Jul 17, 2025
…ture flag from `ethrex-blockchain` (lambdaclass#3371) > [!WARNING] > Merge after lambdaclass#3367 **Motivation** To completely remove the `l2` feature flag from `cmd/ethrex` in favor of having a single binary for running ethrex (L1 and L2), there are some local dependencies from which to remove this feature first. These are: 1. `ethrex-vm`. 2. `ethrex-levm`. 3. `ethrex-blockchain`. 1 and 2 are removed in lambdaclass#3367, and 3 is meant to be removed in this PR. **Description** Decouples the L2 metrics logic from the L1's, allowing to remove the use of the `l2` feature flag from the crate `ethrex-blockchain`. - Creates a `crates/blockchain/metrics/l2` module with `metrics.rs` and `api.rs` submodules. - Makes use of this new module in `cmd/ethrex`. - Removes `l2` feature flag from `ethrex-blockchain` crate. - Removes the import of `ethrex-blockchain/l2` where needed.
d-roak
pushed a commit
to 1sixtech/ethrex
that referenced
this pull request
Jul 17, 2025
…e binary for both L1 and L2 (lambdaclass#3381) > [!WARNING] > Merge after lambdaclass#3371 **Motivation** We want `ethrex` to be a single binary for both running L1 and L2. For this, we need to remove the `l2` feature flag from `cmd/ethrex`, a work that includes removing the same feature flag from the following crates: 1. `ethrex-vm`. 2. `ethrex-levm`. 3. `ethrex-blockchain`. 1 and 2 are removed in lambdaclass#3367, and 3 is removed in lambdaclass#3371. **Description** Removes `l2` feature flag from `cmd/ethrex`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
L1
Ethereum client
L2
Rollup client
simplification
Any refactor that makes code simpler and more straight forward
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Warning
Merge after #3367
Motivation
To completely remove the
l2
feature flag fromcmd/ethrex
in favor of having a single binary for running ethrex (L1 and L2), there are some local dependencies from which to remove this feature first. These are:ethrex-vm
.ethrex-levm
.ethrex-blockchain
.1 and 2 are removed in #3367, and 3 is meant to be removed in this PR.
Description
Decouples the L2 metrics logic from the L1's, allowing to remove the use of the
l2
feature flag from the crateethrex-blockchain
.crates/blockchain/metrics/l2
module withmetrics.rs
andapi.rs
submodules.cmd/ethrex
.l2
feature flag fromethrex-blockchain
crate.ethrex-blockchain/l2
where needed.