-
Notifications
You must be signed in to change notification settings - Fork 220
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: change truncate_from_bits to from_bits #5773
fix: change truncate_from_bits to from_bits #5773
Conversation
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.
utACK but since RpcFlags are not part of blockchain data this does limit some upgrade paths. Say we wanted to add a new optional flag to RPC that e.g. enables an optimisation for newer nodes, we could do that without breaking existing nodes. With this change we'll need to wait until sufficient have upgraded.
I can't think of a risk of truncating here given that this is a message flag for an ephemeral/non-persistent/not-hashed message.
Yeah we may revert this when we add new flags, but I think for now let's err on the side of caution and use |
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.
utACK
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.
utACK
Description
This removes all occurrences of
truncate_from_bits
tofrom_bits
Motivation and Context
truncate_from_bits
will truncate all unknown bits and may cause bits to be interpreted as the wrong flag. This changes to the much more strictfrom_bits
which forces peers to use the correct bits and only the correct bits.