From 2a6b5af35d90a31b0c35dada989ab97d4d59b3c0 Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 27 Feb 2023 16:47:04 -0300 Subject: [PATCH 1/4] feat: gracefully handle join spot pool with wrong tokens denom --- x/spot/client/testutil/suite.go | 8 ++++++++ x/spot/keeper/keeper.go | 5 +++++ x/spot/types/pool.go | 22 ++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/x/spot/client/testutil/suite.go b/x/spot/client/testutil/suite.go index 81f34a453..bc25db753 100644 --- a/x/spot/client/testutil/suite.go +++ b/x/spot/client/testutil/suite.go @@ -244,6 +244,14 @@ func (s *IntegrationTestSuite) TestNewJoinPoolCmd() { respType: &sdk.TxResponse{}, expectedCode: 5, // bankKeeper code for insufficient funds }, + { + name: "join pool with wrong tokens", + poolId: poolID, + tokensIn: fmt.Sprintf("1000000000%s,10000000000%s", "coin-1", "coin-3"), + expectErr: false, + respType: &sdk.TxResponse{}, + expectedCode: 13, // bankKeeper code for insufficient funds + }, { name: "join pool with sufficient balance", poolId: poolID, diff --git a/x/spot/keeper/keeper.go b/x/spot/keeper/keeper.go index e4bbb074c..3a8231bb9 100644 --- a/x/spot/keeper/keeper.go +++ b/x/spot/keeper/keeper.go @@ -457,6 +457,11 @@ func (k Keeper) JoinPool( return pool, numSharesOut, remCoins, errors.New("too few assets to join this pool") } + if !pool.AreTokensInDenomInPoolAssets(tokensIn) { + err = types.ErrTokenDenomNotFound + return + } + poolAddr := pool.GetAddress() var numShares sdk.Int diff --git a/x/spot/types/pool.go b/x/spot/types/pool.go index fa2258632..b189d2202 100644 --- a/x/spot/types/pool.go +++ b/x/spot/types/pool.go @@ -82,6 +82,28 @@ func NewPool( return pool, nil } +/* +Ensure the denoms of the tokens in are assets of the pool + +args: + + - tokensIn: the tokens to check + +ret: + + - ok: true if all the denom from tokens in tokens in are in the pool +*/ +func (pool Pool) AreTokensInDenomInPoolAssets(tokensIn sdk.Coins) bool { + for _, asset := range tokensIn { + _, _, err := pool.getPoolAssetAndIndex(asset.Denom) + + if err != nil { + return false + } + } + return true +} + /* Adds tokens to a pool and updates the pool balances (i.e. liquidity). From 9315b605c167406665027ee14ce2fcb5ae70759c Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 27 Feb 2023 16:51:23 -0300 Subject: [PATCH 2/4] chore: changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fdd7e4aa..a41dadfd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -98,6 +98,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#1131](https://github.com/NibiruChain/nibiru/pull/1131) - fix(oracle): use correct distribution module account * [#1151](https://github.com/NibiruChain/nibiru/pull/1151) - fix(dex): fix swap calculation for stableswap pools * [#1210](https://github.com/NibiruChain/nibiru/pull/1210) - fix(ci): fix docker push workflow +* [#1212](https://github.com/NibiruChain/nibiru/pull/1212) - fix(spot): gracefully handle join spot pool with wrong tokens denom ## [v0.16.3](https://github.com/NibiruChain/nibiru/releases/tag/v0.16.3) From 0b8553a0fc15f9e14ed254ffa1a390787593148c Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 27 Feb 2023 19:09:54 -0300 Subject: [PATCH 3/4] chore: lint --- simapp/sim_test.go | 5 ++- x/common/testutil/cli/network.go | 3 +- x/perp/genesis_test.go | 7 +-- x/perp/keeper/liquidate.go | 6 +-- x/spot/types/pool.go | 3 +- x/spot/types/pool_test.go | 75 ++++++++++++++++++++++++++++++++ x/spot/types/shares.go | 20 +++++++++ 7 files changed, 109 insertions(+), 10 deletions(-) diff --git a/simapp/sim_test.go b/simapp/sim_test.go index 4afb8d3ff..1cdccff8f 100644 --- a/simapp/sim_test.go +++ b/simapp/sim_test.go @@ -7,8 +7,6 @@ import ( "os" "testing" - "github.com/NibiruChain/nibiru/app" - "github.com/NibiruChain/nibiru/x/common/testutil/testapp" sdkSimapp "github.com/cosmos/cosmos-sdk/simapp" "github.com/cosmos/cosmos-sdk/simapp/helpers" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" @@ -16,6 +14,9 @@ import ( "github.com/cosmos/cosmos-sdk/x/simulation" "github.com/stretchr/testify/require" dbm "github.com/tendermint/tm-db" + + "github.com/NibiruChain/nibiru/app" + "github.com/NibiruChain/nibiru/x/common/testutil/testapp" ) func init() { diff --git a/x/common/testutil/cli/network.go b/x/common/testutil/cli/network.go index 88514ddf3..ea485a33b 100644 --- a/x/common/testutil/cli/network.go +++ b/x/common/testutil/cli/network.go @@ -17,7 +17,6 @@ import ( "github.com/NibiruChain/nibiru/x/common/denoms" "github.com/NibiruChain/nibiru/x/common/testutil/testapp" - "github.com/NibiruChain/nibiru/app" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/codec" @@ -44,6 +43,8 @@ import ( "github.com/tendermint/tendermint/node" tmclient "github.com/tendermint/tendermint/rpc/client" "google.golang.org/grpc" + + "github.com/NibiruChain/nibiru/app" ) // package-wide network lock to only allow one test network at a time diff --git a/x/perp/genesis_test.go b/x/perp/genesis_test.go index 493bf3f2b..f2cbeffa4 100644 --- a/x/perp/genesis_test.go +++ b/x/perp/genesis_test.go @@ -6,6 +6,10 @@ import ( "time" "github.com/NibiruChain/collections" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + "github.com/NibiruChain/nibiru/app" "github.com/NibiruChain/nibiru/x/common/asset" "github.com/NibiruChain/nibiru/x/common/denoms" @@ -13,9 +17,6 @@ import ( "github.com/NibiruChain/nibiru/x/common/testutil/testapp" "github.com/NibiruChain/nibiru/x/perp" "github.com/NibiruChain/nibiru/x/perp/types" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/stretchr/testify/require" - tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) func TestGenesis(t *testing.T) { diff --git a/x/perp/keeper/liquidate.go b/x/perp/keeper/liquidate.go index 211e5cf44..de1c083a9 100644 --- a/x/perp/keeper/liquidate.go +++ b/x/perp/keeper/liquidate.go @@ -33,7 +33,7 @@ func (k Keeper) Liquidate( ) (liquidatorFee sdk.Coin, perpEcosystemFundFee sdk.Coin, err error) { err = k.requireVpool(ctx, pair) if err != nil { - ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ + err = ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ Pair: pair, Trader: trader.String(), Liquidator: liquidator.String(), @@ -44,7 +44,7 @@ func (k Keeper) Liquidate( position, err := k.Positions.Get(ctx, collections.Join(pair, trader)) if err != nil { - ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ + err = ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ Pair: pair, Trader: trader.String(), Liquidator: liquidator.String(), @@ -84,7 +84,7 @@ func (k Keeper) Liquidate( } err = validateMarginRatio(marginRatio, maintenanceMarginRatio, false) if err != nil { - ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ + err = ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ Pair: pair, Trader: trader.String(), Liquidator: liquidator.String(), diff --git a/x/spot/types/pool.go b/x/spot/types/pool.go index b189d2202..9d66bf1f4 100644 --- a/x/spot/types/pool.go +++ b/x/spot/types/pool.go @@ -214,7 +214,8 @@ func (pool *Pool) ExitPool(exitingShares sdk.Int) ( } if !exitedCoins.IsValid() { - return sdk.Coins{}, errors.New("not enough pool shares to withdraw") + minShares := pool.MinSharesInForTokensOut() + return sdk.Coins{}, fmt.Errorf("not enough pool shares to withdraw - please provide at least %v shares", minShares) } // update the pool's balances diff --git a/x/spot/types/pool_test.go b/x/spot/types/pool_test.go index e0d13f529..ac941b6f0 100644 --- a/x/spot/types/pool_test.go +++ b/x/spot/types/pool_test.go @@ -69,6 +69,32 @@ func TestGetAddress(t *testing.T) { } } +func TestMinSharesInForTokensOut(t *testing.T) { + poolAccountAddr := testutil.AccAddress() + poolParams := PoolParams{ + PoolType: PoolType_BALANCER, + SwapFee: sdk.NewDecWithPrec(3, 2), + ExitFee: sdk.NewDecWithPrec(3, 2), + } + poolAssets := []PoolAsset{ + { + Token: sdk.NewInt64Coin("foo", 123124124124), + Weight: sdk.NewInt(1), + }, + { + Token: sdk.NewInt64Coin("bar", 13224112312124124), + Weight: sdk.NewInt(1), + }, + } + + pool, err := NewPool(1 /*=poold*/, poolAccountAddr, poolParams, poolAssets) + require.NoError(t, err) + + tokenOut, err := pool.TokensOutFromPoolSharesIn(pool.MinSharesInForTokensOut()) + require.NoError(t, err) + require.True(t, tokenOut.IsValid()) +} + func TestNewPool(t *testing.T) { poolAccountAddr := testutil.AccAddress() poolParams := PoolParams{ @@ -474,6 +500,55 @@ func TestJoinPoolAllTokens(t *testing.T) { } } +func TestExitPoolError(t *testing.T) { + for _, tc := range []struct { + name string + pool Pool + exitingShares sdk.Coin + expectedCoins sdk.Coins + expectedRemainingShares sdk.Coin + expectedExitedCoins sdk.Coins + }{ + { + name: "all coins withdrawn, no exit fee", + pool: Pool{ + PoolAssets: []PoolAsset{ + { + Token: sdk.NewInt64Coin("aaa", 1000000), + }, + { + Token: sdk.NewInt64Coin("bbb", 2000000), + }, + }, + TotalShares: sdk.NewInt64Coin("nibiru/pool/1", 10000000000000), + PoolParams: PoolParams{ + PoolType: PoolType_BALANCER, + ExitFee: sdk.ZeroDec(), + }, + }, + exitingShares: sdk.NewInt64Coin("nibiru/pool/1", 100), + expectedRemainingShares: sdk.NewInt64Coin("nibiru/pool/1", 0), + expectedCoins: nil, + expectedExitedCoins: sdk.NewCoins( + sdk.NewInt64Coin("aaa", 100), + sdk.NewInt64Coin("bbb", 200), + ), + }, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + _, err := tc.pool.ExitPool(sdk.OneInt()) + fmt.Println(err) + require.Error(t, err) + expectedErrorMsg := fmt.Sprintf("not enough pool shares to withdraw - please provide at least %v shares", tc.pool.MinSharesInForTokensOut()) + require.Contains(t, err.Error(), expectedErrorMsg) + + _, err = tc.pool.ExitPool(tc.pool.MinSharesInForTokensOut()) + require.NoError(t, err) + }) + } +} + func TestExitPoolHappyPath(t *testing.T) { for _, tc := range []struct { name string diff --git a/x/spot/types/shares.go b/x/spot/types/shares.go index 899d90a44..afb464484 100644 --- a/x/spot/types/shares.go +++ b/x/spot/types/shares.go @@ -194,6 +194,26 @@ func (pool Pool) TokensOutFromPoolSharesIn(numSharesIn sdk.Int) ( return tokensOut, nil } +/* +Compute the minimum number of shares a user need to provide to get at least one u-token +*/ +func (pool Pool) MinSharesInForTokensOut() (minShares sdk.Int) { + poolLiquidity := pool.PoolBalances() + + minShares = sdk.ZeroInt() + + for _, coin := range poolLiquidity { + shareRatio := sdk.MustNewDecFromStr("2").Quo(coin.Amount.ToDec()).Quo(sdk.OneDec().Sub(pool.PoolParams.ExitFee)) + + shares := shareRatio.MulInt(pool.TotalShares.Amount).TruncateInt() + + if minShares.IsZero() || minShares.LT(shares) { + minShares = shares + } + } + return +} + /* Adds new liquidity to the pool and increments the total number of shares. From 4bcc5893bb98c539a166fa8bd818c7b7d56e392c Mon Sep 17 00:00:00 2001 From: Matthias Date: Tue, 28 Feb 2023 07:44:20 -0300 Subject: [PATCH 4/4] chore: lint --- x/perp/keeper/liquidate.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/x/perp/keeper/liquidate.go b/x/perp/keeper/liquidate.go index de1c083a9..8359c2622 100644 --- a/x/perp/keeper/liquidate.go +++ b/x/perp/keeper/liquidate.go @@ -33,7 +33,7 @@ func (k Keeper) Liquidate( ) (liquidatorFee sdk.Coin, perpEcosystemFundFee sdk.Coin, err error) { err = k.requireVpool(ctx, pair) if err != nil { - err = ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ + _ = ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ Pair: pair, Trader: trader.String(), Liquidator: liquidator.String(), @@ -44,7 +44,7 @@ func (k Keeper) Liquidate( position, err := k.Positions.Get(ctx, collections.Join(pair, trader)) if err != nil { - err = ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ + _ = ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ Pair: pair, Trader: trader.String(), Liquidator: liquidator.String(), @@ -84,7 +84,7 @@ func (k Keeper) Liquidate( } err = validateMarginRatio(marginRatio, maintenanceMarginRatio, false) if err != nil { - err = ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ + _ = ctx.EventManager().EmitTypedEvent(&types.LiquidationFailedEvent{ Pair: pair, Trader: trader.String(), Liquidator: liquidator.String(), @@ -204,7 +204,7 @@ func (k Keeper) ExecuteFullLiquidation( return types.LiquidateResp{}, err } - err = ctx.EventManager().EmitTypedEvent(&types.PositionLiquidatedEvent{ + _ = ctx.EventManager().EmitTypedEvent(&types.PositionLiquidatedEvent{ Pair: position.Pair, TraderAddress: traderAddr.String(), ExchangedQuoteAmount: positionResp.ExchangedNotionalValue, @@ -347,7 +347,7 @@ func (k Keeper) ExecutePartialLiquidation( return types.LiquidateResp{}, err } - err = ctx.EventManager().EmitTypedEvent(&types.PositionLiquidatedEvent{ + _ = ctx.EventManager().EmitTypedEvent(&types.PositionLiquidatedEvent{ Pair: currentPosition.Pair, TraderAddress: traderAddr.String(), ExchangedQuoteAmount: positionResp.ExchangedNotionalValue,