recovery when bootstrapping and no new blocks produced - #7964
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/testnet-fixes #7964 +/- ##
======================================================
- Coverage 77.72% 77.68% -0.05%
======================================================
Files 892 893 +1
Lines 129078 129503 +425
======================================================
+ Hits 100323 100601 +278
- Misses 22203 22316 +113
- Partials 6552 6586 +34 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a recovery mechanism for situations where a node is bootstrapping but the network is not producing new blocks, improving the sync loop’s ability to “unstick” itself by proactively requesting missing headers/proofs and by temporarily bypassing the post-bootstrap watchdog behavior.
Changes:
- Added a resync recovery subsystem that observes incoming headers, tracks stable “parent” candidates, and triggers fast recovery actions (header/proof requests) with cooldown/TTL controls.
- Extended
baseBootstrapwith recovery state (mutex + atomics), added a bounded post-bootstrap watchdog bypass, and refined “request header + proof” flows (including early-return paths). - Added new unit tests for recovery/bypass behavior and adjusted existing sync tests to match the updated request/ack timing.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| process/sync/baseSync.go | Adds recovery state fields, watchdog bypass gating, and refines header/proof request paths + sync loop integration. |
| process/sync/resyncRecovery.go | New recovery implementation: candidate observation, fast recovery execution, cooldowns, and watchdog bypass state. |
| process/sync/resyncRecovery_test.go | New test coverage for recovery candidate selection, request ordering, watchdog bypass bounds, and generation safety. |
| process/sync/shardblock_test.go | Adjusts shard bootstrap equivalent-proof sync tests to the updated request/wait behavior. |
| process/sync/metablock_test.go | Adjusts meta bootstrap equivalent-proof sync tests to the updated request/wait behavior. |
Suppressed comments (2)
process/sync/shardblock_test.go:3036
- The proofs pool mock is internally inconsistent here:
HasProofCalledreturnstrue, butGetProofCalled/GetProofByNonceCalledalways returnmissing proof. In production, these two should agree; otherwise the test can pass while modeling an impossible pool state.
Either simulate proof arrival explicitly (e.g. via ReceivedProof) or make GetProof* return a non-nil proof when HasProof is true.
GetProofCalled: func(shardID uint32, headerHash []byte) (data.HeaderProofHandler, error) {
return nil, errors.New("missing proof")
},
process/sync/metablock_test.go:2196
- The proofs pool mock is internally inconsistent:
HasProofCalledreturnstrue, whileGetProofCalled/GetProofByNonceCalledalways return an error. This makes the test pass while representing a state that shouldn’t happen in the real proofs pool.
Consider returning a stub proof from GetProof* when HasProof is true (or explicitly simulate proof arrival via ReceivedProof).
GetProofCalled: func(shardID uint32, headerHash []byte) (data.HeaderProofHandler, error) {
return nil, errors.New("missing proof")
},
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -2540,16 +2595,10 @@ func (boot *baseBootstrap) requestHeaderAndProofByHashIfMissing( | |||
|
|
|||
| boot.setRequestedHeaderHash(hash) | |||
There was a problem hiding this comment.
the other sets are undex mutex protection? does this needs to be also?
| return | ||
| } | ||
|
|
||
| round := boot.roundHandler.Index() |
There was a problem hiding this comment.
take also round index under lock protection?
| candidate := &boot.recoveryState.candidates[idx] | ||
| if candidate.active { | ||
| continue | ||
| } | ||
|
|
||
| boot.recoveryState.nextGeneration++ | ||
| if boot.recoveryState.nextGeneration == 0 { | ||
| boot.recoveryState.nextGeneration++ | ||
| } | ||
| *candidate = resyncRecoveryCandidate{ |
There was a problem hiding this comment.
can we avoid this pattern and update candidate at index directly? it's being used multiple times while iterating over candidates
| return header | ||
| } | ||
|
|
||
| log.Debug("requesting equivalent proof from network", |
There was a problem hiding this comment.
can this be moved below the newly added if, on line 2604? if the condition is true, not request is being made but the log currently printrs.
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?