handle invalid signers for supernova#7875
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/testnet-fixes #7875 +/- ##
======================================================
- Coverage 77.54% 77.53% -0.01%
======================================================
Files 883 883
Lines 124525 124545 +20
======================================================
+ Hits 96564 96571 +7
- Misses 21550 21559 +9
- Partials 6411 6415 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| return false | ||
| } | ||
|
|
||
| if !sr.IsConsensusDataEqual(cnsDta.BlockHeaderHash) { |
There was a problem hiding this comment.
I think we still need at least this check + for prev hash somehow.. less checks = more changes for attacks
There was a problem hiding this comment.
if we accept the message for the next round, the data will be reset and it'll not match
with supernova rounds the invalid signers data will most probably be propagated in the next round/rounds
There was a problem hiding this comment.
tbd if we want to check in another way if that header hash was handled/processed
| } | ||
|
|
||
| if !sr.CanProcessReceivedMessage(cnsDta, sr.RoundHandler().Index(), sr.Current()) { | ||
| if !sr.IsNodeInConsensusGroup(messageSender) { |
There was a problem hiding this comment.
this check works by accident because we have all eligible in consensus every time since andromeda, but for smaller consensus (before andromeda) groups than shard eligible, this will not be correct.
| msgTimestampSec := timeStampSec | ||
| currTimestampSec := sr.SyncTimer().CurrentTime().Unix() | ||
|
|
||
| if msgTimestampSec > currTimestampSec { |
There was a problem hiding this comment.
maybe we can allow also a bounded drift (we do have NTP adjustment)
3 participants
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?