From f511a2cb5354904ad4e91a8e96357078d8e75f2c Mon Sep 17 00:00:00 2001 From: Jay Yu <103467857+jayy04@users.noreply.github.com> Date: Wed, 27 Nov 2024 10:31:07 -0500 Subject: [PATCH 1/6] [CT-1327] place post only orders first in prepare check state --- protocol/mocks/MemClob.go | 10 +++--- protocol/x/clob/abci.go | 47 +++++++++++++++++++++------ protocol/x/clob/keeper/orders.go | 15 ++++++++- protocol/x/clob/keeper/orders_test.go | 9 +++-- protocol/x/clob/memclob/memclob.go | 6 ++++ protocol/x/clob/types/memclob.go | 1 + protocol/x/clob/types/order.go | 5 +++ 7 files changed, 74 insertions(+), 19 deletions(-) diff --git a/protocol/mocks/MemClob.go b/protocol/mocks/MemClob.go index 9db224e05b..93e0eacb2c 100644 --- a/protocol/mocks/MemClob.go +++ b/protocol/mocks/MemClob.go @@ -578,17 +578,17 @@ func (_m *MemClob) RemoveOrderIfFilled(ctx types.Context, orderId clobtypes.Orde _m.Called(ctx, orderId) } -// ReplayOperations provides a mock function with given fields: ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates -func (_m *MemClob) ReplayOperations(ctx types.Context, localOperations []clobtypes.InternalOperation, shortTermOrderTxBytes map[clobtypes.OrderHash][]byte, existingOffchainUpdates *clobtypes.OffchainUpdates) *clobtypes.OffchainUpdates { - ret := _m.Called(ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates) +// ReplayOperations provides a mock function with given fields: ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates, onlyPlacePostOnly +func (_m *MemClob) ReplayOperations(ctx types.Context, localOperations []clobtypes.InternalOperation, shortTermOrderTxBytes map[clobtypes.OrderHash][]byte, existingOffchainUpdates *clobtypes.OffchainUpdates, onlyPlacePostOnly bool) *clobtypes.OffchainUpdates { + ret := _m.Called(ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates, onlyPlacePostOnly) if len(ret) == 0 { panic("no return value specified for ReplayOperations") } var r0 *clobtypes.OffchainUpdates - if rf, ok := ret.Get(0).(func(types.Context, []clobtypes.InternalOperation, map[clobtypes.OrderHash][]byte, *clobtypes.OffchainUpdates) *clobtypes.OffchainUpdates); ok { - r0 = rf(ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates) + if rf, ok := ret.Get(0).(func(types.Context, []clobtypes.InternalOperation, map[clobtypes.OrderHash][]byte, *clobtypes.OffchainUpdates, bool) *clobtypes.OffchainUpdates); ok { + r0 = rf(ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates, onlyPlacePostOnly) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*clobtypes.OffchainUpdates) diff --git a/protocol/x/clob/abci.go b/protocol/x/clob/abci.go index 11ac034253..2168664988 100644 --- a/protocol/x/clob/abci.go +++ b/protocol/x/clob/abci.go @@ -185,15 +185,42 @@ func PrepareCheckState( offchainUpdates, ) - // 3. Place all stateful order placements included in the last block on the memclob. + // 3. Go through the orders two times and only place the post only orders during the first pass. + longTermOrderIds := keeper.GetDeliveredLongTermOrderIds(ctx) + offchainUpdates = keeper.PlaceStatefulOrdersFromLastBlock( + ctx, + longTermOrderIds, + offchainUpdates, + true, // post only + ) + + offchainUpdates = keeper.PlaceConditionalOrdersTriggeredInLastBlock( + ctx, + processProposerMatchesEvents.ConditionalOrderIdsTriggeredInLastBlock, + offchainUpdates, + true, // post only + ) + + replayUpdates := keeper.MemClob.ReplayOperations( + ctx, + localValidatorOperationsQueue, + shortTermOrderTxBytes, + offchainUpdates, + true, // post only + ) + if replayUpdates != nil { + offchainUpdates = replayUpdates + } + + // 4. Place all stateful order placements included in the last block on the memclob. // Note telemetry is measured outside of the function call because `PlaceStatefulOrdersFromLastBlock` // is called within `PlaceConditionalOrdersTriggeredInLastBlock`. startPlaceLongTermOrders := time.Now() - longTermOrderIds := keeper.GetDeliveredLongTermOrderIds(ctx) offchainUpdates = keeper.PlaceStatefulOrdersFromLastBlock( ctx, longTermOrderIds, offchainUpdates, + false, // post only ) telemetry.MeasureSince( startPlaceLongTermOrders, @@ -208,27 +235,27 @@ func PrepareCheckState( metrics.Count, ) - // 4. Place all conditional orders triggered in EndBlocker of last block on the memclob. + // 5. Place all conditional orders triggered in EndBlocker of last block on the memclob. offchainUpdates = keeper.PlaceConditionalOrdersTriggeredInLastBlock( ctx, processProposerMatchesEvents.ConditionalOrderIdsTriggeredInLastBlock, offchainUpdates, + false, // post only ) - // 5. Replay the local validator’s operations onto the book. - replayUpdates := keeper.MemClob.ReplayOperations( + // 6. Replay the local validator’s operations onto the book. + replayUpdates = keeper.MemClob.ReplayOperations( ctx, localValidatorOperationsQueue, shortTermOrderTxBytes, offchainUpdates, + false, // post only ) - - // TODO(CLOB-275): Do not gracefully handle panics in `PrepareCheckState`. if replayUpdates != nil { offchainUpdates = replayUpdates } - // 6. Get all potentially liquidatable subaccount IDs and attempt to liquidate them. + // 7. Get all potentially liquidatable subaccount IDs and attempt to liquidate them. liquidatableSubaccountIds := keeper.DaemonLiquidationInfo.GetLiquidatableSubaccountIds() subaccountsToDeleverage, err := keeper.LiquidateSubaccountsAgainstOrderbook(ctx, liquidatableSubaccountIds) if err != nil { @@ -241,14 +268,14 @@ func PrepareCheckState( keeper.GetSubaccountsWithPositionsInFinalSettlementMarkets(ctx)..., ) - // 7. Deleverage subaccounts. + // 8. Deleverage subaccounts. // TODO(CLOB-1052) - decouple steps 6 and 7 by using DaemonLiquidationInfo.NegativeTncSubaccounts // as the input for this function. if err := keeper.DeleverageSubaccounts(ctx, subaccountsToDeleverage); err != nil { panic(err) } - // 8. Gate withdrawals by inserting a zero-fill deleveraging operation into the operations queue if any + // 9. Gate withdrawals by inserting a zero-fill deleveraging operation into the operations queue if any // of the negative TNC subaccounts still have negative TNC after liquidations and deleveraging steps. negativeTncSubaccountIds := keeper.DaemonLiquidationInfo.GetNegativeTncSubaccountIds() if err := keeper.GateWithdrawalsIfNegativeTncSubaccountSeen(ctx, negativeTncSubaccountIds); err != nil { diff --git a/protocol/x/clob/keeper/orders.go b/protocol/x/clob/keeper/orders.go index f7447ff8d9..7ff3626427 100644 --- a/protocol/x/clob/keeper/orders.go +++ b/protocol/x/clob/keeper/orders.go @@ -496,6 +496,7 @@ func (k Keeper) PlaceStatefulOrdersFromLastBlock( ctx sdk.Context, placedStatefulOrderIds []types.OrderId, existingOffchainUpdates *types.OffchainUpdates, + onlyPlacePostOnly bool, ) ( offchainUpdates *types.OffchainUpdates, ) { @@ -521,6 +522,12 @@ func (k Keeper) PlaceStatefulOrdersFromLastBlock( } order := orderPlacement.GetOrder() + + // Prioritize placing post-only orders if the flag is set. + if onlyPlacePostOnly && !order.IsPostOnlyOrder() { + continue + } + // Validate and place order. _, orderStatus, placeOrderOffchainUpdates, err := k.AddPreexistingStatefulOrder( ctx, @@ -579,6 +586,7 @@ func (k Keeper) PlaceConditionalOrdersTriggeredInLastBlock( ctx sdk.Context, conditionalOrderIdsTriggeredInLastBlock []types.OrderId, existingOffchainUpdates *types.OffchainUpdates, + onlyPlacePostOnly bool, ) ( offchainUpdates *types.OffchainUpdates, ) { @@ -608,7 +616,12 @@ func (k Keeper) PlaceConditionalOrdersTriggeredInLastBlock( } } - return k.PlaceStatefulOrdersFromLastBlock(ctx, conditionalOrderIdsTriggeredInLastBlock, existingOffchainUpdates) + return k.PlaceStatefulOrdersFromLastBlock( + ctx, + conditionalOrderIdsTriggeredInLastBlock, + existingOffchainUpdates, + onlyPlacePostOnly, + ) } // PerformOrderCancellationStatefulValidation performs stateful validation on an order cancellation. diff --git a/protocol/x/clob/keeper/orders_test.go b/protocol/x/clob/keeper/orders_test.go index 9c31b2bdb6..e7d95a42e3 100644 --- a/protocol/x/clob/keeper/orders_test.go +++ b/protocol/x/clob/keeper/orders_test.go @@ -2218,7 +2218,8 @@ func TestPlaceStatefulOrdersFromLastBlock(t *testing.T) { for _, order := range tc.orders { orderIds = append(orderIds, order.OrderId) } - ks.ClobKeeper.PlaceStatefulOrdersFromLastBlock(ctx, orderIds, offchainUpdates) + ks.ClobKeeper.PlaceStatefulOrdersFromLastBlock(ctx, orderIds, offchainUpdates, true) + ks.ClobKeeper.PlaceStatefulOrdersFromLastBlock(ctx, orderIds, offchainUpdates, false) // PlaceStatefulOrdersFromLastBlock utilizes the memclob's PlaceOrder flow, but we // do not want to emit PlaceMessages in offchain events for stateful orders. This assertion @@ -2376,13 +2377,15 @@ func TestPlaceConditionalOrdersTriggeredInLastBlock(t *testing.T) { t, tc.expectedPanic, func() { - ks.ClobKeeper.PlaceConditionalOrdersTriggeredInLastBlock(ctx, orderIds, offchainUpdates) + ks.ClobKeeper.PlaceConditionalOrdersTriggeredInLastBlock(ctx, orderIds, offchainUpdates, true) + ks.ClobKeeper.PlaceConditionalOrdersTriggeredInLastBlock(ctx, orderIds, offchainUpdates, false) }, ) return } - ks.ClobKeeper.PlaceConditionalOrdersTriggeredInLastBlock(ctx, orderIds, offchainUpdates) + ks.ClobKeeper.PlaceConditionalOrdersTriggeredInLastBlock(ctx, orderIds, offchainUpdates, true) + ks.ClobKeeper.PlaceConditionalOrdersTriggeredInLastBlock(ctx, orderIds, offchainUpdates, false) // PlaceStatefulOrdersFromLastBlock utilizes the memclob's PlaceOrder flow, but we // do not want to emit PlaceMessages in offchain events for stateful orders. This assertion diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go index 1229e8d3ec..4db70ecff5 100644 --- a/protocol/x/clob/memclob/memclob.go +++ b/protocol/x/clob/memclob/memclob.go @@ -879,6 +879,7 @@ func (m *MemClobPriceTimePriority) ReplayOperations( localOperations []types.InternalOperation, shortTermOrderTxBytes map[types.OrderHash][]byte, existingOffchainUpdates *types.OffchainUpdates, + onlyPlacePostOnly bool, ) *types.OffchainUpdates { lib.AssertCheckTxMode(ctx) @@ -923,6 +924,11 @@ func (m *MemClobPriceTimePriority) ReplayOperations( case *types.InternalOperation_ShortTermOrderPlacement: order := operation.GetShortTermOrderPlacement().Order + // Skip the order if it is a post-only order and we are only replaying post-only orders. + if onlyPlacePostOnly && !order.IsPostOnlyOrder() { + continue + } + // Set underlying tx bytes so OperationsToPropose may access it and // store the tx bytes on OperationHashToTxBytes data structure shortTermOrderTxBytes, exists := shortTermOrderTxBytes[order.GetOrderHash()] diff --git a/protocol/x/clob/types/memclob.go b/protocol/x/clob/types/memclob.go index 86191376f7..ab63fe5fca 100644 --- a/protocol/x/clob/types/memclob.go +++ b/protocol/x/clob/types/memclob.go @@ -110,6 +110,7 @@ type MemClob interface { localOperations []InternalOperation, shortTermOrderTxBytes map[OrderHash][]byte, existingOffchainUpdates *OffchainUpdates, + onlyPlacePostOnly bool, ) (offchainUpdates *OffchainUpdates) SetMemclobGauges( ctx sdk.Context, diff --git a/protocol/x/clob/types/order.go b/protocol/x/clob/types/order.go index deac6716e1..060e1fd20f 100644 --- a/protocol/x/clob/types/order.go +++ b/protocol/x/clob/types/order.go @@ -180,6 +180,11 @@ func (o *Order) IsConditionalOrder() bool { return o.OrderId.IsConditionalOrder() } +// IsPostOnlyOrder returns whether this order is a post only order. +func (o *Order) IsPostOnlyOrder() bool { + return o.GetTimeInForce() == Order_TIME_IN_FORCE_POST_ONLY +} + // CanTrigger returns if a condition order is eligible to be triggered based on a given // subticks value. Function will panic if order is not a conditional order. func (o *Order) CanTrigger(subticks Subticks) bool { From 88dcf1af0421154b4a21357e184d0730d4487bcc Mon Sep 17 00:00:00 2001 From: Jay Yu <103467857+jayy04@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:41:35 -0500 Subject: [PATCH 2/6] comments --- protocol/x/clob/memclob/memclob.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go index 4db70ecff5..f69a6227d2 100644 --- a/protocol/x/clob/memclob/memclob.go +++ b/protocol/x/clob/memclob/memclob.go @@ -1019,6 +1019,11 @@ func (m *MemClobPriceTimePriority) ReplayOperations( continue } + // Skip the order if it is a post-only order and we are only replaying post-only orders. + if onlyPlacePostOnly && !statefulOrderPlacement.Order.IsPostOnlyOrder() { + continue + } + // TODO(DEC-998): Research whether it's fine for two post-only orders to be matched. Currently they are dropped. _, orderStatus, placeOrderOffchainUpdates, err := m.clobKeeper.AddPreexistingStatefulOrder( ctx, @@ -1072,6 +1077,11 @@ func (m *MemClobPriceTimePriority) ReplayOperations( continue } + // Skip the order if it is a post-only order and we are only replaying post-only orders. + if onlyPlacePostOnly && !statefulOrderPlacement.Order.IsPostOnlyOrder() { + continue + } + _, orderStatus, placeOrderOffchainUpdates, err := m.PlaceOrder( ctx, statefulOrderPlacement.Order, From 480927414432ecb5e28b384e40e600c9b8a7d800 Mon Sep 17 00:00:00 2001 From: Jay Yu <103467857+jayy04@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:02:17 -0500 Subject: [PATCH 3/6] e2e --- .../testutil/constants/stateful_orders.go | 26 +++++++++++++++++++ .../x/clob/e2e/conditional_orders_test.go | 2 +- protocol/x/clob/e2e/order_removal_test.go | 4 +-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/protocol/testutil/constants/stateful_orders.go b/protocol/testutil/constants/stateful_orders.go index 3e0eb8a80d..fd034cdde4 100644 --- a/protocol/testutil/constants/stateful_orders.go +++ b/protocol/testutil/constants/stateful_orders.go @@ -1308,6 +1308,19 @@ var ( } // Long-Term post-only orders. + LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5_PO = clobtypes.Order{ + OrderId: clobtypes.OrderId{ + SubaccountId: Alice_Num0, + ClientId: 0, + OrderFlags: clobtypes.OrderIdFlags_LongTerm, + ClobPairId: 0, + }, + Side: clobtypes.Order_SIDE_BUY, + Quantums: 5, + Subticks: 10, + GoodTilOneof: &clobtypes.Order_GoodTilBlockTime{GoodTilBlockTime: 5}, + TimeInForce: clobtypes.Order_TIME_IN_FORCE_POST_ONLY, + } LongTermOrder_Alice_Num0_Id0_Clob0_Buy100_Price10_GTBT15_PO = clobtypes.Order{ OrderId: clobtypes.OrderId{ SubaccountId: Alice_Num0, @@ -1360,6 +1373,19 @@ var ( GoodTilOneof: &clobtypes.Order_GoodTilBlockTime{GoodTilBlockTime: 10}, TimeInForce: clobtypes.Order_TIME_IN_FORCE_POST_ONLY, } + LongTermOrder_Dave_Num0_Id0_Clob0_Sell025BTC_Price50000_GTBT10_PO = clobtypes.Order{ + OrderId: clobtypes.OrderId{ + SubaccountId: Dave_Num0, + ClientId: 0, + OrderFlags: clobtypes.OrderIdFlags_LongTerm, + ClobPairId: 0, + }, + Side: clobtypes.Order_SIDE_SELL, + Quantums: 25_000_000, + Subticks: 50_000_000_000, + GoodTilOneof: &clobtypes.Order_GoodTilBlockTime{GoodTilBlockTime: 10}, + TimeInForce: clobtypes.Order_TIME_IN_FORCE_POST_ONLY, + } // Long-Term reduce-only orders. LongTermOrder_Bob_Num0_Id2_Clob0_Sell10_Price35_GTB20_RO = clobtypes.Order{ diff --git a/protocol/x/clob/e2e/conditional_orders_test.go b/protocol/x/clob/e2e/conditional_orders_test.go index 2342906bf7..a9aa654571 100644 --- a/protocol/x/clob/e2e/conditional_orders_test.go +++ b/protocol/x/clob/e2e/conditional_orders_test.go @@ -1869,7 +1869,7 @@ func TestConditionalOrder_TriggeringUsingMatchedPrice(t *testing.T) { constants.Order_Dave_Num1_Id0_Clob0_Sell1BTC_Price49997_GTB10, constants.Order_Carl_Num1_Id0_Clob0_Buy1BTC_Price50003_GTB10, // Place the conditional order. - constants.LongTermOrder_Dave_Num0_Id0_Clob0_Sell025BTC_Price50000_GTBT10, + constants.LongTermOrder_Dave_Num0_Id0_Clob0_Sell025BTC_Price50000_GTBT10_PO, constants.ConditionalOrder_Carl_Num0_Id0_Clob0_Buy05BTC_Price50000_GTBT10_TP_49999_PO, }, expectedInTriggeredStateAfterBlock: map[uint32]map[clobtypes.OrderId]bool{ diff --git a/protocol/x/clob/e2e/order_removal_test.go b/protocol/x/clob/e2e/order_removal_test.go index 2261b4fdf6..0771219e01 100644 --- a/protocol/x/clob/e2e/order_removal_test.go +++ b/protocol/x/clob/e2e/order_removal_test.go @@ -43,7 +43,7 @@ func TestConditionalOrderRemoval(t *testing.T) { constants.Bob_Num0_10_000USD, }, orders: []clobtypes.Order{ - constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5, + constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5_PO, constants.ConditionalOrder_Bob_Num0_Id0_Clob0_Sell10_Price10_GTBT10_PO_SL_15, }, @@ -629,7 +629,7 @@ func TestOrderRemoval(t *testing.T) { constants.Alice_Num0_10_000USD, constants.Bob_Num0_10_000USD, }, - firstOrder: constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5, + firstOrder: constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5_PO, secondOrder: constants.LongTermOrder_Bob_Num0_Id0_Clob0_Sell10_Price10_GTBT10_PO, expectedFirstOrderRemoved: false, From a93998df39cea38642bba08030846d89e9a45b6f Mon Sep 17 00:00:00 2001 From: Jay Yu <103467857+jayy04@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:21:12 -0500 Subject: [PATCH 4/6] test --- protocol/x/clob/e2e/conditional_orders_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/x/clob/e2e/conditional_orders_test.go b/protocol/x/clob/e2e/conditional_orders_test.go index a9aa654571..6647861175 100644 --- a/protocol/x/clob/e2e/conditional_orders_test.go +++ b/protocol/x/clob/e2e/conditional_orders_test.go @@ -705,7 +705,7 @@ func TestConditionalOrder(t *testing.T) { constants.Dave_Num0_500000USD, }, orders: []clobtypes.Order{ - constants.LongTermOrder_Dave_Num0_Id0_Clob0_Sell025BTC_Price50000_GTBT10, + constants.LongTermOrder_Dave_Num0_Id0_Clob0_Sell025BTC_Price50000_GTBT10_PO, constants.ConditionalOrder_Carl_Num0_Id0_Clob0_Buy05BTC_Price50000_GTBT10_TP_49999_PO, }, priceUpdateForFirstBlock: &prices.MsgUpdateMarketPrices{ From 498f72a56c1d47bbde47daf5ced2b96ef929f0a2 Mon Sep 17 00:00:00 2001 From: Jay Yu <103467857+jayy04@users.noreply.github.com> Date: Sun, 1 Dec 2024 18:08:27 -0800 Subject: [PATCH 5/6] fix condition, add unit test --- protocol/x/clob/keeper/orders.go | 4 +- protocol/x/clob/keeper/orders_test.go | 131 ++++++++++++++++++++++++++ protocol/x/clob/memclob/memclob.go | 6 +- 3 files changed, 136 insertions(+), 5 deletions(-) diff --git a/protocol/x/clob/keeper/orders.go b/protocol/x/clob/keeper/orders.go index 7ff3626427..3316ff0948 100644 --- a/protocol/x/clob/keeper/orders.go +++ b/protocol/x/clob/keeper/orders.go @@ -523,8 +523,8 @@ func (k Keeper) PlaceStatefulOrdersFromLastBlock( order := orderPlacement.GetOrder() - // Prioritize placing post-only orders if the flag is set. - if onlyPlacePostOnly && !order.IsPostOnlyOrder() { + // Skip the order if it is a post-only order and we are only placing post-only orders. + if onlyPlacePostOnly != order.IsPostOnlyOrder() { continue } diff --git a/protocol/x/clob/keeper/orders_test.go b/protocol/x/clob/keeper/orders_test.go index e7d95a42e3..be55fe27d4 100644 --- a/protocol/x/clob/keeper/orders_test.go +++ b/protocol/x/clob/keeper/orders_test.go @@ -2248,6 +2248,137 @@ func TestPlaceStatefulOrdersFromLastBlock(t *testing.T) { } } +func TestPlaceStatefulOrdersFromLastBlock_PostOnly(t *testing.T) { + tests := map[string]struct { + orders []types.Order + onlyPlacePostOnly bool + + expectedOrderPlacementCalls []types.Order + }{ + "places PO stateful orders from last block when onlyPlacePostOnly = true": { + onlyPlacePostOnly: true, + orders: []types.Order{ + constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5_PO, + }, + expectedOrderPlacementCalls: []types.Order{ + constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5_PO, + }, + }, + "does not places non-PO stateful orders when onlyPlacePostOnly = true": { + onlyPlacePostOnly: true, + orders: []types.Order{ + constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5, + }, + expectedOrderPlacementCalls: []types.Order{}, + }, + "does not places PO stateful orders from last block, when onlyPlacePostOnly = false": { + onlyPlacePostOnly: false, + orders: []types.Order{ + constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5_PO, + }, + expectedOrderPlacementCalls: []types.Order{}, + }, + "places non-PO stateful orders from last block when onlyPlacePostOnly = false": { + onlyPlacePostOnly: false, + orders: []types.Order{ + constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5, + }, + expectedOrderPlacementCalls: []types.Order{ + constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5, + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + // Setup state. + memClob := &mocks.MemClob{} + + memClob.On("SetClobKeeper", mock.Anything).Return() + + ks := keepertest.NewClobKeepersTestContext( + t, + memClob, + &mocks.BankKeeper{}, + indexer_manager.NewIndexerEventManagerNoop(), + ) + + ks.MarketMapKeeper.InitGenesis(ks.Ctx, constants.MarketMap_DefaultGenesisState) + prices.InitGenesis(ks.Ctx, *ks.PricesKeeper, constants.Prices_DefaultGenesisState) + perpetuals.InitGenesis(ks.Ctx, *ks.PerpetualsKeeper, constants.Perpetuals_DefaultGenesisState) + + ctx := ks.Ctx.WithBlockHeight(int64(100)).WithBlockTime(time.Unix(5, 0)) + ctx = ctx.WithIsCheckTx(true) + ks.BlockTimeKeeper.SetPreviousBlockInfo(ctx, &blocktimetypes.BlockInfo{ + Height: 100, + Timestamp: time.Unix(int64(2), 0), + }) + + // Create CLOB pair. + memClob.On("CreateOrderbook", constants.ClobPair_Btc).Return() + _, err := ks.ClobKeeper.CreatePerpetualClobPairAndMemStructs( + ctx, + constants.ClobPair_Btc.Id, + clobtest.MustPerpetualId(constants.ClobPair_Btc), + satypes.BaseQuantums(constants.ClobPair_Btc.StepBaseQuantums), + constants.ClobPair_Btc.QuantumConversionExponent, + constants.ClobPair_Btc.SubticksPerTick, + constants.ClobPair_Btc.Status, + ) + require.NoError(t, err) + + // Create each stateful order placement in state + for i, order := range tc.orders { + require.True(t, order.IsStatefulOrder()) + + ks.ClobKeeper.SetLongTermOrderPlacement(ctx.WithIsCheckTx(false), order, uint32(i)) + } + + // Assert expected order placement memclob calls. + for _, order := range tc.expectedOrderPlacementCalls { + memClob.On("PlaceOrder", mock.Anything, order).Return( + satypes.BaseQuantums(0), + types.Success, + constants.TestOffchainUpdates, + nil, + ).Once() + } + + // Run the test and verify expectations. + offchainUpdates := types.NewOffchainUpdates() + orderIds := make([]types.OrderId, 0) + for _, order := range tc.orders { + orderIds = append(orderIds, order.OrderId) + } + ks.ClobKeeper.PlaceStatefulOrdersFromLastBlock(ctx, orderIds, offchainUpdates, tc.onlyPlacePostOnly) + + // PlaceStatefulOrdersFromLastBlock utilizes the memclob's PlaceOrder flow, but we + // do not want to emit PlaceMessages in offchain events for stateful orders. This assertion + // verifies that we call `ClearPlaceMessages()` on the offchain updates before returning. + require.Equal(t, 0, memclobtest.MessageCountOfType(offchainUpdates, types.PlaceMessageType)) + + // Verify that all removed orders have an associated off-chain update. + orderMap := make(map[types.OrderId]bool) + for _, order := range tc.orders { + orderMap[order.OrderId] = true + } + + removedOrders := lib.FilterSlice(tc.expectedOrderPlacementCalls, func(order types.Order) bool { + return !orderMap[order.OrderId] + }) + + for _, order := range removedOrders { + require.True( + t, + memclobtest.HasMessage(offchainUpdates, order.OrderId, types.RemoveMessageType), + ) + } + + memClob.AssertExpectations(t) + }) + } +} + func TestPlaceConditionalOrdersTriggeredInLastBlock(t *testing.T) { tests := map[string]struct { triggeredOrders []types.Order diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go index f69a6227d2..6b38c2f9f8 100644 --- a/protocol/x/clob/memclob/memclob.go +++ b/protocol/x/clob/memclob/memclob.go @@ -925,7 +925,7 @@ func (m *MemClobPriceTimePriority) ReplayOperations( order := operation.GetShortTermOrderPlacement().Order // Skip the order if it is a post-only order and we are only replaying post-only orders. - if onlyPlacePostOnly && !order.IsPostOnlyOrder() { + if onlyPlacePostOnly != order.IsPostOnlyOrder() { continue } @@ -1020,7 +1020,7 @@ func (m *MemClobPriceTimePriority) ReplayOperations( } // Skip the order if it is a post-only order and we are only replaying post-only orders. - if onlyPlacePostOnly && !statefulOrderPlacement.Order.IsPostOnlyOrder() { + if onlyPlacePostOnly != statefulOrderPlacement.Order.IsPostOnlyOrder() { continue } @@ -1078,7 +1078,7 @@ func (m *MemClobPriceTimePriority) ReplayOperations( } // Skip the order if it is a post-only order and we are only replaying post-only orders. - if onlyPlacePostOnly && !statefulOrderPlacement.Order.IsPostOnlyOrder() { + if onlyPlacePostOnly != statefulOrderPlacement.Order.IsPostOnlyOrder() { continue } From 73add8faea34ebcaa3fb1d26f006abfd87447382 Mon Sep 17 00:00:00 2001 From: Jay Yu <103467857+jayy04@users.noreply.github.com> Date: Mon, 2 Dec 2024 12:24:23 -0500 Subject: [PATCH 6/6] comments --- protocol/mocks/MemClob.go | 8 ++++---- protocol/x/clob/keeper/orders.go | 16 +++++++++++----- protocol/x/clob/keeper/orders_test.go | 22 +++++++++++----------- protocol/x/clob/memclob/memclob.go | 17 ++++++++++------- protocol/x/clob/types/memclob.go | 2 +- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/protocol/mocks/MemClob.go b/protocol/mocks/MemClob.go index 93e0eacb2c..60cb6aca69 100644 --- a/protocol/mocks/MemClob.go +++ b/protocol/mocks/MemClob.go @@ -578,9 +578,9 @@ func (_m *MemClob) RemoveOrderIfFilled(ctx types.Context, orderId clobtypes.Orde _m.Called(ctx, orderId) } -// ReplayOperations provides a mock function with given fields: ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates, onlyPlacePostOnly -func (_m *MemClob) ReplayOperations(ctx types.Context, localOperations []clobtypes.InternalOperation, shortTermOrderTxBytes map[clobtypes.OrderHash][]byte, existingOffchainUpdates *clobtypes.OffchainUpdates, onlyPlacePostOnly bool) *clobtypes.OffchainUpdates { - ret := _m.Called(ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates, onlyPlacePostOnly) +// ReplayOperations provides a mock function with given fields: ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates, postOnlyFilter +func (_m *MemClob) ReplayOperations(ctx types.Context, localOperations []clobtypes.InternalOperation, shortTermOrderTxBytes map[clobtypes.OrderHash][]byte, existingOffchainUpdates *clobtypes.OffchainUpdates, postOnlyFilter bool) *clobtypes.OffchainUpdates { + ret := _m.Called(ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates, postOnlyFilter) if len(ret) == 0 { panic("no return value specified for ReplayOperations") @@ -588,7 +588,7 @@ func (_m *MemClob) ReplayOperations(ctx types.Context, localOperations []clobtyp var r0 *clobtypes.OffchainUpdates if rf, ok := ret.Get(0).(func(types.Context, []clobtypes.InternalOperation, map[clobtypes.OrderHash][]byte, *clobtypes.OffchainUpdates, bool) *clobtypes.OffchainUpdates); ok { - r0 = rf(ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates, onlyPlacePostOnly) + r0 = rf(ctx, localOperations, shortTermOrderTxBytes, existingOffchainUpdates, postOnlyFilter) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*clobtypes.OffchainUpdates) diff --git a/protocol/x/clob/keeper/orders.go b/protocol/x/clob/keeper/orders.go index 3316ff0948..eba66f9f69 100644 --- a/protocol/x/clob/keeper/orders.go +++ b/protocol/x/clob/keeper/orders.go @@ -487,6 +487,9 @@ func (k Keeper) AddPreexistingStatefulOrder( // PlaceStatefulOrdersFromLastBlock validates and places stateful orders from the last block onto the memclob. // Note that stateful orders could fail to be placed due to various reasons such as collateralization // check failures, self-trade errors, etc. In these cases the `checkState` will not be written to. +// Note that this function also takes in a postOnlyFilter variable and only places post-only orders if +// postOnlyFilter is true and non-post-only orders if postOnlyFilter is false. +// // This function is used in: // 1. `PrepareCheckState` to place newly placed long term orders from the last // block from ProcessProposerMatchesEvents.PlacedStatefulOrderIds. This is step 3 in PrepareCheckState. @@ -496,7 +499,7 @@ func (k Keeper) PlaceStatefulOrdersFromLastBlock( ctx sdk.Context, placedStatefulOrderIds []types.OrderId, existingOffchainUpdates *types.OffchainUpdates, - onlyPlacePostOnly bool, + postOnlyFilter bool, ) ( offchainUpdates *types.OffchainUpdates, ) { @@ -523,8 +526,8 @@ func (k Keeper) PlaceStatefulOrdersFromLastBlock( order := orderPlacement.GetOrder() - // Skip the order if it is a post-only order and we are only placing post-only orders. - if onlyPlacePostOnly != order.IsPostOnlyOrder() { + // Skip post-only orders if postOnlyFilter is false or non-post-only orders if postOnlyFilter is true. + if postOnlyFilter != order.IsPostOnlyOrder() { continue } @@ -582,11 +585,14 @@ func (k Keeper) PlaceStatefulOrdersFromLastBlock( // PlaceConditionalOrdersTriggeredInLastBlock takes in a list of conditional order ids that were triggered // in the last block, verifies they are conditional orders, verifies they are in triggered state, and places // the orders on the memclob. +// +// Note that this function also takes in a postOnlyFilter variable and only places post-only orders if +// postOnlyFilter is true and non-post-only orders if postOnlyFilter is false. func (k Keeper) PlaceConditionalOrdersTriggeredInLastBlock( ctx sdk.Context, conditionalOrderIdsTriggeredInLastBlock []types.OrderId, existingOffchainUpdates *types.OffchainUpdates, - onlyPlacePostOnly bool, + postOnlyFilter bool, ) ( offchainUpdates *types.OffchainUpdates, ) { @@ -620,7 +626,7 @@ func (k Keeper) PlaceConditionalOrdersTriggeredInLastBlock( ctx, conditionalOrderIdsTriggeredInLastBlock, existingOffchainUpdates, - onlyPlacePostOnly, + postOnlyFilter, ) } diff --git a/protocol/x/clob/keeper/orders_test.go b/protocol/x/clob/keeper/orders_test.go index be55fe27d4..09d2025918 100644 --- a/protocol/x/clob/keeper/orders_test.go +++ b/protocol/x/clob/keeper/orders_test.go @@ -2250,13 +2250,13 @@ func TestPlaceStatefulOrdersFromLastBlock(t *testing.T) { func TestPlaceStatefulOrdersFromLastBlock_PostOnly(t *testing.T) { tests := map[string]struct { - orders []types.Order - onlyPlacePostOnly bool + orders []types.Order + postOnlyFilter bool expectedOrderPlacementCalls []types.Order }{ - "places PO stateful orders from last block when onlyPlacePostOnly = true": { - onlyPlacePostOnly: true, + "places PO stateful orders from last block when postOnlyFilter = true": { + postOnlyFilter: true, orders: []types.Order{ constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5_PO, }, @@ -2264,22 +2264,22 @@ func TestPlaceStatefulOrdersFromLastBlock_PostOnly(t *testing.T) { constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5_PO, }, }, - "does not places non-PO stateful orders when onlyPlacePostOnly = true": { - onlyPlacePostOnly: true, + "does not places non-PO stateful orders when postOnlyFilter = true": { + postOnlyFilter: true, orders: []types.Order{ constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5, }, expectedOrderPlacementCalls: []types.Order{}, }, - "does not places PO stateful orders from last block, when onlyPlacePostOnly = false": { - onlyPlacePostOnly: false, + "does not places PO stateful orders from last block, when postOnlyFilter = false": { + postOnlyFilter: false, orders: []types.Order{ constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5_PO, }, expectedOrderPlacementCalls: []types.Order{}, }, - "places non-PO stateful orders from last block when onlyPlacePostOnly = false": { - onlyPlacePostOnly: false, + "places non-PO stateful orders from last block when postOnlyFilter = false": { + postOnlyFilter: false, orders: []types.Order{ constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT5, }, @@ -2350,7 +2350,7 @@ func TestPlaceStatefulOrdersFromLastBlock_PostOnly(t *testing.T) { for _, order := range tc.orders { orderIds = append(orderIds, order.OrderId) } - ks.ClobKeeper.PlaceStatefulOrdersFromLastBlock(ctx, orderIds, offchainUpdates, tc.onlyPlacePostOnly) + ks.ClobKeeper.PlaceStatefulOrdersFromLastBlock(ctx, orderIds, offchainUpdates, tc.postOnlyFilter) // PlaceStatefulOrdersFromLastBlock utilizes the memclob's PlaceOrder flow, but we // do not want to emit PlaceMessages in offchain events for stateful orders. This assertion diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go index 6b38c2f9f8..953d0d758f 100644 --- a/protocol/x/clob/memclob/memclob.go +++ b/protocol/x/clob/memclob/memclob.go @@ -874,12 +874,15 @@ func (m *MemClobPriceTimePriority) matchOrder( // - Pre-existing stateful orders. // - Stateful cancelations. // Note that match operations are no-op. +// +// Note that this function also takes in a postOnlyFilter variable and only places post-only orders if +// postOnlyFilter is true and non-post-only orders if postOnlyFilter is false. func (m *MemClobPriceTimePriority) ReplayOperations( ctx sdk.Context, localOperations []types.InternalOperation, shortTermOrderTxBytes map[types.OrderHash][]byte, existingOffchainUpdates *types.OffchainUpdates, - onlyPlacePostOnly bool, + postOnlyFilter bool, ) *types.OffchainUpdates { lib.AssertCheckTxMode(ctx) @@ -924,8 +927,8 @@ func (m *MemClobPriceTimePriority) ReplayOperations( case *types.InternalOperation_ShortTermOrderPlacement: order := operation.GetShortTermOrderPlacement().Order - // Skip the order if it is a post-only order and we are only replaying post-only orders. - if onlyPlacePostOnly != order.IsPostOnlyOrder() { + // Skip post-only orders if postOnlyFilter is false or non-post-only orders if postOnlyFilter is true. + if postOnlyFilter != order.IsPostOnlyOrder() { continue } @@ -1019,8 +1022,8 @@ func (m *MemClobPriceTimePriority) ReplayOperations( continue } - // Skip the order if it is a post-only order and we are only replaying post-only orders. - if onlyPlacePostOnly != statefulOrderPlacement.Order.IsPostOnlyOrder() { + // Skip post-only orders if postOnlyFilter is false or non-post-only orders if postOnlyFilter is true. + if postOnlyFilter != statefulOrderPlacement.Order.IsPostOnlyOrder() { continue } @@ -1077,8 +1080,8 @@ func (m *MemClobPriceTimePriority) ReplayOperations( continue } - // Skip the order if it is a post-only order and we are only replaying post-only orders. - if onlyPlacePostOnly != statefulOrderPlacement.Order.IsPostOnlyOrder() { + // Skip post-only orders if postOnlyFilter is false or non-post-only orders if postOnlyFilter is true. + if postOnlyFilter != statefulOrderPlacement.Order.IsPostOnlyOrder() { continue } diff --git a/protocol/x/clob/types/memclob.go b/protocol/x/clob/types/memclob.go index ab63fe5fca..2c55eba839 100644 --- a/protocol/x/clob/types/memclob.go +++ b/protocol/x/clob/types/memclob.go @@ -110,7 +110,7 @@ type MemClob interface { localOperations []InternalOperation, shortTermOrderTxBytes map[OrderHash][]byte, existingOffchainUpdates *OffchainUpdates, - onlyPlacePostOnly bool, + postOnlyFilter bool, ) (offchainUpdates *OffchainUpdates) SetMemclobGauges( ctx sdk.Context,