Browse Source

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

pull/5679/head
Callum Waters 4 years ago
committed by GitHub
parent
commit
909da42789
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 45 additions and 21 deletions
  1. +2
    -0
      CHANGELOG_PENDING.md
  2. +15
    -9
      libs/math/fraction.go
  3. +22
    -4
      libs/math/fraction_test.go
  4. +4
    -6
      light/verifier_test.go
  5. +2
    -2
      types/validator_set.go

+ 2
- 0
CHANGELOG_PENDING.md View File

@ -23,6 +23,8 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
### FEATURES ### FEATURES
- [libs/math] \#5665 Make fractions unsigned integers (uint64) (@cmwaters)
### IMPROVEMENTS ### IMPROVEMENTS
- [crypto/ed25519] \#5632 Adopt zip215 `ed25519` verification. (@marbar3778) - [crypto/ed25519] \#5632 Adopt zip215 `ed25519` verification. (@marbar3778)


+ 15
- 9
libs/math/fraction.go View File

@ -3,18 +3,18 @@ package math
import ( import (
"errors" "errors"
"fmt" "fmt"
"math"
"strconv" "strconv"
"strings" "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 { type Fraction struct {
// The portion of the denominator in the faction, e.g. 2 in 2/3. // 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 { func (fr Fraction) String() string {
@ -27,16 +27,22 @@ func (fr Fraction) String() string {
func ParseFraction(f string) (Fraction, error) { func ParseFraction(f string) (Fraction, error) {
o := strings.Split(f, "/") o := strings.Split(f, "/")
if len(o) != 2 { 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 { if err != nil {
return Fraction{}, fmt.Errorf("incorrect formatting, err: %w", err) 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 { if err != nil {
return Fraction{}, fmt.Errorf("incorrect formatting, err: %w", err) 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 return Fraction{Numerator: numerator, Denominator: denominator}, nil
} }

+ 22
- 4
libs/math/fraction_test.go View File

@ -23,15 +23,33 @@ func TestParseFraction(t *testing.T) {
exp: Fraction{15, 5}, exp: Fraction{15, 5},
err: false, err: false,
}, },
// test divide by zero error
{
f: "2/0",
exp: Fraction{},
err: true,
},
// test negative
{ {
f: "-1/2", f: "-1/2",
exp: Fraction{-1, 2},
err: false,
exp: Fraction{},
err: true,
}, },
{ {
f: "1/-2", 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", f: "2/3/4",


+ 4
- 6
light/verifier_test.go View File

@ -318,12 +318,10 @@ func TestValidateTrustLevel(t *testing.T) {
4: {tmmath.Fraction{Numerator: 4, Denominator: 5}, true}, 4: {tmmath.Fraction{Numerator: 4, Denominator: 5}, true},
// invalid // 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 { for _, tc := range testCases {


+ 2
- 2
types/validator_set.go View File

@ -788,11 +788,11 @@ func (vals *ValidatorSet) VerifyCommitLightTrusting(chainID string, commit *Comm
) )
// Safely calculate voting power needed. // Safely calculate voting power needed.
totalVotingPowerMulByNumerator, overflow := safeMul(vals.TotalVotingPower(), trustLevel.Numerator)
totalVotingPowerMulByNumerator, overflow := safeMul(vals.TotalVotingPower(), int64(trustLevel.Numerator))
if overflow { if overflow {
return errors.New("int64 overflow while calculating voting power needed. please provide smaller trustLevel numerator") 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 { for idx, commitSig := range commit.Signatures {
// No need to verify absent or nil votes. // No need to verify absent or nil votes.


Loading…
Cancel
Save