fix(core): let known blocks above the head advance the chain, #19417 close #2535

[gzliudan]

Problem

insertChain only skips known blocks while they sit at or below the head:

case err == ErrKnownBlock:
	// Skip all known blocks that behind us
	current := bc.CurrentBlock().Number.Uint64()
	for block != nil && err == ErrKnownBlock && current >= block.NumberU64() {

A block that is already stored with its state but sits above the head leaves err == ErrKnownBlock. The main import loop requires err == nil, so it never runs and the head does not move.

Upstream go-ethereum handles this with writeKnownBlock, which reorgs when the block does not sit on the current head and then moves the head, driven by skipBlock in the import loop. Neither writeKnownBlock nor skipBlock exists in this repository.

For --gcmode archive this is severe: once a block has been executed and its state committed without being adopted, it stays known forever, so every later attempt stops on it. Measured on a catching-up mainnet node (#2535): the head advanced 5114 blocks in 58 minutes (~88 blocks/min), with 692 sidechain imports of 50–1160 blocks each and add=2 for 284 of the 457 Extend chain events — roughly 1000 downloaded and rewritten blocks per ~6 blocks of progress, and consecutive rounds re-fetch almost the same range.

Changes

• core/blockchain.go: port geth core: import known blocks ethereum/go-ethereum#19417 (c113723f).
  • new writeKnownBlock: reorgs when the known block does not sit on the current head, then moves the head; the import loop now tolerates ErrKnownBlock instead of re-executing the block. A gap block reaching the head this way refreshes the masternode set, mirroring writeBlockWithState, otherwise its snapshot would never be written.
  • reorg stops one block short of the new head and leaves it to the caller (now both writeBlockWithState and writeKnownBlock), because applying it inside reorg as well wrote the same canonical markers twice and, for a gap block, ran UpdateM1 twice.
  • insertSidechain is renamed to insertSideChain, as upstream did.
• Two points upstream leaves open are closed here, because XDPoS chains adopt blocks without snapshots and have a wider set of head side effects:
  • the fork-choice rule lives in blockBeatsHead and writeBlockWithState reuses it, so a known block is adopted exactly when an executed block would be; an edit to one path can no longer silently diverge from the other. An unknown parent total difficulty is treated as not beating the head, so the batch keeps the current chain.
  • a known block that was not canonical yet is adopted for the first time, so its logs were never delivered; insertChain now delivers them with the rest of the batch. A rollback re-import is the opposite case — the block was canonical before, its logs already went out, and they are not sent a second time.
• Known-block adoption also refreshes the blocks-hash cache and the signing-tx cache like the canonical import path does, and the gap-block predicate is shared through isGapBlock so a zero epoch can no longer divide by zero.

Tests

• core/blockchain_test.go
  • TestInsertChainAdvancesHeadOverKnownBlocks: rolls the head back over blocks that keep their state, asserts that re-inserting them moves the head to the tip.
  • TestReorgLeavesNewHeadToCaller: pins the reorg contract.
  • TestWriteKnownBlockLogDelivery: a promoted side chain delivers its logs exactly once, a rollback re-import delivers none.
  • TestWriteBlockWithStateRejectsUnknownParentTd: keeps the unknown-parent error that sharing the fork-choice rule could otherwise swallow.
• consensus/tests/engine_v2_tests/known_block_test.go
  • TestInsertChainKnownGapBlockRefreshesSnapshot: rolls the head back below the gap block, drops its snapshot and re-imports the already executed blocks; the known-block path must move the head forward and rewrite the snapshot.

Trade-off

• The known-block path now performs a reorg and a head write, so it costs GetTd lookups and, on a gap block, UpdateM1. Those blocks were previously imported for nothing at all, so the cost replaces zero progress.
• reorg no longer writes the new head, so every caller must do it. There are exactly two (writeBlockWithState, writeKnownBlock) and both do.
• A synchronous head move on the known-block path means chainmu is held for the reorg; this is the same cost an executed block of the same segment already pays.

Verification

• make all, make quick-test, make tidy, make generate all pass.
• The new test TestInsertChainAdvancesHeadOverKnownBlocks fails on the pre-fix code (the head stays on the rolled-back block) and passes after.

geth PR: ethereum#19417
Closes #2535

[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

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5e7d8d92-d114-49ad-a6e1-6581a2d09924

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

---

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]

🟡 Changes recommended

Known-block adoption omits ChainEvent, leaving new-head subscribers and chain indexers temporarily stale.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes stalled synchronization by allowing known blocks above the current head to advance the canonical chain.

Changes:

• Adds shared fork-choice and known-block adoption logic.
• Preserves reorg side effects, logs, caches, and XDPoS snapshots.
• Adds regression and consensus tests.

File summaries

File	Description
core/blockchain.go	Implements known-block adoption and updates reorg handling.
core/blockchain_test.go	Tests fork choice, reorgs, logs, and gap detection.
consensus/tests/engine_v2_tests/known_block_test.go	Tests XDPoS snapshot restoration.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Log-delivery detection can both omit first-adoption logs and duplicate intermediate rollback logs.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

core/blockchain.go:1957

• GetCanonicalHash reflects the header chain’s canonical table, not whether this full block was previously adopted and its logs delivered (core/blockchain_reader.go:237-239, core/headerchain.go:171-196). If header sync has already canonicalized an executed side fork before these known blocks advance the full-block head, this comparison is false and the block’s live logs are silently omitted. Please use full-block/event history for this distinction, or normalize rollback/reorg log semantics so every canonical transition can emit logs safely.

			promoted := bc.GetCanonicalHash(block.NumberU64()) != block.Hash()

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[gzliudan]

replaced by #2566