Disallow order removal reason fully filled #776

dydxprotocol · Nov 20, 2023 · 2ebe335 · 2ebe335
1 parent 8487af8
commit 2ebe335
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 19 deletions.
diff --git a/protocol/lib/metrics/constants.go b/protocol/lib/metrics/constants.go
@@ -180,7 +180,7 @@ const (
 	Status                                       = "status"
 	SubaccountPendingMatches                     = "subaccount_pending_matches"
 	TimeInForce                                  = "time_in_force"
-	TotalOrdersClobPair                          = "total_orders_in_clob"
+	TotalOrdersInClob                            = "total_orders_in_clob"
 	TotalQuoteQuantums                           = "total_quote_quantums"
 	Unfilled                                     = "unfilled"
 	UnfilledLiquidationOrders                    = "unfilled_liquidation_orders"

diff --git a/protocol/testutil/constants/clob_pair.go b/protocol/testutil/constants/clob_pair.go
@@ -91,7 +91,7 @@ var (
 		QuantumConversionExponent: -8,
 		Status:                    clobtypes.ClobPair_STATUS_ACTIVE,
 	}
-	ClobPair_Btc_Init = clobtypes.ClobPair{
+	ClobPair_Btc_Initializing = clobtypes.ClobPair{
 		Id: 0,
 		Metadata: &clobtypes.ClobPair_PerpetualClobMetadata{
 			PerpetualClobMetadata: &clobtypes.PerpetualClobMetadata{

diff --git a/protocol/x/clob/keeper/clob_pair_test.go b/protocol/x/clob/keeper/clob_pair_test.go
@@ -896,7 +896,7 @@ func TestIsPerpetualClobPairActive(t *testing.T) {
 			perpetualIdToClobPairId: map[uint32][]types.ClobPairId{
 				0: {types.ClobPairId(0)},
 			},
-			clobPair: &constants.ClobPair_Btc_Init,
+			clobPair: &constants.ClobPair_Btc_Initializing,
 			resp:     false,
 		},
 		"Succeeds when clob pair is active": {

diff --git a/protocol/x/clob/keeper/get_price_premium_test.go b/protocol/x/clob/keeper/get_price_premium_test.go
@@ -142,7 +142,7 @@ func TestGetPricePremiumForPerpetual(t *testing.T) {
 		"Success, premium is zeroed for initializing clob pair": {
 			perpetualId: 0,
 			args: testMemClobMethodArgs{
-				clobPair: constants.ClobPair_Btc_Init,
+				clobPair: constants.ClobPair_Btc_Initializing,
 			},
 			setUpMockMemClob: func(mck *mocks.MemClob, args testMemClobMethodArgs) {},
 		},

diff --git a/protocol/x/clob/keeper/process_operations.go b/protocol/x/clob/keeper/process_operations.go
@@ -252,6 +252,7 @@ func (k Keeper) PersistOrderRemovalToState(
 	ctx sdk.Context,
 	orderRemoval types.OrderRemoval,
 ) error {
+	lib.AssertDeliverTxMode(ctx)
 	orderIdToRemove := orderRemoval.GetOrderId()
 	orderIdToRemove.MustBeStatefulOrder()
 
@@ -356,16 +357,13 @@ func (k Keeper) PersistOrderRemovalToState(
 			)
 		}
 	case types.OrderRemoval_REMOVAL_REASON_FULLY_FILLED:
-		// The order should be fully filled.
-		remainingAmount, hasRemainingAmount := k.MemClob.GetOrderRemainingAmount(ctx, orderToRemove)
-		if hasRemainingAmount {
-			return errorsmod.Wrapf(
-				types.ErrOrderHasRemainingSize,
-				"Fill amount (%+v) and total size (%+v).",
-				remainingAmount,
-				orderToRemove.GetBaseQuantums(),
-			)
-		}
+		// Order removal reason fully filled is only used within indexer services.
+		// Fully filled orders are removed from the protocol after persisting the operations queue
+		// to state, instead of through the operations queue.
+		return errorsmod.Wrapf(
+			types.ErrInvalidOrderRemovalReason,
+			"Order removal reason fully filled should not be part of the operations queue.",
+		)
 	// TODO - uncomment when reduce only orders are enabled. Order Removals of this type will fail ValidateBasic.
 	// case types.OrderRemoval_REMOVAL_REASON_INVALID_REDUCE_ONLY:
 	// 	if !orderToRemove.IsReduceOnly() {

diff --git a/protocol/x/clob/keeper/process_operations_test.go b/protocol/x/clob/keeper/process_operations_test.go
@@ -1358,7 +1358,7 @@ func TestProcessProposerOperations(t *testing.T) {
 			},
 			perpetualFeeParams: &constants.PerpetualFeeParams,
 			clobPairs: []types.ClobPair{
-				constants.ClobPair_Btc_Init,
+				constants.ClobPair_Btc_Initializing,
 			},
 			rawOperations: []types.OperationRaw{
 				clobtest.NewMatchOperationRaw(
@@ -1379,7 +1379,7 @@ func TestProcessProposerOperations(t *testing.T) {
 			},
 			perpetualFeeParams: &constants.PerpetualFeeParams,
 			clobPairs: []types.ClobPair{
-				constants.ClobPair_Btc_Init,
+				constants.ClobPair_Btc_Initializing,
 			},
 			rawOperations: []types.OperationRaw{
 				clobtest.NewShortTermOrderPlacementOperationRaw(
@@ -1394,7 +1394,7 @@ func TestProcessProposerOperations(t *testing.T) {
 			},
 			perpetualFeeParams: &constants.PerpetualFeeParams,
 			clobPairs: []types.ClobPair{
-				constants.ClobPair_Btc_Init,
+				constants.ClobPair_Btc_Initializing,
 			},
 			preExistingStatefulOrders: []types.Order{
 				constants.LongTermOrder_Bob_Num0_Id0_Clob0_Buy25_Price30_GTBT10,
@@ -1407,6 +1407,25 @@ func TestProcessProposerOperations(t *testing.T) {
 			},
 			expectedError: types.ErrOperationConflictsWithClobPairStatus,
 		},
+		"Fails with order removal reason fully filled": {
+			perpetuals: []*perptypes.Perpetual{
+				&constants.BtcUsd_100PercentMarginRequirement,
+			},
+			perpetualFeeParams: &constants.PerpetualFeeParams,
+			clobPairs: []types.ClobPair{
+				constants.ClobPair_Btc,
+			},
+			preExistingStatefulOrders: []types.Order{
+				constants.LongTermOrder_Bob_Num0_Id0_Clob0_Buy25_Price30_GTBT10,
+			},
+			rawOperations: []types.OperationRaw{
+				clobtest.NewOrderRemovalOperationRaw(
+					constants.LongTermOrder_Bob_Num0_Id0_Clob0_Buy25_Price30_GTBT10.OrderId,
+					types.OrderRemoval_REMOVAL_REASON_FULLY_FILLED,
+				),
+			},
+			expectedError: types.ErrInvalidOrderRemoval,
+		},
 	}
 
 	for name, tc := range tests {

diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go
@@ -1828,7 +1828,7 @@ func (m *MemClobPriceTimePriority) SetMemclobGauges(
 		telemetry.SetGaugeWithLabels(
 			[]string{
 				types.ModuleName,
-				metrics.TotalOrdersClobPair,
+				metrics.TotalOrdersInClob,
 			},
 			float32(orderbook.TotalOpenOrders),
 			[]gometrics.Label{

diff --git a/protocol/x/clob/types/message_proposed_operations.go b/protocol/x/clob/types/message_proposed_operations.go
@@ -114,7 +114,10 @@ func ValidateAndTransformRawOperations(
 			if err := orderRemoval.OrderId.Validate(); err != nil {
 				return nil, err
 			}
-			if orderRemoval.RemovalReason == OrderRemoval_REMOVAL_REASON_UNSPECIFIED {
+			// Order removal reason fully filled is only used by indexer and should not be
+			// placed in the operations queue.
+			if orderRemoval.RemovalReason == OrderRemoval_REMOVAL_REASON_UNSPECIFIED ||
+				orderRemoval.RemovalReason == OrderRemoval_REMOVAL_REASON_FULLY_FILLED {
 				return nil, errorsmod.Wrapf(
 					ErrInvalidOrderRemoval,
 					"Invalid order removal reason: %+v",