Validate construction and deserialization of validator::Set
#1350
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the field is present, deserialize it into the struct field rather than skipping it completely. This will allow validation when deserializing validator::Set in tendermint.
Calculate the total voting power without overflows and validate against the protocol-defined limit, provided as the Set::MAX_TOTAL_VOTING_POWER const. In the conversion from tendermint-proto struct VaidatorSet, check the unmarshalled total_voting_power value to match the calculated sum of the list's validators, unless it's the default 0.
Use the validating conversion from the protobuf struct ValidatorSet, which should have the same schema including the optionally deserialized total_voting_power field. This is to ensure we don't blindly trust total_voting_power if it's present in the serialization, and accept when it's absent.
The Display impl called itself in infinite recursion.
These are used in serialization for ValidatorSet and LightClientAttackEvidence.
Codecov Report
@@ Coverage Diff @@
## main #1350 +/- ##
=======================================
- Coverage 59.5% 59.5% -0.1%
=======================================
Files 272 273 +1
Lines 26974 26986 +12
=======================================
- Hits 16068 16060 -8
- Misses 10906 10926 +20
... and 11 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Test for correct deserialization, the total voting power exceeding the protocol-defined limit, the total voting power computation that would overflow the u64 rejecting the value with the appropriate error, and a mismatch of total_voting_power field value.
Some light-client-js tests use fixtures where it was null. The field used to be skipped in previous serialization.
The optimization broke deserialize with deserializers that cannot always borrow.
mzabaluev
changed the title
validator::Set
Python tricked me and the values were a bit low.
romac
reviewed
Sep 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1348
Use the validating conversion from the protobuf struct
ValidatorSet, which has the same schema including the now optionally deserializedtotal_voting_powerfield.This is to ensure we don't blindly trust
total_voting_powerif it'spresent in the serialization, and accept when it's absent.
Also ensure no integer overflows can occur while calculating the total voting power, and check the resulting power against
the protocol-defined sanity limit.
.changelog/