Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Handle MAX_INT_256 #9511

Merged
merged 9 commits into from
Jun 16, 2021
Merged

fix: Handle MAX_INT_256 #9511

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
efe4177
Handle MAX_INT_256
mankenavenkatesh Jun 14, 2021
adf73ca
Update CHANGELOG.md
mankenavenkatesh Jun 14, 2021
94b91e5
Use maxBitLen instead of hardcoded value in sdk.Dec overflow checks.
mankenavenkatesh Jun 14, 2021
0a80ea9
Merge branch 'master' into handle_MAX_INT_256
albertchon Jun 14, 2021
3f284ea
Update CHANGELOG.md
albertchon Jun 14, 2021
202ee48
Merge branch 'master' into handle_MAX_INT_256
albertchon Jun 14, 2021
c3492be
Merge branch 'master' into handle_MAX_INT_256
albertchon Jun 15, 2021
161c111
constant for maxDecBitLen
mankenavenkatesh Jun 16, 2021
d50db36
Merge branch 'master' into handle_MAX_INT_256
albertchon Jun 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ if input key is empty, or input data contains empty key.
* (x/bank) [\#8434](https://github.com/cosmos/cosmos-sdk/pull/8434) Fix legacy REST API `GET /bank/total` and `GET /bank/total/{denom}` in swagger
* (x/slashing) [\#8427](https://github.com/cosmos/cosmos-sdk/pull/8427) Fix query signing infos command
* (x/bank) [\#9229](https://github.com/cosmos/cosmos-sdk/pull/9229) Now zero coin balances cannot be added to balances & supply stores. If any denom becomes zero corresponding key gets deleted from store.
* (sdk types) [\#9511](https://github.com/cosmos/cosmos-sdk/pull/9511) Change maxBitLen of sdk.Int to handle max Erc20 value.
albertchon marked this conversation as resolved.
Show resolved Hide resolved

### Deprecated

Expand Down
18 changes: 9 additions & 9 deletions types/decimal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func (d Dec) BigInt() *big.Int {
func (d Dec) Add(d2 Dec) Dec {
res := new(big.Int).Add(d.i, d2.i)

if res.BitLen() > 255+DecimalPrecisionBits {
if res.BitLen() > (maxBitLen + DecimalPrecisionBits) {
panic("Int overflow")
}
return Dec{res}
Expand All @@ -232,7 +232,7 @@ func (d Dec) Add(d2 Dec) Dec {
func (d Dec) Sub(d2 Dec) Dec {
res := new(big.Int).Sub(d.i, d2.i)

if res.BitLen() > 255+DecimalPrecisionBits {
if res.BitLen() > (maxBitLen + DecimalPrecisionBits) {
panic("Int overflow")
}
return Dec{res}
Expand All @@ -243,7 +243,7 @@ func (d Dec) Mul(d2 Dec) Dec {
mul := new(big.Int).Mul(d.i, d2.i)
chopped := chopPrecisionAndRound(mul)

if chopped.BitLen() > 255+DecimalPrecisionBits {
if chopped.BitLen() > (maxBitLen + DecimalPrecisionBits) {
panic("Int overflow")
}
return Dec{chopped}
Expand All @@ -254,7 +254,7 @@ func (d Dec) MulTruncate(d2 Dec) Dec {
mul := new(big.Int).Mul(d.i, d2.i)
chopped := chopPrecisionAndTruncate(mul)

if chopped.BitLen() > 255+DecimalPrecisionBits {
if chopped.BitLen() > (maxBitLen + DecimalPrecisionBits) {
panic("Int overflow")
}
return Dec{chopped}
Expand All @@ -264,7 +264,7 @@ func (d Dec) MulTruncate(d2 Dec) Dec {
func (d Dec) MulInt(i Int) Dec {
mul := new(big.Int).Mul(d.i, i.i)

if mul.BitLen() > 255+DecimalPrecisionBits {
if mul.BitLen() > (maxBitLen + DecimalPrecisionBits) {
panic("Int overflow")
}
return Dec{mul}
Expand All @@ -274,7 +274,7 @@ func (d Dec) MulInt(i Int) Dec {
func (d Dec) MulInt64(i int64) Dec {
mul := new(big.Int).Mul(d.i, big.NewInt(i))

if mul.BitLen() > 255+DecimalPrecisionBits {
if mul.BitLen() > (maxBitLen + DecimalPrecisionBits) {
panic("Int overflow")
}
return Dec{mul}
Expand All @@ -289,7 +289,7 @@ func (d Dec) Quo(d2 Dec) Dec {
quo := new(big.Int).Quo(mul, d2.i)
chopped := chopPrecisionAndRound(quo)

if chopped.BitLen() > 255+DecimalPrecisionBits {
if chopped.BitLen() > (maxBitLen + DecimalPrecisionBits) {
panic("Int overflow")
}
return Dec{chopped}
Expand All @@ -304,7 +304,7 @@ func (d Dec) QuoTruncate(d2 Dec) Dec {
quo := mul.Quo(mul, d2.i)
chopped := chopPrecisionAndTruncate(quo)

if chopped.BitLen() > 255+DecimalPrecisionBits {
if chopped.BitLen() > (maxBitLen + DecimalPrecisionBits) {
panic("Int overflow")
}
return Dec{chopped}
Expand All @@ -319,7 +319,7 @@ func (d Dec) QuoRoundUp(d2 Dec) Dec {
quo := new(big.Int).Quo(mul, d2.i)
chopped := chopPrecisionAndRoundUp(quo)

if chopped.BitLen() > 255+DecimalPrecisionBits {
if chopped.BitLen() > (maxBitLen + DecimalPrecisionBits) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's create a constant for maxBitLen + DecimalPrecisionBits. It's repeated few times.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

panic("Int overflow")
}
return Dec{chopped}
Expand Down
6 changes: 3 additions & 3 deletions types/int.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"math/big"
)

const maxBitLen = 255
const maxBitLen = 256
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why previously it was 255 instead of 256?


func newIntegerFromString(s string) (*big.Int, bool) {
return new(big.Int).SetString(s, 0)
Expand Down Expand Up @@ -69,9 +69,9 @@ func unmarshalText(i *big.Int, text string) error {

var _ CustomProtobufType = (*Int)(nil)

// Int wraps big.Int with a 256 bit range bound
// Int wraps big.Int with a 257 bit range bound
// Checks overflow, underflow and division by zero
// Exists in range from -(2^255 - 1) to 2^255 - 1
// Exists in range from -(2^256 - 1) to 2^256 - 1
type Int struct {
i *big.Int
}
Expand Down
20 changes: 19 additions & 1 deletion types/int_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (s *internalIntTestSuite) TestEncodingRandom() {
}

func (s *internalIntTestSuite) TestSerializationOverflow() {
bx, _ := new(big.Int).SetString("91888242871839275229946405745257275988696311157297823662689937894645226298583", 10)
bx, _ := new(big.Int).SetString("115792089237316195423570985008687907853269984665640564039457584007913129639936", 10)
x := Int{bx}
y := new(Int)

Expand All @@ -80,6 +80,24 @@ func (s *internalIntTestSuite) TestSerializationOverflow() {
s.Require().Error(y.UnmarshalJSON(bz))
}

func (s *internalIntTestSuite) TestDeserializeMaxERC20() {
bx, _ := new(big.Int).SetString("115792089237316195423570985008687907853269984665640564039457584007913129639935", 10)
x := Int{bx}
y := new(Int)

bz, err := x.Marshal()
s.Require().NoError(err)

// require deserialization to be successful
s.Require().NoError(y.Unmarshal(bz))

// require JSON deserialization to succeed
bz, err = x.MarshalJSON()
s.Require().NoError(err)

s.Require().NoError(y.UnmarshalJSON(bz))
}

func (s *internalIntTestSuite) TestImmutabilityArithInt() {
size := 500

Expand Down
24 changes: 12 additions & 12 deletions types/int_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ func (s *intTestSuite) TestFromUint64() {
}

func (s *intTestSuite) TestIntPanic() {
// Max Int = 2^255-1 = 5.789e+76
// Min Int = -(2^255-1) = -5.789e+76
s.Require().NotPanics(func() { sdk.NewIntWithDecimal(1, 76) })
i1 := sdk.NewIntWithDecimal(1, 76)
s.Require().NotPanics(func() { sdk.NewIntWithDecimal(2, 76) })
i2 := sdk.NewIntWithDecimal(2, 76)
s.Require().NotPanics(func() { sdk.NewIntWithDecimal(3, 76) })
i3 := sdk.NewIntWithDecimal(3, 76)

s.Require().Panics(func() { sdk.NewIntWithDecimal(6, 76) })
// Max Int = 2^256-1 = 1.1579209e+77
// Min Int = -(2^256-1) = -1.1579209e+77
s.Require().NotPanics(func() { sdk.NewIntWithDecimal(4, 76) })
i1 := sdk.NewIntWithDecimal(4, 76)
s.Require().NotPanics(func() { sdk.NewIntWithDecimal(5, 76) })
i2 := sdk.NewIntWithDecimal(5, 76)
s.Require().NotPanics(func() { sdk.NewIntWithDecimal(6, 76) })
i3 := sdk.NewIntWithDecimal(6, 76)

s.Require().Panics(func() { sdk.NewIntWithDecimal(2, 77) })
s.Require().Panics(func() { sdk.NewIntWithDecimal(9, 80) })

// Overflow check
Expand All @@ -69,7 +69,7 @@ func (s *intTestSuite) TestIntPanic() {
s.Require().Panics(func() { i2.Neg().Mul(i2.Neg()) })
s.Require().Panics(func() { i3.Neg().Mul(i3.Neg()) })

// Underflow check
// // Underflow check
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, this is not an underflow

Storing values that are too low in an integer variable (e.g., attempting to store −1 in an unsigned integer) is properly referred to as integer overflow, or more broadly, integer wraparound. The term underflow normally refers to floating point numbers only, which is a separate issue.

https://en.wikipedia.org/wiki/Arithmetic_underflow

i3n := i3.Neg()
s.Require().NotPanics(func() { i3n.Sub(i1) })
s.Require().NotPanics(func() { i3n.Sub(i2) })
Expand All @@ -84,7 +84,7 @@ func (s *intTestSuite) TestIntPanic() {
s.Require().Panics(func() { i3.Mul(i3.Neg()) })

// Bound check
intmax := sdk.NewIntFromBigInt(new(big.Int).Sub(new(big.Int).Exp(big.NewInt(2), big.NewInt(255), nil), big.NewInt(1)))
intmax := sdk.NewIntFromBigInt(new(big.Int).Sub(new(big.Int).Exp(big.NewInt(2), big.NewInt(256), nil), big.NewInt(1)))
intmin := intmax.Neg()
s.Require().NotPanics(func() { intmax.Add(sdk.ZeroInt()) })
s.Require().NotPanics(func() { intmin.Sub(sdk.ZeroInt()) })
Expand Down