Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts the metachain epoch-start trigger to avoid a devnet edge case where an epoch could effectively last only one block, leading to inconsistent in-memory vs DB state during epoch-start processing.
Changes:
- Added a
minimumBlocksPerEpoch = 2guard totrigger.Update()so epoch start is deferred until at least 2 blocks exist in the current epoch. - Added unit tests covering both normal epoch-start triggering and forced epoch-start behavior under the new minimum-blocks constraint.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| epochStart/metachain/trigger.go | Enforces a minimum number of blocks per epoch before allowing an epoch transition. |
| epochStart/metachain/trigger_test.go | Adds regression tests ensuring epoch start/forced epoch start won’t happen with only 1 block in the current epoch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| isZeroEpochEdgeCase := nonce < minimumNonceToStartEpoch | ||
| epochStartNonce := t.epochStartMeta.GetNonce() | ||
| hasMinBlocksInEpoch := nonce >= epochStartNonce+minimumBlocksPerEpoch |
There was a problem hiding this comment.
hasMinBlocksInEpoch := nonce >= epochStartNonce+minimumBlocksPerEpoch can overflow when epochStartNonce is close to math.MaxUint64, potentially making the comparison unexpectedly true/false and triggering epoch start incorrectly. Please rewrite the check to avoid uint64 addition overflow (e.g., compare using nonce >= epochStartNonce and then nonce-epochStartNonce >= minimumBlocksPerEpoch, or guard the addition with an overflow-safe check).
| hasMinBlocksInEpoch := nonce >= epochStartNonce+minimumBlocksPerEpoch | |
| hasMinBlocksInEpoch := nonce >= epochStartNonce && nonce-epochStartNonce >= minimumBlocksPerEpoch |
9ace5b4
7a196f3
epoch start trigger and force epoch start with minimum blocks per epoch
Reasoning behind the pull request
A 1-block epoch causes processIfFirstBlockAfterEpochStart to dirty the peer trie in memory, then createEpochStartBody fails to recreate it from DB because the root hash was never committed
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?