-
Notifications
You must be signed in to change notification settings - Fork 124
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
cancel existing orders when setting a vault to deactivated or stand-by #2310
Conversation
WalkthroughThe pull request introduces a new method, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Files skipped from review as they are similar to previous changes (4)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 5
Outside diff range and nitpick comments (1)
protocol/x/vault/keeper/params_test.go (1)
Line range hint
269-300
: Refactor repeated assertions into a helper functionThe assertions checking the number of vault orders before and after setting vault parameters are repeated in both the error and success cases. To enhance code maintainability and readability, consider refactoring these assertions into a helper function.
Create a helper function to encapsulate the repeated assertions:
func assertVaultOrderCounts(t *testing.T, ctx sdk.Context, tApp *testapp.TestApplication, k keeper.Keeper, expectedCount int, vaultId vaulttypes.VaultId) { require.Len(t, tApp.App.ClobKeeper.GetAllStatefulOrders(ctx), expectedCount) require.Len(t, k.GetMostRecentClientIds(ctx, vaultId), expectedCount) }Then, replace the repeated assertions with calls to this helper function:
// Before setting vault params assertVaultOrderCounts(t, ctx, tApp, k, int(tc.numVaultOrdersPreSet), tc.vaultId) // After setting vault params (in both error and success cases) assertVaultOrderCounts(t, ctx, tApp, k, int(tc.numVaultOrdersPostSet), tc.vaultId)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- protocol/x/vault/keeper/orders.go (2 hunks)
- protocol/x/vault/keeper/orders_test.go (1 hunks)
- protocol/x/vault/keeper/params.go (2 hunks)
- protocol/x/vault/keeper/params_test.go (5 hunks)
Additional comments not posted (2)
protocol/x/vault/keeper/orders_test.go (2)
748-841
: LGTM!The
TestRefreshVaultClobOrders
function is well-structured and comprehensive. It covers various scenarios for refreshing vault orders, such as status changes, equity changes, and more. The test cases are properly set up, perform the necessary actions, and verify the expected behavior of the refreshed orders.
Line range hint
1-747
: LGTM!The
TestGetVaultClobOrders
function is well-structured and comprehensive. It covers various scenarios for getting vault orders, such as different vault statuses, quoting parameters, leverage, and more. The test cases are properly set up, calculate the expected order details based on the provided formulas and logic, and verify the expected orders against the actual orders retrieved from the vault. The function also covers error scenarios, ensuring the proper handling of edge cases.
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
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
.Summary by CodeRabbit
New Features
Bug Fixes
Tests