Release gravity-devs/liquidity v1.4.1 (tendermint#5)

* feat: add markdown link checker github workflow	 9d729d0

* Merge pull request tendermint#453 from b-harvest/kogisin/451-bump-sdk-v0.44.1

feat: bump Cosmos SDK from v0.44.0 to v0.44.1
(cherry picked from commit 9e955b0)

* Merge pull request tendermint#455 from b-harvest/454-bump-sdk-v0.44.2

feat: bump cosmos-sdk to v0.44.2
(cherry picked from commit e118e21)

* fix: Pool Coin Decimal Truncation During Deposit (tendermint#446)

* fix: wip poc for reproduce and fix poolcoin truncation

* fix: simplify calculation logic and add more tests

* fix: use equality check in MintingPoolCoinsInvariant

* test: fix expected value

due to the change in deposit logic, expected value in test
also changed

* test: add test for MintingPoolCoinsInvariant

* fix: update deposit truncation logic and simulation ordering

* docs: update changelog and readme

* fix: revert MulTruncate to Mul on Deposit

Co-authored-by: Hanjun Kim <hallazzang@gmail.com>
(cherry picked from commit 616985f)

* Fix: add overflow checking and test codes for cover edge cases (tendermint#458)

* test: add testcase for cover small withdrawal case

* test: add test case for CreatePool

* fix: refactor and optimize depleted pool validation

* feat: add overflow checking logic

* chore: add testcase and remove comments

* test: add test code for big deposit

* fix: apply PR suggestions

* fix: add overflow checking logic and test cases

.github/workflows/linkchecker.yml

@@ -0,0 +1,19 @@
+name: Check Markdown links
+
+on:
+  pull_request:
+  push:
+    branches:
+      - master
+      - develop
+  schedule: 
+    - cron: '* */24 * * *'
+
+jobs:
+  markdown-link-check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@master
+      - uses: gaurav-nelson/github-action-markdown-link-check@1.0.13
+        with:
+          folder-path: "."

CHANGELOG.md

@@ -37,6 +37,13 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [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
+
+
 ## [v1.4.0](https://github.com/tendermint/liquidity/releases/tag/v1.4.0) - 2021.09.07
 
 * [\#440](https://github.com/tendermint/liquidity/pull/440) (sdk) Bump SDK version to [v0.44.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.44.0)

README.md

@@ -35,7 +35,7 @@ For details, see the [Liquidity Module Light Paper](doc/LiquidityModuleLightPape
 Requirement | Notes
 ----------- | -----------------
 Go version  | Go1.15 or higher
-Cosmos SDK  | v0.44.0 or higher
+Cosmos SDK  | v0.44.2 or higher
 
 ### Get Liquidity Module source code
 

app/app.go

@@ -381,8 +381,8 @@ func NewLiquidityApp(
 		slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper),
 		params.NewAppModule(app.ParamsKeeper),
 		evidence.NewAppModule(app.EvidenceKeeper),
-		authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry),
 		liquidity.NewAppModule(appCodec, app.LiquidityKeeper, app.AccountKeeper, app.BankKeeper, app.DistrKeeper),
+		authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry),
 	)
 
 	// register the store decoders for simulation tests

go.mod

@@ -3,7 +3,7 @@ go 1.15
 module github.com/gravity-devs/liquidity
 
 require (
-	github.com/cosmos/cosmos-sdk v0.44.0
+	github.com/cosmos/cosmos-sdk v0.44.2
 	github.com/gogo/protobuf v1.3.3
 	github.com/golang/mock v1.6.0
 	github.com/golang/protobuf v1.5.2
@@ -12,15 +12,15 @@ require (
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.0.1
 	github.com/rakyll/statik v0.1.7
 	github.com/spf13/cast v1.3.1
-	github.com/spf13/cobra v1.1.3
+	github.com/spf13/cobra v1.2.1
 	github.com/spf13/pflag v1.0.5
-	github.com/spf13/viper v1.8.0
+	github.com/spf13/viper v1.8.1
 	github.com/stretchr/testify v1.7.0
-	github.com/tendermint/tendermint v0.34.12
+	github.com/tendermint/tendermint v0.34.13
 	github.com/tendermint/tm-db v0.6.4
 	google.golang.org/genproto v0.0.0-20210602131652-f16073e35f0c
-	google.golang.org/grpc v1.38.0
-	google.golang.org/protobuf v1.26.0
+	google.golang.org/grpc v1.40.0
+	google.golang.org/protobuf v1.27.1
 	gopkg.in/yaml.v2 v2.4.0
 )
 

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/invariants.go

@@ -94,10 +94,15 @@ func MintingPoolCoinsInvariant(poolCoinTotalSupply, mintPoolCoin, depositCoinA,
 	expectedMintPoolCoinAmtBasedA := depositCoinARatio.MulInt(poolCoinTotalSupply).TruncateInt()
 	expectedMintPoolCoinAmtBasedB := depositCoinBRatio.MulInt(poolCoinTotalSupply).TruncateInt()
 
-	// NewPoolCoinAmount / LastPoolCoinSupply <= AfterRefundedDepositCoinA / LastReserveCoinA
-	// NewPoolCoinAmount / LastPoolCoinSupply <= AfterRefundedDepositCoinA / LastReserveCoinB
-	if depositCoinARatio.LT(poolCoinRatio) || depositCoinBRatio.LT(poolCoinRatio) {
-		panic("invariant check fails due to incorrect ratio of pool coins")
+	// NewPoolCoinAmount / LastPoolCoinSupply == AfterRefundedDepositCoinA / LastReserveCoinA
+	// NewPoolCoinAmount / LastPoolCoinSupply == AfterRefundedDepositCoinA / LastReserveCoinB
+	if depositCoinA.GTE(coinAmountThreshold) && depositCoinB.GTE(coinAmountThreshold) &&
+		lastReserveCoinA.GTE(coinAmountThreshold) && lastReserveCoinB.GTE(coinAmountThreshold) &&
+		mintPoolCoin.GTE(coinAmountThreshold) && poolCoinTotalSupply.GTE(coinAmountThreshold) {
+		if errorRate(depositCoinARatio, poolCoinRatio).GT(errorRateThreshold) ||
+			errorRate(depositCoinBRatio, poolCoinRatio).GT(errorRateThreshold) {
+			panic("invariant check fails due to incorrect ratio of pool coins")
+		}
 	}
 
 	if mintPoolCoin.GTE(coinAmountThreshold) &&

x/liquidity/keeper/invariants_test.go

@@ -21,6 +21,62 @@ func TestWithdrawRatioInvariant(t *testing.T) {
 	})
 }
 
+func TestMintingPoolCoinsInvariant(t *testing.T) {
+	for _, tc := range []struct {
+		poolCoinSupply  int64
+		mintingPoolCoin int64
+		reserveA        int64
+		depositA        int64
+		refundedA       int64
+		reserveB        int64
+		depositB        int64
+		refundedB       int64
+		expectPanic     bool
+	}{
+		{
+			10000, 1000,
+			100000, 10000, 0,
+			100000, 10000, 0,
+			false,
+		},
+		{
+			10000, 1000,
+			100000, 10000, 5000,
+			100000, 10000, 300,
+			true,
+		},
+		{
+			3000000, 100,
+			100000000, 4000, 667,
+			200000000, 8000, 1334,
+			false,
+		},
+		{
+			3000000, 100,
+			100000000, 4000, 0,
+			200000000, 8000, 1334,
+			true,
+		},
+	} {
+		f := require.NotPanics
+		if tc.expectPanic {
+			f = require.Panics
+		}
+		f(t, func() {
+			keeper.MintingPoolCoinsInvariant(
+				sdk.NewInt(tc.poolCoinSupply),
+				sdk.NewInt(tc.mintingPoolCoin),
+				sdk.NewInt(tc.depositA),
+				sdk.NewInt(tc.depositB),
+				sdk.NewInt(tc.reserveA),
+				sdk.NewInt(tc.reserveB),
+				sdk.NewInt(tc.refundedA),
+				sdk.NewInt(tc.refundedB),
+			)
+		})
+	}
+}
+
 func TestLiquidityPoolsEscrowAmountInvariant(t *testing.T) {
 	simapp, ctx := app.CreateTestInput()
 

x/liquidity/keeper/liquidity_pool.go

@@ -188,9 +188,6 @@ func (k Keeper) ExecuteDeposit(ctx sdk.Context, msg types.DepositMsgState, batch
 
 	params := k.GetParams(ctx)
 
-	var inputs []banktypes.Input
-	var outputs []banktypes.Output
-
 	reserveCoins := k.GetReserveCoins(ctx, pool)
 
 	// reinitialize pool if the pool is depleted
@@ -240,77 +237,59 @@ func (k Keeper) ExecuteDeposit(ctx sdk.Context, msg types.DepositMsgState, batch
 		return nil
 	}
 
-	// only two coins are acceptable
-	if reserveCoins.Len() != msg.Msg.DepositCoins.Len() {
-		return types.ErrNumOfReserveCoin
-	}
-
 	reserveCoins.Sort()
 
-	// Decimal Error, divide the Int coin amount by the Decimal Rate and erase the decimal point to deposit a lower value
-	lastReserveCoinA := reserveCoins[0].Amount
-	lastReserveCoinB := reserveCoins[1].Amount
-	lastReserveRatio := lastReserveCoinA.ToDec().QuoTruncate(lastReserveCoinB.ToDec())
+	lastReserveCoinA := reserveCoins[0]
+	lastReserveCoinB := reserveCoins[1]
 
 	depositCoinA := depositCoins[0]
 	depositCoinB := depositCoins[1]
-	depositCoinAmountA := depositCoinA.Amount
-	depositCoinAmountB := depositCoinB.Amount
-	depositableCoinAmountA := depositCoinB.Amount.ToDec().MulTruncate(lastReserveRatio).TruncateInt()
-
-	refundedCoins := sdk.NewCoins()
-	refundedCoinA := sdk.ZeroInt()
-	refundedCoinB := sdk.ZeroInt()
-
-	var acceptedCoins sdk.Coins
-	// handle when depositing coin A amount is less than, greater than or equal to depositable amount
-	if depositCoinA.Amount.LT(depositableCoinAmountA) {
-		depositCoinAmountB = depositCoinA.Amount.ToDec().QuoTruncate(lastReserveRatio).TruncateInt()
-		acceptedCoins = sdk.NewCoins(depositCoinA, sdk.NewCoin(depositCoinB.Denom, depositCoinAmountB))
-
-		inputs = append(inputs, banktypes.NewInput(batchEscrowAcc, acceptedCoins))
-		outputs = append(outputs, banktypes.NewOutput(reserveAcc, acceptedCoins))
-
-		refundedCoinB = depositCoinB.Amount.Sub(depositCoinAmountB)
-
-		if refundedCoinB.IsPositive() {
-			refundedCoins = sdk.NewCoins(sdk.NewCoin(depositCoinB.Denom, refundedCoinB))
-			inputs = append(inputs, banktypes.NewInput(batchEscrowAcc, refundedCoins))
-			outputs = append(outputs, banktypes.NewOutput(depositor, refundedCoins))
-		}
-	} else if depositCoinA.Amount.GT(depositableCoinAmountA) {
-		depositCoinAmountA = depositCoinB.Amount.ToDec().MulTruncate(lastReserveRatio).TruncateInt()
-		acceptedCoins = sdk.NewCoins(depositCoinB, sdk.NewCoin(depositCoinA.Denom, depositCoinAmountA))
 
-		inputs = append(inputs, banktypes.NewInput(batchEscrowAcc, acceptedCoins))
-		outputs = append(outputs, banktypes.NewOutput(reserveAcc, acceptedCoins))
-
-		refundedCoinA = depositCoinA.Amount.Sub(depositCoinAmountA)
-
-		if refundedCoinA.IsPositive() {
-			refundedCoins = sdk.NewCoins(sdk.NewCoin(depositCoinA.Denom, refundedCoinA))
-			inputs = append(inputs, banktypes.NewInput(batchEscrowAcc, refundedCoins))
-			outputs = append(outputs, banktypes.NewOutput(depositor, refundedCoins))
-		}
-	} else {
-		acceptedCoins = sdk.NewCoins(depositCoinA, depositCoinB)
-		inputs = append(inputs, banktypes.NewInput(batchEscrowAcc, acceptedCoins))
-		outputs = append(outputs, banktypes.NewOutput(reserveAcc, acceptedCoins))
+	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.MulTruncate(depositCoinA.Amount.ToDec()).QuoTruncate(lastReserveCoinA.Amount.ToDec()),
+		poolCoinTotalSupply.MulTruncate(depositCoinB.Amount.ToDec()).QuoTruncate(lastReserveCoinB.Amount.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()),
+	)
+	refundedCoins := depositCoins.Sub(acceptedCoins)
+	refundedCoinA := sdk.NewCoin(depositCoinA.Denom, refundedCoins.AmountOf(depositCoinA.Denom))
+	refundedCoinB := sdk.NewCoin(depositCoinB.Denom, refundedCoins.AmountOf(depositCoinB.Denom))
 
-	// calculate pool token mint amount
-	poolCoinTotalSupply := k.GetPoolCoinTotalSupply(ctx, pool)
-	poolCoinAmt := sdk.MinInt(
-		poolCoinTotalSupply.ToDec().MulTruncate(depositCoinAmountA.ToDec()).QuoTruncate(reserveCoins[0].Amount.ToDec()).TruncateInt(),
-		poolCoinTotalSupply.ToDec().MulTruncate(depositCoinAmountB.ToDec()).QuoTruncate(reserveCoins[1].Amount.ToDec()).TruncateInt())
-	mintPoolCoin := sdk.NewCoin(pool.PoolCoinDenom, poolCoinAmt)
+	mintPoolCoin := sdk.NewCoin(pool.PoolCoinDenom, poolCoinMintAmt.TruncateInt())
 	mintPoolCoins := sdk.NewCoins(mintPoolCoin)
 
-	// mint pool token to the depositor
+	if mintPoolCoins.IsZero() || acceptedCoins.IsZero() {
+		return fmt.Errorf("pool coin truncated, no accepted coin, refund")
+	}
+
 	if err := k.bankKeeper.MintCoins(ctx, types.ModuleName, mintPoolCoins); err != nil {
 		return err
 	}
 
+	var inputs []banktypes.Input
+	var outputs []banktypes.Output
+
+	if !refundedCoins.IsZero() {
+		// refund truncated deposit coins
+		inputs = append(inputs, banktypes.NewInput(batchEscrowAcc, refundedCoins))
+		outputs = append(outputs, banktypes.NewOutput(depositor, refundedCoins))
+	}
+
+	// send accepted deposit coins
+	inputs = append(inputs, banktypes.NewInput(batchEscrowAcc, acceptedCoins))
+	outputs = append(outputs, banktypes.NewOutput(reserveAcc, acceptedCoins))
+
+	// send minted pool coins
 	inputs = append(inputs, banktypes.NewInput(batchEscrowAcc, mintPoolCoins))
 	outputs = append(outputs, banktypes.NewOutput(depositor, mintPoolCoins))
 
@@ -328,10 +307,10 @@ 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,
-			lastReserveCoinA, lastReserveCoinB, refundedCoinA, refundedCoinB)
-		DepositInvariant(lastReserveCoinA, lastReserveCoinB, depositCoinA.Amount, depositCoinB.Amount,
-			afterReserveCoinA, afterReserveCoinB, refundedCoinA, refundedCoinB)
+		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)
 	}
 
 	ctx.EventManager().EmitEvent(
@@ -350,7 +329,7 @@ func (k Keeper) ExecuteDeposit(ctx sdk.Context, msg types.DepositMsgState, batch
 	)
 
 	reserveCoins = k.GetReserveCoins(ctx, pool)
-	lastReserveRatio = sdk.NewDecFromInt(reserveCoins[0].Amount).Quo(sdk.NewDecFromInt(reserveCoins[1].Amount))
+	lastReserveRatio := sdk.NewDecFromInt(reserveCoins[0].Amount).Quo(sdk.NewDecFromInt(reserveCoins[1].Amount))
 
 	logger := k.Logger(ctx)
 	logger.Debug(
@@ -405,6 +384,12 @@ 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
+			}
+			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)
@@ -797,21 +782,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
@@ -827,6 +803,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 err
+	}
+
 	if !msg.OfferCoinFee.Equal(types.GetOfferCoinFee(msg.OfferCoin, params.SwapFeeRate)) {
 		return types.ErrBadOfferCoinFee
 	}