feat(x/nft): add new queries for nft modules that support query string #18355
Conversation
WalkthroughWalkthroughThe Cosmos NFT module has been updated to include new RPC methods that allow querying NFT data using query strings. These additions provide alternative ways to request information about NFT balances, owners, supplies, individual NFTs, and classes. The changes are reflected in the service's protocol buffers and the corresponding Go implementation, including tests. The updates are documented in the module's changelog, highlighting the new query capabilities introduced in the system. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (4)
- api/cosmos/nft/v1beta1/query_grpc.pb.go
- client/docs/swagger-ui/swagger.yaml
- x/nft/query.pb.go
- x/nft/query.pb.gw.go
Files selected for processing (3)
- proto/cosmos/nft/v1beta1/query.proto (5 hunks)
- x/nft/keeper/grpc_query.go (5 hunks)
- x/nft/keeper/grpc_query_test.go (5 hunks)
Additional comments: 25
x/nft/keeper/grpc_query.go (5)
34-56: The function
BalanceByQueryStringis similar toBalancebut accepts a request via a query string. Ensure that the query string is properly parsed and validated before being used in the function. Also, consider adding error handling for invalid query strings.81-109: The function
OwnerByQueryStringis similar toOwnerbut accepts a request via a query string. Ensure that the query string is properly parsed and validated before being used in the function. Also, consider adding error handling for invalid query strings.122-138: The function
SupplyByQueryStringis similar toSupplybut accepts a request via a query string. Ensure that the query string is properly parsed and validated before being used in the function. Also, consider adding error handling for invalid query strings.220-242: The function
NFTByQueryStringis similar toNFTbut accepts a request via a query string. Ensure that the query string is properly parsed and validated before being used in the function. Also, consider adding error handling for invalid query strings.260-279: The function
ClassByQueryStringis similar toClassbut accepts a request via a query string. Ensure that the query string is properly parsed and validated before being used in the function. Also, consider adding error handling for invalid query strings.proto/cosmos/nft/v1beta1/query.proto (15)
14-20: The new RPC method
BalanceByQueryStringhas been added. Ensure that the server-side implementation correctly handles the query string parameters.28-30: The new RPC method
OwnerByQueryStringhas been added. Ensure that the server-side implementation correctly handles the query string parameters.38-40: The new RPC method
SupplyByQueryStringhas been added. Ensure that the server-side implementation correctly handles the query string parameters.54-56: The new RPC method
NFTByQueryStringhas been added. Ensure that the server-side implementation correctly handles the query string parameters.64-66: The new RPC method
ClassByQueryStringhas been added. Ensure that the server-side implementation correctly handles the query string parameters.83-90: The new request message type
QueryBalanceByQueryStringRequesthas been added. Ensure that the server-side implementation correctly handles the fields in this message.98-102: The new response message type
QueryBalanceByQueryStringResponsehas been added. Ensure that the server-side implementation correctly populates the fields in this message.113-120: The new request message type
QueryOwnerByQueryStringRequesthas been added. Ensure that the server-side implementation correctly handles the fields in this message.128-132: The new response message type
QueryOwnerByQueryStringResponsehas been added. Ensure that the server-side implementation correctly populates the fields in this message.140-144: The new request message type
QuerySupplyByQueryStringRequesthas been added. Ensure that the server-side implementation correctly handles the fields in this message.152-156: The new response message type
QuerySupplyByQueryStringResponsehas been added. Ensure that the server-side implementation correctly populates the fields in this message.188-195: The new request message type
QueryNFTByQueryStringRequesthas been added. Ensure that the server-side implementation correctly handles the fields in this message.203-207: The new response message type
QueryNFTByQueryStringResponsehas been added. Ensure that the server-side implementation correctly populates the fields in this message.215-219: The new request message type
QueryClassByQueryStringRequesthas been added. Ensure that the server-side implementation correctly handles the fields in this message.227-231: The new response message type
QueryClassByQueryStringResponsehas been added. Ensure that the server-side implementation correctly populates the fields in this message.x/nft/keeper/grpc_query_test.go (5)
83-146: The
TestBalanceByQueryStringfunction tests theBalanceByQueryStringmethod. It covers various scenarios including emptyClassId, invalidOwneraddress, and successful query. The test cases are well-structured and cover the necessary edge cases. However, it would be better to add a test case for a non-existentOwneraddress.237-324: The
TestOwnerByQueryStringfunction tests theOwnerByQueryStringmethod. It covers various scenarios including emptyClassId, emptynft id, non-existentnft id, non-existentClassId, and successful query. The test cases are well-structured and cover the necessary edge cases.409-491: The
TestSupplyByQueryStringfunction tests theSupplyByQueryStringmethod. It covers various scenarios including emptyClassId, non-existentClassId, zero supply, and successful query. The test cases are well-structured and cover the necessary edge cases.708-793: The
TestNFTByQueryStringfunction tests theNFTByQueryStringmethod. It covers various scenarios including emptyClassId, emptynft id, non-existentClassId, non-existentnft id, and successful query. The test cases are well-structured and cover the necessary edge cases.862-927: The
TestClassByQueryStringfunction tests theClassByQueryStringmethod. It covers various scenarios including emptyClassId, non-existentClassId, and successful query. The test cases are well-structured and cover the necessary edge cases.
julienrbrt
changed the title
CHANGELOG.md
Outdated
| @@ -52,6 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||
| * (server) [#17094](https://github.com/cosmos/cosmos-sdk/pull/17094) Add duration `shutdown-grace` for resource clean up (closing database handles) before exit. | |||
| * (x/auth/vesting) [#17810](https://github.com/cosmos/cosmos-sdk/pull/17810) Add the ability to specify a start time for continuous vesting accounts. | |||
| * (baseapp) [#18071](https://github.com/cosmos/cosmos-sdk/pull/18071) Add hybrid handlers to `MsgServiceRouter`. | |||
| * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string.. | |||
There was a problem hiding this comment.
There's a typo in the changelog entry. It should be "Adds" instead of "Addes". Also, there's an extra period at the end of the sentence. Here's the corrected version:
- * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string..
+ * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Adds new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receive requests via query string.Commitable suggestion (Beta)
| * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string.. | |
| * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Adds new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receive requests via query string. | |
| [ISSUE] |
Co-authored-by: Julien Robert <julien@rbrt.fr>
CHANGELOG.md
Outdated
| @@ -52,6 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||
| * (server) [#17094](https://github.com/cosmos/cosmos-sdk/pull/17094) Add duration `shutdown-grace` for resource clean up (closing database handles) before exit. | |||
| * (x/auth/vesting) [#17810](https://github.com/cosmos/cosmos-sdk/pull/17810) Add the ability to specify a start time for continuous vesting accounts. | |||
| * (baseapp) [#18071](https://github.com/cosmos/cosmos-sdk/pull/18071) Add hybrid handlers to `MsgServiceRouter`. | |||
| * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string. | |||
There was a problem hiding this comment.
Minor typo in the changelog entry. It should be "Adds" instead of "Addes".
- * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string.
+ * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Adds new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string.Commitable suggestion (Beta)
| * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string. | |
| * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Adds new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string. |
x/nft/keeper/grpc_query.go
Outdated
There was a problem hiding this comment.
Imho, XXXByQueryString should simply call the other method XXX with the request correctly filled.
This avoids repeating the method logic, which could inevitably differ mistakenly after a while.
CHANGELOG.md
Outdated
| @@ -52,6 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||
| * (server) [#17094](https://github.com/cosmos/cosmos-sdk/pull/17094) Add duration `shutdown-grace` for resource clean up (closing database handles) before exit. | |||
| * (x/auth/vesting) [#17810](https://github.com/cosmos/cosmos-sdk/pull/17810) Add the ability to specify a start time for continuous vesting accounts. | |||
| * (baseapp) [#18071](https://github.com/cosmos/cosmos-sdk/pull/18071) Add hybrid handlers to `MsgServiceRouter`. | |||
| * (x/nft) [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string. | |||
There was a problem hiding this comment.
lgtm, can you delete this and put it under x/nft/CHANGELOG.md
x/nft/CHANGELOG.md
Outdated
| ### Features | ||
|
|
||
| * [#17825](https://github.com/cosmos/cosmos-sdk/pull/17825) Add AutoCLI Options. | ||
| * [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string. |
There was a problem hiding this comment.
Typographical error in the changelog entry. The word "Addes" should be corrected to "Adds", and "receives" should be "receive" to match subject-verb agreement.
- * [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string.
+ * [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Adds new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receive requests via query string.Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| ### Features | |
| * [#17825](https://github.com/cosmos/cosmos-sdk/pull/17825) Add AutoCLI Options. | |
| * [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Addes new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receives request via query string. | |
| ### Features | |
| * [#17825](https://github.com/cosmos/cosmos-sdk/pull/17825) Add AutoCLI Options. | |
| * [#18355](https://github.com/cosmos/cosmos-sdk/pull/18355) Adds new versions for `Balance`, `Owner`, `Supply`, `NFT`, `Class` queries that receive requests via query string. |
There was a problem hiding this comment.
Can you add a since comment above all the messages and rpcs?
| @@ -55,12 +80,27 @@ message QueryBalanceRequest { | |||
| string owner = 2; | |||
| } | |||
|
|
|||
| // QueryBalanceByQueryStringRequest is the request type for the Query/Balance RPC method | |||
| message QueryBalanceByQueryStringRequest { | |||
There was a problem hiding this comment.
| message QueryBalanceByQueryStringRequest { | |
| // | |
| // Since: nft v0.1.1 | |
| message QueryBalanceByQueryStringRequest { |
| @@ -14,16 +14,31 @@ service Query { | |||
| option (google.api.http).get = "/cosmos/nft/v1beta1/balance/{owner}/{class_id}"; | |||
| } | |||
|
|
|||
| // BalancebyQueryString queries the number of NFTs of a given class owned by the owner, same as balanceOf in ERC721 | |||
| rpc BalanceByQueryString(QueryBalanceByQueryStringRequest) returns (QueryBalanceByQueryStringResponse) { | |||
There was a problem hiding this comment.
| rpc BalanceByQueryString(QueryBalanceByQueryStringRequest) returns (QueryBalanceByQueryStringResponse) { | |
| // | |
| // Since: nft v0.1.1 | |
| rpc BalanceByQueryString(QueryBalanceByQueryStringRequest) returns (QueryBalanceByQueryStringResponse) { |
Description
Closes: #17488
Addes new versions for
Balance,Owner,Supply,NFT,Classthat receives request via query string.Tested new funcs
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdmake lintandmake testReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking changeSummary by CodeRabbit
New Features
Documentation