-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes header sig verification #6665
Fixes header sig verification #6665
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/equivalent-messages #6665 +/- ##
============================================================
- Coverage 78.82% 78.74% -0.08%
============================================================
Files 756 769 +13
Lines 99946 101810 +1864
============================================================
+ Hits 78778 80167 +1389
- Misses 15872 16215 +343
- Partials 5296 5428 +132 ☔ View full report in Codecov by Sentry. |
…/mx-chain-go into fixes-header-sig-verification
if check.IfNil(headerHandler) { | ||
return true | ||
} | ||
return headerHandler.GetPreviousProof() != nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return headerHandler.GetPreviousProof() != nil | |
return !check.IfNilReflect(headerHandler.GetPreviousProof()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, forgot the Get was returning an interface not a pointer to the proof
@@ -535,6 +545,13 @@ func (sr *subroundBlock) receivedBlockHeader(ctx context.Context, cnsDta *consen | |||
return blockProcessedWithSuccess | |||
} | |||
|
|||
func headerHasProof(headerHandler data.HeaderHandler) bool { | |||
if check.IfNil(headerHandler) { | |||
return true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return true to force return false on the main methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meant to return false
@@ -69,6 +71,8 @@ func TestConsensusWithInvalidSigners(t *testing.T) { | |||
t.Skip("this is not a short test") | |||
} | |||
|
|||
_ = logger.SetLogLevel("*:DEBUG") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this still needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, forgot this
@@ -103,7 +107,7 @@ func TestConsensusWithInvalidSigners(t *testing.T) { | |||
go checkBlockProposedEveryRound(numCommBlock, nonceForRoundMap, mutex, chDone, t) | |||
|
|||
extraTime := uint64(2) | |||
endTime := time.Duration(roundTime)*time.Duration(numCommBlock+extraTime)*time.Millisecond + time.Minute | |||
endTime := time.Duration(roundTime)*time.Duration(numCommBlock+extraTime)*time.Millisecond + 10*time.Minute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to extend end time so much?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgot it, used while debugging
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:
feat
branch created?feat
branch merging, do all satellite projects have a proper tag insidego.mod
?