Skip to content

Commit

Permalink
refactor: refactor x/token,collection query errors (Finschia#980)
Browse files Browse the repository at this point in the history 
* Refactor x/token

* Refactor x/collection

* Update CHANGELOG.md

* Update the cli test case

* Lint
  • Loading branch information
@0Tech
0Tech authored Apr 19, 2023
1 parent 3a7a409 commit 4845096
Show file tree
Hide file tree
Showing 6 changed files with 254 additions and 257 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 11 additions & 8 deletions x/collection/client/testutil/query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
173 changes: 39 additions & 134 deletions x/collection/keeper/grpc_query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
}
Expand All @@ -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
}
Expand All @@ -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())
Expand All @@ -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")
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand All @@ -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
Expand Down
Loading

0 comments on commit 4845096

Please sign in to comment.