Disable create pool with non exit fee (#4767)

* disable in create pool * fix ttests * fix e2e * add test case * change exit fee e2e * refactor * update changelog * update proto
osmosis-labs · Mar 31, 2023 · 1b71ef6 · 1b71ef6
1 parent 85ab280
commit 1b71ef6
Show file tree

Hide file tree

Showing 25 changed files with 98 additions and 50 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
   * [#4582](https://github.com/osmosis-labs/osmosis/pull/4582) Consistently generate build tags metadata, to return a comma-separated list without stray quotes. This affects the output from `version` CLI subcommand and server info API calls.
   * [#4549](https://github.com/osmosis-labs/osmosis/pull/4549) Add single pool price estimate queries
+  * [#4767](https://github.com/osmosis-labs/osmosis/pull/4767) Disable create pool with non-zero exit fee
 
 ### API Breaks
 

diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go
@@ -300,7 +300,7 @@ func (s *KeeperTestHelper) SetupGammPoolsWithBondDenomMultiplier(multipliers []s
 
 		poolParams := balancer.PoolParams{
 			SwapFee: sdk.NewDecWithPrec(1, 2),
-			ExitFee: sdk.NewDecWithPrec(1, 2),
+			ExitFee: sdk.Dec(sdk.ZeroInt()),
 		}
 		msg := balancer.NewMsgCreateBalancerPool(acc1, poolParams, poolAssets, defaultFutureGovernor)
 

diff --git a/app/upgrades/v15/upgrade_test.go b/app/upgrades/v15/upgrade_test.go
@@ -101,7 +101,7 @@ func (suite *UpgradeTestSuite) TestMigrateBalancerToStablePools() {
 
 	// Create the balancer pool
 	swapFee := sdk.MustNewDecFromStr("0.003")
-	exitFee := sdk.MustNewDecFromStr("0.025")
+	exitFee := sdk.ZeroDec()
 	poolID, err := suite.App.PoolManagerKeeper.CreatePool(
 		suite.Ctx,
 		balancer.NewMsgCreateBalancerPool(suite.TestAccs[0],

diff --git a/proto/osmosis/gamm/pool-models/balancer/balancerPool.proto b/proto/osmosis/gamm/pool-models/balancer/balancerPool.proto
@@ -79,6 +79,9 @@ message PoolParams {
     (gogoproto.moretags) = "yaml:\"swap_fee\"",
     (gogoproto.nullable) = false
   ];
+  // N.B.: exit fee is disabled during pool creation in x/poolmanager. While old
+  // pools can maintain a non-zero fee. No new pool can be created with non-zero
+  // fee anymore
   string exit_fee = 2 [
     (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
     (gogoproto.moretags) = "yaml:\"exit_fee\"",

diff --git a/proto/osmosis/gamm/pool-models/stableswap/stableswap_pool.proto b/proto/osmosis/gamm/pool-models/stableswap/stableswap_pool.proto
@@ -22,6 +22,9 @@ message PoolParams {
     (gogoproto.moretags) = "yaml:\"swap_fee\"",
     (gogoproto.nullable) = false
   ];
+  // N.B.: exit fee is disabled during pool creation in x/poolmanager. While old
+  // pools can maintain a non-zero fee. No new pool can be created with non-zero
+  // fee anymore
   string exit_fee = 2 [
     (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
     (gogoproto.moretags) = "yaml:\"exit_fee\"",

diff --git a/tests/e2e/scripts/ibcDenomPool.json b/tests/e2e/scripts/ibcDenomPool.json
@@ -2,6 +2,6 @@
     "weights": "5ibc/C053D637CCA2A2BA030E2C5EE1B28A16F71CCB0E45E8BE52766DC1B241B77878,5uosmo",
     "initial-deposit": "499404ibc/C053D637CCA2A2BA030E2C5EE1B28A16F71CCB0E45E8BE52766DC1B241B77878,500000uosmo",
     "swap-fee": "0.01",
-    "exit-fee": "0.01",
+    "exit-fee": "0.00",
     "future-governor": ""
 }
diff --git a/tests/e2e/scripts/nativeDenomPool.json b/tests/e2e/scripts/nativeDenomPool.json
@@ -2,6 +2,6 @@
     "weights": "5uosmo,5stake",
     "initial-deposit": "499404uosmo,500000stake",
     "swap-fee": "0.01",
-    "exit-fee": "0.01",
+    "exit-fee": "0.00",
     "future-governor": ""
 }
diff --git a/tests/e2e/scripts/nativeDenomThreeAssetPool.json b/tests/e2e/scripts/nativeDenomThreeAssetPool.json
@@ -2,6 +2,6 @@
     "weights": "5stake,5uion,5uosmo",
     "initial-deposit": "10000000stake,1000000uion,20000000uosmo",
     "swap-fee": "0.01",
-    "exit-fee": "0.01",
+    "exit-fee": "0.00",
     "future-governor": ""
 }
diff --git a/tests/e2e/scripts/pool1A.json b/tests/e2e/scripts/pool1A.json
@@ -2,6 +2,6 @@
     "weights": "5ibc/ED07A3391A112B175915CD8FAF43A2DA8E4790EDE12566649D0C2F97716B8518,5uosmo",
     "initial-deposit": "499404ibc/ED07A3391A112B175915CD8FAF43A2DA8E4790EDE12566649D0C2F97716B8518,500000uosmo",
     "swap-fee": "0.01",
-    "exit-fee": "0.01",
+    "exit-fee": "0.0",
     "future-governor": ""
 }
diff --git a/tests/e2e/scripts/pool1B.json b/tests/e2e/scripts/pool1B.json
@@ -2,6 +2,6 @@
     "weights": "5ibc/C053D637CCA2A2BA030E2C5EE1B28A16F71CCB0E45E8BE52766DC1B241B77878,5uosmo",
     "initial-deposit": "499404ibc/C053D637CCA2A2BA030E2C5EE1B28A16F71CCB0E45E8BE52766DC1B241B77878,500000uosmo",
     "swap-fee": "0.01",
-    "exit-fee": "0.01",
+    "exit-fee": "0.0",
     "future-governor": ""
 }
diff --git a/tests/ibc-hooks/ibc_middleware_test.go b/tests/ibc-hooks/ibc_middleware_test.go
@@ -629,7 +629,7 @@ func (suite *HooksTestSuite) SetupPools(chainName Chain, multipliers []sdk.Dec)
 
 		poolParams := balancer.PoolParams{
 			SwapFee: sdk.NewDecWithPrec(1, 2),
-			ExitFee: sdk.NewDecWithPrec(1, 2),
+			ExitFee: sdk.ZeroDec(),
 		}
 		msg := balancer.NewMsgCreateBalancerPool(acc1, poolParams, poolAssets, defaultFutureGovernor)
 
@@ -1268,7 +1268,7 @@ func (suite *HooksTestSuite) CreateIBCPoolOnChainB() uint64 {
 
 	poolParams := balancer.PoolParams{
 		SwapFee: sdk.NewDecWithPrec(1, 2),
-		ExitFee: sdk.NewDecWithPrec(1, 2),
+		ExitFee: sdk.ZeroDec(),
 	}
 	msg := balancer.NewMsgCreateBalancerPool(acc1, poolParams, poolAssets, defaultFutureGovernor)
 

diff --git a/x/gamm/keeper/genesis_test.go b/x/gamm/keeper/genesis_test.go
@@ -31,7 +31,7 @@ func TestGammInitGenesis(t *testing.T) {
 
 	balancerPool, err := balancer.NewBalancerPool(1, balancer.PoolParams{
 		SwapFee: sdk.NewDecWithPrec(1, 2),
-		ExitFee: sdk.NewDecWithPrec(1, 2),
+		ExitFee: sdk.ZeroDec(),
 	}, []balancer.PoolAsset{
 		{
 			Weight: sdk.NewInt(1),
@@ -92,7 +92,7 @@ func TestGammExportGenesis(t *testing.T) {
 
 	msg := balancer.NewMsgCreateBalancerPool(acc1, balancer.PoolParams{
 		SwapFee: sdk.NewDecWithPrec(1, 2),
-		ExitFee: sdk.NewDecWithPrec(1, 2),
+		ExitFee: sdk.ZeroDec(),
 	}, []balancer.PoolAsset{{
 		Weight: sdk.NewInt(100),
 		Token:  sdk.NewCoin("foo", sdk.NewInt(10000)),
@@ -105,7 +105,7 @@ func TestGammExportGenesis(t *testing.T) {
 
 	msg = balancer.NewMsgCreateBalancerPool(acc1, balancer.PoolParams{
 		SwapFee: sdk.NewDecWithPrec(1, 2),
-		ExitFee: sdk.NewDecWithPrec(1, 2),
+		ExitFee: sdk.ZeroDec(),
 	}, []balancer.PoolAsset{{
 		Weight: sdk.NewInt(70),
 		Token:  sdk.NewCoin("foo", sdk.NewInt(10000)),
@@ -146,7 +146,7 @@ func TestMarshalUnmarshalGenesis(t *testing.T) {
 
 	msg := balancer.NewMsgCreateBalancerPool(acc1, balancer.PoolParams{
 		SwapFee: sdk.NewDecWithPrec(1, 2),
-		ExitFee: sdk.NewDecWithPrec(1, 2),
+		ExitFee: sdk.ZeroDec(),
 	}, []balancer.PoolAsset{{
 		Weight: sdk.NewInt(100),
 		Token:  sdk.NewCoin("foo", sdk.NewInt(10000)),

diff --git a/x/gamm/keeper/keeper_test.go b/x/gamm/keeper/keeper_test.go
@@ -19,6 +19,11 @@ type KeeperTestSuite struct {
 
 	queryClient types.QueryClient
 }
+var (
+	defaultSwapFee    = sdk.MustNewDecFromStr("0.025")
+	defaultZeroExitFee    = sdk.ZeroDec()
+)
+
 
 func TestKeeperTestSuite(t *testing.T) {
 	suite.Run(t, new(KeeperTestSuite))

diff --git a/x/gamm/keeper/pool_service_test.go b/x/gamm/keeper/pool_service_test.go
@@ -18,15 +18,14 @@ import (
 )
 
 var (
-	defaultSwapFee    = sdk.MustNewDecFromStr("0.025")
-	defaultExitFee    = sdk.MustNewDecFromStr("0.025")
+
 	defaultPoolParams = balancer.PoolParams{
 		SwapFee: defaultSwapFee,
-		ExitFee: defaultExitFee,
+		ExitFee: defaultZeroExitFee,
 	}
 	defaultStableSwapPoolParams = stableswap.PoolParams{
 		SwapFee: defaultSwapFee,
-		ExitFee: defaultExitFee,
+		ExitFee: defaultZeroExitFee,
 	}
 	defaultScalingFactor  = []uint64{1, 1}
 	defaultFutureGovernor = ""
@@ -88,7 +87,7 @@ func (suite *KeeperTestSuite) TestCreateBalancerPool() {
 			name: "create a pool with negative swap fee",
 			msg: balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
 				SwapFee: sdk.NewDecWithPrec(-1, 2),
-				ExitFee: sdk.NewDecWithPrec(1, 2),
+				ExitFee: defaultZeroExitFee,
 			}, defaultPoolAssets, defaultFutureGovernor),
 			emptySender: false,
 			expectPass:  false,
@@ -101,18 +100,26 @@ func (suite *KeeperTestSuite) TestCreateBalancerPool() {
 			emptySender: false,
 			expectPass:  false,
 		}, {
-			name: "create the pool with empty PoolAssets",
+			name: "create a pool with non zero exit fee",
 			msg: balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
 				SwapFee: sdk.NewDecWithPrec(1, 2),
 				ExitFee: sdk.NewDecWithPrec(1, 2),
+			}, defaultPoolAssets, defaultFutureGovernor),
+			emptySender: false,
+			expectPass:  false,
+		}, {
+			name: "create the pool with empty PoolAssets",
+			msg: balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
+				SwapFee: sdk.NewDecWithPrec(1, 2),
+				ExitFee: defaultZeroExitFee,
 			}, []balancertypes.PoolAsset{}, defaultFutureGovernor),
 			emptySender: false,
 			expectPass:  false,
 		}, {
 			name: "create the pool with 0 weighted PoolAsset",
 			msg: balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
 				SwapFee: sdk.NewDecWithPrec(1, 2),
-				ExitFee: sdk.NewDecWithPrec(1, 2),
+				ExitFee: defaultZeroExitFee,
 			}, []balancertypes.PoolAsset{{
 				Weight: sdk.NewInt(0),
 				Token:  sdk.NewCoin("foo", sdk.NewInt(10000)),
@@ -126,7 +133,7 @@ func (suite *KeeperTestSuite) TestCreateBalancerPool() {
 			name: "create the pool with negative weighted PoolAsset",
 			msg: balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
 				SwapFee: sdk.NewDecWithPrec(1, 2),
-				ExitFee: sdk.NewDecWithPrec(1, 2),
+				ExitFee: defaultZeroExitFee,
 			}, []balancertypes.PoolAsset{{
 				Weight: sdk.NewInt(-1),
 				Token:  sdk.NewCoin("foo", sdk.NewInt(10000)),
@@ -140,7 +147,7 @@ func (suite *KeeperTestSuite) TestCreateBalancerPool() {
 			name: "create the pool with 0 balance PoolAsset",
 			msg: balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
 				SwapFee: sdk.NewDecWithPrec(1, 2),
-				ExitFee: sdk.NewDecWithPrec(1, 2),
+				ExitFee: defaultZeroExitFee,
 			}, []balancertypes.PoolAsset{{
 				Weight: sdk.NewInt(100),
 				Token:  sdk.NewCoin("foo", sdk.NewInt(0)),
@@ -154,7 +161,7 @@ func (suite *KeeperTestSuite) TestCreateBalancerPool() {
 			name: "create the pool with negative balance PoolAsset",
 			msg: balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
 				SwapFee: sdk.NewDecWithPrec(1, 2),
-				ExitFee: sdk.NewDecWithPrec(1, 2),
+				ExitFee: defaultZeroExitFee,
 			}, []balancertypes.PoolAsset{{
 				Weight: sdk.NewInt(100),
 				Token: sdk.Coin{
@@ -171,7 +178,7 @@ func (suite *KeeperTestSuite) TestCreateBalancerPool() {
 			name: "create the pool with duplicated PoolAssets",
 			msg: balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
 				SwapFee: sdk.NewDecWithPrec(1, 2),
-				ExitFee: sdk.NewDecWithPrec(1, 2),
+				ExitFee: defaultZeroExitFee,
 			}, []balancertypes.PoolAsset{{
 				Weight: sdk.NewInt(100),
 				Token:  sdk.NewCoin("foo", sdk.NewInt(10000)),
@@ -515,7 +522,7 @@ func (suite *KeeperTestSuite) TestJoinPoolNoSwap() {
 		// Create the pool at first
 		msg := balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
 			SwapFee: sdk.NewDecWithPrec(1, 2),
-			ExitFee: sdk.NewDecWithPrec(1, 2),
+			ExitFee: defaultZeroExitFee,
 		}, defaultPoolAssets, defaultFutureGovernor)
 		poolId, err := poolmanagerKeeper.CreatePool(suite.Ctx, msg)
 		suite.Require().NoError(err, "test: %v", test.name)

diff --git a/x/gamm/keeper/pool_test.go b/x/gamm/keeper/pool_test.go
@@ -23,7 +23,7 @@ var (
 	}
 	defaultPoolParamsStableSwap = stableswap.PoolParams{
 		SwapFee: sdk.NewDecWithPrec(1, 2),
-		ExitFee: sdk.NewDecWithPrec(1, 2),
+		ExitFee: sdk.ZeroDec(),
 	}
 	defaultPoolId                        = uint64(1)
 	defaultAcctFundsStableSwap sdk.Coins = sdk.NewCoins(
@@ -286,7 +286,7 @@ func (suite *KeeperTestSuite) TestGetPoolAndPoke() {
 			isPokePool: true,
 			poolId: suite.prepareCustomBalancerPool(defaultAcctFunds, startPoolWeightAssets, balancer.PoolParams{
 				SwapFee: defaultSwapFee,
-				ExitFee: defaultExitFee,
+				ExitFee: defaultZeroExitFee,
 				SmoothWeightChangeParams: &balancer.SmoothWeightChangeParams{
 					StartTime:          time.Unix(startTime, 0), // start time is before block time so the weights should change
 					Duration:           time.Hour,
@@ -300,7 +300,7 @@ func (suite *KeeperTestSuite) TestGetPoolAndPoke() {
 				defaultAcctFunds,
 				stableswap.PoolParams{
 					SwapFee: defaultSwapFee,
-					ExitFee: defaultExitFee,
+					ExitFee: defaultZeroExitFee,
 				},
 				sdk.NewCoins(sdk.NewCoin(defaultAcctFunds[0].Denom, defaultAcctFunds[0].Amount.QuoRaw(2)), sdk.NewCoin(defaultAcctFunds[1].Denom, defaultAcctFunds[1].Amount.QuoRaw(2))),
 				[]uint64{1, 1},
@@ -502,7 +502,7 @@ func (suite *KeeperTestSuite) TestSetStableSwapScalingFactors() {
 					defaultAcctFunds,
 					stableswap.PoolParams{
 						SwapFee: defaultSwapFee,
-						ExitFee: defaultExitFee,
+						ExitFee: defaultZeroExitFee,
 					},
 					sdk.NewCoins(sdk.NewCoin(defaultAcctFunds[0].Denom, defaultAcctFunds[0].Amount.QuoRaw(2)), sdk.NewCoin(defaultAcctFunds[1].Denom, defaultAcctFunds[1].Amount.QuoRaw(2))),
 					tc.scalingFactors,

diff --git a/x/gamm/pool-models/balancer/amm_test.go b/x/gamm/pool-models/balancer/amm_test.go
@@ -30,13 +30,13 @@ func TestBalancerPoolParams(t *testing.T) {
 		shouldErr bool
 	}{
 		// Should work
-		{defaultSwapFee, defaultExitFee, noErr},
+		{defaultSwapFee, defaultZeroExitFee, noErr},
 		// Can't set the swap fee as negative
-		{sdk.NewDecWithPrec(-1, 2), defaultExitFee, wantErr},
+		{sdk.NewDecWithPrec(-1, 2), defaultZeroExitFee, wantErr},
 		// Can't set the swap fee as 1
-		{sdk.NewDec(1), defaultExitFee, wantErr},
+		{sdk.NewDec(1), defaultZeroExitFee, wantErr},
 		// Can't set the swap fee above 1
-		{sdk.NewDecWithPrec(15, 1), defaultExitFee, wantErr},
+		{sdk.NewDecWithPrec(15, 1), defaultZeroExitFee, wantErr},
 		// Can't set the exit fee as negative
 		{defaultSwapFee, sdk.NewDecWithPrec(-1, 2), wantErr},
 		// Can't set the exit fee as 1

diff --git a/x/gamm/pool-models/balancer/balancerPool.pb.go b/x/gamm/pool-models/balancer/balancerPool.pb.go
diff --git a/x/gamm/pool-models/balancer/marshal_test.go b/x/gamm/pool-models/balancer/marshal_test.go
@@ -39,7 +39,7 @@ func TestPoolJson(t *testing.T) {
 	}
 	pacc, err := balancer.NewBalancerPool(poolId, balancer.PoolParams{
 		SwapFee: defaultSwapFee,
-		ExitFee: defaultExitFee,
+		ExitFee: defaultZeroExitFee,
 	}, jsonAssetTest, defaultFutureGovernor, defaultCurBlockTime)
 	require.NoError(t, err)
 
@@ -66,7 +66,7 @@ func TestPoolProtoMarshal(t *testing.T) {
 
 	require.Equal(t, pool2.Id, uint64(10))
 	require.Equal(t, pool2.PoolParams.SwapFee, defaultSwapFee)
-	require.Equal(t, pool2.PoolParams.ExitFee, defaultExitFee)
+	require.Equal(t, pool2.PoolParams.ExitFee, sdk.MustNewDecFromStr("0.025"))
 	require.Equal(t, pool2.FuturePoolGovernor, "")
 	require.Equal(t, pool2.TotalShares, sdk.Coin{Denom: "gamm/pool/10", Amount: sdk.ZeroInt()})
 	require.Equal(t, pool2.PoolAssets, []balancer.PoolAsset{

diff --git a/x/gamm/pool-models/balancer/msgs_test.go b/x/gamm/pool-models/balancer/msgs_test.go
@@ -34,7 +34,7 @@ func TestMsgCreateBalancerPool_ValidateBasic(t *testing.T) {
 
 		poolParams := &balancer.PoolParams{
 			SwapFee: sdk.NewDecWithPrec(1, 2),
-			ExitFee: sdk.NewDecWithPrec(1, 2),
+			ExitFee: sdk.ZeroDec(),
 		}
 
 		msg := &balancer.MsgCreateBalancerPool{
@@ -252,7 +252,7 @@ func (suite *KeeperTestSuite) TestMsgCreateBalancerPool() {
 		"basic success test": {
 			msg: balancer.MsgCreateBalancerPool{
 				Sender:             suite.TestAccs[0].String(),
-				PoolParams:         &balancer.PoolParams{SwapFee: sdk.NewDecWithPrec(1, 2), ExitFee: sdk.NewDecWithPrec(1, 3)},
+				PoolParams:         &balancer.PoolParams{SwapFee: sdk.NewDecWithPrec(1, 2), ExitFee: sdk.ZeroDec()},
 				PoolAssets:         apptesting.DefaultPoolAssets,
 				FuturePoolGovernor: "",
 			},
@@ -261,7 +261,7 @@ func (suite *KeeperTestSuite) TestMsgCreateBalancerPool() {
 		"error due to negative swap fee": {
 			msg: balancer.MsgCreateBalancerPool{
 				Sender:             suite.TestAccs[0].String(),
-				PoolParams:         &balancer.PoolParams{SwapFee: sdk.NewDecWithPrec(1, 2).Neg(), ExitFee: sdk.NewDecWithPrec(1, 3)},
+				PoolParams:         &balancer.PoolParams{SwapFee: sdk.NewDecWithPrec(1, 2).Neg(), ExitFee: sdk.ZeroDec()},
 				PoolAssets:         apptesting.DefaultPoolAssets,
 				FuturePoolGovernor: "",
 			},