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

chore: move and integrate ConfigureEvmFor #13896

Merged
merged 1 commit into from
Jan 21, 2025
Merged

Conversation

Rjected
Copy link
Member

@Rjected Rjected commented Jan 21, 2025

This trait was actually not in a place where it could be used properly (it would have caused cyclic deps), it's now moved to reth_evm where the ConfigureEvm trait actually is. the evm crate also imports primitives-traits, so we can define it there for no extra deps on ConfigureEvm consumers. Node-builder only really makes sense when we need to connect e.g. reth-provider and network.

@Rjected Rjected added C-enhancement New feature or request A-sdk Related to reth's use as a library labels Jan 21, 2025
@Rjected Rjected force-pushed the dan/use-configure-evm-for branch 2 times, most recently from 9e9ed92 to 84e837c Compare January 21, 2025 05:00
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -43,7 +43,7 @@ pub trait NodeComponents<T: FullNodeTypes>: Clone + Unpin + Send + Sync + 'stati
type Pool: TransactionPool<Transaction: PoolTransaction<Consensus = TxTy<T::Types>>> + Unpin;

/// The node's EVM configuration, defining settings for the Ethereum Virtual Machine.
type Evm: ConfigureEvm<Header = HeaderTy<T::Types>, Transaction = TxTy<T::Types>>;
type Evm: ConfigureEvmFor<<T::Types as NodeTypes>::Primitives>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

we could also introduce an alias for this PrimitivesFor for example

Copy link
Member Author

Choose a reason for hiding this comment

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

yep

@Rjected Rjected force-pushed the dan/use-configure-evm-for branch from 84e837c to a0df055 Compare January 21, 2025 14:47
@Rjected Rjected added this pull request to the merge queue Jan 21, 2025
Merged via the queue into main with commit ace28d8 Jan 21, 2025
43 checks passed
@Rjected Rjected deleted the dan/use-configure-evm-for branch January 21, 2025 18:07
refcell pushed a commit to ethereum-optimism/op-reth that referenced this pull request Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sdk Related to reth's use as a library C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants