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

fix: also set configured bootnodes for discv5 #9885

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Jul 29, 2024

closes #9883

we didn't configure the provided --bootnodes for discv5.

geth also uses the --bootnodes values to initialize discv5 bootnodes:

https://github.com/ethereum/go-ethereum/blob/380688c636a654becc8f114438c2a5d93d2db032/cmd/utils/flags.go#L1087-L1087

so this fix is reasonable imo.

I also added extend the discv5 bootnodes during network launch, this is redundant if the discv5 is already configured with the bootnodes, but doesn't hurt

if let Some(discv5) = discovery_v5_config.as_mut() {
// merge configured boot nodes
discv5.extend_unsigned_boot_nodes(resolved_boot_nodes)
}

@github-actions github-actions bot added the C-enhancement New feature or request label Jul 29, 2024
@mattsse mattsse added C-bug An unexpected or incorrect behavior A-networking Related to networking in general A-op-reth Related to Optimism and op-reth and removed C-enhancement New feature or request labels Jul 29, 2024
ctx.config()
.network
.resolved_bootnodes()
.or_else(|| ctx.chain_spec().bootnodes())
Copy link
Member

Choose a reason for hiding this comment

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

makes sense

@mattsse mattsse added this pull request to the merge queue Jul 29, 2024
Merged via the queue into main with commit eb2d0a2 Jul 29, 2024
33 checks passed
@mattsse mattsse deleted the matt/also-set-bootnodes-for-discv5 branch July 29, 2024 23:53
@emhane
Copy link
Member

emhane commented Jul 30, 2024

hm, this breaks the builder pattern by adding methods to Config taking mutable ref to self. this worsens devex for the experienced dev, since one expects an instance built by a builder to be immutable.

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 C-bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--bootnodes are ignored for discv5
3 participants