-
Notifications
You must be signed in to change notification settings - Fork 918
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(share/p2p/peers): Prevent making pools for nil DataHash #2761
fix(share/p2p/peers): Prevent making pools for nil DataHash #2761
Conversation
Actually this doesn't pass tests so converting to draft til i figure it out, will do tmw |
Codecov Report
@@ Coverage Diff @@
## main #2761 +/- ##
==========================================
+ Coverage 51.73% 51.90% +0.17%
==========================================
Files 161 161
Lines 10826 10827 +1
==========================================
+ Hits 5601 5620 +19
+ Misses 4731 4713 -18
Partials 494 494
|
Actuallyshould move this to outer validate func |
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.
seems reasonable, only suggestion might be to move the msg.DataHash.Validate() to be the first check so as to skip all the other checks proceeding first?
…nc mounted on shrexsub
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.
…org#2761) Not critical, but if nil DataHash is broadcasted, node could create pool for empty datahash and just keep it around until garbage collected. Best to short-circuit instead and reject message.
Not critical, but if nil DataHash is broadcasted, node could create pool for empty datahash and just keep it around until garbage collected. Best to short-circuit instead and reject message.