-
Notifications
You must be signed in to change notification settings - Fork 607
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.
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
precision increase solution to infinite loop bug #5612
Merged
Merged
Changes from 60 commits
Commits
Show all changes
69 commits
Select commit
Hold shift + click to select a range
ab831f3
repro infinite loop in swap logic
AlpinYukseloglu 3f9df41
precision increase solution to infinite loop bug
p0mvn 7d992c7
fix
p0mvn b6db5ea
remove logic
p0mvn a9bdb33
remove error
p0mvn d107a16
updates
p0mvn b9b6a58
updates
p0mvn e07c97b
updates
p0mvn d175fd6
updates
p0mvn 914d1d2
Merge branch 'main' into roman/precision-inc-bug
p0mvn e6fccd3
begin switching zeroForOneStrategy.ComputeSwapWithinBucketInGivenOut
p0mvn 1ac001c
switch remaining zero for one swap step and add rounding comments
p0mvn cf7c5ab
begin switching cur sqrt price to big dec in swaps
p0mvn b5f0380
try adding CalculateSqrtPriceToTickBigDec
p0mvn 6cca545
one for zero out given in tests
p0mvn fa78f67
fix one for zero tests
p0mvn 9cb3379
update one for zero tests
p0mvn d015454
in-progress test
p0mvn 42241d3
updates
p0mvn 52d7617
fix TestComputeSwapStepOutGivenIn_ZeroForOne
p0mvn 970bcaf
fix TestComputeSwapStepInGivenOut_ZeroForOne
p0mvn 0a77a4c
convert current sqrt price to BigDec and resolve syntax errors
p0mvn 596a757
updates
p0mvn 68717bc
clean up
p0mvn ea315fc
clean ups
p0mvn 8eeac17
clean ups
p0mvn ff65cf1
fix TestComputeSwapState_Inverse
p0mvn 445b58d
comment
p0mvn 4976fec
updates
p0mvn f9c393e
clean ups
p0mvn d509138
Merge branch 'main' into roman/precision-inc-bug
p0mvn 5a61b5d
remove unused function
p0mvn 19eb70e
remove unused GetNextSqrtPriceFromAmount1OutRoundingDown
p0mvn e5bdf60
switch GetNextSqrtPriceFromAmount0InRoundingUp to big dec
p0mvn 740d51c
fully convert GetNextSqrtPriceFromAmount0OutRoundingUp to big dec
p0mvn d192088
clean up math tests more
p0mvn 57fd285
more math test clean up
p0mvn e434e9f
fully convert calc methods to big dec
p0mvn 8d7fb70
fix model package tests
p0mvn c7bf6f8
fix TestAddToPosition
p0mvn 7562387
fix TestCalculateSpotPrice
p0mvn e2d2480
fix TestSingleSidedAddToPosition
p0mvn f375469
fix TestUninitializePool
p0mvn 5a91731
go mod update
p0mvn 4b92717
big dec comparison correction and prints
p0mvn abdd04e
fix equality issues
p0mvn 94b2d3d
convert tests to big dec
p0mvn c8eebaf
fix a few swap tests
p0mvn b28c5bb
small e2e fix
p0mvn 2064474
more swap test fixes
p0mvn cf8e5b3
updates
p0mvn 369a655
fix another test
p0mvn 64bd82d
please get fixed
p0mvn bec4ebc
fix all out given in swap tests
p0mvn bad6419
e2e
p0mvn 4b91093
fix all one for zero in given out
p0mvn bd18637
I can fix you
p0mvn f2fb617
clean ups
p0mvn fe1e084
MulRoundUp test
p0mvn dea96d6
test SDKDecRoundUp
p0mvn cfa290c
clean up
p0mvn fbc9d1d
revert launch.json
p0mvn 85ec720
comment
p0mvn e17c8b2
Merge branch 'main' into roman/precision-inc-bug
p0mvn 55126a8
Merge branch 'main' into roman/precision-inc-bug
p0mvn 82f33ce
add zero for one swap strategy ULP tests
p0mvn 56471c5
typo
p0mvn b5d6707
remove errors
p0mvn 92b4d04
remove prints
p0mvn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,6 +244,32 @@ func (s *decimalTestSuite) TestSdkDec() { | |
} | ||
} | ||
|
||
func (s *decimalTestSuite) TestSdkDecRoundUp() { | ||
tests := []struct { | ||
d osmomath.BigDec | ||
want sdk.Dec | ||
expPanic bool | ||
}{ | ||
{osmomath.NewBigDec(0), sdk.MustNewDecFromStr("0.000000000000000000"), false}, | ||
{osmomath.NewBigDec(1), sdk.MustNewDecFromStr("1.000000000000000000"), false}, | ||
{osmomath.NewBigDec(10), sdk.MustNewDecFromStr("10.000000000000000000"), false}, | ||
{osmomath.NewBigDec(12340), sdk.MustNewDecFromStr("12340.000000000000000000"), false}, | ||
{osmomath.NewDecWithPrec(12340, 4), sdk.MustNewDecFromStr("1.234000000000000000"), false}, | ||
{osmomath.NewDecWithPrec(12340, 5), sdk.MustNewDecFromStr("0.123400000000000000"), false}, | ||
{osmomath.NewDecWithPrec(12340, 8), sdk.MustNewDecFromStr("0.000123400000000000"), false}, | ||
{osmomath.NewDecWithPrec(1009009009009009009, 17), sdk.MustNewDecFromStr("10.090090090090090090"), false}, | ||
{osmomath.NewDecWithPrec(1009009009009009009, 19), sdk.MustNewDecFromStr("0.100900900900900901"), false}, | ||
} | ||
for tcIndex, tc := range tests { | ||
if tc.expPanic { | ||
s.Require().Panics(func() { tc.d.SDKDecRoundUp() }) | ||
} else { | ||
value := tc.d.SDKDecRoundUp() | ||
s.Require().Equal(tc.want, value, "bad SdkDec(), index: %v", tcIndex) | ||
} | ||
} | ||
} | ||
|
||
func (s *decimalTestSuite) TestBigDecFromSdkDec() { | ||
tests := []struct { | ||
d sdk.Dec | ||
|
@@ -356,40 +382,40 @@ func (s *decimalTestSuite) TestDecsEqual() { | |
func (s *decimalTestSuite) TestArithmetic() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to reviewer: all tests below are copied from: osmosis-labs/cosmos-sdk#430 |
||
tests := []struct { | ||
d1, d2 osmomath.BigDec | ||
expMul, expMulTruncate osmomath.BigDec | ||
expMul, expMulTruncate, expMulRoundUp osmomath.BigDec | ||
expQuo, expQuoRoundUp, expQuoTruncate osmomath.BigDec | ||
expAdd, expSub osmomath.BigDec | ||
}{ | ||
// d1 d2 MUL MulTruncate QUO QUORoundUp QUOTrunctate ADD SUB | ||
{osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0)}, | ||
{osmomath.NewBigDec(1), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(1), osmomath.NewBigDec(1)}, | ||
{osmomath.NewBigDec(0), osmomath.NewBigDec(1), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(1), osmomath.NewBigDec(-1)}, | ||
{osmomath.NewBigDec(0), osmomath.NewBigDec(-1), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(-1), osmomath.NewBigDec(1)}, | ||
{osmomath.NewBigDec(-1), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1)}, | ||
|
||
{osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(2), osmomath.NewBigDec(0)}, | ||
{osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(-2), osmomath.NewBigDec(0)}, | ||
{osmomath.NewBigDec(1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(0), osmomath.NewBigDec(2)}, | ||
{osmomath.NewBigDec(-1), osmomath.NewBigDec(1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(0), osmomath.NewBigDec(-2)}, | ||
// d1 d2 MUL MulTruncate MulRoundUp QUO QUORoundUp QUOTrunctate ADD SUB | ||
{osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0)}, | ||
{osmomath.NewBigDec(1), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(1), osmomath.NewBigDec(1)}, | ||
{osmomath.NewBigDec(0), osmomath.NewBigDec(1), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(1), osmomath.NewBigDec(-1)}, | ||
{osmomath.NewBigDec(0), osmomath.NewBigDec(-1), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(-1), osmomath.NewBigDec(1)}, | ||
{osmomath.NewBigDec(-1), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(0), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1)}, | ||
|
||
{osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(2), osmomath.NewBigDec(0)}, | ||
{osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(-2), osmomath.NewBigDec(0)}, | ||
{osmomath.NewBigDec(1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(0), osmomath.NewBigDec(2)}, | ||
{osmomath.NewBigDec(-1), osmomath.NewBigDec(1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(-1), osmomath.NewBigDec(0), osmomath.NewBigDec(-2)}, | ||
|
||
{ | ||
osmomath.NewBigDec(3), osmomath.NewBigDec(7), osmomath.NewBigDec(21), osmomath.NewBigDec(21), | ||
osmomath.NewBigDec(3), osmomath.NewBigDec(7), osmomath.NewBigDec(21), osmomath.NewBigDec(21), osmomath.NewBigDec(21), | ||
osmomath.MustNewDecFromStr("0.428571428571428571428571428571428571"), osmomath.MustNewDecFromStr("0.428571428571428571428571428571428572"), osmomath.MustNewDecFromStr("0.428571428571428571428571428571428571"), | ||
osmomath.NewBigDec(10), osmomath.NewBigDec(-4), | ||
}, | ||
{ | ||
osmomath.NewBigDec(2), osmomath.NewBigDec(4), osmomath.NewBigDec(8), osmomath.NewBigDec(8), osmomath.NewDecWithPrec(5, 1), osmomath.NewDecWithPrec(5, 1), osmomath.NewDecWithPrec(5, 1), | ||
osmomath.NewBigDec(2), osmomath.NewBigDec(4), osmomath.NewBigDec(8), osmomath.NewBigDec(8), osmomath.NewBigDec(8), osmomath.NewDecWithPrec(5, 1), osmomath.NewDecWithPrec(5, 1), osmomath.NewDecWithPrec(5, 1), | ||
osmomath.NewBigDec(6), osmomath.NewBigDec(-2), | ||
}, | ||
|
||
{osmomath.NewBigDec(100), osmomath.NewBigDec(100), osmomath.NewBigDec(10000), osmomath.NewBigDec(10000), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(200), osmomath.NewBigDec(0)}, | ||
{osmomath.NewBigDec(100), osmomath.NewBigDec(100), osmomath.NewBigDec(10000), osmomath.NewBigDec(10000), osmomath.NewBigDec(10000), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(200), osmomath.NewBigDec(0)}, | ||
|
||
{ | ||
osmomath.NewDecWithPrec(15, 1), osmomath.NewDecWithPrec(15, 1), osmomath.NewDecWithPrec(225, 2), osmomath.NewDecWithPrec(225, 2), | ||
osmomath.NewDecWithPrec(15, 1), osmomath.NewDecWithPrec(15, 1), osmomath.NewDecWithPrec(225, 2), osmomath.NewDecWithPrec(225, 2), osmomath.NewDecWithPrec(225, 2), | ||
osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(1), osmomath.NewBigDec(3), osmomath.NewBigDec(0), | ||
}, | ||
{ | ||
osmomath.NewDecWithPrec(3333, 4), osmomath.NewDecWithPrec(333, 4), osmomath.NewDecWithPrec(1109889, 8), osmomath.NewDecWithPrec(1109889, 8), | ||
osmomath.NewDecWithPrec(3333, 4), osmomath.NewDecWithPrec(333, 4), osmomath.NewDecWithPrec(1109889, 8), osmomath.NewDecWithPrec(1109889, 8), osmomath.NewDecWithPrec(1109889, 8), | ||
osmomath.MustNewDecFromStr("10.009009009009009009009009009009009009"), osmomath.MustNewDecFromStr("10.009009009009009009009009009009009010"), osmomath.MustNewDecFromStr("10.009009009009009009009009009009009009"), | ||
osmomath.NewDecWithPrec(3666, 4), osmomath.NewDecWithPrec(3, 1), | ||
}, | ||
|
@@ -401,10 +427,12 @@ func (s *decimalTestSuite) TestArithmetic() { | |
resSub := tc.d1.Sub(tc.d2) | ||
resMul := tc.d1.Mul(tc.d2) | ||
resMulTruncate := tc.d1.MulTruncate(tc.d2) | ||
resMulRoundUp := tc.d1.MulRoundUp(tc.d2) | ||
s.Require().True(tc.expAdd.Equal(resAdd), "exp %v, res %v, tc %d", tc.expAdd, resAdd, tcIndex) | ||
s.Require().True(tc.expSub.Equal(resSub), "exp %v, res %v, tc %d", tc.expSub, resSub, tcIndex) | ||
s.Require().True(tc.expMul.Equal(resMul), "exp %v, res %v, tc %d", tc.expMul, resMul, tcIndex) | ||
s.Require().True(tc.expMulTruncate.Equal(resMulTruncate), "exp %v, res %v, tc %d", tc.expMulTruncate, resMulTruncate, tcIndex) | ||
s.Require().True(tc.expMulRoundUp.Equal(resMulRoundUp), "exp %v, res %v, tc %d", tc.expMulRoundUp, resMulRoundUp, tcIndex) | ||
|
||
if tc.d2.IsZero() { // panic for divide by zero | ||
s.Require().Panics(func() { tc.d1.Quo(tc.d2) }) | ||
|
@@ -423,6 +451,21 @@ func (s *decimalTestSuite) TestArithmetic() { | |
} | ||
} | ||
|
||
func (s *decimalTestSuite) TestMulRoundUp_RoundingAtPrecisionEnd() { | ||
var ( | ||
a = osmomath.MustNewDecFromStr("0.000000000000000000000000000000000009") | ||
b = osmomath.MustNewDecFromStr("0.000000000000000000000000000000000009") | ||
expectedRoundUp = osmomath.MustNewDecFromStr("0.000000000000000000000000000000000001") | ||
expectedTruncate = osmomath.MustNewDecFromStr("0.000000000000000000000000000000000000") | ||
) | ||
|
||
actualRoundUp := a.MulRoundUp(b) | ||
s.Require().Equal(expectedRoundUp.String(), actualRoundUp.String(), "exp %v, res %v", expectedRoundUp, actualRoundUp) | ||
|
||
actualTruncate := a.MulTruncate(b) | ||
s.Require().Equal(expectedTruncate.String(), actualTruncate.String(), "exp %v, res %v", expectedRoundUp, actualTruncate) | ||
p0mvn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
func (s *decimalTestSuite) TestBankerRoundChop() { | ||
tests := []struct { | ||
d1 osmomath.BigDec | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to reviewer: all tests are coped from
SDKDec
. Added the last one to display round up behavior