Skip to content

Commit

Permalink
fix: apply PR suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
dongsam committed Oct 25, 2021
1 parent 75aba90 commit fc10fbf
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 25 deletions.
20 changes: 13 additions & 7 deletions x/liquidity/keeper/liquidity_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,15 +245,18 @@ func (k Keeper) ExecuteDeposit(ctx sdk.Context, msg types.DepositMsgState, batch
depositCoinA := depositCoins[0]
depositCoinB := depositCoins[1]

poolCoinTotalSupply := k.GetPoolCoinTotalSupply(ctx, pool)
if err := types.CheckOverflow(poolCoinTotalSupply, depositCoinA.Amount); err != nil {
poolCoinTotalSupply := k.GetPoolCoinTotalSupply(ctx, pool).ToDec()
if err := types.CheckOverflowWithDec(poolCoinTotalSupply, depositCoinA.Amount.ToDec()); err != nil {
return err
}
if err := types.CheckOverflowWithDec(poolCoinTotalSupply, depositCoinB.Amount.ToDec()); 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()),
poolCoinTotalSupply.MulTruncate(depositCoinA.Amount.ToDec()).QuoTruncate(lastReserveCoinA.Amount.ToDec()),
poolCoinTotalSupply.MulTruncate(depositCoinB.Amount.ToDec()).QuoTruncate(lastReserveCoinB.Amount.ToDec()),
)
mintRate := poolCoinMintAmt.TruncateDec().QuoTruncate(poolCoinTotalSupply.ToDec())
mintRate := poolCoinMintAmt.TruncateDec().QuoTruncate(poolCoinTotalSupply)
acceptedCoins := sdk.NewCoins(
sdk.NewCoin(depositCoins[0].Denom, lastReserveCoinA.Amount.ToDec().Mul(mintRate).TruncateInt()),
sdk.NewCoin(depositCoins[1].Denom, lastReserveCoinB.Amount.ToDec().Mul(mintRate).TruncateInt()),
Expand Down Expand Up @@ -304,7 +307,7 @@ func (k Keeper) ExecuteDeposit(ctx sdk.Context, msg types.DepositMsgState, batch
afterReserveCoinA := afterReserveCoins[0].Amount
afterReserveCoinB := afterReserveCoins[1].Amount

MintingPoolCoinsInvariant(poolCoinTotalSupply, mintPoolCoin.Amount, depositCoinA.Amount, depositCoinB.Amount,
MintingPoolCoinsInvariant(poolCoinTotalSupply.TruncateInt(), mintPoolCoin.Amount, depositCoinA.Amount, depositCoinB.Amount,
lastReserveCoinA.Amount, lastReserveCoinB.Amount, refundedCoinA.Amount, refundedCoinB.Amount)
DepositInvariant(lastReserveCoinA.Amount, lastReserveCoinB.Amount, depositCoinA.Amount, depositCoinB.Amount,
afterReserveCoinA, afterReserveCoinB, refundedCoinA.Amount, refundedCoinB.Amount)
Expand Down Expand Up @@ -384,6 +387,9 @@ func (k Keeper) ExecuteWithdrawal(ctx sdk.Context, msg types.WithdrawMsgState, b
if err := types.CheckOverflow(reserveCoin.Amount, msg.Msg.PoolCoin.Amount); err != nil {
return err
}
if err := types.CheckOverflow(reserveCoin.Amount.Mul(msg.Msg.PoolCoin.Amount).ToDec().TruncateInt(), poolCoinTotalSupply); 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)
Expand Down Expand Up @@ -798,7 +804,7 @@ func (k Keeper) ValidateMsgSwapWithinBatch(ctx sdk.Context, msg types.MsgSwapWit
}

if err := types.CheckOverflowWithDec(msg.OfferCoin.Amount.ToDec(), msg.OrderPrice); err != nil {
return types.ErrOverflowAmount
return err
}

if !msg.OfferCoinFee.Equal(types.GetOfferCoinFee(msg.OfferCoin, params.SwapFeeRate)) {
Expand Down
55 changes: 41 additions & 14 deletions x/liquidity/keeper/liquidity_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,21 @@ func TestDepositWithCoinsSent(t *testing.T) {
require.True(sdk.IntEq(t, sdk.NewInt(1000000), balances.AmountOf(pool.PoolCoinDenom)))
}

func TestPoolEdgeCases(t *testing.T) {
func TestCreatePoolEqualDenom(t *testing.T) {
simapp, ctx := createTestInput()
params := types.DefaultParams()
simapp.LiquidityKeeper.SetParams(ctx, params)
addrs := app.AddTestAddrs(simapp, ctx, 1, params.PoolCreationFee)

msg := types.NewMsgCreatePool(addrs[0], types.DefaultPoolTypeID,
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)
}

func TestOverflowAndZeroCases(t *testing.T) {
simapp, ctx := createTestInput()
params := types.DefaultParams()
simapp.LiquidityKeeper.SetParams(ctx, params)
Expand All @@ -1120,14 +1134,6 @@ func TestPoolEdgeCases(t *testing.T) {
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)),
Expand All @@ -1137,8 +1143,8 @@ func TestPoolEdgeCases(t *testing.T) {
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)
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])
Expand Down Expand Up @@ -1221,6 +1227,7 @@ func TestExecuteBigDeposit(t *testing.T) {
simapp, ctx := createTestInput()
simapp.LiquidityKeeper.SetParams(ctx, types.DefaultParams())
params := simapp.LiquidityKeeper.GetParams(ctx)
keeper.BatchLogicInvariantCheckFlag = false

poolTypeID := types.DefaultPoolTypeID
addrs := app.AddTestAddrs(simapp, ctx, 3, params.PoolCreationFee)
Expand All @@ -1229,7 +1236,9 @@ func TestExecuteBigDeposit(t *testing.T) {
denomB := "uUSD"
denomA, denomB = types.AlphabeticalDenomPair(denomA, denomB)

initDeposit := sdk.NewCoins(sdk.NewCoin(denomA, sdk.NewInt(9223372036854775807)), sdk.NewCoin(denomB, sdk.NewInt(9223372036854775807)))
// 2^63-1
hugeInt := int64(9223372036854775807)
initDeposit := sdk.NewCoins(sdk.NewCoin(denomA, sdk.NewInt(hugeInt)), sdk.NewCoin(denomB, sdk.NewInt(hugeInt)))
app.SaveAccount(simapp, ctx, addrs[0], initDeposit)
app.SaveAccount(simapp, ctx, addrs[1], initDeposit)
app.SaveAccount(simapp, ctx, addrs[2], initDeposit)
Expand Down Expand Up @@ -1282,6 +1291,24 @@ func TestExecuteBigDeposit(t *testing.T) {

require.True(t, simapp.BankKeeper.GetBalance(ctx, addrs[1], denomA).IsZero())
require.True(t, simapp.BankKeeper.GetBalance(ctx, addrs[1], denomB).IsZero())
require.False(t, simapp.BankKeeper.GetBalance(ctx, addrs[2], denomA).IsZero())
require.False(t, simapp.BankKeeper.GetBalance(ctx, addrs[2], denomB).IsZero())

// Error due to decimal operation exceeding precision
require.Equal(t, sdk.NewInt(8), simapp.BankKeeper.GetBalance(ctx, addrs[2], denomA).Amount)
require.Equal(t, sdk.NewInt(8), simapp.BankKeeper.GetBalance(ctx, addrs[2], denomB).Amount)

poolCoinAmt := simapp.BankKeeper.GetBalance(ctx, addrs[1], pool.PoolCoinDenom)
state, err := simapp.LiquidityKeeper.WithdrawWithinBatch(ctx, types.NewMsgWithdrawWithinBatch(addrs[1], pool.Id, poolCoinAmt))
require.NoError(t, err)

err = simapp.LiquidityKeeper.ExecuteWithdrawal(ctx, state, poolBatch)
require.NoError(t, err)

balanceAfter := simapp.BankKeeper.GetAllBalances(ctx, addrs[1])
liquidity.EndBlocker(ctx, simapp.LiquidityKeeper)
liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)

// Error due to decimal operation exceeding precision
require.Equal(t, sdk.ZeroInt(), balanceAfter.AmountOf(pool.PoolCoinDenom))
require.Equal(t, sdk.NewInt(-4), balanceAfter.AmountOf(denomA).SubRaw(hugeInt))
require.Equal(t, sdk.NewInt(-4), balanceAfter.AmountOf(denomB).SubRaw(hugeInt))
}
6 changes: 3 additions & 3 deletions x/liquidity/types/swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ func (orderBook OrderBook) Match(x, y sdk.Dec) (BatchResult, bool) {
func (orderBook OrderBook) Validate(currentPrice sdk.Dec) bool {
maxBuyOrderPrice := sdk.ZeroDec()
minSellOrderPrice := sdk.NewDec(1000000000000)
if !currentPrice.IsPositive() {
return false
}
for _, order := range orderBook {
if order.BuyOfferAmt.IsPositive() && order.Price.GT(maxBuyOrderPrice) {
maxBuyOrderPrice = order.Price
Expand All @@ -166,9 +169,6 @@ 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")) {
Expand Down
1 change: 0 additions & 1 deletion x/liquidity/types/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ func CheckOverflow(a, b sdk.Int) (err error) {
}
}()
a.Mul(b)
a.ToDec().Mul(b.ToDec())
return nil
}

Expand Down

0 comments on commit fc10fbf

Please sign in to comment.