[CT-856] Add order replacement to fix vault causing orderbook flickering

.github/workflows/protocol-build-and-push.yml

@@ -6,6 +6,7 @@ on:  # yamllint disable-line rule:truthy
       - main
       - 'release/protocol/v[0-9]+.[0-9]+.x'  # e.g. release/protocol/v0.1.x
       - 'release/protocol/v[0-9]+.x'  # e.g. release/protocol/v1.x
+      - 'chenyao/order-replacement-messages-for-vault'
 
 jobs:
   build-and-push-dev:

protocol/app/app.go

@@ -1088,6 +1088,7 @@ func New(
 		app.PricesKeeper,
 		app.SendingKeeper,
 		app.SubaccountsKeeper,
+		app.IndexerEventManager,
 		[]string{
 			lib.GovModuleAddress.String(),
 			delaymsgmoduletypes.ModuleAddress.String(),

protocol/testutil/keeper/vault.go

@@ -58,6 +58,7 @@ func createVaultKeeper(
 		&mocks.PricesKeeper{},
 		&mocks.SendingKeeper{},
 		&mocks.SubaccountsKeeper{},
+		&mocks.IndexerEventManager{},
 		[]string{
 			lib.GovModuleAddress.String(),
 			delaymsgtypes.ModuleAddress.String(),

protocol/x/clob/keeper/msg_server_cancel_orders.go

@@ -23,7 +23,7 @@ func (k msgServer) CancelOrder(
 ) (resp *types.MsgCancelOrderResponse, err error) {
 	ctx := lib.UnwrapSDKContext(goCtx, types.ModuleName)
 
-	if err := k.Keeper.HandleMsgCancelOrder(ctx, msg); err != nil {
+	if err := k.Keeper.HandleMsgCancelOrder(ctx, msg, false); err != nil {
 		return nil, err
 	}
 
@@ -37,6 +37,7 @@ func (k msgServer) CancelOrder(
 func (k Keeper) HandleMsgCancelOrder(
 	ctx sdk.Context,
 	msg *types.MsgCancelOrder,
+	isInternalOrder bool,
 ) (err error) {
 	lib.AssertDeliverTxMode(ctx)
 
@@ -111,17 +112,19 @@ func (k Keeper) HandleMsgCancelOrder(
 	k.MustSetProcessProposerMatchesEvents(ctx, processProposerMatchesEvents)
 
 	// 4. Add the relevant on-chain Indexer event for the cancellation.
-	k.GetIndexerEventManager().AddTxnEvent(
-		ctx,
-		indexerevents.SubtypeStatefulOrder,
-		indexerevents.StatefulOrderEventVersion,
-		indexer_manager.GetBytes(
-			indexerevents.NewStatefulOrderRemovalEvent(
-				msg.OrderId,
-				indexershared.OrderRemovalReason_ORDER_REMOVAL_REASON_USER_CANCELED,
+	if !isInternalOrder {
+		k.GetIndexerEventManager().AddTxnEvent(
+			ctx,
+			indexerevents.SubtypeStatefulOrder,
+			indexerevents.StatefulOrderEventVersion,
+			indexer_manager.GetBytes(
+				indexerevents.NewStatefulOrderRemovalEvent(
+					msg.OrderId,
+					indexershared.OrderRemovalReason_ORDER_REMOVAL_REASON_USER_CANCELED,
+				),
 			),
-		),
-	)
+		)
+	}
 
 	return nil
 }

protocol/x/clob/keeper/msg_server_place_order.go

@@ -88,13 +88,15 @@ func (k Keeper) HandleMsgPlaceOrder(
 
 	// 2. Return an error if an associated cancellation or removal already exists in the current block.
 	processProposerMatchesEvents := k.GetProcessProposerMatchesEvents(ctx)
-	cancelledOrderIds := lib.UniqueSliceToSet(processProposerMatchesEvents.PlacedStatefulCancellationOrderIds)
-	if _, found := cancelledOrderIds[order.GetOrderId()]; found {
-		return errorsmod.Wrapf(
-			types.ErrStatefulOrderPreviouslyCancelled,
-			"PlaceOrder: order (%+v)",
-			order,
-		)
+	if !isInternalOrder { // If vault order, we allow the order to replace a cancelled order with the same order ID
+		cancelledOrderIds := lib.UniqueSliceToSet(processProposerMatchesEvents.PlacedStatefulCancellationOrderIds)
+		if _, found := cancelledOrderIds[order.GetOrderId()]; found {
+			return errorsmod.Wrapf(
+				types.ErrStatefulOrderPreviouslyCancelled,
+				"PlaceOrder: order (%+v)",
+				order,
+			)
+		}
 	}
 	removedOrderIds := lib.UniqueSliceToSet(processProposerMatchesEvents.RemovedStatefulOrderIds)
 	if _, found := removedOrderIds[order.GetOrderId()]; found {
@@ -115,31 +117,35 @@ func (k Keeper) HandleMsgPlaceOrder(
 
 	// 4. Emit the new order placement indexer event.
 	if order.IsConditionalOrder() {
-		k.GetIndexerEventManager().AddTxnEvent(
-			ctx,
-			indexerevents.SubtypeStatefulOrder,
-			indexerevents.StatefulOrderEventVersion,
-			indexer_manager.GetBytes(
-				indexerevents.NewConditionalOrderPlacementEvent(
-					order,
+		if !isInternalOrder {
+			k.GetIndexerEventManager().AddTxnEvent(
+				ctx,
+				indexerevents.SubtypeStatefulOrder,
+				indexerevents.StatefulOrderEventVersion,
+				indexer_manager.GetBytes(
+					indexerevents.NewConditionalOrderPlacementEvent(
+						order,
+					),
 				),
-			),
-		)
+			)
+		}
 		processProposerMatchesEvents.PlacedConditionalOrderIds = append(
 			processProposerMatchesEvents.PlacedConditionalOrderIds,
 			order.OrderId,
 		)
 	} else {
-		k.GetIndexerEventManager().AddTxnEvent(
-			ctx,
-			indexerevents.SubtypeStatefulOrder,
-			indexerevents.StatefulOrderEventVersion,
-			indexer_manager.GetBytes(
-				indexerevents.NewLongTermOrderPlacementEvent(
-					order,
+		if !isInternalOrder {
+			k.GetIndexerEventManager().AddTxnEvent(
+				ctx,
+				indexerevents.SubtypeStatefulOrder,
+				indexerevents.StatefulOrderEventVersion,
+				indexer_manager.GetBytes(
+					indexerevents.NewLongTermOrderPlacementEvent(
+						order,
+					),
 				),
-			),
-		)
+			)
+		}
 		processProposerMatchesEvents.PlacedLongTermOrderIds = append(
 			processProposerMatchesEvents.PlacedLongTermOrderIds,
 			order.OrderId,

protocol/x/clob/keeper/msg_server_place_order_test.go

@@ -435,13 +435,12 @@ func TestHandleMsgPlaceOrder(t *testing.T) {
 			removalExists:         false,
 			equityTierLimitExists: true,
 		},
-		"Error - Place an Internal Order, Order Already Cancelled": {
+		"Success - Place an Internal Order, Order Already Cancelled": {
 			isInternalOrder:       true,
 			assetQuantums:         -1_000_000_000,
 			cancellationExists:    true,
 			removalExists:         false,
 			equityTierLimitExists: true,
-			expectedError:         types.ErrStatefulOrderPreviouslyCancelled,
 		},
 		"Error - Place an Internal Order, Order Already Removed": {
 			isInternalOrder:       true,

protocol/x/clob/types/clob_keeper.go

@@ -46,6 +46,7 @@ type ClobKeeper interface {
 	HandleMsgCancelOrder(
 		ctx sdk.Context,
 		msg *MsgCancelOrder,
+		isInternalOrder bool,
 	) (err error)
 	HandleMsgPlaceOrder(
 		ctx sdk.Context,

protocol/x/vault/keeper/keeper.go

@@ -7,20 +7,22 @@ import (
 	storetypes "cosmossdk.io/store/types"
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/dydxprotocol/v4-chain/protocol/indexer/indexer_manager"
 	"github.com/dydxprotocol/v4-chain/protocol/lib"
 	"github.com/dydxprotocol/v4-chain/protocol/x/vault/types"
 )
 
 type (
 	Keeper struct {
-		cdc               codec.BinaryCodec
-		storeKey          storetypes.StoreKey
-		clobKeeper        types.ClobKeeper
-		perpetualsKeeper  types.PerpetualsKeeper
-		pricesKeeper      types.PricesKeeper
-		sendingKeeper     types.SendingKeeper
-		subaccountsKeeper types.SubaccountsKeeper
-		authorities       map[string]struct{}
+		cdc                 codec.BinaryCodec
+		storeKey            storetypes.StoreKey
+		clobKeeper          types.ClobKeeper
+		perpetualsKeeper    types.PerpetualsKeeper
+		pricesKeeper        types.PricesKeeper
+		sendingKeeper       types.SendingKeeper
+		subaccountsKeeper   types.SubaccountsKeeper
+		indexerEventManager indexer_manager.IndexerEventManager
+		authorities         map[string]struct{}
 	}
 )
 
@@ -32,20 +34,26 @@ func NewKeeper(
 	pricesKeeper types.PricesKeeper,
 	sendingKeeper types.SendingKeeper,
 	subaccountsKeeper types.SubaccountsKeeper,
+	indexerEventManager indexer_manager.IndexerEventManager,
 	authorities []string,
 ) *Keeper {
 	return &Keeper{
-		cdc:               cdc,
-		storeKey:          storeKey,
-		clobKeeper:        clobKeeper,
-		perpetualsKeeper:  perpetualsKeeper,
-		pricesKeeper:      pricesKeeper,
-		sendingKeeper:     sendingKeeper,
-		subaccountsKeeper: subaccountsKeeper,
-		authorities:       lib.UniqueSliceToSet(authorities),
+		cdc:                 cdc,
+		storeKey:            storeKey,
+		clobKeeper:          clobKeeper,
+		perpetualsKeeper:    perpetualsKeeper,
+		pricesKeeper:        pricesKeeper,
+		sendingKeeper:       sendingKeeper,
+		subaccountsKeeper:   subaccountsKeeper,
+		indexerEventManager: indexerEventManager,
+		authorities:         lib.UniqueSliceToSet(authorities),
 	}
 }
 
+func (k Keeper) GetIndexerEventManager() indexer_manager.IndexerEventManager {
+	return k.indexerEventManager
+}
+
 func (k Keeper) HasAuthority(authority string) bool {
 	_, ok := k.authorities[authority]
 	return ok

protocol/x/vault/keeper/orders.go

@@ -6,6 +6,9 @@ import (
 
 	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	indexerevents "github.com/dydxprotocol/v4-chain/protocol/indexer/events"
+	"github.com/dydxprotocol/v4-chain/protocol/indexer/indexer_manager"
+	indexershared "github.com/dydxprotocol/v4-chain/protocol/indexer/shared/types"
 	"github.com/dydxprotocol/v4-chain/protocol/lib"
 	"github.com/dydxprotocol/v4-chain/protocol/lib/log"
 	"github.com/dydxprotocol/v4-chain/protocol/lib/metrics"
@@ -24,6 +27,7 @@ func (k Keeper) RefreshAllVaultOrders(ctx sdk.Context) {
 	defer totalSharesIterator.Close()
 	for ; totalSharesIterator.Valid(); totalSharesIterator.Next() {
 		vaultId, err := types.GetVaultIdFromStateKey(totalSharesIterator.Key())
+
 		if err != nil {
 			log.ErrorLogWithError(ctx, "Failed to get vault ID from state key", err)
 			continue
@@ -84,7 +88,7 @@ func (k Keeper) RefreshVaultClobOrders(ctx sdk.Context, vaultId types.VaultId) (
 			err := k.clobKeeper.HandleMsgCancelOrder(ctx, clobtypes.NewMsgCancelOrderStateful(
 				order.OrderId,
 				uint32(ctx.BlockTime().Unix())+orderExpirationSeconds,
-			))
+			), true)
 			if err != nil {
 				log.ErrorLogWithError(ctx, "Failed to cancel order", err, "order", order, "vaultId", vaultId)
 			}
@@ -97,11 +101,12 @@ func (k Keeper) RefreshVaultClobOrders(ctx sdk.Context, vaultId types.VaultId) (
 
 	// Place new CLOB orders.
 	ordersToPlace, err := k.GetVaultClobOrders(ctx, vaultId)
+
 	if err != nil {
 		log.ErrorLogWithError(ctx, "Failed to get vault clob orders to place", err, "vaultId", vaultId)
 		return err
 	}
-	for _, order := range ordersToPlace {
+	for i, order := range ordersToPlace {
 		err := k.PlaceVaultClobOrder(ctx, order)
 		if err != nil {
 			log.ErrorLogWithError(ctx, "Failed to place order", err, "order", order, "vaultId", vaultId)
@@ -110,8 +115,46 @@ func (k Keeper) RefreshVaultClobOrders(ctx sdk.Context, vaultId types.VaultId) (
 			metrics.VaultPlaceOrder,
 			metrics.GetLabelForBoolValue(metrics.Success, err == nil),
 		)
-	}
 
+		// Send indexer messages.
+		// If the price of the old and new orders are different, send a cancel and a place message
+		// Otherwise, send an order place message only.
+		replacedOrder := ordersToCancel[i]
+		if replacedOrder.Subticks != order.Subticks {
+			k.GetIndexerEventManager().AddTxnEvent(
+				ctx,
+				indexerevents.SubtypeStatefulOrder,
+				indexerevents.StatefulOrderEventVersion,
+				indexer_manager.GetBytes(
+					indexerevents.NewStatefulOrderRemovalEvent(
+						replacedOrder.OrderId,
+						indexershared.OrderRemovalReason_ORDER_REMOVAL_REASON_REPLACED,
+					),
+				),
+			)
+			k.GetIndexerEventManager().AddTxnEvent(
+				ctx,
+				indexerevents.SubtypeStatefulOrder,
+				indexerevents.StatefulOrderEventVersion,
+				indexer_manager.GetBytes(
+					indexerevents.NewLongTermOrderPlacementEvent(
+						*order,
+					),
+				),
+			)
+		} else {
+			k.GetIndexerEventManager().AddTxnEvent(
+				ctx,
+				indexerevents.SubtypeStatefulOrder,
+				indexerevents.StatefulOrderEventVersion,
+				indexer_manager.GetBytes(
+					indexerevents.NewLongTermOrderPlacementEvent(
+						*order,
+					),
+				),
+			)
+		}
+	}
 	return nil
 }
 
@@ -335,11 +378,6 @@ func (k Keeper) GetVaultClobOrders(
 // GetVaultClobOrderClientId returns the client ID for a CLOB order where
 // - 1st bit is `side-1` (subtract 1 as buy_side = 1, sell_side = 2)
 //
-// - 2nd bit is `block height % 2`
-//   - block height bit alternates between 0 and 1 to ensure that client IDs
-//     are different in two consecutive blocks (otherwise, order placement would
-//     fail because the same order IDs are already marked for cancellation)
-//
 // - next 8 bits are `layer`
 func (k Keeper) GetVaultClobOrderClientId(
 	ctx sdk.Context,
@@ -349,12 +387,9 @@ func (k Keeper) GetVaultClobOrderClientId(
 	sideBit := uint32(side - 1)
 	sideBit <<= 31
 
-	blockHeightBit := uint32(ctx.BlockHeight() % 2)
-	blockHeightBit <<= 30
-
-	layerBits := uint32(layer) << 22
+	layerBits := uint32(layer) << 23
 
-	return sideBit | blockHeightBit | layerBits
+	return sideBit | layerBits
 }
 
 // PlaceVaultClobOrder places a vault CLOB order as an order internal to the protocol,