[OTE-529] feat(OE): Deprecate in-mem Untriggered Conditional Orders #1879
Conversation
WalkthroughThe recent updates enhance the management of conditional orders within the system by adding new order configurations, removing logic for expired orders, and improving how untriggered conditional orders are organized. These changes streamline order processing, optimize memory usage, and facilitate more flexible trading strategies for users. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant System
participant OrderManager
participant MemoryStore
User->>System: Place Conditional Order
System->>OrderManager: Add Conditional Order
OrderManager->>MemoryStore: Store Conditional Order
MemoryStore-->>OrderManager: Confirm Storage
Note over OrderManager: On specific event
OrderManager->>MemoryStore: Retrieve Conditional Orders
MemoryStore-->>OrderManager: Provide Orders
OrderManager->>System: Trigger Conditional Orders
System->>User: Confirmation
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 Configuration File (
|
| // trigger in the same block they are placed. Skip triggering orders which have been cancelled | ||
| // or expired. | ||
| // TODO(CLOB-773) Support conditional order replacements. Ensure replacements are de-duplicated. | ||
| conditionalOrdersIds := keeper.GetDeliveredConditionalOrderIds(ctx) |
There was a problem hiding this comment.
Don't think GetDeliveredConditionalOrderIds is used for anything else, so we can remove it as well
teddyding
force-pushed
the
td/deprecate-mem-uco
branch
from
0c451ef to
40cbc21
Compare
| @@ -235,8 +235,6 @@ func TestEndBlocker_Success(t *testing.T) { | |||
| orderToPrune1 := constants.ConditionalOrder_Alice_Num0_Id0_Clob0_Buy5_Price10_GTBT15_StopLoss20 | |||
| orderToPrune2 := constants.ConditionalOrder_Alice_Num0_Id1_Clob0_Buy15_Price25_GTBT15_StopLoss25 | |||
| orderToPrune3 := constants.ConditionalOrder_Alice_Num0_Id0_Clob1_Buy5_Price10_GTBT15_TakeProfit20 | |||
| // cancelled order | |||
| orderToPrune4 := constants.ConditionalOrder_Alice_Num1_Id1_Clob0_Sell50_Price5_GTB30_TakeProfit20 | |||
There was a problem hiding this comment.
EndBlocker doesn't require this information anymore
![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 (10)
- protocol/testutil/constants/stateful_orders.go (5 hunks)
- protocol/x/clob/abci.go (1 hunks)
- protocol/x/clob/abci_test.go (6 hunks)
- protocol/x/clob/keeper/clob_pair_test.go (3 hunks)
- protocol/x/clob/keeper/final_settlement.go (1 hunks)
- protocol/x/clob/keeper/keeper.go (3 hunks)
- protocol/x/clob/keeper/orders.go (1 hunks)
- protocol/x/clob/keeper/orders_test.go (1 hunks)
- protocol/x/clob/keeper/untriggered_conditional_orders.go (5 hunks)
- protocol/x/clob/keeper/untriggered_conditional_orders_test.go (4 hunks)
Files skipped from review due to trivial changes (3)
- protocol/x/clob/abci.go
- protocol/x/clob/keeper/clob_pair_test.go
- protocol/x/clob/keeper/orders_test.go
Additional comments not posted (17)
protocol/x/clob/keeper/final_settlement.go (2)
Line range hint
9-12:
LGTM!The function correctly forcefully cancels all stateful orders for the given clob pair.
Line range hint
15-61:
LGTM!The function correctly cancels stateful orders, appends to
RemovedStatefulOrderIds, and adds events to the indexer event manager.protocol/x/clob/keeper/keeper.go (2)
31-31: Removal ofMemClobfield.Ensure that the removal of the
MemClobfield does not affect other parts of the codebase that might rely on it.
94-113: LGTM!The initialization logic correctly sets up the
Keeperstruct without theMemClobfield.protocol/x/clob/keeper/untriggered_conditional_orders.go (1)
178-196: LGTM!The function correctly organizes untriggered conditional orders read from state and returns them in the expected format.
protocol/x/clob/keeper/untriggered_conditional_orders_test.go (4)
90-90: LGTM!The test cases correctly cover the scenarios for adding untriggered conditional orders.
126-216: LGTM!The test cases correctly cover the scenarios for organizing untriggered conditional orders from state.
285-285: LGTM!The test cases correctly cover the scenarios for removing untriggered conditional orders.
285-285: LGTM!The test cases correctly cover the scenarios for polling triggered conditional orders.
protocol/x/clob/abci_test.go (3)
310-324: Ensure correct initialization of untriggered conditional orders.The added lines initialize a list of untriggered conditional orders. Verify that these orders are correctly set up and reflect the intended test scenario.
327-328: Verify order placement in the state.The loop places each untriggered conditional order in the state. Ensure that this setup is necessary for the test case and that the orders are correctly placed.
747-752: Check the organization of untriggered conditional orders.The
OrganizeUntriggeredConditionalOrdersFromStatefunction is used to organize untriggered conditional orders from the state. Verify that this function is correctly used and that the expected and actual results are accurately compared.Also applies to: 756-756
protocol/testutil/constants/stateful_orders.go (5)
459-472: LGTM!The
ConditionalOrder_Alice_Num1_Id0_Clob0_Buy5_Price20_GTBT15_StopLoss20order is correctly defined.
571-584: LGTM!The
ConditionalOrder_Alice_Num1_Id1_Clob0_Buy15_Price25_GTBT15_StopLoss25order is correctly defined.
655-668: LGTM!The
ConditionalOrder_Alice_Num1_Id2_Clob0_Sell20_Price20_GTBT15_TakeProfit20order is correctly defined.
711-724: LGTM!The
ConditionalOrder_Alice_Num1_Id3_Clob0_Buy25_Price25_GTBT15_StopLoss25order is correctly defined.
781-794: LGTM!The
ConditionalOrder_Alice_Num0_Id1_Clob1_Buy5_Price10_GTBT15_StopLoss20order is correctly defined.
teddyding
changed the title
| @@ -64,94 +64,6 @@ func assertFillAmountAndPruneState( | |||
| } | |||
| } | |||
|
|
|||
| func TestEndBlocker_Failure(t *testing.T) { | |||
There was a problem hiding this comment.
Tested scenario no longer applies
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- protocol/x/clob/abci_test.go (7 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/x/clob/abci_test.go
teddyding
force-pushed
the
td/deprecate-mem-uco
branch
from
45b3363 to
645a161
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- protocol/testutil/constants/stateful_orders.go (5 hunks)
- protocol/x/clob/abci.go (1 hunks)
- protocol/x/clob/abci_test.go (7 hunks)
- protocol/x/clob/keeper/clob_pair_test.go (3 hunks)
- protocol/x/clob/keeper/final_settlement.go (1 hunks)
- protocol/x/clob/keeper/keeper.go (3 hunks)
- protocol/x/clob/keeper/orders.go (1 hunks)
- protocol/x/clob/keeper/orders_test.go (1 hunks)
- protocol/x/clob/keeper/untriggered_conditional_orders.go (5 hunks)
- protocol/x/clob/keeper/untriggered_conditional_orders_test.go (4 hunks)
Files skipped from review due to trivial changes (1)
- protocol/x/clob/keeper/clob_pair_test.go
Files skipped from review as they are similar to previous changes (9)
- protocol/testutil/constants/stateful_orders.go
- protocol/x/clob/abci.go
- protocol/x/clob/abci_test.go
- protocol/x/clob/keeper/final_settlement.go
- protocol/x/clob/keeper/keeper.go
- protocol/x/clob/keeper/orders.go
- protocol/x/clob/keeper/orders_test.go
- protocol/x/clob/keeper/untriggered_conditional_orders.go
- protocol/x/clob/keeper/untriggered_conditional_orders_test.go
teddyding
force-pushed
the
td/deprecate-mem-uco
branch
from
645a161 to
46451c3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (6)
protocol/x/clob/keeper/untriggered_conditional_orders.go (2)
178-179: Improve function comment.The function comment should be more descriptive and grammatically correct.
-// OrganizeUntriggeredConditionalOrdersFromState takes in a list of conditional orders read from -// state, organize them and return in form of `UntriggeredConditionalOrders` struct. +// OrganizeUntriggeredConditionalOrdersFromState takes a list of conditional orders read from +// the state, organizes them, and returns them in the form of a `UntriggeredConditionalOrders` struct.
197-197: Update function comment.The function comment should be updated to reflect the new logic of organizing untriggered conditional orders.
-// any conditional orders in `UntriggeredConditionalOrders` that can be triggered. For each triggered +// any conditional orders that can be triggered. For each triggeredprotocol/x/clob/keeper/untriggered_conditional_orders_test.go (1)
126-126: Fix typo in test case names.There is a typo in the test case names. It should be
ClobPairinstead ofClobPair.- "Only GTE orders, one ClobPair": { + "Only GTE orders, one ClobPair": {protocol/x/clob/abci_test.go (3)
239-239: Remove commented-out code.The TODO comment indicates that the test assertions should be replaced with new verifications on the operations queue. Ensure that the commented-out code is removed once the new verifications are implemented.
239-239: Ensure consistent naming conventions.The function name
TestPrepareCheckState_WithProcessProposerMatchesEventsWithBadBlockHeightis quite long. Consider using a more concise and consistent naming convention for better readability.
239-239: Ensure consistent naming conventions.The function name
TestCommitBlocker_WithProcessProposerMatchesEventsWithBadBlockHeightis quite long. Consider using a more concise and consistent naming convention for better readability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- protocol/testutil/constants/stateful_orders.go (5 hunks)
- protocol/x/clob/abci.go (1 hunks)
- protocol/x/clob/abci_test.go (7 hunks)
- protocol/x/clob/keeper/clob_pair_test.go (3 hunks)
- protocol/x/clob/keeper/final_settlement.go (1 hunks)
- protocol/x/clob/keeper/keeper.go (3 hunks)
- protocol/x/clob/keeper/orders.go (1 hunks)
- protocol/x/clob/keeper/orders_test.go (1 hunks)
- protocol/x/clob/keeper/untriggered_conditional_orders.go (5 hunks)
- protocol/x/clob/keeper/untriggered_conditional_orders_test.go (4 hunks)
Files skipped from review due to trivial changes (1)
- protocol/x/clob/keeper/clob_pair_test.go
Additional comments not posted (17)
protocol/x/clob/keeper/final_settlement.go (1)
Line range hint
10-13:
LGTM!The code changes are approved.
protocol/x/clob/abci.go (3)
Line range hint
16-19:
LGTM!The code changes are approved.
Line range hint
22-31:
LGTM!The code changes are approved.
Line range hint
34-80:
Verify the correctness of the new approach.The removal of logic related to untriggered conditional orders simplifies the function, but it's important to ensure that the new approach is correctly integrated.
protocol/x/clob/keeper/keeper.go (3)
93-114: Verify the correctness of the new approach.The removal of
UntriggeredConditionalOrdersinitialization simplifies the function, but it's important to ensure that the new approach is correctly integrated.
30-32: Verify the correctness of the new approach.The removal of
UntriggeredConditionalOrderssimplifies the struct, but it's important to ensure that the new approach is correctly integrated.
Line range hint
142-160:
Verify the correctness of the new approach.The removal of the call to
k.HydrateUntriggeredConditionalOrderssimplifies the function, but it's important to ensure that the new approach is correctly integrated.Verification successful
The search results indicate that
UntriggeredConditionalOrdersis still being referenced in multiple files, suggesting that it has not been completely deprecated. To ensure the correctness of the new approach, we need to verify if the removal ofk.HydrateUntriggeredConditionalOrdersfrom theInitializeForGenesisfunction has been compensated by other initializations or if there are any alternative mechanisms in place.Let's search for the usage of
HydrateUntriggeredConditionalOrdersand related initializations within the codebase to understand the impact of its removal.
Verify the correctness of the new approach.
The removal of
k.HydrateUntriggeredConditionalOrdershas been confirmed, and no alternative initialization or related mechanism was found in theInitializeForGenesisfunction or elsewhere in the codebase. This indicates that the functionality has been deprecated or is no longer required.
protocol/x/clob/keeper/keeper.go:InitializeForGenesisfunction does not initializeUntriggeredConditionalOrders.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new approach for handling conditional orders. # Test: Search for the usage of `UntriggeredConditionalOrders` to ensure it has been completely deprecated. rg --type go 'UntriggeredConditionalOrders'Length of output: 8133
Script:
#!/bin/bash # Description: Verify the impact of the removal of `k.HydrateUntriggeredConditionalOrders` by checking for its usage and related initializations. # Test: Search for the usage of `HydrateUntriggeredConditionalOrders` to understand its impact. rg --type go 'HydrateUntriggeredConditionalOrders' # Test: Search for the initialization of `UntriggeredConditionalOrders` to check for any alternative mechanisms. rg --type go 'InitializeForGenesis'Length of output: 3181
protocol/x/clob/keeper/untriggered_conditional_orders_test.go (1)
90-90: LGTM!The changes improve the test setup by using
NewUntriggeredConditionalOrdersdirectly.protocol/x/clob/keeper/orders.go (1)
Line range hint
1-1: Verify new mechanism for managing untriggered conditional orders.The removal of
HydrateUntriggeredConditionalOrdersindicates a change in how untriggered conditional orders are managed. Ensure that the new mechanism is thoroughly tested and documented.protocol/x/clob/abci_test.go (2)
222-236: Ensure comprehensive test coverage for all scenarios.The test cases for
TestEndBlocker_Successseem to cover various scenarios, but it's crucial to ensure that all edge cases are tested, especially with the removal ofTestEndBlocker_Failure. Consider adding more test cases if any edge cases are missed.
239-239: Ensure comprehensive test coverage for all scenarios.The test cases for
TestBeginBlocker_Successseem to cover various scenarios, but it's crucial to ensure that all edge cases are tested. Consider adding more test cases if any edge cases are missed.protocol/testutil/constants/stateful_orders.go (5)
459-472: LGTM!The new conditional order
ConditionalOrder_Alice_Num1_Id0_Clob0_Buy5_Price20_GTBT15_StopLoss20is consistent with the existing order structure and parameters.
571-584: LGTM!The new conditional order
ConditionalOrder_Alice_Num1_Id1_Clob0_Buy15_Price25_GTBT15_StopLoss25is consistent with the existing order structure and parameters.
655-668: LGTM!The new conditional order
ConditionalOrder_Alice_Num1_Id2_Clob0_Sell20_Price20_GTBT15_TakeProfit20is consistent with the existing order structure and parameters.
711-724: LGTM!The new conditional order
ConditionalOrder_Alice_Num1_Id3_Clob0_Buy25_Price25_GTBT15_StopLoss25is consistent with the existing order structure and parameters.
781-794: LGTM!The new conditional order
ConditionalOrder_Alice_Num0_Id1_Clob1_Buy5_Price10_GTBT15_StopLoss20is consistent with the existing order structure and parameters.protocol/x/clob/keeper/orders_test.go (1)
Line range hint
15-15:
Ensure adequate test coverage for untriggered conditional orders.The removal of the
TestHydrateUntriggeredConditionalOrdersInMemClobfunction suggests a change in the testing strategy or a refactor in how untriggered conditional orders are managed. Ensure that the functionality covered by this test is still adequately tested elsewhere.Verification successful
Ensure adequate test coverage for untriggered conditional orders.
The removal of the
TestHydrateUntriggeredConditionalOrdersInMemClobfunction suggests a change in the testing strategy or a refactor in how untriggered conditional orders are managed. Ensure that the functionality covered by this test is still adequately tested elsewhere.Based on the results from the shell script, it appears that there are multiple test cases related to untriggered conditional orders in other files, such as
protocol/x/clob/abci_test.goandprotocol/x/clob/keeper/stateful_order_state_test.go. These tests seem to cover various aspects of untriggered conditional orders, including their organization, removal, and triggering.
File:
protocol/x/clob/abci_test.go
- Tests related to expected untriggered conditional orders and their handling.
File:
protocol/x/clob/keeper/stateful_order_state_test.go
- Tests for reading stateful orders and ensuring untriggered conditional orders are returned correctly.
File:
protocol/x/clob/keeper/untriggered_conditional_orders_test.go
- Tests for organizing, adding, and removing untriggered conditional orders.
These findings suggest that the functionality related to untriggered conditional orders is still being tested in other parts of the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the functionality covered by the removed test is still adequately tested. # Test: Search for tests related to untriggered conditional orders. Expect: Tests covering the relevant functionality. rg --type go -A 5 'UntriggeredConditionalOrders'Length of output: 28670
| } | ||
|
|
||
| // mustCancelStatefulOrdersForFinalSettlement forcefully cancels all stateful orders |
There was a problem hiding this comment.
Reminder: Address the TODO comment.
The TODO comment indicates a potential improvement to iterate over stateful orders for only the specified clob pair.
Do you want me to generate the code to address this TODO or open a GitHub issue to track this task?
teddyding
force-pushed
the
td/deprecate-mem-uco
branch
from
46451c3 to
a5fd89d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
protocol/x/clob/keeper/untriggered_conditional_orders_test.go (1)
Add Edge Cases to
TestRemoveUntriggeredConditionalOrdersThe current test cases are comprehensive but do not explicitly cover some important edge cases. Please consider adding the following edge cases to ensure robustness:
No orders to remove.
Orders with the same ID.
Orders with invalid IDs.
Orders with boundary conditions (e.g., minimum and maximum values).
Analysis chain
Line range hint
217-285:
LGTM! Ensure edge cases are covered.The test cases are comprehensive and validate the removal logic effectively.
Verify that all edge cases are covered appropriately.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify edge cases in TestRemoveUntriggeredConditionalOrders function. # Test: Search for edge cases in TestRemoveUntriggeredConditionalOrders function. Expect: Proper coverage. rg --type go -A 5 'func TestRemoveUntriggeredConditionalOrders' | rg 'edge cases'Length of output: 83
Script:
#!/bin/bash # Description: Verify edge cases in TestRemoveUntriggeredConditionalOrders function. # Test: Search for specific edge cases in TestRemoveUntriggeredConditionalOrders function. # Check for test cases with no orders to remove rg --type go -A 10 'func TestRemoveUntriggeredConditionalOrders' | rg -i 'no orders' # Check for test cases with orders having the same ID rg --type go -A 10 'func TestRemoveUntriggeredConditionalOrders' | rg -i 'same id' # Check for test cases with orders having invalid IDs rg --type go -A 10 'func TestRemoveUntriggeredConditionalOrders' | rg -i 'invalid id' # Check for test cases with boundary conditions (e.g., min and max values) rg --type go -A 10 'func TestRemoveUntriggeredConditionalOrders' | rg -i 'boundary|min|max'Length of output: 353
Script:
#!/bin/bash # Description: Extract and review test cases in TestRemoveUntriggeredConditionalOrders function to identify implicit coverage of edge cases. # Extract the entire TestRemoveUntriggeredConditionalOrders function for review rg --type go -A 100 'func TestRemoveUntriggeredConditionalOrders' | rg -v '^\s*$'Length of output: 10636
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- protocol/testutil/constants/stateful_orders.go (5 hunks)
- protocol/x/clob/abci.go (1 hunks)
- protocol/x/clob/abci_test.go (7 hunks)
- protocol/x/clob/keeper/clob_pair_test.go (3 hunks)
- protocol/x/clob/keeper/final_settlement.go (1 hunks)
- protocol/x/clob/keeper/keeper.go (3 hunks)
- protocol/x/clob/keeper/orders.go (1 hunks)
- protocol/x/clob/keeper/orders_test.go (1 hunks)
- protocol/x/clob/keeper/untriggered_conditional_orders.go (5 hunks)
- protocol/x/clob/keeper/untriggered_conditional_orders_test.go (4 hunks)
Files skipped from review as they are similar to previous changes (4)
- protocol/testutil/constants/stateful_orders.go
- protocol/x/clob/abci.go
- protocol/x/clob/keeper/final_settlement.go
- protocol/x/clob/keeper/keeper.go
Additional comments not posted (10)
protocol/x/clob/keeper/untriggered_conditional_orders.go (2)
Line range hint
197-215:
LGTM! Verify edge cases and metrics emission.The function is straightforward and performs the necessary state mutations.
Verify that all edge cases are handled appropriately and metrics are emitted correctly.
Line range hint
197-218:
LGTM! Ensure proper error handling.The function is well-organized and utilizes the new
OrganizeUntriggeredConditionalOrdersFromStatefunction effectively.Verify that all error handling scenarios are covered appropriately.
protocol/x/clob/keeper/untriggered_conditional_orders_test.go (3)
Line range hint
286-486:
LGTM! Ensure edge cases are covered.The test cases are comprehensive and validate the polling logic effectively.
Verify that all edge cases are covered appropriately.
Verification successful
Test cases cover edge cases effectively.
The
TestPollTriggeredConditionalOrdersfunction includes comprehensive test cases, including critical edge cases related to rounding behavior.
- No conditional orders triggered
- Trigger GTE orders
- Trigger LTE orders
- Edge cases involving rounding (e.g., rounding down to not trigger, rounding up to not trigger)
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify edge cases in TestPollTriggeredConditionalOrders function. # Test: Search for edge cases in TestPollTriggeredConditionalOrders function. Expect: Proper coverage. rg --type go -A 5 'func TestPollTriggeredConditionalOrders' | rg 'edge cases'Length of output: 79
Script:
#!/bin/bash # Extract the entire TestPollTriggeredConditionalOrders function to verify edge case coverage. ast-grep --lang go --pattern 'func TestPollTriggeredConditionalOrders($_, $_) { $$$ }'Length of output: 88
Script:
#!/bin/bash # Extract the TestPollTriggeredConditionalOrders function to verify edge case coverage. rg --type go -A 100 'func TestPollTriggeredConditionalOrders'Length of output: 12044
126-216: LGTM! Ensure edge cases are covered.The test cases are comprehensive and validate the organization logic effectively.
Verify that all edge cases are covered appropriately.
Line range hint
1-90:
LGTM! Ensure edge cases are covered.The test cases are comprehensive and validate the addition logic effectively.
Verify that all edge cases are covered appropriately.
protocol/x/clob/keeper/clob_pair_test.go (1)
Line range hint
1-486:
LGTM! Ensure edge cases are covered.The test cases are comprehensive and validate the update logic effectively.
Verify that all edge cases are covered appropriately.
protocol/x/clob/keeper/orders.go (1)
Line range hint
15-18:
Verify the removal ofHydrateUntriggeredConditionalOrdersand its impact.The removal of
HydrateUntriggeredConditionalOrderschanges how untriggered conditional orders are managed. Ensure that the new mechanism for handling untriggered conditional orders is in place and properly documented.Verification successful
Verify the removal of
HydrateUntriggeredConditionalOrdersand its impact.The function
HydrateUntriggeredConditionalOrdershas been removed, and the new mechanism for handling untriggered conditional orders is in place. The system now uses functions likeMaybeTriggerConditionalOrdersand theUntriggeredConditionalOrdersdata structure to manage these orders.
- The new mechanism is documented and implemented across various parts of the codebase, ensuring proper handling of untriggered conditional orders.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the removal of `HydrateUntriggeredConditionalOrders` and ensure that the new mechanism for handling untriggered conditional orders is in place. # Test: Search for the function usage. Expect: No occurrences of the old function and presence of new mechanism. rg --type go -A 5 $'HydrateUntriggeredConditionalOrders'Length of output: 55
Script:
#!/bin/bash # Description: Verify the new mechanism for handling untriggered conditional orders. # Search for keywords related to untriggered conditional orders in Go files. rg --type go -i 'untriggered' -A 5 # Check for changes in relevant files that might indicate a new mechanism. git diff HEAD~1 -- protocol/x/clob/keeper/orders.goLength of output: 80814
protocol/x/clob/abci_test.go (2)
222-240: LGTM! Simplified logic for handling untriggered conditional orders.The changes to
TestEndBlocker_Successsimplify the logic by reducing the complexity of handling orders based on their trigger conditions. This improves readability and maintainability.
67-67: Verify the removal ofTestEndBlocker_Failureand its impact.The removal of
TestEndBlocker_Failureindicates that the scenarios it tested are no longer relevant. Ensure that these scenarios are covered by other tests or are no longer applicable.protocol/x/clob/keeper/orders_test.go (1)
Line range hint
1-1:
Significant Test Removal: Ensure Coverage is MaintainedThe removal of the
TestHydrateUntriggeredConditionalOrdersInMemClobfunction indicates a shift in testing strategy or functionality. Ensure that the new system of triggering based on state is adequately tested elsewhere to maintain coverage.
Changelist
Deprecate
UntriggeredConditionalOrdersobject on the ClobKeeper and triggers based on state instead. This is required for OE integration. Since this slightly increasesEndBlockerlatency (see results below), will hold-off on merging until OE is ready to be released. More context hereTest Plan
Tested this implementation on mainnet full node (relevant period, details):
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
New Features
Bug Fixes
Refactor
Tests