-
Notifications
You must be signed in to change notification settings - Fork 170
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
feat: query RegisteredTokenMarkets #2369
Conversation
WalkthroughThe update introduces a new feature in the Umee leverage module, allowing users to query a list of registered tokens and their market summaries. This is achieved through a new gRPC service, along with associated message types for the request and response. The backend logic to handle these queries has been implemented, and tests have been added to ensure the functionality works as expected. 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? TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- x/leverage/types/query.pb.go
- x/leverage/types/query.pb.gw.go
Files selected for processing (3)
- proto/umee/leverage/v1/query.proto (2 hunks)
- x/leverage/keeper/grpc_query.go (1 hunks)
- x/leverage/keeper/grpc_query_test.go (1 hunks)
Additional comments: 4
proto/umee/leverage/v1/query.proto (4)
27-31: The addition of the
RegisteredTokenMarkets
RPC service with its corresponding HTTP GET route is correctly implemented and follows the existing pattern of service definitions in the file.121-123: The
QueryRegisteredTokenMarkets
message type is correctly defined for a request that requires no parameters.125-129: The
QueryRegisteredTokenMarketsResponse
message type is well-defined with a repeated field ofTokenMarket
to encapsulate the response data for the registered token markets.131-137: The
TokenMarket
message type is properly structured to include both the token details and its market summary, ensuring a comprehensive response for theRegisteredTokenMarkets
RPC service.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional comments: 3
CHANGELOG.md (3)
49-51: The entry for PR 2369 correctly documents the addition of the
RegisteredTokenMarkets
query feature. This aligns with the PR objectives and AI-generated summary.57-57: Correction of the PR number from 2215 to 2315 for the MaxBorrow and MaxWithdraw reliability improvement is noted and approved.
58-58: The entry for PR 2346 correctly documents the bug fix where metokens were not included in historic data.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2369 +/- ##
==========================================
- Coverage 75.38% 69.56% -5.83%
==========================================
Files 100 172 +72
Lines 8025 12780 +4755
==========================================
+ Hits 6050 8890 +2840
- Misses 1589 3282 +1693
- Partials 386 608 +222
|
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.
LGTM
@toteki @robert-zaremba WDYT may be we can add this query as optional integration (customizable query) to binary |
Since this doesn't iterate over users, it doesn't need to be enabled by flag like the |
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- x/leverage/types/query.pb.go
- x/uibc/uibc.pb.go
Files selected for processing (3)
- proto/umee/leverage/v1/query.proto (2 hunks)
- x/leverage/keeper/grpc_query.go (1 hunks)
- x/leverage/keeper/grpc_query_test.go (1 hunks)
Additional comments: 4
proto/umee/leverage/v1/query.proto (4)
27-31: The new gRPC service
RegisteredTokenMarkets
is well-defined with appropriate HTTP GET path annotations. Ensure that the corresponding server implementation handles this endpoint correctly and that the endpoint is documented in any user-facing materials.121-123: The message
QueryRegisteredTokenMarkets
is defined without any fields. If this is intentional (e.g., the query does not require any parameters), this is fine. Otherwise, consider if any parameters should be included to filter or customize the query results.125-128: The message
QueryRegisteredTokenMarketsResponse
correctly uses a repeated field ofTokenMarket
to represent multiple market summaries. Ensure that the implementation correctly populates this message with the expected data.131-137: The
TokenMarket
message is well-structured, combining aToken
and its market summary. Verify that theToken
andQueryMarketSummaryResponse
messages provide all necessary information and that there are no missing fields that might be required for a complete market summary.Verification successful
Based on the provided output from the shell scripts, we can see the fields defined in the
Token
andQueryMarketSummaryResponse
messages:For
Token
:
base_denom
: The base denomination of the token.reserve_factor
: The portion of accrued interest that goes to reserves when the token is borrowed.For
QueryMarketSummaryResponse
:
symbol_denom
: The human-readable representation of a token denom.exponent
: The power of ten required to get from base denom to symbol denom.oracle_price
: The current USD value of a token, which can be nil if the oracle is down.uToken Exchange Rate
: The amount of base tokens received when withdrawing 1 uToken.These fields appear to provide a comprehensive overview of a token's parameters and its market summary. However, without domain-specific knowledge of the Umee blockchain's requirements for a market summary, it's not possible to definitively say whether all necessary information is present. The fields listed seem to cover basic financial and identification aspects of a token and its market status.
Therefore, based on the information available, the
TokenMarket
message structure, which includes bothToken
andQueryMarketSummaryResponse
, seems to be well-structured and likely provides the necessary information for a market summary as per the design of the Umee blockchain's leverage module.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the Token and QueryMarketSummaryResponse messages are complete and provide all necessary information. ast-grep --lang proto --pattern $'message Token { $$$ }' ast-grep --lang proto --pattern $'message QueryMarketSummaryResponse { $$$ }'Length of output: 362
Script:
#!/bin/bash # Search for the definitions of `Token` and `QueryMarketSummaryResponse` messages in .proto files. rg 'message Token {' rg 'message QueryMarketSummaryResponse {'Length of output: 296
Script:
#!/bin/bash # Search for the definitions of `Token` and `QueryMarketSummaryResponse` messages in .proto files, escaping curly braces. rg 'message Token \{' rg 'message QueryMarketSummaryResponse \{'Length of output: 194
Script:
#!/bin/bash # Display the contents of the `Token` and `QueryMarketSummaryResponse` message definitions. cat proto/umee/leverage/v1/leverage.proto | grep -A 10 'message Token {' cat proto/umee/leverage/v1/query.proto | grep -A 10 'message QueryMarketSummaryResponse {'Length of output: 1478
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.
utACK. Left one comment to discuss the API
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- proto/umee/leverage/v1/query.proto (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/umee/leverage/v1/query.proto
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- x/leverage/types/query.pb.go
- x/leverage/types/query.pb.gw.go
Files selected for processing (3)
- proto/umee/leverage/v1/query.proto (2 hunks)
- x/leverage/keeper/grpc_query.go (1 hunks)
- x/leverage/keeper/grpc_query_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- proto/umee/leverage/v1/query.proto
- x/leverage/keeper/grpc_query.go
- x/leverage/keeper/grpc_query_test.go
Summary by CodeRabbit
New Features
Documentation
Tests