-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TRA-195] Add new error code to be returned when orders fail isolated subaccount checks. #1331
[TRA-195] Add new error code to be returned when orders fail isolated subaccount checks. #1331
Conversation
… subaccount checks.
WalkthroughThe recent updates focus on enhancing order handling within a trading protocol, particularly for isolated subaccount orders. New order definitions for specific users were added, alongside improvements in error handling for order collateralization and status violations. A significant addition is the error type Changes
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (8)
- protocol/testutil/constants/orders.go (2 hunks)
- protocol/x/clob/e2e/isolated_subaccount_orders_test.go (7 hunks)
- protocol/x/clob/keeper/orders.go (1 hunks)
- protocol/x/clob/memclob/memclob.go (3 hunks)
- protocol/x/clob/types/errors.go (1 hunks)
- protocol/x/clob/types/orderbook.go (2 hunks)
- protocol/x/clob/types/orderbook_test.go (1 hunks)
- protocol/x/subaccounts/types/update.go (1 hunks)
Additional Context Used
Path-based Instructions (8)
protocol/x/clob/types/orderbook_test.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/subaccounts/types/update.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/types/orderbook.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/types/errors.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/e2e/isolated_subaccount_orders_test.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/keeper/orders.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/testutil/constants/orders.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/memclob/memclob.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
Learnings (1)
protocol/x/clob/memclob/memclob.go (1)
User: jonfung-dydx" PR: dydxprotocol/v4-chain#1009 File: protocol/x/clob/memclob/memclob_place_order_test.go:4108-4108 Timestamp: 2024-02-05T20:01:55.204Z Learning: The `assertPlaceOrderOffchainMessages` function in `memclob_test_util.go` indeed requires `ctx` as a parameter for its operations, contrary to the initial comment made.
Additional comments not posted (10)
protocol/x/clob/types/orderbook_test.go (1)
81-85
: LGTM! The test case for "ViolatesIsolatedSubaccountConstraints" correctly follows the established pattern for testingOrderStatus
string representations.protocol/x/subaccounts/types/update.go (1)
26-28
: LGTM! The methodIsIsolatedSubaccountError()
correctly implements the functionality to check for isolated subaccount errors, following established patterns.protocol/x/clob/types/orderbook.go (1)
185-189
: LGTM! The addition of the constantViolatesIsolatedSubaccountConstraints
to theOrderStatus
enum is correctly implemented and follows established naming conventions.protocol/x/clob/types/errors.go (1)
359-363
: LGTM! The errorErrWouldViolateIsolatedSubaccountContraints
is correctly registered with a unique code and a descriptive message, following established error definition patterns.protocol/x/clob/e2e/isolated_subaccount_orders_test.go (4)
38-46
: Ensure the new order placementPlaceOrder_Alice_Num0_Id0_Clob3_Buy_1ISO_Price10_GTB5_FOK
is correctly used in the test cases to validate the behavior of Fill-or-Kill orders in isolated subaccount scenarios.
55-66
: Verify that the long-term order definitionLongTermOrder_Alice_Num0_Id0_Clob3_Buy_1ISO_Price5_GTBT5
is appropriately utilized in test scenarios to assess the handling of long-term orders in isolated subaccount contexts.
365-366
: Ensure the use ofsdkerrors.ErrPanic.ABCICode()
is appropriate for the test case scenarios where insufficient funds are expected. Consider if a more specific error code should be used to reflect the scenario accurately.
444-445
: Confirm that the test caseSubaccount with isolated perpetual position fails to place stateful order for non-isolated perpetual
correctly asserts the new error codeErrWouldViolateIsolatedSubaccountContraints
and provides a meaningful error message.protocol/testutil/constants/orders.go (1)
403-408
: Consider adding a comment aboveOrder_Bob_Num0_Id0_Clob0_Sell1BTC_Price50000_GTB10
to describe its purpose or scenario it's used for, similar to other orders in this file for consistency and clarity.protocol/x/clob/memclob/memclob.go (1)
2030-2033
: The mapping fromsatypes.UpdateResult
totypes.OrderStatus
inupdateResultToOrderStatus
should be exhaustive to cover all possibleUpdateResult
values. Ensure that all cases are handled appropriately to prevent unexpected behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- protocol/x/clob/memclob/memclob.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/x/clob/memclob/memclob.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (16)
- protocol/mocks/ClobKeeper.go (2 hunks)
- protocol/mocks/MemClobKeeper.go (2 hunks)
- protocol/mocks/mocks/CacheMultiStore.go (1 hunks)
- protocol/mocks/mocks/MultiStore.go (1 hunks)
- protocol/testutil/memclob/keeper.go (1 hunks)
- protocol/x/clob/e2e/isolated_subaccount_orders_test.go (7 hunks)
- protocol/x/clob/keeper/orders.go (3 hunks)
- protocol/x/clob/keeper/process_operations.go (1 hunks)
- protocol/x/clob/memclob/memclob.go (6 hunks)
- protocol/x/clob/memclob/memclob_get_order_filled_amount_test.go (1 hunks)
- protocol/x/clob/memclob/memclob_place_order_test.go (2 hunks)
- protocol/x/clob/memclob/memclob_purge_invalid_memclob_state_test.go (1 hunks)
- protocol/x/clob/memclob/memclob_remove_order_test.go (1 hunks)
- protocol/x/clob/types/clob_keeper.go (1 hunks)
- protocol/x/clob/types/errors.go (1 hunks)
- protocol/x/clob/types/mem_clob_keeper.go (1 hunks)
Files skipped from review due to trivial changes (2)
- protocol/mocks/mocks/CacheMultiStore.go
- protocol/mocks/mocks/MultiStore.go
Files skipped from review as they are similar to previous changes (4)
- protocol/x/clob/e2e/isolated_subaccount_orders_test.go
- protocol/x/clob/keeper/orders.go
- protocol/x/clob/memclob/memclob.go
- protocol/x/clob/types/errors.go
Additional Context Used
Path-based Instructions (10)
protocol/x/clob/memclob/memclob_get_order_filled_amount_test.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/types/mem_clob_keeper.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/types/clob_keeper.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/testutil/memclob/keeper.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/mocks/MemClobKeeper.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/memclob/memclob_purge_invalid_memclob_state_test.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/memclob/memclob_remove_order_test.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/keeper/process_operations.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/mocks/ClobKeeper.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/memclob/memclob_place_order_test.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
Learnings (3)
protocol/x/clob/memclob/memclob_purge_invalid_memclob_state_test.go (1)
User: jonfung-dydx" PR: dydxprotocol/v4-chain#1009 File: protocol/x/clob/memclob/memclob_place_order_test.go:4108-4108 Timestamp: 2024-02-05T20:01:55.204Z Learning: The `assertPlaceOrderOffchainMessages` function in `memclob_test_util.go` indeed requires `ctx` as a parameter for its operations, contrary to the initial comment made.
protocol/x/clob/memclob/memclob_remove_order_test.go (1)
User: jonfung-dydx" PR: dydxprotocol/v4-chain#1009 File: protocol/x/clob/memclob/memclob_place_order_test.go:4108-4108 Timestamp: 2024-02-05T20:01:55.204Z Learning: The `assertPlaceOrderOffchainMessages` function in `memclob_test_util.go` indeed requires `ctx` as a parameter for its operations, contrary to the initial comment made.
protocol/x/clob/memclob/memclob_place_order_test.go (1)
User: jonfung-dydx" PR: dydxprotocol/v4-chain#1009 File: protocol/x/clob/memclob/memclob_place_order_test.go:4108-4108 Timestamp: 2024-02-05T20:01:55.204Z Learning: The `assertPlaceOrderOffchainMessages` function in `memclob_test_util.go` indeed requires `ctx` as a parameter for its operations, contrary to the initial comment made.
Additional comments not posted (14)
protocol/x/clob/memclob/memclob_get_order_filled_amount_test.go (1)
49-49
: The change fromAddOrderToOrderbookCollatCheck
toAddOrderToOrderbookSubaccountUpdatesCheck
correctly reflects the PR's objective to enhance error handling for isolated subaccount checks.protocol/x/clob/types/mem_clob_keeper.go (1)
32-32
: RenamingAddOrderToOrderbookCollatCheck
toAddOrderToOrderbookSubaccountUpdatesCheck
in the interface aligns with the PR's goal of enhancing error handling for isolated subaccount checks.protocol/x/clob/types/clob_keeper.go (1)
17-17
: RenamingAddOrderToOrderbookCollatCheck
toAddOrderToOrderbookSubaccountUpdatesCheck
in theClobKeeper
interface is consistent with the PR's objectives to improve error handling for isolated subaccount checks.protocol/testutil/memclob/keeper.go (1)
287-287
: RenamingAddOrderToOrderbookCollatCheck
toAddOrderToOrderbookSubaccountUpdatesCheck
in theFakeMemClobKeeper
struct aligns with the PR's goal of enhancing error handling for isolated subaccount checks.protocol/mocks/MemClobKeeper.go (1)
27-27
: RenamingAddOrderToOrderbookCollatCheck
toAddOrderToOrderbookSubaccountUpdatesCheck
in the autogenerated mock file is consistent with the PR's objectives to improve error handling for isolated subaccount checks.protocol/x/clob/memclob/memclob_purge_invalid_memclob_state_test.go (1)
269-269
: Ensure the new functionality introduced byAddOrderToOrderbookSubaccountUpdatesCheck
is thoroughly tested, particularly focusing on subaccount updates and their impact on order handling.Verification successful
The method
AddOrderToOrderbookSubaccountUpdatesCheck
is referenced in multiple test files, indicating that the functionality it introduces is being tested across different scenarios within the memclob module. This suggests that the review comment regarding the need for thorough testing of the new functionality has been addressed. However, for a comprehensive evaluation of the test coverage and quality, a manual review of these tests is recommended.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if specific tests for AddOrderToOrderbookSubaccountUpdatesCheck exist rg "AddOrderToOrderbookSubaccountUpdatesCheck" --type go | grep -i "test"Length of output: 1009
protocol/x/clob/memclob/memclob_remove_order_test.go (1)
330-330
: Ensure the new functionality introduced byAddOrderToOrderbookSubaccountUpdatesCheck
is thoroughly tested, especially in the context of order removal, to maintain the integrity of the memclob state.protocol/mocks/ClobKeeper.go (1)
27-32
: The renaming and implementation ofAddOrderToOrderbookSubaccountUpdatesCheck
align well with the PR's objectives and Go best practices.protocol/x/clob/memclob/memclob_place_order_test.go (6)
2859-2859
: Ensure that the test case forTestAddOrderToOrderbook_PanicsIfFullyFilled
accurately reflects the expected behavior of the system when an order is already fully filled. It's crucial to verify that the system handles this scenario gracefully without causing unexpected panics in production.
2822-2828
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [2825-2859]
The test cases
TestAddOrderToOrderbook_ErrorPlaceNewFullyFilledOrder
andTestAddOrderToOrderbook_PanicsIfFullyFilled
seem to test similar scenarios but with different outcomes (error vs panic). Ensure consistency in handling fully filled orders across the system to avoid confusion and potential bugs.
2822-2828
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [2825-2859]
For the test cases involving panics (
TestAddOrderToOrderbook_PanicsOnInvalidSide
,TestAddOrderToOrderbook_PanicsOnInvalidClob
,TestUpdateOrderbookStateWithMatchedMakerOrder_PanicsOnInvalidFillAmount
,TestAddOrderToSubaccountOrders_PanicsOnInvalidSide
,TestAddOrderToSubaccountOrders_PanicsOnInvalidClob
), consider usingrequire.PanicsWithValue
to ensure that the panic message is as expected. This adds an additional layer of verification to ensure that the panic is caused by the expected condition.
2822-2828
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [2825-2859]
The test
TestPlaceOrder_Telemetry
verifies telemetry metrics are correctly gathered. Ensure that the telemetry setup and teardown are properly handled to avoid side effects on other tests, especially when running tests in parallel or in a shared environment.
2822-2828
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [2825-2859]
In
TestPlaceOrder_GenerateOffchainUpdatesFalse_NoMessagesSent
, it's good to see a test ensuring no offchain messages are sent whenGenerateOffchainUpdates
is false. Consider also verifying that the state changes (e.g., order book updates) are still correctly applied to ensure the flag only affects offchain messages.
2822-2828
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [2825-2859]
The test
TestPlaceOrder_DuplicateOrder
correctly checks forErrInvalidReplacement
when placing a duplicate order. This is crucial for ensuring order uniqueness in the system. Consider adding a test case for a scenario where a duplicate order is placed with a different size or price to verify that it's treated as a new order or correctly rejected based on the system's rules.Would you like me to generate the additional test case code for handling duplicate orders with different sizes or prices?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
protocol/indexer/shared/types/removal_reason.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
Files selected for processing (5)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/shared/removal_reason.ts (4 hunks)
- proto/dydxprotocol/indexer/shared/removal_reason.proto (1 hunks)
- protocol/indexer/shared/order_removal_reason.go (1 hunks)
- protocol/indexer/shared/order_removal_reason_test.go (1 hunks)
- protocol/x/clob/simulation/place_order.go (1 hunks)
Additional Context Used
Path-based Instructions (5)
proto/dydxprotocol/indexer/shared/removal_reason.proto (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/indexer/shared/order_removal_reason_test.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/indexer/shared/order_removal_reason.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.protocol/x/clob/simulation/place_order.go (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/shared/removal_reason.ts (1)
Pattern
**/**
: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
Additional comments not posted (8)
proto/dydxprotocol/indexer/shared/removal_reason.proto (1)
56-58
: The addition ofORDER_REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS
is correctly implemented and follows the existing naming conventions and numeric ordering.protocol/indexer/shared/order_removal_reason_test.go (1)
38-42
: The addition of the test case forViolatesIsolatedSubaccountConstraints
is correctly implemented, aligning with the structure and expectations of existing test cases.protocol/indexer/shared/order_removal_reason.go (1)
63-64
: The addition of the case forclobtypes.ViolatesIsolatedSubaccountConstraints
in theGetOrderRemovalReason
function is correctly implemented, aligning with the structure and expectations of the switch statement.protocol/x/clob/simulation/place_order.go (1)
177-178
: The addition of the error conditiontypes.ErrWouldViolateIsolatedSubaccountConstraints
in theSimulateMsgPlaceOrder
function is correctly implemented, aligning with the structure and expectations of the switch statement.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/shared/removal_reason.ts (4)
81-86
: The addition ofORDER_REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS
to theOrderRemovalReason
enum is correctly implemented and follows the existing naming conventions and numeric ordering.
170-175
: The addition ofORDER_REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS
to theOrderRemovalReasonSDKType
enum is correctly implemented and follows the existing naming conventions and numeric ordering.
240-243
: The update toorderRemovalReasonFromJSON
function to handleORDER_REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS
is correctly implemented.
297-299
: The update toorderRemovalReasonToJSON
function to handleORDER_REMOVAL_REASON_VIOLATES_ISOLATED_SUBACCOUNT_CONSTRAINTS
is correctly implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (3)
- protocol/mocks/ClobKeeper.go (1 hunks)
- protocol/x/clob/keeper/orders.go (3 hunks)
- protocol/x/clob/types/clob_keeper.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- protocol/mocks/ClobKeeper.go
- protocol/x/clob/keeper/orders.go
- protocol/x/clob/types/clob_keeper.go
@Mergifyio backport release/protocol/v5.x |
✅ Backports have been created
|
Changelist
Updated returned error for FOK / stateful orders when isolated subaccount checks fail.
Added new removal reason for orders that fail the isolated subaccount constraint checks.
Renamed
AddOrderToOrderbookCollatCheck
toAddOrdertoOrderbookSubaccountUpdatesCheck
to indicate the check is more than for collateralization, but also any other constraints on the resulting subaccount updates.Test Plan
Added E2E test cases.
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.