Repository: osmosis-labs/osmosis
Pull request #3768 (2022-12-16 20:14:09)

Remove osmomath's dependency on osmoassert & gammtypes
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰    
v    Before smashing the submit button please review the checkboxes.
v    If a checkbox is n/a - please still include it but + a little note why
v    If your PR doesn't close an issue, that's OK!  Just remove the Closes: #XXX line!
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## What is the purpose of the change

Needed to split osmomath into its own go.mod

This basically involved making an exported var for `gammtypes.MaxSpotPrice`, and removing Conditional Panic cases. Only affects test files.


Patch:
diff --git a/x/gamm/types/constants.go b/x/gamm/types/constants.go
index 4a525de4c..d2b7415a9 100644
--- a/x/gamm/types/constants.go
+++ b/x/gamm/types/constants.go
@@ -30,6 +30,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.


Test patch:
diff --git a/osmomath/decimal_test.go b/osmomath/decimal_test.go
index c6983617c..a33e08647 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 48ab3f233..bdb6c71f9 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 23d267176..d9fb3626f 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 a50cc4574..07d21abec 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 b91d42690..2aeb2474b 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 24640cbaf..71cbb96ac 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/swaprouter/create_pool_test.go b/x/swaprouter/create_pool_test.go
index 5a3e39ff1..db62cf653 100644
--- a/x/swaprouter/create_pool_test.go
+++ b/x/swaprouter/create_pool_test.go
@@ -2,14 +2,15 @@ package swaprouter_test
 
 import (
 	"fmt"
+	"reflect"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/osmosis-labs/osmosis/v13/app/apptesting"
 	"github.com/osmosis-labs/osmosis/v13/x/gamm/pool-models/balancer"
 	balancertypes "github.com/osmosis-labs/osmosis/v13/x/gamm/pool-models/balancer"
-	"github.com/osmosis-labs/osmosis/v13/x/gamm/types"
-	swaproutertypes "github.com/osmosis-labs/osmosis/v13/x/swaprouter/types"
+	gammtypes "github.com/osmosis-labs/osmosis/v13/x/gamm/types"
+	"github.com/osmosis-labs/osmosis/v13/x/swaprouter/types"
 )
 
 func (suite *KeeperTestSuite) TestPoolCreationFee() {
@@ -62,7 +63,7 @@ func (suite *KeeperTestSuite) TestPoolCreationFee() {
 		swaprouterKeeper := suite.App.SwapRouterKeeper
 
 		// set pool creation fee
-		swaprouterKeeper.SetParams(suite.Ctx, swaproutertypes.Params{
+		swaprouterKeeper.SetParams(suite.Ctx, types.Params{
 			PoolCreationFee: test.poolCreationFee,
 		})
 
@@ -84,8 +85,8 @@ func (suite *KeeperTestSuite) TestPoolCreationFee() {
 			// check to make sure new pool exists and has minted the correct number of pool shares
 			pool, err := gammKeeper.GetPoolAndPoke(suite.Ctx, poolId)
 			suite.Require().NoError(err, "test: %v", test.name)
-			suite.Require().Equal(types.InitPoolSharesSupply.String(), pool.GetTotalShares().String(),
-				fmt.Sprintf("share token should be minted as %s initially", types.InitPoolSharesSupply.String()),
+			suite.Require().Equal(gammtypes.InitPoolSharesSupply.String(), pool.GetTotalShares().String(),
+				fmt.Sprintf("share token should be minted as %s initially", gammtypes.InitPoolSharesSupply.String()),
 			)
 
 			// make sure pool creation fee is correctly sent to community pool
@@ -96,7 +97,7 @@ func (suite *KeeperTestSuite) TestPoolCreationFee() {
 			for _, asset := range test.msg.GetPoolAssets() {
 				expectedPoolTokens = expectedPoolTokens.Add(asset.Token)
 			}
-			expectedPoolShares := sdk.NewCoin(types.GetPoolShareDenom(pool.GetId()), types.InitPoolSharesSupply)
+			expectedPoolShares := sdk.NewCoin(gammtypes.GetPoolShareDenom(pool.GetId()), gammtypes.InitPoolSharesSupply)
 
 			// make sure sender's balance is updated correctly
 			senderBal := bankKeeper.GetAllBalances(suite.Ctx, sender)
@@ -111,3 +112,70 @@ func (suite *KeeperTestSuite) TestPoolCreationFee() {
 		}
 	}
 }
+
+// TestCreatePool tests that all possible pools are created correctly.
+func (suite *KeeperTestSuite) TestCreatePool() {
+
+	validBalancerPoolMsg := balancer.NewMsgCreateBalancerPool(suite.TestAccs[0], balancer.NewPoolParams(sdk.ZeroDec(), sdk.ZeroDec(), nil), []balancer.PoolAsset{
+		{
+			Token:  sdk.NewCoin(foo, defaultInitPoolAmount),
+			Weight: sdk.NewInt(1),
+		},
+		{
+			Token:  sdk.NewCoin(bar, defaultInitPoolAmount),
+			Weight: sdk.NewInt(1),
+		},
+	}, "")
+
+	tests := []struct {
+		name               string
+		creatorFundAmount  sdk.Coins
+		msg                types.CreatePoolMsg
+		expectedModuleType reflect.Type
+		expectError        bool
+	}{
+		{
+			name:               "first balancer pool - success",
+			creatorFundAmount:  sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount.Mul(sdk.NewInt(2))), sdk.NewCoin(bar, defaultInitPoolAmount.Mul(sdk.NewInt(2)))),
+			msg:                validBalancerPoolMsg,
+			expectedModuleType: gammKeeperType,
+		},
+		{
+			name:               "second balancer pool - success",
+			creatorFundAmount:  sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount.Mul(sdk.NewInt(2))), sdk.NewCoin(bar, defaultInitPoolAmount.Mul(sdk.NewInt(2)))),
+			msg:                validBalancerPoolMsg,
+			expectedModuleType: gammKeeperType,
+		},
+		// TODO: add stableswap test
+		// TODO: add concentrated-liquidity test
+		// TODO: cover errors and edge cases
+	}
+
+	for i, tc := range tests {
+		suite.Run(tc.name, func() {
+			tc := tc
+
+			swaprouterKeeper := suite.App.SwapRouterKeeper
+			ctx := suite.Ctx
+
+			poolCreationFee := swaprouterKeeper.GetParams(suite.Ctx).PoolCreationFee
+			suite.FundAcc(suite.TestAccs[0], append(tc.creatorFundAmount, poolCreationFee...))
+
+			poolId, err := swaprouterKeeper.CreatePool(ctx, tc.msg)
+
+			if tc.expectError {
+				suite.Require().Error(err)
+				return
+			}
+
+			// Validate pool.
+			suite.Require().NoError(err)
+			suite.Require().Equal(uint64(i+1), poolId)
+
+			// Validate that mapping pool id -> module type has been persisted.
+			swapModule, err := swaprouterKeeper.GetPoolModule(ctx, poolId)
+			suite.Require().NoError(err)
+			suite.Require().Equal(tc.expectedModuleType, reflect.TypeOf(swapModule))
+		})
+	}
+}