Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: add overflow checking logic and test cases
Browse files Browse the repository at this point in the history
@dongsam
dongsam committed Oct 25, 2021

Verified

This commit was signed with the committer’s verified signature. The key has expired.
tvdeyen Thomas von Deyen
GPG key ID: F2548C2CBA04A848
Expired
Verified
Learn about vigilant mode
1 parent fc10fbf commit 59d7de7
Showing 5 changed files with 59 additions and 33 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -35,12 +35,14 @@ Ref: https://keepachangelog.com/en/1.0.0/

# Changelog

## v1.4.x - 2021.10.xx
## [Unreleased]

## [v1.4.1](https://github.com/tendermint/liquidity/releases) - 2021.10.25

* [\#455](https://github.com/tendermint/liquidity/pull/455) (sdk) Bump SDK version to [v0.44.2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.44.2)
* [\#446](https://github.com/tendermint/liquidity/pull/446) Fix: Pool Coin Decimal Truncation During Deposit
* [\#448](https://github.com/tendermint/liquidity/pull/448) Fix: add overflow checking and test codes for cover edge cases

## [Unreleased]

## [v1.4.0](https://github.com/tendermint/liquidity/releases/tag/v1.4.0) - 2021.09.07

21 changes: 15 additions & 6 deletions x/liquidity/keeper/liquidity_pool_test.go
View file
Original file line number Diff line number Diff line change
@@ -1141,7 +1141,10 @@ func TestOverflowAndZeroCases(t *testing.T) {
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...))
hugeCoins2 := sdk.NewCoins(
sdk.NewCoin(denomA, sdk.NewInt(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.Add(hugeCoins2...)...))

msg := types.NewMsgCreatePool(addrs[0], poolTypeID, deposit)
_, err := simapp.LiquidityKeeper.CreatePool(ctx, msg)
@@ -1151,33 +1154,39 @@ func TestOverflowAndZeroCases(t *testing.T) {

depositorBalance := simapp.BankKeeper.GetAllBalances(ctx, addrs[0])
depositMsg := types.NewMsgDepositWithinBatch(addrs[0], pools[0].Id, hugeCoins)
depositMsg2 := types.NewMsgDepositWithinBatch(addrs[0], pools[0].Id, hugeCoins2)
_, err = simapp.LiquidityKeeper.DepositWithinBatch(ctx, depositMsg)
_, err = simapp.LiquidityKeeper.DepositWithinBatch(ctx, depositMsg2)
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))
require.Equal(t, 2, 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)
err = simapp.LiquidityKeeper.ExecuteDeposit(ctx, msgs[1], poolBatch)
require.ErrorIs(t, err, types.ErrOverflowAmount)
err = simapp.LiquidityKeeper.RefundDeposit(ctx, msgs[1], 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(
hugeCoins3 := 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)
depositMsg = types.NewMsgDepositWithinBatch(addrs[0], pools[0].Id, hugeCoins3)
_, 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.Equal(t, 3, len(msgs))
err = simapp.LiquidityKeeper.ExecuteDeposit(ctx, msgs[2], poolBatch)
require.NoError(t, err)

// Check overflow case on withdraw
4 changes: 2 additions & 2 deletions x/liquidity/types/swap.go
View file
Original file line number Diff line number Diff line change
@@ -156,11 +156,11 @@ func (orderBook OrderBook) Match(x, y sdk.Dec) (BatchResult, bool) {

// Check orderbook validity naively
func (orderBook OrderBook) Validate(currentPrice sdk.Dec) bool {
maxBuyOrderPrice := sdk.ZeroDec()
minSellOrderPrice := sdk.NewDec(1000000000000)
if !currentPrice.IsPositive() {
return false
}
maxBuyOrderPrice := sdk.ZeroDec()
minSellOrderPrice := sdk.NewDec(1000000000000)
for _, order := range orderBook {
if order.BuyOfferAmt.IsPositive() && order.Price.GT(maxBuyOrderPrice) {
maxBuyOrderPrice = order.Price
59 changes: 36 additions & 23 deletions x/liquidity/types/swap_test.go
View file
Original file line number Diff line number Diff line change
@@ -550,44 +550,57 @@ func TestMakeOrderMapEdgeCase(t *testing.T) {
}

func TestOrderbookValidate(t *testing.T) {
currentPrice := sdk.MustNewDecFromStr("1.0")
for _, testCase := range []struct {
buyPrice string
sellPrice string
valid bool
currentPrice string
buyPrice string
sellPrice string
valid bool
}{
{
buyPrice: "0.99",
sellPrice: "1.01",
valid: true,
currentPrice: "1.0",
buyPrice: "0.99",
sellPrice: "1.01",
valid: true,
},
{
// maxBuyOrderPrice > minSellOrderPrice
buyPrice: "1.01",
sellPrice: "0.99",
valid: false,
currentPrice: "1.0",
buyPrice: "1.01",
sellPrice: "0.99",
valid: false,
},
{
buyPrice: "1.1",
sellPrice: "1.2",
valid: true,
currentPrice: "1.0",
buyPrice: "1.1",
sellPrice: "1.2",
valid: true,
},
{
// maxBuyOrderPrice/currentPrice > 1.10
buyPrice: "1.11",
sellPrice: "1.2",
valid: false,
currentPrice: "1.0",
buyPrice: "1.11",
sellPrice: "1.2",
valid: false,
},
{
buyPrice: "0.8",
sellPrice: "0.9",
valid: true,
currentPrice: "1.0",
buyPrice: "0.8",
sellPrice: "0.9",
valid: true,
},
{
// minSellOrderPrice/currentPrice < 0.90
buyPrice: "0.8",
sellPrice: "0.89",
valid: false,
currentPrice: "1.0",
buyPrice: "0.8",
sellPrice: "0.89",
valid: false,
},
{
// not positive price
currentPrice: "0.0",
buyPrice: "0.00000000001",
sellPrice: "0.000000000011",
valid: false,
},
} {
buyPrice := sdk.MustNewDecFromStr(testCase.buyPrice)
@@ -605,7 +618,7 @@ func TestOrderbookValidate(t *testing.T) {
},
}
orderBook := orderMap.SortOrderBook()
require.Equal(t, testCase.valid, orderBook.Validate(currentPrice))
require.Equal(t, testCase.valid, orderBook.Validate(sdk.MustNewDecFromStr(testCase.currentPrice)))
}
}

2 changes: 2 additions & 0 deletions x/liquidity/types/utils.go
View file
Original file line number Diff line number Diff line change
@@ -106,6 +106,8 @@ func CheckOverflow(a, b sdk.Int) (err error) {
}
}()
a.Mul(b)
a.Quo(b)
b.Quo(a)
return nil
}

0 comments on commit 59d7de7

Please sign in to comment.