Statefully validate stateful orders in PrepareCheckState Replay (dydx…

…protocol#797) * statefully validate stateful orders when replaying * AddPreExistingStatefulOrder does not need block height * fake memclob keeper * fix order removal test
kruspy · Nov 22, 2023 · 2c64cf6 · 2c64cf6
1 parent 68119d8
commit 2c64cf6
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 25 deletions.
diff --git a/protocol/mocks/MemClobKeeper.go b/protocol/mocks/MemClobKeeper.go
diff --git a/protocol/testutil/memclob/keeper.go b/protocol/testutil/memclob/keeper.go
@@ -470,6 +470,19 @@ func (f *FakeMemClobKeeper) OffsetSubaccountPerpetualPosition(
 	panic("This function should not be implemented as FakeMemClobKeeper is getting deprecated (CLOB-175)")
 }
 
+func (f *FakeMemClobKeeper) AddPreexistingStatefulOrder(
+	ctx sdk.Context,
+	order *types.Order,
+	memclob types.MemClob,
+) (
+	orderSizeOptimisticallyFilledFromMatchingQuantums satypes.BaseQuantums,
+	orderStatus types.OrderStatus,
+	offchainUpdates *types.OffchainUpdates,
+	err error,
+) {
+	panic("This function should not be implemented as FakeMemClobKeeper is getting deprecated (CLOB-175)")
+}
+
 func (f *FakeMemClobKeeper) IsLiquidatable(
 	ctx sdk.Context,
 	subaccountId satypes.SubaccountId,

diff --git a/protocol/x/clob/e2e/order_removal_test.go b/protocol/x/clob/e2e/order_removal_test.go
@@ -606,23 +606,6 @@ func TestOrderRemoval_Invalid(t *testing.T) {
 			},
 			expectedErr: "Immediate-or-cancel order is fully filled",
 		},
-		"invalid proposal: non fully-filled long-term order cannot be removed with fully filled removal reason": {
-			subaccounts: []satypes.Subaccount{
-				constants.Alice_Num0_100_000USD,
-			},
-			orders: []clobtypes.Order{
-				constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy100_Price10_GTBT15,
-			},
-			msgProposedOperations: &clobtypes.MsgProposedOperations{
-				OperationsQueue: []clobtypes.OperationRaw{
-					clobtestutils.NewOrderRemovalOperationRaw(
-						constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy100_Price10_GTBT15.OrderId,
-						clobtypes.OrderRemoval_REMOVAL_REASON_FULLY_FILLED,
-					),
-				},
-			},
-			expectedErr: "Order has remaining size",
-		},
 		"invalid proposal: post-only removal reason used for non post-only order": {
 			subaccounts: []satypes.Subaccount{
 				constants.Alice_Num0_100_000USD,

diff --git a/protocol/x/clob/keeper/orders.go b/protocol/x/clob/keeper/orders.go
@@ -389,7 +389,6 @@ func (k Keeper) ReplayPlaceOrder(
 func (k Keeper) AddPreexistingStatefulOrder(
 	ctx sdk.Context,
 	order *types.Order,
-	blockHeight uint32,
 	memclob types.MemClob,
 ) (
 	orderSizeOptimisticallyFilledFromMatchingQuantums satypes.BaseQuantums,
@@ -399,7 +398,8 @@ func (k Keeper) AddPreexistingStatefulOrder(
 ) {
 	order.MustBeStatefulOrder()
 	// Perform stateful validation without checking existing order in state.
-	err = k.PerformStatefulOrderValidation(ctx, order, blockHeight, true)
+	// Block height is not used when validating stateful orders, so always pass in zero.
+	err = k.PerformStatefulOrderValidation(ctx, order, 0, true)
 	if err != nil {
 		return 0, 0, nil, err
 	}
@@ -456,7 +456,6 @@ func (k Keeper) PlaceStatefulOrdersFromLastBlock(
 		_, orderStatus, placeOrderOffchainUpdates, err := k.AddPreexistingStatefulOrder(
 			ctx,
 			&order,
-			0,
 			k.MemClob,
 		)
 

diff --git a/protocol/x/clob/keeper/orders_test.go b/protocol/x/clob/keeper/orders_test.go
@@ -937,7 +937,6 @@ func TestAddPreexistingStatefulOrder(t *testing.T) {
 				_, orderStatus, _, err := ks.ClobKeeper.AddPreexistingStatefulOrder(
 					ctx.WithIsCheckTx(true),
 					&order,
-					blockHeight,
 					memClob,
 				)
 				require.NoError(t, err)
@@ -966,7 +965,7 @@ func TestAddPreexistingStatefulOrder(t *testing.T) {
 			orderSizeOptimisticallyFilledFromMatching,
 				orderStatus,
 				_,
-				err := ks.ClobKeeper.AddPreexistingStatefulOrder(ctx.WithIsCheckTx(true), &tc.order, blockHeight, memClob)
+				err := ks.ClobKeeper.AddPreexistingStatefulOrder(ctx.WithIsCheckTx(true), &tc.order, memClob)
 
 			// Verify test expectations.
 			require.ErrorIs(t, err, tc.expectedErr)

diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go
@@ -969,11 +969,11 @@ func (m *MemClobPriceTimePriority) ReplayOperations(
 				continue
 			}
 
-			// Note that we use `memclob.PlaceOrder` here, this will skip writing the stateful order placement to state.
 			// TODO(DEC-998): Research whether it's fine for two post-only orders to be matched. Currently they are dropped.
-			_, orderStatus, placeOrderOffchainUpdates, err := m.PlaceOrder(
+			_, orderStatus, placeOrderOffchainUpdates, err := m.clobKeeper.AddPreexistingStatefulOrder(
 				ctx,
-				statefulOrderPlacement.Order,
+				&statefulOrderPlacement.Order,
+				m,
 			)
 			placedPreexistingStatefulOrderIds[*orderId] = struct{}{}
 			existingOffchainUpdates = m.GenerateOffchainUpdatesForReplayPlaceOrder(

diff --git a/protocol/x/clob/types/mem_clob_keeper.go b/protocol/x/clob/types/mem_clob_keeper.go
@@ -57,6 +57,16 @@ type MemClobKeeper interface {
 		offchainUpdates *OffchainUpdates,
 		err error,
 	)
+	AddPreexistingStatefulOrder(
+		ctx sdk.Context,
+		order *Order,
+		memclob MemClob,
+	) (
+		orderSizeOptimisticallyFilledFromMatchingQuantums satypes.BaseQuantums,
+		orderStatus OrderStatus,
+		offchainUpdates *OffchainUpdates,
+		err error,
+	)
 	CancelShortTermOrder(
 		ctx sdk.Context,
 		msgCancelOrder *MsgCancelOrder,