From 6f84a81c815e58ea43d7ba312458e1f44366ca9a Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 16 Dec 2022 22:09:29 +0100 Subject: [PATCH] Remove osmomath's dependency on osmoassert & gammtypes (#3768) (#3769) * Remove osmomath's dependency on osmoassert & gammtypes * Add note in gamm constant * Bring pack conditional panic * One more cond panic (cherry picked from commit db97b16199141485274e4612ae162d04b21b4787) Co-authored-by: Dev Ojha --- osmomath/decimal_test.go | 14 ++++---------- osmomath/export_test.go | 24 ++++++++++++++++++++++++ osmomath/log2_bench_test.go | 4 +--- osmomath/math_test.go | 6 ++---- osmomath/rounding_direction_test.go | 8 +++++--- osmomath/sigfig_round_test.go | 4 +--- x/gamm/types/constants.go | 3 +++ 7 files changed, 40 insertions(+), 23 deletions(-) diff --git a/osmomath/decimal_test.go b/osmomath/decimal_test.go index c6983617c6c..a33e0864733 100644 --- a/osmomath/decimal_test.go +++ b/osmomath/decimal_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/suite" "gopkg.in/yaml.v2" - "github.com/osmosis-labs/osmosis/v13/app/apptesting/osmoassert" "github.com/osmosis-labs/osmosis/v13/osmomath" gammtypes "github.com/osmosis-labs/osmosis/v13/x/gamm/types" ) @@ -734,12 +733,11 @@ func (s *decimalTestSuite) TestLog2() { for name, tc := range tests { s.Run(name, func() { - osmoassert.ConditionalPanic(s.T(), tc.expectedPanic, func() { + osmomath.ConditionalPanic(s.T(), tc.expectedPanic, func() { // Create a copy to test that the original was not modified. // That is, that LogbBase2() is non-mutative. initialCopy := tc.initialValue.Clone() - // system under test. res := tc.initialValue.LogBase2() require.True(osmomath.DecApproxEq(s.T(), tc.expected, res, expectedErrTolerance)) require.Equal(s.T(), initialCopy, tc.initialValue) @@ -818,12 +816,11 @@ func (s *decimalTestSuite) TestLn() { for name, tc := range tests { s.Run(name, func() { - osmoassert.ConditionalPanic(s.T(), tc.expectedPanic, func() { + osmomath.ConditionalPanic(s.T(), tc.expectedPanic, func() { // Create a copy to test that the original was not modified. // That is, that Ln() is non-mutative. initialCopy := tc.initialValue.Clone() - // system under test. res := tc.initialValue.Ln() require.True(osmomath.DecApproxEq(s.T(), tc.expected, res, expectedErrTolerance)) require.Equal(s.T(), initialCopy, tc.initialValue) @@ -894,12 +891,11 @@ func (s *decimalTestSuite) TestTickLog() { for name, tc := range tests { s.Run(name, func() { - osmoassert.ConditionalPanic(s.T(), tc.expectedPanic, func() { + osmomath.ConditionalPanic(s.T(), tc.expectedPanic, func() { // Create a copy to test that the original was not modified. // That is, that Ln() is non-mutative. initialCopy := tc.initialValue.Clone() - // system under test. res := tc.initialValue.TickLog() fmt.Println(name, res.Sub(tc.expected).Abs()) require.True(osmomath.DecApproxEq(s.T(), tc.expected, res, tc.expectedErrTolerance)) @@ -1010,11 +1006,10 @@ func (s *decimalTestSuite) TestCustomBaseLog() { } for name, tc := range tests { s.Run(name, func() { - osmoassert.ConditionalPanic(s.T(), tc.expectedPanic, func() { + osmomath.ConditionalPanic(s.T(), tc.expectedPanic, func() { // Create a copy to test that the original was not modified. // That is, that Ln() is non-mutative. initialCopy := tc.initialValue.Clone() - // system under test. res := tc.initialValue.CustomBaseLog(tc.base) require.True(osmomath.DecApproxEq(s.T(), tc.expected, res, tc.expectedErrTolerance)) require.Equal(s.T(), initialCopy, tc.initialValue) @@ -1213,7 +1208,6 @@ func (s *decimalTestSuite) TestMul_Mutation() { for name, tc := range tests { tc := tc s.Run(name, func() { - startMut := tc.startValue.Clone() startNonMut := tc.startValue.Clone() diff --git a/osmomath/export_test.go b/osmomath/export_test.go index 48ab3f2337c..bdb6c71f959 100644 --- a/osmomath/export_test.go +++ b/osmomath/export_test.go @@ -1,6 +1,30 @@ package osmomath +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" +) + var ( EulersNumber = eulersNumber TwoBigDec = twoBigDec ) + +// 2^128 - 1, needs to be the same as gammtypes.MaxSpotPrice +// but we can't directly import that due to import cycles. +// Hence we use the same var name, in hopes that if any change there happens, +// this is caught via a CTRL+F +var MaxSpotPrice = sdk.NewDec(2).Power(128).Sub(sdk.OneDec()) + +// ConditionalPanic checks if expectPanic is true, asserts that sut (system under test) +// panics. If expectPanic is false, asserts that sut does not panic. +// returns true if sut panics and false it it does not +func ConditionalPanic(t *testing.T, expectPanic bool, sut func()) { + if expectPanic { + require.Panics(t, sut) + return + } + require.NotPanics(t, sut) +} diff --git a/osmomath/log2_bench_test.go b/osmomath/log2_bench_test.go index 23d267176ed..d9fb3626fae 100644 --- a/osmomath/log2_bench_test.go +++ b/osmomath/log2_bench_test.go @@ -3,8 +3,6 @@ package osmomath import ( "math/rand" "testing" - - gammtypes "github.com/osmosis-labs/osmosis/v13/x/gamm/types" ) func BenchmarkLog2(b *testing.B) { @@ -15,7 +13,7 @@ func BenchmarkLog2(b *testing.B) { NewBigDec(2048 * 2048 * 2048 * 2048 * 2048), MustNewDecFromStr("999999999999999999999999999999999999999999999999999999.9122181273612911"), MustNewDecFromStr("0.563289239121902491248219047129047129"), - BigDecFromSDKDec(gammtypes.MaxSpotPrice), // 2^128 - 1 + BigDecFromSDKDec(MaxSpotPrice), // 2^128 - 1 MustNewDecFromStr("336879543251729078828740861357450529340.45"), // (2^128 - 1) * 0.99 } diff --git a/osmomath/math_test.go b/osmomath/math_test.go index a50cc457485..07d21abece5 100644 --- a/osmomath/math_test.go +++ b/osmomath/math_test.go @@ -6,8 +6,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/osmosis-labs/osmosis/v13/app/apptesting/osmoassert" - "github.com/stretchr/testify/require" ) @@ -92,7 +90,7 @@ func TestPowApprox(t *testing.T) { for i, tc := range testCases { var actualResult sdk.Dec - osmoassert.ConditionalPanic(t, tc.base.Equal(sdk.ZeroDec()), func() { + ConditionalPanic(t, tc.base.Equal(sdk.ZeroDec()), func() { fmt.Println(tc.base) actualResult = PowApprox(tc.base, tc.exp, tc.powPrecision) require.True( @@ -156,7 +154,7 @@ func TestPow(t *testing.T) { for i, tc := range testCases { var actualResult sdk.Dec - osmoassert.ConditionalPanic(t, tc.base.Equal(sdk.ZeroDec()), func() { + ConditionalPanic(t, tc.base.Equal(sdk.ZeroDec()), func() { actualResult = Pow(tc.base, tc.exp) require.True( t, diff --git a/osmomath/rounding_direction_test.go b/osmomath/rounding_direction_test.go index b91d4269036..2aeb2474b02 100644 --- a/osmomath/rounding_direction_test.go +++ b/osmomath/rounding_direction_test.go @@ -6,8 +6,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" - - "github.com/osmosis-labs/osmosis/v13/app/apptesting/osmoassert" ) func TestDivIntByU64ToBigDec(t *testing.T) { @@ -45,7 +43,11 @@ func TestDivIntByU64ToBigDec(t *testing.T) { t.Run(name, func(t *testing.T) { got, err := DivIntByU64ToBigDec(tt.i, tt.u, tt.round) require.Equal(t, tt.want, got) - osmoassert.ConditionalError(t, tt.expErr, err) + if tt.expErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } }) } } diff --git a/osmomath/sigfig_round_test.go b/osmomath/sigfig_round_test.go index 24640cbaf71..71cbb96ac6f 100644 --- a/osmomath/sigfig_round_test.go +++ b/osmomath/sigfig_round_test.go @@ -7,8 +7,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" - - "github.com/osmosis-labs/osmosis/v13/app/apptesting/osmoassert" ) func TestSigFigRound(t *testing.T) { @@ -78,7 +76,7 @@ func TestSigFigRound(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { var actualResult sdk.Dec - osmoassert.ConditionalPanic(t, tc.tenToSigFig.Equal(sdk.ZeroInt()), func() { + ConditionalPanic(t, tc.tenToSigFig.Equal(sdk.ZeroInt()), func() { actualResult = SigFigRound(tc.decimal, tc.tenToSigFig) require.Equal( t, diff --git a/x/gamm/types/constants.go b/x/gamm/types/constants.go index 5b6fd7962e2..e7f3694ef3d 100644 --- a/x/gamm/types/constants.go +++ b/x/gamm/types/constants.go @@ -33,6 +33,9 @@ var ( // SpotPriceSigFigs is the amount of significant figures used in return value of calculate SpotPrice SpotPriceSigFigs = sdk.NewDec(10).Power(SigFigsExponent).TruncateInt() // MaxSpotPrice is the maximum supported spot price. Anything greater than this will error. + // Internal note: Ctrl+F for MaxSpotPrice in code if ever changed. + // Other tests depend on being equal to MaxSpotPrice, + // but don't directly import it due to import issues. MaxSpotPrice = sdk.NewDec(2).Power(128).Sub(sdk.OneDec()) // MultihopSwapFeeMultiplierForOsmoPools if a swap fees multiplier for trades consists of just two OSMO pools during a single transaction.