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 optimism bootnodes to reth-primitives #7657

Merged
merged 6 commits into from
Apr 16, 2024
Merged

Conversation

joshieDo
Copy link
Collaborator

As title suggests.

Additionally it fixes an issue where op stack chains were loading ethereum mainnet boot nodes (in addition to op ones) because there were no bootnodes associated with their chain specs.

@joshieDo joshieDo added A-networking Related to networking in general A-op-reth Related to Optimism and op-reth labels Apr 15, 2024
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, 1 q re match

Comment on lines 919 to 926
#[cfg(feature = "optimism")]
C::Base | C::Optimism => Some(op_nodes()),
#[cfg(feature = "optimism")]
C::BaseGoerli |
C::BaseSepolia |
C::OptimismGoerli |
C::OptimismSepolia |
C::OptimismKovan => Some(op_testnet_nodes()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this correct?

this uses the same nodes for multiple different chains?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added the differentiation

@joshieDo joshieDo added this pull request to the merge queue Apr 16, 2024
Merged via the queue into main with commit 566480b Apr 16, 2024
27 checks passed
@joshieDo joshieDo deleted the joshie/op-boot branch April 16, 2024 16:04
Comment on lines -135 to -142
if chain == Chain::optimism_mainnet() || chain == Chain::base_mainnet() {
builder = builder.add_optimism_mainnet_boot_nodes()
} else if chain == Chain::from_named(NamedChain::OptimismSepolia) ||
chain == Chain::from_named(NamedChain::BaseSepolia)
{
builder = builder.add_optimism_sepolia_boot_nodes()
}

Copy link
Member

Choose a reason for hiding this comment

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

how are you adding the boot nodes now that this is removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

By adding the nodes to the ChainSpec, NetworkConfig will have them to serve to discovery_v5_with_config_builder

mw2000 pushed a commit to mw2000/reth that referenced this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Related to networking in general A-op-reth Related to Optimism and op-reth
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants