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 db97b16) Co-authored-by: Dev Ojha <[email protected]>
osmosis-labs · Dec 16, 2022 · 6f84a81 · 6f84a81
1 parent b8a6309
commit 6f84a81
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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.