feat: gracefully handle join spot pool with wrong tokens denom

CHANGELOG.md

@@ -106,6 +106,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#1126](https://github.com/NibiruChain/nibiru/pull/1126) - test(oracle): stop the tyrannical behavior of TestFuzz_PickReferencePair
 * [#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)
 

simapp/sim_test.go

@@ -7,15 +7,16 @@ 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"
 	simulationtypes "github.com/cosmos/cosmos-sdk/types/simulation"
 	"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() {

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

x/perp/genesis_test.go

@@ -6,16 +6,17 @@ 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"
 	"github.com/NibiruChain/nibiru/x/common/testutil"
 	"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) {

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{
+		_ = 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{
+		_ = 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{
+		_ = 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,

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,

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

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).
 
@@ -192,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

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

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.