feat(consensus): report catching_up when fallen behind peers

[pratikspatil024]

Summary

/status's catching_up is derived solely from the consensus reactor's WaitSync() latch. WaitSync is set once at startup and cleared the first time the node switches from block-sync to the consensus reactor, after which it stays false for the rest of the process. So the field only ever reflects the initial block-sync and never reflects a node that later stops making progress relative to its peers — that node keeps answering catching_up=false even while it has fallen well behind, and anything that reads the field as a readiness/health signal (load balancers, k8s probes, dashboards, and in-process consumers) treats it as fully synced.

This PR adds Reactor.IsBehind(), evaluated from peer-reported heights, and folds it into the field:

CatchingUp = WaitSync() || IsBehind()

IsBehind() is true when a majority of at least two connected peers report a height more than catchup_lag_threshold blocks ahead of our consensus height, or when the node has no peers and is not the sole validator (it cannot make progress on its own, so it cannot establish that it is current).

It uses peer heights, not local block-time staleness, on purpose. A bare staleness check cannot distinguish:

• a node that is genuinely behind its peers (some peer reports a greater height) — which is catching up, from
• a network in which every node has legitimately stopped at the same height (no peer is ahead) — which is not catching up.

Reporting the second case as catching up would be a false positive, so height comparison is the discriminator rather than "my last block is old".

Peer heights are unverified gossip (NewRoundStepMessage; ValidateBasic checks shape only), so the lag decision requires a majority of at least two connected peers to corroborate it rather than trusting a single peer's max — no single peer (and no lone peer, which can't corroborate) advertising an inflated height can drive the signal (see Known limitations).

The zero-peer case is self-determining (no configuration): a node with no peers reports catching_up=true unless it is the only validator in the set, in which case it finalizes blocks on its own and needs no peers. Sole-validator status uses the same local-validator membership test as the signing path and is read live from consensus state, so it follows validator-set changes the node has committed into local round state and behaves identically on mainnet, testnet, and local devnets — no network-specific config, and the scenario is testable on any kurtosis network id.

A short debounce (catchup_debounce_duration) requires the lag condition to hold continuously before catching_up flips to true, damping flapping at the threshold boundary; the transition back to false is immediate.

New [consensus] config (defaults)

Key	Default	Notes
catchup_lag_threshold	5	Blocks ahead a peer must be to count as reporting us behind; a majority of at least two connected peers must agree before we report behind. 0 disables peer-height lag detection only — the zero-peer rule still applies. Must be >=2 when enabled, to absorb the normal one-height round skew between synced peers.
catchup_debounce_duration	10s	How long the lag condition must hold before flipping to true.

Both are additive with their defaults; behaviour only changes once a majority of (≥2) peers are >threshold ahead, or the node has no peers while other validators exist. The zero-peer rule needs no config.

Implementation notes / design decisions

• Fixed at the reactor, OR'd into status.go. IsBehind() is a new read-only method on *Reactor; the RPC consensusReactor interface gains it and Status ORs it into CatchingUp.
• WaitSync is left untouched. It also gates consensus message handling (reactor.go channel handlers), so re-arming it to express "behind" would disable consensus rather than just relabel health. IsBehind() is therefore a separate signal.
• Majority-of-≥2 corroboration, not raw max. Peer heights are untrusted gossip, so evaluateBehind trips only when more than half of the connected peers are beyond the threshold and there are at least minCorroboratingPeers (2) of them — a lone peer is a trivial "majority", so it's excluded. This is fail-safe (the signal can only over-report "catching up", never forge progress) and removes the single-peer lever.
• Zero-peer handling is self-determining, not configured. A node with no peers reports behind unless it is the sole validator (State.isLocalSoleValidator, the same membership test as the signing path). No per-network configuration; testable on any devnet regardless of chain id; a non-validator node has no private validator and so always needs peers.
• Decision logic is split into pure helpers (evaluateBehind, applyDebounceLocked, collectPeerHeights) so the branching and the peer-height aggregation are unit-testable without p2p plumbing; isBehindRaw only wires them to the live Switch peers and the sole-validator flag.
• Offline inspect server stubs IsBehind() => false.

Executed tests

Unit (consensus/reactor_catchup_test.go, config/config_test.go, inspect/rpc/rpc_test.go)

• evaluateBehind — majority far ahead, single peer is a minority, majority of three, two-peer split, both-of-two ahead, lone peer cannot corroborate (incl. far ahead), within threshold, one over threshold, equal height (network halt), one-height round skew, no peer height learned, threshold disabled, sole-validator zero-peer, non-sole zero-peer, zero-peer behind with lag disabled.
• applyDebounceLocked — within-debounce stays false, past-debounce flips true, transient blip resets the timer, zero-debounce immediate, not-behind resets state.
• collectPeerHeights — collects every height in order, no peers → empty, skips a peer with no PeerState, skips a peer with a wrong-type value at the key (covers the aggregation loop + the peer.Get(...) assertion).
• isLocalSoleValidator — sole & we're it, sole but not us, larger set, nil key, nil validator set.
• ReactorCatchupConfig — option setter assigns lag threshold + debounce.
• ConsensusConfig.ValidateBasic — catchup_lag_threshold 0 / 1 / 2 / negative, catchup_debounce_duration zero / negative; plus the shipped defaults (5, 10s).
• offline inspect checker — WaitSync / IsBehind both report synced.

Integration

• TestIsBehindIntegration drives the full IsBehind() path through a (non-started) Switch populated with mock peers + a minimal State (debounce 0): majority of peers ahead → behind; synced multi-peer → not behind; lone peer ahead → not behind; zero-peer non-sole → behind; zero-peer sole → not behind.

Mutation

• diffguard --base develop (diff mode — mutates only the changed functions across the touched files, including status.go): 100% (47/47), tier-1 logic 100%, tier-2 100%, tier-3 100%. Adding the direct ReactorCatchupConfig test also removed a flake where that mutant's kill depended on a flaky consensus-suite test and flip-flopped between runs. (diffguard's overall verdict still reports FAIL on pre-existing items not introduced here: consensus/reactor.go is a 2066-line upstream file over the 500-line cap, plus churn/SDP warnings on upstream code.)

End-to-end (Kurtosis, 5-validator devnet)

• Built this branch into a Heimdall v0.7.1 image and spawned a 5-validator devnet. Isolated one validator's Heimdall P2P: after cometbft evicted the now-dead peers to 0, catching_up flipped true (zero-peer rule); on reconnect the node reported true while blocksyncing ~100 blocks behind its peers (lag branch), then returned to false once it caught up. The control node reported catching_up=false throughout — no false positive. Recovery was automatic on heal.

Build / lint

• go build ./consensus/... ./config/... ./rpc/... ./node/... ./inspect/... — clean.
• go test ./consensus/ ./config/ ./node/ ./inspect/rpc/ — pass. gofmt -l — clean. golangci-lint / gofumpt / diffguard mutation enforced by CI.

Rollout notes

• Not consensus-affecting. This relabels a reported /status field; it does not change block validity, state transition, app hash, or wire format.
• Backward-compatible. Defaults leave catching_up unchanged for synced nodes; single-validator / single-node deployments are unaffected (the sole validator advances without peers).
• No configuration required. The zero-peer behavior ships in the binary and is identical across networks; operators only need config work if they want to tune catchup_lag_threshold / catchup_debounce_duration away from the defaults.

Known limitations

• Peer heights are unverified and only as fresh as gossip. They come from NewRoundStepMessage (ValidateBasic checks shape only), so they are not proof a peer committed/verified those blocks, and a peer that stays connected but stops sending round-step updates (e.g. a half-open TCP connection before p2p eviction) leaves a stale height until eviction. The signal is fail-safe (it can only push catching_up toward true / not-ready, never forge progress or admit invalid blocks), and the majority-of-≥2 requirement means a single (or minority of) misbehaving peer — or a lone peer — cannot drive it. The residual is an attacker controlling a majority of a node's direct peers (eclipse-level) — but a validator's direct peers in the recommended sentry topology are its own operator-run sentries; the exposed surface is open-peering (pex=true) RPC/full nodes, where the impact is availability only. A verified / quorum-of-voting-power height would be stronger but amounts to re-deriving block-sync, so it's out of scope here.
• Detection latency on a sudden isolation is bounded by p2p peer eviction, not the debounce. Observed in the e2e run above: after a hard network cut, cometbft kept the dead peer entries (frozen at the same height, so not ahead) for ~99s before evicting them to 0, so catching_up flipped ~1.5 min after the cut (eviction + the 10s debounce), not seconds. This is fail-safe, but it bounds how quickly a downstream consumer (e.g. a Bor seal-gate) can react to an abrupt isolation; tightening it would mean tuning the p2p liveness timeouts, which is out of scope here.
• IsBehind() is read-triggered. The debounce state advances on /status reads, so a sparse poller can under-report sustained lag until a second poll occurring at least catchup_debounce_duration after the first lagging observation. Reactor-side sampling/caching is a possible follow-up if /status semantics need to be wall-clock accurate independent of callers.
• With fewer than two connected peers, or when no majority of peers reports a height ahead of ours, the node is treated as not-behind (the zero-peer / sole-validator rule still covers the no-peer case).

Open design decisions (feedback wanted)

1. catchup_lag_threshold default (5). Right value? And should the threshold be height-based (current) or time-derived (e.g. "behind by more than N block-times")? Height-based is deterministic and simpler; time-based is more portable across block-time configs.
2. min_expected_peers default. Resolved — replaced with the self-determining sole-validator rule above (no config knob), chosen so the zero-peer scenario is testable on any kurtosis network id rather than hardcoding network names.
3. catchup_debounce_duration default (10s). Worth tuning against typical block time; also whether the debounce should be symmetric (currently fast-to-recover, slow-to-assert).
4. External authoritative height. For forks that have a stronger external height signal (e.g. a checkpoint-derived height), should IsBehind() optionally accept that as an additional input, or is that better layered downstream of this method?
5. Surface area. Should "behind" also be exposed as its own /status field and/or a metric, in addition to folding into catching_up, so consumers can distinguish "initial block-sync" from "fell behind after sync"?
6. minCorroboratingPeers (2). Hardcoded minimum peers before peer-height lag is trusted. Is 2 right, or should it be higher (e.g. 3) for stronger corroboration on well-connected nodes?

Part of a broader reliability effort; downstream wiring/tuning lands separately. End-to-end partition validation: see the Executed tests section.

[kamuikatsurgi]

Should this kind of behaviour reported upstream? They would have faced this issue somewhere no? Worth confirming why they have deliberately chosen this behaviour.

[pratikspatil024]

@claude review

[marcello33]

Left some inline comments.
Also, as per primer doc: in phase 2 (not this PR) we mention to deliver a network-aware min_expected_peers = 1 default via Heimdall’s initCometBFTConfig. AFAICT we removed that field no? Shall we update the doc or apply any code change?

[pratikspatil024]

@marcello33 - updated the primer doc with a new version. Check here. Thanks!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[marcello33]

LGTM