refactor(x/gamm): remove CreatePool and pool id index boilerplate in …

…gamm (merge) (#3739)

* refactor(x/gamm): remove CreatePool and pool id index in gamm

* fix TestMigrateExistingPoolsError

* fix TestGammExportGenesis

* pool incentives use swaprouter to get pool id

* restore downtime detector

* remove swaprouter init

* fix TestInactivePoolFreezeSwaps

* GetNextPoolId deprecation comment

* fix TestQueryNumPools2

* simulation

* lint

* Update app/keepers/keepers.go

* lint

app/apptesting/gamm.go

@@ -106,7 +106,7 @@ func (s *KeeperTestHelper) PrepareBasicStableswapPool() uint64 {
 	}
 
 	msg := stableswap.NewMsgCreateStableswapPool(s.TestAccs[0], params, DefaultStableswapLiquidity, []uint64{}, "")
-	poolId, err := s.App.GAMMKeeper.CreatePool(s.Ctx, msg)
+	poolId, err := s.App.SwapRouterKeeper.CreatePool(s.Ctx, msg)
 	s.NoError(err)
 	return poolId
 }
@@ -121,7 +121,7 @@ func (s *KeeperTestHelper) PrepareImbalancedStableswapPool() uint64 {
 	}
 
 	msg := stableswap.NewMsgCreateStableswapPool(s.TestAccs[0], params, ImbalancedStableswapLiquidity, []uint64{1, 1, 1}, "")
-	poolId, err := s.App.GAMMKeeper.CreatePool(s.Ctx, msg)
+	poolId, err := s.App.SwapRouterKeeper.CreatePool(s.Ctx, msg)
 	s.NoError(err)
 	return poolId
 }
@@ -132,7 +132,7 @@ func (s *KeeperTestHelper) PrepareBalancerPoolWithPoolParams(poolParams balancer
 	s.FundAcc(s.TestAccs[0], DefaultAcctFunds)
 
 	msg := balancer.NewMsgCreateBalancerPool(s.TestAccs[0], poolParams, DefaultPoolAssets, "")
-	poolId, err := s.App.GAMMKeeper.CreatePool(s.Ctx, msg)
+	poolId, err := s.App.SwapRouterKeeper.CreatePool(s.Ctx, msg)
 	s.NoError(err)
 	return poolId
 }
@@ -150,7 +150,7 @@ func (s *KeeperTestHelper) PrepareBalancerPoolWithPoolAsset(assets []balancer.Po
 		SwapFee: sdk.ZeroDec(),
 		ExitFee: sdk.ZeroDec(),
 	}, assets, "")
-	poolId, err := s.App.GAMMKeeper.CreatePool(s.Ctx, msg)
+	poolId, err := s.App.SwapRouterKeeper.CreatePool(s.Ctx, msg)
 	s.NoError(err)
 	return poolId
 }

app/apptesting/test_suite.go

@@ -301,7 +301,7 @@ func (s *KeeperTestHelper) SetupGammPoolsWithBondDenomMultiplier(multipliers []s
 		}
 		msg := balancer.NewMsgCreateBalancerPool(acc1, poolParams, poolAssets, defaultFutureGovernor)
 
-		poolId, err := s.App.GAMMKeeper.CreatePool(s.Ctx, msg)
+		poolId, err := s.App.SwapRouterKeeper.CreatePool(s.Ctx, msg)
 		s.Require().NoError(err)
 
 		pool, err := s.App.GAMMKeeper.GetPoolAndPoke(s.Ctx, poolId)

app/keepers/keepers.go

@@ -275,6 +275,17 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 		appKeepers.GetSubspace(twaptypes.ModuleName),
 		appKeepers.GAMMKeeper)
 
+	appKeepers.SwapRouterKeeper = swaprouter.NewKeeper(
+		appKeepers.keys[swaproutertypes.StoreKey],
+		appKeepers.GetSubspace(swaproutertypes.ModuleName),
+		appKeepers.GAMMKeeper,
+		nil, // TODO: add concentrated liquidity keeper once it is merged
+		appKeepers.BankKeeper,
+		appKeepers.AccountKeeper,
+		appKeepers.DistrKeeper,
+	)
+	appKeepers.GAMMKeeper.SetPoolCreationManager(appKeepers.SwapRouterKeeper)
+
 	appKeepers.LockupKeeper = lockupkeeper.NewKeeper(
 		appKeepers.keys[lockuptypes.StoreKey],
 		// TODO: Visit why this needs to be deref'd
@@ -332,9 +343,11 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 		appKeepers.BankKeeper,
 		appKeepers.IncentivesKeeper,
 		appKeepers.DistrKeeper,
-		appKeepers.GAMMKeeper,
+		appKeepers.SwapRouterKeeper,
 	)
 	appKeepers.PoolIncentivesKeeper = &poolIncentivesKeeper
+	appKeepers.SwapRouterKeeper.SetPoolIncentivesKeeper(appKeepers.PoolIncentivesKeeper)
+	// TODO: remove the line below once multihop is ported to swaprouter.
 	appKeepers.GAMMKeeper.SetPoolIncentivesKeeper(appKeepers.PoolIncentivesKeeper)
 
 	tokenFactoryKeeper := tokenfactorykeeper.NewKeeper(
@@ -346,17 +359,6 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 	)
 	appKeepers.TokenFactoryKeeper = &tokenFactoryKeeper
 
-	appKeepers.SwapRouterKeeper = swaprouter.NewKeeper(
-		appKeepers.keys[swaproutertypes.StoreKey],
-		appKeepers.GetSubspace(swaproutertypes.ModuleName),
-		appKeepers.GAMMKeeper,
-		nil,
-		appKeepers.BankKeeper,
-		appKeepers.AccountKeeper,
-		appKeepers.DistrKeeper,
-	)
-	appKeepers.GAMMKeeper.SetPoolCreationManager(appKeepers.SwapRouterKeeper)
-
 	validatorSetPreferenceKeeper := valsetpref.NewKeeper(
 		appKeepers.keys[valsetpreftypes.StoreKey],
 		appKeepers.GetSubspace(valsetpreftypes.ModuleName),

app/upgrades/v12/upgrades.go

@@ -75,6 +75,8 @@ func CreateUpgradeHandler(
 		keepers.ICAHostKeeper.SetParams(ctx, hostParams)
 
 		// Initialize TWAP state
+		// N.B.: deprecation nolint
+		// nolint: staticcheck
 		latestPoolId := keepers.GAMMKeeper.GetNextPoolId(ctx) - 1
 		err := keepers.TwapKeeper.MigrateExistingPools(ctx, latestPoolId)
 		if err != nil {

app/upgrades/v14/upgrades.go

@@ -39,6 +39,7 @@ func CreateUpgradeHandler(
 func migrateNextPoolId(ctx sdk.Context, gammKeeper *gammkeeper.Keeper, swaprouterKeeper *swaprouter.Keeper) {
 	// N.B: pool id in gamm is to be deprecated in the future
 	// Instead,it is moved to swaprouter.
+	// nolint: staticcheck
 	nextPoolId := gammKeeper.GetNextPoolId(ctx)
 	swaprouterKeeper.SetNextPoolId(ctx, nextPoolId)
 

wasmbinding/query_plugin_test.go

@@ -80,7 +80,7 @@ func (suite *StargateTestSuite) TestStargateQuerier() {
 				msg := balancer.NewMsgCreateBalancerPool(sender,
 					balancer.NewPoolParams(sdk.ZeroDec(), sdk.ZeroDec(), nil),
 					apptesting.DefaultPoolAssets, "")
-				_, err = suite.app.GAMMKeeper.CreatePool(suite.ctx, msg)
+				_, err = suite.app.SwapRouterKeeper.CreatePool(suite.ctx, msg)
 				suite.NoError(err)
 			},
 			requestData: func() []byte {

wasmbinding/test/custom_query_test.go

@@ -352,7 +352,7 @@ func preparePool(t *testing.T, ctx sdk.Context, osmosis *app.OsmosisApp, addr sd
 	}
 
 	msg := balancer.NewMsgCreateBalancerPool(addr, poolParams, assets, "")
-	poolId, err := osmosis.GAMMKeeper.CreatePool(ctx, &msg)
+	poolId, err := osmosis.SwapRouterKeeper.CreatePool(ctx, &msg)
 	require.NoError(t, err)
 	return poolId
 }

x/gamm/keeper/export_test.go

@@ -17,10 +17,6 @@ func (k Keeper) SetPool(ctx sdk.Context, pool swaproutertypes.PoolI) error {
 	return k.setPool(ctx, pool)
 }
 
-func (k Keeper) GetNextPoolIdAndIncrement(ctx sdk.Context) uint64 {
-	return k.getNextPoolIdAndIncrement(ctx)
-}
-
 func (k Keeper) SetStableSwapScalingFactors(ctx sdk.Context, poolId uint64, scalingFactors []uint64, sender string) error {
 	return k.setStableSwapScalingFactors(ctx, poolId, scalingFactors, sender)
 }

x/gamm/keeper/gas_test.go

@@ -124,7 +124,7 @@ func (suite *KeeperTestSuite) TestRepeatedJoinPoolDistinctDenom() {
 			},
 		}
 		msg := balancer.NewMsgCreateBalancerPool(defaultAddr, defaultPoolParams, poolAssets, "")
-		_, err := suite.App.GAMMKeeper.CreatePool(suite.Ctx, msg)
+		_, err := suite.App.SwapRouterKeeper.CreatePool(suite.Ctx, msg)
 		suite.Require().NoError(err)
 	}
 

x/gamm/keeper/genesis_test.go

@@ -47,7 +47,7 @@ func TestGammInitGenesis(t *testing.T) {
 		},
 	}, app.AppCodec())
 
-	require.Equal(t, app.GAMMKeeper.GetNextPoolIdAndIncrement(ctx), uint64(2))
+	require.Equal(t, app.SwapRouterKeeper.GetNextPoolId(ctx), uint64(1))
 	poolStored, err := app.GAMMKeeper.GetPoolAndPoke(ctx, 1)
 	require.NoError(t, err)
 	require.Equal(t, balancerPool.GetId(), poolStored.GetId())
@@ -88,7 +88,7 @@ func TestGammExportGenesis(t *testing.T) {
 		Weight: sdk.NewInt(100),
 		Token:  sdk.NewCoin("bar", sdk.NewInt(10000)),
 	}}, "")
-	_, err = app.GAMMKeeper.CreatePool(ctx, msg)
+	_, err = app.SwapRouterKeeper.CreatePool(ctx, msg)
 	require.NoError(t, err)
 
 	msg = balancer.NewMsgCreateBalancerPool(acc1, balancer.PoolParams{
@@ -101,11 +101,16 @@ func TestGammExportGenesis(t *testing.T) {
 		Weight: sdk.NewInt(100),
 		Token:  sdk.NewCoin("bar", sdk.NewInt(10000)),
 	}}, "")
-	_, err = app.GAMMKeeper.CreatePool(ctx, msg)
+	_, err = app.SwapRouterKeeper.CreatePool(ctx, msg)
 	require.NoError(t, err)
 
 	genesis := app.GAMMKeeper.ExportGenesis(ctx)
-	require.Equal(t, genesis.NextPoolNumber, uint64(3))
+	// Note: the next pool number index has been migrated to
+	// swaprouter.
+	// The reason it is kept in gamm is for migrations.
+	// As a result, it is 1 here. This index is to be removed
+	// in a subsequent upgrade.
+	require.Equal(t, genesis.NextPoolNumber, uint64(1))
 	require.Len(t, genesis.Pools, 2)
 }
 
@@ -134,7 +139,7 @@ func TestMarshalUnmarshalGenesis(t *testing.T) {
 		Weight: sdk.NewInt(100),
 		Token:  sdk.NewCoin("bar", sdk.NewInt(10000)),
 	}}, "")
-	_, err = app.GAMMKeeper.CreatePool(ctx, msg)
+	_, err = app.SwapRouterKeeper.CreatePool(ctx, msg)
 	require.NoError(t, err)
 
 	genesis := am.ExportGenesis(ctx, appCodec)

x/gamm/keeper/grpc_query.go

@@ -109,12 +109,12 @@ func (q Querier) Pools(
 	}, nil
 }
 
-// NumPools returns total number of pools.
+// TODO: mark deprecated and move to swaprouter.
 func (q Querier) NumPools(ctx context.Context, _ *types.QueryNumPoolsRequest) (*types.QueryNumPoolsResponse, error) {
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
 
 	return &types.QueryNumPoolsResponse{
-		NumPools: q.Keeper.GetNextPoolId(sdkCtx) - 1,
+		NumPools: q.poolCreationManager.GetNextPoolId(sdkCtx) - 1,
 	}, nil
 }
 

x/gamm/keeper/keeper_test.go

@@ -37,7 +37,7 @@ func (suite *KeeperTestSuite) prepareCustomBalancerPool(
 ) uint64 {
 	suite.fundAllAccountsWith(balances)
 
-	poolID, err := suite.App.GAMMKeeper.CreatePool(
+	poolID, err := suite.App.SwapRouterKeeper.CreatePool(
 		suite.Ctx,
 		balancer.NewMsgCreateBalancerPool(suite.TestAccs[0], poolParams, poolAssets, ""),
 	)
@@ -54,7 +54,7 @@ func (suite *KeeperTestSuite) prepareCustomStableswapPool(
 ) uint64 {
 	suite.fundAllAccountsWith(balances)
 
-	poolID, err := suite.App.GAMMKeeper.CreatePool(
+	poolID, err := suite.App.SwapRouterKeeper.CreatePool(
 		suite.Ctx,
 		stableswap.NewMsgCreateStableswapPool(suite.TestAccs[0], poolParams, initialLiquidity, scalingFactors, ""),
 	)

x/gamm/keeper/msg_server.go

@@ -70,7 +70,7 @@ func (server msgServer) StableSwapAdjustScalingFactors(goCtx context.Context, ms
 func (server msgServer) CreatePool(goCtx context.Context, msg swaproutertypes.CreatePoolMsg) (poolId uint64, err error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	poolId, err = server.keeper.CreatePool(ctx, msg)
+	poolId, err = server.keeper.poolCreationManager.CreatePool(ctx, msg)
 	if err != nil {
 		return 0, err
 	}

x/gamm/keeper/pool.go

@@ -222,7 +222,7 @@ func (k Keeper) setNextPoolId(ctx sdk.Context, poolId uint64) {
 	store.Set(types.KeyNextGlobalPoolId, bz)
 }
 
-// GetNextPoolId returns the next pool Id.
+// Deprecated: pool id index has been moved to x/swaprouter.
 func (k Keeper) GetNextPoolId(ctx sdk.Context) uint64 {
 	var nextPoolId uint64
 	store := ctx.KVStore(k.storeKey)
@@ -260,13 +260,6 @@ func (k Keeper) GetPoolType(ctx sdk.Context, poolId uint64) (swaproutertypes.Poo
 	}
 }
 
-// getNextPoolIdAndIncrement returns the next pool Id, and increments the corresponding state entry.
-func (k Keeper) getNextPoolIdAndIncrement(ctx sdk.Context) uint64 {
-	nextPoolId := k.GetNextPoolId(ctx)
-	k.setNextPoolId(ctx, nextPoolId+1)
-	return nextPoolId
-}
-
 // setStableSwapScalingFactors sets the stable swap scaling factors.
 // errors if the pool does not exist, the sender is not the scaling factor controller, or due to other
 // internal errors.

x/gamm/keeper/pool_service.go

@@ -56,29 +56,6 @@ func (k Keeper) CalculateSpotPrice(
 	return spotPrice, err
 }
 
-// CreatePool attempts to create a pool returning the newly created pool ID or
-// an error upon failure. The pool creation fee is used to fund the community
-// pool. It will create a dedicated module account for the pool and sends the
-// initial liquidity to the created module account.
-//
-// After the initial liquidity is sent to the pool's account, shares are minted
-// and sent to the pool creator. The shares are created using a denomination in
-// the form of gamm/pool/{poolID}. In addition, the x/bank metadata is updated
-// to reflect the newly created GAMM share denomination.
-// LEGACY, consider removing in subsequent PR
-func (k Keeper) CreatePool(ctx sdk.Context, msg swaproutertypes.CreatePoolMsg) (uint64, error) {
-	poolId, err := k.poolCreationManager.CreatePool(ctx, msg)
-	if err != nil {
-		return 0, err
-	}
-	expectedPoolId := k.getNextPoolIdAndIncrement(ctx)
-	if poolId != expectedPoolId {
-		return 0, fmt.Errorf("Intermediate code that will get removed in swaprouter transition"+
-			"expected pool id %d, got %d", expectedPoolId, poolId)
-	}
-	return poolId, err
-}
-
 // This function:
 // - saves the pool to state
 // - Mints LP shares to the pool creator

x/gamm/keeper/pool_service_test.go

@@ -169,7 +169,7 @@ func (suite *KeeperTestSuite) TestCreateBalancerPool() {
 	for _, test := range tests {
 		suite.SetupTest()
 		gammKeeper := suite.App.GAMMKeeper
-		swaprouterKeeper := suite.App.GAMMKeeper
+		swaprouterKeeper := suite.App.SwapRouterKeeper
 		distributionKeeper := suite.App.DistrKeeper
 		bankKeeper := suite.App.BankKeeper
 
@@ -357,7 +357,7 @@ func (suite *KeeperTestSuite) TestJoinPoolNoSwap() {
 
 		ctx := suite.Ctx
 		gammKeeper := suite.App.GAMMKeeper
-		swaprouterKeeper := suite.App.GAMMKeeper
+		swaprouterKeeper := suite.App.SwapRouterKeeper
 		bankKeeper := suite.App.BankKeeper
 		testAccount := suite.TestAccs[0]
 
@@ -470,7 +470,7 @@ func (suite *KeeperTestSuite) TestExitPool() {
 
 			gammKeeper := suite.App.GAMMKeeper
 			bankKeeper := suite.App.BankKeeper
-			swaprouterKeeper := suite.App.GAMMKeeper
+			swaprouterKeeper := suite.App.SwapRouterKeeper
 
 			// Mint assets to the pool creator
 			suite.FundAcc(test.txSender, defaultAcctFunds)
@@ -562,7 +562,7 @@ func (suite *KeeperTestSuite) TestJoinPoolExitPool_InverseRelationship() {
 		suite.Run(tc.name, func() {
 			ctx := suite.Ctx
 			gammKeeper := suite.App.GAMMKeeper
-			swaprouterKeeper := suite.App.GAMMKeeper
+			swaprouterKeeper := suite.App.SwapRouterKeeper
 
 			for _, acc := range suite.TestAccs {
 				suite.FundAcc(acc, defaultAcctFunds)
@@ -757,7 +757,7 @@ func (suite *KeeperTestSuite) TestGetPoolDenom() {
 	// setup pool with denoms
 	suite.FundAcc(suite.TestAccs[0], defaultAcctFunds)
 	poolCreateMsg := balancer.NewMsgCreateBalancerPool(suite.TestAccs[0], defaultPoolParams, defaultPoolAssets, defaultFutureGovernor)
-	_, err := suite.App.GAMMKeeper.CreatePool(suite.Ctx, poolCreateMsg)
+	_, err := suite.App.SwapRouterKeeper.CreatePool(suite.Ctx, poolCreateMsg)
 	suite.Require().NoError(err)
 
 	for _, tc := range []struct {

x/gamm/keeper/swap_test.go

@@ -293,7 +293,7 @@ func (suite *KeeperTestSuite) TestInactivePoolFreezeSwaps() {
 	ctrl := gomock.NewController(suite.T())
 	defer ctrl.Finish()
 	inactivePool := mocks.NewMockCFMMPoolI(ctrl)
-	inactivePoolId := gammKeeper.GetNextPoolIdAndIncrement(suite.Ctx)
+	inactivePoolId := activePoolId + 1
 	// Add mock return values for pool -- we need to do this because
 	// mock objects don't have interface functions implemented by default.
 	inactivePool.EXPECT().IsActive(suite.Ctx).Return(false).AnyTimes()

x/gamm/simulation/sim_msgs.go

@@ -403,18 +403,30 @@ func createPoolRestriction(k keeper.Keeper, sim *simtypes.SimCtx, ctx sdk.Contex
 	return func(acc legacysimulationtype.Account) bool {
 		accCoins := sim.BankKeeper().SpendableCoins(ctx, acc.Address)
 		hasTwoCoins := len(accCoins) >= 2
-		hasPoolCreationFee := accCoins.AmountOf("stake").GT(PoolCreationFee.Amount)
+		hasPoolCreationFee := accCoins.AmountOf(PoolCreationFee.Denom).GT(PoolCreationFee.Amount)
 		return hasTwoCoins && hasPoolCreationFee
 	}
 }
 
 func getRandPool(k keeper.Keeper, sim *simtypes.SimCtx, ctx sdk.Context) (uint64, types.CFMMPoolI, sdk.Coin, sdk.Coin, []string, string, error) {
 	// select a pseudo-random pool ID, max bound by the upcoming pool ID
-	pool_id := simtypes.RandLTBound(sim, k.GetNextPoolId(ctx))
-	pool, err := k.GetPoolAndPoke(ctx, pool_id)
+	pools, err := k.GetPoolsAndPoke(ctx)
 	if err != nil {
 		return 0, nil, sdk.NewCoin("denom", sdk.ZeroInt()), sdk.NewCoin("denom", sdk.ZeroInt()), []string{}, "", err
 	}
+
+	numPools := len(pools)
+	if numPools == 0 {
+		return 0, nil, sdk.NewCoin("denom", sdk.ZeroInt()), sdk.NewCoin("denom", sdk.ZeroInt()), []string{}, "", fmt.Errorf("no pools exist")
+	}
+
+	pool_id := uint64(simtypes.RandLTBound(sim, numPools) + 1)
+
+	pool := pools[pool_id-1]
+	if err != nil {
+		return 0, nil, sdk.NewCoin("denom", sdk.ZeroInt()), sdk.NewCoin("denom", sdk.ZeroInt()), []string{}, "", err
+	}
+
 	poolCoins := pool.GetTotalPoolLiquidity(ctx)
 
 	// TODO: Improve this, don't just assume two asset pools

x/gamm/types/expected_keepers.go

@@ -50,4 +50,5 @@ type PoolIncentivesKeeper interface {
 
 type PoolCreationManager interface {
 	CreatePool(ctx sdk.Context, msg swaproutertypes.CreatePoolMsg) (uint64, error)
+	GetNextPoolId(ctx sdk.Context) uint64
 }

x/pool-incentives/keeper/genesis.go

@@ -26,7 +26,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
 
 func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 	distrInfo := k.GetDistrInfo(ctx)
-	lastPoolId := k.gammKeeper.GetNextPoolId(ctx)
+	lastPoolId := k.swaprouterKeeper.GetNextPoolId(ctx)
 	lockableDurations := k.GetLockableDurations(ctx)
 	var poolToGauges types.PoolToGauges
 	for i := 1; i < int(lastPoolId); i++ {