Skip to content

Commit

Permalink
fix(math): revert #16263 and add test cases (#17489)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt authored Aug 21, 2023
1 parent 1089f71 commit bf24916
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 94 deletions.
6 changes: 6 additions & 0 deletions math/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ Ref: https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.j

# Changelog

## [math/v1.1.2](https://github.com/cosmos/cosmos-sdk/releases/tag/math/v1.1.2) - 2023-08-21

### Bug Fixes

* [#17489](https://github.com/cosmos/cosmos-sdk/pull/17489) Revert [#16263](https://github.com/cosmos/cosmos-sdk/pull/16263).

## [math/v1.1.1](https://github.com/cosmos/cosmos-sdk/releases/tag/math/v1.1.1) - 2023-08-21

### Bug Fixes
Expand Down
6 changes: 0 additions & 6 deletions math/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,3 @@ func FuzzLegacyNewDecFromStr(f *testing.F) {
}
})
}

func TestDecNegativePrecision(t *testing.T) {
t.Skip("https://github.com/cosmos/cosmos-sdk/issues/14004 is not yet addressed")

LegacyNewDecWithPrec(10, -1)
}
3 changes: 3 additions & 0 deletions math/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ require (
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

// Issue with math.Int{}.Size() implementation.
retract [v1.1.0, v1.1.1]
91 changes: 3 additions & 88 deletions math/int.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding"
"encoding/json"
"fmt"
stdmath "math"
"math/big"
"strings"
"sync"
Expand Down Expand Up @@ -429,93 +428,9 @@ func (i *Int) Unmarshal(data []byte) error {
}

// Size implements the gogo proto custom type interface.
// Reduction power of 10 is the smallest power of 10, than 1<<64-1
//
// 18446744073709551615
//
// and the next value fitting with the digits of (1<<64)-1 is:
//
// 10000000000000000000
var (
big10Pow19, _ = new(big.Int).SetString("1"+strings.Repeat("0", 19), 10)
log10Of2 = stdmath.Log10(2)
)

func (i *Int) Size() (size int) {
if i == nil || i.i == nil {
return 1
}

sign := i.Sign()
if sign == 0 { // It is zero.
// log*(0) is undefined hence return early.
return 1
}

ii := i.i
alreadyMadeCopy := false
if sign < 0 { // Negative sign encountered, so consider len("-")
// The reason that we make this comparison in here is to
// allow checking for negatives exactly once, to reduce
// on comparisons inside sizeBigInt, hence we make a copy
// of ii and make it absolute having taken note of the sign
// already.
size++
// We already accounted for the negative sign above, thus
// we can now compute the length of the absolute value.
ii = new(big.Int).Abs(ii)
alreadyMadeCopy = true
}

// From here on, we are now dealing with non-0, non-negative values.
return size + sizeBigInt(ii, alreadyMadeCopy)
}

func sizeBigInt(i *big.Int, alreadyMadeCopy bool) (size int) {
// This code assumes that non-0, non-negative values have been passed in.
bitLen := i.BitLen()

res := float64(bitLen) * log10Of2
ires := int(res)
if diff := res - float64(ires); diff == 0.0 {
return size + ires
} else if diff >= 0.3 { // There are other digits past the bitLen, this is a heuristic.
return size + ires + 1
}

// Use Log10(x) for values less than (1<<64)-1, given it is only defined for [1, (1<<64)-1]
if bitLen <= 64 {
return size + 1 + int(stdmath.Log10(float64(i.Uint64())))
}
// Past this point, the value is greater than (1<<64)-1 and 10^19.

// The prior above computation of i.BitLen() * log10Of2 is inaccurate for powers of 10
// and values like "9999999999999999999999999999"; that computation always overshoots by 1
// hence our next alternative is to just go old school and keep dividing the value by:
// 10^19 aka "10000000000000000000" while incrementing size += 19

// At this point we should just keep reducing by 10^19 as that's the smallest multiple
// of 10 that matches the digit length of (1<<64)-1
var ri *big.Int
if alreadyMadeCopy {
ri = i
} else {
ri = new(big.Int).Set(i)
alreadyMadeCopy = true
}

for ri.Cmp(big10Pow19) >= 0 { // Keep reducing the value by 10^19 and increment size by 19
ri = ri.Quo(ri, big10Pow19)
size += 19
}

if ri.Sign() == 0 { // if the value is zero, no need for the recursion, just return immediately
return size
}

// Otherwise we already know how many times we reduced the value, so its
// remnants less than 10^19 and those can be computed by again calling sizeBigInt.
return size + sizeBigInt(ri, alreadyMadeCopy)
func (i *Int) Size() int {
bz, _ := i.Marshal()
return len(bz)
}

// Override Amino binary serialization by proxying to protobuf.
Expand Down
23 changes: 23 additions & 0 deletions math/int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,23 @@ var sizeTests = []struct {
{"10000", 5},
{"100000", 6},
{"99999", 5},
{"9999999999", 10},
{"10000000000", 11},
{"99999999999", 11},
{"999999999999", 12},
{"9999999999999", 13},
{"99999999999999", 14},
{"999999999999999", 15},
{"9999999999999999", 16},
{"99999999999999999", 17},
{"999999999999999999", 18},
{"-999999999999999999", 19},
{"9000000000000000000", 19},
{"-9999999999999990000", 20},
{"9999999999999990000", 19},
{"9999999999999999000", 19},
{"9999999999999999999", 19},
{"-9999999999999999999", 20},
{"18446744073709551616", 20},
{"18446744073709551618", 20},
{"184467440737095516181", 21},
Expand Down Expand Up @@ -568,6 +584,13 @@ var sizeTests = []struct {
{"110000000000000000000000000000000000000000000000000000000000000000000000000009", 78},
}

func TestNewIntFromString(t *testing.T) {
for _, st := range sizeTests {
ii, _ := math.NewIntFromString(st.s)
require.Equal(t, st.want, ii.Size(), "size mismatch for %q", st.s)
}
}

func BenchmarkIntSize(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
Expand Down

0 comments on commit bf24916

Please sign in to comment.