diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 78e78e56a..73acf5426 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -20,6 +20,8 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi ### FEATURES +- [libs/math] \#5665 Make fractions unsigned integers (uint64) (@cmwaters) + ### IMPROVEMENTS ### BUG FIXES diff --git a/libs/math/fraction.go b/libs/math/fraction.go index 399bc1c18..e38636485 100644 --- a/libs/math/fraction.go +++ b/libs/math/fraction.go @@ -3,18 +3,18 @@ package math import ( "errors" "fmt" + "math" "strconv" "strings" ) -// Fraction defined in terms of a numerator divided by a denominator in int64 -// format. +// Fraction defined in terms of a numerator divided by a denominator in uint64 +// format. Fraction must be positive. type Fraction struct { // The portion of the denominator in the faction, e.g. 2 in 2/3. - Numerator int64 `json:"numerator"` - // The value by which the numerator is divided, e.g. 3 in 2/3. Must be - // positive. - Denominator int64 `json:"denominator"` + Numerator uint64 `json:"numerator"` + // The value by which the numerator is divided, e.g. 3 in 2/3. + Denominator uint64 `json:"denominator"` } func (fr Fraction) String() string { @@ -27,16 +27,22 @@ func (fr Fraction) String() string { func ParseFraction(f string) (Fraction, error) { o := strings.Split(f, "/") if len(o) != 2 { - return Fraction{}, errors.New("incorrect formating: should be like \"1/3\"") + return Fraction{}, errors.New("incorrect formating: should have a single slash i.e. \"1/3\"") } - numerator, err := strconv.ParseInt(o[0], 10, 64) + numerator, err := strconv.ParseUint(o[0], 10, 64) if err != nil { return Fraction{}, fmt.Errorf("incorrect formatting, err: %w", err) } - denominator, err := strconv.ParseInt(o[1], 10, 64) + denominator, err := strconv.ParseUint(o[1], 10, 64) if err != nil { return Fraction{}, fmt.Errorf("incorrect formatting, err: %w", err) } + if denominator == 0 { + return Fraction{}, errors.New("denominator can't be 0") + } + if numerator > math.MaxInt64 || denominator > math.MaxInt64 { + return Fraction{}, fmt.Errorf("value overflow, numerator and denominator must be less than %d", math.MaxInt64) + } return Fraction{Numerator: numerator, Denominator: denominator}, nil } diff --git a/libs/math/fraction_test.go b/libs/math/fraction_test.go index e4cabd32d..73ca0f6c8 100644 --- a/libs/math/fraction_test.go +++ b/libs/math/fraction_test.go @@ -23,15 +23,33 @@ func TestParseFraction(t *testing.T) { exp: Fraction{15, 5}, err: false, }, + // test divide by zero error + { + f: "2/0", + exp: Fraction{}, + err: true, + }, + // test negative { f: "-1/2", - exp: Fraction{-1, 2}, - err: false, + exp: Fraction{}, + err: true, }, { f: "1/-2", - exp: Fraction{1, -2}, - err: false, + exp: Fraction{}, + err: true, + }, + // test overflow + { + f: "9223372036854775808/2", + exp: Fraction{}, + err: true, + }, + { + f: "2/9223372036854775808", + exp: Fraction{}, + err: true, }, { f: "2/3/4", diff --git a/light/verifier_test.go b/light/verifier_test.go index 712f3dac5..9e10810b2 100644 --- a/light/verifier_test.go +++ b/light/verifier_test.go @@ -318,12 +318,10 @@ func TestValidateTrustLevel(t *testing.T) { 4: {tmmath.Fraction{Numerator: 4, Denominator: 5}, true}, // invalid - 5: {tmmath.Fraction{Numerator: 6, Denominator: 5}, false}, - 6: {tmmath.Fraction{Numerator: -1, Denominator: 3}, false}, - 7: {tmmath.Fraction{Numerator: 0, Denominator: 1}, false}, - 8: {tmmath.Fraction{Numerator: -1, Denominator: -3}, false}, - 9: {tmmath.Fraction{Numerator: 0, Denominator: 0}, false}, - 10: {tmmath.Fraction{Numerator: 1, Denominator: 0}, false}, + 5: {tmmath.Fraction{Numerator: 6, Denominator: 5}, false}, + 6: {tmmath.Fraction{Numerator: 0, Denominator: 1}, false}, + 7: {tmmath.Fraction{Numerator: 0, Denominator: 0}, false}, + 8: {tmmath.Fraction{Numerator: 1, Denominator: 0}, false}, } for _, tc := range testCases { diff --git a/types/validator_set.go b/types/validator_set.go index cc1a452a1..af7b4034e 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -779,11 +779,11 @@ func (vals *ValidatorSet) VerifyCommitLightTrusting(chainID string, commit *Comm ) // Safely calculate voting power needed. - totalVotingPowerMulByNumerator, overflow := safeMul(vals.TotalVotingPower(), trustLevel.Numerator) + totalVotingPowerMulByNumerator, overflow := safeMul(vals.TotalVotingPower(), int64(trustLevel.Numerator)) if overflow { return errors.New("int64 overflow while calculating voting power needed. please provide smaller trustLevel numerator") } - votingPowerNeeded := totalVotingPowerMulByNumerator / trustLevel.Denominator + votingPowerNeeded := totalVotingPowerMulByNumerator / int64(trustLevel.Denominator) for idx, commitSig := range commit.Signatures { // No need to verify absent or nil votes.