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

[TRA-182] rename order_size_ppm to order_size_pct_ppm in vault params #1271

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

tqin7
Copy link
Contributor

@tqin7 tqin7 commented Mar 28, 2024

Changelist

rename order_size_ppm to order_size_pct_ppm in vault params to be clearer that order_size_ppm is not the actual order size

Test Plan

N/A

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.

Copy link

linear bot commented Mar 28, 2024

Copy link

coderabbitai bot commented Mar 28, 2024

Walkthrough

The update primarily involves renaming the orderSizePpm field to orderSizePctPpm across various files in the codebase. This change was made to enhance consistency and clarity in the naming convention. It affects TypeScript and Go source files, including protocol definitions, error handling, and tests within the vault component of the dYdX protocol. The modifications ensure that the term orderSizePctPpm is uniformly used throughout the codebase, reflecting its purpose more accurately.

Changes

Files Change Summary
.../v4-protos/.../params.ts, .../params.proto Renamed orderSizePpm to orderSizePctPpm for consistency in proto definitions.
.../testutil/constants/genesis.go, .../x/vault/types/errors.go Updated order_size_ppm to order_size_pct_ppm in error handling and genesis state.
.../x/vault/keeper/..., .../x/vault/types/... Renamed OrderSizePpm to OrderSizePctPpm in various functions and tests for clarity and consistency.

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-tests 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 tests 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 tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

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

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • 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/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link

@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.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 3555a49 and d49f215.
Files ignored due to path filters (3)
  • protocol/app/testdata/default_genesis_state.json is excluded by !**/*.json
  • protocol/scripts/genesis/sample_pregenesis.json is excluded by !**/*.json
  • protocol/x/vault/types/params.pb.go is excluded by !**/*.pb.go
Files selected for processing (10)
  • indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/params.ts (6 hunks)
  • proto/dydxprotocol/vault/params.proto (1 hunks)
  • protocol/testutil/constants/genesis.go (1 hunks)
  • protocol/x/vault/keeper/msg_server_update_params_test.go (1 hunks)
  • protocol/x/vault/keeper/orders.go (2 hunks)
  • protocol/x/vault/keeper/orders_test.go (4 hunks)
  • protocol/x/vault/keeper/params_test.go (2 hunks)
  • protocol/x/vault/types/errors.go (1 hunks)
  • protocol/x/vault/types/params.go (2 hunks)
  • protocol/x/vault/types/params_test.go (2 hunks)
Path instructions used (10)
proto/dydxprotocol/vault/params.proto (1)

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

protocol/x/vault/types/errors.go (1)

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

protocol/testutil/constants/genesis.go (1)

/
Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

Additional comments (25)
proto/dydxprotocol/vault/params.proto (1)
  • 24-24: The renaming of the field from order_size_ppm to order_size_pct_ppm clarifies its purpose as representing a percentage in parts per million, aligning with the PR's objectives. This change maintains backward compatibility by not altering the field number.
protocol/x/vault/types/params.go (2)
  • 12-12: Renaming the field from OrderSizePpm to OrderSizePctPpm in the Params struct aligns with the PR's objective to clarify the parameter's purpose. This change is correctly applied throughout the file.
  • 28-29: The validation logic correctly uses the new field name OrderSizePctPpm, ensuring that the parameter is validated as intended.
protocol/x/vault/types/errors.go (1)
  • 43-46: Renaming the error variable from ErrInvalidOrderSizePpm to ErrInvalidOrderSizePctPpm and updating its message enhances clarity, aligning with the PR's objectives.
protocol/x/vault/keeper/params_test.go (2)
  • 26-26: The update of the field name to OrderSizePctPpm in the test case aligns with the changes made in the main code, ensuring the test reflects the current codebase accurately.
  • 39-39: Correctly updating the field name in the test case for invalid parameters ensures that the test remains relevant and accurate.
protocol/x/vault/keeper/msg_server_update_params_test.go (2)
  • 44-44: Updating the field name to OrderSizePctPpm in the test function aligns with the changes made in the main code, ensuring the test accurately reflects the current codebase.
  • 48-48: The error message update to use the new error variable name ErrInvalidOrderSizePctPpm ensures that the test accurately reflects the updated codebase.
protocol/x/vault/types/params_test.go (4)
  • 27-27: The update of the field name to OrderSizePctPpm in the test case aligns with the changes made in the main code, ensuring the test reflects the current codebase accurately.
  • 38-38: Correctly updating the field name in the test case for invalid parameters ensures that the test remains relevant and accurate.
  • 43-49: Updating the field name to OrderSizePctPpm in the test case for a failure scenario aligns with the changes made in the main code, ensuring the test accurately reflects the current codebase.
  • 60-60: Correctly updating the field name in another test case for invalid parameters ensures that the test remains relevant and accurate.
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/params.ts (6)
  • 26-26: Renaming the field from orderSizePpm to orderSizePctPpm in the Params interface aligns with the PR's objective to clarify the parameter's purpose. This change is correctly applied throughout the file.
  • 54-54: The serialization/deserialization logic correctly uses the new field name order_size_pct_ppm, ensuring that the parameter is handled as intended.
  • 66-66: Correctly initializing the new field name in the createBaseParams function ensures that the default values are set accurately.
  • 89-90: The encoding function correctly handles the new field name, ensuring that the parameter is serialized accurately.
  • 126-126: The decoding function correctly handles the new field name, ensuring that the parameter is deserialized accurately.
  • 148-148: Correctly handling the new field name in the fromPartial function ensures that partial objects are constructed accurately.
protocol/x/vault/keeper/orders.go (2)
  • 192-192: Updating the parameter name to OrderSizePctPpm in the GetVaultClobOrders function aligns with the PR's objective to clarify the parameter's purpose. This change is correctly applied, ensuring accurate calculation logic.
  • 231-231: Correctly handling the new parameter name in the calculation logic ensures that the function operates as intended with the updated parameter.
protocol/x/vault/keeper/orders_test.go (4)
  • 257-257: The variable OrderSizePpm has been correctly renamed to OrderSizePctPpm in the TestGetVaultClobOrders function. This change aligns with the PR's objective to enhance clarity in naming conventions. However, ensure that all references to this variable across the project, including documentation and comments, are updated to reflect this change.
  • 276-276: The comment explaining the calculation of order size has been updated to reflect the new variable name order_size_pct_ppm. This is a good practice as it maintains the accuracy and relevance of the documentation inline with the code changes. Always ensure that comments are kept up-to-date with the code logic to aid future maintainability.
  • 315-315: The variable OrderSizePpm has been correctly renamed to OrderSizePctPpm in another instance within the TestGetVaultClobOrders function. This consistent application of the new naming convention across the file is commendable and improves code clarity.
  • 334-334: Again, the comment has been updated to reflect the new variable name order_size_pct_ppm, which is consistent with the changes made in the code. This attention to detail in keeping the comments aligned with the code changes is crucial for maintaining code readability and understandability.
protocol/testutil/constants/genesis.go (1)
  • 1501-1501: The renaming of order_size_ppm to order_size_pct_ppm enhances clarity in the naming convention, making it explicit that the value represents a percentage in parts per million format. Ensure that all parts of the system that parse or access this JSON data are updated to use the new key name. This includes any parsing logic, data access patterns, and documentation that references the old key name.

@tqin7 tqin7 merged commit 31a0fb7 into main Mar 28, 2024
31 of 32 checks passed
@tqin7 tqin7 deleted the tq/tra-182 branch March 28, 2024 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants