x/gamm: Make all internal set functions private (#2013)

Closes: #1945 ## What is the purpose of the change I believe these functions were only public because they needed to be for an older SDK version. I think it would be appropriate to make them private now so that they aren't exposed to other modules. ## Brief Changelog - Made the following functions private: `SetParams` `SetPool` `SetTotalLiquidity` `SetDenomLiquidity` - Commented out old upgrade code that prevented `SetParams` and `SetPool` from being made private - Removed use of `SetParams` in a superfluid test ## Testing and Verifying This change is a trivial rework / code cleanup without any test coverage. ## Documentation and Release Note - Does this pull request introduce a new feature or user-facing behavior changes? (no) - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? (no) - How is the feature or change documented? (not documented) (cherry picked from commit 7fb5f82) # Conflicts: # CHANGELOG.md # app/upgrades/v4/upgrade_test.go # app/upgrades/v4/upgrades.go # x/gamm/genesis.go
osmosis-labs · Jul 18, 2022 · f5581fd · f5581fd
1 parent fddd61f
commit f5581fd
Show file tree

Hide file tree

Showing 14 changed files with 76 additions and 36 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -55,7 +55,19 @@ This release contains minor CLI bug fixes.
 ## v10.0.0
 
 
+<<<<<<< HEAD
 ## v9.0.1
+=======
+* [#1987](https://github.com/osmosis-labs/osmosis/pull/1987) Remove `GammKeeper.GetNextPoolNumberAndIncrement` in favor of the non-mutative `GammKeeper.GetNextPoolNumber`.
+* [#1937](https://github.com/osmosis-labs/osmosis/pull/1937) Change `lockupKeeper.ExtendLock` to take in lockID instead of the direct lock struct.
+* [#1893](https://github.com/osmosis-labs/osmosis/pull/1893) Change `EpochsKeeper.SetEpochInfo` to `AddEpochInfo`, which has more safety checks with it. (Makes it suitable to be called within upgrades)
+* [#1671](https://github.com/osmosis-labs/osmosis/pull/1671) Remove methods that constitute AppModuleSimulation APIs for several modules' AppModules, which implemented no-ops
+* [#1671](https://github.com/osmosis-labs/osmosis/pull/1671) Add hourly epochs to `x/epochs` DefaultGenesis.
+* [#1665](https://github.com/osmosis-labs/osmosis/pull/1665) Delete app/App interface, instead use simapp.App
+* [#1630](https://github.com/osmosis-labs/osmosis/pull/1630) Delete the v043_temp module, now that we're on an updated SDK version.
+* [#1667](https://github.com/osmosis-labs/osmosis/pull/1673) Move wasm-bindings code out of app package into its own root level package.
+* [#2013](https://github.com/osmosis-labs/osmosis/pull/2013) Make `SetParams`, `SetPool`, `SetTotalLiquidity`, and `SetDenomLiquidity` GAMM APIs private
+>>>>>>> 7fb5f824 (x/gamm: Make all internal set functions private (#2013))
 
 ### Breaking Changes
 

diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go
@@ -162,16 +162,11 @@ func (s *KeeperTestHelper) AllocateRewardsToValidator(valAddr sdk.ValAddress, re
 
 // SetupGammPoolsWithBondDenomMultiplier uses given multipliers to set initial pool supply of bond denom.
 func (s *KeeperTestHelper) SetupGammPoolsWithBondDenomMultiplier(multipliers []sdk.Dec) []gammtypes.PoolI {
-	s.App.GAMMKeeper.SetParams(s.Ctx, gammtypes.Params{
-		PoolCreationFee: sdk.Coins{},
-	})
-
 	bondDenom := s.App.StakingKeeper.BondDenom(s.Ctx)
 	// TODO: use sdk crypto instead of tendermint to generate address
 	acc1 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address().Bytes())
 
-	poolCreationFee := s.App.GAMMKeeper.GetParams(s.Ctx)
-	s.FundAcc(acc1, poolCreationFee.PoolCreationFee)
+	params := s.App.GAMMKeeper.GetParams(s.Ctx)
 
 	pools := []gammtypes.PoolI{}
 	for index, multiplier := range multipliers {
@@ -181,7 +176,7 @@ func (s *KeeperTestHelper) SetupGammPoolsWithBondDenomMultiplier(multipliers []s
 		s.FundAcc(acc1, sdk.NewCoins(
 			sdk.NewCoin(bondDenom, uosmoAmount.Mul(sdk.NewInt(10))),
 			sdk.NewInt64Coin(token, 100000),
-		))
+		).Add(params.PoolCreationFee...))
 
 		var (
 			defaultFutureGovernor = ""

diff --git a/app/upgrades/v4/upgrade_test.go b/app/upgrades/v4/upgrade_test.go
@@ -11,8 +11,13 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 
+<<<<<<< HEAD
 	"github.com/osmosis-labs/osmosis/v10/app"
 	v4 "github.com/osmosis-labs/osmosis/v10/app/upgrades/v4"
+=======
+	"github.com/osmosis-labs/osmosis/v7/app"
+	v4 "github.com/osmosis-labs/osmosis/v7/app/upgrades/v4"
+>>>>>>> 7fb5f824 (x/gamm: Make all internal set functions private (#2013))
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
@@ -108,9 +113,11 @@ func (suite *UpgradeTestSuite) TestUpgradePayments() {
 				suite.Require().Equal(feePool.GetCommunityPool(), sdk.NewDecCoins(sdk.NewInt64DecCoin("uosmo", expectedBal)))
 
 				// Check that gamm Minimum Fee has been set correctly
-				gammParams := suite.app.GAMMKeeper.GetParams(suite.ctx)
-				expectedCreationFee := sdk.NewCoins(sdk.NewCoin("uosmo", sdk.OneInt()))
-				suite.Require().Equal(gammParams.PoolCreationFee, expectedCreationFee)
+
+				// Kept as comments for recordkeeping. Since SetParams is now private, the changes being tested for can no longer be made:
+				//  	gammParams := suite.app.GAMMKeeper.GetParams(suite.ctx)
+				//  	expectedCreationFee := sdk.NewCoins(sdk.NewCoin("uosmo", sdk.OneInt()))
+				//  	suite.Require().Equal(gammParams.PoolCreationFee, expectedCreationFee)
 			},
 			true,
 		},

diff --git a/app/upgrades/v4/upgrades.go b/app/upgrades/v4/upgrades.go
@@ -5,8 +5,13 @@ import (
 	"github.com/cosmos/cosmos-sdk/types/module"
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
 
+<<<<<<< HEAD
 	"github.com/osmosis-labs/osmosis/v10/app/keepers"
 	gammtypes "github.com/osmosis-labs/osmosis/v10/x/gamm/types"
+=======
+	"github.com/osmosis-labs/osmosis/v7/app/keepers"
+	"github.com/osmosis-labs/osmosis/v7/app/upgrades"
+>>>>>>> 7fb5f824 (x/gamm: Make all internal set functions private (#2013))
 )
 
 // CreateUpgradeHandler returns an x/upgrade handler for the Osmosis v4 on-chain
@@ -20,8 +25,8 @@ func CreateUpgradeHandler(
 	keepers *keepers.AppKeepers,
 ) upgradetypes.UpgradeHandler {
 	return func(ctx sdk.Context, _plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
-		// configure upgrade for x/gamm module pool creation fee param
-		keepers.GAMMKeeper.SetParams(ctx, gammtypes.NewParams(sdk.Coins{sdk.NewInt64Coin("uosmo", 1)})) // 1 uOSMO
+		// Kept as comments for recordkeeping. SetParams is now private:
+		// 		keepers.GAMMKeeper.SetParams(ctx, gammtypes.NewParams(sdk.Coins{sdk.NewInt64Coin("uosmo", 1)})) // 1 uOSMO
 
 		Prop12(ctx, keepers.BankKeeper, keepers.DistrKeeper)
 

diff --git a/app/upgrades/v9/prop214.go b/app/upgrades/v9/prop214.go
@@ -25,8 +25,9 @@ func ExecuteProp214(ctx sdk.Context, gamm *gammkeeper.Keeper) {
 
 	balancerPool.PoolParams.SwapFee = sdk.MustNewDecFromStr("0.002")
 
-	err = gamm.SetPool(ctx, balancerPool)
-	if err != nil {
-		panic(err)
-	}
+	// Kept as comments for recordkeeping. SetPool is now private:
+	// 		err = gamm.SetPool(ctx, balancerPool)
+	// 		if err != nil {
+	//	 		panic(err)
+	//  	}
 }
diff --git a/app/upgrades/v9/prop214_test.go b/app/upgrades/v9/prop214_test.go
@@ -3,7 +3,6 @@ package v9_test
 import (
 	"testing"
 
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/osmosis-labs/osmosis/v10/app/apptesting"
@@ -26,11 +25,12 @@ func (suite *UpgradeTestSuite) TestProp214() {
 	poolId := suite.PrepareBalancerPool()
 	v9.ExecuteProp214(suite.Ctx, suite.App.GAMMKeeper)
 
-	pool, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, poolId)
+	_, err := suite.App.GAMMKeeper.GetPoolAndPoke(suite.Ctx, poolId)
 	suite.Require().NoError(err)
 
-	swapFee := pool.GetSwapFee(suite.Ctx)
-	expectedSwapFee := sdk.MustNewDecFromStr("0.002")
-
-	suite.Require().Equal(expectedSwapFee, swapFee)
+	// Kept as comments for recordkeeping. Since SetPool is now private, the changes being tested for can no longer be made:
+	// 		swapFee := pool.GetSwapFee(suite.Ctx)
+	//  	expectedSwapFee := sdk.MustNewDecFromStr("0.002")
+	//
+	//  	suite.Require().Equal(expectedSwapFee, swapFee)
 }
diff --git a/x/gamm/genesis.go b/x/gamm/genesis.go
@@ -8,11 +8,19 @@ import (
 	"github.com/osmosis-labs/osmosis/v10/x/gamm/types"
 )
 
+<<<<<<< HEAD:x/gamm/genesis.go
 // InitGenesis initializes the capability module's state from a provided genesis
 // state.
 func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState, unpacker codectypes.AnyUnpacker) {
 	k.SetParams(ctx, genState.Params)
 	k.SetNextPoolNumber(ctx, genState.NextPoolNumber)
+=======
+// InitGenesis initializes the x/gamm module's state from a provided genesis
+// state, which includes the current live pools, global pool parameters (e.g. pool creation fee), next pool number etc.
+func (k Keeper) InitGenesis(ctx sdk.Context, genState types.GenesisState, unpacker codectypes.AnyUnpacker) {
+	k.setParams(ctx, genState.Params)
+	k.setNextPoolNumber(ctx, genState.NextPoolNumber)
+>>>>>>> 7fb5f824 (x/gamm: Make all internal set functions private (#2013)):x/gamm/keeper/genesis.go
 
 	liquidity := sdk.Coins{}
 	for _, any := range genState.Pools {
@@ -21,7 +29,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState,
 		if err != nil {
 			panic(err)
 		}
-		err = k.SetPool(ctx, pool)
+		err = k.setPool(ctx, pool)
 		if err != nil {
 			panic(err)
 		}
@@ -32,7 +40,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState,
 		}
 	}
 
-	k.SetTotalLiquidity(ctx, liquidity)
+	k.setTotalLiquidity(ctx, liquidity)
 }
 
 // ExportGenesis returns the capability module's exported genesis.

diff --git a/x/gamm/keeper/export_test.go b/x/gamm/keeper/export_test.go
@@ -0,0 +1,12 @@
+package keeper
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/osmosis-labs/osmosis/v7/x/gamm/types"
+)
+
+// SetParams sets the total set of params.
+func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
+	k.setParams(ctx, params)
+}
diff --git a/x/gamm/keeper/params.go b/x/gamm/keeper/params.go
@@ -13,6 +13,6 @@ func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
 }
 
 // SetParams sets the total set of params.
-func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
+func (k Keeper) setParams(ctx sdk.Context, params types.Params) {
 	k.paramSpace.SetParamSet(ctx, &params)
 }
diff --git a/x/gamm/keeper/pool.go b/x/gamm/keeper/pool.go
@@ -80,7 +80,7 @@ func (k Keeper) GetPoolsAndPoke(ctx sdk.Context) (res []types.PoolI, err error)
 	return res, nil
 }
 
-func (k Keeper) SetPool(ctx sdk.Context, pool types.PoolI) error {
+func (k Keeper) setPool(ctx sdk.Context, pool types.PoolI) error {
 	bz, err := k.MarshalPool(pool)
 	if err != nil {
 		return err
@@ -248,7 +248,7 @@ func (k *Keeper) SetStableSwapScalingFactors(ctx sdk.Context, scalingFactors []u
 
 	stableswapPool.ScalingFactor = scalingFactors
 
-	if err = k.SetPool(ctx, stableswapPool); err != nil {
+	if err = k.setPool(ctx, stableswapPool); err != nil {
 		return err
 	}
 	return nil

diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go
@@ -160,7 +160,7 @@ func (k Keeper) CreatePool(ctx sdk.Context, msg types.CreatePoolMsg) (uint64, er
 		Display: poolShareDisplayDenom,
 	})
 
-	if err := k.SetPool(ctx, pool); err != nil {
+	if err := k.setPool(ctx, pool); err != nil {
 		return 0, err
 	}
 

diff --git a/x/gamm/keeper/share.go b/x/gamm/keeper/share.go
@@ -17,7 +17,7 @@ func (k Keeper) applyJoinPoolStateChange(ctx sdk.Context, pool types.PoolI, join
 		return err
 	}
 
-	err = k.SetPool(ctx, pool)
+	err = k.setPool(ctx, pool)
 	if err != nil {
 		return err
 	}
@@ -39,7 +39,7 @@ func (k Keeper) applyExitPoolStateChange(ctx sdk.Context, pool types.PoolI, exit
 		return err
 	}
 
-	err = k.SetPool(ctx, pool)
+	err = k.setPool(ctx, pool)
 	if err != nil {
 		return err
 	}

diff --git a/x/gamm/keeper/swap.go b/x/gamm/keeper/swap.go
@@ -143,7 +143,7 @@ func (k Keeper) updatePoolForSwap(
 	tokensIn := sdk.Coins{tokenIn}
 	tokensOut := sdk.Coins{tokenOut}
 
-	err := k.SetPool(ctx, pool)
+	err := k.setPool(ctx, pool)
 	if err != nil {
 		return err
 	}

diff --git a/x/gamm/keeper/total_liquidity.go b/x/gamm/keeper/total_liquidity.go
@@ -16,13 +16,13 @@ func (k Keeper) GetTotalLiquidity(ctx sdk.Context) sdk.Coins {
 	return coins
 }
 
-func (k Keeper) SetTotalLiquidity(ctx sdk.Context, coins sdk.Coins) {
+func (k Keeper) setTotalLiquidity(ctx sdk.Context, coins sdk.Coins) {
 	for _, coin := range coins {
-		k.SetDenomLiquidity(ctx, coin.Denom, coin.Amount)
+		k.setDenomLiquidity(ctx, coin.Denom, coin.Amount)
 	}
 }
 
-func (k Keeper) SetDenomLiquidity(ctx sdk.Context, denom string, amount sdk.Int) {
+func (k Keeper) setDenomLiquidity(ctx sdk.Context, denom string, amount sdk.Int) {
 	store := ctx.KVStore(k.storeKey)
 	bz, err := amount.Marshal()
 	if err != nil {
@@ -68,14 +68,14 @@ func (k Keeper) RecordTotalLiquidityIncrease(ctx sdk.Context, coins sdk.Coins) {
 	for _, coin := range coins {
 		amount := k.GetDenomLiquidity(ctx, coin.Denom)
 		amount = amount.Add(coin.Amount)
-		k.SetDenomLiquidity(ctx, coin.Denom, amount)
+		k.setDenomLiquidity(ctx, coin.Denom, amount)
 	}
 }
 
 func (k Keeper) RecordTotalLiquidityDecrease(ctx sdk.Context, coins sdk.Coins) {
 	for _, coin := range coins {
 		amount := k.GetDenomLiquidity(ctx, coin.Denom)
 		amount = amount.Sub(coin.Amount)
-		k.SetDenomLiquidity(ctx, coin.Denom, amount)
+		k.setDenomLiquidity(ctx, coin.Denom, amount)
 	}
 }