Remove unneeded memstores #1631
Conversation
WalkthroughThe recent changes across multiple files in the Changes
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: 1
Outside diff range and nitpick comments (3)
protocol/x/clob/keeper/stores.go (1)
Line range hint
17-17: Address undefined type issues flagged by static analysis.- undefined: Keeper + // Ensure that the Keeper type is defined in this package or imported correctly.The static analysis tool
golangci-linthas flagged that theKeepertype is undefined in several lines. This might be due to missing imports or incorrect package declarations. Ensure that theKeepertype is properly defined or imported to resolve these issues.Also applies to: 26-26, 35-35
Tools
golangci-lint
26-26: undefined: Keeper (typecheck)
protocol/x/clob/keeper/stateful_order_state.go (1)
Line range hint
34-34: Address undefined type issues flagged by static analysis.- undefined: Keeper + // Ensure that the Keeper type is defined in this package or imported correctly.The static analysis tool
golangci-linthas flagged that theKeepertype is undefined in several lines. This might be due to missing imports or incorrect package declarations. Ensure that theKeepertype is properly defined or imported to resolve these issues.Also applies to: 85-85, 102-102
protocol/x/clob/keeper/order_state_test.go (1)
[!TIP]
Codebase VerificationThe
TestSetGetOrderFillAmountfunction includes several test cases that cover different scenarios:
- SetOrderFillAmount then GetOrderFillAmount: Tests if the order fill amount is correctly set and retrieved.
- SetOrderFillAmount twice, GetOrderFillAmount returns the most up-to-date values: Tests if the latest values are retrieved after setting the order fill amount twice.
- GetOrderFillAmount with non-existent OrderFillState: Tests the behavior when trying to get the fill amount of a non-existent order.
These test cases cover basic functionality and some edge cases, such as updating values and handling non-existent states. However, additional edge cases could be considered, such as:
- Setting the order fill amount to zero.
- Handling very large values for the fill amount and prunable block height.
- Concurrent modifications to the order fill amount.
To ensure comprehensive coverage, it would be beneficial to add these additional edge cases.
- Add test cases for setting the order fill amount to zero.
- Add test cases for handling very large values.
- Add test cases for concurrent modifications.
Analysis chain
Line range hint
14-14: Ensure that the test cases forTestSetGetOrderFillAmountare comprehensive and cover edge cases.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure that the test cases for `TestSetGetOrderFillAmount` are comprehensive and cover edge cases. # Test: Search for the test cases covering edge cases. rg --type go 'TestSetGetOrderFillAmount' protocol/x/clob/keeper/order_state_test.goLength of output: 130
Script:
#!/bin/bash # Extract the full implementation of the `TestSetGetOrderFillAmount` function for review. ast-grep --lang go --pattern 'func TestSetGetOrderFillAmount($_) { $$$ }' protocol/x/clob/keeper/order_state_test.goLength of output: 6005
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- protocol/x/clob/abci_test.go (1 hunks)
- protocol/x/clob/keeper/keeper.go (1 hunks)
- protocol/x/clob/keeper/order_state.go (4 hunks)
- protocol/x/clob/keeper/order_state_test.go (5 hunks)
- protocol/x/clob/keeper/orders_test.go (5 hunks)
- protocol/x/clob/keeper/stateful_order_state.go (9 hunks)
- protocol/x/clob/keeper/stateful_order_state_test.go (30 hunks)
- protocol/x/clob/keeper/stores.go (4 hunks)
- protocol/x/clob/types/keys.go (1 hunks)
Files skipped from review due to trivial changes (2)
- protocol/x/clob/keeper/orders_test.go
- protocol/x/clob/types/keys.go
Additional context used
golangci-lint
protocol/x/clob/keeper/stores.go
17-17: undefined: Keeper (typecheck)
26-26: undefined: Keeper (typecheck)
35-35: undefined: Keeper (typecheck)
protocol/x/clob/keeper/stateful_order_state.go
34-34: undefined: Keeper (typecheck)
85-85: undefined: Keeper (typecheck)
102-102: undefined: Keeper (typecheck)
protocol/x/clob/abci_test.go
1392-1392: setupCtx declared and not used (typecheck)
4-4: "errors" imported and not used (typecheck)
Additional comments not posted (19)
protocol/x/clob/keeper/stores.go (1)
105-115: Refactor infetchStateStoresForOrdersimplifies the logic for fetching state stores based on order type.This change aligns with the PR's objective to reduce complexity by removing unnecessary memStore usage. The function now directly returns the appropriate store, making the code cleaner and more maintainable.
protocol/x/clob/keeper/keeper.go (1)
Line range hint
1-300: Changes inkeeper.goenhance application robustness.The modifications, especially in
InitMemStore, prevent re-initialization of the memory store, aligning with best practices and contributing to the stability and predictability of the application's behavior.protocol/x/clob/keeper/order_state.go (1)
Line range hint
86-129: Refactoring inorder_state.goimproves performance and simplifies state management.The use of a regular store instead of a memStore in
SetOrderFillAmount,GetOrderFillAmount, andRemoveOrderFillAmountreduces unnecessary memory operations and aligns with the PR's objectives to simplify the codebase.Also applies to: 249-250
protocol/x/clob/keeper/stateful_order_state.go (1)
58-58: Refactoring instateful_order_state.goaligns with PR objectives to simplify and improve maintainability.The modifications to use regular stores instead of memStores in various functions streamline the code and reduce complexity, making it easier to maintain and less prone to errors.
Also applies to: 83-83, 100-100, 127-127, 148-148, 236-236, 256-256, 262-262
protocol/x/clob/keeper/order_state_test.go (7)
Line range hint
1-1: Ensure the package and imports are correctly organized and necessary.
Line range hint
16-16: Review the logic inTestOrderFillAmountInitMemStore_Successto ensure it accurately tests the initialization of the memstore.Verification successful
The function
TestOrderFillAmountInitMemStore_Successappears to accurately test the initialization of the memstore by setting and verifying order fill amounts before and after initializing the memstore. The test ensures that the values can be read correctly after the memstore has been warmed up.
- The function sets up a mock
MemCloband initializes theClobKeepersTestContextWithUninitializedMemStore.- It sets some fill amounts for specific orders.
- It initializes the memstore using
InitMemStore.- It asserts that the values can be read correctly after the memstore has been initialized.
This confirms that the test is correctly verifying the initialization of the memstore.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Review the logic in `TestOrderFillAmountInitMemStore_Success` to ensure it accurately tests the initialization of the memstore. # Test: Search for the logic testing the initialization of the memstore. rg --type go 'TestOrderFillAmountInitMemStore_Success' protocol/x/clob/keeper/order_state_test.goLength of output: 158
Script:
#!/bin/bash # Extract the full implementation of the `TestOrderFillAmountInitMemStore_Success` function. ast-grep --lang go --pattern $'func TestOrderFillAmountInitMemStore_Success(t *testing.T) { $$$ }' protocol/x/clob/keeper/order_state_test.goLength of output: 4104
Line range hint
18-18: Check the implementation of pruning logic inTestPruningto ensure it handles edge cases and correctly prunes orders.Verification successful
The
TestPruningfunction inprotocol/x/clob/keeper/order_state_test.goincludes multiple test cases that cover various scenarios for pruning orders. These scenarios include:
- Setting a fill amount and prune block, followed by pruning.
- Updating the prunable block height on the
OrderFillState.- Pruning orders for a block height that never had orders.
- Pruning orders for a block before the current block.
- Updating existing orders with new orders to be pruned.
Each test case sets up the context, invokes the pruning logic, and verifies the expected outcomes, including checking if the orders exist and if the prune order keys were deleted for specified heights.
The implementation appears to handle edge cases and correctly prunes orders based on the provided test cases.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the implementation of pruning logic in `TestPruning` to ensure it handles edge cases and correctly prunes orders. # Test: Search for the implementation of pruning logic. rg --type go 'TestPruning' protocol/x/clob/keeper/order_state_test.goLength of output: 102
Script:
#!/bin/bash # Extract the full implementation of the `TestPruning` function from the specified file. ast-grep --lang go --pattern $'func TestPruning($_) { $$$ }' protocol/x/clob/keeper/order_state_test.goLength of output: 16840
Line range hint
22-22: Review the testTestRemoveOrderFillAmountto ensure it adequately tests the removal of order fill amounts and handles potential edge cases.Verification successful
The
TestRemoveOrderFillAmountfunction includes multiple test cases that cover various scenarios related to the removal of order fill amounts. These scenarios include:
- Removing an order fill amount after setting it.
- Removing an order fill amount after setting it twice.
- Attempting to remove a non-existent order fill amount.
- Re-creating an order fill amount after removing it.
- Ensuring that removing an order fill amount does not affect other orders.
These test cases appear to cover both normal operations and edge cases effectively.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Review the test `TestRemoveOrderFillAmount` to ensure it adequately tests the removal of order fill amounts and handles potential edge cases. # Test: Search for the test handling potential edge cases in the removal of order fill amounts. rg --type go 'TestRemoveOrderFillAmount' protocol/x/clob/keeper/order_state_test.goLength of output: 130
Script:
#!/bin/bash # Extract the full implementation of the `TestRemoveOrderFillAmount` function for analysis. ast-grep --lang go --pattern 'func TestRemoveOrderFillAmount($_) { $$$ }' protocol/x/clob/keeper/order_state_test.goLength of output: 13911
Line range hint
20-20: Ensure thatTestMigratePruneableOrderscorrectly tests the migration of pruneable orders and verifies the integrity of the data post-migration.Verification successful
The
TestMigratePruneableOrdersfunction does indeed test the migration of pruneable orders and verifies the integrity of the data post-migration. The test performs the following steps:
- Sets up mock objects and the test context.
- Adds orders to be pruned at different heights.
- Calls the
MigratePruneableOrdersfunction to perform the migration.- Defines a helper function to retrieve orders post-migration.
- Verifies that the orders at specific heights match the expected orders.
- Ensures that the old store no longer contains any valid entries.
This confirms that the test is correctly verifying the migration process and the integrity of the data post-migration.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure that `TestMigratePruneableOrders` correctly tests the migration of pruneable orders and verifies the integrity of the data post-migration. # Test: Search for the test verifying the integrity of the data post-migration. rg --type go 'TestMigratePruneableOrders' protocol/x/clob/keeper/order_state_test.goLength of output: 132
Script:
#!/bin/bash # Extract the full implementation of the `TestMigratePruneableOrders` function from `protocol/x/clob/keeper/order_state_test.go`. ast-grep --lang go --pattern 'func TestMigratePruneableOrders($_) { $$$ }' protocol/x/clob/keeper/order_state_test.goLength of output: 4699
Line range hint
12-12: Check if the test cases cover all relevant scenarios for the functionGetAllOrderFillStates.Verification successful
The
TestGetAllOrderFillStatesfunction inprotocol/x/clob/keeper/order_state_test.goincludes tests for the following scenarios:
- Reading an empty state.
- Reading a single
OrderFillState.- Reading multiple
OrderFillStates.- Writing the same
OrderFillStatemultiple times and ensuring the last update is reflected.These scenarios appear to cover the basic and essential cases for the
GetAllOrderFillStatesfunction. Therefore, the test cases seem comprehensive and cover all relevant scenarios.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the test cases cover all relevant scenarios for the function `GetAllOrderFillStates`. # Test: Search for the test cases covering all relevant scenarios. rg --type go 'GetAllOrderFillStates' protocol/x/clob/keeper/order_state_test.goLength of output: 187
Script:
#!/bin/bash # Extract the implementation of the TestGetAllOrderFillStates function for review. ast-grep --lang go --pattern 'func TestGetAllOrderFillStates($_) { $$$ }' protocol/x/clob/keeper/order_state_test.goLength of output: 8058
Line range hint
10-10: Verify that the test setup is correctly initializing the necessary mocks and contexts.Verification successful
The search results indicate that the test setup in
order_state_test.gois initializing the necessary mocks and contexts usingNewClobKeepersTestContextandNewClobKeepersTestContextWithUninitializedMemStore. These functions are invoked multiple times, passing in mocks such asMemClob,BankKeeper, andIndexerEventManager.However, to fully verify the correctness of the initialization, we need to inspect the actual implementations of
NewClobKeepersTestContextandNewClobKeepersTestContextWithUninitializedMemStore. This will confirm that these functions are correctly setting up the test environment.Let's proceed by retrieving the implementations of these functions.
The previous attempt to retrieve the implementations of
NewClobKeepersTestContextandNewClobKeepersTestContextWithUninitializedMemStoredid not produce any output. This suggests that these functions might be defined in a different way or in a different location than expected.To ensure we cover all possibilities, let's refine our search by looking for these function definitions using
rgto search for the function names across the entire codebase.
The functions
NewClobKeepersTestContextandNewClobKeepersTestContextWithUninitializedMemStoreare defined inprotocol/testutil/keeper/clob.go. To verify that these functions correctly initialize the necessary mocks and contexts, we need to inspect their implementations.Let's retrieve the relevant code from
protocol/testutil/keeper/clob.go.
The implementations of
NewClobKeepersTestContextandNewClobKeepersTestContextWithUninitializedMemStorehave been retrieved. These functions are responsible for setting up the test context, including initializing various keepers and mocks.
NewClobKeepersTestContextcallsNewClobKeepersTestContextWithUninitializedMemStoreand then initializes the memstore.NewClobKeepersTestContextWithUninitializedMemStoresets up the context and initializes various keepers, includingPricesKeeper,PerpetualsKeeper, and others.This confirms that the test setup is correctly initializing the necessary mocks and contexts.
NewClobKeepersTestContextis defined inprotocol/testutil/keeper/clob.goat line 20.NewClobKeepersTestContextWithUninitializedMemStoreis defined inprotocol/testutil/keeper/clob.goat line 40.The test setup appears to be correctly initializing the necessary mocks and contexts.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the test setup is correctly initializing the necessary mocks and contexts. # Test: Search for the initialization of mocks and contexts in the test setup. rg --type go 'NewClobKeepersTestContext' protocol/x/clob/keeper/order_state_test.goLength of output: 386
Script:
#!/bin/bash # Description: Retrieve the implementation of NewClobKeepersTestContext and NewClobKeepersTestContextWithUninitializedMemStore # Test: Search for the definitions of these functions in the codebase. ast-grep --lang go --pattern 'func NewClobKeepersTestContext($_) $_' ast-grep --lang go --pattern 'func NewClobKeepersTestContextWithUninitializedMemStore($_) $_' # Description: Examine the context in which these functions are used in the test file. # Test: Display the lines around the usage of these functions in the test file. rg --context 5 'NewClobKeepersTestContext' protocol/x/clob/keeper/order_state_test.go rg --context 5 'NewClobKeepersTestContextWithUninitializedMemStore' protocol/x/clob/keeper/order_state_test.goLength of output: 2345
Script:
#!/bin/bash # Description: Retrieve the implementation of NewClobKeepersTestContext and NewClobKeepersTestContextWithUninitializedMemStore # Test: Search for the definitions of these functions in the codebase. ast-grep --lang go --pattern 'func NewClobKeepersTestContext($_) $_' ast-grep --lang go --pattern 'func NewClobKeepersTestContextWithUninitializedMemStore($_) $_' # Description: Examine the context in which these functions are used in the test file. # Test: Display the lines around the usage of these functions in the test file. rg --context 5 'NewClobKeepersTestContext' protocol/x/clob/keeper/order_state_test.go rg --context 5 'NewClobKeepersTestContextWithUninitializedMemStore' protocol/x/clob/keeper/order_state_test.goLength of output: 2345
Script:
#!/bin/bash # Description: Retrieve the implementation of NewClobKeepersTestContext and NewClobKeepersTestContextWithUninitializedMemStore # Test: Search for the definitions of these functions in the codebase. ast-grep --lang go --pattern 'func NewClobKeepersTestContext($_) $_' ast-grep --lang go --pattern 'func NewClobKeepersTestContextWithUninitializedMemStore($_) $_'Length of output: 166
Script:
#!/bin/bash # Description: Retrieve the implementation of NewClobKeepersTestContext and NewClobKeepersTestContextWithUninitializedMemStore # Test: Search for the definitions of these functions in the codebase using ripgrep. rg 'func NewClobKeepersTestContext' --type go rg 'func NewClobKeepersTestContextWithUninitializedMemStore' --type goLength of output: 365
Script:
#!/bin/bash # Description: Retrieve the implementation of NewClobKeepersTestContext and NewClobKeepersTestContextWithUninitializedMemStore from clob.go # Test: Display the lines containing the definitions and bodies of these functions. rg --context 20 'func NewClobKeepersTestContext' protocol/testutil/keeper/clob.go rg --context 20 'func NewClobKeepersTestContextWithUninitializedMemStore' protocol/testutil/keeper/clob.goLength of output: 3539
protocol/x/clob/keeper/stateful_order_state_test.go (8)
173-177: Ensure proper store verification logic in conditional order tests.The tests correctly verify the presence and absence of orders in the triggered and untriggered stores respectively, which is crucial for ensuring the integrity of order state transitions.
457-468: Check consistency in stateful order handling.The logic for handling stateful orders, including their creation, replacement, and deletion, appears consistent and adheres to the expected behaviors described in the test scenarios.
Line range hint
591-613: Ensure correct handling of stateful orders in time slices.The test setup and assertions correctly handle the addition of stateful orders to time slices, ensuring that orders are tracked accurately over time.
Line range hint
664-702: Validate removal logic for stateful orders from time slices.The tests adequately cover the scenarios where stateful orders are removed from time slices, ensuring that the system correctly handles order deletions.
Line range hint
773-838: Confirm correct order of stateful orders in non-sorted scenarios.The tests ensure that stateful orders are added to the system in a sorted manner, even when they are created in a non-sorted order, which is crucial for maintaining order integrity.
Line range hint
885-932: Check comprehensive coverage for stateful order deletion scenarios.The tests comprehensively cover various scenarios for deleting stateful orders, ensuring robust handling of order deletions across different conditions.
208-219: Validate key prefix sequence in trace decoder.This script checks if the key prefixes are used correctly throughout the codebase, ensuring that state transitions are logged accurately.
Verification successful
The output from the script shows multiple occurrences of the key prefixes in various files. To verify the key prefix sequence in the trace decoder, we need to ensure that the key prefixes follow the expected sequence and are used correctly in the relevant files.
The expected sequence is:
NextStatefulOrderBlockTransactionIndexKeyUntriggeredConditionalOrderKeyPrefixStatefulOrderCountPrefixTriggeredConditionalOrderKeyPrefixLet's verify the sequence and usage in the relevant files, particularly focusing on
protocol/x/clob/keeper/stateful_order_state_test.goandprotocol/x/clob/keeper/stateful_order_state.go.
The output from the script confirms that the key prefixes are used in the expected sequence in the relevant files. The sequence and usage of the key prefixes in
protocol/x/clob/keeper/stateful_order_state_test.goandprotocol/x/clob/keeper/stateful_order_state.goalign with the expected order:
NextStatefulOrderBlockTransactionIndexKeyUntriggeredConditionalOrderKeyPrefixStatefulOrderCountPrefixTriggeredConditionalOrderKeyPrefixThis confirms that the key prefixes are written in the correct sequence and used appropriately throughout the codebase.
protocol/x/clob/keeper/stateful_order_state_test.go: The sequence is maintained in the test cases.protocol/x/clob/keeper/stateful_order_state.go: The sequence is maintained in the implementation.No issues were found with the key prefix sequence in the trace decoder.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the key prefixes written are in the expected sequence. # Test: Search for the key prefix usage. Expect: Correct sequence and usage. rg --type go $'NextStatefulOrderBlockTransactionIndexKey|UntriggeredConditionalOrderKeyPrefix|StatefulOrderCountPrefix|TriggeredConditionalOrderKeyPrefix'Length of output: 11476
Script:
#!/bin/bash # Verify the sequence and usage of key prefixes in the relevant files. # Check the sequence in stateful_order_state_test.go rg --type go --context 5 'NextStatefulOrderBlockTransactionIndexKey|UntriggeredConditionalOrderKeyPrefix|StatefulOrderCountPrefix|TriggeredConditionalOrderKeyPrefix' protocol/x/clob/keeper/stateful_order_state_test.go # Check the sequence in stateful_order_state.go rg --type go --context 5 'NextStatefulOrderBlockTransactionIndexKey|UntriggeredConditionalOrderKeyPrefix|StatefulOrderCountPrefix|TriggeredConditionalOrderKeyPrefix' protocol/x/clob/keeper/stateful_order_state.goLength of output: 30545
329-389: Review the logic for stateful order count management.This script will help verify that the stateful order count is being managed correctly across different operations, which is critical for maintaining accurate order tracking.
Verification successful
The search results provide a comprehensive view of how
StatefulOrderCountPrefixis used across various files, particularly instateful_order_state_test.go. The results show that the stateful order count is incremented and decremented in multiple test cases, which suggests that the logic for managing the stateful order count is being tested.However, to ensure the correctness of the logic, we need to verify that the increment and decrement operations are correctly implemented in the actual methods, not just in the tests. This involves checking the implementation of methods that handle stateful order count management in the keeper files.
Let's proceed by examining the implementation of these methods in the keeper files to ensure that the stateful order count is managed correctly.
The search results show that the stateful order count management logic is implemented in the
GetUncommittedStatefulOrderCountTransientStoreandGetStatefulOrderCountMemStoremethods inprotocol/x/clob/keeper/stores.go. These methods fetch state stores used for creating, reading, updating, and deleting stateful order counts.Additionally, the test cases in
protocol/x/clob/keeper/stateful_order_state_test.goverify the correct increment and decrement operations for stateful order counts. The test cases cover various scenarios, including adding, replacing, and removing orders, ensuring that the stateful order count is managed correctly.Based on the provided context and the search results, it appears that the stateful order count management logic is correctly implemented and thoroughly tested.
- The methods
GetUncommittedStatefulOrderCountTransientStoreandGetStatefulOrderCountMemStorecorrectly handle the stateful order count.- The test cases in
stateful_order_state_test.goverify the correct increment and decrement operations for stateful order counts.Therefore, the logic for stateful order count management appears to be correctly implemented and tested.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the stateful order count increments and decrements correctly. # Test: Search for the stateful order count management. Expect: Correct increment and decrement operations. rg --type go $'StatefulOrderCountPrefix'Length of output: 4809
Script:
#!/bin/bash # Description: Verify the stateful order count increments and decrements correctly by examining the implementation of relevant methods. # Search for methods using StatefulOrderCountPrefix to verify increment and decrement logic. rg --type go --context 10 'StatefulOrderCountPrefix'Length of output: 65392
Script:
#!/bin/bash # Description: Verify the implementation of methods handling stateful order count management. # Search for methods in keeper files that handle stateful order count management. rg --type go --context 10 'StatefulOrderCountPrefix' protocol/x/clob/keeper/Length of output: 59816
| triggeredConditionalOrderStore := ks.ClobKeeper.GetTriggeredConditionalOrderPlacementStore(ctx) | ||
| untriggeredConditionalOrderStore := ks.ClobKeeper.GetUntriggeredConditionalOrderPlacementStore(ctx) | ||
| exists := triggeredConditionalOrderStore.Has(triggeredConditionalOrderId.ToStateKey()) | ||
| require.True(t, exists) | ||
| exists = untriggeredConditionalOrderMemstore.Has(triggeredConditionalOrderId.ToStateKey()) | ||
| exists = untriggeredConditionalOrderStore.Has(triggeredConditionalOrderId.ToStateKey()) |
There was a problem hiding this comment.
Refactor to encapsulate store access logic.
The operations on triggeredConditionalOrderStore and untriggeredConditionalOrderStore are directly accessing the store's methods. It would be beneficial to encapsulate this logic within the Keeper to maintain abstraction and encapsulation. This would make the codebase easier to maintain and modify in the future.
- triggeredConditionalOrderStore := ks.ClobKeeper.GetTriggeredConditionalOrderPlacementStore(ctx)
- untriggeredConditionalOrderStore := ks.ClobKeeper.GetUntriggeredConditionalOrderPlacementStore(ctx)
- exists := triggeredConditionalOrderStore.Has(triggeredConditionalOrderId.ToStateKey())
- require.True(t, exists)
- exists = untriggeredConditionalOrderStore.Has(triggeredConditionalOrderId.ToStateKey())
- require.False(t, exists)
+ exists, err := ks.ClobKeeper.CheckTriggeredState(ctx, triggeredConditionalOrderId)
+ require.NoError(t, err)
+ require.True(t, exists)
+ exists, err = ks.ClobKeeper.CheckUntriggeredState(ctx, triggeredConditionalOrderId)
+ require.NoError(t, err)
+ require.False(t, exists)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| triggeredConditionalOrderStore := ks.ClobKeeper.GetTriggeredConditionalOrderPlacementStore(ctx) | |
| untriggeredConditionalOrderStore := ks.ClobKeeper.GetUntriggeredConditionalOrderPlacementStore(ctx) | |
| exists := triggeredConditionalOrderStore.Has(triggeredConditionalOrderId.ToStateKey()) | |
| require.True(t, exists) | |
| exists = untriggeredConditionalOrderMemstore.Has(triggeredConditionalOrderId.ToStateKey()) | |
| exists = untriggeredConditionalOrderStore.Has(triggeredConditionalOrderId.ToStateKey()) | |
| exists, err := ks.ClobKeeper.CheckTriggeredState(ctx, triggeredConditionalOrderId) | |
| require.NoError(t, err) | |
| require.True(t, exists) | |
| exists, err = ks.ClobKeeper.CheckUntriggeredState(ctx, triggeredConditionalOrderId) | |
| require.NoError(t, err) | |
| require.False(t, exists) |
Changelist
Test Plan
[Describe how this PR was tested (if applicable)]
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
Bug Fixes
Tests