fix(core): queue all future blocks of a batch instead of failing

[gzliudan]

Summary

insertChain treats a mid-batch ErrFutureBlock as a fatal verification error: both future-block loops only accepted ErrUnknownAncestor, so a batch whose first or middle block was in the future stopped at its second block and returned consensus.ErrFutureBlock. The downloader wraps any non-nil InsertChain error into errInvalidChain (eth/downloader/downloader.go) and Synchronise answers that by dropping the delivering peer, aborting the round and retrying with another peer. Near the chain tip this repeats and continuously evicts innocent peers.

Root cause

The XDPoS engines check the header timestamp before the parent lookup and with zero tolerance (header.Time > now), unlike upstream geth's 15-second allowedFutureBlockTimeSeconds. Consequently the children of a future block fail the future check too — they surface as ErrFutureBlock, never as ErrUnknownAncestor, so the inner loops (which only accept ErrUnknownAncestor) stop at the second block and the rest of the batch is silently skipped.

Before #2534 the same scenario silently dropped the tail and failed one batch later with a bogus bad-block report.

Fix

Accept consensus.ErrFutureBlock in both future-block loops of insertChain (first-block path and tail path), so the whole tail enters the futureBlocks queue and the import reports success. No error is swallowed: the loops drain naturally to (nil, nil) and genuine verification errors still propagate. The queued blocks are imported by procFutureBlocks (100 ms ticker) once their timestamps are reached, matching the upstream geth design intent.

Blocks beyond the now+30s future window still return an error (abort + peer drop), which is the documented behaviour for a local clock more than 30 seconds behind — an environment problem, not an invalid chain.

A genuine verification error on a block after the queued future tail (e.g. a malformed validator field, which the XDPoS engines check before the timestamp) also surfaces: it is recorded as a bad block and returned, so only a fully drained tail reports success. Future, pruned-ancestor and known blocks remain exempt from the bad-block report — they are legitimate chain states. The same applies to a genuine error after the queued prefix when the batch starts with a future block: it is recorded and returned as well. When the known-block import PR lands, its final-return filter reports the same block again; WriteBadBlock deduplicates by (number, hash), so the only effect is one extra BAD BLOCK log line.

Tests

New core/blockchain_futureblocks_test.go (self-contained, own failRangeEngine with an atomic.Uint64 fail range because the chain's future-block loop calls VerifyHeaders concurrently):

• TestInsertChainQueuesMidBatchFutureBlocks — a batch rejected as future mid-way returns n == len(blocks), err nil, head stays at blocks[1], the whole tail is in futureBlocks, no bad-block records.
• TestInsertChainQueuesFutureBatchFromFirstBlock — same for a batch whose first block is already in the future (pre-existing scenario).
• TestInsertChainProcFutureBlocksResumesImport — once headers verify again, the queued tail imports and the head reaches the batch tip (head polled with a deadline because InsertChain uses TryLock and the background loop may be mid-import).

All three are mutation-verified: reverting either loop condition alone makes the corresponding test fail. go test ./core/ and go test ./eth/downloader/ pass, also under -race; gofmt/go vet clean.

Compatibility

No consensus-rule changes: block/receipt structure, state transition and wire protocol are untouched. Only the error handling of the import path changes. No migration or node-operator action required. Suggested label: consensus.

[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: Team

Run ID: 06bcda29-2a4d-4d99-be58-a6bfdc83230b

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

A verification error following queued future blocks is still discarded by the final nil-error return.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Queues entire future-block batches to prevent downloader peer eviction near the chain tip.

Changes:

• Accepts ErrFutureBlock while queuing descendants.
• Adds tests for first/mid-batch queuing and resumed import.

File summaries

File	Description
core/blockchain.go	Expands future-block queue handling.
core/blockchain_futureblocks_test.go	Tests queuing and later processing.

Review details

• Files reviewed: 2/2 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

Two error paths can omit bad-block reporting or silently report an incomplete import as successful.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

core/blockchain.go:1959

• These exemptions currently suppress both the bad-block report and the return. If iteration stops on ErrPrunedAncestor or ErrKnownBlock, block is still unconsumed but execution falls through to the final nil return, recreating a silent partial import. Exempt legitimate states only from reportBlock; every non-nil iterator error must still be returned.

		if err != nil && !errors.Is(err, consensus.ErrFutureBlock) &&
			!errors.Is(err, consensus.ErrPrunedAncestor) && !errors.Is(err, ErrKnownBlock) {
			bc.reportBlock(block, nil, err)
			return it.index, events, coalescedLogs, err

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