Import db check in round handler#7701
Conversation
| log.Error("computeInflationBeforeSupernova: sub second round time before supernova activation") | ||
| roundDurationInSec = 1 |
There was a problem hiding this comment.
- added special treatment to avoid divide by zero at any cost
- this case should not happen, in case it happens it should be treated as an error and fixed
There was a problem hiding this comment.
for before Supernova, if we anyway set a default value, we might as well set it with 6s
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/supernova-async-exec #7701 +/- ##
=============================================================
- Coverage 77.51% 77.50% -0.02%
=============================================================
Files 882 882
Lines 122754 122760 +6
=============================================================
- Hits 95157 95139 -18
- Misses 21257 21279 +22
- Partials 6340 6342 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request addresses a critical issue in the round handler initialization where supernova parameters could be incorrectly activated when starting a node in ImportDB mode. The PR ensures that supernova activation based on time is skipped during database import, relying instead on explicit flag-based activation.
Changes:
- Added ImportDBMode flag to round handler to prevent time-based supernova activation during database import
- Added division-by-zero protection in economics calculation for sub-second round durations
- Added comprehensive test coverage for ImportDB mode behavior with supernova activation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| factory/core/coreComponents.go | Passes ImportDBMode flag from core components to round handler initialization |
| epochStart/metachain/economics.go | Adds safety check to prevent division by zero when round duration is sub-second |
| consensus/round/round_test.go | Adds test case verifying supernova doesn't auto-activate in ImportDB mode |
| consensus/round/round.go | Adds ImportDBMode field and logic to prevent time-based supernova force activation during DB import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| roundDuration := 10 * time.Millisecond | ||
|
|
||
| supernovaRoundDuration := 5 * time.Millisecond | ||
| supernovaStartRond := int64(5) |
There was a problem hiding this comment.
Variable name has a typo: "supernovaStartRond" should be "supernovaStartRound" (missing 'u' in 'Round'). This inconsistency appears throughout this test case and other test cases in this file.
| roundDurationInSec := e.roundTime.TimeDuration().Seconds() | ||
| if roundDurationInSec <= 0 { | ||
| // this means that round duration is sub-seconds | ||
| // set it to 1 second | ||
| log.Error("computeInflationBeforeSupernova: sub second round time before supernova activation") | ||
| roundDurationInSec = 1 | ||
| } |
There was a problem hiding this comment.
The division by zero protection added here lacks test coverage. While the fix is important for edge cases where round duration might be sub-second, there should be a test case that verifies this behavior, especially since the mock RoundTimeDurationHandler can return custom durations. Consider adding a test that sets roundTime.TimeDuration() to return a sub-second value (e.g., 500 microseconds) to ensure the error is logged and the fallback to 1 second works correctly.
sstanculeanu
merged commit 862985f
into
feat/supernova-async-exec
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?