Skip to content

Conversation

@hmoragrega
Copy link
Contributor

@hmoragrega hmoragrega commented May 20, 2025

Summary by CodeRabbit

  • Chores
    • Improved support for the exchangev2 module, ensuring its features are properly recognized and handled by the client.
  • Bug Fixes
    • Handled edge cases in price parsing to prevent errors with empty or zero values.
  • Enhancements
    • Updated validation rules for order trigger prices and market margin ratios to improve data integrity.
  • New Features
    • Added support for reduce_margin_ratio in derivative market configurations.
    • Introduced a new gas price field in streaming responses to provide additional transaction cost information.

@coderabbitai
Copy link

coderabbitai bot commented May 20, 2025

Walkthrough

The update registers the exchangev2 module's interfaces with the interface registry in the client/chain/context.go file. This registration occurs within the NewTxConfig and NewClientContext functions, as well as through an added import. The exchange module's legacy prefix variables are commented out, and the price parsing function is improved. Validation logic for various message types is adjusted to include a new ReduceMarginRatio field and refine trigger price checks. Protobuf definitions are extended to add reduce_margin_ratio to DerivativeMarket and gas_price to StreamResponse.

Changes

File(s) Change Summary
client/chain/context.go Registered exchangev2 module interfaces in interface registry inside NewTxConfig and NewClientContext; added import for exchangev2.
chain/exchange/types/key.go Commented out legacy prefix variables for conditional spot and limit orders; updated GetPriceFromPaddedPrice to handle empty trimmed strings as zero.
chain/exchange/types/msgs.go Modified SpotOrder validation to allow only nil or zero TriggerPrice; renamed local variable for source subaccount ID in MsgExternalTransfer for consistency.
chain/exchange/types/v2/msgs.go Extended MsgUpdateDerivativeMarket validation to include ReduceMarginRatio; changed margin relation checks to use AND; added validation for ReduceMarginRatio in MsgInstantPerpetualMarketLaunch.
proto/injective/exchange/v1beta1/exchange.proto Added reduce_margin_ratio field to DerivativeMarket protobuf message.
proto/injective/stream/v2/query.proto Added optional gas_price field to StreamResponse protobuf message.

Poem

🐇 A hop, a skip, a jump anew,
Legacy prefixes bid adieu.
Prices trimmed, now zero true,
Messages check margins through and through.
Protos grow with fields so bright,
Gas price streams in clear daylight,
CodeRabbit hops with pure delight! 🌿

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (1.64.8)

Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2
Failed executing command with error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
Learn more here.


Note

⚡️ Faster reviews with caching

CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.
Enjoy the performance boost—your workflow just got faster.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 6ff6254 and a363d3a.

⛔ Files ignored due to path filters (2)
  • chain/exchange/types/exchange.pb.go is excluded by !**/*.pb.go
  • chain/stream/types/v2/query.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (5)
  • chain/exchange/types/key.go (2 hunks)
  • chain/exchange/types/msgs.go (3 hunks)
  • chain/exchange/types/v2/msgs.go (3 hunks)
  • proto/injective/exchange/v1beta1/exchange.proto (1 hunks)
  • proto/injective/stream/v2/query.proto (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • proto/injective/stream/v2/query.proto
  • proto/injective/exchange/v1beta1/exchange.proto
🧰 Additional context used
🧬 Code Graph Analysis (2)
chain/exchange/types/msgs.go (1)
chain/exchange/types/common_utils.go (2)
  • GetSubaccountIDOrDeriveFromNonce (203-213)
  • SubaccountIDToSdkAddress (241-243)
chain/exchange/types/v2/msgs.go (2)
chain/exchange/types/errors.go (1)
  • ErrMarginsRelation (53-53)
chain/exchange/types/params.go (1)
  • ValidateMarginRatio (429-446)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: run-tests
  • GitHub Check: lint
🔇 Additional comments (13)
chain/exchange/types/key.go (2)

104-108: Legacy code comment identifies deprecated prefixes

The code properly comments out old legacy prefixes for spot conditional orders while keeping them in the code for reference. This approach maintains backward compatibility for documentation purposes while disabling their active use.


342-345: Good edge case handling for empty price strings

The added check properly handles the edge case where the price string becomes empty after trimming zeros and decimal points. This prevents potential errors when calling math.LegacyMustNewDecFromStr() with an empty string.

chain/exchange/types/msgs.go (3)

300-302: Simplified trigger price validation

The validation for non-conditional orders' trigger price has been simplified. Now it only allows a nil or zero trigger price, which is clearer and more restrictive than the previous validation that checked for negative or excessively large values.


1395-1398: Variable naming convention improvement

The variable naming has been updated from sourceSubaccountId to sourceSubaccountID for consistency with the codebase's naming convention where "ID" is capitalized.


1408-1410: Consistent variable naming use

The renamed variable sourceSubaccountID is now consistently used in this line, which completes the naming improvement started above.

chain/exchange/types/v2/msgs.go (8)

217-218: Change adds checks for the new ReduceMarginRatio field.

The update correctly includes the HasReduceMarginRatioUpdate() check in the hasNoUpdate condition, ensuring that users can update the ReduceMarginRatio field through this message.


258-262: LGTM - Proper validation for the new field.

This validation correctly applies the ValidateMarginRatio check to the new ReduceMarginRatio field, ensuring it meets the required constraints for margin ratios (not nil, greater than minimum margin ratio, less than 1).


264-265: Improved validation logic structure.

The validation approach has been refined by separating margin ratio relationship checks, making the code clearer and preventing invalid comparisons between unset values.


271-275: Correct validation of InitialMarginRatio and ReduceMarginRatio relationship.

The check enforces that ReduceMarginRatio must not be less than InitialMarginRatio, following the required relationship: MaintenanceMarginRatio < InitialMarginRatio <= ReduceMarginRatio.


277-281: New validation for margin ratio relationships.

This check ensures that ReduceMarginRatio is not less than MaintenanceMarginRatio, completing the validation of the three-way relationship between margin ratios.


322-324: LGTM - Added helper method for the new field.

The HasReduceMarginRatioUpdate() method correctly checks if the NewReduceMarginRatio field has a valid non-zero value, following the pattern used for other fields.


669-671: Added validation for ReduceMarginRatio in perpetal market launch.

The validation correctly applies the same margin ratio validation to the MsgInstantPerpetualMarketLaunch message.


678-680: Validate ReduceMarginRatio relationship in market launch.

The validation ensures that ReduceMarginRatio is not less than InitialMarginRatio, maintaining consistency with the update message validation.


🪧 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>, please review it.
    • Explain this complex logic.
    • 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 explain this code block.
    • @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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @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.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @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 generate sequence diagram to generate a sequence diagram of the changes in this 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.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

Base automatically changed from feat/add_exchange_v2_support to dev May 22, 2025 19:23
@aarmoa aarmoa changed the base branch from dev to master May 23, 2025 02:18
@aarmoa aarmoa changed the base branch from master to dev May 23, 2025 02:18
@aarmoa
Copy link
Collaborator

aarmoa commented May 23, 2025

@hmoragrega when you have a moment please check if we still need this PR after the latest updates in the dev branch

@hmoragrega hmoragrega requested a review from aarmoa May 27, 2025 10:40
@hmoragrega hmoragrega closed this May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants