-
Notifications
You must be signed in to change notification settings - Fork 117
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(lib/scale): Handle eof in scale decoding. #1675
Conversation
Codecov Report
@@ Coverage Diff @@
## development #1675 +/- ##
===============================================
+ Coverage 58.50% 59.19% +0.69%
===============================================
Files 160 181 +21
Lines 15899 18501 +2602
===============================================
+ Hits 9301 10952 +1651
- Misses 4924 5638 +714
- Partials 1674 1911 +237
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
if errors.Is(err, io.EOF) { | ||
return b, 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.
this seems wrong - if there's an EOF it's because there weren't enough bytes in the reader to create the public key, so there should be an error or else this will create an invalid public key
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.
I think this PR will likely fix any EOF issues you encountered #1662
we shouldn't be ignoring the EOFs as it will result in invalid votes/public keys
Yes, I agree that we should return the error. But the node was panicking due to incorrect data from another peer. We should handle these errors gracefully in such cases. |
@arijitAD please update this to handle the error instead of ignoring, then should be good |
No longer seeing this issue. |
Changes
Tests
Issues