Guard emergency recovery ViewID transitions - #15
Conversation
Greptile SummaryThe PR introduces one-off emergency-recovery ViewID protections and deterministic leader selection for the retained mainnet recovery state.
Confidence Score: 4/5The PR is not yet safe to merge because a failed ViewID or leader transition can permanently strand a validator in ViewChanging mode without a retry timer. startViewChange stops the active consensus and bootstrap timers before operations that can fail, while the replacement view-change timer is armed only after those operations succeed; an early return therefore leaves no timer that can trigger another attempt. Files Needing Attention: consensus/view_change.go
|
| Filename | Overview |
|---|---|
| consensus/view_change.go | Adds checked recovery-safe view transitions and leader selection, but its transition-error paths still disable all timer-driven retries. |
| consensus/recovery_view_id.go | Centralizes the recovery floor, checked ViewID arithmetic, deterministic leader derivation, and message/block ViewID validation. |
| consensus/checks.go | Enforces the recovery floor and deterministic sender selection during VIEWCHANGE and NEWVIEW sanity checks. |
| cmd/harmony/main.go | Installs the recovery floor and leader before networked consensus startup and rejects exhausted startup ViewIDs. |
| consensus/consensus_v2.go | Applies checked ViewID advancement and validates recovery ViewIDs across synchronization, commit, and catch-up paths. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Load retained chain head] --> B[Configure recovery ViewID floor]
B --> C[Set checked successor ViewID]
C --> D[Derive deterministic leader]
D --> E[Start consensus]
E --> F{Consensus timeout}
F --> G[startViewChange]
G --> H{ViewID and leader transition succeeds?}
H -- Yes --> I[Arm view-change timer and broadcast VIEWCHANGE]
H -- No --> J[Remain in ViewChanging with retry timers stopped]
Reviews (5): Last reviewed commit: "fix: apply recovery ViewID floor to shar..." | Re-trigger Greptile
| if err != nil { | ||
| consensus.getLogger().Error().Err(err).Msg("[startViewChange] cannot advance ViewID") | ||
| return | ||
| } |
There was a problem hiding this comment.
Failed transition disables retries
When checked ViewID advancement or leader selection fails, startViewChange returns after entering ViewChanging mode and stopping the consensus and bootstrap timers but before arming the view-change timer. The validator is then left without a timer that can retry the transition, so restore the prior state or explicitly schedule another attempt on these failure paths.
8dc7753 to
c0df248
Compare
c0df248 to
0a2a771
Compare
Summary
1,000,000,000ViewID floor for mainnet shard 0 at the retained recovery heightThis extracts only the ViewID/view-change protection from #13. It does not include abandoned-block rejection, rollback, staged-sync, receipt, staking, or recovery feature-freeze changes.
Test
go test -mod=readonly -count=1 ./consensus ./cmd/harmonygo vet -mod=readonly ./consensus ./cmd/harmonygo test -race -mod=readonly -count=1 ./consensus -run 'Test(EmergencyRecovery|Recovery|NewViewRequires)'git diff --check