-
Notifications
You must be signed in to change notification settings - Fork 129
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-442] Add query for market mapper revenue share params #1746
Conversation
Signed-off-by: Shrenuj Bansal <[email protected]>
WalkthroughThe changes introduce a new method to query market mapper revenue share parameters across various layers of a system, including gRPC service definitions, client implementation, and testing. The new functionality encompasses proto file updates, code generation adjustments, and the addition of corresponding handlers and tests within the backend logic to manage these parameters. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/revshare/types/query.pb.go
is excluded by!**/*.pb.go
Files selected for processing (5)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.rpc.Query.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.ts (1 hunks)
- proto/dydxprotocol/revshare/query.proto (1 hunks)
- protocol/x/revshare/keeper/grpc_query_marketmapper_revenue_params.go (1 hunks)
- protocol/x/revshare/keeper/grpc_query_marketmapper_revenue_params_test.go (1 hunks)
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.ts
[error] 6-6: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
[error] 9-9: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
Additional comments not posted (10)
protocol/x/revshare/keeper/grpc_query_marketmapper_revenue_params.go (1)
10-18
: The implementation ofMarketMapperRevenueShareParams
adheres to Cosmos SDK standards and correctly handles the context conversion and parameter retrieval.proto/dydxprotocol/revshare/query.proto (1)
10-14
: TheQuery
service and its messages are correctly defined. Using(gogoproto.nullable) = false
for theparams
field inQueryMarketMapperRevenueShareParamsResponse
is a good practice to ensure robustness in client code.Also applies to: 16-22
protocol/x/revshare/keeper/grpc_query_marketmapper_revenue_params_test.go (1)
12-30
: The testTestQueryMarketMapperRevenueParams
is well-constructed, thoroughly checking both the error handling and the correctness of the data returned by the query.indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.rpc.Query.ts (3)
7-10
: TheQuery
interface is correctly extended to include the newmarketMapperRevenueShareParams
method. The optional parameter handling is appropriate for this context.
16-22
: TheQueryClientImpl
class correctly implements the new query method, including proper binding in the constructor to handlethis
context correctly. The gRPC request handling is standard and well-implemented.
29-34
: ThecreateRpcQueryExtension
function correctly extends an existingQueryClient
with the newmarketMapperRevenueShareParams
method, following best practices for object extension in TypeScript.indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.ts (4)
21-23
: The functioncreateBaseQueryMarketMapperRevenueShareParams
correctly initializes an empty object. This is consistent with the expected behavior for a base creator function.
25-51
: TheQueryMarketMapperRevenueShareParams
object methods are implemented correctly. Theencode
method returns the writer as expected for an empty message, anddecode
properly handles the protobuf reading process. ThefromPartial
method also correctly returns a base object.
55-58
: The functioncreateBaseQueryMarketMapperRevenueShareParamsResponse
initializes the response object withparams
as undefined, which is appropriate for a base creator function.
61-96
: The methods withinQueryMarketMapperRevenueShareParamsResponse
handle encoding and decoding of the response object effectively. Theencode
method checks for the presence ofparams
before encoding, which is a good practice to avoid errors. Thedecode
method processes the protobuf input correctly, and thefromPartial
method handles partial inputs appropriately.
/** Queries for the default market mapper revenue share params */ | ||
|
||
export interface QueryMarketMapperRevenueShareParams {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider replacing the empty interface with a type alias for clarity and to adhere to TypeScript best practices.
- export interface QueryMarketMapperRevenueShareParams {}
+ export type QueryMarketMapperRevenueShareParams = {};
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 interface QueryMarketMapperRevenueShareParams {} | |
export type QueryMarketMapperRevenueShareParams = {}; |
Tools
Biome
[error] 6-6: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
export interface QueryMarketMapperRevenueShareParams {} | ||
/** Queries for the default market mapper revenue share params */ | ||
|
||
export interface QueryMarketMapperRevenueShareParamsSDKType {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the previous comment, replace this empty interface with a type alias.
- export interface QueryMarketMapperRevenueShareParamsSDKType {}
+ export type QueryMarketMapperRevenueShareParamsSDKType = {};
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 interface QueryMarketMapperRevenueShareParamsSDKType {} | |
export type QueryMarketMapperRevenueShareParamsSDKType = {}; |
Tools
Biome
[error] 9-9: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
Signed-off-by: Shrenuj Bansal <[email protected]>
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
protocol/x/revshare/types/query.pb.go
is excluded by!**/*.pb.go
protocol/x/revshare/types/query.pb.gw.go
is excluded by!**/*.pb.gw.go
Files selected for processing (6)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.lcd.ts (2 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.rpc.Query.ts (2 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.ts (2 hunks)
- proto/dydxprotocol/revshare/query.proto (2 hunks)
- protocol/x/revshare/keeper/grpc_query_marketmapper_revenue_params_test.go (1 hunks)
- protocol/x/revshare/keeper/grpc_query_marketmapper_revshare_details.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- proto/dydxprotocol/revshare/query.proto
- protocol/x/revshare/keeper/grpc_query_marketmapper_revenue_params_test.go
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.ts
[error] 7-7: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
[error] 10-10: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
Additional comments not posted (5)
protocol/x/revshare/keeper/grpc_query_marketmapper_revshare_details.go (1)
19-19
: Simplified return statement inMarketMapperRevShareDetails
.The change from returning a pointer (
&revShareDetails
) to returning the object directly (revShareDetails
) simplifies the code and reduces the risk of nil pointer dereferences, which is a good practice in Go.indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.lcd.ts (1)
2-2
: Enhanced querying capabilities inLCDQueryClient
.The addition of new imports and the method
marketMapperRevenueShareParams
aligns well with the PR's objectives to extend querying capabilities for market mapper revenue share parameters. This method is well-implemented and follows best practices for asynchronous functions in TypeScript.Also applies to: 12-12, 19-22
indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.rpc.Query.ts (1)
4-4
: Implemented new querying method and improved method binding.The addition of the
marketMapperRevenueShareParams
method and its binding in the constructor enhance the querying capabilities and ensure the correct handling of thethis
context in asynchronous operations. The method's implementation is robust, handling encoding, making the RPC request, and decoding the response efficiently.Also applies to: 8-12, 22-22, 26-30, 43-45
indexer/packages/v4-protos/src/codegen/dydxprotocol/revshare/query.ts (2)
7-7
: Consider replacing the empty interface with a type alias.As suggested in the previous review, replacing the empty interface
QueryMarketMapperRevenueShareParams
with a type alias would adhere to TypeScript best practices and improve code clarity.- export interface QueryMarketMapperRevenueShareParams {} + export type QueryMarketMapperRevenueShareParams = {};Tools
Biome
[error] 7-7: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
10-10
: Consider replacing the empty interface with a type alias.Similarly, the empty interface
QueryMarketMapperRevenueShareParamsSDKType
should be replaced with a type alias to enhance code clarity and adhere to TypeScript best practices.- export interface QueryMarketMapperRevenueShareParamsSDKType {} + export type QueryMarketMapperRevenueShareParamsSDKType = {};Tools
Biome
[error] 10-10: An empty interface is equivalent to {}. (lint/suspicious/noEmptyInterface)
Safe fix: Use a type alias instead.
Signed-off-by: Shrenuj Bansal <[email protected]>
Changelist
Add query for market mapper revenue share params
Test Plan
Added tests
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
New Features
Tests
Bug Fixes