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

feat: use chain specific bootnodes #1826

Merged
merged 3 commits into from
Mar 17, 2023
Merged

feat: use chain specific bootnodes #1826

merged 3 commits into from
Mar 17, 2023

Conversation

Rjected
Copy link
Member

@Rjected Rjected commented Mar 17, 2023

Previously, unless explicitly specified, we would always use the mainnet bootnodes. This will use the bootnodes specific to the specified chain when running reth node.

@Rjected Rjected requested a review from onbjerg as a code owner March 17, 2023 22:13
@Rjected Rjected requested review from mattsse and removed request for onbjerg March 17, 2023 22:13
@Rjected Rjected force-pushed the dan/use-chain-bootnodes branch from ef7567d to 5652cb1 Compare March 17, 2023 22:16
@Rjected Rjected requested a review from onbjerg March 17, 2023 22:16
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2023

Codecov Report

Merging #1826 (3381089) into main (d65cab9) will increase coverage by 1.00%.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #1826      +/-   ##
==========================================
+ Coverage   72.50%   73.51%   +1.00%     
==========================================
  Files         411      410       -1     
  Lines       50502    50510       +8     
==========================================
+ Hits        36617    37130     +513     
+ Misses      13885    13380     -505     
Flag Coverage Δ
integration-tests 19.71% <0.00%> (+2.11%) ⬆️
unit-tests 67.85% <0.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
bin/reth/src/args/network_args.rs 5.55% <0.00%> (-0.22%) ⬇️
crates/net/discv4/src/lib.rs 66.09% <ø> (+0.14%) ⬆️
crates/net/network/src/config.rs 71.36% <ø> (+1.81%) ⬆️
crates/net/network/src/lib.rs 100.00% <ø> (ø)
crates/net/network/src/manager.rs 52.49% <ø> (+9.24%) ⬆️
crates/primitives/src/chain/mod.rs 91.58% <0.00%> (-2.81%) ⬇️
crates/primitives/src/lib.rs 100.00% <ø> (ø)
crates/primitives/src/net.rs 91.30% <0.00%> (-5.62%) ⬇️

... and 38 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Comment on lines 58 to 64
let chain_bootnodes = match chain_spec.chain.try_into() {
Ok(Chain::Mainnet) => mainnet_nodes(),
Ok(Chain::Goerli) => goerli_nodes(),
Ok(Chain::Sepolia) => sepolia_nodes(),
_ => mainnet_nodes(),
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to make this a Chain function, similar to

pub fn public_dns_network_protocol(self) -> Option<String> {

Copy link
Member Author

@Rjected Rjected Mar 17, 2023

Choose a reason for hiding this comment

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

Since the constants are in reth_discv4, I guess those should move to primitives as well

@Rjected Rjected requested a review from gakonst as a code owner March 17, 2023 22:57
@Rjected Rjected merged commit 7ac06cd into main Mar 17, 2023
@Rjected Rjected deleted the dan/use-chain-bootnodes branch March 17, 2023 23:28
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.

4 participants