eth: finish the wire-layer alignment pass and bound the GetBlockHeaders header walk #2856

Follow-up to ADR-0003 (the eth wire-layer split). This spec is the "later alignment pass" that ADR-0003 twice promises and never names.

Problem Statement

Two problems, both visible only to someone reading the finished split against upstream go-ethereum.

One: a remotely reachable memory bomb survived the split. When a peer asks for block headers by hash with a non-zero Skip, the answering path walks the chain by materialising a slice of Skip+1 hashes. Skip comes straight off the wire and is bounded only by "the target block number must exist", i.e. by chain height. On a mainnet-length chain a single request can therefore ask a node to allocate a multi-gigabyte slice and then perform tens of millions of header lookups — per message, repeatable, from any peer. Upstream go-ethereum fixed exactly this in 2018; this tree never took the fix, and the split moved the vulnerable code into the new wire package without touching it. The same code is still live on dev-upgrade, so the defect is not a property of the split.

Two: the split landed with half-finished seams. Four serve loops answer requests without any bound on how much work a single message may cost, so a request made entirely of hashes the node does not have spends unbounded lookups while returning nothing. The wire package still imports the downloader purely to borrow four batch-size constants — the very dependency direction the split exists to remove. The peer's block/transaction marks, the block-payload sanity checks and the ReceivedFrom stamp still live on the node layer although the split moved every other wire concern into the wire package, so a reader cannot tell where the wire layer ends. And the node layer's type is still called by a name upstream retired in the same commit the split came from, which makes every future upstream diff read as if it were written about a different codebase.

For a node operator the first problem is an availability risk and the second is a maintenance tax. For a reviewer the cost is worse: ADR-0003 records two deviations that were only ever meant to be temporary, so the ADR currently describes the code inaccurately.

Solution

Complete the alignment pass that ADR-0003 froze as a follow-up, and fix the header-walk defect while the code is in hand.

The header walk stops materialising hashes at all: it asks the chain for the ancestor at a given distance, which answers canonical chains through the canonical-hash index and bounds any walk off the canonical chain by an explicit budget. The obsolete "give me N hashes from here" API disappears, because after this change nothing in production uses it.

The four serve loops gain two explicit limits each: a cap on how many items a reply may carry, and a separate budget on how many lookups one message may cost, so requests for unknown data stop being free. Those limits move into the wire package, which then no longer imports the downloader.

The peer marks, the block-payload sanity checks and the ReceivedFrom stamp move into the wire package's message loop, next to the ReceivedAt stamp that is already there. What remains on the node layer is exactly the reactions that need node state: fetcher enqueue, tip tracking, total-difficulty comparison and sync requests.

The node layer's type is renamed to the upstream name, with a configuration struct replacing the ten positional constructor arguments. The downloader takes the protocol version as an unsigned number and stops printing full 64-character peer ids in every log line.

ADR-0003 is rewritten to describe what the code now is, including the decisions not to follow upstream, so the next reader is told why rather than left to guess.

User Stories

1. As a node operator, I want a peer to be unable to make my node allocate memory proportional to chain height from a single header request, so that one hostile peer cannot take my node down.
2. As a node operator, I want the cost of answering a data request to be bounded by a constant rather than by how much of the request happens to be unknown to me, so that request floods cannot turn into lookup floods.
3. As a node operator, I want the fix to reach the branch my node actually runs, so that I do not have to wait for a refactor to merge before the availability risk goes away.
4. As a node operator, I want header replies to stay protocol-compatible after the change, so that other clients are unaffected.
5. As a node operator, I want header replies for canonical chains to be answered without walking the chain one header at a time, so that serving historical header queries does not pin CPU.
6. As a node operator, I want a header query that would wander deep off the canonical chain to be cut off rather than answered, so that the work I spend on a fringe query is predictable.
7. As a wire-protocol maintainer, I want the wire package to depend only on packages that sit below it, so that the package boundary means something.
8. As a wire-protocol maintainer, I want all the per-message reactions that need no node state to live in the wire package, so that I know where to look when a message is mishandled.
9. As a wire-protocol maintainer, I want the node layer to hold only reactions that need node state, so that the seam between the two layers is a rule rather than a habit.
10. As a wire-protocol maintainer, I want the block-payload sanity checks to sit next to the message that carries the payload, so that adding a packet type cannot silently skip its checks.
11. As a wire-protocol maintainer, I want the ReceivedAt and ReceivedFrom stamps set in the same place, so that transport facts are not half in one layer and half in another.
12. As a wire-protocol maintainer, I want the packet-type validation helpers to be unexported unless something outside the package needs them, so that the package's public surface states what callers actually rely on.
13. As a reviewer, I want each change in this pass to land as its own reviewable commit with no semantic drift hidden inside a rename, so that I can review behaviour and renaming separately.
14. As a reviewer, I want the node layer's type and constructor to carry the names upstream uses, so that I can read an upstream diff and this tree side by side without translation.
15. As a reviewer, I want the constructor to take a configuration struct with named fields rather than ten positional arguments, so that a call site is readable without counting parameters.
16. As a future port author, I want the downloader's protocol-version parameter to have upstream's type, so that a ported patch compiles without a cast I have to invent.
17. As a future port author, I want log lines to identify peers by a short id, so that ported logging code matches instead of conflicting.
18. As a future port author, I want ADR-0003 to list which upstream alignment items this tree deliberately did not take, so that I do not re-derive the same argument from scratch.
19. As a future port author, I want the surviving pre-2019 algorithm to be gone rather than merely unused, so that I cannot accidentally call it again.
20. As a maintainer of the issue tracker, I want the availability defect tracked as its own unit of work against the branch it affects, so that its fix can ship before the refactor it happens to live next to.
21. As a maintainer, I want the test suite to fail if the header walk regresses to materialising a peer-controlled slice, so that the defect cannot come back quietly.
22. As a maintainer, I want the tests to exercise the message loop rather than private helpers, so that a green suite means the wire behaviour is right, not that a helper is consistent with itself.
23. As a maintainer, I want the self-contained API debug tests to run in parallel, so that the suite's wall-clock time keeps up with its size.
24. As a maintainer, I want every commit in this pass to build and pass tests on its own, so that bisecting a regression points at one change.

Implementation Decisions

Header walk (the defect). Adopt upstream's ancestor lookup rather than clamping peer input. The new chain API is a bounded hash-based ancestor walk, and its signature is copied verbatim from upstream so later patches apply unchanged:

GetAncestor(hash, number, ancestor uint64, maxNonCanonical *uint64) (common.Hash, uint64)

The *uint64 is both an input and an out-parameter: it is the non-canonical header budget, decremented as the walk leaves the canonical chain. A canonical chain is answered through the canonical-hash index without walking; ancestor > number yields the zero value. Clamping Skip instead was rejected: it would leave the allocation site intact and turn legitimate large-Skip requests into refusals.

Header-serving loop. Rewritten to upstream's shape: a first-iteration flag so the hash origin is resolved once, canonical traversal via the ancestor lookup, a serve cap on headers, and a lookup budget of twice that cap. The lookup budget is what closes the "expensive misses" hole.

Serve limits. Two distinct limits per request type — a serve cap (how many items one reply may carry) and a lookup budget (twice the cap) — because they answer different questions and upstream conflates them in one constant pair. The numeric caps stay at this tree's existing values, not upstream's 1024: the answered batch sizes are unchanged by this pass, and raising them would be an untested throughput change smuggled into a hardening commit. The caps become wire-package constants, which removes the wire package's import of the downloader.

Request decoding. The four serve loops decode the query into its packet type and iterate it, instead of hand-rolling an RLP stream. Enumeration is then bounded by message size and the loops stop being four slightly different transcription of the same idea.

Removal. The per-hash "give me N hashes from here" API on the chain and header chain is deleted. After the rewrite it has no production caller; keeping it leaves a public function whose only safe use is one nobody should write.

Mark and stamp ownership. Block/transaction marks, the propagated-block payload sanity checks and the ReceivedFrom stamp move into the wire package's message loop. Fetcher enqueue, tip-number tracking with its plausibility window, total-difficulty comparison and sync requests stay on the node layer, because they need fetcher or chain-head state. Net semantics are unchanged.

Validation helpers. The two packet validation helpers become unexported once their only caller is inside the wire package.

Node-layer naming. The node layer's type becomes the upstream name, constructed through a configuration struct with named fields. The configuration carries this tree's extra dependencies (chain config, order pool, lending pool, engine); upstream's bloom-cache, checkpoint and whitelist fields have no counterpart here and are not invented. The existing two constructors collapse into one, because the exported variant has a single caller. The ethHandler adapter keeps its current embedded-struct shape rather than upstream's type conversion, because embedding is what supplies the node metadata the wire package asks for. The receiver identifier stays as it is: renaming it would touch hundreds of lines and buy nothing.

Not taken: peerset event subscriptions. Upstream's peer set grew four event feeds because it serves two protocols that must each observe joins and drops. This tree has one protocol on that peer set, so the subscriber count would be permanently zero. The synchronous peer set stays as it is.

Downloader. The protocol-version parameter becomes unsigned, matching the peer's own accessor, which removes the only cast at the call site. Log lines that name a peer use a short id via one shared helper rather than upstream's inline duplication; short ids are passed through untruncated because tests use them.

Error model and wire invariants unchanged. Responses keep this tree's error codes and formatting; the capability name, version numbers, message codes, protocol lengths and the status-handshake payload stay byte-identical. The only permitted wire-behaviour change is the GetBlockHeaders reply content for a non-canonical origin, which now stops after the non-canonical budget instead of walking arbitrarily far.

Delivery split. The defect ships first, on its own, against the branch every node runs; the rest rides on the split's existing branch. Two consequences: the same loop rewrite is written twice (once in the still-monolithic node layer, once in the wire package), and the split branch rebases over the other, resolving one conflict by keeping its own deletion of the old code.

Commit shape. One commit per decision above, ordered defect → serve limits → marks → downloader type → downloader logging → test parallelisation → rename → documentation. The rename lands last so that the behavioural diffs are not buried under a mechanical sweep, and every commit builds and passes tests on its own.

Documentation. ADR-0003 is amended in place, not superseded: the two deviations it records as temporary are rewritten as resolved, a section enumerates every item of this pass with its disposition and the reason for the items declined, and the wire-invariant statement gains a footnote scoping it to encoding rather than to reply content. Deliberate departures from upstream are recorded alongside — the retained numeric caps, the retained receiver name, the shared log helper, the retained ethHandler embedding. No new ADR, and the project glossary is untouched: every term this pass introduces is an implementation detail, not domain vocabulary.

Amendment (2026-10-24): that glossary clause is superseded for exactly two sections by #2872 — the wire-protocol vocabulary that rides with the ADR-0003 commit (ADR-0003 cross-references it) and the standalone block-schedule section. Everything else the clause guards stands.

Testing Decisions

What makes a good test here. Assert on the bytes a peer would receive and on the fact that the node survives, never on which helper was called. A test that asserts "the ancestor API was used" would pass on a build that still allocates; a test that sends a hostile query and asserts a bounded reply fails on it.

Seam 1 — the wire message loop (existing, highest). The wire package already has a fake backend satisfying the package's backend interface plus a raw peer, so a test can drive a real message loop with real RLP. Everything about header and data serving is asserted here: reply contents for every query shape this tree's tests already cover, the new bounded behaviour under a chain-height-magnitude Skip, replies for requests made entirely of unknown hashes, and the moved marks and stamps. The existing per-version header test triplet is the prior art; the new cases join that table.

Seam 2 — the node layer (existing). The node layer's own fixtures already build a protocol manager and a wired-up peer, which is where the moved marks are checked from the other side: after a propagated block, the peer must be recorded as knowing that block, and the fetcher must have been handed it.

Seam 3 — the chain (new, minimal). The ancestor lookup has no caller outside the wire package at first, so it needs its own test in the chain package: canonical chain answered without walking, walk off the canonical chain consuming the budget, budget exhaustion yielding the zero value, and ancestor > number yielding the zero value. This is one new seam and it is justified by the API being new; no further seams are introduced.

Helper test. The short-id logging helper is a pure function and gets a table test for the short and long id cases, because it is cheaper to assert than to eyeball in a log.

No tests for the rename (compilation and the existing suite are the assertion), the parallelised tests (the code under test does not change), or the removed API (its callers are gone; the wire test that used it to build expectations is rewritten against the chain directly).

Not tested, and recorded as such: the non-canonical budget is not exercised by the live-node test suite, which never sends a Skip and never queries a non-canonical origin. The ADR records this gap rather than implying coverage.

Gate. Formatting and import ordering, build of every command, module tidiness, the affected package tests, a race run on the wire and node packages, and the full suite before review. No new benchmarks and no fuzzing.

Out of Scope

• Upstream's snap protocol, snap sync and everything they pull in. This tree has neither.
• The peerset event-subscription rewrite, for the reason given above.
• Any change to wire encoding: capability name, version numbers, message codes, protocol lengths, status-handshake payload.
• Privatising the protocol name, version and length tables. Two external callers still read them, and closing that seam is a separate decision with its own blast radius.
• Adding live-node test cases for the new canonicality behaviour.
• Changing the answered batch sizes, adding benchmarks, or adding fuzzers.
• The project glossary. (Amended 2026-10-24: the two sections shipped with the split are ratified in docs: ratify the two glossary sections that ride with the eth wire-layer split #2872.)

Further Notes

Why the defect ships separately. It predates the split, it is exploitable on the branch that is actually deployed, and it touches the chain package rather than the split's own code. Bundling it into the refactor's pull request would delay the availability fix behind a review that is about something else.

Issue and pull-request linkage. The repository's default branch is not the branch these pull requests target, so a closing keyword in the pull-request body will not close this issue automatically; close it by hand once the defect's pull request merges. The split-side pull request should reference this issue without closing it, since it only moves the same fix into the new package.

Disclosure. The mechanism and its magnitude are stated plainly here because the defect is public code that upstream fixed years ago. No ready-to-send message is included.

Measurement caveat. The memory magnitude is derived from chain height, not observed: the walk stops at genesis, so the largest reachable allocation is chain height × one hash, and the same figure bounds the lookup count. No live reproduction was attempted.

Acceptance. The pass is done when: the wire package no longer imports the downloader; no production caller of the removed hash-walk API remains; a hostile Skip produces a bounded reply instead of a large allocation; the wire and node packages agree on where marks and stamps live; the node layer's constructor is a configuration struct; and ADR-0003 describes the code as it now stands, including what was declined.