[gzliudan]

Refs #2856, #2858.

Stacked on #2866 (ticket #2857), which has since merged: this branch starts where that one ended, so GetAncestor is available, and after the rebase onto the resulting dev-upgrade the diff shows only this PR's own two commits. The base stays dev-upgrade because the forks' branches do not exist in this repository.

Summary

A by-hash GetBlockHeaders request carrying a non-zero Skip used to walk the chain by materialising a slice of Skip+1 hashes per step. Skip comes straight off the wire and is bounded only by chain height, so one message from any peer could ask this node to allocate a chain-height-sized slice and perform tens of millions of header lookups. The fix is upstream's: the loop body is #16946's (049f5b3572, the first flag, maxNonCanonical := uint64(100) and the GetAncestor walk), the serve lookup budget comes from #21482 (017831dd5b), and the dead GetBlockHashesFromHash helper was removed by #23100 (6289137827) — none of which this tree ever took.

• The hash origin is now resolved once (first flag) and both hash-traversal directions go through GetAncestor, so a canonical chain is answered from the canonical hash index without walking.
• A walk that leaves the canonical chain is paid for out of one 100-header non-canonical budget per message; an exhausted budget ends the reply instead of walking on.
• The loop carries over the lookup budget of twice the serve cap (2 × 192). With the serve cap applied first the budget cannot be reached here, so it is a defensive bound rather than the gate that ends a walk: the gate is the 100-header non-canonical budget.
• GetBlockHashesFromHash (the hash-per-step walk itself) has no production caller left and is deleted from BlockChain and HeaderChain; the test that used it builds its expectation from the chain by number instead.

Canonical-origin replies are byte-identical, except for a reverse skip whose +1 overflows (Skip == MaxUint64): the old code repeated the origin, the new code stops after it, matching upstream. Otherwise the only wire-visible change is that a reply from a non-canonical origin stops after the budget rather than walking arbitrarily far.

Upstream reference

• #16946 — 049f5b3572 (core, eth, les: more efficient hash-based header chain retrieval): the first flag, maxNonCanonical := uint64(100), the GetAncestor walk and the non-canonical budget it spends.
• #21482 — 017831dd5b (core, eth: split eth package, implement snap protocol): the serve cap and the lookups < 2*maxHeadersServe budget.
• #23100 — 6289137827 (consensus/clique, core: API cleanup): the removal of GetBlockHashesFromHash, which upstream also found test-only.

Evidence

• TestGetBlockHeadersSkipAllocation — a forward hash query with Skip on the order of the chain height must not allocate in proportion to that skip (measured with runtime.MemStats against a benign query of the same reply size). Fails on the pre-fix code.
• Table cases added for a request whose origin hash the node cannot resolve and for a chain-height Skip; TestGetBlockHeadersNonCanonicalBudget covers a reverse walk from a side chain deeper than the non-canonical budget.
• gofmt -l / goimports -l clean, go build ./..., go vet ./core/ ./eth/, go test -count=1 ./core/ ./eth/ all pass.
• grep -rn GetBlockHashesFromHash --include=*.go . is empty.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: c40f4c3c-2574-442c-91e6-320ef0c482e3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

🟢 Approval recommended

The focused upstream-aligned change has targeted regression coverage and no unresolved review findings; tests were not independently executed.

No open issues 🎉

What changed in this PR

This PR addresses the peer-controlled header-walk allocation risk on dev-upgrade by using the existing bounded GetAncestor lookup, following upstream geth behavior.

Changes:

• Replaces hash-by-hash traversal with ancestor lookups, a shared non-canonical traversal budget, and a defensive lookup limit.
• Adds regression coverage for allocation growth, side-chain traversal, unknown origins, and skip boundaries.
• Removes the obsolete GetBlockHashesFromHash API and updates test expectations.

File	Description
eth/​handler.go	Uses bounded ancestor lookups for hash-based header requests.
eth/​handler_test.go	Adds allocation, traversal-budget, and boundary regression tests.
core/​headerchain.go	Removes the obsolete hash-slice traversal helper.
core/​blockchain_reader.go	Removes the unused wrapper for that helper.

_{Review effort: Balanced}