Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable create pool with non exit fee #4767

Merged
merged 8 commits into from
Mar 31, 2023
Merged

Disable create pool with non exit fee #4767

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
061f0e4
disable in create pool
hieuvubk Mar 27, 2023
4a35d22
fix ttests
hieuvubk Mar 28, 2023
4f5773a
fix e2e
hieuvubk Mar 28, 2023
6af7c9c
add test case
hieuvubk Mar 28, 2023
5536b82
change exit fee e2e
hieuvubk Mar 28, 2023
90730d9
refactor
hieuvubk Mar 29, 2023
4a0cc6a
update changelog
hieuvubk Mar 29, 2023
e1304d8
update proto
hieuvubk Mar 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/apptesting/test_suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion app/upgrades/v15/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/scripts/ibcDenomPool.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": ""
}
2 changes: 1 addition & 1 deletion tests/e2e/scripts/nativeDenomPool.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": ""
}
2 changes: 1 addition & 1 deletion tests/e2e/scripts/nativeDenomThreeAssetPool.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": ""
}
2 changes: 1 addition & 1 deletion tests/e2e/scripts/pool1A.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": ""
}
2 changes: 1 addition & 1 deletion tests/e2e/scripts/pool1B.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": ""
}
4 changes: 2 additions & 2 deletions tests/ibc-hooks/ibc_middleware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
8 changes: 4 additions & 4 deletions x/gamm/keeper/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)),
Expand All @@ -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)),
Expand Down Expand Up @@ -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)),
Expand Down
22 changes: 11 additions & 11 deletions x/gamm/keeper/pool_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (

var (
defaultSwapFee = sdk.MustNewDecFromStr("0.025")
defaultExitFee = sdk.MustNewDecFromStr("0.025")
defaultExitFee = sdk.ZeroDec()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we just define a defaultZeroExitFee in the keeper_test file as a global var and use it everywhere?

defaultPoolParams = balancer.PoolParams{
SwapFee: defaultSwapFee,
ExitFee: defaultExitFee,
Expand Down Expand Up @@ -88,31 +88,31 @@ 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: defaultExitFee,
}, defaultPoolAssets, defaultFutureGovernor),
emptySender: false,
expectPass: false,
}, {
name: "create a pool with negative exit fee",
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),
ExitFee: sdk.NewDecWithPrec(1, 2),
}, defaultPoolAssets, defaultFutureGovernor),
emptySender: false,
expectPass: false,
}, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo we should test both a negative and a non zero exit fee

name: "create the pool with empty PoolAssets",
msg: balancer.NewMsgCreateBalancerPool(testAccount, balancer.PoolParams{
SwapFee: sdk.NewDecWithPrec(1, 2),
ExitFee: sdk.NewDecWithPrec(1, 2),
ExitFee: defaultExitFee,
}, []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: defaultExitFee,
}, []balancertypes.PoolAsset{{
Weight: sdk.NewInt(0),
Token: sdk.NewCoin("foo", sdk.NewInt(10000)),
Expand All @@ -126,7 +126,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: defaultExitFee,
}, []balancertypes.PoolAsset{{
Weight: sdk.NewInt(-1),
Token: sdk.NewCoin("foo", sdk.NewInt(10000)),
Expand All @@ -140,7 +140,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: defaultExitFee,
}, []balancertypes.PoolAsset{{
Weight: sdk.NewInt(100),
Token: sdk.NewCoin("foo", sdk.NewInt(0)),
Expand All @@ -154,7 +154,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: defaultExitFee,
}, []balancertypes.PoolAsset{{
Weight: sdk.NewInt(100),
Token: sdk.Coin{
Expand All @@ -171,7 +171,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: defaultExitFee,
}, []balancertypes.PoolAsset{{
Weight: sdk.NewInt(100),
Token: sdk.NewCoin("foo", sdk.NewInt(10000)),
Expand Down Expand Up @@ -515,7 +515,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: defaultExitFee,
}, defaultPoolAssets, defaultFutureGovernor)
poolId, err := poolmanagerKeeper.CreatePool(suite.Ctx, msg)
suite.Require().NoError(err, "test: %v", test.name)
Expand Down
2 changes: 1 addition & 1 deletion x/gamm/keeper/pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion x/gamm/pool-models/balancer/marshal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
6 changes: 3 additions & 3 deletions x/gamm/pool-models/balancer/msgs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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: "",
},
Expand All @@ -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: "",
},
Expand Down
2 changes: 1 addition & 1 deletion x/gamm/pool-models/balancer/pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (

var (
defaultSwapFee = sdk.MustNewDecFromStr("0.025")
defaultExitFee = sdk.MustNewDecFromStr("0.025")
defaultExitFee = sdk.ZeroDec()
defaultPoolId = uint64(10)
defaultBalancerPoolParams = balancer.PoolParams{
SwapFee: defaultSwapFee,
Expand Down
4 changes: 2 additions & 2 deletions x/gamm/pool-models/stableswap/msgs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func baseCreatePoolMsgGen(sender sdk.AccAddress) *stableswap.MsgCreateStableswap

poolParams := &stableswap.PoolParams{
SwapFee: sdk.NewDecWithPrec(1, 2),
ExitFee: sdk.NewDecWithPrec(1, 2),
ExitFee: sdk.ZeroDec(),
}

msg := &stableswap.MsgCreateStableswapPool{
Expand Down Expand Up @@ -300,7 +300,7 @@ func (suite *TestSuite) TestMsgCreateStableswapPool() {
suite.SetupTest()

var (
validParams = &stableswap.PoolParams{SwapFee: sdk.NewDecWithPrec(1, 2), ExitFee: sdk.NewDecWithPrec(1, 3)}
validParams = &stableswap.PoolParams{SwapFee: sdk.NewDecWithPrec(1, 2), ExitFee: sdk.ZeroDec()}
validInitialLiquidity = sdk.NewCoins(sdk.NewCoin("usdc", sdk.NewInt(1_000_000)), sdk.NewCoin("usdt", sdk.NewInt(2_000_000)))
validScalingFactors = []uint64{1, 1}
invalidScalingFactors = []uint64{1, 1, 1}
Expand Down
5 changes: 5 additions & 0 deletions x/poolmanager/create_pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ func (k Keeper) CreatePool(ctx sdk.Context, msg types.CreatePoolMsg) (uint64, er
return 0, err
}

exitFee := pool.GetExitFee(ctx)
if !exitFee.Equal(sdk.ZeroDec()) {
return 0, fmt.Errorf("can not create pool with non zero exit fee, got %d", exitFee)
}

k.SetPoolRoute(ctx, poolId, msg.GetPoolType())

if err := k.validateCreatedPool(ctx, poolId, pool); err != nil {
Expand Down
24 changes: 21 additions & 3 deletions x/poolmanager/create_pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,23 @@ func (suite *KeeperTestSuite) TestPoolCreationFee() {
poolCreationFee: sdk.Coins{},
msg: balancer.NewMsgCreateBalancerPool(suite.TestAccs[0], balancer.PoolParams{
SwapFee: sdk.NewDecWithPrec(1, 2),
ExitFee: sdk.NewDecWithPrec(1, 2),
ExitFee: sdk.ZeroDec(),
}, apptesting.DefaultPoolAssets, ""),
expectPass: true,
}, {
name: "nil pool creation fee on basic pool",
poolCreationFee: nil,
msg: balancer.NewMsgCreateBalancerPool(suite.TestAccs[0], balancer.PoolParams{
SwapFee: sdk.NewDecWithPrec(1, 2),
ExitFee: sdk.NewDecWithPrec(1, 2),
ExitFee: sdk.ZeroDec(),
}, apptesting.DefaultPoolAssets, ""),
expectPass: true,
}, {
name: "attempt pool creation without sufficient funds for fees",
poolCreationFee: sdk.Coins{sdk.NewCoin("atom", sdk.NewInt(10000))},
msg: balancer.NewMsgCreateBalancerPool(suite.TestAccs[0], balancer.PoolParams{
SwapFee: sdk.NewDecWithPrec(1, 2),
ExitFee: sdk.NewDecWithPrec(1, 2),
ExitFee: sdk.ZeroDec(),
}, apptesting.DefaultPoolAssets, ""),
expectPass: false,
},
Expand Down Expand Up @@ -128,6 +128,17 @@ func (suite *KeeperTestSuite) TestCreatePool() {
},
}, "")

invalidBalancerPoolMsg := balancer.NewMsgCreateBalancerPool(suite.TestAccs[0], balancer.NewPoolParams(sdk.ZeroDec(), sdk.NewDecWithPrec(1, 2), nil), []balancer.PoolAsset{
{
Token: sdk.NewCoin(foo, defaultInitPoolAmount),
Weight: sdk.NewInt(1),
},
{
Token: sdk.NewCoin(bar, defaultInitPoolAmount),
Weight: sdk.NewInt(1),
},
}, "")

validConcentratedPoolMsg := clmodel.NewMsgCreateConcentratedPool(suite.TestAccs[0], foo, bar, 1, DefaultExponentAtPriceOne, defaultPoolSwapFee)

tests := []struct {
Expand Down Expand Up @@ -155,6 +166,13 @@ func (suite *KeeperTestSuite) TestCreatePool() {
msg: validConcentratedPoolMsg,
expectedModuleType: concentratedKeeperType,
},
{
name: "pool with non zero exit fee - success",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name: "pool with non zero exit fee - success",
name: "pool with non zero exit fee - error",

creatorFundAmount: sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount.Mul(sdk.NewInt(2))), sdk.NewCoin(bar, defaultInitPoolAmount.Mul(sdk.NewInt(2)))),
msg: invalidBalancerPoolMsg,
expectedModuleType: gammKeeperType,
expectError: true,
},
// TODO: add stableswap test
// TODO: add concentrated-liquidity test
// TODO: cover errors and edge cases
Expand Down
2 changes: 1 addition & 1 deletion x/superfluid/keeper/unpool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

var (
defaultSwapFee = sdk.MustNewDecFromStr("0.025")
defaultExitFee = sdk.MustNewDecFromStr("0.025")
defaultExitFee = sdk.ZeroDec()
defaultPoolParams = balancer.PoolParams{
SwapFee: defaultSwapFee,
ExitFee: defaultExitFee,
Expand Down