From 484509608285663c4f83e752675a3c95e32a0b0a Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 19 Apr 2023 05:05:27 +0000 Subject: [PATCH] refactor: refactor x/token,collection query errors (#980) * Refactor x/token * Refactor x/collection * Update CHANGELOG.md * Update the cli test case * Lint --- CHANGELOG.md | 1 + x/collection/client/testutil/query.go | 19 ++- x/collection/keeper/grpc_query.go | 173 +++++-------------- x/collection/keeper/grpc_query_test.go | 224 ++++++++++++++++++------- x/token/keeper/grpc_query.go | 38 ----- x/token/keeper/grpc_query_test.go | 56 +++++-- 6 files changed, 254 insertions(+), 257 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38b54c34cb..59895ed32d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/foundation) [\#952](https://github.com/Finschia/finschia-sdk/pull/952) Address generation of the empty coins in x/foundation * (x/collection,token,foundation) [\#963](https://github.com/Finschia/finschia-sdk/pull/963) Check event determinism on original modules * (x/collection) [\#965](https://github.com/Finschia/finschia-sdk/pull/965) Provide specific error messages on x/collection queries +* (x/collection,token) [\#980](https://github.com/Finschia/finschia-sdk/pull/980) refactor x/token,collection query errors ### Bug Fixes * (swagger) [\#898](https://github.com/Finschia/finschia-sdk/pull/898) fix a bug not added `lbm.tx.v1beta1.Service/GetBlockWithTxs` in swagger diff --git a/x/collection/client/testutil/query.go b/x/collection/client/testutil/query.go index 36033b04b5..592567c363 100644 --- a/x/collection/client/testutil/query.go +++ b/x/collection/client/testutil/query.go @@ -911,26 +911,29 @@ func (s *IntegrationTestSuite) TestNewQueryCmdChildren() { Pagination: &query.PageResponse{}, }, }, - "extra args": { + "token not found": { []string{ s.contractID, - tokenID, - "extra", + collection.NewNFTID("deadbeef", 1), + }, + true, + &collection.QueryChildrenResponse{ + Children: []collection.NFT{}, + Pagination: &query.PageResponse{}, }, - false, - nil, }, - "not enough args": { + "extra args": { []string{ s.contractID, + tokenID, + "extra", }, false, nil, }, - "token not found": { + "not enough args": { []string{ s.contractID, - collection.NewNFTID("deadbeef", 1), }, false, nil, diff --git a/x/collection/keeper/grpc_query.go b/x/collection/keeper/grpc_query.go index 624f3ce362..f2f4ea6086 100644 --- a/x/collection/keeper/grpc_query.go +++ b/x/collection/keeper/grpc_query.go @@ -28,47 +28,39 @@ func NewQueryServer(keeper Keeper) collection.QueryServer { } } -func (s queryServer) validateExistenceOfCollectionGRPC(ctx sdk.Context, id string) error { - if _, err := s.keeper.GetContract(ctx, id); err != nil { - return status.Error(codes.NotFound, err.Error()) +func (s queryServer) addressFromBech32GRPC(bech32 string, context string) (sdk.AccAddress, error) { + addr, err := sdk.AccAddressFromBech32(bech32) + if err != nil { + return nil, status.Error(codes.InvalidArgument, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress.Wrap(bech32), context).Error()) } - return nil + return addr, nil } -func (s queryServer) validateExistenceOfFTClassGRPC(ctx sdk.Context, contractID, classID string) error { +func (s queryServer) assertTokenIsFungible(ctx sdk.Context, contractID string, classID string) error { class, err := s.keeper.GetTokenClass(ctx, contractID, classID) if err != nil { - return status.Error(codes.NotFound, err.Error()) + return err } - _, ok := class.(*collection.FTClass) - if !ok { - return status.Error(codes.NotFound, sdkerrors.ErrInvalidType.Wrapf("not a class of fungible token: %s", classID).Error()) + if _, ok := class.(*collection.FTClass); !ok { + return collection.ErrTokenNotExist.Wrap(collection.NewFTID(classID)) } + return nil } -func (s queryServer) validateExistenceOfNFTClassGRPC(ctx sdk.Context, contractID, classID string) error { +func (s queryServer) assertTokenTypeIsNonFungible(ctx sdk.Context, contractID string, classID string) error { class, err := s.keeper.GetTokenClass(ctx, contractID, classID) if err != nil { - return status.Error(codes.NotFound, err.Error()) + return err } - _, ok := class.(*collection.NFTClass) - if !ok { - return status.Error(codes.NotFound, sdkerrors.ErrInvalidType.Wrapf("not a class of non-fungible token: %s", classID).Error()) + if _, ok := class.(*collection.NFTClass); !ok { + return collection.ErrTokenTypeNotExist.Wrap(classID) } - return nil -} -func (s queryServer) addressFromBech32GRPC(bech32 string, context string) (sdk.AccAddress, error) { - addr, err := sdk.AccAddressFromBech32(bech32) - if err != nil { - return nil, status.Error(codes.InvalidArgument, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress.Wrap(bech32), context).Error()) - } - - return addr, nil + return nil } var _ collection.QueryServer = queryServer{} @@ -157,12 +149,8 @@ func (s queryServer) FTSupply(c context.Context, req *collection.QueryFTSupplyRe ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - - if err := s.validateExistenceOfFTClassGRPC(ctx, req.ContractId, classID); err != nil { - return nil, err + if err := s.assertTokenIsFungible(ctx, req.ContractId, classID); err != nil { + return &collection.QueryFTSupplyResponse{Supply: sdk.ZeroInt()}, nil } supply := s.keeper.GetSupply(ctx, req.ContractId, classID) @@ -187,12 +175,8 @@ func (s queryServer) FTMinted(c context.Context, req *collection.QueryFTMintedRe ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - - if err := s.validateExistenceOfFTClassGRPC(ctx, req.ContractId, classID); err != nil { - return nil, err + if err := s.assertTokenIsFungible(ctx, req.ContractId, classID); err != nil { + return &collection.QueryFTMintedResponse{Minted: sdk.ZeroInt()}, nil } minted := s.keeper.GetMinted(ctx, req.ContractId, classID) @@ -217,12 +201,8 @@ func (s queryServer) FTBurnt(c context.Context, req *collection.QueryFTBurntRequ ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - - if err := s.validateExistenceOfFTClassGRPC(ctx, req.ContractId, classID); err != nil { - return nil, err + if err := s.assertTokenIsFungible(ctx, req.ContractId, classID); err != nil { + return &collection.QueryFTBurntResponse{Burnt: sdk.ZeroInt()}, nil } burnt := s.keeper.GetBurnt(ctx, req.ContractId, classID) @@ -246,12 +226,8 @@ func (s queryServer) NFTSupply(c context.Context, req *collection.QueryNFTSupply ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - - if err := s.validateExistenceOfNFTClassGRPC(ctx, req.ContractId, classID); err != nil { - return nil, err + if err := s.assertTokenTypeIsNonFungible(ctx, req.ContractId, classID); err != nil { + return &collection.QueryNFTSupplyResponse{Supply: sdk.ZeroInt()}, nil } supply := s.keeper.GetSupply(ctx, req.ContractId, classID) @@ -275,12 +251,8 @@ func (s queryServer) NFTMinted(c context.Context, req *collection.QueryNFTMinted ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - - if err := s.validateExistenceOfNFTClassGRPC(ctx, req.ContractId, classID); err != nil { - return nil, err + if err := s.assertTokenTypeIsNonFungible(ctx, req.ContractId, classID); err != nil { + return &collection.QueryNFTMintedResponse{Minted: sdk.ZeroInt()}, nil } minted := s.keeper.GetMinted(ctx, req.ContractId, classID) @@ -304,12 +276,8 @@ func (s queryServer) NFTBurnt(c context.Context, req *collection.QueryNFTBurntRe ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - - if err := s.validateExistenceOfNFTClassGRPC(ctx, req.ContractId, classID); err != nil { - return nil, err + if err := s.assertTokenTypeIsNonFungible(ctx, req.ContractId, classID); err != nil { + return &collection.QueryNFTBurntResponse{Burnt: sdk.ZeroInt()}, nil } burnt := s.keeper.GetBurnt(ctx, req.ContractId, classID) @@ -350,11 +318,6 @@ func (s queryServer) TokenClassTypeName(c context.Context, req *collection.Query } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - class, err := s.keeper.GetTokenClass(ctx, req.ContractId, req.ClassId) if err != nil { return nil, status.Error(codes.NotFound, err.Error()) @@ -379,11 +342,6 @@ func (s queryServer) TokenType(c context.Context, req *collection.QueryTokenType } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - class, err := s.keeper.GetTokenClass(ctx, req.ContractId, classID) if err != nil { return nil, status.Error(codes.NotFound, err.Error()) @@ -459,11 +417,6 @@ func (s queryServer) Token(c context.Context, req *collection.QueryTokenRequest) } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - legacyToken, err := s.getToken(ctx, req.ContractId, req.TokenId) if err != nil { return nil, status.Error(codes.NotFound, err.Error()) @@ -491,11 +444,6 @@ func (s queryServer) Root(c context.Context, req *collection.QueryRootRequest) ( } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - if err := s.keeper.hasNFT(ctx, req.ContractId, req.TokenId); err != nil { return nil, status.Error(codes.NotFound, err.Error()) } @@ -514,12 +462,15 @@ func (s queryServer) HasParent(c context.Context, req *collection.QueryHasParent return nil, status.Error(codes.InvalidArgument, "empty request") } - ctx := sdk.UnwrapSDKContext(c) + if err := collection.ValidateContractID(req.GetContractId()); err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } - if err := s.validateGetParentVariants(ctx, req); err != nil { - return nil, err + if err := collection.ValidateNFTID(req.GetTokenId()); err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } + ctx := sdk.UnwrapSDKContext(c) _, err := s.keeper.GetParent(ctx, req.ContractId, req.TokenId) return &collection.QueryHasParentResponse{HasParent: (err == nil)}, nil } @@ -529,12 +480,15 @@ func (s queryServer) Parent(c context.Context, req *collection.QueryParentReques return nil, status.Error(codes.InvalidArgument, "empty request") } - ctx := sdk.UnwrapSDKContext(c) + if err := collection.ValidateContractID(req.GetContractId()); err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } - if err := s.validateGetParentVariants(ctx, req); err != nil { - return nil, err + if err := collection.ValidateNFTID(req.GetTokenId()); err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) } + ctx := sdk.UnwrapSDKContext(c) parent, err := s.keeper.GetParent(ctx, req.ContractId, req.TokenId) if err != nil { return nil, status.Error(codes.NotFound, err.Error()) @@ -548,31 +502,6 @@ func (s queryServer) Parent(c context.Context, req *collection.QueryParentReques return &collection.QueryParentResponse{Parent: *token}, nil } -type parentRequest interface { - GetContractId() string - GetTokenId() string -} - -func (s queryServer) validateGetParentVariants(ctx sdk.Context, req parentRequest) error { - if err := collection.ValidateContractID(req.GetContractId()); err != nil { - return status.Error(codes.InvalidArgument, err.Error()) - } - - if err := collection.ValidateNFTID(req.GetTokenId()); err != nil { - return status.Error(codes.InvalidArgument, err.Error()) - } - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.GetContractId()); err != nil { - return err - } - - if err := s.keeper.hasNFT(ctx, req.GetContractId(), req.GetTokenId()); err != nil { - return status.Error(codes.NotFound, err.Error()) - } - - return nil -} - func (s queryServer) Children(c context.Context, req *collection.QueryChildrenRequest) (*collection.QueryChildrenResponse, error) { if req == nil { return nil, status.Error(codes.InvalidArgument, "empty request") @@ -587,15 +516,6 @@ func (s queryServer) Children(c context.Context, req *collection.QueryChildrenRe } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - - if err := s.keeper.hasNFT(ctx, req.ContractId, req.TokenId); err != nil { - return nil, status.Error(codes.NotFound, err.Error()) - } - store := ctx.KVStore(s.keeper.storeKey) childStore := prefix.NewStore(store, childKeyPrefixByTokenID(req.ContractId, req.TokenId)) var children []collection.NFT @@ -631,11 +551,6 @@ func (s queryServer) GranteeGrants(c context.Context, req *collection.QueryGrant } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - store := ctx.KVStore(s.keeper.storeKey) grantStore := prefix.NewStore(store, grantKeyPrefixByGrantee(req.ContractId, granteeAddr)) var grants []collection.Grant @@ -673,11 +588,6 @@ func (s queryServer) IsOperatorFor(c context.Context, req *collection.QueryIsOpe } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - _, err = s.keeper.GetAuthorization(ctx, req.ContractId, holder, operator) authorized := (err == nil) @@ -699,11 +609,6 @@ func (s queryServer) HoldersByOperator(c context.Context, req *collection.QueryH } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - store := ctx.KVStore(s.keeper.storeKey) authorizationStore := prefix.NewStore(store, authorizationKeyPrefixByOperator(req.ContractId, operator)) var holders []string diff --git a/x/collection/keeper/grpc_query_test.go b/x/collection/keeper/grpc_query_test.go index 171e30b413..8e6af2dc11 100644 --- a/x/collection/keeper/grpc_query_test.go +++ b/x/collection/keeper/grpc_query_test.go @@ -160,23 +160,35 @@ func (s *KeeperTestSuite) TestQueryFTSupply() { s.Require().Equal(s.balance.Mul(sdk.NewInt(3)), res.Supply) }, }, - "invalid contract id": { - tokenID: tokenID, - }, - "invalid token id": { - contractID: s.contractID, - }, "collection not found": { contractID: "deadbeef", tokenID: tokenID, + valid: true, + postTest: func(res *collection.QueryFTSupplyResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Supply) + }, }, "token not found": { contractID: s.contractID, tokenID: collection.NewFTID("00bab10c"), + valid: true, + postTest: func(res *collection.QueryFTSupplyResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Supply) + }, }, "not a class of ft": { contractID: s.contractID, tokenID: collection.NewFTID(s.nftClassID), + valid: true, + postTest: func(res *collection.QueryFTSupplyResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Supply) + }, + }, + "invalid contract id": { + tokenID: tokenID, + }, + "invalid token id": { + contractID: s.contractID, }, } @@ -218,23 +230,35 @@ func (s *KeeperTestSuite) TestQueryFTMinted() { s.Require().Equal(s.balance.Mul(sdk.NewInt(6)), res.Minted) }, }, - "invalid contract id": { - tokenID: tokenID, - }, - "invalid token id": { - contractID: s.contractID, - }, "collection not found": { contractID: "deadbeef", tokenID: tokenID, + valid: true, + postTest: func(res *collection.QueryFTMintedResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Minted) + }, }, "token not found": { contractID: s.contractID, tokenID: collection.NewFTID("00bab10c"), + valid: true, + postTest: func(res *collection.QueryFTMintedResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Minted) + }, }, "not a class of ft": { contractID: s.contractID, tokenID: collection.NewFTID(s.nftClassID), + valid: true, + postTest: func(res *collection.QueryFTMintedResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Minted) + }, + }, + "invalid contract id": { + tokenID: tokenID, + }, + "invalid token id": { + contractID: s.contractID, }, } @@ -276,23 +300,35 @@ func (s *KeeperTestSuite) TestQueryFTBurnt() { s.Require().Equal(s.balance.Mul(sdk.NewInt(3)), res.Burnt) }, }, - "invalid contract id": { - tokenID: tokenID, - }, - "invalid token id": { - contractID: s.contractID, - }, "collection not found": { contractID: "deadbeef", tokenID: tokenID, + valid: true, + postTest: func(res *collection.QueryFTBurntResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Burnt) + }, }, "token not found": { contractID: s.contractID, tokenID: collection.NewFTID("00bab10c"), + valid: true, + postTest: func(res *collection.QueryFTBurntResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Burnt) + }, }, "not a class of ft": { contractID: s.contractID, tokenID: collection.NewFTID(s.nftClassID), + valid: true, + postTest: func(res *collection.QueryFTBurntResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Burnt) + }, + }, + "invalid contract id": { + tokenID: tokenID, + }, + "invalid token id": { + contractID: s.contractID, }, } @@ -333,23 +369,35 @@ func (s *KeeperTestSuite) TestQueryNFTSupply() { s.Require().EqualValues(s.numNFTs*3, res.Supply.Int64()) }, }, - "invalid contract id": { - tokenType: s.nftClassID, - }, - "invalid token type": { - contractID: s.contractID, - }, "collection not found": { contractID: "deadbeef", tokenType: s.nftClassID, + valid: true, + postTest: func(res *collection.QueryNFTSupplyResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Supply) + }, }, "token type not found": { contractID: s.contractID, tokenType: "deadbeef", + valid: true, + postTest: func(res *collection.QueryNFTSupplyResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Supply) + }, }, "not a class of nft": { contractID: s.contractID, tokenType: s.ftClassID, + valid: true, + postTest: func(res *collection.QueryNFTSupplyResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Supply) + }, + }, + "invalid contract id": { + tokenType: s.nftClassID, + }, + "invalid token type": { + contractID: s.contractID, }, } @@ -390,23 +438,35 @@ func (s *KeeperTestSuite) TestQueryNFTMinted() { s.Require().EqualValues(s.numNFTs*3, res.Minted.Int64()) }, }, - "invalid contract id": { - tokenType: s.nftClassID, - }, - "invalid token type": { - contractID: s.contractID, - }, "collection not found": { contractID: "deadbeef", tokenType: s.nftClassID, + valid: true, + postTest: func(res *collection.QueryNFTMintedResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Minted) + }, }, "token type not found": { contractID: s.contractID, tokenType: "deadbeef", + valid: true, + postTest: func(res *collection.QueryNFTMintedResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Minted) + }, }, "not a class of nft": { contractID: s.contractID, tokenType: s.ftClassID, + valid: true, + postTest: func(res *collection.QueryNFTMintedResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Minted) + }, + }, + "invalid contract id": { + tokenType: s.nftClassID, + }, + "invalid token type": { + contractID: s.contractID, }, } @@ -447,23 +507,35 @@ func (s *KeeperTestSuite) TestQueryNFTBurnt() { s.Require().Equal(sdk.ZeroInt(), res.Burnt) }, }, - "invalid contract id": { - tokenType: s.nftClassID, - }, - "invalid token type": { - contractID: s.contractID, - }, "collection not found": { contractID: "deadbeef", tokenType: s.nftClassID, + valid: true, + postTest: func(res *collection.QueryNFTBurntResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Burnt) + }, }, "token type not found": { contractID: s.contractID, tokenType: "deadbeef", + valid: true, + postTest: func(res *collection.QueryNFTBurntResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Burnt) + }, }, "not a class of nft": { contractID: s.contractID, tokenType: s.ftClassID, + valid: true, + postTest: func(res *collection.QueryNFTBurntResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Burnt) + }, + }, + "invalid contract id": { + tokenType: s.nftClassID, + }, + "invalid token type": { + contractID: s.contractID, }, } @@ -797,19 +869,29 @@ func (s *KeeperTestSuite) TestQueryHasParent() { s.Require().Equal(false, res.HasParent) }, }, - "invalid contract id": { - tokenID: tokenID, - }, - "invalid token id": { - contractID: s.contractID, - }, "collection not found": { contractID: "deadbeef", tokenID: tokenID, + valid: true, + postTest: func(res *collection.QueryHasParentResponse) { + s.Require().NotNil(res) + s.Require().Equal(false, res.HasParent) + }, }, "token not found": { contractID: s.contractID, tokenID: collection.NewNFTID("deadbeef", 1), + valid: true, + postTest: func(res *collection.QueryHasParentResponse) { + s.Require().NotNil(res) + s.Require().Equal(false, res.HasParent) + }, + }, + "invalid contract id": { + tokenID: tokenID, + }, + "invalid token id": { + contractID: s.contractID, }, } @@ -920,19 +1002,27 @@ func (s *KeeperTestSuite) TestQueryChildren() { s.Require().Equal(collection.NewNFTID(s.nftClassID, 2), res.Children[0].TokenId) }, }, - "invalid contract id": { - tokenID: tokenID, - }, - "invalid token id": { - contractID: s.contractID, - }, "collection not found": { contractID: "deadbeef", tokenID: tokenID, + valid: true, + postTest: func(res *collection.QueryChildrenResponse) { + s.Require().Equal(0, len(res.Children)) + }, }, "token not found": { contractID: s.contractID, tokenID: collection.NewNFTID("deadbeef", 1), + valid: true, + postTest: func(res *collection.QueryChildrenResponse) { + s.Require().Equal(0, len(res.Children)) + }, + }, + "invalid contract id": { + tokenID: tokenID, + }, + "invalid token id": { + contractID: s.contractID, }, } @@ -978,16 +1068,20 @@ func (s *KeeperTestSuite) TestQueryGranteeGrants() { s.Require().Equal(4, len(res.Grants)) }, }, + "collection not found": { + contractID: "deadbeef", + grantee: s.vendor, + valid: true, + postTest: func(res *collection.QueryGranteeGrantsResponse) { + s.Require().Equal(0, len(res.Grants)) + }, + }, "invalid contract id": { grantee: s.vendor, }, "invalid grantee": { contractID: s.contractID, }, - "collection not found": { - contractID: "deadbeef", - grantee: s.vendor, - }, } for name, tc := range testCases { @@ -1029,6 +1123,15 @@ func (s *KeeperTestSuite) TestQueryIsOperatorFor() { s.Require().True(res.Authorized) }, }, + "collection not found": { + contractID: "deadbeef", + operator: s.operator, + holder: s.vendor, + valid: true, + postTest: func(res *collection.QueryIsOperatorForResponse) { + s.Require().False(res.Authorized) + }, + }, "invalid contract id": { operator: s.operator, holder: s.customer, @@ -1041,11 +1144,6 @@ func (s *KeeperTestSuite) TestQueryIsOperatorFor() { contractID: s.contractID, operator: s.operator, }, - "collection not found": { - contractID: "deadbeef", - operator: s.operator, - holder: s.vendor, - }, } for name, tc := range testCases { @@ -1096,16 +1194,20 @@ func (s *KeeperTestSuite) TestQueryHoldersByOperator() { s.Require().Equal(1, len(res.Holders)) }, }, + "collection not found": { + contractID: "deadbeef", + operator: s.operator, + valid: true, + postTest: func(res *collection.QueryHoldersByOperatorResponse) { + s.Require().Equal(0, len(res.Holders)) + }, + }, "invalid contract id": { operator: s.operator, }, "invalid operator": { contractID: s.contractID, }, - "collection not found": { - contractID: "deadbeef", - operator: s.operator, - }, } for name, tc := range testCases { diff --git a/x/token/keeper/grpc_query.go b/x/token/keeper/grpc_query.go index 4f0f9913c6..9da18e4c7a 100644 --- a/x/token/keeper/grpc_query.go +++ b/x/token/keeper/grpc_query.go @@ -27,14 +27,6 @@ func NewQueryServer(keeper Keeper) token.QueryServer { var _ token.QueryServer = queryServer{} -func (s queryServer) validateExistenceOfClassGRPC(ctx sdk.Context, id string) error { - if _, err := s.keeper.GetClass(ctx, id); err != nil { - return status.Error(codes.NotFound, err.Error()) - } - - return nil -} - func (s queryServer) addressFromBech32GRPC(address string, context string) (sdk.AccAddress, error) { addr, err := sdk.AccAddressFromBech32(address) if err != nil { @@ -75,11 +67,6 @@ func (s queryServer) Supply(c context.Context, req *token.QuerySupplyRequest) (* } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfClassGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - supply := s.keeper.GetSupply(ctx, req.ContractId) return &token.QuerySupplyResponse{Amount: supply}, nil @@ -96,11 +83,6 @@ func (s queryServer) Minted(c context.Context, req *token.QueryMintedRequest) (* } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfClassGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - minted := s.keeper.GetMinted(ctx, req.ContractId) return &token.QueryMintedResponse{Amount: minted}, nil @@ -117,11 +99,6 @@ func (s queryServer) Burnt(c context.Context, req *token.QueryBurntRequest) (*to } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfClassGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - burnt := s.keeper.GetBurnt(ctx, req.ContractId) return &token.QueryBurntResponse{Amount: burnt}, nil @@ -160,11 +137,6 @@ func (s queryServer) GranteeGrants(c context.Context, req *token.QueryGranteeGra } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfClassGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - store := ctx.KVStore(s.keeper.storeKey) grantStore := prefix.NewStore(store, grantKeyPrefixByGrantee(req.ContractId, grantee)) var grants []token.Grant @@ -201,11 +173,6 @@ func (s queryServer) IsOperatorFor(c context.Context, req *token.QueryIsOperator } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfClassGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - _, err = s.keeper.GetAuthorization(ctx, req.ContractId, holder, operator) authorized := err == nil @@ -226,11 +193,6 @@ func (s queryServer) HoldersByOperator(c context.Context, req *token.QueryHolder } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfClassGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - store := ctx.KVStore(s.keeper.storeKey) authorizationStore := prefix.NewStore(store, authorizationKeyPrefixByOperator(req.ContractId, operator)) var holders []string diff --git a/x/token/keeper/grpc_query_test.go b/x/token/keeper/grpc_query_test.go index 483a392e1c..28f9a650c0 100644 --- a/x/token/keeper/grpc_query_test.go +++ b/x/token/keeper/grpc_query_test.go @@ -69,10 +69,14 @@ func (s *KeeperTestSuite) TestQuerySupply() { s.Require().Equal(s.balance.Mul(sdk.NewInt(3)), res.Amount) }, }, - "invalid contract id": {}, "no such a contract id": { contractID: "fee1dead", + valid: true, + postTest: func(res *token.QuerySupplyResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Amount) + }, }, + "invalid contract id": {}, } for name, tc := range testCases { @@ -109,10 +113,14 @@ func (s *KeeperTestSuite) TestQueryMinted() { s.Require().Equal(s.balance.Mul(sdk.NewInt(4)), res.Amount) }, }, - "invalid contract id": {}, "no such a contract id": { contractID: "fee1dead", + valid: true, + postTest: func(res *token.QueryMintedResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Amount) + }, }, + "invalid contract id": {}, } for name, tc := range testCases { @@ -149,10 +157,14 @@ func (s *KeeperTestSuite) TestQueryBurnt() { s.Require().Equal(s.balance, res.Amount) }, }, - "invalid contract id": {}, "no such a contract id": { contractID: "fee1dead", + valid: true, + postTest: func(res *token.QueryBurntResponse) { + s.Require().Equal(sdk.ZeroInt(), res.Amount) + }, }, + "invalid contract id": {}, } for name, tc := range testCases { @@ -231,16 +243,20 @@ func (s *KeeperTestSuite) TestQueryGranteeGrants() { s.Require().Equal(3, len(res.Grants)) }, }, + "class not found": { + contractID: "fee1dead", + grantee: s.vendor, + valid: true, + postTest: func(res *token.QueryGranteeGrantsResponse) { + s.Require().Equal(0, len(res.Grants)) + }, + }, "invalid contract id": { grantee: s.vendor, }, "invalid grantee": { contractID: s.contractID, }, - "class not found": { - contractID: "fee1dead", - grantee: s.vendor, - }, } for name, tc := range testCases { @@ -282,6 +298,15 @@ func (s *KeeperTestSuite) TestQueryIsOperatorFor() { s.Require().True(res.Authorized) }, }, + "class not found": { + contractID: "fee1dead", + operator: s.operator, + holder: s.vendor, + valid: true, + postTest: func(res *token.QueryIsOperatorForResponse) { + s.Require().False(res.Authorized) + }, + }, "invalid contract id": { operator: s.operator, holder: s.customer, @@ -294,11 +319,6 @@ func (s *KeeperTestSuite) TestQueryIsOperatorFor() { contractID: s.contractID, operator: s.operator, }, - "class not found": { - contractID: "fee1dead", - operator: s.operator, - holder: s.vendor, - }, } for name, tc := range testCases { @@ -349,16 +369,20 @@ func (s *KeeperTestSuite) TestQueryHoldersByOperator() { s.Require().Equal(1, len(res.Holders)) }, }, + "class not found": { + contractID: "fee1dead", + operator: s.operator, + valid: true, + postTest: func(res *token.QueryHoldersByOperatorResponse) { + s.Require().Equal(0, len(res.Holders)) + }, + }, "invalid contract id": { operator: s.operator, }, "invalid operator": { contractID: s.contractID, }, - "class not found": { - contractID: "fee1dead", - operator: s.operator, - }, } for name, tc := range testCases {