Skip to content

Commit

Permalink
feat!: enhanced grpc interface of nft module (cosmos#10709)
Browse files Browse the repository at this point in the history 
* add grpc NFTsOfOwner method

* add comment

* fix test error

* apply comment from github

* format code

* apply comments from github

* add test case

* assert error msg

* update adr-043 docs

Co-authored-by: Marko <[email protected]>
  • Loading branch information
@tac0turtle
Zhiqiang Zhang and tac0turtle authored Jan 27, 2022
1 parent 9606c16 commit 006650c
Show file tree
Hide file tree
Showing 13 changed files with 394 additions and 458 deletions.
46 changes: 17 additions & 29 deletions docs/architecture/adr-043-nft-module.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ message NFT {
}
```

- `class_id` is the identifier of the NFT class where the NFT belongs; _required_
- `id` is an alphanumeric identifier of the NFT, unique within the scope of its class. It is specified by the creator of the NFT and may be expanded to use DID in the future. `class_id` combined with `id` uniquely identifies an NFT and is used as the primary index for storing the NFT; _required_
- `class_id` is the identifier of the NFT class where the NFT belongs; _required_,`[a-zA-Z][a-zA-Z0-9/:-]{2,100}`
- `id` is an alphanumeric identifier of the NFT, unique within the scope of its class. It is specified by the creator of the NFT and may be expanded to use DID in the future. `class_id` combined with `id` uniquely identifies an NFT and is used as the primary index for storing the NFT; _required_,`[a-zA-Z][a-zA-Z0-9/:-]{2,100}`

```
{class_id}/{id} --> NFT (bytes)
Expand Down Expand Up @@ -175,30 +175,24 @@ The query service methods for the `x/nft` module are:

```proto
service Query {
// Balance queries the number of NFTs of a given class owned by the owner, same as balanceOf in ERC721
rpc Balance(QueryBalanceRequest) returns (QueryBalanceResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/balance/{class_id}/{owner}";
option (google.api.http).get = "/cosmos/nft/v1beta1/balance/{owner}/{class_id}";
}
// Owner queries the owner of the NFT based on its class and id, same as ownerOf in ERC721
rpc Owner(QueryOwnerRequest) returns (QueryOwnerResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/owner/{class_id}/{id}";
}
// Supply queries the number of NFTs of a given class, same as totalSupply in ERC721Enumerable
// Supply queries the number of NFTs from the given class, same as totalSupply of ERC721.
rpc Supply(QuerySupplyRequest) returns (QuerySupplyResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/supply/{class_id}";
}
// NFTsOfClassByOwner queries the NFTs of a given class owned by the owner, similar to tokenOfOwnerByIndex in ERC721Enumerable
rpc NFTsOfClassByOwner(QueryNFTsOfClassByOwnerRequest) returns (QueryNFTsResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/owned_nfts/{class_id}/{owner}";
}
// NFTsOfClass queries all NFTs of a given class, similar to tokenByIndex in ERC721Enumerable
rpc NFTsOfClass(QueryNFTsOfClassRequest) returns (QueryNFTsResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/nfts/{class_id}";
// NFTs queries all NFTs of a given class or owner,choose at least one of the two, similar to tokenByIndex in ERC721Enumerable
rpc NFTs(QueryNFTsRequest) returns (QueryNFTsResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/nfts";
}
// NFT queries an NFT based on its class and id.
Expand All @@ -208,12 +202,12 @@ service Query {
// Class queries an NFT class based on its id
rpc Class(QueryClassRequest) returns (QueryClassResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/classes/{class_id}";
option (google.api.http).get = "/cosmos/nft/v1beta1/classes/{class_id}";
}
// Classes queries all NFT classes
rpc Classes(QueryClassesRequest) returns (QueryClassesResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/classes";
option (google.api.http).get = "/cosmos/nft/v1beta1/classes";
}
}
Expand All @@ -224,7 +218,7 @@ message QueryBalanceRequest {
}
// QueryBalanceResponse is the response type for the Query/Balance RPC method
message QueryBalanceResponse{
message QueryBalanceResponse {
uint64 amount = 1;
}
Expand All @@ -235,7 +229,7 @@ message QueryOwnerRequest {
}
// QueryOwnerResponse is the response type for the Query/Owner RPC method
message QueryOwnerResponse{
message QueryOwnerResponse {
string owner = 1;
}
Expand All @@ -249,20 +243,14 @@ message QuerySupplyResponse {
uint64 amount = 1;
}
// QueryNFTsOfClassByOwnerRequest is the request type for the Query/NFTsOfClassByOwner RPC method
message QueryNFTsOfClassByOwnerRequest {
string class_id = 1;
string owner = 2;
cosmos.base.query.v1beta1.PageResponse pagination = 3;
}
// QueryNFTsOfClassRequest is the request type for the Query/NFTsOfClass RPC method
message QueryNFTsOfClassRequest {
string class_id = 1;
cosmos.base.query.v1beta1.PageResponse pagination = 2;
// QueryNFTstRequest is the request type for the Query/NFTs RPC method
message QueryNFTsRequest {
string class_id = 1;
string owner = 2;
cosmos.base.query.v1beta1.PageRequest pagination = 3;
}
// QueryNFTsResponse is the response type for the Query/NFTsOfClass and Query/NFTsOfClassByOwner RPC methods
// QueryNFTsResponse is the response type for the Query/NFTs RPC methods
message QueryNFTsResponse {
repeated cosmos.nft.v1beta1.NFT nfts = 1;
cosmos.base.query.v1beta1.PageResponse pagination = 2;
Expand Down
14 changes: 7 additions & 7 deletions proto/cosmos/nft/v1beta1/query.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ service Query {
option (google.api.http).get = "/cosmos/nft/v1beta1/supply/{class_id}";
}

// NFTsOfClass queries all NFTs of a given class or optional owner, similar to tokenByIndex in ERC721Enumerable
rpc NFTsOfClass(QueryNFTsOfClassRequest) returns (QueryNFTsOfClassResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/nfts/{class_id}";
// NFTs queries all NFTs of a given class or owner,choose at least one of the two, similar to tokenByIndex in ERC721Enumerable
rpc NFTs(QueryNFTsRequest) returns (QueryNFTsResponse) {
option (google.api.http).get = "/cosmos/nft/v1beta1/nfts";
}

// NFT queries an NFT based on its class and id.
Expand Down Expand Up @@ -77,15 +77,15 @@ message QuerySupplyResponse {
uint64 amount = 1;
}

// QueryNFTsOfClassRequest is the request type for the Query/NFTsOfClass RPC method
message QueryNFTsOfClassRequest {
// QueryNFTstRequest is the request type for the Query/NFTs RPC method
message QueryNFTsRequest {
string class_id = 1;
string owner = 2;
cosmos.base.query.v1beta1.PageRequest pagination = 3;
}

// QueryNFTsOfClassResponse is the response type for the Query/NFTsOfClass and Query/NFTsOfClassByOwner RPC methods
message QueryNFTsOfClassResponse {
// QueryNFTsResponse is the response type for the Query/NFTs RPC methods
message QueryNFTsResponse {
repeated cosmos.nft.v1beta1.NFT nfts = 1;
cosmos.base.query.v1beta1.PageResponse pagination = 2;
}
Expand Down
44 changes: 32 additions & 12 deletions x/nft/client/cli/query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,21 @@ package cli

import (
"fmt"
"strings"

"github.com/spf13/cobra"
"strings"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/version"
"github.com/cosmos/cosmos-sdk/x/nft"
)

// Flag names and values
const (
FlagOwner = "owner"
FlagOwner = "owner"
FlagClassID = "class-id"
)

// GetQueryCmd returns the cli query commands for this module
Expand Down Expand Up @@ -126,8 +128,7 @@ func GetCmdQueryNFT() *cobra.Command {
// GetCmdQueryNFTs implements the query nft command.
func GetCmdQueryNFTs() *cobra.Command {
cmd := &cobra.Command{
Use: "nfts [class-id]",
Args: cobra.ExactArgs(1),
Use: "nfts",
Short: "query all NFTs of a given class or owner address.",
Long: strings.TrimSpace(
fmt.Sprintf(`Query all NFTs of a given class or owner address. If owner
Expand All @@ -148,20 +149,38 @@ $ %s query %s nfts <class-id> --owner=<owner>
return err
}

request := &nft.QueryNFTsOfClassRequest{
ClassId: args[0],
Pagination: pageReq,
}

owner, err := cmd.Flags().GetString(FlagOwner)
if err != nil {
return err
}

if len(owner) > 0 {
request.Owner = owner
if _, err := sdk.AccAddressFromBech32(owner); err != nil {
return err
}
}

classID, err := cmd.Flags().GetString(FlagClassID)
if err != nil {
return err
}

if len(classID) > 0 {
if err := nft.ValidateClassID(classID); err != nil {
return err
}
}

if len(owner) == 0 && len(classID) == 0 {
return errors.ErrInvalidRequest.Wrap("must provide at least one of classID or owner")
}

request := &nft.QueryNFTsRequest{
ClassId: classID,
Owner: owner,
Pagination: pageReq,
}
res, err := queryClient.NFTsOfClass(cmd.Context(), request)
res, err := queryClient.NFTs(cmd.Context(), request)
if err != nil {
return err
}
Expand All @@ -171,6 +190,7 @@ $ %s query %s nfts <class-id> --owner=<owner>
flags.AddQueryFlagsToCmd(cmd)
flags.AddPaginationFlagsToCmd(cmd, "nfts")
cmd.Flags().String(FlagOwner, "", "The owner of the nft")
cmd.Flags().String(FlagClassID, "", "The class-id of the nft")
return cmd
}

Expand Down
84 changes: 21 additions & 63 deletions x/nft/client/testutil/grpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ func (s *IntegrationTestSuite) TestQueryOwnerGRPC() {
var result nft.QueryOwnerResponse
err = val.ClientCtx.Codec.UnmarshalJSON(resp, &result)
s.Require().NoError(err)
s.Require().EqualValues(tc.expectResult, result.Owner)
}
})
}
Expand Down Expand Up @@ -228,7 +229,7 @@ func (s *IntegrationTestSuite) TestQuerySupplyGRPC() {
}
}

func (s *IntegrationTestSuite) TestQueryNFTsByOwnerGRPC() {
func (s *IntegrationTestSuite) TestQueryNFTsGRPC() {
val := s.network.Validators[0]
testCases := []struct {
name string
Expand All @@ -241,125 +242,82 @@ func (s *IntegrationTestSuite) TestQueryNFTsByOwnerGRPC() {
expectResult []*nft.NFT
}{
{
name: "class id is invalid",
name: "classID and owner are both empty",
args: struct {
ClassId string
Owner string
}{
ClassId: "invalid_class_id",
Owner: s.owner.String(),
},
}{},
errorMsg: "must provide at least one of classID or owner",
expectErr: true,
errorMsg: "invalid class id",
expectResult: []*nft.NFT{},
},
{
name: "class id does not exist",
name: "classID is invalid",
args: struct {
ClassId string
Owner string
}{
ClassId: "class-id",
Owner: s.owner.String(),
ClassId: "invalid_class_id",
},
expectErr: false,
expectErr: true,
expectResult: []*nft.NFT{},
},
{
name: "owner does not exist",
name: "classID does not exist",
args: struct {
ClassId string
Owner string
}{
ClassId: ExpNFT.ClassId,
Owner: s.owner.String(),
ClassId: "class-id",
},
expectErr: false,
expectResult: []*nft.NFT{},
},
{
name: "nft exist",
name: "success query by classID",
args: struct {
ClassId string
Owner string
}{
ClassId: ExpNFT.ClassId,
Owner: val.Address.String(),
},
expectErr: false,
expectResult: []*nft.NFT{&ExpNFT},
},
}
nftsOfClassURL := val.APIAddress + "/cosmos/nft/v1beta1/nfts/%s?owner=%s"
for _, tc := range testCases {
uri := fmt.Sprintf(nftsOfClassURL, tc.args.ClassId, tc.args.Owner)
s.Run(tc.name, func() {
resp, err := rest.GetRequest(uri)
if tc.expectErr {
s.Require().Contains(string(resp), tc.errorMsg)
} else {
s.Require().NoError(err)
var result nft.QueryNFTsOfClassResponse
err = val.ClientCtx.Codec.UnmarshalJSON(resp, &result)
s.Require().NoError(err)
s.Require().EqualValues(tc.expectResult, result.Nfts)
}
})
}
}

func (s *IntegrationTestSuite) TestQueryNFTsOfClassGRPC() {
val := s.network.Validators[0]
testCases := []struct {
name string
args struct {
ClassId string
}
expectErr bool
errorMsg string
expectResult []*nft.NFT
}{
{
name: "class id is invalid",
args: struct {
ClassId string
}{
ClassId: "invalid_class_id",
},
expectErr: true,
expectResult: []*nft.NFT{},
},
{
name: "class id does not exist",
name: "success query by owner",
args: struct {
ClassId string
Owner string
}{
ClassId: "class-id",
Owner: val.Address.String(),
},
expectErr: false,
expectResult: []*nft.NFT{},
expectResult: []*nft.NFT{&ExpNFT},
},
{
name: "class id exist",
name: "success query by owner and classID",
args: struct {
ClassId string
Owner string
}{
ClassId: ExpNFT.ClassId,
Owner: val.Address.String(),
},
expectErr: false,
expectResult: []*nft.NFT{&ExpNFT},
},
}
nftsOfClassURL := val.APIAddress + "/cosmos/nft/v1beta1/nfts/%s"
nftsOfClassURL := val.APIAddress + "/cosmos/nft/v1beta1/nfts?class_id=%s&owner=%s"
for _, tc := range testCases {
uri := fmt.Sprintf(nftsOfClassURL, tc.args.ClassId)
uri := fmt.Sprintf(nftsOfClassURL, tc.args.ClassId, tc.args.Owner)
s.Run(tc.name, func() {
resp, err := rest.GetRequest(uri)
if tc.expectErr {
s.Require().Contains(string(resp), tc.errorMsg)
} else {
s.Require().NoError(err)
var result nft.QueryNFTsOfClassResponse
var result nft.QueryNFTsResponse
err = val.ClientCtx.Codec.UnmarshalJSON(resp, &result)
s.Require().NoError(err)
s.Require().EqualValues(tc.expectResult, result.Nfts)
Expand Down
Loading

0 comments on commit 006650c

Please sign in to comment.