Store stateful order expirations individually in state #1875
Conversation
WalkthroughThe changes modify handling of stateful orders expiration in a clob protocol, replacing functions and altering interfaces for streamlined management. They adjust Changes
Sequence Diagram(s)State Order Expiration Handling - Before ChangessequenceDiagram
participant User
participant ClobKeeper
participant TimeSliceStore
participant ExpirationHandler
User->>ClobKeeper: Place Order
ClobKeeper->>TimeSliceStore: MustAddOrderToStatefulOrdersTimeSlice
TimeSliceStore-->>ClobKeeper: Order Added
ClobKeeper-->>User: Order Placed
ClobKeeper-->>ExpirationHandler: RemoveExpiredStatefulOrdersTimeSlices
ExpirationHandler->>TimeSliceStore: Fetch Expired TimeSlice
TimeSliceStore-->>ExpirationHandler: Return TimeSlice
ExpirationHandler-->>ClobKeeper: Orders Removed
State Order Expiration Handling - After ChangessequenceDiagram
participant User
participant ClobKeeper
participant KVStore
participant ExpirationHandler
User->>ClobKeeper: Place Order
ClobKeeper->>KVStore: AddStatefulOrderIdExpiration
KVStore-->>ClobKeeper: Order Added
ClobKeeper-->>User: Order Placed
ClobKeeper-->>ExpirationHandler: RemoveExpiredStatefulOrders
ExpirationHandler->>KVStore: Fetch Expired Orders
KVStore-->>ExpirationHandler: Return Expired Orders
ExpirationHandler-->>ClobKeeper: Orders Removed
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (20)
- protocol/x/clob/abci.go (1 hunks)
- protocol/x/clob/abci_test.go (8 hunks)
- protocol/x/clob/e2e/app_test.go (3 hunks)
- protocol/x/clob/e2e/long_term_orders_test.go (1 hunks)
- protocol/x/clob/keeper/clob_pair_test.go (1 hunks)
- protocol/x/clob/keeper/msg_server_cancel_orders_test.go (1 hunks)
- protocol/x/clob/keeper/msg_server_place_order_test.go (1 hunks)
- protocol/x/clob/keeper/order_cancellation_test.go (8 hunks)
- protocol/x/clob/keeper/orders.go (1 hunks)
- protocol/x/clob/keeper/orders_test.go (3 hunks)
- protocol/x/clob/keeper/process_operations_test.go (2 hunks)
- protocol/x/clob/keeper/process_proposer_matches_events.go (6 hunks)
- protocol/x/clob/keeper/process_proposer_matches_events_test.go (2 hunks)
- protocol/x/clob/keeper/stateful_order_state.go (5 hunks)
- protocol/x/clob/keeper/stateful_order_state_test.go (17 hunks)
- protocol/x/clob/keeper/stores.go (1 hunks)
- protocol/x/clob/types/clob_keeper.go (2 hunks)
- protocol/x/clob/types/keys.go (1 hunks)
- protocol/x/clob/types/keys_test.go (1 hunks)
- protocol/x/clob/types/mem_clob_keeper.go (2 hunks)
Files skipped from review due to trivial changes (3)
- protocol/x/clob/keeper/stores.go
- protocol/x/clob/types/clob_keeper.go
- protocol/x/clob/types/keys_test.go
Additional comments not posted (61)
protocol/x/clob/types/keys.go (2)
56-58: Deprecation Notice: LegacyStatefulOrdersTimeSlicePrefixThe
LegacyStatefulOrdersTimeSlicePrefixis marked as deprecated. Ensure this prefix is not used in new code.
60-62: New Key Prefix: StatefulOrdersExpirationsKeyPrefixA new key prefix
StatefulOrdersExpirationsKeyPrefixhas been introduced. Ensure this is used consistently across the codebase.protocol/x/clob/keeper/process_proposer_matches_events.go (17)
8-8: New Import: storetypesThe
storetypespackage is imported. Ensure it is used correctly and consistently.
75-76: Store Instance: memStoreThe
memStoreinstance is correctly retrieved usingctx.KVStore(k.memKey).
79-85: Function Call: ResetOrderedOrderIdsThe
ResetOrderedOrderIdsfunction now takes an additionalstoreparameter. Ensure all calls to this function are updated accordingly.
96-96: Function Call: AppendOrderedOrderIdThe
AppendOrderedOrderIdfunction now takes an additionalstoreparameter. Ensure all calls to this function are updated accordingly.
108-108: Function Call: AppendOrderedOrderIdThe
AppendOrderedOrderIdfunction now takes an additionalstoreparameter. Ensure all calls to this function are updated accordingly.
118-118: Function Call: SetUnorderedOrderIdThe
SetUnorderedOrderIdfunction now takes an additionalstoreparameter. Ensure all calls to this function are updated accordingly.
123-123: Function Call: GetOrderIdsThe
GetOrderIdsfunction now takes an additionalstoreparameter. Ensure all calls to this function are updated accordingly.
128-128: Function Call: GetOrderIdsThe
GetOrderIdsfunction now takes an additionalstoreparameter. Ensure all calls to this function are updated accordingly.
133-133: Function Call: GetOrderIdsThe
GetOrderIdsfunction now takes an additionalstoreparameter. Ensure all calls to this function are updated accordingly.
139-139: Function Call: HasUnorderedOrderIdThe
HasUnorderedOrderIdfunction now takes an additionalstoreparameter. Ensure all calls to this function are updated accordingly.
142-150: Function Update: ResetOrderedOrderIdsThe
ResetOrderedOrderIdsfunction now takes an additionalstoreparameter. Ensure the new implementation correctly resets the ordered order IDs.
153-157: Function Update: ResetUnorderedOrderIdsThe
ResetUnorderedOrderIdsfunction now takes an additionalstoreparameter. Ensure the new implementation correctly resets the unordered order IDs.
163-178: Function Update: AppendOrderedOrderIdThe
AppendOrderedOrderIdfunction now takes an additionalstoreparameter. Ensure the new implementation correctly appends the ordered order IDs.
Line range hint
183-193:
Function Update: GetOrderIdsThe
GetOrderIdsfunction now takes an additionalstoreparameter. Ensure the new implementation correctly retrieves the order IDs.
199-204: Function Update: HasUnorderedOrderIdThe
HasUnorderedOrderIdfunction now takes an additionalstoreparameter. Ensure the new implementation correctly checks the existence of unordered order IDs.
209-214: Function Update: SetUnorderedOrderIdThe
SetUnorderedOrderIdfunction now takes an additionalstoreparameter. Ensure the new implementation correctly sets the unordered order IDs.
217-224: New Function: RemoveUnorderedOrderIdThe
RemoveUnorderedOrderIdfunction is a new addition. Ensure this function is used correctly and consistently across the codebase.protocol/x/clob/keeper/process_proposer_matches_events_test.go (6)
200-201: Store Instance: memStoreThe
memStoreinstance is correctly retrieved usingks.Ctx.KVStore(ks.MemKey).
203-205: Function Call: AppendOrderedOrderIdThe
AppendOrderedOrderIdfunction now takes an additionalmemStoreparameter. Ensure all calls to this function are updated accordingly.
207-211: Function Calls: ResetOrderedOrderIds and AppendOrderedOrderIdThe
ResetOrderedOrderIdsandAppendOrderedOrderIdfunctions now take an additionalmemStoreparameter. Ensure all calls to these functions are updated accordingly.
241-242: Store Instance: memStoreThe
memStoreinstance is correctly retrieved usingks.Ctx.KVStore(ks.MemKey).
244-246: Function Call: SetUnorderedOrderIdThe
SetUnorderedOrderIdfunction now takes an additionalmemStoreparameter. Ensure all calls to this function are updated accordingly.
248-252: Function Calls: ResetOrderedOrderIds and SetUnorderedOrderIdThe
ResetOrderedOrderIdsandSetUnorderedOrderIdfunctions now take an additionalmemStoreparameter. Ensure all calls to these functions are updated accordingly.protocol/x/clob/keeper/msg_server_cancel_orders_test.go (1)
229-229: LGTM! Ensure the correctness of the new function usage.The new function
AddStatefulOrderIdExpirationis correctly used. Ensure that it behaves as expected in all test cases.protocol/x/clob/abci.go (1)
64-64: LGTM! Ensure the correctness of the new function usage.The new function
RemoveExpiredStatefulOrdersis correctly used. Ensure that it behaves as expected in all scenarios.protocol/x/clob/keeper/stateful_order_state.go (5)
178-188: LGTM! Ensure the correctness of the new function.The new function
GetStatefulOrderIdExpirationsis correctly implemented. Ensure that it behaves as expected in all scenarios.
190-203: LGTM! Ensure the correctness of the new function.The new function
RemoveStatefulOrderIdExpirationis correctly implemented. Ensure that it behaves as expected in all scenarios.
205-218: LGTM! Ensure the correctness of the new function.The new function
AddStatefulOrderIdExpirationis correctly implemented. Ensure that it behaves as expected in all scenarios.
220-238: LGTM! Ensure the correctness of the new function.The new function
RemoveExpiredStatefulOrdersis correctly implemented. Ensure that it behaves as expected in all scenarios.
326-326: LGTM! Ensure the correctness of the new function usage.The new function
RemoveStatefulOrderIdExpirationis correctly used. Ensure that it behaves as expected in all scenarios.protocol/x/clob/keeper/order_cancellation_test.go (2)
149-149: LGTM! Ensure the correctness of the new function usage.The new function
AddStatefulOrderIdExpirationis correctly used. Ensure that it behaves as expected in all test cases.
260-260: LGTM! Ensure the correctness of the new function usage.The new function
AddStatefulOrderIdExpirationis correctly used. Ensure that it behaves as expected in all test cases.Also applies to: 289-289, 310-310, 330-330, 351-351, 371-371, 391-391
protocol/x/clob/keeper/msg_server_place_order_test.go (1)
209-211: Ensure proper cleanup of stateful order ID expirations.The addition of stateful order ID expirations is correctly implemented. However, ensure that these expirations are properly cleaned up after each test case to avoid state leakage between tests.
protocol/x/clob/e2e/app_test.go (2)
367-369: Ensure proper handling of stateful order ID expirations.The addition of stateful order ID expirations is correctly implemented. Ensure that the expirations are properly managed and cleaned up to avoid state inconsistencies.
443-445: Ensure proper handling of stateful order ID expirations.The addition of stateful order ID expirations is correctly implemented. Ensure that the expirations are properly managed and cleaned up to avoid state inconsistencies.
Also applies to: 455-457
protocol/x/clob/keeper/stateful_order_state_test.go (9)
41-44: LGTM!The changes align with the new function name
AddStatefulOrderIdExpiration.
Line range hint
476-517:
LGTM!The changes align with the new function name
AddStatefulOrderIdExpiration.
607-613: LGTM!The changes align with the new function name
AddStatefulOrderIdExpiration.
654-658: LGTM!The changes align with the new function name
AddStatefulOrderIdExpiration.
671-675: LGTM!The changes align with the new function name
AddStatefulOrderIdExpiration.
Line range hint
706-716:
LGTM!The changes align with the new function name
GetStatefulOrderIdExpirations.
732-741: LGTM!The changes align with the new function name
RemoveExpiredStatefulOrders.
848-853: LGTM!The changes align with the new function name
AddStatefulOrderIdExpiration.
Line range hint
860-865:
LGTM!The changes align with the new function name
RemoveExpiredStatefulOrders.protocol/x/clob/keeper/clob_pair_test.go (1)
666-666: Verify the usage ofAddStatefulOrderIdExpiration.Ensure that the new function
AddStatefulOrderIdExpirationis used correctly and integrates well with the rest of the code. Verify that it correctly handles the order ID expirations as intended.protocol/x/clob/keeper/orders.go (1)
401-403: Ensure proper handling of stateful order expiration.The addition of
AddStatefulOrderIdExpirationlooks correct. Ensure that theorder.MustGetUnixGoodTilBlockTime()andorder.GetOrderId()are correctly handled and validated before this call.protocol/x/clob/abci_test.go (7)
Line range hint
85-89: Correctness: Verify the function call replacement.The function call
MustAddOrderToStatefulOrdersTimeSlicehas been replaced withAddStatefulOrderIdExpiration. Ensure that the new function is correctly handling the stateful order ID expiration.
Line range hint
249-253: Correctness: Verify the function call replacement.The function call
MustAddOrderToStatefulOrdersTimeSlicehas been replaced withAddStatefulOrderIdExpiration. Ensure that the new function is correctly handling the stateful order ID expiration.
292-296: Correctness: Verify the function call replacement.The function call
MustAddOrderToStatefulOrdersTimeSlicehas been replaced withAddStatefulOrderIdExpiration. Ensure that the new function is correctly handling the stateful order ID expiration.
Line range hint
480-484: Correctness: Verify the function call replacement.The function call
MustAddOrderToStatefulOrdersTimeSlicehas been replaced withAddStatefulOrderIdExpiration. Ensure that the new function is correctly handling the stateful order ID expiration.
Line range hint
496-500: Correctness: Verify the function call replacement.The function call
MustAddOrderToStatefulOrdersTimeSlicehas been replaced withAddStatefulOrderIdExpiration. Ensure that the new function is correctly handling the stateful order ID expiration.
Line range hint
522-526: Correctness: Verify the function call replacement.The function call
MustAddOrderToStatefulOrdersTimeSlicehas been replaced withAddStatefulOrderIdExpiration. Ensure that the new function is correctly handling the stateful order ID expiration.
811-812: Correctness: Verify the function call replacement.The function call
GetStatefulOrdersTimeSlicehas been replaced withGetStatefulOrderIdExpirations. Ensure that the new function is correctly handling the stateful order ID expirations.protocol/x/clob/e2e/long_term_orders_test.go (1)
1782-1783: Ensure correct usage ofAddStatefulOrderIdExpiration.The added code segment correctly uses
AddStatefulOrderIdExpirationto add the order ID to the expiration state. Ensure that the expiration time and order ID are correctly managed in the context.protocol/x/clob/keeper/orders_test.go (3)
863-865: Verify correctness of function replacement.The function
MustAddOrderToStatefulOrdersTimeSlicehas been replaced withAddStatefulOrderIdExpiration. Ensure that the new function correctly handles the addition of stateful order ID expirations.
884-886: Verify correctness of function replacement.The function
MustAddOrderToStatefulOrdersTimeSlicehas been replaced withAddStatefulOrderIdExpiration. Ensure that the new function correctly handles the addition of stateful order ID expirations.
906-907: Verify correctness of function replacement.The function
GetStatefulOrdersTimeSlicehas been replaced withGetStatefulOrderIdExpirations. Ensure that the new function correctly retrieves stateful order ID expirations.protocol/x/clob/keeper/process_operations_test.go (2)
2474-2476: Correctly replaced function call.The function
MustAddOrderToStatefulOrdersTimeSlicehas been correctly replaced withAddStatefulOrderIdExpiration.
Line range hint
2486-2490:
Correctly replaced function call and added necessary trigger.The function
MustAddOrderToStatefulOrdersTimeSlicehas been correctly replaced withAddStatefulOrderIdExpiration, and the necessary trigger for the conditional order has been added.
| // Increment the byte representation of `/` at the end of the prefix so that blockTime is included in the iteration | ||
| end[len(end)-1] += 1 |
There was a problem hiding this comment.
can this overflow? not sure what formatted time string this is using
there is a util function in sdk store types, which might be better
// InclusiveEndBytes returns the []byte that would end a
// range query such that the input would be included
func InclusiveEndBytes(inclusiveBytes []byte) []byte {
return append(inclusiveBytes, byte(0x00))
}
There was a problem hiding this comment.
Since the key prefix is Exp/%s:, if you do that, you'd get Exp/{time}:\00 as the exclusive end, which would not cover all the keys that the orders are stored it. For example `"Exp/{time}:{order_id}" would not get iterated over.
The idea is to increment the byte representation of : by 1 so that all order ids at the ending time would be included.
If you can think of better ways of doing this, I'm open to it.
There was a problem hiding this comment.
Thinking out loud - could we define another constant StatefulOrdersExpirationsKeyPrefixExclusiveEnd = "Exp/%s;", and do end := []byte(fmt.Sprintf(types.StatefulOrdersExpirationsKeyPrefixExclusiveEnd, sdk.FormatTimeString(blockTime)))
There was a problem hiding this comment.
Yeah that's pretty much the same thing though.
There was a problem hiding this comment.
there is also another function that we can use called PrefixEndBytes
// PrefixEndBytes returns the []byte that would end a
// range query for all []byte with a certain prefix
// Deals with last byte of prefix being FF without overflowing
func PrefixEndBytes(prefix []byte) []byte {
if len(prefix) == 0 {
return nil
}
end := make([]byte, len(prefix))
copy(end, prefix)
for {
if end[len(end)-1] != byte(255) {
end[len(end)-1]++
break
}
end = end[:len(end)-1]
if len(end) == 0 {
end = nil
break
}
}
return end
}
| memStore := ctx.KVStore(k.memKey) | ||
| store := prefix.NewStore(memStore, []byte(keyPrefix)) | ||
| it := store.Iterator(nil, nil) | ||
| func (k Keeper) ResetUnorderedOrderIds(ctx sdk.Context, store storetypes.KVStore, keyPrefix string) { |
There was a problem hiding this comment.
For my understanding what's the motivation behind changing this interface? Do we plan to call this with any store other than memstore?
There was a problem hiding this comment.
I'm using it with persisted KVStore in this PR.
| @@ -1216,12 +795,6 @@ func TestRemoveExpiredStatefulOrdersTimeSlices(t *testing.T) { | |||
| }, | |||
There was a problem hiding this comment.
Can we add a test where multiple orders under the inclusive end blocktime are all removed?
There was a problem hiding this comment.
Modified the existing test to cover this
| store := ctx.KVStore(k.storeKey) | ||
| start := []byte(fmt.Sprintf(types.StatefulOrdersExpirationsKeyPrefix, sdk.FormatTimeString(time.Time{}))) | ||
| end := []byte(fmt.Sprintf(types.StatefulOrdersExpirationsKeyPrefix, sdk.FormatTimeString(blockTime))) | ||
| // Increment the byte representation of `/` at the end of the prefix so that blockTime is included in the iteration |
There was a problem hiding this comment.
| // Increment the byte representation of `/` at the end of the prefix so that blockTime is included in the iteration | |
| // Increment the byte representation of `:` at the end of the prefix so that blockTime is included in the iteration |
Do you mean this, given the key prefix format is Exp/%s:?
| // GetStatefulOrdersTimeSlice gets a slice of stateful order IDs that expire at `goodTilBlockTime`, | ||
| // sorted by order ID. | ||
| func (k Keeper) GetStatefulOrdersTimeSlice(ctx sdk.Context, goodTilBlockTime time.Time) ( | ||
| // GetStatefulOrderIdExpirations gets a slice stateful order IDs that expire at `goodTilBlockTime`, |
There was a problem hiding this comment.
| // GetStatefulOrderIdExpirations gets a slice stateful order IDs that expire at `goodTilBlockTime`, | |
| // GetStatefulOrderIdExpirations gets a slice of stateful order IDs that expire at `goodTilBlockTime`, |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (20)
- protocol/x/clob/abci.go (1 hunks)
- protocol/x/clob/abci_test.go (8 hunks)
- protocol/x/clob/e2e/app_test.go (3 hunks)
- protocol/x/clob/e2e/long_term_orders_test.go (1 hunks)
- protocol/x/clob/keeper/clob_pair_test.go (1 hunks)
- protocol/x/clob/keeper/msg_server_cancel_orders_test.go (1 hunks)
- protocol/x/clob/keeper/msg_server_place_order_test.go (1 hunks)
- protocol/x/clob/keeper/order_cancellation_test.go (8 hunks)
- protocol/x/clob/keeper/orders.go (1 hunks)
- protocol/x/clob/keeper/orders_test.go (3 hunks)
- protocol/x/clob/keeper/process_operations_test.go (2 hunks)
- protocol/x/clob/keeper/process_proposer_matches_events.go (6 hunks)
- protocol/x/clob/keeper/process_proposer_matches_events_test.go (2 hunks)
- protocol/x/clob/keeper/stateful_order_state.go (5 hunks)
- protocol/x/clob/keeper/stateful_order_state_test.go (17 hunks)
- protocol/x/clob/keeper/stores.go (1 hunks)
- protocol/x/clob/types/clob_keeper.go (2 hunks)
- protocol/x/clob/types/keys.go (1 hunks)
- protocol/x/clob/types/keys_test.go (1 hunks)
- protocol/x/clob/types/mem_clob_keeper.go (2 hunks)
Files not summarized due to errors (1)
- protocol/x/clob/keeper/stateful_order_state_test.go: Error: Message exceeds token limit
Files skipped from review due to trivial changes (2)
- protocol/x/clob/keeper/msg_server_cancel_orders_test.go
- protocol/x/clob/types/mem_clob_keeper.go
Files skipped from review as they are similar to previous changes (12)
- protocol/x/clob/abci.go
- protocol/x/clob/e2e/app_test.go
- protocol/x/clob/keeper/clob_pair_test.go
- protocol/x/clob/keeper/msg_server_place_order_test.go
- protocol/x/clob/keeper/order_cancellation_test.go
- protocol/x/clob/keeper/orders_test.go
- protocol/x/clob/keeper/process_proposer_matches_events.go
- protocol/x/clob/keeper/process_proposer_matches_events_test.go
- protocol/x/clob/keeper/stateful_order_state.go
- protocol/x/clob/keeper/stores.go
- protocol/x/clob/types/clob_keeper.go
- protocol/x/clob/types/keys.go
Additional comments not posted (16)
protocol/x/clob/keeper/orders.go (1)
401-403: Ensure correct expiration handling for stateful orders.The addition of
AddStatefulOrderIdExpirationappears to correctly handle the expiration of stateful orders by adding them to the expiration list. Ensure that theorder.MustGetUnixGoodTilBlockTime()andorder.GetOrderId()return the correct values and are consistent with the rest of the system.protocol/x/clob/abci_test.go (7)
85-88: Ensure correctness ofAddStatefulOrderIdExpirationusage.The function
AddStatefulOrderIdExpirationis correctly used to add stateful order ID expirations. Ensure that this change aligns with the new expiration management logic.
249-252: Ensure correctness ofAddStatefulOrderIdExpirationusage.The function
AddStatefulOrderIdExpirationis correctly used to add stateful order ID expirations. Ensure that this change aligns with the new expiration management logic.
292-295: Ensure correctness ofAddStatefulOrderIdExpirationusage.The function
AddStatefulOrderIdExpirationis correctly used to add stateful order ID expirations. Ensure that this change aligns with the new expiration management logic.
480-483: Ensure correctness ofAddStatefulOrderIdExpirationusage.The function
AddStatefulOrderIdExpirationis correctly used to add stateful order ID expirations. Ensure that this change aligns with the new expiration management logic.
496-499: Ensure correctness ofAddStatefulOrderIdExpirationusage.The function
AddStatefulOrderIdExpirationis correctly used to add stateful order ID expirations. Ensure that this change aligns with the new expiration management logic.
522-525: Ensure correctness ofAddStatefulOrderIdExpirationusage.The function
AddStatefulOrderIdExpirationis correctly used to add stateful order ID expirations. Ensure that this change aligns with the new expiration management logic.
811-814: Ensure correctness ofAddStatefulOrderIdExpirationusage.The function
AddStatefulOrderIdExpirationis correctly used to add stateful order ID expirations. Ensure that this change aligns with the new expiration management logic.protocol/x/clob/e2e/long_term_orders_test.go (1)
1782-1782: Ensure proper handling of stateful order expirations.The addition of
AddStatefulOrderIdExpirationis appropriate for managing stateful order expirations. Verify that the expiration logic aligns with the new stateful order management implementation.protocol/x/clob/keeper/process_operations_test.go (2)
2474-2475: LGTM! Ensure proper order expiration tracking.The addition of
AddStatefulOrderIdExpirationensures that the order expiration is tracked correctly.
2486-2487: LGTM! Ensure proper order expiration tracking.The addition of
AddStatefulOrderIdExpirationensures that the order expiration is tracked correctly.protocol/x/clob/keeper/stateful_order_state_test.go (5)
41-41: LGTM!The function
createPartiallyFilledStatefulOrderInStatecorrectly callsAddStatefulOrderIdExpiration.
476-476: Well-structured test cases.The test function
TestGetAddAndRemoveStatefulOrderExpirationsis comprehensive and covers various scenarios effectively.
732-732: Well-structured test cases.The test function
TestRemoveExpiredStatefulOrdersis comprehensive and covers various scenarios effectively.
793-793: Comprehensive test case.The test case "Deletes all time slices before blockTime inclusive" ensures correct removal of all time slices before and including the block time.
805-805: Comprehensive test case.The test case "Does not delete time slices after blockTime" ensures that time slices after the block time are not deleted.
| constants.ConditionalOrder_Alice_Num1_Id1_Clob0_Sell50_Price5_GTB30.OrderId, | ||
| }, | ||
| constants.Time_21st_Feb_2021.Add(77): { | ||
| constants.LongTermOrder_Alice_Num1_Id1_Clob0_Sell25_Price30_GTBT10.OrderId, | ||
| constants.LongTermOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT20.OrderId, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Consider adding a test for multiple orders under the same block time.
Adding a test where multiple orders under the inclusive end block time are all removed would enhance coverage.
Do you want me to add this test case or open a GitHub issue to track this task?
Changelist
Test Plan
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
Refactor
Tests
Chores