fix(eth): log periodic sync status every cycle

[gzliudan]

Problem

The "Block synchronisation in progress" warn log was designed to be emitted every 10 minutes so operators can see sync progress even when info/debug logs are filtered out. In practice it barely fires: over 10+ hours of normal operation it appeared only once on a mainnet node and not at all on a testnet node, instead of every cycle.

Root cause

The logger gated on pm.downloader.Synchronising(), which is only true while the downloader is running a bulk historical sync (d.synchronising is set inside Downloader.synchronise and cleared when it returns). On XDPoS, once a node catches up to the network head, new blocks are followed through the block fetcher / BFT announcements, so the downloader flag stays false even while the node is still catching up. As a result the warn log almost never fires during steady-state operation.

Fix

The design intent of this logger is a periodic sync status heartbeat that is always visible, not an indicator of downloader activity. The logger now emits a status line on every 10-minute cycle with a neutral message (Block synchronisation status with current / highest / behind / peers).

The reported values are assembled from two sources, merged so the higher one wins:

• While the downloader is actively synchronising, its progress is used: it knows the discovered target (peers may not send block announcements during a bulk sync) and, in fast sync, reports the snap block as the current height via CurrentSnapBlock.
• Otherwise the local head is used, merged with a live network high-water mark fed by per-peer announced tips (tipNumber, updated on NewBlockMsg / NewBlockHashesMsg), which stays current in the steady-state announcement-driven catch-up path.

Because announced block numbers are untrusted, they are only recorded after passing the fetcher's plausibility window, exposed as fetcher.IsPlausibleAnnouncement ([-maxUncleDist, +maxQueueDist] = 7 behind / 32 ahead). The helper uses an overflow-safe branch formulation, and it is now reused by every untrusted-number path: the NewBlockMsg / NewBlockHashesMsg tip recording in the handler and the BlockFetcher.enqueue drop check for propagated blocks (which previously used a signed int64 subtraction, so a MaxUint64 block wrapped to distance -1 and could enter the queue on low-height nodes). All paths now reject the same values, so a peer cannot inflate the recorded tip and make the heartbeat report a false gap.

Behavior comparison (before / after)

Scenario	Before this PR	After this PR
Bulk historical sync in progress	Logged	Logged with downloader target + snap-block current
Announcement-driven catch-up while behind	Not logged (bug)	Logged with accurate behind gap
Fully in sync	Not logged	Logged, behind=0
No peers (isolated node)	Not logged	Logged, behind=0
Malicious far-future announcement / block	n/a	Ignored (outside plausibility window, incl. MaxUint64)

Testing

Added/updated tests in eth/sync_test.go and eth/fetcher/block_fetcher_test.go: TestSyncStatusLogger covers the steady-state heartbeat (live high-water mark, gap, zero-gap); TestAnnouncementUpdatesPeerTip and TestNewBlockMsgUpdatesTip deliver real NewBlockHashesMsg / NewBlockMsg messages through the handler and verify plausible values update the tip while far-future (MaxUint64) values do not; TestComputeSyncStatus covers the merge logic; TestSyncStatusDuringSync verifies the heartbeat reports the downloader target while synchronising, holding the downloader deterministically in that state via a stalledDownloaderPeer stub (which answers the height probe and ancestor search, then blocks the bulk download) instead of relying on a real sync remaining in flight; TestIsPlausibleAnnouncement covers the plausibility window boundaries; TestMaxUint64PropagationDiscarding verifies a MaxUint64 block is dropped by enqueue on a low-height chain where the old signed subtraction wrapped and accepted it. Log capture uses a mutex-protected lockedBuffer so the concurrent protocol/downloader goroutines writing trace logs cannot race with the test's read/reset. Verified with go build ./..., go vet ./eth/..., the full eth and eth/fetcher test suites, and the sync/fetcher tests under -race.

Compatibility

Logging-only change plus a small security hardening of tip tracking and propagated-block validation. Adds a per-peer counter and unifies the plausibility checks on the two untrusted announcement paths; no impact on consensus, block validation, RPC, database schema, or network protocol. No configuration or migration required for node operators. The only behavioral change is a warn-level status line every 10 minutes, which matches the documented intent of the logger.

[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: Pro Plus

Run ID: f771e2aa-f4f7-474c-aecf-d97f649aca1b

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]

Pull request overview

Updates sync-status logging to compare local TD with the best peer rather than downloader activity.

Changes:

• Extracts sync reporting into reportSyncStatus.
• Adds tests for TD-based logging.
• The announcement-driven path remains incompletely covered because peer TD can be stale or reference the announced block’s parent.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
eth/sync.go	Adds peer-TD-based sync-status reporting.
eth/sync_test.go	Tests logging for synthetic peer TD states.

---

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

eth/handler.go:776

• A propagated block's number is also copied into the monotonic tip before the fetcher applies its distance/queue checks (eth/fetcher/block_fetcher.go:661-667). SanityCheck only requires the number to fit in uint64, so an untrusted far-future block that the fetcher rejects can permanently poison this peer's reported high-water mark. Record the tip only once the fetcher accepts or validates the block.

		p.SetTipNumber(request.Block.NumberU64())

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

eth/fetcher/block_fetcher.go:60

• The bounds checks can still overflow when the chain height approaches math.MaxUint64: height + maxQueueDist (and number + maxUncleDist) wraps, causing values that are actually inside the plausibility window to be rejected. Compare the ordered values by subtraction instead; subtraction is safe after the branch and also handles the far-future input correctly.

	if number < height {
		return height <= maxUncleDist+number
	}
	return number <= maxQueueDist+height

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.