Skip to content
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

have individual vault quoting parameters #1997

Merged
merged 10 commits into from
Aug 2, 2024
Merged

have individual vault quoting parameters #1997

merged 10 commits into from
Aug 2, 2024

Conversation

tqin7
Copy link
Contributor

@tqin7 tqin7 commented Jul 31, 2024

Changelist

New vault parameter behavior

  • Params renamed to QuotingParams
  • x/vault has QuotingParams default_quoting_parameters
  • each vault has QuotingParams quoting_params, which inherits from above by default
  • governance has the ability to set individual vault parameters (will be added in a later PR)
  • users can query individual vault parameters (will be added in a later PR)

Test Plan

TBD

Author/Reviewer Checklist

  • If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
  • If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
  • If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
  • If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
  • If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
  • Manually add any of the following labels: refactor, chore, bug.

Summary by CodeRabbit

  • New Features

    • Introduced a new field for default quoting parameters in the vault's genesis state, enhancing the configuration process.
    • Added new message types to improve management of default quoting parameters within the vault.
    • Updated API methods to specifically handle default quoting parameters.
  • Bug Fixes

    • Updated relevant structures to ensure clarity and functionality regarding the new quoting parameters.
  • Refactor

    • Renamed existing fields to better emphasize the role of quoting parameters in vault operations.
  • Chores

    • Cleaned up deprecated message definitions to streamline the protocol structure.

Copy link

linear bot commented Jul 31, 2024

Copy link
Contributor

coderabbitai bot commented Jul 31, 2024

Walkthrough

The updates to the dydxprotocol vault module enhance clarity and focus on quoting configurations. Key changes include the introduction of default_quoting_params, the deprecation of Params, and the reorganization of related messages. This shift streamlines vault management and improves protocol efficiency, reflecting a targeted approach to handling vault operations while emphasizing the importance of quoting parameters.

Changes

Files Change Summary
proto/dydxprotocol/vault/genesis.proto, protocol/app/msgs/*, indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/* Deprecated Params in favor of default_quoting_params, added new message types, and updated interfaces focusing on quoting parameters.
proto/dydxprotocol/vault/tx.proto, protocol/lib/ante/internal_msg.go Deprecated UpdateParams RPC method, replaced with UpdateDefaultQuotingParams, enhancing clarity in API functionality.
protocol/scripts/genesis/sample_pregenesis.json, protocol/testutil/constants/genesis.go Renamed "params" to "default_quoting_params" for clearer configuration semantics and introduced a null value for "params".

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant VaultService
    participant GenesisState

    Client->>VaultService: Request to initialize vault
    VaultService->>GenesisState: Set default_quoting_params
    GenesisState-->>VaultService: Confirm setup
    VaultService-->>Client: Vault initialized with quoting parameters
Loading

🐰 In the vaults where rabbits dwell,
Quoting params now ring a bell.
With clearer paths and simpler ways,
Our coins will hop through brighter days!
Let's celebrate, oh what a cheer,
For every change brings us near! 🐇✨


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 757cd57 and 3282629.

Files ignored due to path filters (6)
  • protocol/x/vault/types/genesis.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/params.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/query.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/share.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/tx.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/vault.pb.go is excluded by !**/*.pb.go
Files selected for processing (6)
  • proto/dydxprotocol/vault/genesis.proto (1 hunks)
  • proto/dydxprotocol/vault/params.proto (2 hunks)
  • proto/dydxprotocol/vault/query.proto (2 hunks)
  • proto/dydxprotocol/vault/share.proto (1 hunks)
  • proto/dydxprotocol/vault/tx.proto (2 hunks)
  • proto/dydxprotocol/vault/vault.proto (2 hunks)
Additional comments not posted (13)
proto/dydxprotocol/vault/vault.proto (1)

1-2: LGTM!

The VaultType enum and VaultId message are well-defined and provide clear context for vault identification.

Also applies to: 4-5, 7-8

proto/dydxprotocol/vault/share.proto (3)

1-3: LGTM!

The imports and options are correctly specified.

Also applies to: 7-8


9-17: LGTM!

The NumShares message is well-defined and uses custom types appropriately.


19-23: LGTM!

The OwnerShare message is well-defined and correctly references NumShares.

proto/dydxprotocol/vault/genesis.proto (2)

Line range hint 1-2:
LGTM!

The imports and options are correctly specified.

Also applies to: 6-7


13-16: LGTM!

The default_quoting_params field in the GenesisState message is well-defined and ensures that a default quoting configuration is provided.

proto/dydxprotocol/vault/params.proto (2)

Line range hint 8-40:
LGTM! Well-defined QuotingParams message.

The QuotingParams message is well-structured and includes detailed comments for each field. The use of gogoproto options for custom types and non-nullable fields is appropriate.


42-45: LGTM! Well-defined VaultParams message.

The VaultParams message is well-structured and includes a detailed comment for the quoting_params field. The use of gogoproto options for non-nullable fields is appropriate.

proto/dydxprotocol/vault/tx.proto (3)

18-19: LGTM! Well-defined UpdateDefaultQuotingParams RPC method.

The UpdateDefaultQuotingParams RPC method is well-structured and includes a detailed comment explaining its purpose. The change reflects a shift in focus towards managing quoting parameters.


47-54: LGTM! Well-defined MsgUpdateDefaultQuotingParams message.

The MsgUpdateDefaultQuotingParams message is well-structured and includes detailed comments for each field. The use of cosmos.msg.v1.signer and gogoproto options for custom types and non-nullable fields is appropriate.


57-58: LGTM! Well-defined MsgUpdateDefaultQuotingParamsResponse message.

The MsgUpdateDefaultQuotingParamsResponse message is well-structured and includes a detailed comment explaining its purpose.

proto/dydxprotocol/vault/query.proto (2)

9-9: LGTM! Necessary import statement for share.proto.

The import statement for share.proto is necessary to include the definitions used in the file.


40-40: LGTM! Well-defined QueryParamsResponse message.

The QueryParamsResponse message is well-structured and includes a detailed comment explaining its purpose. The change reflects a shift in focus towards managing quoting parameters.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Outside diff range, codebase verification and nitpick comments (2)
proto/dydxprotocol/vault/tx.proto (2)

18-19: Update all references to UpdateParams to UpdateDefaultQuotingParams in the following .proto files:

  • proto/dydxprotocol/stats/tx.proto
  • proto/dydxprotocol/rewards/tx.proto
  • proto/dydxprotocol/perpetuals/tx.proto

Ensure consistency across the codebase to avoid potential issues.

Analysis chain

LGTM! But verify the RPC method usage in the codebase.

The changes to the RPC method name and description accurately reflect the new functionality.

However, ensure that all references to UpdateParams are updated to UpdateDefaultQuotingParams in the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `UpdateParams` are updated to `UpdateDefaultQuotingParams`.

# Test: Search for the RPC method usage. Expect: Only occurrences of the new method name.
rg --type proto -A 5 $'UpdateParams'

Length of output: 65


Script:

#!/bin/bash
# Description: Verify all references to `UpdateParams` are updated to `UpdateDefaultQuotingParams`.

# Test: Search for the RPC method usage. Expect: Only occurrences of the new method name.
rg --glob '*.proto' -A 5 'UpdateParams'

Length of output: 3459


47-54: Update all references to MsgUpdateParams to MsgUpdateDefaultQuotingParams.

The following files still contain references to MsgUpdateParams. Please update them to MsgUpdateDefaultQuotingParams to ensure consistency across the codebase:

  • protocol/x/vault/keeper/msg_server_update_params_test.go
  • protocol/x/vault/keeper/msg_server_update_params.go
  • protocol/x/rewards/types/tx_test.go
  • protocol/x/rewards/types/tx.pb.go
  • protocol/x/rewards/types/tx.go
  • protocol/x/rewards/keeper/msg_server.go
  • protocol/x/rewards/keeper/msg_server_test.go
  • protocol/x/stats/types/tx_test.go
  • protocol/x/stats/types/tx.pb.go
  • protocol/x/stats/types/tx.go
  • protocol/x/stats/keeper/msg_server.go
  • protocol/x/stats/keeper/msg_server_test.go
  • protocol/x/perpetuals/types/message_update_params_test.go
  • protocol/x/perpetuals/types/message_update_params.go
  • protocol/x/perpetuals/types/tx.pb.go
  • protocol/x/perpetuals/types/tx.go
  • protocol/x/perpetuals/keeper/msg_server_update_params.go
  • protocol/x/perpetuals/keeper/msg_server_update_params_test.go
  • protocol/x/feetiers/keeper/msg_server_test.go
  • protocol/x/blocktime/keeper/msg_server_test.go
  • proto/dydxprotocol/rewards/tx.proto
  • proto/dydxprotocol/perpetuals/tx.proto
  • proto/dydxprotocol/stats/tx.proto
  • protocol/scripts/governance/bridge_vesters_set_rewards.py
  • protocol/testing/e2e/gov/sending_test.go
  • protocol/testing/e2e/authz/authz_test.go
  • protocol/testing/e2e/gov/stats_test.go
  • protocol/testing/e2e/gov/rewards_test.go
  • protocol/testing/e2e/gov/feetiers_test.go
  • protocol/testing/e2e/gov/perpetuals_test.go
  • protocol/lib/ante/unsupported_msgs.go
  • protocol/lib/ante/internal_msg_test.go
  • protocol/lib/ante/internal_msg.go
  • protocol/app/msgs/unsupported_msgs.go
  • protocol/app/msgs/internal_msgs_test.go
  • protocol/app/msgs/internal_msgs.go
  • protocol/app/msgs/all_msgs.go
  • protocol/app/msgs/unsupported_msgs_test.go
  • protocol/app/upgrades/v5.0.0/upgrade.go
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/stats/tx.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/stats/tx.rpc.msg.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/tx.rpc.msg.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/tx.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/rewards/tx.rpc.msg.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/rewards/tx.ts
Analysis chain

LGTM! But verify the message usage in the codebase.

The changes to the message name and fields accurately reflect the new functionality.

However, ensure that all references to MsgUpdateParams are updated to MsgUpdateDefaultQuotingParams in the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `MsgUpdateParams` are updated to `MsgUpdateDefaultQuotingParams`.

# Test: Search for the message usage. Expect: Only occurrences of the new message name.
rg --type proto -A 5 $'MsgUpdateParams'

Length of output: 68


Script:

#!/bin/bash
# Description: Verify all references to `MsgUpdateParams` are updated to `MsgUpdateDefaultQuotingParams`.

# Test: Search for the message usage. Expect: Only occurrences of the new message name.
rg -A 5 'MsgUpdateParams'

Length of output: 144212

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3282629 and 23413f8.

Files ignored due to path filters (6)
  • protocol/x/vault/types/genesis.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/params.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/query.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/share.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/tx.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/vault.pb.go is excluded by !**/*.pb.go
Files selected for processing (6)
  • proto/dydxprotocol/vault/genesis.proto (1 hunks)
  • proto/dydxprotocol/vault/params.proto (2 hunks)
  • proto/dydxprotocol/vault/query.proto (2 hunks)
  • proto/dydxprotocol/vault/share.proto (1 hunks)
  • proto/dydxprotocol/vault/tx.proto (2 hunks)
  • proto/dydxprotocol/vault/vault.proto (2 hunks)
Files skipped from review as they are similar to previous changes (5)
  • proto/dydxprotocol/vault/genesis.proto
  • proto/dydxprotocol/vault/params.proto
  • proto/dydxprotocol/vault/query.proto
  • proto/dydxprotocol/vault/share.proto
  • proto/dydxprotocol/vault/vault.proto
Additional comments not posted (2)
proto/dydxprotocol/vault/tx.proto (2)

53-54: LGTM! But verify the field usage in the codebase.

The changes to the field name and type accurately reflect the new functionality.

However, ensure that all references to params are updated to default_quoting_params in the codebase.


57-58: LGTM! But verify the message usage in the codebase.

The changes to the message name accurately reflect the new functionality.

However, ensure that all references to MsgUpdateParamsResponse are updated to MsgUpdateDefaultQuotingParamsResponse in the codebase.

// VaultParams stores individual parameters of a vault.
message VaultParams {
// The quoting parameters specific to this vault.
QuotingParams quoting_params = 1 [ (gogoproto.nullable) = false ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This isn't nullable, how do we know we should use the default? Or is the plan to set it to equal to the default at the start?

Copy link
Contributor Author

@tqin7 tqin7 Jul 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i was thinking about setting this to the default when a vault is created (i.e. first deposited into).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought more about this and simply removed VaultParams proto

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (1)
protocol/testutil/constants/genesis.go (1)

4517-4517: Update required: Inconsistent key usage detected

The key "params" is still being used in multiple JSON files. Please update the key to "default_quoting_params" in the following files to ensure consistency:

  • protocol/x/rewards/testdata/expected_default_genesis.json
  • protocol/testing/delaymsg_config/perpetual_fee_params_msg.json
  • protocol/scripts/genesis/sample_pregenesis.json
  • protocol/testing/containertest/preupgrade_genesis.json
  • protocol/app/testdata/default_genesis_state.json
  • protocol/app/module/testdata/slashing_default_genesis_state.json
Analysis chain

LGTM! But verify the usage of the new key in the codebase.

The renaming of "params" to "default_quoting_params" enhances clarity and maintainability.

However, ensure that all references to the old key "params" are updated to "default_quoting_params" throughout the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to the old key "params" are updated to "default_quoting_params".

# Test: Search for the old key usage. Expect: No occurrences of the old key.
rg --type json -A 5 $'"params"'

Length of output: 62496

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 23413f8 and 466425d.

Files ignored due to path filters (2)
  • protocol/x/vault/types/genesis.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/params.pb.go is excluded by !**/*.pb.go
Files selected for processing (36)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (5 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts (9 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/params.ts (6 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (5 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/share.ts (1 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (2 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (6 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts (3 hunks)
  • indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts (1 hunks)
  • indexer/packages/v4-protos/src/codegen/google/bundle.ts (1 hunks)
  • proto/dydxprotocol/vault/genesis.proto (2 hunks)
  • proto/dydxprotocol/vault/params.proto (1 hunks)
  • protocol/app/msgs/internal_msgs.go (1 hunks)
  • protocol/app/testdata/default_genesis_state.json (1 hunks)
  • protocol/app/upgrades/v5.0.0/upgrade.go (1 hunks)
  • protocol/app/upgrades/v5.2.0/upgrade.go (1 hunks)
  • protocol/lib/ante/internal_msg.go (1 hunks)
  • protocol/mocks/VaultKeeper.go (2 hunks)
  • protocol/scripts/genesis/sample_pregenesis.json (1 hunks)
  • protocol/testutil/constants/genesis.go (1 hunks)
  • protocol/x/vault/genesis.go (3 hunks)
  • protocol/x/vault/keeper/grpc_query.go (1 hunks)
  • protocol/x/vault/keeper/grpc_query_test.go (1 hunks)
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params.go (2 hunks)
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params_test.go (2 hunks)
  • protocol/x/vault/keeper/orders.go (8 hunks)
  • protocol/x/vault/keeper/orders_test.go (21 hunks)
  • protocol/x/vault/keeper/params.go (1 hunks)
  • protocol/x/vault/keeper/params_test.go (3 hunks)
  • protocol/x/vault/keeper/vault.go (1 hunks)
  • protocol/x/vault/keeper/vault_test.go (1 hunks)
  • protocol/x/vault/types/genesis.go (2 hunks)
  • protocol/x/vault/types/keys.go (1 hunks)
  • protocol/x/vault/types/params.go (3 hunks)
  • protocol/x/vault/types/params_test.go (5 hunks)
  • protocol/x/vault/types/types.go (1 hunks)
Files skipped from review due to trivial changes (2)
  • indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts
  • protocol/x/vault/keeper/vault_test.go
Files skipped from review as they are similar to previous changes (2)
  • proto/dydxprotocol/vault/genesis.proto
  • proto/dydxprotocol/vault/params.proto
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts

[error] 60-60: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


[error] 63-63: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

Additional comments not posted (134)
indexer/packages/v4-protos/src/codegen/google/bundle.ts (2)

1-6: LGTM! Imports are updated correctly.

The import statements have been updated to include necessary modules and remove unused ones.


8-14: LGTM! Namespace exports are updated correctly.

The google namespace exports have been updated to include the new imports.

protocol/x/vault/keeper/grpc_query.go (1)

20-22: LGTM! But verify the function usage in the codebase.

The function Params has been updated correctly to include a new field DefaultQuotingParams in its response structure.

However, ensure that all function calls to Params handle the new response structure correctly.

protocol/x/vault/keeper/msg_server_update_default_quoting_params.go (1)

Line range hint 13-31:
LGTM! But verify the function usage in the codebase.

The function UpdateParams has been correctly renamed to UpdateDefaultQuotingParams and updated to handle default quoting parameters.

However, ensure that all function calls to UpdateDefaultQuotingParams handle the new behavior correctly.

Verification successful

LGTM! The function UpdateParams has been correctly renamed to UpdateDefaultQuotingParams and updated to handle default quoting parameters.

The test cases and gRPC handler definitions have been updated to reflect the new behavior of the function.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `UpdateDefaultQuotingParams` handle the new behavior correctly.

# Test: Search for the function usage. Expect: All occurrences handle the new behavior.
rg --type go -A 5 $'UpdateDefaultQuotingParams'

Length of output: 22175

protocol/x/vault/keeper/grpc_query_test.go (1)

26-26: Verify the correctness of DefaultQuotingParams.

Ensure that the DefaultQuotingParams function returns the correct default values and that this change aligns with the new functionality introduced in the PR.

protocol/x/vault/types/types.go (2)

28-30: Verify the method GetDefaultQuotingParams.

Ensure that the method correctly retrieves the default quoting parameters and that this change aligns with the new functionality introduced in the PR.


31-33: Verify the method SetDefaultQuotingParams.

Ensure that the method correctly sets the default quoting parameters and that this change aligns with the new functionality introduced in the PR.

protocol/x/vault/types/keys.go (2)

21-23: Verify the constant DefaultQuotingParamsKey.

Ensure that the key correctly represents the default quoting parameters and that this change aligns with the new functionality introduced in the PR.


25-27: Verify the constant QuotingParamsKeyPrefix.

Ensure that the key prefix correctly represents the quoting parameters for individual vaults and that this change aligns with the new functionality introduced in the PR.

protocol/x/vault/types/genesis.go (3)

6-6: LGTM!

The change to initialize DefaultQuotingParams in DefaultGenesis is clear and aligns with the PR objectives.


14-14: LGTM!

The validation of DefaultQuotingParams in Validate enhances robustness and aligns with the PR objectives.


39-43: LGTM!

The added validation for vault.QuotingParams ensures that all relevant parameters are checked for correctness.

protocol/x/vault/types/params.go (2)

9-11: LGTM!

The change to initialize QuotingParams with default values in DefaultQuotingParams is clear and aligns with the PR objectives.


23-23: LGTM!

The comprehensive validation logic for QuotingParams ensures robustness and aligns with the PR objectives.

protocol/x/vault/genesis.go (3)

13-14: LGTM!

The change to set DefaultQuotingParams in InitGenesis is clear and aligns with the PR objectives.


32-33: LGTM!

The logic to handle QuotingParams for each vault in InitGenesis is clear and aligns with the PR objectives.


47-47: LGTM!

The change to export DefaultQuotingParams in ExportGenesis is clear and aligns with the PR objectives.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (3)

3-3: LGTM! Imports are correctly updated.

The new message types MsgUpdateDefaultQuotingParams and MsgUpdateDefaultQuotingParamsResponse are correctly imported.


9-11: LGTM! Method in Msg interface correctly updated.

The method updateParams has been replaced with updateDefaultQuotingParams, aligning with the new functionality.


Line range hint 19-31:
LGTM! Method in MsgClientImpl class correctly updated.

The method updateParams has been replaced with updateDefaultQuotingParams, and the binding in the constructor is correctly updated.

protocol/app/upgrades/v5.2.0/upgrade.go (1)

47-61: LGTM! Deprecation notice and commented-out implementation are clear.

The function setVaultOrderExpirationSecondsToSixty is correctly marked as deprecated and its implementation is commented out, aligning with the transition to DefaultQuotingParams.

protocol/x/vault/keeper/params.go (4)

9-16: LGTM! Method GetDefaultQuotingParams correctly implemented.

The method retrieves DefaultQuotingParams from the state and unmarshals it correctly.


21-33: LGTM! Method SetDefaultQuotingParams correctly implemented.

The method validates, marshals, and sets DefaultQuotingParams in the state correctly.


38-54: LGTM! Method GetVaultQuotingParams correctly implemented.

The method retrieves quoting parameters for a specific vault, returning default quoting parameters if none exist, and unmarshals them correctly.


57-69: LGTM! Method SetVaultQuotingParams correctly implemented.

The method validates, marshals, and sets quoting parameters for a specific vault correctly.

protocol/x/vault/keeper/msg_server_update_default_quoting_params_test.go (6)

17-20: Update function name and message type.

The function name and message type have been updated to reflect the new focus on default quoting parameters.


24-29: Success case: Update to default quoting parameters.

The test case successfully updates to default quoting parameters. Ensure that the default values are correct.


30-41: Success case: Update to non-default quoting parameters.

The test case successfully updates to non-default quoting parameters. Ensure that the non-default values are valid and correctly set.


45-49: Failure case: Invalid authority.

The test case correctly handles the scenario where the authority is invalid. Ensure that the error message is accurate.


52-57: Failure case: Invalid quoting parameters.

The test case correctly handles the scenario where the quoting parameters are invalid. Ensure that the error message is accurate.


75-81: Update function call and validation.

The function call and validation have been updated to reflect the new message type and expected outcomes.

protocol/x/vault/types/params_test.go (7)

11-14: Update function name and parameter type.

The function name and parameter type have been updated to reflect the new focus on quoting parameters.


19-19: Success case: Validate default quoting parameters.

The test case successfully validates the default quoting parameters. Ensure that the default values are correct.


Line range hint 23-32:
Failure case: Layer is greater than MaxUint8.

The test case correctly handles the scenario where the layer value exceeds the maximum allowed value. Ensure that the error message is accurate.


Line range hint 35-44:
Failure case: SpreadMinPpm is 0.

The test case correctly handles the scenario where the SpreadMinPpm value is zero. Ensure that the error message is accurate.


Line range hint 47-56:
Failure case: OrderSizePctPpm is 0.

The test case correctly handles the scenario where the OrderSizePctPpm value is zero. Ensure that the error message is accurate.


Line range hint 59-68:
Failure case: OrderExpirationSeconds is 0.

The test case correctly handles the scenario where the OrderExpirationSeconds value is zero. Ensure that the error message is accurate.


Line range hint 71-80:
Failure case: ActivationThresholdQuoteQuantums is negative.

The test case correctly handles the scenario where the ActivationThresholdQuoteQuantums value is negative. Ensure that the error message is accurate.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts (3)

1-1: Removal of NumShares and NumSharesSDKType interfaces.

The interfaces NumShares and NumSharesSDKType and their associated encoding/decoding logic have been removed. Ensure that the removal does not negatively impact the functionality and maintainability of the codebase.


1-1: Removal of OwnerShare and OwnerShareSDKType interfaces.

The interfaces OwnerShare and OwnerShareSDKType and their associated encoding/decoding logic have been removed. Ensure that the removal does not negatively impact the functionality and maintainability of the codebase.


1-1: Removal of VaultParams and VaultParamsSDKType interfaces.

The interfaces VaultParams and VaultParamsSDKType and their associated encoding/decoding logic have been removed. Ensure that the removal does not negatively impact the functionality and maintainability of the codebase.

protocol/x/vault/keeper/params_test.go (8)

13-13: Function renaming is appropriate.

The function name has been updated to reflect its focus on default quoting parameters.


19-20: Correct usage of GetDefaultQuotingParams.

The function now retrieves default quoting parameters, aligning with the new parameter structure.


Line range hint 23-34:
Setting and verifying new quoting parameters.

The new parameters are correctly set and verified. Ensure that the QuotingParams structure is well-defined and validated.


Line range hint 37-48:
Handling invalid quoting parameters.

The test correctly handles invalid parameters and verifies that the default parameters remain unchanged.


51-51: Function renaming is appropriate.

The function name has been updated to reflect its focus on vault quoting parameters.


57-58: Correct usage of GetVaultQuotingParams.

The function now retrieves quoting parameters for a specific vault, aligning with the new parameter structure.


61-75: Setting and verifying vault quoting parameters.

The new parameters for vaultClob0 are correctly set and verified. Ensure that the QuotingParams structure is well-defined and validated.


78-92: Setting and verifying vault quoting parameters for another vault.

The new parameters for vaultClob1 are correctly set and verified. Ensure that the QuotingParams structure is well-defined and validated.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/share.ts (13)

1-2: Imports are appropriate.

The necessary modules are imported correctly.


5-8: NumShares interface definition is correct.

The NumShares interface is well-defined.


11-14: NumSharesSDKType interface definition is correct.

The NumSharesSDKType interface is well-defined.


17-20: OwnerShare interface definition is correct.

The OwnerShare interface is well-defined.


23-26: OwnerShareSDKType interface definition is correct.

The OwnerShareSDKType interface is well-defined.


28-32: createBaseNumShares function is correct.

The function correctly initializes a NumShares object.


34-41: NumShares encode function is correct.

The function correctly encodes a NumShares message.


43-63: NumShares decode function is correct.

The function correctly decodes a NumShares message.


65-69: NumShares fromPartial function is correct.

The function correctly creates a NumShares message from a partial object.


73-77: createBaseOwnerShare function is correct.

The function correctly initializes an OwnerShare object.


80-91: OwnerShare encode function is correct.

The function correctly encodes an OwnerShare message.


93-117: OwnerShare decode function is correct.

The function correctly decodes an OwnerShare message.


119-124: OwnerShare fromPartial function is correct.

The function correctly creates an OwnerShare message from a partial object.

protocol/lib/ante/internal_msg.go (1)

129-129: Update to reference MsgUpdateDefaultQuotingParams is correct.

The function now correctly references vault.MsgUpdateDefaultQuotingParams, aligning with the new parameter structure.

protocol/x/vault/keeper/vault.go (1)

142-150: Ensure the correctness of the new quotingParams.

The changes correctly rename vaultParams to quotingParams and adjust the data structure accordingly. Ensure that the GetVaultQuotingParams method is correctly implemented and returns the expected values.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/params.ts (6)

Line range hint 3-19:
Verify the correctness and consistency of the QuotingParams interface.

The interface QuotingParams is well-defined with clear documentation comments. Ensure that the naming conventions and data types are consistent with the rest of the codebase.


Line range hint 39-55:
Verify the correctness and consistency of the QuotingParamsSDKType interface.

The interface QuotingParamsSDKType is well-defined with clear documentation comments. Ensure that the naming conventions and data types are consistent with the rest of the codebase.


Line range hint 76-86:
Verify the correctness and appropriateness of the default values in createBaseQuotingParams.

The function createBaseQuotingParams initializes all fields of the QuotingParams interface. Ensure that the default values are appropriate for the intended use cases.


Line range hint 88-119:
Verify the correctness of the encoding logic for QuotingParams.

The encoding method for QuotingParams appears to be well-defined, encoding all fields of the interface. Ensure that the encoding logic is correct and handles all edge cases.


Line range hint 121-165:
Verify the correctness of the decoding logic for QuotingParams.

The decoding method for QuotingParams appears to be well-defined, decoding all fields of the interface. Ensure that the decoding logic is correct and handles all edge cases.


Line range hint 167-175:
Verify the correctness of the fromPartial method for QuotingParams.

The fromPartial method for QuotingParams appears to be well-defined, constructing an object from a partial input. Ensure that the logic is correct and handles all edge cases.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts (6)

1-19: Verify the correctness of the GenesisState interface changes.

The GenesisState interface has been modified to use defaultQuotingParams instead of params. Ensure that the new defaultQuotingParams are correctly integrated and that the changes are consistent with the rest of the codebase.


Line range hint 35-55:
Verify the correctness of the Vault interface changes.

The Vault interface has been modified to use quotingParams instead of vaultParams. Ensure that the new quotingParams are correctly integrated and that the changes are consistent with the rest of the codebase.


63-65: Verify the correctness of the createBaseGenesisState function changes.

The createBaseGenesisState function has been modified to initialize defaultQuotingParams. Ensure that the default values are appropriate and consistent with the rest of the codebase.


70-73: Verify the correctness of the encoding logic for GenesisState.

The encoding method for GenesisState has been modified to encode defaultQuotingParams. Ensure that the encoding logic is correct and handles all edge cases.


91-93: Verify the correctness of the decoding logic for GenesisState.

The decoding method for GenesisState has been modified to decode defaultQuotingParams. Ensure that the decoding logic is correct and handles all edge cases.


140-143: Verify the correctness of the encoding logic for Vault.

The encoding method for Vault has been modified to encode quotingParams. Ensure that the encoding logic is correct and handles all edge cases.

protocol/mocks/VaultKeeper.go (2)

21-33: LGTM!

The function GetDefaultQuotingParams has been correctly updated to reflect the new return type vaulttypes.QuotingParams.


186-195: LGTM!

The function SetDefaultQuotingParams has been correctly updated to reflect the new parameter type vaulttypes.QuotingParams.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (8)

42-48: LGTM!

The interface MsgUpdateDefaultQuotingParams has been correctly updated to reflect the new property defaultQuotingParams of type QuotingParams.


50-56: LGTM!

The interface MsgUpdateDefaultQuotingParamsSDKType has been correctly updated to reflect the new property defaultQuotingParams of type QuotingParamsSDKType.


58-61: LGTM!

The interface MsgUpdateDefaultQuotingParamsResponse has been correctly updated to align with the new focus on quoting parameters.

Tools
Biome

[error] 60-60: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


63-63: LGTM!

The interface MsgUpdateDefaultQuotingParamsResponseSDKType has been correctly updated to align with the new focus on quoting parameters.

Tools
Biome

[error] 63-63: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


164-167: LGTM!

The function createBaseMsgUpdateDefaultQuotingParams has been correctly updated to align with the new focus on quoting parameters.


Line range hint 171-213:
LGTM!

The function MsgUpdateDefaultQuotingParams has been correctly updated to align with the new focus on quoting parameters.


219-223: LGTM!

The function createBaseMsgUpdateDefaultQuotingParamsResponse has been correctly updated to align with the new focus on quoting parameters.


Line range hint 224-247:
LGTM!

The function MsgUpdateDefaultQuotingParamsResponse has been correctly updated to align with the new focus on quoting parameters.

protocol/app/msgs/internal_msgs.go (1)

189-190: LGTM!

The message entries have been correctly updated to reflect the new MsgUpdateDefaultQuotingParams and MsgUpdateDefaultQuotingParamsResponse.

indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (2)

105-127: LGTM! Import statements are correctly integrated.

The new imports for vault/share, vest/vest_entry, and vest/query.lcd are necessary and correctly integrated.


357-367: LGTM! Export statements are correctly updated.

The export structure for vest and ClientFactory has been updated to include the new modules.

protocol/app/upgrades/v5.0.0/upgrade.go (1)

295-297: LGTM! But verify the function usage in the codebase.

The function now sets default quoting parameters, which is a more focused configuration process. The error handling remains appropriate.

However, ensure that all function calls to initializeVaultModuleParams match the new implementation.

Verification successful

LGTM! But verify the function usage in the codebase.

The function now sets default quoting parameters, which is a more focused configuration process. The error handling remains appropriate.

  • protocol/app/upgrades/v5.0.0/upgrade.go: The function initializeVaultModuleParams is only used within this file, and the usage matches the new implementation.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `initializeVaultModuleParams` match the new implementation.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg --type go -A 5 $'initializeVaultModuleParams'

Length of output: 807


Script:

#!/bin/bash
# Description: Verify all references to `initializeVaultModuleParams` in the entire codebase to ensure consistency.

# Test: Search for any additional references to `initializeVaultModuleParams`.
rg --type go 'initializeVaultModuleParams'

Length of output: 207

protocol/app/testdata/default_genesis_state.json (1)

484-484: LGTM! The key renaming is consistent with the changes in the codebase.

The key params has been renamed to default_quoting_params, which enhances clarity regarding the purpose of the parameters.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (6)

17-17: LGTM!

The params property has been correctly replaced by defaultQuotingParams in the QueryParamsResponse interface.


22-22: LGTM!

The params property has been correctly replaced by default_quoting_params in the QueryParamsResponseSDKType interface.


139-139: LGTM!

The createBaseQueryParamsResponse function correctly initializes defaultQuotingParams instead of params.


145-146: LGTM!

The encode method correctly checks and encodes defaultQuotingParams instead of params.


162-162: LGTM!

The decode method correctly decodes defaultQuotingParams instead of params.


176-176: LGTM!

The fromPartial method correctly handles defaultQuotingParams instead of params.

protocol/x/vault/keeper/orders.go (8)

43-44: LGTM!

The RefreshAllVaultOrders function now retrieves quoting parameters for each vault, enhancing parameter specificity.


210-211: LGTM!

The GetVaultClobOrders function now retrieves quoting parameters, ensuring calculations are aligned with each vault's characteristics.


214-214: LGTM!

The calculation of order size now uses quoting parameters, ensuring more accurate order placements.


244-245: LGTM!

The calculation of spread now uses quoting parameters, ensuring more accurate order placements.


256-256: LGTM!

The order expiration time now uses quoting parameters, ensuring more accurate order placements.


258-258: LGTM!

The skew factor now uses quoting parameters, ensuring more accurate order placements.


396-396: LGTM!

The GetVaultClobOrderIds function now retrieves quoting parameters to determine the number of layers, ensuring alignment with vault characteristics.


438-438: LGTM!

The ReplaceVaultClobOrder function now uses quoting parameters for determining order expiration, ensuring alignment with vault characteristics.

protocol/x/vault/keeper/orders_test.go (28)

149-150: LGTM!

The test now uses defaultQuotingParams instead of Params, aligning with the new structure of vault parameters.


153-153: LGTM!

The test now correctly sets defaultQuotingParams in the genesis state.


240-240: LGTM!

The test now uses defaultQuotingParams instead of Params, aligning with the new structure of vault parameters.


245-245: LGTM!

The test now correctly uses defaultQuotingParams for order expiration seconds.


319-319: LGTM!

The test now initializes defaultQuotingParams correctly.


353-353: LGTM!

The test now correctly sets defaultQuotingParams in the genesis state for subaccounts.


362-362: LGTM!

The test now correctly sets defaultQuotingParams in the genesis state for subaccounts.


384-384: LGTM!

The test now verifies the number of vault orders against defaultQuotingParams.Layers.


411-413: LGTM!

The test correctly verifies the order expiration time using defaultQuotingParams.OrderExpirationSeconds.


420-420: LGTM!

The test correctly verifies the order expiration time using defaultQuotingParams.OrderExpirationSeconds.


425-425: LGTM!

The test correctly verifies the order expiration time using defaultQuotingParams.OrderExpirationSeconds.


432-432: LGTM!

The test correctly verifies the order expiration time using defaultQuotingParams.OrderExpirationSeconds.


443-444: LGTM!

The test now uses vaultQuotingParams instead of vaultParams, aligning with the new structure of vault parameters.


466-466: LGTM!

The test case correctly initializes vaultQuotingParams with specific values.


540-540: LGTM!

The test case correctly initializes vaultQuotingParams with specific values.


620-620: LGTM!

The test case correctly initializes vaultQuotingParams with specific values.


700-700: LGTM!

The test case correctly initializes vaultQuotingParams with specific values.


768-768: LGTM!

The test case correctly initializes vaultQuotingParams with specific values.


792-793: LGTM!

The test case correctly initializes vaultQuotingParams with default values.


801-801: LGTM!

The test case correctly initializes vaultQuotingParams with default values.


812-812: LGTM!

The test case correctly initializes vaultQuotingParams with default values.


823-823: LGTM!

The test case correctly initializes vaultQuotingParams with default values.


906-907: LGTM!

The test correctly sets vaultQuotingParams for the vault.


908-908: LGTM!

The test correctly verifies that vaultQuotingParams is set without errors.


919-919: LGTM!

The test correctly retrieves vaultQuotingParams for the vault.


937-937: LGTM!

The test correctly sets the order expiration time using vaultQuotingParams.OrderExpirationSeconds.


1013-1014: LGTM!

The test now retrieves quoting parameters to determine the number of layers, ensuring alignment with vault characteristics.


1015-1015: LGTM!

The test correctly sets quoting parameters for the vault.

protocol/scripts/genesis/sample_pregenesis.json (1)

Line range hint 3905-3912:
LGTM! The renaming enhances clarity.

The key "params" has been renamed to "default_quoting_params" under the "vault" section. This change improves the clarity and specificity of the configuration.

protocol/x/vault/keeper/params.go Outdated Show resolved Hide resolved
@tqin7 tqin7 force-pushed the tq/tra-511 branch 2 times, most recently from 61af221 to ab61e12 Compare August 1, 2024 18:32
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 466425d and 61af221.

Files ignored due to path filters (2)
  • protocol/x/vault/types/genesis.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/params.pb.go is excluded by !**/*.pb.go
Files selected for processing (38)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (5 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts (9 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/params.ts (6 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (5 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/share.ts (1 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (2 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (6 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts (3 hunks)
  • indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts (1 hunks)
  • indexer/packages/v4-protos/src/codegen/google/bundle.ts (1 hunks)
  • proto/dydxprotocol/vault/genesis.proto (2 hunks)
  • proto/dydxprotocol/vault/params.proto (1 hunks)
  • proto/dydxprotocol/vault/tx.proto (2 hunks)
  • protocol/app/msgs/internal_msgs.go (1 hunks)
  • protocol/app/testdata/default_genesis_state.json (1 hunks)
  • protocol/app/upgrades/v5.0.0/upgrade.go (1 hunks)
  • protocol/app/upgrades/v5.2.0/upgrade.go (1 hunks)
  • protocol/lib/ante/internal_msg.go (1 hunks)
  • protocol/mocks/VaultKeeper.go (2 hunks)
  • protocol/scripts/genesis/sample_pregenesis.json (1 hunks)
  • protocol/testutil/constants/genesis.go (1 hunks)
  • protocol/x/vault/client/cli/query_test.go (1 hunks)
  • protocol/x/vault/genesis.go (3 hunks)
  • protocol/x/vault/keeper/grpc_query.go (1 hunks)
  • protocol/x/vault/keeper/grpc_query_test.go (1 hunks)
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params.go (2 hunks)
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params_test.go (2 hunks)
  • protocol/x/vault/keeper/orders.go (8 hunks)
  • protocol/x/vault/keeper/orders_test.go (21 hunks)
  • protocol/x/vault/keeper/params.go (1 hunks)
  • protocol/x/vault/keeper/params_test.go (3 hunks)
  • protocol/x/vault/keeper/vault.go (1 hunks)
  • protocol/x/vault/keeper/vault_test.go (1 hunks)
  • protocol/x/vault/types/genesis.go (2 hunks)
  • protocol/x/vault/types/keys.go (1 hunks)
  • protocol/x/vault/types/params.go (3 hunks)
  • protocol/x/vault/types/params_test.go (5 hunks)
  • protocol/x/vault/types/types.go (1 hunks)
Files skipped from review due to trivial changes (2)
  • indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts
  • protocol/x/vault/types/keys.go
Files skipped from review as they are similar to previous changes (28)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/params.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/share.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts
  • indexer/packages/v4-protos/src/codegen/google/bundle.ts
  • proto/dydxprotocol/vault/tx.proto
  • protocol/app/msgs/internal_msgs.go
  • protocol/app/testdata/default_genesis_state.json
  • protocol/app/upgrades/v5.0.0/upgrade.go
  • protocol/app/upgrades/v5.2.0/upgrade.go
  • protocol/lib/ante/internal_msg.go
  • protocol/mocks/VaultKeeper.go
  • protocol/scripts/genesis/sample_pregenesis.json
  • protocol/testutil/constants/genesis.go
  • protocol/x/vault/genesis.go
  • protocol/x/vault/keeper/grpc_query.go
  • protocol/x/vault/keeper/grpc_query_test.go
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params.go
  • protocol/x/vault/keeper/orders.go
  • protocol/x/vault/keeper/orders_test.go
  • protocol/x/vault/keeper/params.go
  • protocol/x/vault/keeper/params_test.go
  • protocol/x/vault/keeper/vault.go
  • protocol/x/vault/keeper/vault_test.go
  • protocol/x/vault/types/genesis.go
  • protocol/x/vault/types/params_test.go
  • protocol/x/vault/types/types.go
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts

[error] 60-60: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


[error] 63-63: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

Additional comments not posted (27)
proto/dydxprotocol/vault/genesis.proto (3)

6-6: Import statement for share.proto added.

The import statement for share.proto has been added, which is necessary for the new QuotingParams field.


13-14: default_quoting_params field added to GenesisState.

The Params field has been replaced with default_quoting_params, marked as non-nullable, ensuring a default quoting configuration is mandatory.

Ensure that the new field is correctly referenced and utilized throughout the codebase.

Verification successful

default_quoting_params field added to GenesisState.

The Params field has been replaced with default_quoting_params, marked as non-nullable, ensuring a default quoting configuration is mandatory.

The new field is correctly referenced and utilized throughout the codebase in various files:

  • protocol/x/vault/types/tx.pb.go
  • protocol/x/vault/types/genesis.pb.go
  • protocol/x/vault/types/query.pb.go
  • protocol/testutil/constants/genesis.go
  • protocol/scripts/genesis/sample_pregenesis.json
  • protocol/app/testdata/default_genesis_state.json
  • proto/dydxprotocol/vault/tx.proto
  • proto/dydxprotocol/vault/query.proto
  • proto/dydxprotocol/vault/genesis.proto
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `default_quoting_params` in the codebase.

# Test: Search for the field usage. Expect: Only occurrences of the new field.
rg --type proto -A 5 $'default_quoting_params'

Length of output: 75


Script:

#!/bin/bash
# Description: Verify all references to `default_quoting_params` in the codebase.

# Test: Search for the field usage. Expect: Only occurrences of the new field.
rg -A 5 'default_quoting_params'

Length of output: 6325


27-28: quoting_params field added to Vault.

The new quoting_params field has been added to the Vault message, allowing each vault to maintain its own set of quoting parameters.

Ensure that the new field is correctly referenced and utilized throughout the codebase.

Verification successful

quoting_params field added to Vault.

The new quoting_params field has been added to the Vault message, allowing each vault to maintain its own set of quoting parameters. The field is correctly referenced and utilized throughout the codebase.

  • protocol/x/vault/types/genesis.pb.go
  • protocol/x/vault/types/tx.pb.go
  • protocol/x/vault/types/query.pb.go
  • proto/dydxprotocol/vault/genesis.proto
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `quoting_params` in the codebase.

# Test: Search for the field usage. Expect: Only occurrences of the new field.
rg --type proto -A 5 $'quoting_params'

Length of output: 67


Script:

#!/bin/bash
# Description: Verify all references to `quoting_params` in the codebase.

# Find all .proto files and search for the field usage within them.
fd -e proto -x rg 'quoting_params' {}

Length of output: 306


Script:

#!/bin/bash
# Description: Verify all references to `quoting_params` in the codebase.

# Search for the field usage in all files.
rg 'quoting_params'

Length of output: 1858

protocol/x/vault/client/cli/query_test.go (1)

53-53: Test case updated to compare DefaultQuotingParams.

The assertion in the TestQueryParams test case has been updated to compare DefaultQuotingParams instead of Params, aligning the test with the new functionality.

Ensure that the test case provides adequate coverage for the new functionality.

Verification successful

Test case updated to compare DefaultQuotingParams.

The assertion in the TestQueryParams test case has been updated to compare DefaultQuotingParams instead of Params, aligning the test with the new functionality. The search results indicate that there is adequate test coverage for DefaultQuotingParams.

  • params_test.go
  • grpc_query_test.go
  • msg_server_update_default_quoting_params_test.go
  • orders_test.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the test coverage for `DefaultQuotingParams`.

# Test: Search for test cases involving `DefaultQuotingParams`. Expect: Adequate test coverage.
rg --type go -A 5 $'DefaultQuotingParams'

Length of output: 46121

proto/dydxprotocol/vault/params.proto (1)

8-8: QuotingParams message added.

The Params message has been renamed to QuotingParams, clarifying its function as it pertains specifically to vault quoting parameters.

Ensure that the new message is correctly referenced and utilized throughout the codebase.

Verification successful

QuotingParams message added and correctly referenced.

The Params message has been renamed to QuotingParams, and it is correctly referenced throughout the codebase.

  • QuotingParams is used in various contexts, such as default_quoting_params and quoting_params.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `QuotingParams` in the codebase.

# Test: Search for the message usage. Expect: Only occurrences of the new message.
rg --type proto -A 5 $'QuotingParams'

Length of output: 66


Script:

#!/bin/bash
# Description: Verify all references to `QuotingParams` in the codebase.

# Find all .proto files and search for the message usage of `QuotingParams`.
fd -e proto -x rg 'QuotingParams' {}

Length of output: 822

protocol/x/vault/types/params.go (2)

Line range hint 9-21:
Default parameters look good.

The default quoting parameters are set sensibly and should work well for the application.


Line range hint 23-37:
Validation logic looks good.

The validation logic for the quoting parameters is thorough and covers all necessary cases.

protocol/x/vault/keeper/msg_server_update_default_quoting_params_test.go (4)

24-29: Test case for updating to default looks good.

The test case correctly verifies the successful update to default quoting parameters.


30-41: Test case for updating to non-default looks good.

The test case correctly verifies the successful update to non-default quoting parameters.


45-49: Test case for invalid authority looks good.

The test case correctly verifies the failure scenario for invalid authority.


Line range hint 52-60:
Test case for invalid parameters looks good.

The test case correctly verifies the failure scenario for invalid quoting parameters.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts (3)

1-1: Removal of NumShares and NumSharesSDKType looks good.

The removal simplifies the data structure related to vault shares and should not affect the functionality.


1-1: Removal of OwnerShare and OwnerShareSDKType looks good.

The removal simplifies the data structure related to vault ownership and should not affect the functionality.


1-1: Removal of VaultParams and VaultParamsSDKType looks good.

The removal simplifies the data structure related to vault parameters and should not affect the functionality.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (11)

3-3: LGTM!

The import statement for QuotingParams and QuotingParamsSDKType aligns with the new focus on quoting parameters.


42-48: LGTM!

The new MsgUpdateDefaultQuotingParams interface correctly introduces authority and defaultQuotingParams, aligning with the updated focus.


52-56: LGTM!

The new MsgUpdateDefaultQuotingParamsSDKType interface correctly introduces authority and default_quoting_params, aligning with the updated focus.


164-167: LGTM!

The createBaseMsgUpdateDefaultQuotingParams function correctly initializes authority and defaultQuotingParams to default values.


171-178: LGTM!

The encoding function for MsgUpdateDefaultQuotingParams correctly encodes authority and defaultQuotingParams.


Line range hint 184-198: LGTM!

The decoding function for MsgUpdateDefaultQuotingParams correctly decodes authority and defaultQuotingParams.


210-213: LGTM!

The fromPartial function for MsgUpdateDefaultQuotingParams correctly handles partial updates for authority and defaultQuotingParams.


219-219: LGTM!

The createBaseMsgUpdateDefaultQuotingParamsResponse function correctly initializes an empty object.


223-223: LGTM!

The encoding function for MsgUpdateDefaultQuotingParamsResponse correctly encodes an empty object.


228-231: LGTM!

The decoding function for MsgUpdateDefaultQuotingParamsResponse correctly decodes an empty object.


246-247: LGTM!

The fromPartial function for MsgUpdateDefaultQuotingParamsResponse correctly handles partial updates for an empty object.

indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (2)

357-364: LGTM!

The updated vest export structure correctly includes additional modules, enhancing vesting functionalities.


365-367: LGTM!

The updated ClientFactory export structure correctly includes an additional module, expanding the factory's capabilities.

Comments failed to post (2)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts

58-60: Avoid using empty interfaces.

An empty interface is equivalent to {}. Consider using a type alias instead.

- export interface MsgUpdateDefaultQuotingParamsResponse {}
+ export type MsgUpdateDefaultQuotingParamsResponse = {};
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

/** MsgUpdateDefaultQuotingParamsResponse is the Msg/UpdateDefaultQuotingParams response type. */

export type MsgUpdateDefaultQuotingParamsResponse = {};
Tools
Biome

[error] 60-60: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


63-63: Avoid using empty interfaces.

An empty interface is equivalent to {}. Consider using a type alias instead.

- export interface MsgUpdateDefaultQuotingParamsResponseSDKType {}
+ export type MsgUpdateDefaultQuotingParamsResponseSDKType = {};
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

export type MsgUpdateDefaultQuotingParamsResponseSDKType = {};
Tools
Biome

[error] 63-63: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

// Set vault module parameters to default values.
vaultParams := vaulttypes.DefaultParams()
if err := vaultKeeper.SetParams(ctx, vaultParams); err != nil {
// Set vault module quoting parameters to default values.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Comment out code rather than changing.

import "dydxprotocol/vault/vault.proto";

option go_package = "github.com/dydxprotocol/v4-chain/protocol/x/vault/types";

// GenesisState defines `x/vault`'s genesis state.
message GenesisState {
// The parameters of the module.
Params params = 1 [ (gogoproto.nullable) = false ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we make these changes backwards compatible?

@@ -5,8 +5,8 @@ import "gogoproto/gogo.proto";

option go_package = "github.com/dydxprotocol/v4-chain/protocol/x/vault/types";

// Params stores `x/vault` parameters.
message Params {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: deprecate this rather than just outright deleting.

@@ -36,7 +37,7 @@ message QueryParamsRequest {}

// QueryParamsResponse is a response type for the Params RPC method.
message QueryParamsResponse {
Params params = 1 [ (gogoproto.nullable) = false ];
QuotingParams default_quoting_params = 1 [ (gogoproto.nullable) = false ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Make this backwards compatible.

@@ -15,8 +15,9 @@ service Msg {
// DepositToVault deposits funds into a vault.
rpc DepositToVault(MsgDepositToVault) returns (MsgDepositToVaultResponse);

// UpdateParams updates the Params in state.
rpc UpdateParams(MsgUpdateParams) returns (MsgUpdateParamsResponse);
// UpdateDefaultQuotingParams updates the default quoting params in state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Create new queries / handlers rather than replacing old ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that for MsgServer, i should replace the old method. It's a bit too messy otherwise with UpdateParams still there (implementation, internal msg testing, etc.)

}

func TestGetSetVaultParams(t *testing.T) {
func TestGetSetVaultQuotingParams(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Given the paramaterized nature of the tests now, prefer test cases pattern.

_, exists := k.GetTotalShares(ctx, tc.vaultId)
require.Equal(t, false, exists)
for _, owner := range tc.owners {
_, exists = k.GetOwnerShares(ctx, tc.vaultId, owner)
require.Equal(t, false, exists)
}
_, exists = k.GetVaultParams(ctx, tc.vaultId)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Do you need to check if the quote params were deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep. will do so in the later PR that adds a governance message to set individual vault quoting params

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Outside diff range, codebase verification and nitpick comments (5)
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts (1)

Incomplete removal of interfaces detected.

The interfaces NumShares, NumSharesSDKType, OwnerShare, OwnerShareSDKType, VaultParams, and VaultParamsSDKType have been removed but are still referenced in several files. This incomplete removal will likely cause issues with the functionality that depends on these interfaces. Please review and update all dependent code to ensure consistency.

  • Files with references to removed interfaces:
    • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts
    • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/share.ts
    • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.lcd.ts
    • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts
    • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.rpc.Query.ts
Analysis chain

Line range hint 1-99: Verify the impact of interface deletions.

The removal of interfaces NumShares, NumSharesSDKType, OwnerShare, OwnerShareSDKType, VaultParams, and VaultParamsSDKType suggests a significant change in the data model. Ensure that the existing functionality is not adversely affected by these deletions.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the impact of interface deletions on existing functionality.

# Test: Search for the usage of the removed interfaces. Expect: No occurrences of the removed interfaces.
rg --type ts -A 5 $'NumShares|NumSharesSDKType|OwnerShare|OwnerShareSDKType|VaultParams|VaultParamsSDKType'

Length of output: 33354

proto/dydxprotocol/vault/tx.proto (2)

19-21: Deprecation Notice: UpdateParams

The UpdateParams RPC method is deprecated. Ensure that all references to this method are updated to use the new UpdateDefaultQuotingParams method.

  • Files with references to UpdateParams:
    • proto/dydxprotocol/vault/tx.proto
    • proto/dydxprotocol/stats/tx.proto
    • proto/dydxprotocol/rewards/tx.proto
    • proto/dydxprotocol/perpetuals/tx.proto
Analysis chain

Deprecation Notice: UpdateParams

The UpdateParams RPC method is deprecated. Ensure that all references to this method are updated to use the new UpdateDefaultQuotingParams method.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to `UpdateParams` are updated to `UpdateDefaultQuotingParams`.

# Test: Search for the usage of `UpdateParams`. Expect: No occurrences.
rg --type proto -A 5 $'UpdateParams'

Length of output: 65


Script:

#!/bin/bash
# Description: Verify that all references to `UpdateParams` are updated to `UpdateDefaultQuotingParams`.

# Test: Search for the usage of `UpdateParams`. Expect: No occurrences.
rg 'UpdateParams' -g '*.proto' -A 5

Length of output: 5411


Deprecation Notice: MsgUpdateParams

The MsgUpdateParams message is still being referenced in multiple locations across the codebase. Please update all references to use the new MsgUpdateDefaultQuotingParams message instead. The following files contain references to MsgUpdateParams:

  • proto/dydxprotocol/vault/tx.proto
  • proto/dydxprotocol/stats/tx.proto
  • proto/dydxprotocol/rewards/tx.proto
  • proto/dydxprotocol/perpetuals/tx.proto
  • protocol/x/rewards/types/tx.go
  • protocol/x/stats/types/tx.pb.go
  • protocol/x/vault/types/tx.pb.go
  • protocol/x/perpetuals/types/tx.pb.go
  • protocol/x/rewards/keeper/msg_server.go
  • protocol/x/stats/keeper/msg_server.go
  • protocol/x/perpetuals/keeper/msg_server.go

Please ensure that all these references are updated to maintain consistency with the deprecation notice.

Analysis chain

Line range hint 53-66: Deprecation Notice: MsgUpdateParams

The MsgUpdateParams message is deprecated. Ensure that all references to this message are updated to use the new MsgUpdateDefaultQuotingParams message.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to `MsgUpdateParams` are updated to `MsgUpdateDefaultQuotingParams`.

# Test: Search for the usage of `MsgUpdateParams`. Expect: No occurrences.
rg --type proto -A 5 $'MsgUpdateParams'

Length of output: 68


Script:

#!/bin/bash
# Description: Verify that all references to `MsgUpdateParams` are updated to `MsgUpdateDefaultQuotingParams`.

# Test: Search for the usage of `MsgUpdateParams` across all files. Expect: No occurrences.
rg -A 5 $'MsgUpdateParams'

Length of output: 158831

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (2)

99-109: Consider using a type alias for empty interfaces.

The MsgUpdateDefaultQuotingParamsResponse interface is consistent with the naming conventions and structure. However, an empty interface is equivalent to {} and can be replaced with a type alias.

- export interface MsgUpdateDefaultQuotingParamsResponse {}
+ export type MsgUpdateDefaultQuotingParamsResponse = {};
Tools
Biome

[error] 103-103: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


[error] 109-109: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


354-356: Consider using a type alias for empty interfaces.

The createBaseMsgUpdateDefaultQuotingParamsResponse function is well-defined but initializes an empty interface. An empty interface is equivalent to {} and can be replaced with a type alias.

- export interface MsgUpdateDefaultQuotingParamsResponse {}
+ export type MsgUpdateDefaultQuotingParamsResponse = {};
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 61af221 and 21cf0ba.

Files ignored due to path filters (4)
  • protocol/x/vault/types/genesis.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/params.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/query.pb.go is excluded by !**/*.pb.go
  • protocol/x/vault/types/tx.pb.go is excluded by !**/*.pb.go
Files selected for processing (41)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (5 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts (11 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/params.ts (3 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (6 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/share.ts (1 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (3 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (3 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/vault.ts (3 hunks)
  • indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts (1 hunks)
  • indexer/packages/v4-protos/src/codegen/google/bundle.ts (1 hunks)
  • proto/dydxprotocol/vault/genesis.proto (2 hunks)
  • proto/dydxprotocol/vault/params.proto (1 hunks)
  • proto/dydxprotocol/vault/query.proto (2 hunks)
  • proto/dydxprotocol/vault/tx.proto (3 hunks)
  • protocol/app/msgs/internal_msgs.go (1 hunks)
  • protocol/app/testdata/default_genesis_state.json (1 hunks)
  • protocol/app/upgrades/v5.0.0/upgrade.go (1 hunks)
  • protocol/app/upgrades/v5.2.0/upgrade.go (1 hunks)
  • protocol/lib/ante/internal_msg.go (1 hunks)
  • protocol/mocks/VaultKeeper.go (2 hunks)
  • protocol/scripts/genesis/sample_pregenesis.json (1 hunks)
  • protocol/testing/genesis.sh (1 hunks)
  • protocol/testutil/constants/genesis.go (1 hunks)
  • protocol/x/vault/client/cli/query_test.go (1 hunks)
  • protocol/x/vault/genesis.go (3 hunks)
  • protocol/x/vault/keeper/grpc_query.go (1 hunks)
  • protocol/x/vault/keeper/grpc_query_test.go (1 hunks)
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params.go (1 hunks)
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params_test.go (2 hunks)
  • protocol/x/vault/keeper/msg_server_update_params.go (1 hunks)
  • protocol/x/vault/keeper/orders.go (8 hunks)
  • protocol/x/vault/keeper/orders_test.go (21 hunks)
  • protocol/x/vault/keeper/params.go (1 hunks)
  • protocol/x/vault/keeper/params_test.go (3 hunks)
  • protocol/x/vault/keeper/vault.go (1 hunks)
  • protocol/x/vault/keeper/vault_test.go (1 hunks)
  • protocol/x/vault/types/genesis.go (2 hunks)
  • protocol/x/vault/types/keys.go (1 hunks)
  • protocol/x/vault/types/params.go (3 hunks)
  • protocol/x/vault/types/params_test.go (5 hunks)
  • protocol/x/vault/types/types.go (1 hunks)
Files skipped from review due to trivial changes (1)
  • indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts
Files skipped from review as they are similar to previous changes (28)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/share.ts
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts
  • indexer/packages/v4-protos/src/codegen/google/bundle.ts
  • proto/dydxprotocol/vault/genesis.proto
  • proto/dydxprotocol/vault/query.proto
  • protocol/app/msgs/internal_msgs.go
  • protocol/app/testdata/default_genesis_state.json
  • protocol/app/upgrades/v5.0.0/upgrade.go
  • protocol/app/upgrades/v5.2.0/upgrade.go
  • protocol/lib/ante/internal_msg.go
  • protocol/mocks/VaultKeeper.go
  • protocol/scripts/genesis/sample_pregenesis.json
  • protocol/testutil/constants/genesis.go
  • protocol/x/vault/client/cli/query_test.go
  • protocol/x/vault/genesis.go
  • protocol/x/vault/keeper/grpc_query.go
  • protocol/x/vault/keeper/grpc_query_test.go
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params_test.go
  • protocol/x/vault/keeper/orders.go
  • protocol/x/vault/keeper/orders_test.go
  • protocol/x/vault/keeper/params.go
  • protocol/x/vault/keeper/params_test.go
  • protocol/x/vault/keeper/vault.go
  • protocol/x/vault/keeper/vault_test.go
  • protocol/x/vault/types/keys.go
  • protocol/x/vault/types/params_test.go
  • protocol/x/vault/types/types.go
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts

[error] 69-69: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


[error] 75-75: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


[error] 103-103: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


[error] 109-109: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

Additional comments not posted (36)
protocol/x/vault/keeper/msg_server_update_params.go (1)

11-16: Deprecation Notice: Ensure no calls to UpdateParams.

The UpdateParams function has been deprecated and now only returns an error message. Verify that no parts of the codebase are still calling this function.

Verification successful

Deprecation Notice: Ensure no calls to UpdateParams.

The UpdateParams function has been deprecated and now only returns an error message. Upon verification, no calls to the UpdateParams function were found in the main application logic, excluding test and generated files.

  • The only instances of UpdateParams found were within the definitions and comments, not actual function calls.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that no calls are made to the deprecated `UpdateParams` function.

# Test: Search for the function usage. Expect: No occurrences of `UpdateParams` calls.
rg --type python -A 5 $'UpdateParams'

Length of output: 67


Script:

#!/bin/bash
# Description: Verify that no calls are made to the deprecated `UpdateParams` function.

# Test: Search for the function usage. Expect: No occurrences of `UpdateParams` calls.
rg --type go -A 5 $'UpdateParams'

Length of output: 119477


Script:

#!/bin/bash
# Description: Verify that no calls are made to the deprecated `UpdateParams` function, excluding test and generated files.

# Test: Search for the function usage in Go files, excluding test and generated files. Expect: No occurrences of `UpdateParams` calls.
rg --type go -A 5 'UpdateParams' --glob '!**/*_test.go' --glob '!**/*.pb.go'

Length of output: 24218

protocol/x/vault/keeper/msg_server_update_default_quoting_params.go (1)

13-32: Ensure proper error handling and authority checks.

The UpdateDefaultQuotingParams function correctly handles authority checks and updates the default quoting parameters. Verify that the function is correctly integrated and called within the codebase.

Verification successful

Function UpdateDefaultQuotingParams is properly integrated and called within the codebase.

The function is referenced in multiple places, including the keeper, tests, and other protocol files, indicating correct usage and testing.

  • protocol/x/vault/keeper/msg_server_update_default_quoting_params.go
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params_test.go
  • protocol/x/vault/types/tx.pb.go
  • protocol/lib/ante/internal_msg.go
  • protocol/app/msgs/internal_msgs.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration and calls to `UpdateDefaultQuotingParams`.

# Test: Search for the function usage. Expect: Correct integration and calls to `UpdateDefaultQuotingParams`.
rg --type python -A 5 $'UpdateDefaultQuotingParams'

Length of output: 81


Script:

#!/bin/bash
# Description: Verify the integration and calls to `UpdateDefaultQuotingParams`.

# Test: Search for the function usage. Expect: Correct integration and calls to `UpdateDefaultQuotingParams`.
rg --type go -A 5 $'UpdateDefaultQuotingParams'

Length of output: 24472

protocol/x/vault/types/genesis.go (3)

6-6: Initialization Change: Ensure correct initialization of DefaultQuotingParams.

The DefaultGenesis function now initializes DefaultQuotingParams instead of Params. Verify that this change is correctly implemented and does not affect other parts of the codebase.

Verification successful

Initialization Change Verified: DefaultQuotingParams is correctly initialized and used throughout the codebase.

The initialization of DefaultQuotingParams has been correctly implemented and does not negatively impact other parts of the codebase.

  • protocol/x/vault/types/params.go: Initialization of DefaultQuotingParams.
  • protocol/x/vault/types/genesis.go: Usage in genesis state.
  • protocol/x/vault/keeper/params_test.go: Tests for setting and getting DefaultQuotingParams.
  • protocol/x/vault/keeper/msg_server_update_default_quoting_params_test.go: Tests for updating DefaultQuotingParams.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the initialization of `DefaultQuotingParams`.

# Test: Search for the initialization of `DefaultQuotingParams`. Expect: Correct initialization in all necessary parts of the codebase.
rg --type python -A 5 $'DefaultQuotingParams'

Length of output: 75


Script:

#!/bin/bash
# Description: Verify the initialization of `DefaultQuotingParams`.

# Test: Search for the initialization of `DefaultQuotingParams`. Expect: Correct initialization in all necessary parts of the codebase.
rg --type go -A 5 $'DefaultQuotingParams'

Length of output: 48161


14-14: Validation Change: Ensure correct validation of DefaultQuotingParams.

The Validate method now validates DefaultQuotingParams instead of Params. Verify that this change is correctly implemented and does not affect other parts of the codebase.

Verification successful

Validation Change: Ensure correct validation of DefaultQuotingParams.

The change to use DefaultQuotingParams.Validate instead of Params.Validate in protocol/x/vault/types/genesis.go is correctly implemented and does not affect other parts of the codebase.

  • protocol/x/vault/types/genesis.go correctly uses DefaultQuotingParams.Validate.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the validation of `DefaultQuotingParams`.

# Test: Search for the validation of `DefaultQuotingParams`. Expect: Correct validation in all necessary parts of the codebase.
rg --type python -A 5 $'DefaultQuotingParams.Validate'

Length of output: 84


Script:

#!/bin/bash
# Description: Verify the validation of `DefaultQuotingParams`.

# Test: Search for the validation of `DefaultQuotingParams`. Expect: Correct validation in all necessary parts of the codebase.
rg --type go -A 5 $'DefaultQuotingParams.Validate'

Length of output: 417


Script:

#!/bin/bash
# Description: Verify if instances of `Params.Validate` have been correctly updated.

# Test: Search for instances of `Params.Validate`. Expect: No outdated instances remaining.
rg --type go -A 5 $'Params.Validate'

Length of output: 13786


39-43: New Validation: Ensure correct validation of vault.QuotingParams.

The Validate method now includes validation for vault.QuotingParams. Verify that this change is correctly implemented and does not affect other parts of the codebase.

Verification successful

Validation of vault.QuotingParams is correctly implemented.

The Validate method for QuotingParams is thoroughly implemented in protocol/x/vault/types/params.go and is correctly called in protocol/x/vault/types/genesis.go. This ensures proper validation of vault.QuotingParams without affecting other parts of the codebase.

  • protocol/x/vault/types/params.go: Implementation of Validate method.
  • protocol/x/vault/types/genesis.go: Call to vault.QuotingParams.Validate().
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the validation of `vault.QuotingParams`.

# Test: Search for the validation of `vault.QuotingParams`. Expect: Correct validation in all necessary parts of the codebase.
rg --type python -A 5 $'vault.QuotingParams.Validate'

Length of output: 83


Script:

#!/bin/bash
# Description: Verify the validation of `vault.QuotingParams`.

# Search for the implementation of the `Validate` method for `vault.QuotingParams`.
ast-grep --lang go --pattern 'func ($_ QuotingParams) Validate() error { $$$ }'

# Search for all occurrences of `vault.QuotingParams.Validate()` in the Go codebase.
rg 'vault.QuotingParams.Validate()'

Length of output: 1738

protocol/x/vault/types/params.go (2)

Line range hint 9-21: LGTM!

The DefaultQuotingParams function correctly returns a default set of quoting parameters with clear inline comments.


Line range hint 23-39: LGTM!

The Validate function correctly validates the QuotingParams with appropriate checks for each parameter.

proto/dydxprotocol/vault/params.proto (2)

8-40: LGTM!

The QuotingParams message is well-defined with clear comments and appropriate use of gogoproto options.


42-43: LGTM!

The Params message is correctly marked as deprecated, with a clear notice indicating its replacement by QuotingParams.

proto/dydxprotocol/vault/tx.proto (4)

22-24: New Method: UpdateDefaultQuotingParams

The new UpdateDefaultQuotingParams RPC method is correctly defined. Ensure that this method is properly integrated and tested.


67-76: New Message: MsgUpdateDefaultQuotingParams

The new MsgUpdateDefaultQuotingParams message is correctly defined. Ensure that this message is properly integrated and tested.


78-80: New Message: MsgUpdateDefaultQuotingParamsResponse

The new MsgUpdateDefaultQuotingParamsResponse message is correctly defined. Ensure that this message is properly integrated and tested.


64-66: Deprecation Notice: MsgUpdateParamsResponse

The MsgUpdateParamsResponse message is deprecated. Ensure that all references to this message are updated to use the new MsgUpdateDefaultQuotingParamsResponse message.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts (4)

1-21: Renaming: params to defaultQuotingParams

The params field is correctly renamed to defaultQuotingParams in the GenesisState interface. Ensure that all references to params are updated to defaultQuotingParams.


26-38: Renaming: params to default_quoting_params

The params field is correctly renamed to default_quoting_params in the GenesisStateSDKType interface. Ensure that all references to params are updated to default_quoting_params.


51-53: Renaming: vaultParams to quotingParams

The vaultParams field is correctly renamed to quotingParams in the Vault interface. Ensure that all references to vaultParams are updated to quotingParams.


69-71: Renaming: vault_params to quoting_params

The vault_params field is correctly renamed to quoting_params in the VaultSDKType interface. Ensure that all references to vault_params are updated to quoting_params.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/params.ts (4)

3-38: Renaming: Params to QuotingParams

The Params interface is correctly renamed to QuotingParams. Ensure that all references to Params are updated to QuotingParams.


41-74: Renaming: ParamsSDKType to QuotingParamsSDKType

The ParamsSDKType interface is correctly renamed to QuotingParamsSDKType. Ensure that all references to ParamsSDKType are updated to QuotingParamsSDKType.


154-164: Renaming: createBaseParams to createBaseQuotingParams

The createBaseParams function is correctly renamed to createBaseQuotingParams. Ensure that all references to createBaseParams are updated to createBaseQuotingParams.


166-255: Renaming: Encoding and Decoding Methods

The encoding and decoding methods are correctly updated to accommodate QuotingParams. Ensure that all references to Params are updated to QuotingParams.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (8)

3-3: Imports look good!

The import changes align with the updated focus on quoting parameters.


42-45: Deprecation and replacement look good!

The deprecation of MsgUpdateParams and its replacement with MsgUpdateDefaultQuotingParams align with the new focus on quoting parameters.

Also applies to: 53-56, 64-67, 71-73


77-86: New interface looks good!

The MsgUpdateDefaultQuotingParams interface is well-defined and aligns with the updated focus on quoting parameters.

Also applies to: 92-97


297-304: Deprecation and replacement look good!

The deprecation of createBaseMsgUpdateParams and its replacement with createBaseMsgUpdateDefaultQuotingParams align with the new focus on quoting parameters.


299-304: New function looks good!

The createBaseMsgUpdateDefaultQuotingParams function is well-defined and aligns with the updated focus on quoting parameters.


306-350: Deprecation and replacement look good!

The deprecation of encoding/decoding functions for MsgUpdateParams and their replacement with those for MsgUpdateDefaultQuotingParams align with the new focus on quoting parameters.


306-350: New encoding/decoding functions look good!

The encoding/decoding functions for MsgUpdateDefaultQuotingParams are well-defined and align with the updated focus on quoting parameters.


354-356: Deprecation and replacement look good!

The deprecation of createBaseMsgUpdateParamsResponse and its replacement with createBaseMsgUpdateDefaultQuotingParamsResponse align with the new focus on quoting parameters.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/query.ts (6)

3-3: Imports look good!

The import changes align with the updated focus on quoting parameters.


17-21: Deprecation and replacement look good!

The deprecation of the params property and its replacement with defaultQuotingParams align with the new focus on quoting parameters.

Also applies to: 26-30


147-148: Changes look good!

The createBaseQueryParamsResponse function now initializes defaultQuotingParams, aligning with the updated focus on quoting parameters.


158-160: Changes look good!

The QueryParamsResponse encoding function now handles defaultQuotingParams, aligning with the updated focus on quoting parameters.


178-180: Changes look good!

The QueryParamsResponse decoding function now handles defaultQuotingParams, aligning with the updated focus on quoting parameters.


194-194: Changes look good!

The QueryParamsResponse fromPartial function now handles defaultQuotingParams, aligning with the updated focus on quoting parameters.

protocol/testing/genesis.sh (1)

2237-2237: Verify the updated path for setting the minimum spread parameter.

The path for the minimum spread parameter has been updated to .app_state.vault.default_quoting_params.spread_min_ppm. Ensure this path is consistent with the new structure of the configuration data.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 21cf0ba and 6eaaad4.

Files ignored due to path filters (1)
  • protocol/x/vault/types/genesis.pb.go is excluded by !**/*.pb.go
Files selected for processing (2)
  • proto/dydxprotocol/vault/genesis.proto (2 hunks)
  • protocol/app/msgs/internal_msgs.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • protocol/app/msgs/internal_msgs.go
Additional comments not posted (4)
proto/dydxprotocol/vault/genesis.proto (4)

6-6: Import statement added for share.proto.

The addition of this import statement suggests that the new quoting parameters may be detailed or utilized within the share.proto file.


14-15: Field Params deprecated.

The Params field has been marked as deprecated since v6.x in favor of default_quoting_params.


18-19: Field default_quoting_params added.

The default_quoting_params field has been added to enhance the configuration capabilities of vaults.


30-31: Field vault_params renamed to quoting_params.

The vault_params field has been renamed to quoting_params to reinforce the focus on quoting parameters.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6eaaad4 and a812362.

Files selected for processing (3)
  • protocol/app/msgs/all_msgs.go (1 hunks)
  • protocol/app/msgs/internal_msgs_test.go (1 hunks)
  • protocol/scripts/genesis/sample_pregenesis.json (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • protocol/scripts/genesis/sample_pregenesis.json
Additional comments not posted (2)
protocol/app/msgs/internal_msgs_test.go (1)

149-150: LGTM! New message paths are correctly added.

The new message paths for MsgUpdateDefaultQuotingParams and MsgUpdateDefaultQuotingParamsResponse are correctly integrated within the existing structure.

protocol/app/msgs/all_msgs.go (1)

246-247: LGTM! New message types are correctly added.

The new message types for MsgUpdateDefaultQuotingParams and MsgUpdateDefaultQuotingParamsResponse are correctly integrated within the existing structure.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a812362 and 332b611.

Files selected for processing (1)
  • protocol/lib/ante/internal_msg.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • protocol/lib/ante/internal_msg.go

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 332b611 and e1e6ffc.

Files ignored due to path filters (1)
  • protocol/x/vault/types/tx.pb.go is excluded by !**/*.pb.go
Files selected for processing (7)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (2 hunks)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (6 hunks)
  • proto/dydxprotocol/vault/tx.proto (2 hunks)
  • protocol/app/msgs/all_msgs.go (1 hunks)
  • protocol/app/msgs/internal_msgs.go (1 hunks)
  • protocol/app/msgs/internal_msgs_test.go (1 hunks)
  • protocol/lib/ante/internal_msg.go (1 hunks)
Files skipped from review as they are similar to previous changes (5)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts
  • proto/dydxprotocol/vault/tx.proto
  • protocol/app/msgs/all_msgs.go
  • protocol/app/msgs/internal_msgs.go
  • protocol/lib/ante/internal_msg.go
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts

[error] 69-69: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


[error] 75-75: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

Additional comments not posted (16)
protocol/app/msgs/internal_msgs_test.go (1)

147-148: Added new message paths for vault quoting parameters.

The new message paths for MsgUpdateDefaultQuotingParams and MsgUpdateDefaultQuotingParamsResponse are correctly added to the test case. Ensure corresponding message handlers are implemented and tested.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (15)

3-3: Import statement updated for quoting parameters.

The import statement for QuotingParams and QuotingParamsSDKType is correctly added.


42-45: Added new interface MsgUpdateDefaultQuotingParams.

The new interface MsgUpdateDefaultQuotingParams is correctly defined to handle default quoting parameters.


47-51: Interface MsgUpdateDefaultQuotingParams correctly defines fields.

The interface correctly includes authority and defaultQuotingParams fields.


53-56: Added new SDK type interface MsgUpdateDefaultQuotingParamsSDKType.

The new SDK type interface MsgUpdateDefaultQuotingParamsSDKType is correctly defined.


58-62: SDK type interface MsgUpdateDefaultQuotingParamsSDKType correctly defines fields.

The interface correctly includes authority and default_quoting_params fields.


64-67: Added new interface MsgUpdateDefaultQuotingParamsResponse.

The new interface MsgUpdateDefaultQuotingParamsResponse is correctly defined.


69-73: Added new SDK type interface MsgUpdateDefaultQuotingParamsResponseSDKType.

The new SDK type interface MsgUpdateDefaultQuotingParamsResponseSDKType is correctly defined.

Tools
Biome

[error] 69-69: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)


176-179: Function createBaseMsgUpdateDefaultQuotingParams correctly initializes default values.

The function correctly initializes default values for MsgUpdateDefaultQuotingParams.


183-190: Function MsgUpdateDefaultQuotingParams.encode correctly encodes message fields.

The function correctly encodes the authority and defaultQuotingParams fields.


Line range hint 196-210:
Function MsgUpdateDefaultQuotingParams.decode correctly decodes message fields.

The function correctly decodes the authority and defaultQuotingParams fields.


222-225: Function MsgUpdateDefaultQuotingParams.fromPartial correctly handles partial messages.

The function correctly handles partial messages for MsgUpdateDefaultQuotingParams.


231-235: Function createBaseMsgUpdateDefaultQuotingParamsResponse correctly initializes default values.

The function correctly initializes default values for MsgUpdateDefaultQuotingParamsResponse.


236-236: Function MsgUpdateDefaultQuotingParamsResponse.encode correctly encodes message fields.

The function correctly encodes the message fields.


240-243: Function MsgUpdateDefaultQuotingParamsResponse.decode correctly decodes message fields.

The function correctly decodes the message fields.


258-259: Function MsgUpdateDefaultQuotingParamsResponse.fromPartial correctly handles partial messages.

The function correctly handles partial messages for MsgUpdateDefaultQuotingParamsResponse.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e1e6ffc and 16018d0.

Files selected for processing (2)
  • protocol/app/testdata/default_genesis_state.json (2 hunks)
  • protocol/testutil/constants/genesis.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • protocol/testutil/constants/genesis.go
Additional comments not posted (2)
protocol/app/testdata/default_genesis_state.json (2)

Line range hint 484-491:
LGTM! Renaming params to default_quoting_params enhances clarity.

The change makes it clear that these parameters are related to quoting defaults.


493-493: Verify the purpose of adding a params key with a null value.

The addition of a params key with a null value might be a placeholder for future configuration or to indicate the absence of parameters. Please confirm the intent.

import "dydxprotocol/vault/vault.proto";

option go_package = "github.com/dydxprotocol/v4-chain/protocol/x/vault/types";

// GenesisState defines `x/vault`'s genesis state.
message GenesisState {
// The parameters of the module.
Params params = 1 [ (gogoproto.nullable) = false ];
// Deprecated since v6.x in favor of default_quoting_params.
Params params = 1 [ deprecated = true ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Any issue in using reserved instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm yeah that's probably cleaner!

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 16018d0 and 64c0619.

Files ignored due to path filters (1)
  • protocol/x/vault/types/genesis.pb.go is excluded by !**/*.pb.go
Files selected for processing (5)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts (9 hunks)
  • proto/dydxprotocol/vault/genesis.proto (2 hunks)
  • protocol/app/testdata/default_genesis_state.json (1 hunks)
  • protocol/scripts/genesis/sample_pregenesis.json (1 hunks)
  • protocol/testutil/constants/genesis.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • protocol/app/testdata/default_genesis_state.json
  • protocol/scripts/genesis/sample_pregenesis.json
  • protocol/testutil/constants/genesis.go
Additional comments not posted (9)
proto/dydxprotocol/vault/genesis.proto (3)

6-6: Import statement addition looks good.

The addition of the share.proto import is appropriate given the context of the changes.


13-18: Deprecation and addition of fields are handled correctly.

The params field is deprecated and replaced with default_quoting_params, which is correctly defined as non-nullable.


29-30: Renaming of vault_params to quoting_params is handled correctly.

The quoting_params field is properly defined and the renaming is consistent with the overall changes.

indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/genesis.ts (6)

1-2: Import statements addition looks good.

The addition of the QuotingParams and QuotingParamsSDKType imports is appropriate given the context of the changes.


11-13: Replacement of params with defaultQuotingParams is handled correctly.

The defaultQuotingParams field is properly defined and the replacement is consistent with the overall changes.


20-22: Replacement of params with default_quoting_params is handled correctly.

The default_quoting_params field is properly defined and the replacement is consistent with the overall changes.


35-37: Renaming of vaultParams to quotingParams is handled correctly.

The quotingParams field is properly defined and the renaming is consistent with the overall changes.


53-55: Renaming of vault_params to quoting_params is handled correctly.

The quoting_params field is properly defined and the renaming is consistent with the overall changes.


63-64: Updates to handle new fields in methods are correct.

The encode, decode, and fromPartial methods are correctly updated to handle the new defaultQuotingParams and quotingParams fields.

Also applies to: 74-77, 94-97, 110-110, 121-121, 140-141, 176-176, 206-206

@tqin7 tqin7 merged commit b1cb4d0 into main Aug 2, 2024
32 of 34 checks passed
@tqin7 tqin7 deleted the tq/tra-511 branch August 2, 2024 18:38
@tqin7
Copy link
Contributor Author

tqin7 commented Aug 5, 2024

@Mergifyio backport release/protocol/v6.x

Copy link
Contributor

mergify bot commented Aug 5, 2024

backport release/protocol/v6.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Aug 5, 2024
(cherry picked from commit b1cb4d0)

# Conflicts:
#	protocol/app/upgrades/v5.2.0/upgrade.go
#	protocol/testing/genesis.sh
tqin7 added a commit that referenced this pull request Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants