Add GRPC gateway route for listing module and add hard cap setting #2489
Conversation
Signed-off-by: Shrenuj Bansal <[email protected]>
WalkthroughThe changes involve the addition of an import statement for the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
protocol/x/listing/module.go (1)
80-83: LGTM with a suggestion: Consider more graceful error handling.The changes improve the
RegisterGRPCGatewayRoutesmethod by adding context and error handling. The use ofcontext.Background()is appropriate here. However, consider a more graceful error handling approach instead of panicking, especially if this method might be called during runtime rather than just at initialization.Consider replacing the panic with a logged error or a returned error:
if err := types.RegisterQueryHandlerClient(context.Background(), mux, types.NewQueryClient(clientCtx)); err != nil { // Option 1: Log the error log.Printf("Failed to register query handler client: %v", err) // Option 2: Return the error (requires changing the method signature) // return fmt.Errorf("failed to register query handler client: %w", err) }This change would make the code more robust and easier to manage in production environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- protocol/x/listing/module.go (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
protocol/x/listing/module.go (2)
4-4: LGTM: Context import added correctly.The addition of the
contextimport is appropriate for the changes made in theRegisterGRPCGatewayRoutesmethod.
Line range hint
1-183: Overall assessment: Changes look good with minor suggestion.The modifications to the
RegisterGRPCGatewayRoutesmethod successfully implement the GRPC gateway route for the listing module as intended. The addition of context and error handling improves the robustness of the code. Consider the suggestion for more graceful error handling, but otherwise, the changes are appropriate and well-implemented.
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.lcd.ts (1)
15-21: LGTM: NewmarketsHardCapmethod implemented correctlyThe new
marketsHardCapmethod has been implemented correctly. It follows the existing pattern in the class and properly utilizes the new types imported at the top of the file.For consistency with the existing
listingVaultDepositParamsmethod, consider updating the comment style:- /* Queries for the hard cap number of listed markets */ + /* Queries the hard cap number of listed markets */This minor change would align the comment style with the existing method, improving overall code consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
protocol/x/listing/types/query.pb.gois excluded by!**/*.pb.goprotocol/x/listing/types/query.pb.gw.gois excluded by!**/*.pb.gw.go
📒 Files selected for processing (2)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.lcd.ts (2 hunks)
- proto/dydxprotocol/listing/query.proto (1 hunks)
🧰 Additional context used
🔇 Additional comments (3)
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.lcd.ts (2)
2-2: LGTM: Import statement updated correctlyThe import statement has been properly updated to include the new types
QueryMarketsHardCapandQueryMarketsHardCapResponseSDKType. These additions are consistent with the newmarketsHardCapmethod implementation.
12-12: LGTM: Constructor updated correctlyThe constructor has been properly updated to bind the new
marketsHardCapmethod. This is consistent with the existing pattern and ensures that the method will have the correctthiscontext when invoked.proto/dydxprotocol/listing/query.proto (1)
13-15: LGTM! HTTP mapping added for MarketsHardCap RPC method.The addition of the HTTP GET mapping for the
MarketsHardCapRPC method is a good improvement. It allows the method to be accessed via a RESTful HTTP endpoint, enhancing the API's accessibility and usability.To ensure consistency and completeness, please run the following script:
Please review the output of this script to ensure that:
- The HTTP mapping style is consistent with other RPC methods in the listing module.
- Any relevant API documentation is updated to reflect this new endpoint.
shrenujb
changed the title
Signed-off-by: Shrenuj Bansal <[email protected]>
…2489) Signed-off-by: Shrenuj Bansal <[email protected]> (cherry picked from commit ca15c4d)
|
@Mergifyio backport release/protocol/v7.x |
✅ Backports have been created
|
…ackport #2489) (#2490) Co-authored-by: shrenujb <[email protected]>
Changelist
Add GRPC query handler for listing module
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
marketsHardCap, to query the hard cap number of listed markets.MarketsHardCapRPC method, enabling RESTful access.Bug Fixes