Skip to content

Commit 4845096

Browse files
0Tech0Tech
authored
refactor: refactor x/token,collection query errors (#980)
* Refactor x/token * Refactor x/collection * Update CHANGELOG.md * Update the cli test case * Lint
1 parent 3a7a409 commit 4845096

File tree

6 files changed

+254
-257
lines changed

6 files changed

+254
-257
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
4848
* (x/foundation) [\#952](https://github.com/Finschia/finschia-sdk/pull/952) Address generation of the empty coins in x/foundation
4949
* (x/collection,token,foundation) [\#963](https://github.com/Finschia/finschia-sdk/pull/963) Check event determinism on original modules
5050
* (x/collection) [\#965](https://github.com/Finschia/finschia-sdk/pull/965) Provide specific error messages on x/collection queries
51+
* (x/collection,token) [\#980](https://github.com/Finschia/finschia-sdk/pull/980) refactor x/token,collection query errors
5152

5253
### Bug Fixes
5354
* (swagger) [\#898](https://github.com/Finschia/finschia-sdk/pull/898) fix a bug not added `lbm.tx.v1beta1.Service/GetBlockWithTxs` in swagger

‎x/collection/client/testutil/query.go

+11-8
Original file line numberDiff line numberDiff line change
@@ -911,26 +911,29 @@ func (s *IntegrationTestSuite) TestNewQueryCmdChildren() {
911911
Pagination: &query.PageResponse{},
912912
},
913913
},
914-
"extra args": {
914+
"token not found": {
915915
[]string{
916916
s.contractID,
917-
tokenID,
918-
"extra",
917+
collection.NewNFTID("deadbeef", 1),
918+
},
919+
true,
920+
&collection.QueryChildrenResponse{
921+
Children: []collection.NFT{},
922+
Pagination: &query.PageResponse{},
919923
},
920-
false,
921-
nil,
922924
},
923-
"not enough args": {
925+
"extra args": {
924926
[]string{
925927
s.contractID,
928+
tokenID,
929+
"extra",
926930
},
927931
false,
928932
nil,
929933
},
930-
"token not found": {
934+
"not enough args": {
931935
[]string{
932936
s.contractID,
933-
collection.NewNFTID("deadbeef", 1),
934937
},
935938
false,
936939
nil,

‎x/collection/keeper/grpc_query.go

+39-134
Original file line numberDiff line numberDiff line change
@@ -28,47 +28,39 @@ func NewQueryServer(keeper Keeper) collection.QueryServer {
2828
}
2929
}
3030

31-
func (s queryServer) validateExistenceOfCollectionGRPC(ctx sdk.Context, id string) error {
32-
if _, err := s.keeper.GetContract(ctx, id); err != nil {
33-
return status.Error(codes.NotFound, err.Error())
31+
func (s queryServer) addressFromBech32GRPC(bech32 string, context string) (sdk.AccAddress, error) {
32+
addr, err := sdk.AccAddressFromBech32(bech32)
33+
if err != nil {
34+
return nil, status.Error(codes.InvalidArgument, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress.Wrap(bech32), context).Error())
3435
}
3536

36-
return nil
37+
return addr, nil
3738
}
3839

39-
func (s queryServer) validateExistenceOfFTClassGRPC(ctx sdk.Context, contractID, classID string) error {
40+
func (s queryServer) assertTokenIsFungible(ctx sdk.Context, contractID string, classID string) error {
4041
class, err := s.keeper.GetTokenClass(ctx, contractID, classID)
4142
if err != nil {
42-
return status.Error(codes.NotFound, err.Error())
43+
return err
4344
}
4445

45-
_, ok := class.(*collection.FTClass)
46-
if !ok {
47-
return status.Error(codes.NotFound, sdkerrors.ErrInvalidType.Wrapf("not a class of fungible token: %s", classID).Error())
46+
if _, ok := class.(*collection.FTClass); !ok {
47+
return collection.ErrTokenNotExist.Wrap(collection.NewFTID(classID))
4848
}
49+
4950
return nil
5051
}
5152

52-
func (s queryServer) validateExistenceOfNFTClassGRPC(ctx sdk.Context, contractID, classID string) error {
53+
func (s queryServer) assertTokenTypeIsNonFungible(ctx sdk.Context, contractID string, classID string) error {
5354
class, err := s.keeper.GetTokenClass(ctx, contractID, classID)
5455
if err != nil {
55-
return status.Error(codes.NotFound, err.Error())
56+
return err
5657
}
5758

58-
_, ok := class.(*collection.NFTClass)
59-
if !ok {
60-
return status.Error(codes.NotFound, sdkerrors.ErrInvalidType.Wrapf("not a class of non-fungible token: %s", classID).Error())
59+
if _, ok := class.(*collection.NFTClass); !ok {
60+
return collection.ErrTokenTypeNotExist.Wrap(classID)
6161
}
62-
return nil
63-
}
6462

65-
func (s queryServer) addressFromBech32GRPC(bech32 string, context string) (sdk.AccAddress, error) {
66-
addr, err := sdk.AccAddressFromBech32(bech32)
67-
if err != nil {
68-
return nil, status.Error(codes.InvalidArgument, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress.Wrap(bech32), context).Error())
69-
}
70-
71-
return addr, nil
63+
return nil
7264
}
7365

7466
var _ collection.QueryServer = queryServer{}
@@ -157,12 +149,8 @@ func (s queryServer) FTSupply(c context.Context, req *collection.QueryFTSupplyRe
157149

158150
ctx := sdk.UnwrapSDKContext(c)
159151

160-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
161-
return nil, err
162-
}
163-
164-
if err := s.validateExistenceOfFTClassGRPC(ctx, req.ContractId, classID); err != nil {
165-
return nil, err
152+
if err := s.assertTokenIsFungible(ctx, req.ContractId, classID); err != nil {
153+
return &collection.QueryFTSupplyResponse{Supply: sdk.ZeroInt()}, nil
166154
}
167155

168156
supply := s.keeper.GetSupply(ctx, req.ContractId, classID)
@@ -187,12 +175,8 @@ func (s queryServer) FTMinted(c context.Context, req *collection.QueryFTMintedRe
187175

188176
ctx := sdk.UnwrapSDKContext(c)
189177

190-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
191-
return nil, err
192-
}
193-
194-
if err := s.validateExistenceOfFTClassGRPC(ctx, req.ContractId, classID); err != nil {
195-
return nil, err
178+
if err := s.assertTokenIsFungible(ctx, req.ContractId, classID); err != nil {
179+
return &collection.QueryFTMintedResponse{Minted: sdk.ZeroInt()}, nil
196180
}
197181

198182
minted := s.keeper.GetMinted(ctx, req.ContractId, classID)
@@ -217,12 +201,8 @@ func (s queryServer) FTBurnt(c context.Context, req *collection.QueryFTBurntRequ
217201

218202
ctx := sdk.UnwrapSDKContext(c)
219203

220-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
221-
return nil, err
222-
}
223-
224-
if err := s.validateExistenceOfFTClassGRPC(ctx, req.ContractId, classID); err != nil {
225-
return nil, err
204+
if err := s.assertTokenIsFungible(ctx, req.ContractId, classID); err != nil {
205+
return &collection.QueryFTBurntResponse{Burnt: sdk.ZeroInt()}, nil
226206
}
227207

228208
burnt := s.keeper.GetBurnt(ctx, req.ContractId, classID)
@@ -246,12 +226,8 @@ func (s queryServer) NFTSupply(c context.Context, req *collection.QueryNFTSupply
246226

247227
ctx := sdk.UnwrapSDKContext(c)
248228

249-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
250-
return nil, err
251-
}
252-
253-
if err := s.validateExistenceOfNFTClassGRPC(ctx, req.ContractId, classID); err != nil {
254-
return nil, err
229+
if err := s.assertTokenTypeIsNonFungible(ctx, req.ContractId, classID); err != nil {
230+
return &collection.QueryNFTSupplyResponse{Supply: sdk.ZeroInt()}, nil
255231
}
256232

257233
supply := s.keeper.GetSupply(ctx, req.ContractId, classID)
@@ -275,12 +251,8 @@ func (s queryServer) NFTMinted(c context.Context, req *collection.QueryNFTMinted
275251

276252
ctx := sdk.UnwrapSDKContext(c)
277253

278-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
279-
return nil, err
280-
}
281-
282-
if err := s.validateExistenceOfNFTClassGRPC(ctx, req.ContractId, classID); err != nil {
283-
return nil, err
254+
if err := s.assertTokenTypeIsNonFungible(ctx, req.ContractId, classID); err != nil {
255+
return &collection.QueryNFTMintedResponse{Minted: sdk.ZeroInt()}, nil
284256
}
285257

286258
minted := s.keeper.GetMinted(ctx, req.ContractId, classID)
@@ -304,12 +276,8 @@ func (s queryServer) NFTBurnt(c context.Context, req *collection.QueryNFTBurntRe
304276

305277
ctx := sdk.UnwrapSDKContext(c)
306278

307-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
308-
return nil, err
309-
}
310-
311-
if err := s.validateExistenceOfNFTClassGRPC(ctx, req.ContractId, classID); err != nil {
312-
return nil, err
279+
if err := s.assertTokenTypeIsNonFungible(ctx, req.ContractId, classID); err != nil {
280+
return &collection.QueryNFTBurntResponse{Burnt: sdk.ZeroInt()}, nil
313281
}
314282

315283
burnt := s.keeper.GetBurnt(ctx, req.ContractId, classID)
@@ -350,11 +318,6 @@ func (s queryServer) TokenClassTypeName(c context.Context, req *collection.Query
350318
}
351319

352320
ctx := sdk.UnwrapSDKContext(c)
353-
354-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
355-
return nil, err
356-
}
357-
358321
class, err := s.keeper.GetTokenClass(ctx, req.ContractId, req.ClassId)
359322
if err != nil {
360323
return nil, status.Error(codes.NotFound, err.Error())
@@ -379,11 +342,6 @@ func (s queryServer) TokenType(c context.Context, req *collection.QueryTokenType
379342
}
380343

381344
ctx := sdk.UnwrapSDKContext(c)
382-
383-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
384-
return nil, err
385-
}
386-
387345
class, err := s.keeper.GetTokenClass(ctx, req.ContractId, classID)
388346
if err != nil {
389347
return nil, status.Error(codes.NotFound, err.Error())
@@ -459,11 +417,6 @@ func (s queryServer) Token(c context.Context, req *collection.QueryTokenRequest)
459417
}
460418

461419
ctx := sdk.UnwrapSDKContext(c)
462-
463-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
464-
return nil, err
465-
}
466-
467420
legacyToken, err := s.getToken(ctx, req.ContractId, req.TokenId)
468421
if err != nil {
469422
return nil, status.Error(codes.NotFound, err.Error())
@@ -491,11 +444,6 @@ func (s queryServer) Root(c context.Context, req *collection.QueryRootRequest) (
491444
}
492445

493446
ctx := sdk.UnwrapSDKContext(c)
494-
495-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
496-
return nil, err
497-
}
498-
499447
if err := s.keeper.hasNFT(ctx, req.ContractId, req.TokenId); err != nil {
500448
return nil, status.Error(codes.NotFound, err.Error())
501449
}
@@ -514,12 +462,15 @@ func (s queryServer) HasParent(c context.Context, req *collection.QueryHasParent
514462
return nil, status.Error(codes.InvalidArgument, "empty request")
515463
}
516464

517-
ctx := sdk.UnwrapSDKContext(c)
465+
if err := collection.ValidateContractID(req.GetContractId()); err != nil {
466+
return nil, status.Error(codes.InvalidArgument, err.Error())
467+
}
518468

519-
if err := s.validateGetParentVariants(ctx, req); err != nil {
520-
return nil, err
469+
if err := collection.ValidateNFTID(req.GetTokenId()); err != nil {
470+
return nil, status.Error(codes.InvalidArgument, err.Error())
521471
}
522472

473+
ctx := sdk.UnwrapSDKContext(c)
523474
_, err := s.keeper.GetParent(ctx, req.ContractId, req.TokenId)
524475
return &collection.QueryHasParentResponse{HasParent: (err == nil)}, nil
525476
}
@@ -529,12 +480,15 @@ func (s queryServer) Parent(c context.Context, req *collection.QueryParentReques
529480
return nil, status.Error(codes.InvalidArgument, "empty request")
530481
}
531482

532-
ctx := sdk.UnwrapSDKContext(c)
483+
if err := collection.ValidateContractID(req.GetContractId()); err != nil {
484+
return nil, status.Error(codes.InvalidArgument, err.Error())
485+
}
533486

534-
if err := s.validateGetParentVariants(ctx, req); err != nil {
535-
return nil, err
487+
if err := collection.ValidateNFTID(req.GetTokenId()); err != nil {
488+
return nil, status.Error(codes.InvalidArgument, err.Error())
536489
}
537490

491+
ctx := sdk.UnwrapSDKContext(c)
538492
parent, err := s.keeper.GetParent(ctx, req.ContractId, req.TokenId)
539493
if err != nil {
540494
return nil, status.Error(codes.NotFound, err.Error())
@@ -548,31 +502,6 @@ func (s queryServer) Parent(c context.Context, req *collection.QueryParentReques
548502
return &collection.QueryParentResponse{Parent: *token}, nil
549503
}
550504

551-
type parentRequest interface {
552-
GetContractId() string
553-
GetTokenId() string
554-
}
555-
556-
func (s queryServer) validateGetParentVariants(ctx sdk.Context, req parentRequest) error {
557-
if err := collection.ValidateContractID(req.GetContractId()); err != nil {
558-
return status.Error(codes.InvalidArgument, err.Error())
559-
}
560-
561-
if err := collection.ValidateNFTID(req.GetTokenId()); err != nil {
562-
return status.Error(codes.InvalidArgument, err.Error())
563-
}
564-
565-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.GetContractId()); err != nil {
566-
return err
567-
}
568-
569-
if err := s.keeper.hasNFT(ctx, req.GetContractId(), req.GetTokenId()); err != nil {
570-
return status.Error(codes.NotFound, err.Error())
571-
}
572-
573-
return nil
574-
}
575-
576505
func (s queryServer) Children(c context.Context, req *collection.QueryChildrenRequest) (*collection.QueryChildrenResponse, error) {
577506
if req == nil {
578507
return nil, status.Error(codes.InvalidArgument, "empty request")
@@ -587,15 +516,6 @@ func (s queryServer) Children(c context.Context, req *collection.QueryChildrenRe
587516
}
588517

589518
ctx := sdk.UnwrapSDKContext(c)
590-
591-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
592-
return nil, err
593-
}
594-
595-
if err := s.keeper.hasNFT(ctx, req.ContractId, req.TokenId); err != nil {
596-
return nil, status.Error(codes.NotFound, err.Error())
597-
}
598-
599519
store := ctx.KVStore(s.keeper.storeKey)
600520
childStore := prefix.NewStore(store, childKeyPrefixByTokenID(req.ContractId, req.TokenId))
601521
var children []collection.NFT
@@ -631,11 +551,6 @@ func (s queryServer) GranteeGrants(c context.Context, req *collection.QueryGrant
631551
}
632552

633553
ctx := sdk.UnwrapSDKContext(c)
634-
635-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
636-
return nil, err
637-
}
638-
639554
store := ctx.KVStore(s.keeper.storeKey)
640555
grantStore := prefix.NewStore(store, grantKeyPrefixByGrantee(req.ContractId, granteeAddr))
641556
var grants []collection.Grant
@@ -673,11 +588,6 @@ func (s queryServer) IsOperatorFor(c context.Context, req *collection.QueryIsOpe
673588
}
674589

675590
ctx := sdk.UnwrapSDKContext(c)
676-
677-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
678-
return nil, err
679-
}
680-
681591
_, err = s.keeper.GetAuthorization(ctx, req.ContractId, holder, operator)
682592
authorized := (err == nil)
683593

@@ -699,11 +609,6 @@ func (s queryServer) HoldersByOperator(c context.Context, req *collection.QueryH
699609
}
700610

701611
ctx := sdk.UnwrapSDKContext(c)
702-
703-
if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil {
704-
return nil, err
705-
}
706-
707612
store := ctx.KVStore(s.keeper.storeKey)
708613
authorizationStore := prefix.NewStore(store, authorizationKeyPrefixByOperator(req.ContractId, operator))
709614
var holders []string

0 commit comments

Comments
 (0)