Fix unit batch height issue (#346)

* WIP fix expiryHeight

* fix to deterministic batch height

* set ToBeDeleted for already expired order

* remove debug pprint, Set init batch BeginHeight to current height

* go mod tidy

* fix TestLiquidityUnitBatchHeight

* ignored messages to be deleted and add test

Co-authored-by: dongsam <dongsamb@gmail.com>

Makefile

@@ -157,7 +157,7 @@ benchmark:
 test-sim-nondeterminism:
 	@echo "Running non-determinism test..."
 	@go test -mod=readonly $(SIMAPP) -run TestAppStateDeterminism -Enabled=true \
-		-NumBlocks=100 -BlockSize=1000 -Commit=true -Period=0 -v -timeout 24h
+		-NumBlocks=100 -BlockSize=100 -Commit=true -Period=0 -v -timeout 24h
 
 test-sim-import-export: runsim
 	@echo "Running application import/export simulation. This may take several minutes..."

x/liquidity/keeper/batch.go

@@ -84,7 +84,7 @@ func (k Keeper) ExecutePoolBatch(ctx sdk.Context) {
 	k.IterateAllPoolBatches(ctx, func(poolBatch types.PoolBatch) bool {
 		params := k.GetParams(ctx)
 
-		if !poolBatch.Executed && ctx.BlockHeight()-poolBatch.BeginHeight+1 >= int64(params.UnitBatchHeight) {
+		if !poolBatch.Executed && ctx.BlockHeight()%int64(params.UnitBatchHeight) == 0 {
 			executedMsgCount, err := k.SwapExecution(ctx, poolBatch)
 			if err != nil {
 				panic(err)
@@ -227,7 +227,6 @@ func (k Keeper) SwapLiquidityPoolToBatch(ctx sdk.Context, msg *types.MsgSwapWith
 	if err := k.ValidateMsgSwapWithinBatch(ctx, *msg); err != nil {
 		return nil, err
 	}
-
 	poolBatch, found := k.GetPoolBatch(ctx, msg.PoolId)
 	if !found {
 		return nil, types.ErrPoolBatchNotExists
@@ -250,7 +249,6 @@ func (k Keeper) SwapLiquidityPoolToBatch(ctx sdk.Context, msg *types.MsgSwapWith
 	}
 
 	batchPoolMsg.OrderExpiryHeight = batchPoolMsg.MsgHeight + OrderExpirySpanHeight
-
 	if err := k.HoldEscrow(ctx, msg.GetSwapRequester(), sdk.NewCoins(msg.OfferCoin.Add(msg.OfferCoinFee))); err != nil {
 		return nil, err
 	}

x/liquidity/keeper/batch_test.go

@@ -928,6 +928,8 @@ func TestLiquidityScenario8(t *testing.T) {
 // Test UnitBatchHeight when over 1
 func TestLiquidityUnitBatchHeight(t *testing.T) {
 	simapp, ctx := createTestInput()
+	ctx = ctx.WithBlockHeight(1)
+
 	params := simapp.LiquidityKeeper.GetParams(ctx)
 	params.UnitBatchHeight = 2
 	simapp.LiquidityKeeper.SetParams(ctx, params)
@@ -949,6 +951,7 @@ func TestLiquidityUnitBatchHeight(t *testing.T) {
 	require.Equal(t, sdk.NewInt(1000000), poolCoinBalance.Amount)
 	app.TestWithdrawPool(t, simapp, ctx, poolCoins.QuoRaw(10), addrs[0:1], poolId, false)
 	liquidity.EndBlocker(ctx, simapp.LiquidityKeeper)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
 
 	// batch not executed, 1 >= 2(UnitBatchHeight)
 	batch, found := simapp.LiquidityKeeper.GetPoolBatch(ctx, pool.Id)
@@ -963,11 +966,11 @@ func TestLiquidityUnitBatchHeight(t *testing.T) {
 	require.Equal(t, sdk.NewInt(900000), poolCoinBalance.Amount)
 
 	// next block
-	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
 	liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
 	batchWithdrawMsgs = simapp.LiquidityKeeper.GetAllPoolBatchWithdrawMsgStates(ctx, batch)
 	require.Equal(t, 1, len(batchWithdrawMsgs))
 	liquidity.EndBlocker(ctx, simapp.LiquidityKeeper)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
 
 	// batch executed, 2 >= 2(UnitBatchHeight)
 	batch, found = simapp.LiquidityKeeper.GetPoolBatch(ctx, pool.Id)
@@ -982,7 +985,6 @@ func TestLiquidityUnitBatchHeight(t *testing.T) {
 	require.Equal(t, sdk.NewInt(900000), poolCoinBalance.Amount)
 
 	// next block
-	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
 	liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
 
 	// batch msg deleted after batch execution
@@ -1062,6 +1064,7 @@ func TestDeleteAndInitPoolBatchDeposit(t *testing.T) {
 	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
 	liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
 }
+
 func TestDeleteAndInitPoolBatchWithdraw(t *testing.T) {
 	simapp, ctx := createTestInput()
 	simapp.LiquidityKeeper.SetParams(ctx, types.DefaultParams())
@@ -1114,3 +1117,50 @@ func TestDeleteAndInitPoolBatchWithdraw(t *testing.T) {
 	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
 	liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
 }
+
+func TestUnitBatchHeight(t *testing.T) {
+	for _, unitBatchHeight := range []uint32{1, 2, 3, 5, 9} {
+		simapp, ctx := createTestInput()
+		ctx = ctx.WithBlockHeight(1)
+		params := simapp.LiquidityKeeper.GetParams(ctx)
+		params.UnitBatchHeight = unitBatchHeight
+		simapp.LiquidityKeeper.SetParams(ctx, params)
+
+		X, Y := sdk.NewInt(1000000), sdk.NewInt(1000000)
+		pool, err := createPool(simapp, ctx, X, Y, DenomX, DenomY)
+		require.NoError(t, err)
+
+		johnCoins := sdk.NewCoins(
+			sdk.NewInt64Coin(DenomX, 100000000), sdk.NewInt64Coin(DenomY, 100000000))
+		johnAddr := app.AddRandomTestAddr(simapp, ctx, johnCoins)
+
+		for ; ctx.BlockHeight() <= 100; ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) {
+			liquidity.BeginBlocker(ctx, simapp.LiquidityKeeper)
+
+			batch, ok := simapp.LiquidityKeeper.GetPoolBatch(ctx, pool.Id)
+			require.True(t, ok)
+			require.False(t, batch.Executed, "batch should not be executed")
+			require.Equal(
+				t, (ctx.BlockHeight()-1)/int64(unitBatchHeight)*int64(unitBatchHeight)+1, batch.BeginHeight)
+
+			_, err = simapp.LiquidityKeeper.SwapLiquidityPoolToBatch(
+				ctx, types.NewMsgSwapWithinBatch(
+					johnAddr, pool.Id, types.DefaultSwapTypeId,
+					sdk.NewInt64Coin(DenomX, 1000), DenomY, sdk.MustNewDecFromStr("1.1"),
+					params.SwapFeeRate), 0)
+			require.NoError(t, err)
+
+			liquidity.EndBlocker(ctx, simapp.LiquidityKeeper)
+
+			batch, ok = simapp.LiquidityKeeper.GetPoolBatch(ctx, pool.Id)
+			require.True(t, ok)
+			if ctx.BlockHeight()%int64(unitBatchHeight) == 0 {
+				require.True(t, batch.Executed, "batch should be executed")
+			} else {
+				require.False(t, batch.Executed, "batch should not be executed")
+			}
+			require.Equal(
+				t, (ctx.BlockHeight()-1)/int64(unitBatchHeight)*int64(unitBatchHeight)+1, batch.BeginHeight)
+		}
+	}
+}

x/liquidity/keeper/liquidity_pool.go

@@ -132,6 +132,7 @@ func (k Keeper) CreatePool(ctx sdk.Context, msg *types.MsgCreatePool) (types.Poo
 
 	pool = k.SetPoolAtomic(ctx, pool)
 	batch := types.NewPoolBatch(pool.Id, 1)
+	batch.BeginHeight = ctx.BlockHeight()
 
 	k.SetPoolBatch(ctx, batch)
 

x/liquidity/keeper/swap.go

@@ -21,13 +21,21 @@ func (k Keeper) SwapExecution(ctx sdk.Context, liquidityPoolBatch types.PoolBatc
 		return 0, types.ErrPoolNotExists
 	}
 
+	currentHeight := ctx.BlockHeight()
 	// set executed states of all messages to true
+	var swapMsgStatesNotToBeDeleted []*types.SwapMsgState
 	for _, sms := range swapMsgStates {
 		sms.Executed = true
+		if currentHeight > sms.OrderExpiryHeight {
+			sms.ToBeDeleted = true
+		}
+		if !sms.ToBeDeleted {
+			swapMsgStatesNotToBeDeleted = append(swapMsgStatesNotToBeDeleted, sms)
+		}
 	}
 	k.SetPoolBatchSwapMsgStatesByPointer(ctx, pool.Id, swapMsgStates)
+	swapMsgStates = swapMsgStatesNotToBeDeleted
 
-	currentHeight := ctx.BlockHeight()
 	types.ValidateStateAndExpireOrders(swapMsgStates, currentHeight, false)
 
 	// get reserve coins from the liquidity pool and calculate the current pool price (p = x / y)