fix: add overflow checking and test codes for cover edge cases

x/liquidity/keeper/batch.go

@@ -236,7 +236,14 @@ func (k Keeper) WithdrawWithinBatch(ctx sdk.Context, msg *types.MsgWithdrawWithi
 
 // In order to deal with the batch at the same time, the coins of msgs are deposited in escrow.
 func (k Keeper) SwapWithinBatch(ctx sdk.Context, msg *types.MsgSwapWithinBatch, orderExpirySpanHeight int64) (*types.SwapMsgState, error) {
-	if err := k.ValidateMsgSwapWithinBatch(ctx, *msg); err != nil {
+	pool, found := k.GetPool(ctx, msg.PoolId)
+	if !found {
+		return nil, types.ErrPoolNotExists
+	}
+	if k.IsDepletedPool(ctx, pool) {
+		return nil, types.ErrDepletedPool
+	}
+	if err := k.ValidateMsgSwapWithinBatch(ctx, *msg, pool); err != nil {
 		return nil, err
 	}
 	poolBatch, found := k.GetPoolBatch(ctx, msg.PoolId)

x/liquidity/keeper/liquidity_pool.go

@@ -246,6 +246,9 @@ func (k Keeper) ExecuteDeposit(ctx sdk.Context, msg types.DepositMsgState, batch
 	depositCoinB := depositCoins[1]
 
 	poolCoinTotalSupply := k.GetPoolCoinTotalSupply(ctx, pool)
+	if err := types.CheckOverflow(poolCoinTotalSupply, depositCoinA.Amount); err != nil {
+		return err
+	}
 	poolCoinMintAmt := sdk.MinDec(
 		poolCoinTotalSupply.ToDec().MulTruncate(depositCoinA.Amount.ToDec()).QuoTruncate(lastReserveCoinA.Amount.ToDec()),
 		poolCoinTotalSupply.ToDec().MulTruncate(depositCoinB.Amount.ToDec()).QuoTruncate(lastReserveCoinB.Amount.ToDec()),
@@ -378,6 +381,9 @@ func (k Keeper) ExecuteWithdrawal(ctx sdk.Context, msg types.WithdrawMsgState, b
 	} else {
 		// Calculate withdraw amount of respective reserve coin considering fees and pool coin's totally supply
 		for _, reserveCoin := range reserveCoins {
+			if err := types.CheckOverflow(reserveCoin.Amount, msg.Msg.PoolCoin.Amount); err != nil {
+				return err
+			}
 			// WithdrawAmount = ReserveAmount * PoolCoinAmount * WithdrawFeeProportion / TotalSupply
 			withdrawAmtWithFee := reserveCoin.Amount.Mul(msg.Msg.PoolCoin.Amount).ToDec().TruncateInt().Quo(poolCoinTotalSupply)
 			withdrawAmt := reserveCoin.Amount.Mul(msg.Msg.PoolCoin.Amount).ToDec().MulTruncate(withdrawProportion).TruncateInt().Quo(poolCoinTotalSupply)
@@ -770,21 +776,12 @@ func (k Keeper) ValidateMsgWithdrawWithinBatch(ctx sdk.Context, msg types.MsgWit
 }
 
 // ValidateMsgSwapWithinBatch validates MsgSwapWithinBatch.
-func (k Keeper) ValidateMsgSwapWithinBatch(ctx sdk.Context, msg types.MsgSwapWithinBatch) error {
-	pool, found := k.GetPool(ctx, msg.PoolId)
-	if !found {
-		return types.ErrPoolNotExists
-	}
-
+func (k Keeper) ValidateMsgSwapWithinBatch(ctx sdk.Context, msg types.MsgSwapWithinBatch, pool types.Pool) error {
 	denomA, denomB := types.AlphabeticalDenomPair(msg.OfferCoin.Denom, msg.DemandCoinDenom)
 	if denomA != pool.ReserveCoinDenoms[0] || denomB != pool.ReserveCoinDenoms[1] {
 		return types.ErrNotMatchedReserveCoin
 	}
 
-	if k.IsDepletedPool(ctx, pool) {
-		return types.ErrDepletedPool
-	}
-
 	params := k.GetParams(ctx)
 
 	// can not exceed max order ratio  of reserve coins that can be ordered at a order
@@ -800,6 +797,10 @@ func (k Keeper) ValidateMsgSwapWithinBatch(ctx sdk.Context, msg types.MsgSwapWit
 		return types.ErrBadOfferCoinFee
 	}
 
+	if err := types.CheckOverflowWithDec(msg.OfferCoin.Amount.ToDec(), msg.OrderPrice); err != nil {
+		return types.ErrOverflowAmount
+	}
+
 	if !msg.OfferCoinFee.Equal(types.GetOfferCoinFee(msg.OfferCoin, params.SwapFeeRate)) {
 		return types.ErrBadOfferCoinFee
 	}

x/liquidity/keeper/liquidity_pool_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/tendermint/liquidity/app"
 	"github.com/tendermint/liquidity/x/liquidity"
+	"github.com/tendermint/liquidity/x/liquidity/keeper"
 	"github.com/tendermint/liquidity/x/liquidity/types"
 )
 
@@ -74,6 +75,28 @@ func TestCreatePool(t *testing.T) {
 	require.ErrorIs(t, err, types.ErrPoolAlreadyExists)
 }
 
+func TestCreatePoolInsufficientAmount(t *testing.T) {
+	simapp, ctx := createTestInput()
+	params := simapp.LiquidityKeeper.GetParams(ctx)
+
+	depositCoins := sdk.NewCoins(sdk.NewInt64Coin(DenomX, 1000), sdk.NewInt64Coin(DenomY, 1000))
+	creator := app.AddRandomTestAddr(simapp, ctx, depositCoins.Add(params.PoolCreationFee...))
+
+	// Depositing coins that are less than params.MinInitDepositAmount.
+	_, err := simapp.LiquidityKeeper.CreatePool(ctx, types.NewMsgCreatePool(creator, types.DefaultPoolTypeID, depositCoins))
+	require.ErrorIs(t, err, types.ErrLessThanMinInitDeposit)
+
+	fakeDepositCoins := depositCoins.Add(
+		sdk.NewCoin(DenomX, params.MinInitDepositAmount),
+		sdk.NewCoin(DenomY, params.MinInitDepositAmount),
+	)
+	// Depositing coins that are greater than the depositor has.
+	_, err = simapp.LiquidityKeeper.CreatePool(
+		ctx, types.NewMsgCreatePool(creator, types.DefaultPoolTypeID, fakeDepositCoins),
+	)
+	require.ErrorIs(t, err, types.ErrInsufficientBalance)
+}
+
 func TestPoolCreationFee(t *testing.T) {
 	simapp, ctx := createTestInput()
 	simapp.LiquidityKeeper.SetParams(ctx, types.DefaultParams())
@@ -483,6 +506,73 @@ func TestExecuteWithdrawal(t *testing.T) {
 	require.Equal(t, deposit.AmountOf(pool.ReserveCoinDenoms[1]), withdrawerDenomBBalance.Amount)
 }
 
+func TestSmallWithdrawalCase(t *testing.T) {
+	simapp, ctx := createTestInput()
+	params := types.DefaultParams()
+	params.InitPoolCoinMintAmount = sdk.NewInt(1_000000_000000)
+	simapp.LiquidityKeeper.SetParams(ctx, params)
+
+	poolTypeID := types.DefaultPoolTypeID
+	addrs := app.AddTestAddrs(simapp, ctx, 3, params.PoolCreationFee)
+
+	denomA := "uETH"
+	denomB := "uUSD"
+	denomA, denomB = types.AlphabeticalDenomPair(denomA, denomB)
+
+	deposit := sdk.NewCoins(sdk.NewCoin(denomA, sdk.NewInt(1250001*1000000)), sdk.NewCoin(denomB, sdk.NewInt(9*1000000)))
+	app.SaveAccount(simapp, ctx, addrs[0], deposit)
+
+	depositA := simapp.BankKeeper.GetBalance(ctx, addrs[0], denomA)
+	depositB := simapp.BankKeeper.GetBalance(ctx, addrs[0], denomB)
+	depositBalance := sdk.NewCoins(depositA, depositB)
+
+	require.Equal(t, deposit, depositBalance)
+
+	createMsg := types.NewMsgCreatePool(addrs[0], poolTypeID, depositBalance)
+
+	_, err := simapp.LiquidityKeeper.CreatePool(ctx, createMsg)
+	require.NoError(t, err)
+
+	pools := simapp.LiquidityKeeper.GetAllPools(ctx)
+	pool := pools[0]
+
+	// Case for normal withdrawing
+	poolCoinBefore := simapp.LiquidityKeeper.GetPoolCoinTotalSupply(ctx, pool)
+	withdrawerPoolCoinBefore := simapp.BankKeeper.GetBalance(ctx, addrs[0], pool.PoolCoinDenom)
+
+	withdrawerDenomABalanceBefore := simapp.BankKeeper.GetBalance(ctx, addrs[0], pool.ReserveCoinDenoms[0])
+	withdrawerDenomBBalanceBefore := simapp.BankKeeper.GetBalance(ctx, addrs[0], pool.ReserveCoinDenoms[1])
+
+	require.Equal(t, poolCoinBefore, withdrawerPoolCoinBefore.Amount)
+	withdrawMsg := types.NewMsgWithdrawWithinBatch(addrs[0], pool.Id, sdk.NewCoin(pool.PoolCoinDenom, sdk.NewInt(1)))
+
+	_, err = simapp.LiquidityKeeper.WithdrawWithinBatch(ctx, withdrawMsg)
+	require.NoError(t, err)
+
+	poolBatch, found := simapp.LiquidityKeeper.GetPoolBatch(ctx, withdrawMsg.PoolId)
+	require.True(t, found)
+	msgs := simapp.LiquidityKeeper.GetAllPoolBatchWithdrawMsgStates(ctx, poolBatch)
+	require.Equal(t, 1, len(msgs))
+
+	liquidity.EndBlocker(ctx, simapp.LiquidityKeeper)
+	liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
+
+	poolCoinAfter := simapp.LiquidityKeeper.GetPoolCoinTotalSupply(ctx, pool)
+	withdrawerPoolCoinAfter := simapp.BankKeeper.GetBalance(ctx, addrs[0], pool.PoolCoinDenom)
+
+	require.Equal(t, poolCoinAfter, poolCoinBefore)
+	require.Equal(t, withdrawerPoolCoinAfter.Amount, withdrawerPoolCoinBefore.Amount)
+	withdrawerDenomABalance := simapp.BankKeeper.GetBalance(ctx, addrs[0], pool.ReserveCoinDenoms[0])
+	withdrawerDenomBBalance := simapp.BankKeeper.GetBalance(ctx, addrs[0], pool.ReserveCoinDenoms[1])
+
+	reservePoolBalanceA := simapp.BankKeeper.GetBalance(ctx, pool.GetReserveAccount(), pool.ReserveCoinDenoms[0])
+	reservePoolBalanceB := simapp.BankKeeper.GetBalance(ctx, pool.GetReserveAccount(), pool.ReserveCoinDenoms[1])
+	require.Equal(t, deposit.AmountOf(pool.ReserveCoinDenoms[0]), reservePoolBalanceA.Amount)
+	require.Equal(t, deposit.AmountOf(pool.ReserveCoinDenoms[1]), reservePoolBalanceB.Amount)
+	require.Equal(t, withdrawerDenomABalanceBefore, withdrawerDenomABalance)
+	require.Equal(t, withdrawerDenomBBalanceBefore, withdrawerDenomBBalance)
+}
+
 func TestReinitializePool(t *testing.T) {
 	simapp, ctx := createTestInput()
 	simapp.LiquidityKeeper.SetParams(ctx, types.DefaultParams())
@@ -1016,3 +1106,113 @@ func TestDepositWithCoinsSent(t *testing.T) {
 	require.True(sdk.IntEq(t, sdk.NewInt(0), balances.AmountOf(DenomY)))
 	require.True(sdk.IntEq(t, sdk.NewInt(1000000), balances.AmountOf(pool.PoolCoinDenom)))
 }
+
+func TestPoolEdgeCases(t *testing.T) {
+	simapp, ctx := createTestInput()
+	params := types.DefaultParams()
+	simapp.LiquidityKeeper.SetParams(ctx, params)
+	keeper.BatchLogicInvariantCheckFlag = false
+
+	poolTypeID := types.DefaultPoolTypeID
+	addrs := app.AddTestAddrs(simapp, ctx, 3, params.PoolCreationFee)
+
+	denomA := "uETH"
+	denomB := "uUSD"
+	denomA, denomB = types.AlphabeticalDenomPair(denomA, denomB)
+
+	// Check Equal Denom Case
+	msg := types.NewMsgCreatePool(addrs[0], poolTypeID,
+		sdk.Coins{
+			sdk.NewCoin(denomA, sdk.NewInt(1000000)),
+			sdk.NewCoin(denomA, sdk.NewInt(1000000))})
+	_, err := simapp.LiquidityKeeper.CreatePool(ctx, msg)
+	require.ErrorIs(t, err, types.ErrEqualDenom)
+
+	// Check overflow case on deposit
+	deposit := sdk.NewCoins(
+		sdk.NewCoin(denomA, sdk.NewInt(1_000_000)),
+		sdk.NewCoin(denomB, sdk.NewInt(2_000_000_000_000*1_000_000).MulRaw(1_000_000)))
+	hugeCoins := sdk.NewCoins(
+		sdk.NewCoin(denomA, sdk.NewInt(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000)),
+		sdk.NewCoin(denomB, sdk.NewInt(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000)))
+	app.SaveAccount(simapp, ctx, addrs[0], deposit.Add(hugeCoins...))
+
+	msg = types.NewMsgCreatePool(addrs[0], poolTypeID, deposit)
+	_, err = simapp.LiquidityKeeper.CreatePool(ctx, msg)
+	require.NoError(t, err)
+	pools := simapp.LiquidityKeeper.GetAllPools(ctx)
+	poolCoin := simapp.LiquidityKeeper.GetPoolCoinTotalSupply(ctx, pools[0])
+
+	depositorBalance := simapp.BankKeeper.GetAllBalances(ctx, addrs[0])
+	depositMsg := types.NewMsgDepositWithinBatch(addrs[0], pools[0].Id, hugeCoins)
+	_, err = simapp.LiquidityKeeper.DepositWithinBatch(ctx, depositMsg)
+	require.NoError(t, err)
+
+	poolBatch, found := simapp.LiquidityKeeper.GetPoolBatch(ctx, depositMsg.PoolId)
+	require.True(t, found)
+	msgs := simapp.LiquidityKeeper.GetAllPoolBatchDepositMsgs(ctx, poolBatch)
+	require.Equal(t, 1, len(msgs))
+	err = simapp.LiquidityKeeper.ExecuteDeposit(ctx, msgs[0], poolBatch)
+	require.ErrorIs(t, err, types.ErrOverflowAmount)
+	err = simapp.LiquidityKeeper.RefundDeposit(ctx, msgs[0], poolBatch)
+	require.NoError(t, err)
+
+	poolCoinAfter := simapp.LiquidityKeeper.GetPoolCoinTotalSupply(ctx, pools[0])
+	depositorPoolCoinBalance := simapp.BankKeeper.GetBalance(ctx, addrs[0], pools[0].PoolCoinDenom)
+	require.Equal(t, poolCoin, poolCoinAfter)
+	require.Equal(t, poolCoinAfter, depositorPoolCoinBalance.Amount)
+	require.Equal(t, depositorBalance.AmountOf(pools[0].PoolCoinDenom), depositorPoolCoinBalance.Amount)
+
+	hugeCoins = sdk.NewCoins(
+		sdk.NewCoin(denomA, sdk.NewInt(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000)),
+		sdk.NewCoin(denomB, sdk.NewInt(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000)))
+	depositMsg = types.NewMsgDepositWithinBatch(addrs[0], pools[0].Id, hugeCoins)
+	_, err = simapp.LiquidityKeeper.DepositWithinBatch(ctx, depositMsg)
+	require.NoError(t, err)
+	msgs = simapp.LiquidityKeeper.GetAllPoolBatchDepositMsgs(ctx, poolBatch)
+	require.Equal(t, 2, len(msgs))
+	err = simapp.LiquidityKeeper.ExecuteDeposit(ctx, msgs[1], poolBatch)
+	require.NoError(t, err)
+
+	// Check overflow case on withdraw
+	depositorPoolCoinBalance = simapp.BankKeeper.GetBalance(ctx, addrs[0], pools[0].PoolCoinDenom)
+	_, err = simapp.LiquidityKeeper.WithdrawWithinBatch(ctx, types.NewMsgWithdrawWithinBatch(addrs[0], pools[0].Id, depositorPoolCoinBalance.SubAmount(sdk.NewInt(1))))
+	require.NoError(t, err)
+
+	poolBatch, found = simapp.LiquidityKeeper.GetPoolBatch(ctx, depositMsg.PoolId)
+	require.True(t, found)
+	withdrawMsgs := simapp.LiquidityKeeper.GetAllPoolBatchWithdrawMsgStates(ctx, poolBatch)
+	require.Equal(t, 1, len(withdrawMsgs))
+	err = simapp.LiquidityKeeper.ExecuteWithdrawal(ctx, withdrawMsgs[0], poolBatch)
+	require.ErrorIs(t, err, types.ErrOverflowAmount)
+	err = simapp.LiquidityKeeper.RefundWithdrawal(ctx, withdrawMsgs[0], poolBatch)
+	require.NoError(t, err)
+
+	// Check overflow, division by zero case on swap
+	swapUserBalanceBefore := simapp.BankKeeper.GetAllBalances(ctx, addrs[0])
+	offerCoinA := sdk.NewCoin(denomA, sdk.NewInt(1_000_000_000_000_000_000).MulRaw(1_000_000_000))
+	orderPriceA := sdk.MustNewDecFromStr("110000000000000000000000000000000000000000000000000000000000.000000000000000001")
+	offerCoinB := sdk.NewCoin(denomB, sdk.NewInt(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000_000_000).MulRaw(1_000_000_000_000))
+	orderPriceB := sdk.MustNewDecFromStr("0.000000000000000001")
+	liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
+	_, err = simapp.LiquidityKeeper.SwapWithinBatch(
+		ctx,
+		types.NewMsgSwapWithinBatch(addrs[0], pools[0].Id, types.DefaultSwapTypeID, offerCoinA, denomB, orderPriceA, params.SwapFeeRate),
+		0)
+	require.ErrorIs(t, err, types.ErrOverflowAmount)
+	_, err = simapp.LiquidityKeeper.SwapWithinBatch(
+		ctx,
+		types.NewMsgSwapWithinBatch(addrs[0], pools[0].Id, types.DefaultSwapTypeID, offerCoinB, denomA, orderPriceB, params.SwapFeeRate),
+		0)
+	require.NoError(t, err)
+	liquidity.EndBlocker(ctx, simapp.LiquidityKeeper)
+	liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
+	swapUserBalanceAfter := simapp.BankKeeper.GetAllBalances(ctx, addrs[0])
+	require.Equal(t, swapUserBalanceBefore, swapUserBalanceAfter)
+	depositMsgs := simapp.LiquidityKeeper.GetAllPoolBatchDepositMsgs(ctx, poolBatch)
+	require.Equal(t, 0, len(depositMsgs))
+	withdrawMsgs = simapp.LiquidityKeeper.GetAllPoolBatchWithdrawMsgStates(ctx, poolBatch)
+	require.Equal(t, 0, len(withdrawMsgs))
+	swapMsgs := simapp.LiquidityKeeper.GetAllPoolBatchSwapMsgStates(ctx, poolBatch)
+	require.Equal(t, 0, len(swapMsgs))
+}

x/liquidity/keeper/swap.go

@@ -22,6 +22,10 @@ func (k Keeper) SwapExecution(ctx sdk.Context, poolBatch types.PoolBatch) (uint6
 		return 0, types.ErrPoolNotExists
 	}
 
+	if k.IsDepletedPool(ctx, pool) {
+		return 0, types.ErrDepletedPool
+	}
+
 	currentHeight := ctx.BlockHeight()
 	// set executed states of all messages to true
 	executedMsgCount := uint64(0)
@@ -32,7 +36,7 @@ func (k Keeper) SwapExecution(ctx sdk.Context, poolBatch types.PoolBatch) (uint6
 		if currentHeight > sms.OrderExpiryHeight {
 			sms.ToBeDeleted = true
 		}
-		if err := k.ValidateMsgSwapWithinBatch(ctx, *sms.Msg); err != nil {
+		if err := k.ValidateMsgSwapWithinBatch(ctx, *sms.Msg, pool); err != nil {
 			sms.ToBeDeleted = true
 		}
 		if !sms.ToBeDeleted {
@@ -79,7 +83,7 @@ func (k Keeper) SwapExecution(ctx sdk.Context, poolBatch types.PoolBatch) (uint6
 	// check orderbook validity and compute batchResult(direction, swapPrice, ..)
 	result, found := orderBook.Match(X, Y)
 
-	if !found {
+	if !found || X.Quo(Y).IsZero() {
 		err := k.RefundSwaps(ctx, pool, swapMsgStates)
 		return executedMsgCount, err
 	}

x/liquidity/types/errors.go

@@ -46,4 +46,5 @@ var (
 	ErrExceededReserveCoinLimit     = sdkerrors.Register(ModuleName, 38, "can not exceed reserve coin limit amount")
 	ErrDepletedPool                 = sdkerrors.Register(ModuleName, 39, "the pool is depleted of reserve coin, reinitializing is required by deposit")
 	ErrCircuitBreakerEnabled        = sdkerrors.Register(ModuleName, 40, "circuit breaker is triggered")
+	ErrOverflowAmount               = sdkerrors.Register(ModuleName, 41, "invalid amount that can cause overflow")
 )

x/liquidity/types/swap.go

@@ -166,6 +166,9 @@ func (orderBook OrderBook) Validate(currentPrice sdk.Dec) bool {
 			minSellOrderPrice = order.Price
 		}
 	}
+	if !currentPrice.IsPositive() {
+		return false
+	}
 	if maxBuyOrderPrice.GT(minSellOrderPrice) ||
 		maxBuyOrderPrice.Quo(currentPrice).GT(sdk.MustNewDecFromStr("1.10")) ||
 		minSellOrderPrice.Quo(currentPrice).LT(sdk.MustNewDecFromStr("0.90")) {
@@ -324,7 +327,6 @@ func (orderBook OrderBook) PriceDirection(currentPrice sdk.Dec) PriceDirection {
 			sellAmtUnderCurrentPrice = sellAmtUnderCurrentPrice.Add(order.SellOfferAmt.ToDec())
 		}
 	}
-
 	if buyAmtOverCurrentPrice.GT(currentPrice.Mul(sellAmtUnderCurrentPrice.Add(sellAmtAtCurrentPrice))) {
 		return Increasing
 	} else if currentPrice.Mul(sellAmtUnderCurrentPrice).GT(buyAmtOverCurrentPrice.Add(buyAmtAtCurrentPrice)) {

x/liquidity/types/utils.go

@@ -98,3 +98,26 @@ func MustParseCoinsNormalized(coinStr string) sdk.Coins {
 	}
 	return coins
 }
+
+func CheckOverflow(a, b sdk.Int) (err error) {
+	defer func() {
+		if r := recover(); r != nil {
+			err = ErrOverflowAmount
+		}
+	}()
+	a.Mul(b)
+	a.ToDec().Mul(b.ToDec())
+	return nil
+}
+
+func CheckOverflowWithDec(a, b sdk.Dec) (err error) {
+	defer func() {
+		if r := recover(); r != nil {
+			err = ErrOverflowAmount
+		}
+	}()
+	a.Mul(b)
+	a.Quo(b)
+	b.Quo(a)
+	return nil
+}