Include untriggered conditional orders in stateful order count (#651)

* Include untriggered conditional orders in stateful order count

* add error log

* comments

protocol/x/clob/keeper/keeper.go

@@ -185,6 +185,17 @@ func (k Keeper) InitMemStore(ctx sdk.Context) {
 			memPrefixStore.Set(iterator.Key(), iterator.Value())
 		}
 	}
+
+	// Ensure that the stateful order count is accurately represented in the memstore on restart.
+	statefulOrders := k.GetAllStatefulOrders(ctx)
+	for _, order := range statefulOrders {
+		subaccountId := order.GetSubaccountId()
+		k.SetStatefulOrderCount(
+			ctx,
+			subaccountId,
+			k.GetStatefulOrderCount(ctx, subaccountId)+1,
+		)
+	}
 }
 
 // Sets the ante handler after it has been constructed. This breaks a cycle between

protocol/x/clob/keeper/keeper_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/dydxprotocol/v4-chain/protocol/mocks"
+	"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
 	keepertest "github.com/dydxprotocol/v4-chain/protocol/testutil/keeper"
 	"github.com/dydxprotocol/v4-chain/protocol/x/clob/memclob"
 	"github.com/stretchr/testify/require"
@@ -35,3 +36,45 @@ func TestInitMemStore_OnlyAllowedOnce(t *testing.T) {
 		ks.ClobKeeper.InitMemStore(ks.Ctx)
 	})
 }
+
+func TestInitMemStore_StatefulOrderCount(t *testing.T) {
+	memClob := memclob.NewMemClobPriceTimePriority(false)
+	ks := keepertest.NewClobKeepersTestContextWithUninitializedMemStore(
+		t,
+		memClob,
+		&mocks.BankKeeper{},
+		&mocks.IndexerEventManager{},
+	)
+
+	// Long term order.
+	ks.ClobKeeper.SetLongTermOrderPlacement(
+		ks.Ctx,
+		constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy100_Price10_GTBT15,
+		1,
+	)
+
+	// Triggered conditional order.
+	ks.ClobKeeper.SetLongTermOrderPlacement(
+		ks.Ctx,
+		constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_SL_50001,
+		1,
+	)
+	ks.ClobKeeper.MustTriggerConditionalOrder(
+		ks.Ctx,
+		constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy1BTC_Price50000_GTBT10_SL_50001.OrderId,
+	)
+
+	// Untriggered conditional order.
+	ks.ClobKeeper.SetLongTermOrderPlacement(
+		ks.Ctx,
+		constants.ConditionalOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTBT15_StopLoss20, // Clob 1
+		1,
+	)
+
+	// Reset the stateful order count to zero.
+	ks.ClobKeeper.SetStatefulOrderCount(ks.Ctx, constants.Alice_Num0, 0)
+
+	// InitMemStore should repopulate the count.
+	ks.ClobKeeper.InitMemStore(ks.Ctx)
+	require.Equal(t, uint32(3), ks.ClobKeeper.GetStatefulOrderCount(ks.Ctx, constants.Alice_Num0))
+}

protocol/x/clob/keeper/orders.go

@@ -1146,7 +1146,7 @@ func (k Keeper) GetStatePosition(ctx sdk.Context, subaccountId satypes.Subaccoun
 }
 
 // InitStatefulOrders places all stateful orders in state on the memclob, placed in ascending
-// order by time priority and initializes the stateful order count.
+// order by time priority.
 // This is called during app initialization in `app.go`, before any ABCI calls are received
 // and after all MemClob orderbooks are instantiated.
 func (k Keeper) InitStatefulOrders(
@@ -1164,13 +1164,6 @@ func (k Keeper) InitStatefulOrders(
 	// Place each order in the memclob, ignoring errors if they occur.
 	statefulOrders := k.GetAllPlacedStatefulOrders(ctx)
 	for _, statefulOrder := range statefulOrders {
-		// Ensure that the stateful order count is accurately represented in the memstore on restart.
-		k.SetStatefulOrderCount(
-			ctx,
-			statefulOrder.OrderId.SubaccountId,
-			k.GetStatefulOrderCount(ctx, statefulOrder.OrderId.SubaccountId)+1,
-		)
-
 		// First fork the multistore. If `PlaceOrder` fails, we don't want to write to state.
 		placeOrderCtx, writeCache := ctx.CacheContext()
 
@@ -1230,7 +1223,6 @@ func (k Keeper) HydrateUntriggeredConditionalOrders(
 
 	// Get all untriggered conditional orders in state, ordered by time priority ascending order,
 	// and add them to the `UntriggeredConditionalOrders` data structure.
-	// Place each order in the memclob, ignoring errors if they occur.
 	untriggeredConditionalOrders := k.GetAllUntriggeredConditionalOrders(ctx)
 	k.AddUntriggeredConditionalOrders(
 		ctx,

protocol/x/clob/keeper/orders_test.go

@@ -2040,7 +2040,6 @@ func TestInitStatefulOrders(t *testing.T) {
 
 			// Create each stateful order placement in state and properly mock the MemClob call.
 			expectedPlacedOrders := make([]types.Order, 0)
-			expectedStatefulOrderCounts := make(map[satypes.SubaccountId]uint32)
 			for i, order := range tc.statefulOrdersInState {
 				require.True(t, order.IsStatefulOrder())
 
@@ -2061,12 +2060,6 @@ func TestInitStatefulOrders(t *testing.T) {
 					ks.ClobKeeper.MustTriggerConditionalOrder(ks.Ctx, order.OrderId)
 				}
 
-				// Increment the expected count for non-conditional stateful orders and triggered conditional orders.
-				if (order.IsStatefulOrder() && !order.IsConditionalOrder()) ||
-					(order.IsConditionalOrder() && tc.isConditionalOrderTriggered[order.OrderId]) {
-					expectedStatefulOrderCounts[order.GetSubaccountId()] = expectedStatefulOrderCounts[order.GetSubaccountId()] + 1
-				}
-
 				orderPlacementErr := tc.orderPlacementErrors[order.OrderId]
 				memClob.On("PlaceOrder", mock.Anything, order).Return(
 					satypes.BaseQuantums(0),
@@ -2084,16 +2077,6 @@ func TestInitStatefulOrders(t *testing.T) {
 
 			// Run the test and verify expectations.
 			ks.ClobKeeper.InitStatefulOrders(ks.Ctx)
-			for subaccountId, count := range expectedStatefulOrderCounts {
-				require.Equal(
-					t,
-					count,
-					ks.ClobKeeper.GetStatefulOrderCount(
-						ks.Ctx,
-						subaccountId,
-					),
-				)
-			}
 			indexerEventManager.AssertExpectations(t)
 			indexerEventManager.AssertNumberOfCalls(
 				t,

protocol/x/clob/keeper/stateful_order_state.go

@@ -7,6 +7,7 @@ import (
 
 	gometrics "github.com/armon/go-metrics"
 	db "github.com/cometbft/cometbft-db"
+	"github.com/cometbft/cometbft/libs/log"
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	"github.com/cosmos/cosmos-sdk/telemetry"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -152,7 +153,14 @@ func (k Keeper) DeleteLongTermOrderPlacement(
 	count := k.GetStatefulOrderCount(ctx, orderId.SubaccountId)
 	orderKey := orderId.ToStateKey()
 	if memStore.Has(orderKey) {
-		count--
+		if count == 0 {
+			k.Logger(ctx).Error(
+				"Stateful order count is zero but order is in the memstore. Underflow",
+				"orderId", log.NewLazySprintf("%+v", orderId),
+			)
+		} else {
+			count--
+		}
 	}
 
 	// Delete the `StatefulOrderPlacement` from state.
@@ -397,6 +405,12 @@ func (k Keeper) RemoveExpiredStatefulOrdersTimeSlices(ctx sdk.Context, blockTime
 	return expiredOrderIds
 }
 
+// GetAllStatefulOrders iterates over all stateful order placements and returns a list
+// of orders, ordered by ascending time priority.
+func (k Keeper) GetAllStatefulOrders(ctx sdk.Context) []types.Order {
+	return k.getStatefulOrders(k.getAllOrdersIterator(ctx))
+}
+
 // GetAllPlacedStatefulOrders iterates over all stateful order placements and returns a list
 // of orders, ordered by ascending time priority. Note that this only returns placed orders,
 // and therefore will not return untriggered conditional orders.
@@ -474,6 +488,16 @@ func (k Keeper) getStatefulOrdersTimeSliceIterator(ctx sdk.Context, endTime time
 	)
 }
 
+// getAllOrdersIterator returns an iterator over all stateful orders, which includes all
+// Long-Term orders, triggered and untriggered conditional orders.
+func (k Keeper) getAllOrdersIterator(ctx sdk.Context) sdk.Iterator {
+	store := prefix.NewStore(
+		ctx.KVStore(k.storeKey),
+		[]byte(types.StatefulOrderKeyPrefix),
+	)
+	return sdk.KVStorePrefixIterator(store, []byte{})
+}
+
 // getPlacedOrdersIterator returns an iterator over all placed orders, which includes all
 // Long-Term orders and triggered conditional orders.
 func (k Keeper) getPlacedOrdersIterator(ctx sdk.Context) sdk.Iterator {

protocol/x/clob/keeper/stateful_order_state_test.go

@@ -63,17 +63,19 @@ func TestLongTermOrderInitMemStore_Success(t *testing.T) {
 	longTermOrderStore := ks.ClobKeeper.GetLongTermOrderPlacementStore(ks.Ctx)
 
 	// Set orders only on the store, not the memstore.
+	index := uint32(0)
 	storeOrder := func(order types.Order, store prefix.Store) {
 		longTermOrderPlacement := types.LongTermOrderPlacement{
 			Order: order,
 			PlacementIndex: types.TransactionOrdering{
 				BlockHeight:      0,
-				TransactionIndex: 0,
+				TransactionIndex: index,
 			},
 		}
 		longTermOrderPlacementBytes := ks.Cdc.MustMarshal(&longTermOrderPlacement)
 		orderKey := order.OrderId.ToStateKey()
 		store.Set(orderKey, longTermOrderPlacementBytes)
+		index++
 	}
 
 	// Set some long term orders.