Skip to content
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

Fix the validation and verification #513

Merged
merged 2 commits into from
Dec 2, 2022
Merged

Conversation

tnasu
Copy link
Member

@tnasu tnasu commented Nov 28, 2022

Description

Fix the bugs of validation/verification

  • Remove redundant verification of VotersHash on VerifyAdjacent since verifyNewHeaderAndVoters of VerifyAdjacent is already verified
  • Add the missing validation of ValidatorSet of Light.ValidateBasic()
  • Add the input validation of VerifyCommit/VerifyCommitLight for avoiding the runtime panic

@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

Merging #513 (5fa2aab) into main (e6f57a1) will increase coverage by 0.27%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #513      +/-   ##
==========================================
+ Coverage   65.39%   65.66%   +0.27%     
==========================================
  Files         278      279       +1     
  Lines       37995    38025      +30     
==========================================
+ Hits        24846    24969     +123     
+ Misses      11328    11249      -79     
+ Partials     1821     1807      -14     
Impacted Files Coverage Δ
light/verifier.go 72.38% <ø> (+3.59%) ⬆️
types/light.go 76.82% <100.00%> (+8.24%) ⬆️
types/voter_set.go 86.22% <100.00%> (+1.44%) ⬆️
crypto/sr25519/pubkey.go 35.89% <0.00%> (-7.70%) ⬇️
p2p/pex/pex_reactor.go 79.05% <0.00%> (-0.62%) ⬇️
mempool/clist_mempool.go 82.16% <0.00%> (-0.25%) ⬇️
p2p/pex/addrbook.go 71.57% <0.00%> (-0.10%) ⬇️
mempool/mempool.go 100.00% <0.00%> (ø)
state/tx_filter.go 100.00% <0.00%> (ø)
rpc/core/mempool.go 0.00% <0.00%> (ø)
... and 19 more

@tnasu tnasu marked this pull request as ready for review November 28, 2022 12:41
@tnasu tnasu requested review from Kynea0b and torao as code owners November 28, 2022 12:41
@Kynea0b Kynea0b added the C: bug Classification: Something isn't working label Nov 30, 2022
@Kynea0b Kynea0b requested review from Mdaiki0730 and ulbqb November 30, 2022 02:07
types/evidence_test.go Show resolved Hide resolved
@tnasu tnasu merged commit 160e52f into Finschia:main Dec 2, 2022
@tnasu tnasu deleted the main-light branch December 7, 2022 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: bug Classification: Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants