[CT-776] Fix consensus failure from unhandled error. (#1418)

indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/order_removals.ts

@@ -59,6 +59,12 @@ export enum OrderRemoval_RemovalReason {
    * was fully filled and should therefore be removed from state.
    */
   REMOVAL_REASON_FULLY_FILLED = 7,
+
+  /**
+   * REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS - REMOVAL_REASON_FULLY_FILLED represents a removal of an order that
+   *  would lead to the subaccount violating isolated subaccount constraints.
+   */
+  REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS = 8,
   UNRECOGNIZED = -1,
 }
 export enum OrderRemoval_RemovalReasonSDKType {
@@ -119,6 +125,12 @@ export enum OrderRemoval_RemovalReasonSDKType {
    * was fully filled and should therefore be removed from state.
    */
   REMOVAL_REASON_FULLY_FILLED = 7,
+
+  /**
+   * REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS - REMOVAL_REASON_FULLY_FILLED represents a removal of an order that
+   *  would lead to the subaccount violating isolated subaccount constraints.
+   */
+  REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS = 8,
   UNRECOGNIZED = -1,
 }
 export function orderRemoval_RemovalReasonFromJSON(object: any): OrderRemoval_RemovalReason {
@@ -155,6 +167,10 @@ export function orderRemoval_RemovalReasonFromJSON(object: any): OrderRemoval_Re
     case "REMOVAL_REASON_FULLY_FILLED":
       return OrderRemoval_RemovalReason.REMOVAL_REASON_FULLY_FILLED;
 
+    case 8:
+    case "REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS":
+      return OrderRemoval_RemovalReason.REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS;
+
     case -1:
     case "UNRECOGNIZED":
     default:
@@ -187,6 +203,9 @@ export function orderRemoval_RemovalReasonToJSON(object: OrderRemoval_RemovalRea
     case OrderRemoval_RemovalReason.REMOVAL_REASON_FULLY_FILLED:
       return "REMOVAL_REASON_FULLY_FILLED";
 
+    case OrderRemoval_RemovalReason.REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS:
+      return "REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS";
+
     case OrderRemoval_RemovalReason.UNRECOGNIZED:
     default:
       return "UNRECOGNIZED";

proto/dydxprotocol/clob/order_removals.proto

@@ -45,6 +45,9 @@ message OrderRemoval {
     // REMOVAL_REASON_FULLY_FILLED represents a removal of an order that
     // was fully filled and should therefore be removed from state.
     REMOVAL_REASON_FULLY_FILLED = 7;
+    // REMOVAL_REASON_FULLY_FILLED represents a removal of an order that
+    //  would lead to the subaccount violating isolated subaccount constraints.
+    REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS = 8;
   }
 
   RemovalReason removal_reason = 2;

protocol/indexer/shared/order_removal_reason.go

@@ -28,6 +28,8 @@ func ConvertOrderRemovalReasonToIndexerOrderRemovalReason(
 		reason = sharedtypes.OrderRemovalReason_ORDER_REMOVAL_REASON_FOK_ORDER_COULD_NOT_BE_FULLY_FULLED
 	case clobtypes.OrderRemoval_REMOVAL_REASON_CONDITIONAL_IOC_WOULD_REST_ON_BOOK:
 		reason = sharedtypes.OrderRemovalReason_ORDER_REMOVAL_REASON_IMMEDIATE_OR_CANCEL_WOULD_REST_ON_BOOK
+	case clobtypes.OrderRemoval_REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS:
+		reason = sharedtypes.OrderRemovalReason_ORDER_REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS
 	default:
 		panic("ConvertOrderRemovalReasonToIndexerOrderRemovalReason: unspecified removal reason not allowed")
 	}

protocol/testutil/constants/stateful_orders.go

@@ -943,6 +943,21 @@ var (
 		ConditionalOrderTriggerSubticks: 49_999_000_000,
 		TimeInForce:                     clobtypes.Order_TIME_IN_FORCE_IOC,
 	}
+	ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999_FOK = clobtypes.Order{
+		OrderId: clobtypes.OrderId{
+			SubaccountId: Alice_Num0,
+			ClientId:     0,
+			OrderFlags:   clobtypes.OrderIdFlags_Conditional,
+			ClobPairId:   0,
+		},
+		Side:                            clobtypes.Order_SIDE_BUY,
+		Quantums:                        100_000_000,
+		Subticks:                        50_000_000_000,
+		GoodTilOneof:                    &clobtypes.Order_GoodTilBlockTime{GoodTilBlockTime: 10},
+		ConditionType:                   clobtypes.Order_CONDITION_TYPE_TAKE_PROFIT,
+		ConditionalOrderTriggerSubticks: 49_999_000_000,
+		TimeInForce:                     clobtypes.Order_TIME_IN_FORCE_FILL_OR_KILL,
+	}
 	ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_SL_50001 = clobtypes.Order{
 		OrderId: clobtypes.OrderId{
 			SubaccountId: Alice_Num0,

protocol/x/clob/e2e/conditional_orders_test.go

@@ -851,6 +851,64 @@ func TestConditionalOrder(t *testing.T) {
 				},
 			},
 		},
+		`Conditional order that would violate isolated subaccount constraints can be placed, trigger, 
+		fail isolated subaccount checks and get removed from state`: {
+			subaccounts: []satypes.Subaccount{
+				constants.Alice_Num0_1ISO_LONG_10_000USD,
+				constants.Dave_Num0_10000USD,
+			},
+			orders: []clobtypes.Order{
+				constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999,
+				constants.Order_Dave_Num0_Id1_Clob0_Sell025BTC_Price50000_GTB11,
+			},
+			priceUpdateForFirstBlock: &prices.MsgUpdateMarketPrices{},
+			priceUpdateForSecondBlock: &prices.MsgUpdateMarketPrices{
+				MarketPriceUpdates: []*prices.MsgUpdateMarketPrices_MarketPrice{
+					prices.NewMarketPriceUpdate(0, 4_999_700_000),
+				},
+			},
+
+			expectedExistInState: map[clobtypes.OrderId]bool{
+				constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999.OrderId: false,
+			},
+			expectedInTriggeredStateAfterBlock: map[uint32]map[clobtypes.OrderId]bool{
+				2: {constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999.OrderId: false},
+				3: {constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999.OrderId: true},
+				4: {constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999.OrderId: false},
+			},
+			expectedSubaccounts: []satypes.Subaccount{
+				constants.Alice_Num0_1ISO_LONG_10_000USD,
+			},
+		},
+		`Conditional FOK order that would violate isolated subaccount constraints can be placed, trigger, 
+		fail isolated subaccount checks and get removed from state`: {
+			subaccounts: []satypes.Subaccount{
+				constants.Alice_Num0_1ISO_LONG_10_000USD,
+				constants.Dave_Num0_10000USD,
+			},
+			orders: []clobtypes.Order{
+				constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999_FOK,
+				constants.Order_Dave_Num0_Id1_Clob0_Sell025BTC_Price50000_GTB11,
+			},
+			priceUpdateForFirstBlock: &prices.MsgUpdateMarketPrices{},
+			priceUpdateForSecondBlock: &prices.MsgUpdateMarketPrices{
+				MarketPriceUpdates: []*prices.MsgUpdateMarketPrices_MarketPrice{
+					prices.NewMarketPriceUpdate(0, 4_999_700_000),
+				},
+			},
+
+			expectedExistInState: map[clobtypes.OrderId]bool{
+				constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999_FOK.OrderId: false,
+			},
+			expectedInTriggeredStateAfterBlock: map[uint32]map[clobtypes.OrderId]bool{
+				2: {constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999_FOK.OrderId: false},
+				3: {constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999_FOK.OrderId: true},
+				4: {constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_TP_49999_FOK.OrderId: false},
+			},
+			expectedSubaccounts: []satypes.Subaccount{
+				constants.Alice_Num0_1ISO_LONG_10_000USD,
+			},
+		},
 	}
 
 	for name, tc := range tests {
@@ -876,6 +934,7 @@ func TestConditionalOrder(t *testing.T) {
 						genesisState.LiquidityTiers = constants.LiquidityTiers
 						genesisState.Perpetuals = []perptypes.Perpetual{
 							constants.BtcUsd_20PercentInitial_10PercentMaintenance,
+							constants.IsoUsd_IsolatedMarket,
 						}
 					},
 				)

protocol/x/clob/keeper/process_operations.go

@@ -389,6 +389,9 @@ func (k Keeper) PersistOrderRemovalToState(
 	// 			orderRemoval,
 	// 		)
 	// 	}
+	case types.OrderRemoval_REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS:
+		// TODO (CLOB-877)
+		k.statUnverifiedOrderRemoval(ctx, orderRemoval, orderToRemove)
 	default:
 		return errorsmod.Wrapf(
 			types.ErrInvalidOrderRemovalReason,

protocol/x/clob/memclob/memclob.go

@@ -508,6 +508,8 @@ func (m *MemClobPriceTimePriority) PlaceOrder(
 				removalReason = types.OrderRemoval_REMOVAL_REASON_CONDITIONAL_FOK_COULD_NOT_BE_FULLY_FILLED
 			} else if errors.Is(err, types.ErrPostOnlyWouldCrossMakerOrder) {
 				removalReason = types.OrderRemoval_REMOVAL_REASON_POST_ONLY_WOULD_CROSS_MAKER_ORDER
+			} else if errors.Is(err, types.ErrWouldViolateIsolatedSubaccountConstraints) {
+				removalReason = types.OrderRemoval_REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS
 			}
 
 			if !m.operationsToPropose.IsOrderRemovalInOperationsQueue(order.OrderId) {
@@ -833,11 +835,7 @@ func (m *MemClobPriceTimePriority) matchOrder(
 		// filled or not filled at all.
 		// TODO(CLOB-267): Create more granular error types here that indicate why the order was not
 		// fully filled (i.e. undercollateralized, reduce only resized, etc).
-		if takerOrderStatus.OrderStatus == types.ViolatesIsolatedSubaccountConstraints {
-			matchingErr = types.ErrWouldViolateIsolatedSubaccountConstraints
-		} else {
-			matchingErr = types.ErrFokOrderCouldNotBeFullyFilled
-		}
+		matchingErr = types.ErrFokOrderCouldNotBeFullyFilled
 	}
 
 	// If the order is post only and it's not the rewind step, then it cannot be filled.
@@ -849,6 +847,12 @@ func (m *MemClobPriceTimePriority) matchOrder(
 		matchingErr = types.ErrPostOnlyWouldCrossMakerOrder
 	}
 
+	// If the order filling leads to the subaccount having an invalid state due to failing checks for
+	// isolated subaccount constraints, return an error so that the order is canceled.
+	if !order.IsLiquidation() && takerOrderStatus.OrderStatus == types.ViolatesIsolatedSubaccountConstraints {
+		matchingErr = types.ErrWouldViolateIsolatedSubaccountConstraints
+	}
+
 	// If the match is valid and placing the taker order generated valid matches, update memclob state.
 	takerGeneratedValidMatches := len(newMakerFills) > 0 && matchingErr == nil
 	if takerGeneratedValidMatches {