Conversation
Packet hashes alias across peers (deterministic signatures, 1s Expiration granularity), so a response from one node could resolve a request sent to another. Key pending requests by hash + destination node ID and hold a slice per key so concurrent waiters don't orphan each other. Reject a second in-flight FINDNODE to the same peer: NEIGHBORS carries no reply token, so two cannot be told apart. Harden ENRRESPONSE: require a matching pending request, require the record's key to derive the sender's node ID, and install via seq-monotonic UpdateENR so a replay cannot roll a node back to an older record. Drop the unused map-returning Stats methods.
An IPv6-only record advertising udp6 but no udp was rejected outright, and UpdateENR silently kept a stale address when the new record's family differed. Extract udpEndpoint so both paths pair ip with udp and ip6 with udp6, falling back to udp for dual-stack records.
…on New failure Records without an eth entry are consensus nodes, not wrong-fork execution nodes; counting them made the EL rejection counter track cross-layer traffic, which on a dual-layer network is most of what arrives. Centralize the gate in ENRManager.RecordELAdmission and mirror it in the CL filter, where every EL node was inflating totalChecks. New() now unwinds via a single deferred guard that also cancels the context: a late failure previously leaked the NodeDB queue processors and handler cleanup goroutines hanging off s.ctx. Extract the two lookup admission closures and the repeated bootnode table-add/persist block; reuse discv5/session.Stats instead of redeclaring it.
EncodeRLP had the signature ([]byte, error) instead of (io.Writer) error, so it never satisfied rlp.Encoder. Every field of Record is unexported, so a nested Record fell through to struct encoding and serialized as c0 -- go-ethereum rejected our discv4 ENRRESPONSE with "record contains less than two list elements". discv5 was unaffected because Nodes.Encode calls the byte-slice encoder by hand. Rename the byte-slice form to EncodeRLPBytes (symmetric with DecodeRLPBytes) and add a real EncodeRLP. The receiver stays a pointer, unlike go-ethereum's value receiver, because Record holds a mutex. DecodeRLPBytes also now rejects payloads over 300 bytes, mirroring geth's decodeRecord. encode() already enforced this outbound, but ingest wrote straight to the raw cache, so an oversized wire record could be stored and re-served -- reachable since relayed records became admissible.
Adds an opt-in rendezvous mode. With --serve-all the bootnode skips EL/CL classification and fork-ID/digest filtering entirely: every discovered node is pooled into every enabled table (regardless of eth/eth2 fields) and served to every requester. Turns bootnodoor into a plain discv5 rendezvous, like a stock geth bootnode. Default off; classification + fork filtering unchanged. (cherry picked from commit c9aed7e) Adapted to this branch, which refactored the paths the original patched: - The two OnNodeFound closures are now admitELLookupNode/admitCLLookupNode and return services.AdmissionResult, so the guard moved into those methods. - Gate checkAndAddNodeV4 too. The original missed the discv4 path, so discv4-discovered nodes stayed fork-filtered and were recorded as bad nodes, contradicting "every discovered node is pooled". - Gate the three configured-seed paths. Fork-filtering a seed under serve-all can reject the only seed, leaving the table empty so discovery never starts. - Skip the filters under serve-all instead of overriding their results. This branch records admission stats inside the filters, so calling them would move the counters for decisions never made and report rejections of nodes that were in fact admitted. - Keep onNodeSeen's EL-xor-CL short circuit: computing both up front made the CL filter run for every EL node, drifting its counters in classified mode. Also dedupe onFindNodeV5 results when serving both layers. A node can sit in both tables, so the concatenated response returned it twice. Serve-all makes that systematic, but it already affected dual-stack peers on a shared identity, so the fix is not gated on the flag. Known limitation: a lookup result is still admitted only to its own layer's table. With separate EL and CL identities a peer found via one identity is not served to the other's requesters until it is seen directly.
- elconfig: drop the duplicate crc32 walk (sums) and read checksums off allForkIDs; remove blockHead and the sentry special case, which cannot affect any validate() outcome under the static head stance. Verified equivalent by differential test across every schedule shape, time head and perturbed fork ID. Copy the slice out of GetAllForkIDs now that validate depends on it. - clconfig: slices.Sort/Compact for the boundary walk; GetAllForkDigests projects from GetAllForkDigestInfos instead of duplicating it. - stats: drop Discv5Stats.PacketsReceived/PacketsSent, summed but never read (the UI takes packet totals from transport metrics). - lookup: slices.Contains for the local-identity check. - Fork names: strings.ToUpper/Join instead of hand-rolled byte arithmetic that corrupts any name not starting a-z.
An off-path attacker could move the ENR this bootnode publishes, and a
bonded peer could reflect amplified NEIGHBORS traffic at a third party.
Both follow from the same gap: nothing proved a peer was reachable at the
address it claimed.
discv4 PONG: side effects (bond, external-IP vote, ENR refresh) ran before
any request match, so any well-formed PONG applied them. They now require a
match that is
- source-bound: the PONG source IP must equal the address the PING was
sent to. PendingRequest snapshots that IP at send time, because
getOrCreateNode rewrites ToNode.Addr() from every inbound packet,
including the spoofed one this check exists to catch.
- PING-typed: getPendingRequests ignored PacketType, and peers know the
hashes of packets we sent them, so an ENRREQUEST hash matched as a
reply token.
- consumed once: the entry was removed by the caller, so for the 500ms
Ping() wait every replayed PONG re-cast the IP vote. Varying the
spoofed source reached MinReports/MinDistinctIPs off one exchange.
discv4 bonds: tracked per proven IP rather than per node ID, so a bond
earned at one address no longer serves requests from another. Ports are
excluded, matching go-ethereum (checkBond passes ip.Addr(), dropping the
port) and avoiding a false negative when a NAT mapping rotates inside the
24h bond. Per-IP rather than a single address so dual-stack peers keep both
bonds. handlePing no longer grants a bond for merely receiving a PING: we
pong whatever source the packet claimed, which proves nothing, and that
was the remaining route to bonding a victim's address.
discv5: MatchResponse validated only requestID and node ID, so a PONG could
match and consume a pending FINDNODE, firing the IP-vote path and stranding
the lookup whose channel it closed. It now checks the response against the
stored request; handlePong returns early when unmatched.
IP discovery: consensus counted repeat reports from one peer as independent,
measuring independence only by spoofable source IP, so a single node ID with
three spoofed sources met both thresholds. Distinct reporter node IDs are now
required too, and the reporter key is the full ID rather than an 8-byte
prefix that let two peers count as one.
CL fork filter: Filter was both predicate and stats recorder, and ran on
per-packet classification paths, so the UI's Fork Filter card counted
packets rather than admissions. Split into a pure Matches and a counting
Admit over a shared classify, mirrored as ClassifyCLNode/AdmitCLNode and
ClassifyELNode/AdmitELNode; RecordELAdmission and its five-site paired-call
contract are gone. Renaming made every call site a compile error rather
than a silent behaviour change. classify holds the lock once, which fixes
a concurrent map read/write against Update() on oldForkDigests (an
unrecoverable abort, not an error) and makes TotalChecks always equal the
sum of the buckets.
Also: count FINDNODE responses after the unsolicited gate, not before.
Follow-up from review of c3d9e16. The endpoint proof was incomplete: the recorded destination could differ from where the packet went, and the proven address did not reach the code that consumes it. - Ping/Findnode/RequestENR read Node.Addr() separately for the To endpoint, the pending request and the send. Concurrent receive workers rewrite that address between those reads, so DestIP could name an endpoint the request never went to: legitimate PONGs rejected, or a bond granted for an address that never received a PING. Each sender now captures the address once and addPendingRequest takes it explicitly, so the recorded and actual destinations cannot diverge. - OnPongReceived now carries the proven address. The bootnode was re-reading from.Addr() for the IP-discovery source, which any later spoofed packet from the same identity rewrites, letting an unproven address count toward the distinct-source threshold. - ENRRESPONSE and NEIGHBORS apply the same destination check as PONG. ENRRESPONSE additionally requires an ENRREQUEST: it matched on packet hash alone, so a peer could answer with the hash of some other packet we sent it and resolve the wrong waiter. pendingFindnodeLocked stays address-agnostic; it enforces one in-flight FINDNODE per peer, which is not an endpoint question.
…t ENR IP EnableIPDiscovery defaulted to false and was read in exactly one place — reconcileStoredENR's startup ip6-stripping branch. Nothing in the runtime path consulted it: New built the IPDiscovery service unconditionally, both OnPongReceived callbacks were wired unconditionally, and there was no CLI flag at all. So the feature ran for everyone while claiming to be off. updateENRWithDiscoveredIP also checked neither the flag nor ENRIPProvided, so a bootnode started with an explicit --enr-ip had that address (and, on a shared socket, its port) overwritten in the live ENR and republished once peer reports reached consensus. reconcileStoredENR restored it on restart, making the symptom a flip-flop rather than a permanent change. - Default EnableIPDiscovery to true so current behaviour is preserved, and add --enable-ip-discovery to turn it off. Skipping construction is the whole enforcement: onPongReceived already returns early on a nil service. - Refuse to move an address the operator set explicitly, per the contract Config.ENRIPProvided already documents and only startup honoured. Behaviour change: with the default now true, reconcileStoredENR's !EnableIPDiscovery branch stops firing, so a stored ip6 is retained at startup instead of stripped. That is coherent — with discovery on, let it correct the address rather than discard it — but it is a startup change, not only a flag default.
…ng sessions handleOrdinaryPacket located the session by the unauthenticated src-id header, migrated its address before decrypting, and on decrypt failure deleted it and challenged the packet source. An attacker who knew only a peer's public node ID could therefore migrate then destroy that peer's session from any address, and repeat it — permanent session denial, with in-flight requests dying on timeout. Four routes closed: - Address migration now happens only after DecryptMessage succeeds. AES-GCM over the header proves possession of the session key, and the source address is not in the AAD, so a NAT-rebound peer still migrates from its new address. - Decrypt failure keeps the session. Recovery is a handshake and Cache.Put replaces the entry by node ID when it lands, so deleting bought nothing and was the whole DoS. The challenge still goes to the packet source: a peer that genuinely lost its keys sends a random packet, which by definition fails to decrypt, and its source is the only address it can be reached at. Answering at the session address would be a reflection primitive against the real peer. - The GetByAddr fallback is gone. Sessions are keyed by sender node ID and every creation site uses the correct ID, so it never helped a correct peer — but it reached the same deletion without knowing the victim's node ID at all, by spoofing its IP:port with a random src-id, and was an O(n) scan over up to 1000 sessions driven by unauthenticated traffic. - A forged WHOAREYOU no longer replaces a live session. Not deleting was insufficient: with a request in flight, recovery derives fresh keys and Put overwrites the session before the remote proves anything, leaving a session present but unreadable by the real peer. Sessions now remember the nonces of recent ordinary packets they sent (bounded, 16) and a challenge quoting a nonce we never sent is dropped. Also: Session.RemoteAddr becomes remoteAddr behind an Addr() accessor, fixing an unlocked read racing UpdateAddr; and Session.String() no longer holds the read lock across Age()/IdleTime(), which take it again.
…on it PendingRequest recorded no address, and all three AddRequest callers re-read n.Addr() for the send, so the recorded request and the transmitted destination came from two separate reads of state that a newer ENR can move — including one supplied by the peer being verified. This is the divergence already fixed on the discv4 side in 6dd2f08. AddRequest now takes the destination explicitly and each caller captures n.Addr() once for both the record and the send. MatchResponse returns the matched request instead of a bool so callers can reach DestAddr. handlePong gates the OnPongReceived IP-discovery vote on the PONG's source matching that destination by IP. A session peer can send a correctly typed, correctly encrypted PONG from a forged source, and that source is what feeds the distinct-reporter threshold. Response delivery stays source-agnostic so NAT rebinding and mobile peers keep working — only the vote needs the endpoint proven, which is why the check sits at the side effect and not in the match. handleNodes and handleTalkResp continue to ignore the result; their delivery semantics are unchanged.
getOrCreateNode rewrote a known node's canonical address from every inbound packet's source, before any handler checked expiration or solicitation. That address is what every sender reads and what sendNeighbors republishes, and nodes.NewFromV4 stores the handler's own object — so a peer could name a third party in NEIGHBORS at an address of its choosing and we would republish it to every FINDNODE querier. HandlePacket also refreshed liveness and fired OnNodeSeen before validation, and OnNodeSeen can emit PING/ENRREQUEST traffic. - lookupOrCreateNode uses the address only when creating an unknown node, and is now read-locked on the hit path. handleNeighbors uses it too, so a claimed record can no longer move a third party's address. - promoteAddr installs the address, called only from handlePong for the destination a matched PING was sent to. It promotes req.ToNode as well as fromNode: ping/lookup build v4 nodes ad hoc, so the object a sender read can differ from the one the handler resolved. - noteSeen/noteProven split the pre-dispatch work. noteSeen runs in every handler right after the expiration check — liveness is identity-scoped and the signature authenticates the identity, and withholding it would evict a peer that is actively signing packets but whose bond has lapsed, which then returns with no proven addresses at all. noteProven adds OnNodeSeen and sits behind each handler's proof gate. handlePing's reciprocal PING now targets the source it just ponged rather than the canonical address. Without this the split blackholes any peer that moves: it would be pinged only at its old address, never answer, never bond, and be refused service permanently. This is not a new lever — PINGs are signed, so an attacker can only ping as themselves, and a spoofed source yields one PONG plus one rate-limited PING at the victim, neither larger than the trigger. Scope: this migrates the handler's node, not the routing table's copy. nodes.Node stores addr as a snapshot taken at admission and has no production SetAddr caller, so table-driven pings keep using the frozen value exactly as before. Fixing that needs nodes/iplimit.go re-keyed, since it counts per IP at Add time; tracked separately. Adds discv4/protocol/handle_packet_test.go — the first coverage HandlePacket has ever had, including the moved-peer regression test.
- Cache.Put evicted whenever the map was full, without checking whether the key already existed. fa30b68 made that reachable: handshake recovery now retains the stale session instead of deleting it first, so replacing it at capacity evicted an unrelated live peer and left the map one short. Only evict when the Put actually grows the map. Adds the first tests for discv5/session. - handleENRRequest called neither noteSeen nor noteProven, so accepted ENRREQUEST traffic stopped refreshing LastSeen and never re-admitted the peer, letting an actively communicating node age out. It now matches every other handler: noteSeen after the expiration check, noteProven after the bond gate. - Widen the sent-nonce window from 16 to 64. Too small silently drops a legitimate peer's restart recovery until its next packet; the window only has to cover what we can send to one peer within a request lifetime. Noted, not changed: GetPendingRequestForNode picks an arbitrary pending request for the peer, so with concurrent requests in flight recovery can replay one the challenge did not refer to. That predates these commits — the nonce check gates whether recovery runs at all, not which request it carries.
- discv4 noteProven called noteSeen, but every call site already did, so IncrementPacketsReceived double-counted on most packets. It now only fires the callback, which is the decision it actually owns. - discv5 SendMessage and SendMessageFrom were 122 near-identical lines, and the RecordSentNonce fix had to be pasted into both. SendTo(data, to) is literally Send(data, to, nil), so SendMessage delegates. ~120 lines gone and the sent- nonce invariant has one home. - Delete discv5/ipdiscovery.go: an unreferenced, diverged copy of services/ipdiscovery.go, missing the distinct-reporter hardening and taking no reporter IP at all. The spoofable version of a function just hardened. - Delete getPendingRequests: no production caller, and the one lookup helper with no type or destination binding, i.e. the shape the rest of this work exists to remove. The two tests use pendingRequestsFrom. - Delete four callerless ForkDigestFilter accessors and an unreachable outcomeNotCL switch arm; drop the packetHandler alias for proofHandler; make nodes.ENR delegate to Record rather than repeat it. Efficiency, both measured on the paths they sit on: - enr.EncodeRLPBytes took the write lock to read its cache, costing 8.6x under contention at 10 cores. A bootnode serves the same records to every requester, so those are exactly the contended ones. Read path now takes RLock. - discv5 compared session and packet addresses via String() on every authenticated packet: 77ns/6 allocs against a 271ns decrypt. Now compares IP and port directly, 1.9ns/0 allocs. Also trims comments: the "unauthenticated source must not become canonical" invariant was written out four times; it now lives on lookupOrCreateNode with the others pointing at it.
73aff56 replaced sess.Addr().String() != from.String() with an explicit IP and port comparison to avoid two allocations per authenticated packet, but dropped UDPAddr.Zone. Cache.GetByAddr still matches on the full address string, so for scoped IPv6 the two could disagree: a peer that changed interface would not migrate here, then miss GetByAddr on the WHOAREYOU path and fail to recover. Comparing Zone restores exact equivalence with the old String() form and is still allocation-free.
checkAndAddNode admits EL and CL independently, and each admission builds its own nodes.Node wrapping the same v5 node — two objects, two last-seen fields. onNodeSeen classified EL-xor-CL, so for a record carrying both eth and eth2 only the EL copy was ever refreshed. The CL copy's last-seen stayed at the zero time from admission onwards, making it look infinitely old to the CL table's age-based sweep while the peer was actively talking to us. The xor existed to keep the counting filter off the per-packet path. Classify is pure since the Classify/Admit split, so both layers can be evaluated with no counter effect and ~19ns for the second call. Test asserts both tables' last-seen advance; with the xor restored the CL side reads 0001-01-01.
Review of 165af25 was right that the test proved less than claimed, and that the commit message overstated the bug. NewFromV5 ends in v5.SetStats, so the v5 node shares stats with whichever wrapper was constructed last — CL, since checkAndAddNode builds it second — and handleMessage calls SetLastSeen on the v5 node before OnNodeSeen. So on a freshly admitted peer the handler already refreshes the CL copy for free, and the "CL last-seen stays at the zero value" result came from the test calling onNodeSeen without the handler's preceding SetLastSeen. The staleness is real but narrower: a re-admission (an ENR refresh, say) builds a fresh wrapper, FlatTable.Add keeps the entry it already has and discards the new one, but SetStats has already repointed the v5 node at the discarded copy. From then on the table's own object is refreshed only by onNodeSeen, so classifying one layer and not the other strands the other. The test now re-admits, then applies SetLastSeen and onNodeSeen in the order the handler produces them, and seeds a known admission timestamp so a regression shows a stale time rather than a zero one. With the xor restored the CL entry reads its admission time while the refresh is an hour later. The fix in 165af25 is unchanged and still correct.
Rows in both tables are per node and layer, but nodeid was the sole primary key, so a dual-layer peer's second write replaced its first while every read filtered on (nodeid, layer). Nodes lost a layer on restart; bad_nodes lost the suppression that stops repeated ENR requests.
FlatTable.Add marked nodes dirty but never enqueued them, so organically discovered nodes lived only in memory and were lost on restart. The queue consumer also abandoned its channel backlog on context cancellation, and Stop cancels before closing, so Close now refuses new work and flushes what is left. The full upsert wrote last_active as NULL while its branch cleared the DirtyLastActive that admission had just set, leaving configured bootnodes sorting as the most inactive rows.
TotalNodes came from the database and ActiveNodes from memory, so the six consumers subtracting them reported more active than total and a negative inactive count whenever a write had not landed yet. GetStats now derives the union and the difference directly.
…ones discv5 registers first and rejected anything it could not decode, so every ordinary discv4 packet on the shared socket was counted invalid before the transport re-dispatched it: 17749 of 21140 received in a devnet run. Only the dispatcher knows the final outcome, so it counts fallthrough and unhandled separately and the UI reports both.
A PONG advertising a newer sequence spawned an unguarded RequestENR, which always PINGs and sleeps 500ms before the ENRREQUEST. Its PONG re-entered handlePong with the cached sequence still stale, so refreshes multiplied at RTT speed: a scripted peer saw 25433 PINGs and 18662 ENRREQUESTs in two seconds, matching a devnet capture of ~4000 cycles across two peers during a fork. The claim is taken at the trigger, not inside RequestENR, because a goroutine descheduled past the winner's release would otherwise become a new winner. A sequence observed mid-refresh still earns one more round; failures retry twice with backoff. RequestENR itself is unchanged, so the lookup path is unaffected. Also makes the service-level guard atomic: Load-then-Store let two callers through, and delete-on-completion could drop a later claimant's entry.
The refresh ran on a free-running one-minute ticker, so the advertised eth/eth2 fields kept the previous fork for up to a full period after activation — 8s, 20s, 22s and 51s across six devnet transitions. It now waits for the next scheduled boundary, with a one-minute reconciliation tick as the backstop for a missing schedule, a clock jump, or a boundary passed during startup. Boundaries come from the raw fork epochs rather than GetAllForkDigestInfos, which deduplicates by digest: the eth2 next-fork tuple changes at a boundary even when the current digest does not.
- batchUpdate cleared every dirty flag after writing, discarding any flag marked while the write was in flight: that caller saw the node already queued and did not enqueue it again. Clear only the observed flags and requeue the remainder. - The full upsert's conflict clause never assigned last_active, so an existing row kept a stale or NULL timestamp. COALESCE so a caller without one cannot blank a stored value. - A peer advertising a higher sequence in every PONG could re-arm the ENR refresh indefinitely, holding a goroutine and its traffic until shutdown. Bound the rounds one claim may run; a later PONG opens a fresh claim. - The fork overflow guard divided by a product that could itself wrap to zero, panicking the maintenance goroutine. Check each multiplication first. - Default the refresh backoff when ExpirationWindow is unset, so a retry is delayed rather than immediate.
- Clearing the observed flag bits could still drop a same-bit re-mark made while the write was in flight, since the new mark is indistinguishable from the one written. Snapshot a generation alongside the flags and skip clearing entirely when it moved. - The refresh round bound was per claim, so a peer could send one PING, take the reciprocal PING's higher-sequence PONG, and open a fresh four-round claim immediately. Exhausting the rounds now starts a cooldown. - The signed-range check subtracted an unsigned offset from MaxInt64, which wraps for an offset past MaxInt64 and let an unrepresentable timestamp through.
- The queue-set entry was removed after the flags were cleared, so a caller that marked the node in between had its QueueUpdate swallowed as already-queued and then the entry deleted underneath it. Clear after removal instead. - Bounding rounds per claim did not bound the rate: a peer could advertise one increment per claim, finish in a single round, and reopen on the next PONG. Every completed claim now starts a minimum interval, with the longer cooldown reserved for an exhausted one. - A bump observed during a cooldown was remembered but never fetched unless another PONG arrived later. The cleanup tick now resumes those.
A failed per-field update, and a transaction that failed at commit, both still had their snapshots cleared as if persisted, so the ENR, stats or timestamp was silently lost until an unrelated change happened to mark the node again. Track per-node write failures, discard the processed set entirely when the commit fails, and requeue anything not confirmed written.
Requeueing unwritten nodes keeps data from being lost, but on a persistent database error the requeue refills the batch and the next pass runs immediately. batchUpdate now reports whether it committed, and the consumer sleeps proportionally to the run of failures, capped at five seconds.
Row-level write errors leave the transaction callback returning nil, so a committed transaction was reported as success and reset the backoff even though the failed nodes had been requeued — retrying every 10ms. A batch now counts as successful only when every node in it was persisted.
…next nextForkBoundary only considers activations strictly after now, so a refresh firing on the boundary before the new digest is computable found no change and then armed for the following boundary — a full backstop minute away. A devnet BPO transition took 75s that way, worse than the ticker it replaced. The refresh now polls every second for 90s after any boundary, so the lag is bounded by the poll interval rather than by where the backstop chain happened to land.
The composite-key change had no end-to-end coverage: no real peer advertises eth and eth2 together, since EL and CL run as separate identities with separate keys, so no devnet topology produces one. Serve-all is the reachable path — it pools every discovered peer into every enabled table, so under it an ordinary CL-only node occupies both layers and every peer hit the collision. Verified against the old key: only one row persists and the reload fails.
…metrics - probeV5Support ran inline in the lookup admission path and each probe waits a request timeout, so a 16-node NEIGHBORS response stalled the maintenance loop for over a minute. It now runs after admission, bounded to 8 in flight, and resolves the table entry so the result lands on the object the table kept. - FlatTable.Add refreshed only a newer ENR for a known node, so a peer admitted as v5-only by a completing handshake lost its v4 pointer when the v4 admission followed. Protocols are now merged into the existing entry. - Two discv5 identities share a socket and each rejects the other's packets. That is identity demultiplexing, but it was counted and displayed as other-protocol traffic. Handlers now carry a protocol label and only a real protocol change counts. - The aggregate ping RTT averaged the per-identity averages; it is now weighted by each identity's pong count. - Reset left the two dispatch counters untouched.
- The probe decision read the incoming v4-only wrapper, so a node the table already knew to be v5 was re-probed on every rediscovery and could occupy all the slots that genuinely v4-only nodes needed. The table entry decides now, and one probe per node is in flight at a time. - Resolving the entry before the ping did not close the wrong-object window: the entry can be swept or replaced during the round trip. The result is applied to whichever wrapper the table holds when the answer arrives. - The merge adopted a protocol pointer without checking record freshness. Senders use that pointer's own address, so an older record would aim the protocol at an endpoint the peer has left; adoption now requires a record at least as new.
Adoption only ever fills an empty slot, so a pointer installed from a stale record is permanent. The freshness check and the install ran as two separate operations, letting a concurrent admission advance the entry in between and the older pointer land anyway. AdoptProtocolsFrom now does both under one hold of the node's lock. The v5 probe had the same shape: the probed node was built before the round trip and applied after it, so an ENR that advanced while the ping was outstanding left v5 traffic aimed at the address that happened to answer — and HasV5 then suppressed any corrective probe. SetV5AtSeq discards a result whose record is no longer current.
Splitting adoption from ENR advancement left them interleavable however each was guarded individually: a newer admission could adopt, an older one install its pointer against the still-old record, and the newer one then advance the record, stranding an endpoint UpdateENR does not refresh for v4. AdoptProtocolsFrom now does both under one hold and reports each outcome. Self-merge is also a no-op now. Re-admitting a table entry passes it to itself, and snapshotting its own pointers before taking the lock could reinstall a protocol a concurrent clear had just removed.
Adoption only filled empty slots, so a pointer taken from an early record was permanent: a later record advanced the ENR but could not replace it, and the table went on serving that endpoint. UpdateENR refreshes the v5 node, but nothing refreshes a v4 node's address, so the stale one persisted. Each pointer now records the sequence it came from, and adoption replaces one sourced from an older record. Writing the record's claimed address into the v4 node instead would undo the endpoint-proof work: the table shares that object with the discv4 handler, where only a matched PONG may move an address.
The pointer was stamped with its carrier's sequence, so a pointer built from an older snapshot was marked current and no later admission could replace it. The label now comes from the record the pointer itself carries. A discv4 node created from an enode has no record, so it falls back to the carrier's sequence — without that every such pointer would be labelled 0 and become unreplaceable, which a regression test caught.
An ENR sequence orders signed records. It does not order endpoint proofs, and the per-pointer sequence labels added here treated them as one thing: a discv4 pointer's ENR can advance while its proven address deliberately stays put, so labelling it by record left a proven-old address marked current and unreplaceable. Each attempt to patch that produced another provenance edge case. The table does not arbitrate endpoint freshness. checkAndAddNodeV4 hands the handler's own node to the table, promoteAddr moves that object's address on a matched PONG, and onFindNodeV4 serves the pointer — so the served address already follows proof without adoption doing anything. Adoption fills an empty slot, which is all the original v4-capability loss ever needed. Also here: ApplyProbeResult, so an asynchronous probe applies both protocol outcomes under one lock hold and only while the record it measured is still current; nil-safety in UpdateENR; SetV5AtSeq validating the pointer's record as well as the carrier's; the sequence reads hoisted out of n.mu so it never nests with a protocol node's mutex; and the removal of SetAddr, which had no callers. Part of this reached the branch as an unreviewed tool-generated commit. That has been folded in here, reviewed, and mostly reverted.
CheckProtocolSupport snapshotted the record before probing and then installed pointers rebuilt from that snapshot. It needed no concurrency to go wrong: on a successful v4 ping it fetched a fresh ENR and advanced the node's record itself, then discarded the refreshed probe object and rebuilt from the record it had just superseded. The two clear paths were unguarded as well, so a probe that started before a newer admission could remove the pointer that admission installed. The probe objects themselves are now installed, and the fetched ENR is applied only after ApplyProbeResult so the sequence the install is gated on cannot move underneath it. It is attached to the object that was probed rather than to whatever V4() returns by then, which a concurrent admission may have replaced.
Covers what only a devnet reaches — a fork activating under load, real peers, wire evidence, and restart against a populated database — since every serious defect found in this component came from one of those rather than the unit suite. Includes the discv4 packet-size table and the shared-netns tcpdump recipe for telling who originated traffic, the --serve-all standalone setup that is the only way to exercise the composite key against real peers, and the harness traps: WAL sidecar files, Kurtosis reassigning host ports on restart, hyphenated fork names breaking naive scraping, and no extra-args hook for the packaged service.
An empty routing table previously produced a silent (zero-packet) response, so go-ethereum's querier waited out its full request timeout instead of returning early on the first reply. Always send >=1 NEIGHBORS packet. Also advertise the node's real TCP port from its ENR instead of copying the UDP port.
go-ethereum honours only the first NODES packet's total and reads at most 5 packets; sigp/discv5 caps at 16 nodes. Cap the served set at 15 nodes / <=5 packets so no served node is silently dropped by a requester.
… is full lookupOrCreateNode previously returned a non-retained node once the map was full, so under a flood of distinct signed IDs a genuinely new peer's inbound PING marked bond state on a discarded object and could never bond (memory-growth DoS turned into a bonding-lockout DoS). When full, evict one unbonded entry to admit the new node; bonded, endpoint-proven peers are never evicted this way. Adds regression tests for this and the two preceding NEIGHBORS fixes.
…ap flood eviction discv4/discv5 remainders on develop: NEIGHBORS, NODES cap, node-map flood eviction
At mainnet scale these scale with client population rather than with time, and the unexpected-WHOAREYOU line is remotely triggerable. info now carries startup, fork transitions and periodic aggregates only.
Kept as local notes; not repo content.
pk910
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This pull request has 50 commits. It changes 71 files. It adds 6725 lines and
it removes 1586 lines. Approximately 4000 of the new lines are tests.
The work has five parts:
--serve-alloption.Discovery: a peer must first prove its address
Before this change, a packet with a false source address could change the
state of bootnodoor. An attacker could use this to make bootnodoor send data
to a third party.
The new rules are:
to.
replace a session. A WHOAREYOU packet with no pending request no longer
deletes the session.
Before this change, two requests in the same second were the same bytes.
Then a response from one peer could install the record of a different peer.
Discovery: one refresh at a time
A PONG with a higher ENR sequence started an ENR refresh. Each refresh sent a
PING, and that PING caused a new PONG. The new PONG started the same refresh
again.
The devnet test showed a maximum of 980 packets in one second. bootnodoor sent
2575 PINGs to one peer in 51 seconds. After the fix, the same peer receives a
maximum of 14 PINGs in 60 seconds.
Fork behavior
for a timer of 60 seconds.
to the next boundary, and the refresh was late.
Database
nodestable and thebad_nodestable use the key(nodeid, layer).Before this change the key was
nodeidonly. Then a peer in two layers lostone layer at each restart. The file
db/schema/20260729200000_node_layer_key.sqlmakes this change. bootnodoorruns the migration at start-up. A down migration is available, and a test
covers it.
them, but it did not put them in the write queue.
Counters, the web UI and the responses
subtraction gave a negative count of inactive nodes.
Invalid Packetscounts only thepackets that bootnodoor cannot read. The packets of a different protocol go
to the new
Other Protocolcounter, which the overview page shows.the layer.
/debug/pprof/pages are available only with the--pprofoption.Before this change the pages were always available.
gave no packet, and the peer waited for its full timeout.
change it showed a copy of the UDP port.
go-ethereum reads 5 packets, and it removes the packets after these.
peer can make a bond. bootnodoor never removes a bonded node in this way.
New option
--serve-allstops the classification of the layers and the filter on thefork ID. bootnodoor then puts each peer in all the tables that are available.
Log
Five messages for each peer are now at the debug level. The info level shows
the messages at start-up, the fork transition messages and the
lookup completemessage. One of the five messages was a risk: a remote peercould send packets that cause the message. To see the events for each peer,
use
--log-level debug, or read the/metricspage.Structure
The IP discovery code moves from
discv5/ipdiscovery.gotoservices/ipdiscovery.go. The--enable-ip-discoveryoption now controls it.An explicit
--enr-ipvalue always has precedence.Tests
go build ./...gives no errors.go test ./...passes for all packages.pending requests, the ENR refresh, the layer key, the migration in both
directions, the fork boundary, the two layers, the dispatch counters and the
--pprofoption.Devnet results
Two devnet series ran with 7 pairs of clients. The results are:
from 2 to 5. It made one step for each boundary.
6 seconds.
ethfield and theeth2field were the same bytes as the fields ofgeth and of Lighthouse, at Fulu and after BPO-1.
blobGasUsedwent from0x120000to0x180000.Invalid Packetsstayed at 4.Inactive Nodesstayed at 0.fix did not answer the FINDNODE of a new peer. This build answers it.
--serve-all, 25 node IDs were in both layers. After a restart,bootnodoor read 27 rows and 26 rows from the database.