-
Notifications
You must be signed in to change notification settings - Fork 445
Do not let gossip start with unset shred version #5837
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
Do not let gossip start with unset shred version #5837
Conversation
b4d7b34
to
b3ac904
Compare
9ba3095
to
83a7379
Compare
if my_shred_version != 0 | ||
&& (node.shred_version() != 0 && node.shred_version() != my_shred_version) | ||
{ | ||
if node.shred_version() != 0 && node.shred_version() != my_shred_version { |
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.
Can node shred_version be ever zero in the first place?
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.
not anymore since we switched spy to run on a valid shred version
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.
LGTM, maybe also is possible to follow-up and make it impossible to create Node with invalid shred_version (though it may be overkill)
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.
LGTM
ya not a bad idea. it's a little complicated since when we create a Node we don't know the shred version yet. We need to create the Node to download a snapshot at boot. but we don't need a valid shred version to download a snapshot. and then when we get the snapshot and genesis, we calculate the shred_version from genesis (if not passed in from cli). once we have the shred version, we update Node with the correct shred version. i will have to think about this one. |
Problem
We are in the process of removing the special
shred_version == 0
case where validators can pull from any node if they set the shred version to 0Summary of Changes
Do not let gossip start with an unset shred version. Also, remove the special case check where we check to see if our shred version is 0.