Handle problems of sending coins to the reserve account

[hallazzang]

Summary of Bug

When someone sends coins to the reserve account of a pool, it can cause some problems including:

• A pool with zero amount balance in one side of the reserve coin, leading to incorrect pool price calculation
  • By withdrawing all coins from the pool and then sending only one reserve coin directly to the reserve account
• A panic in deposit logic, when calculating the reserve coin ratio(divide by zero error with a pool described above)
• ...

Version

https://github.com/tendermint/liquidity/releases/tag/v1.2.5

Steps to Reproduce

Running this test causes a panic:

func TestSwapHalfZeroReserve(t *testing.T) {
	simapp, ctx := createTestInput()
	params := simapp.LiquidityKeeper.GetParams(ctx)
	pool, addr, err := createPool(simapp, ctx, sdk.NewInt(1000000), sdk.NewInt(1000000), DenomX, DenomY)
	require.NoError(t, err)
	pc := simapp.BankKeeper.GetBalance(ctx, addr, pool.PoolCoinDenom)
	_, err = simapp.LiquidityKeeper.WithdrawLiquidityPoolToBatch(ctx, types.NewMsgWithdrawWithinBatch(addr, pool.Id, pc))
	require.NoError(t, err)
	liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
	liquidity.EndBlocker(ctx, simapp.LiquidityKeeper)
	require.True(t, simapp.BankKeeper.GetBalance(ctx, pool.GetReserveAccount(), DenomX).IsZero())
	require.True(t, simapp.BankKeeper.GetBalance(ctx, pool.GetReserveAccount(), DenomY).IsZero())
	err = simapp.BankKeeper.SendCoins(ctx, addr, pool.GetReserveAccount(), sdk.NewCoins(sdk.NewInt64Coin(DenomX, 10000)))
	require.NoError(t, err)
	_, err = simapp.LiquidityKeeper.SwapLiquidityPoolToBatch(ctx,
		types.NewMsgSwapWithinBatch(
			addr, pool.Id, types.DefaultSwapTypeId, sdk.NewInt64Coin(DenomX, 1000), DenomY, sdk.MustNewDecFromStr("1.0"), params.SwapFeeRate), 0)
	require.NoError(t, err)
	liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
	liquidity.EndBlocker(ctx, simapp.LiquidityKeeper)
}

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[hallazzang]

If we define a depleted pool as a pool with zero pool coin supply, not a pool with zero reserve, we can prevent chain to panic in the mentioned situations.

I'll work on it.

[migueldingli1997]

Something that can be considered is to make use of module accounts that are blacklisted (via the bank module) from receiving tokens, but this is a greater change compared to the above solution.

[jaybxyz]

@migueldingli1997 We thought about that approach at first, but since ReserveAcc is created dynamically when there is new pool, we think implementation requires a greater change to blockedAddrs. We're still exploring this approach to see if there is workaround.

[migueldingli1997]

Yep that's correct. A workaround to that then is to use just one module account (created at genesis and with the appropriate restrictions) but then you will have to keep track of the reserve balances for each pool in the liquidity module, rather than leaving it up to the bank module. I understand that this might not be desirable however.