fix(core,eth): stop full-sync stalls and peer drops on stored blocks, truncated and admin imports, close #2534 #2535

[gzliudan]

Closes #2534
Closes #2535

Five pre-existing defects turned up while fixing these and are carried by this PR: A14 — a total difficulty this node does not hold was dereferenced instead of reported — A17 — a block could pass for done without ever having been executed — A18, the admin importer's precheck — A19, the chain's own records disagreeing with each other charged to the peer — and A20, a parent whose total-difficulty record is gone answered as ErrUnknownAncestor, so the peer that served the batch was dropped for a record no peer can supply. All five sit in the same "a stored block cannot move the head" area, and A17 is what makes the known-block classification trustworthy. Everything else that turned up is deliberately out of scope; see Impact Scope.

---

Background

Both issues are the same stall, reported from two ends on a mainnet archive node (--syncmode full --gcmode archive, v2.9.0-devnet).

#2534 — a truncated import was reported as a success. insertChain returned nil for a batch that had stopped in the middle. The downloader read that as "the batch is done", advanced past blocks this node never received, and only failed one batch later on an unrelated symptom (handle proposed block has error ... block not found), with the head stuck below blocks that were already on disk. findAncestor then treats those stored blocks as a valid common ancestor, so the skipped range is never requested again: the node stops making progress with no error, no retry and no peer drop to explain it.

#2535 — a stored block could not move the head. A block already stored together with its state but sitting above the head never became the head. There was no writeKnownBlock and no skipBlock equivalent, so the import loop skipped it, and every following sync re-downloaded the same range: 58 minutes bought 5114 blocks, with roughly 1000 blocks rewritten per 6 blocks of actual progress.

Dropping the peer is an aggravating factor rather than the reported symptom: an unclassified local failure was wrapped into errInvalidChain, so the node not only stalled but also lost the peers that had served perfectly valid batches. Fixing the classification is what makes the stall recoverable instead of terminal.

---

Changes

24 commits — the classification refactor first, then one commit per defect (the four commits about a total-difficulty record this node cannot read take one each) — so the branch can be reviewed incrementally and taken apart one change at a time. Every defect below was confirmed to still exist on the base dev-upgrade before it was fixed, with the one exception called out in the table: A13's carry-over half is introduced and fixed inside this branch. Otherwise this is a fix for pre-existing behaviour, not a self-correction of code introduced here.

#2534 — a truncated import was reported as a success (8)

#	Defect
A1	The insertChain entry guard returns 0,nil,nil,nil when the chain is terminating
A2	The import loop only breaks on InterruptInsert, and the trailing return ..., nil is always nil
A3	InsertReceiptChain returns 0, nil on interruption
A4	insertSideChain only reports the pruned-prefix rebuild and never looks at the rest of the batch; an interrupted segment also returns 0,nil,nil,nil
A5	errInsertionInterrupted / errChainStopped are unexported and there is no IsLocalInsertError, so the downloader wraps every unclassified error into errInvalidChain and drops the peer
A7	The future tail only accepts ErrUnknownAncestor, so a batch whose later block reports ErrFutureBlock fails as a whole
A8	addFutureBlock returns a plain fmt.Errorf past the 30s window, which is then blamed on the peer
A9	The downloader unconditionally calls handleProposedBlock with the batch tail

#2535 — a stored block could not move the head (6)

#	Defect
A6	Known blocks are only skipped while current >= block.NumberU64(); there is no writeKnownBlock and no blockBeatsHead / skipBlock
A16	reorg writes the new head itself, unlike upstream go-ethereum
A11	CheckpointCh is unbuffered, so an absent receiver blocks the whole import path
A12	The sidechain ghost-state check compares Root() only and not the hash, so re-importing our own canonical block is mistaken for an attack
A13	externTd is read only once, through if externTd == nil (base core/blockchain.go:2080-2081), so the baseline the segment accumulates onto is set by whichever block first reaches that branch rather than by the fork point. The total-difficulty carry-over this PR adds for a canonical re-import lives in that same branch, and its first form kept an unreadable record in place — weighing the segment from below the fork point instead of reporting it. That second half is introduced and fixed inside this branch, so it is the one entry here that was not a defect of the base
A10	procFutureBlocks calls HandleProposedBlock for the sorted queue tail instead of the block that actually advanced the head

Found in the same area (4)

#	Defect
A17	HasBlockAndFullState is read as "this node already executed the block", but it only asks whether the header's state root resolves in the trie database. writeBlockWithoutState stores a sidechain block with no state and no receipts, so a block naming a root that happens to exist — types.EmptyRootHash included, for which newTrieReader returns before it ever opens the database (trie/trie_reader.go:44-50) and trie.New skips the resolution altogether (trie/trie.go:123) — was adopted as the head through the known-block path, with Process and ValidateState never running on it
A18	hasAllBlocks, the precheck admin_importChain runs on every batch, asked only whether the body is on disk. A batch whose blocks this node wrote as side entries was therefore reported as already imported and skipped whole: the import returned success with nothing run and the head left where it was
A19	errInvalidOldChain / errInvalidNewChain — reorg reading a record of the chain it does not find — were not registered in the classification, so the default class applied and the downloader turned them into errInvalidChain and dropped the peer: this node's own chain disagreeing with itself was charged to whoever served the batch. The known-block path wraps the very same reorg failure in ErrLocalInsertRefused, which is local, so one failure reached two opposite conclusions depending on which adoption path found it. Both are local and not retryable now, and DescribeLocalInsertFailure names them as an inconsistent local chain rather than an interruption, which is what an operator has to act on
A20	writeBlockWithState read the parent's total difficulty and answered consensus.ErrUnknownAncestor whenever it came back empty (base core/blockchain.go:1502-1504), folding two different things into one answer: a parent this node does not hold, and a parent it holds while the record beside it is gone. Only the second reaches that read — the verifiers ask for the parent's header first, and XDPoS answers ErrUnknownAncestor there for a parent it does not have — so the peer was dropped for a record no peer can supply, and the next peer for the same batch. The parent's header decides now: gone with it, ErrUnknownAncestor as before; still there, the same local condition insertSideChain reports for the same read

Also fixed here — a nil total difficulty was dereferenced instead of reported (A14, 3)

A total difficulty this node does not hold is reported as ErrLocalInsertCondition now, at the three sites that used to panic on it. All three were verified on the base branch, where the dereference happens in the middle of an import that the caller could otherwise have retried:

Base core/blockchain.go	Code	Where
:2083	if externTd == nil { externTd = bc.GetTd(block.ParentHash(), ...) } followed by new(big.Int).Add(externTd, block.Difficulty())	the sidechain scan's accumulation: a parent record that cannot be read leaves externTd nil and the Add dereferences it
:2103	localTd := bc.GetTd(bc.CurrentBlock().Hash(), current) then localTd.Cmp(externTd)	the head's own record; a missing one panics in the comparison
:2234	localTd := bc.GetTd(...) and new(big.Int).Add(bc.GetTd(block.ParentHash(), ...), block.Difficulty())	getResultBlock's competitor branch: either read can come back nil

One commit per site — guard a missing parent TD and fix the sidechain index, harden getResultBlock against missing total difficulty, guard the sidechain scan against a missing parent TD. The report is a local condition because the record lives in this node's database rather than in the block, so the failure stays out of the peer-blame and bad-block paths of the classification.

Single-block path, same area (2)

The adoption above is repeated on the entry point the fetcher uses, which never goes through insertChain:

• a known block above the head is adopted there too, with the same fork-choice rule the batch path applies;
• a block this node already executed is answered from the stored copy, instead of being run again by getResultBlock and having the result thrown away.

---

Design notes

The classification is one table, not another list. insertErrClass in core/blockchain_insert_err.go answers, per sentinel, the three questions the insertion paths ask — may a parked block be retried, may the peer be blamed, must the block be recorded as a bad block — and classifyInsertErr reads it. Adding a sentinel is one row instead of a fourth hand-kept errors.Is list. ErrLocalInsertRefused, ErrLocalInsertAheadOfClock and the two chain-record sentinels were added this way.

ErrUnknownAncestor stays out of the local set on purpose: a batch that cannot be linked to our chain is something the peer can be held accountable for. What a missing record earns is decided by what is actually missing: a parent whose header is gone keeps ErrUnknownAncestor (A20), while a parent that is there with its total difficulty gone is the local condition insertSideChain already reports for the same read.

The retryable flag is reserved for the condition that heals. consensus.ErrFutureBlock is absent from IsLocalInsertError for a different reason — inside insertChain every future block is consumed by queueFutureTail, and the one path that can still hand it out wraps it into ErrLocalInsertAheadOfClock, the one local condition a parked block may wait for. Every other local condition — a reorg this node refuses, a chain whose records disagree with each other, a record this node is missing — is registered as not retryable, so procFutureBlocks evicts the parked block instead of re-verifying it on every futureBlocksLoop tick — which is what charging all of them to ErrLocalInsertCondition had been doing at 10 Hz, with one Debug line to show for it.

blockBeatsHead is shared with writeBlockWithState on purpose. A known block must never be adopted under a rule an executed block would have lost, otherwise re-delivering a batch could move the head onto a branch that is not canonical. Upstream has no counterpart because its skipBlock decides from snapshot availability, not fork choice; XDPoS batches can be re-delivered on a branch that must not win.

reorg no longer writes the new head, matching upstream: its callers already did, so the write inside reorg was a duplicate that also forced the marker cleanup to keep a marker that should have been deleted. writeKnownBlock follows the same contract.

The checkpoint signal is a coalescing wake-up, not a queue. CheckpointCh is buffered for one pending signal and SignalCheckpoint never blocks. The receiver re-reads the chain head when it runs, so dropping a duplicate loses nothing — whereas blocking, with the chain mutex held, would stall block import whenever the staking loop is busy.

HasExecutedBlock (A17) asks for the receipts as well as the state, because "the state root resolves" is not "this node executed the block". Nothing writes receipts without writing the state beside them except a fast sync, which writes no state at all; nothing removes receipts without taking the body with them. The consequence is fail-closed: a block on disk with a resolvable root but no receipts is executed like any other, and ValidateState rejects the root its own execution does not reproduce.

The downloader learns the classification through its own BlockChain interface instead of importing core, which keeps core out of the downloader's dependency tree; make baddeps now enforces that.

---

Commit granularity and revertibility

Each commit was checked by reverting it alone against the branch tip, building it, and running the short tests of the packages this PR touches — first with the commit's own tests taken out along with it, then again with them restored, so that a revert that only looks clean is told apart from one its own assertions catch.

21 of the 24 revert cleanly. The three that do not are all apply-time conflicts — none of them is a build failure. They are:

• the classification commit — it creates core/blockchain_insert_err.go, its test and core/blockchain_testfixture_test.go, and later commits rewrite all three, so the revert hits modify/delete conflicts on those files, a content conflict in core/blockchain.go and one in eth/downloader/downloader_test.go;
• stop blaming peers for local failures and adopt the stored prefix of a sidechain segment — each is rewritten in place by the commits above it (core/blockchain.go for both; core/blockchain_insert_err.go, its test and eth/downloader/downloader.go for the first).

The 21 that revert cleanly are not all equally safe to take apart: 15 fail their own tests as soon as the fix is gone; two — adopt a known block that wins fork choice and signal the checkpoint on a reorg over an epoch switch block — leave those tests unable to build, because they are written against an API the commit introduces (blockBeatsHead's second return value, isEpochSwitchBlock), which is a stronger pin than a failing assertion; one — signal the checkpoint without blocking import — leaves the core tests hanging until the 150 s timeout, which is the stall the fix removes seen from the test side; one — guard a missing parent TD and fix the sidechain index — has no test of its own and reverts in silence; and two — harden getResultBlock against missing total difficulty and adopt a known block on the single-block import path — still pass with their tests restored, so the suite does not distinguish them.

That set is a property of the tip rather than of the commits, and it is re-measured each time the branch is restructured — the previous revision had a different one. A commit only becomes revertible by ending up above everything that rewrites it; a commit that must stay revertible cannot be rewritten underneath. Undoing one of the three is done by reverting the commits above it first, or by rebasing it out.

To keep the neighbouring fixes independently revertible, the classification commit both moves the code the later ones land on — getResultBlock reads the competitor's total difficulty before the head's, and insertBlock asks blockAlreadyImported instead of spelling the predicate out at the call site — and lands the loop's report of the block its error stopped on itself, rather than leaving the fix that needs it to insert that block against the future-block gate's comment, which a later commit rewrites. The moved statements are the same code the branch ran before, in another order.

---

Verification

Tests are the bulk of the diff: 37 test files, +5386/-1; the other 11 files carry +1846/-196. Each commit carries the tests for its own defect — the largest being core/blockchain_knownblock_test.go, core/blockchain_sidechain_prefix_test.go, core/blockchain_testfixture_test.go, eth/downloader/downloader_proposedhandler_test.go, eth/downloader/downloader_localinsert_test.go, core/blockchain_insert_err_test.go, core/blockchain_sidechain_empty_test.go and eth/api_admin_test.go.

Check	Result
make all at every commit	24/24 pass
make quick-test at the commits whose content changed	pass (exit 0)
make all at the tip	pass
make quick-test at the tip	pass (exit 0)
go vet ./core/... ./eth/... ./cmd/... ./miner/...	no output
gofmt / goimports on every modified file	no output
make tidy	"No untidy module files detected"
make generate	"No stale files detected"
make baddeps	1 violation — see below

make baddeps reports core/rawdb -> ethdb/leveldb. It is on the base branch as well — verified there — and is unrelated to this PR. The rule this PR adds for the downloader passes for the shipped code; eth/downloader's test files do import core, which is reported as a warning because it never reaches the binary. The same run also warns that the pre-existing core/rawdb -> ethdb/pebbledb rule matches neither build: the rule and the missing dependency are the base branch's, the check that now says so is this PR's.

Negative checks were run for the cases that came out of review, so that each fix is pinned to its own assertion rather than to the surrounding behaviour:

• reverting hasAllBlocks to a body-only check makes TestHasAllBlocksAsksForAnExecutedBlock fail;
• reverting the total-difficulty carry-over to keep a stale value makes TestInsertSideChainDoesNotCarryAnUnreadableBaseline fail;
• putting ErrLocalInsertCondition back into the retryable group makes TestClassifyInsertErr/local_insert_condition and TestProcFutureBlocksEvictsParkedBlockOnLocalInsertCondition fail, which is the pair the retryable/not-retryable split hangs on.

Local private-network run

A three-signer XDPoS chain from its genesis block (epoch 900, gap 450, period 2s, v2 from block 0), plus an observer, driven for 38 minutes over 1107 blocks on this branch. It exercises the paths this PR changes, and it is where B1 and B2 of TODO turned up (both re-observed there).

Scenario	Result
Three masters mining, observer joining afterwards	pass — steady 2.00 s/block; 206 of the 214 samples on which all three masters answered put them at the same height, the other eight being ±1 sampling skew plus the restart window
Observer syncing from scratch through the downloader	pass — Block synchronisation started, segments of 15 and 34 blocks, then block by block up to the network head
admin_exportChain / admin_importChain round trip	pass — the first import ran the blocks above the head (blocks=90 executed, ignored=151, 98 ms), the second was answered by hasAllBlocks and skipped whole in 10 ms
Epoch switch (block 893, round 900)	pass — all three signers and the observer logged Checkpoint!!! and Update consensus parameters, and the 2.00 s cadence is unchanged across the boundary
A master stopped for 45 s and restarted	pass — caught up in 12 s; the slower blocks during the outage (6.8 s/block) are the expected timeouts of two-of-three signing
debug.setHead on the observer, three times	pass — the stored blocks were re-imported within a second or two; the ERROR lines it leaves are B2
Head marker lowered on a stopped node while the blocks above it stay on disk, base vs this branch	base stalls permanently, this branch recovers — see below

The last row is the shape both issues are about. It is built by writing the head marker back on a stopped node (XDC db put … LastBlock <hash>) and leaving the blocks above it on disk with their receipts; debug.setHead cannot produce it, because setHeadBeyondRoot deletes the bodies and receipts it rewinds over. loadLastState then reads the lower head, and the node has to take up blocks it already executed. Both columns are the same datadir and the same target block (826), observed for 60 s each:

	base dev-upgrade	this branch
head after 4 s / after 59 s (network at 1013 → 1043)	826 after 4 s, 826 after 59 s — unmoved	826 after 2 s, 982 after 4 s = the network head
time to recover	never	4 s
Imported new chain segment	none	one batch of 82 blocks in 27.8 ms, then block by block
blocks taken up	—	152, through Writing previously known block — the stored copies, not run again
error log	55 ERROR in 59 s — Fail to processQC and processQC Block not found 14 each, verifyQC and getEpochSwitchInfo 9 each, 9 bad-block dumps — plus Synchronisation terminated and handle proposed block has error 14 each	none
bad blocks	block 1019 written to the bad-block table with Error: unknown ancestor, although the network has it as canonical; the next process to open the database reports Bad blocks in db count=1	none

So the base does not merely leave the head behind: every sync attempt terminates on the same range, the head never moves again, and a block the rest of the network accepted is written into the bad-block table, where it outlives the run. That is #2534 seen from the other end, and the row above is the fix working. The bad-block row is evidence about the base rather than a comparison — the branch is not given the chance to write one, because it never reaches the state that produces it. The run predates the last restructure of the branch, which changed a comment and no code.

---

Impact Scope

Functional changes are concentrated in core/blockchain.go (+1313/-156) and eth/downloader/downloader.go (+151/-17); eth/api_admin.go (+43/-2) carries the follow-on adaptation plus the head-aware precheck below; cmd/utils/cmd.go (+25/-2), cmd/XDC/main.go (+4/-4), ethclient/simulated/backend.go (+39/-2), accounts/abi/bind/backends/simulated.go (+17/-0), core/block_validator.go (+11/-2) and miner/worker.go (+4/-1) are the same kind of adaptation. build/ci.go (+61/-10) adds one dependency rule and the check that reports a rule matching neither build. Two new files carry the classification: core/blockchain_insert_err.go and its test.

Interface change: eth/downloader.BlockChain gains IsLocalInsertError(error) bool.

New exported API in core: ErrInsertionInterrupted, ErrChainStopped, ErrLocalInsertCondition, ErrLocalInsertAheadOfClock, ErrLocalInsertRefused, IsLocalInsertError, DescribeLocalInsertFailure, SignalCheckpoint, HasExecutedBlock; the first two replace the unexported errInsertionInterrupted / errChainStopped.

Behaviour changes:

• the downloader cancels content processing — keeping the peer — for every error IsLocalInsertError classifies, including ErrKnownBlock and consensus.ErrPrunedAncestor, which previously dropped the peer;
• the file importer and admin_importChain report an interruption instead of blaming the file, and the simulated backend no longer panics when the chain it commits to was stopped;
• a local condition a retry cannot repair is registered as not retryable, so the parked block is evicted rather than re-verified on every futureBlocksLoop tick — a reorg this node refuses, the chain's own records disagreeing (A19), a total-difficulty record this node is missing (A20);
• only a block dated ahead of this node's clock stays parked: it is the one local condition that heals, and it now carries ErrLocalInsertAheadOfClock rather than the catch-all ErrLocalInsertCondition;
• a parent whose header is on disk while its total-difficulty record is gone stops being consensus.ErrUnknownAncestor, so the peer that served the batch is no longer dropped for a record no peer can supply (A20);
• a block on disk without the receipts of its own execution is no longer taken for executed: it goes through Process and ValidateState (A17);
• admin_importChain weighs a batch by the same line the CLI importer already drew — a body below the head, HasExecutedBlock at or above it — so blocks this node only stored as side entries are imported instead of being skipped with the head left where it was (A18). It weighs the batch; it is not a head-recovery path - see Out of scope below.

Also worth a look during review: reorg no longer publishes rebirth logs for blocks that were already canonical, so an adopted block's logs are not delivered twice; and writeKnownBlock refreshes the masternode set on a gap block, which can now happen on a path that previously never reached it.

Out of scope: A15 — the verified-header cache is keyed by header hash alone and never records the level it was filled at, so a header that passed at fullVerify=false short-circuits a later fullVerify=true request. Checked line by line on both engines:

	engine_v1 (engine.go)	engine_v2 (engine.go + verifyHeader.go)
cache field	:50 *lru.Cache[common.Hash, struct{}], built at :103	:44, built at :116
lookup	in verifyHeaderWithCache :138; return nil at :139-141	inline in verifyHeader, verifyHeader.go:27; return nil at :28-30
store	:144, after verifyHeader returns nil	verifyHeader.go:206, at the end of verifyHeader
what the level steers	verifyHeader :175 (validator signature, block time) and :168-170 (testnet forces false); verifyCascadingFields :256; verifySeal :657	timestamp check :43; masternode and penalty check :138

The cached value is struct{}, so nothing in the cache can tell a header checked at false from one checked at true; the two engines differ only in where the lookup sits — v1 wraps verifyHeader in verifyHeaderWithCache, v2 inlines both in verifyHeader. fullVerify=false is reachable here: core/blockchain.go calls insertChain(..., false) for the winner re-import in getResultBlock and for the rebuilt prefix in insertSideChain, so the shorter check does get cached.

Both engine files are identical to the base branch, so A15 is pre-existing code whose fix stays in its own change (#2570). This PR's only contact with that area is handing insertSideChain the batch's verifySeals level for the tail the scan never looked at, instead of always re-importing with verification off; the cache itself is untouched.

The importers are not head-recovery tools. A block this node executed at or above a head that stops below it answers as imported, so XDC import and admin_importChain skip that range without running anything and without moving the head (the CLI half is additionally blocked by B1 below). The adoption paths this PR adds are reached by the sync paths instead - the batch importer the downloader drives, and insertBlock for a propagated block - which is what Local private-network run above exercises.

---

Notes for review

Merge order. Two of the defects listed above are not fixed here: the downloadingBlock marks that outlive the import (which short-circuits the known-block adoption added by this PR, since that check runs first) and PrepareBlock's cache keys. Both root fixes live on their own branches and both come with tests:

Defect	Root fix
downloadingBlock marks are only added, never removed	fix-downloading-block-marks
PrepareBlock reads its caches with a different key than it writes them	fix-prepared-block-cache-key

fix-downloading-block-marks should merge before this PR, because the known-block adoption paths this PR adds sit behind that check — landing this one first leaves them unreachable for blocks the downloader recently imported.

---

TODO

Two defects turned up while running this branch on a private network (see Verification / Local private-network run), and both were re-observed on the current revision. Both are pre-existing, neither is fixed here, and each wants its own issue. The files that carry them are untouched by this PR.

#	Defect	Where	Observed
B1	XDC import cannot open the chain database at all, whatever the --datadir	cmd/XDC/chaincmd.go:216 builds the node with makeFullNode, whose RegisterEthService calls eth.New and opens the chain database; :222 then has utils.MakeChain open the same database a second time	XDC import always ends in Fatal: Could not open database: resource temporarily unavailable — leveldb takes the database exclusively, so the second open fails. Three Allocated cache and file handles lines for the same chaindata in one run, the third immediately before the fatal. XDC export is unaffected because it opens read-only (:294/:297). Upstream draws this line differently: its importChain uses makeConfigNode (cmd/geth/chaincmd.go:367), so no service is registered and the database is opened once — the fork's importChain is the only chain command here that builds the node with makeFullNode
B2	A rewind leaves the engine holding a quorum certificate for a block the rewind deleted, and it reports that at ERROR level	consensus/XDPoS/engines/engine_v2/engine.go:1253	Two ERROR [FindParentBlockToAssign] Can not find parent block from highestQC proposedBlockInfo lines per debug.setHead, clearing by themselves within 1–2 s: XDPoS.FindParentBlockToAssign falls back to the current block, so the miner keeps working and nothing stalls. Only the rewound node logs it. The level is the part that is wrong — a state the node repairs on its own should not read as a fault. The engine file is identical to the base branch, and the call site in miner/worker.go (:766) is not one of the lines this PR changes

The rest of what is still open is in the unchecked boxes of Self-check checklist: a multi-epoch private-network run, a devnet/testnet end-to-end run, and co-existence with nodes on the previous version.

---

Self-check checklist

Proposed changes

• The big picture is in Background; the per-defect breakdown is in Changes.

Types of changes — _Put an ✅ in the boxes that apply_

• fix: A bug fix
• build / ci / chore / docs / feat / perf / refactor / revert / style / test

Impacted components — _Put an ✅ in the boxes that apply_

• Geth
• Consensus (the checkpoint signal on epoch-switch and gap blocks)

Checklist — _Put an ✅ in the boxes once you have confirmed below actions (or provide reasons on not doing so)_

• This PR has sufficient test coverage — 37 test files, +5386 lines; each defect has its own case, and the review findings are pinned by negative checks (see Verification)
• Commits follow Conventional Commits, one commit per defect (the four about a total-difficulty record this node cannot read take one each), each referencing the issue it closes (Refs XinFinOrg#2534 / Refs XinFinOrg#2535)
• gofmt and goimports run on every modified file
• make all builds every command, at every commit (24/24) and at the tip
• make quick-test passes at the tip and at every commit whose content changed
• make tidy and make generate report no changes
• Every commit is reverted alone against the tip (apply + build + the short tests of the packages it touches, first with the commit's own tests taken out and then restored) — 21 of 24; the exceptions are listed in Commit granularity and revertibility
• No dependencies added, removed or updated
• No unrelated refactoring, renames or bundled fixes — the two moved statements and the loop's report in the first commit exist so the neighbouring fixes stay independently revertible
• No binaries committed
• Tested on a private network from the genesis block and monitored the chain operating correctly for multiple epochs — one epoch switch covered, from the genesis block (Verification / Local private-network run); a run across several epochs, and the long-running full-sync against a real network that is the check that matters for core: blocks already known with state never advance the head, making full sync crawl #2535, are still to be done
• End-to-end test plan on devnet/testnet — not yet; the stall is reproducible by interrupting a full sync (restart the node mid-sync and watch the head advance again instead of re-downloading the same range)
• Backwards compatibility — no on-disk format change, no consensus-rule change
• Tested with XDC nodes running this version co-existing with those running the previous version — not yet
• Relevant documentation has been updated as part of this PR (the new sentinels and helpers carry godoc explaining the contract each one is used under)

[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: 7dfb7963-46dc-400b-85f3-8d0c36cf9b13

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

Gap-schedule handling remains inconsistent, and unchanged-head batches can still trigger duplicate consensus handoffs.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 48/48 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Copilot review overview

🟡 Changes recommended

Permanent local failures can hot-loop in the future queue, and the proposed-head handoff has a race with concurrent block imports.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Infer head ownership from the atomic insertion result · New
• Avoid retry loops for permanent missing chain records · New

Resolved since last review (2)

• Check whether the head moved before accepting tail == head. When a re-delivered batch already…
• Gap == Epoch is still treated as a valid gap schedule here, but both…

[Copilot]

Copilot review overview

🔵 Needs a closer look

It substantially changes consensus-adjacent import and reorg behavior while multi-epoch, network compatibility, and dependent-branch validation remain outstanding.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Infer head ownership from the atomic insertion result
• Avoid retry loops for permanent missing chain records

[Copilot]

Copilot review overview

🔵 Needs a closer look

The consensus and chain-recovery changes are extensive, and multi-epoch, mixed-version, and devnet/testnet validation remain incomplete.

Review effort: Balanced
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

It changes consensus-adjacent fork choice, reorg, and synchronization behavior while dependent-branch and network compatibility validation remain outstanding.

Review effort: Balanced
Findings: None