light: make fraction parts uint64, ensuring that it is always positive (#5655)

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

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
+// Fraction defined in terms of a numerator divided by a denominator in uint64
-// format.
+// 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"`
+	Numerator uint64 `json:"numerator"`
-	// The value by which the numerator is divided, e.g. 3 in 2/3. Must be
+	// The value by which the numerator is divided, e.g. 3 in 2/3.
-	// positive.
+	Denominator uint64 `json:"denominator"`
-	Denominator int64 `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
 }

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},
+			exp: Fraction{},
-			err: false,
+			err: true,
 		},
 		{
 			f:   "1/-2",
-			exp: Fraction{1, -2},
+			exp: Fraction{},
-			err: false,
+			err: true,
+		},
+		// test overflow
+		{
+			f:   "9223372036854775808/2",
+			exp: Fraction{},
+			err: true,
+		},
+		{
+			f:   "2/9223372036854775808",
+			exp: Fraction{},
+			err: true,
 		},
 		{
 			f:   "2/3/4",

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},
+		5: {tmmath.Fraction{Numerator: 6, Denominator: 5}, false},
-		6:  {tmmath.Fraction{Numerator: -1, Denominator: 3}, false},
+		6: {tmmath.Fraction{Numerator: 0, Denominator: 1}, false},
-		7:  {tmmath.Fraction{Numerator: 0, Denominator: 1}, false},
+		7: {tmmath.Fraction{Numerator: 0, Denominator: 0}, false},
-		8:  {tmmath.Fraction{Numerator: -1, Denominator: -3}, false},
+		8: {tmmath.Fraction{Numerator: 1, Denominator: 0}, false},
-		9:  {tmmath.Fraction{Numerator: 0, Denominator: 0}, false},
-		10: {tmmath.Fraction{Numerator: 1, Denominator: 0}, false},
 	}
 
 	for _, tc := range testCases {

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.