Skip to content

Fix startup session races and active-pool re-admission - #39

Merged
pk910 merged 2 commits into
ethpandaops:masterfrom
MysticRyuujin:fix-startup-session-races
Jul 29, 2026
Merged

Fix startup session races and active-pool re-admission#39
pk910 merged 2 commits into
ethpandaops:masterfrom
MysticRyuujin:fix-startup-session-races

Conversation

@MysticRyuujin

Copy link
Copy Markdown
Collaborator

Three related bugs around service startup and active-pool admission:

1. Sessions we initiate never carry a node reference (discv5/protocol/handler.go)

When we initiate a handshake (we ping, peer responds WHOAREYOU), handleWHOAREYOUPacket created the session and only inherited the node reference from a previous session — pending.ToNode was never attached, even though it's the node we were sending to and was just used to verify the handshake.

After a restart the session cache is empty, so every session created during the startup burst (aliveness pings, random walks, bootnode connects) had Node == nil. Consequences for the whole session lifetime (12h by default):

  • OnHandshakeComplete never fired (guarded by sess.GetNode() != nil), so the peer was never admitted to the active pool via the handshake path.
  • Every subsequent message from the peer flowed through handleMessage with a nil node, so OnNodeSeen never fired either — no lastSeen updates, no ENR-seq refresh.
  • The peer had no reason to re-handshake until session expiry, so a node demoted from the pool could ping us for hours without ever being re-added.

Fix: fall back to pending.ToNode when no previous session provides a node.

2. LoadInitialNodesFromDB mutates the pool without locking (nodes/flattable.go)

The discv5 packet handler is registered and receiving live traffic during bootnode.New(), before Start() runs LoadInitialNodesFromDB. A peer handshaking in that window calls FlatTable.Add (which locks t.mu) concurrently with LoadInitialNodesFromDB writing t.activeNodes and t.ipLimiter with no lock — a data race that can fatal with concurrent map writes.

Fix: hold t.mu for the load, and skip nodes already in the pool so a freshly handshaked node isn't replaced by its stale DB copy or double-counted by the IP limiter.

3. Re-admit known nodes to the active pool on traffic (bootnode/service.go)

Previously the only path into the active pool for a peer that contacts us was OnHandshakeComplete. A node demoted by a sweep — or missed at handshake time — could ping us for the entire session lifetime without ever rejoining the pool, since onNodeSeen only updated lastSeen for nodes already present.

onNodeSeen now attempts table.Add for nodes that pass the layer's fork filter. FlatTable.Get falls back to the DB, so demoted-but-alive nodes are re-admitted as soon as they talk to us; Add is a no-op for already-active nodes and still enforces IP limits and the pool's hard cap.

Verified with go build, go vet, and go test -race ./bootnode/... ./nodes/....

@pk910
pk910 merged commit 1ee5cb8 into ethpandaops:master Jul 29, 2026
1 check passed
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.

2 participants