Skip to content

Fix op-geth bootnode peering #396

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.

Sign up for GitHub

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
dvush merged 3 commits into from
Mar 10, 2026
Merged

Fix op-geth bootnode peering #396

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1bbac9
fix op-geth bootnode peering
canercidam Mar 6, 2026
4f160da
add --nodiscover back conditionally
canercidam Mar 6, 2026
03b4590
op-geth peers
dvush Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions playground/components.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,12 +379,13 @@ func (o *OpGeth) Apply(ctx *ExContext) *Component {
component := NewComponent("op-geth")
o.Enode = ctx.Output.GetEnodeAddr()

// TODO: this does not work for op-geth because hostnames are not allowed
// in geth bootnode config (node will break on startup if we remove --nodiscover below)
// var trustedPeers string
// if ctx.Bootnode != nil {
// trustedPeers = fmt.Sprintf("--bootnodes %s ", ctx.Bootnode.Connect())
// }
var trustedPeers string
if ctx.Bootnode != nil {
// TODO: Figure out the port dynamically.
trustedPeers = fmt.Sprintf("--bootnodes enode://%s@$(getent hosts bootnode | awk '{print $1}'):30303 --discovery.v4 ", ctx.Bootnode.ID)
Copy link
Contributor

@claude claude bot Mar 6, 2026

Choose a reason for hiding this comment

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

Nit: Hardcoded 30303 is correct today but fragile.

Since both op-geth and bootnode are Docker services, the container port is used (not the host port), and the bootnode's default is 30303 (components.go:1085). So this works. However, if someone changes the bootnode's default port, this will silently break.

Consider extracting the bootnode default port as a constant shared between the bootnode component and this line, or using the {{Bootnode}} template and performing hostname-to-IP resolution in the shell (since /bin/sh is already the entrypoint):

// Example: resolve the hostname from the {{Bootnode}} template output at runtime
trustedPeers = fmt.Sprintf("--bootnodes $(echo '%s' | sed 's/@[^:]*/@'$(getent hosts bootnode | awk '{print $1}')'/' ) --discovery.v4 ", ctx.Bootnode.Connect())

This way the port comes from the template system automatically.

} else {
trustedPeers = "--nodiscover "
}
Comment on lines +382 to +388
Copy link
Contributor

@claude claude bot Mar 6, 2026

Choose a reason for hiding this comment

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

Bug: Missing --nodiscover when no bootnode is configured.

When ctx.Bootnode == nil, trustedPeers is empty and --nodiscover was removed unconditionally. This means op-geth will attempt peer discovery even without a bootnode, which is inconsistent with every other component in this file (op-reth, reth, op-rbuilder, flashblocks-rpc all add --disable-discovery in the else branch).

Suggested change
var trustedPeers string
if ctx.Bootnode != nil {
// TODO: Figure out the port dynamically.
trustedPeers = fmt.Sprintf("--bootnodes enode://%s@$(awk '/host.docker.internal/ {print $1; exit}' /etc/hosts):30303 ", ctx.Bootnode.ID)
}
var trustedPeers string
if ctx.Bootnode != nil {
// TODO: Figure out the port dynamically.
trustedPeers = fmt.Sprintf("--bootnodes enode://%s@$(awk '/host.docker.internal/ {print $1; exit}' /etc/hosts):30303 ", ctx.Bootnode.ID)
} else {
trustedPeers = "--nodiscover "
}


svc := component.NewService("op-geth").
WithImage("us-docker.pkg.dev/oplabs-tools-artifacts/images/op-geth").
Expand All @@ -409,7 +410,6 @@ func (o *OpGeth) Apply(ctx *ExContext) *Component {
"--ws.origins \"*\" "+
"--ws.api debug,eth,txpool,net,engine,miner "+
"--syncmode full "+
"--nodiscover "+
"--maxpeers 5 "+
"--rpc.allow-unprotected-txs "+
"--authrpc.addr 0.0.0.0 "+
Expand All @@ -420,7 +420,7 @@ func (o *OpGeth) Apply(ctx *ExContext) *Component {
"--state.scheme hash "+
"--port "+`{{Port "rpc" 30303}} `+
"--nodekey /data/p2p_key.txt "+
// trustedPeers+
trustedPeers+
"--metrics "+
"--metrics.addr 0.0.0.0 "+
"--metrics.port "+`{{Port "metrics" 6061}}`,
Expand Down