Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
3 changes: 3 additions & 0 deletions docs/recipes/opstack.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Deploy an OP stack.

```mermaid
graph LR
bootnode["bootnode<br/>rpc:30303"]
el["el<br/>rpc:30303<br/>http:8545<br/>ws:8546<br/>authrpc:8551<br/>metrics:9090"]
el_healthmon["el_healthmon"]
beacon["beacon<br/>p2p:9000<br/>p2p:9000<br/>quic-p2p:9100<br/>http:3500"]
Expand All @@ -28,10 +29,12 @@ graph LR
op_node["op-node<br/>metrics:7300<br/>http:8549<br/>p2p:9003<br/>p2p:9003"]
op_batcher["op-batcher"]

el -->|rpc| bootnode
el_healthmon -->|http| el
beacon -->|authrpc| el
beacon_healthmon -->|http| beacon
validator -->|http| beacon
op_geth -->|rpc| bootnode
op_geth_healthmon -->|http| op_geth
op_node -->|http| el
op_node -->|http| beacon
Expand Down
1 change: 1 addition & 0 deletions playground/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,5 @@ func init() {
register(&WebsocketProxy{})
register(&BuilderHub{})
register(&ChainMonitor{})
register(&Bootnode{})
}
43 changes: 41 additions & 2 deletions playground/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (o *OpRbuilder) Apply(ctx *ExContext) *Component {
})

if ctx.Bootnode != nil {
service.WithArgs("--trusted-peers", ctx.Bootnode.Connect())
service.WithArgs("--bootnodes", ctx.Bootnode.Connect())
}

if o.Flashblocks {
Expand Down Expand Up @@ -166,7 +166,7 @@ func (f *FlashblocksRPC) Apply(ctx *ExContext) *Component {

if ctx.Bootnode != nil {
service.WithArgs(
"--trusted-peers", ctx.Bootnode.Connect(),
"--bootnodes", ctx.Bootnode.Connect(),
)
}

Expand Down Expand Up @@ -505,6 +505,10 @@ func (r *RethEL) Apply(ctx *ExContext) *Component {
WithArtifact("/data/jwtsecret", "jwtsecret").
WithVolume("data", "/data_reth")

if ctx.Bootnode != nil {
svc.WithArgs("--bootnodes", ctx.Bootnode.Connect())
}

UseHealthmon(component, svc, healthmonExecution)

if r.UseNativeReth {
Expand Down Expand Up @@ -1003,6 +1007,41 @@ func registerBuilderHook(ctx *ExContext, s *Service, b *BuilderHub) error {
return nil
}

type BootnodeProtocol string

const (
BootnodeProtocolV5 BootnodeProtocol = "v5"
)

type Bootnode struct {
Protocol BootnodeProtocol
Enode *EnodeAddr
}

func (b *Bootnode) Apply(ctx *ExContext) *Component {
component := NewComponent("bootnode")

b.Enode = ctx.Output.GetEnodeAddr()
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: This initialization needs to be conditional to support the UseBootnode() helper method. When UseBootnode() is used, the Enode is pre-initialized to establish the BootnodeRef before Apply() runs.

Suggested change
b.Enode = ctx.Output.GetEnodeAddr()
if b.Enode == nil {
b.Enode = ctx.Output.GetEnodeAddr()
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: This unconditionally calls GetEnodeAddr(), which generates a new key each time (incrementing a sequence counter). However, UseBootnode() already calls GetEnodeAddr() to create the Enode and store the NodeID in ctx.Bootnode.

This means:

  1. UseBootnode() generates key readme bullets formatting #1, stores its NodeID in ctx.Bootnode.ID
  2. Bootnode.Apply() generates key CLI could print services and their ports for clarity #2 (different!), uses enode-key-2.txt

Clients will connect to the NodeID from key #1, but the bootnode will be running with key #2.

Fix: Only generate if Enode is nil:

Suggested change
b.Enode = ctx.Output.GetEnodeAddr()
if b.Enode == nil {
b.Enode = ctx.Output.GetEnodeAddr()
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: This unconditionally overwrites b.Enode, but UseBootnode() already initialized it at manifest.go:182 and stored the NodeID in ctx.Bootnode.ID.

Since GetEnodeAddr() increments a sequence counter, this generates a different key. Clients will connect using the NodeID from the first key, but the bootnode runs with the second key.

Suggested change
b.Enode = ctx.Output.GetEnodeAddr()
if b.Enode == nil {
b.Enode = ctx.Output.GetEnodeAddr()
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Enode key mismatch

This unconditionally overwrites b.Enode, but UseBootnode() already initialized it at manifest.go:182 and stored its NodeID in ctx.Bootnode.ID.

Since GetEnodeAddr() increments a sequence counter (see artifacts.go:703), this generates a different key:

  1. UseBootnode() → key readme bullets formatting #1 (sequence=1), stores NodeID in ctx.Bootnode.ID
  2. Bootnode.Apply() → key CLI could print services and their ports for clarity #2 (sequence=2), overwrites b.Enode

Impact: Clients (like RethEL) will try to connect using the NodeID from key #1, but the bootnode service runs with key #2.

Suggested change
b.Enode = ctx.Output.GetEnodeAddr()
if b.Enode == nil {
b.Enode = ctx.Output.GetEnodeAddr()
}

svc := component.NewService("bootnode").
WithImage("ghcr.io/paradigmxyz/reth").
WithTag("v1.9.3").
WithEntrypoint("/usr/local/bin/reth").
WithArgs(
"p2p", "bootnode",
"--addr", `0.0.0.0:{{Port "rpc" 30303}}`,
"--p2p-secret-key", "/data/p2p_key.txt",
"-vvvv",
"--color", "never",
).
WithArtifact("/data/p2p_key.txt", b.Enode.Artifact)

if b.Protocol == BootnodeProtocolV5 {
svc.WithArgs("--v5")
}

return component
}

const (
healthmonBeacon = "beacon"
healthmonExecution = "execution"
Expand Down
2 changes: 1 addition & 1 deletion playground/components_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func (tt *testFramework) test(component ComponentGen, args []string) *Manifest {

exCtx := &ExContext{
Output: o,
LogLevel: LevelDebug,
LogLevel: LevelTrace,
Contender: &ContenderContext{
Enabled: false,
},
Expand Down
18 changes: 15 additions & 3 deletions playground/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ func (p *Component) AddComponent(ctx *ExContext, gen ComponentGen) {
p.Inner = append(p.Inner, gen.Apply(ctx))
}

func (p *Component) AddService(srv ComponentGen) {
p.Inner = append(p.Inner, srv.Apply(nil))
func (p *Component) AddService(ctx *ExContext, srv ComponentGen) {
p.Inner = append(p.Inner, srv.Apply(ctx))
}

func componentToManifest(p *Component) []*Service {
Expand Down Expand Up @@ -177,6 +177,18 @@ type ExContext struct {
Contender *ContenderContext
}

func (e *ExContext) UseBootnode() ComponentGen {
bootnode := &Bootnode{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing initialization: Before calling bootnode.Enode.NodeID() on line 185, you need to initialize the Enode field:

Suggested change
bootnode := &Bootnode{}
bootnode := &Bootnode{}
bootnode.Enode = e.Output.GetEnodeAddr()

bootnode.Enode = e.Output.GetEnodeAddr()

e.Bootnode = &BootnodeRef{
Service: "bootnode",
ID: bootnode.Enode.NodeID(),
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 Critical Bug: Nil pointer dereference

This line calls bootnode.Enode.NodeID(), but at this point bootnode.Enode is nil. The Enode field is only initialized inside the Apply() method (in components.go:1024), which hasn't been called yet.

This will cause a panic at runtime when UseBootnode() is invoked.

Suggested fix: The UseBootnode() method needs to eagerly initialize the EnodeAddr, similar to how the old code in recipe_opstack.go worked. Consider something like:

func (e *ExContext) UseBootnode() ComponentGen {
	bootnode := &Bootnode{}
	bootnode.Enode = e.Output.GetEnodeAddr()  // Initialize eagerly

	e.Bootnode = &BootnodeRef{
		Service: "bootnode",
		ID:      bootnode.Enode.NodeID(),
	}

	return bootnode
}

Then update Bootnode.Apply() to check if Enode is already set before calling ctx.Output.GetEnodeAddr():

func (b *Bootnode) Apply(ctx *ExContext) *Component {
	component := NewComponent("bootnode")

	if b.Enode == nil {
		b.Enode = ctx.Output.GetEnodeAddr()
	}
	// ... rest of the method
}

}

return bootnode
}

type BootnodeRef struct {
Service string
ID string
Expand Down Expand Up @@ -751,6 +763,6 @@ func ReadManifest(outputFolder string) (*Manifest, error) {

func (component *Component) RunContenderIfEnabled(ctx *ExContext) {
if ctx.Contender.Enabled {
component.AddService(ctx.Contender.Contender())
component.AddService(ctx, ctx.Contender.Contender())
}
}
10 changes: 2 additions & 8 deletions playground/recipe_opstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func (o *OpRecipe) Artifacts() *ArtifactsBuilder {

func (o *OpRecipe) Apply(ctx *ExContext) *Component {
component := NewComponent("op-recipe")
component.AddService(ctx, ctx.UseBootnode())

component.AddComponent(ctx, &RethEL{})
component.AddComponent(ctx, &LighthouseBeaconNode{
Expand All @@ -88,14 +89,6 @@ func (o *OpRecipe) Apply(ctx *ExContext) *Component {
externalBuilderRef := o.externalBuilder
peers := []string{}

opGeth := &OpGeth{}
component.AddComponent(ctx, opGeth)

ctx.Bootnode = &BootnodeRef{
Service: "op-geth",
ID: opGeth.Enode.NodeID(),
}

if o.externalBuilder == "op-reth" {
// Add a new op-reth service and connect it to Rollup-boost
component.AddComponent(ctx, &OpReth{})
Expand Down Expand Up @@ -170,6 +163,7 @@ func (o *OpRecipe) Apply(ctx *ExContext) *Component {
})
}

component.AddService(ctx, &OpGeth{})
component.AddComponent(ctx, &OpNode{
L1Node: "el",
L1Beacon: "beacon",
Expand Down