From 029c26a68040d288344b46233df614ba685f96dc Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Sun, 9 Apr 2023 11:48:23 +0900 Subject: [PATCH 01/10] Do not check existence of collection in Query/Balance --- x/collection/keeper/grpc_query.go | 4 ---- x/collection/keeper/grpc_query_test.go | 5 ----- 2 files changed, 9 deletions(-) diff --git a/x/collection/keeper/grpc_query.go b/x/collection/keeper/grpc_query.go index 131fb14485..a29bf710ce 100644 --- a/x/collection/keeper/grpc_query.go +++ b/x/collection/keeper/grpc_query.go @@ -99,10 +99,6 @@ func (s queryServer) Balance(c context.Context, req *collection.QueryBalanceRequ return nil, err } - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - balance := s.keeper.GetBalance(ctx, req.ContractId, addr, req.TokenId) coin := collection.Coin{ TokenId: req.TokenId, diff --git a/x/collection/keeper/grpc_query_test.go b/x/collection/keeper/grpc_query_test.go index a5c6f25bc6..1a66316bec 100644 --- a/x/collection/keeper/grpc_query_test.go +++ b/x/collection/keeper/grpc_query_test.go @@ -64,11 +64,6 @@ func (s *KeeperTestSuite) TestQueryBalance() { address: sdk.AccAddress("notfound"), tokenID: tokenID, }, - "collection not found": { - contractID: "deadbeef", - address: s.vendor, - tokenID: tokenID, - }, } for name, tc := range testCases { From 00b8bc24f5c86a1f1dfcb8c16118f977c99e7236 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Sun, 9 Apr 2023 13:41:06 +0900 Subject: [PATCH 02/10] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 209c622739..d1036f44e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/collection) [\#953](https://github.com/line/lbm-sdk/pull/953) Allow zero amount of coin in x/collection Query/Balance * (x/collection) [\#954](https://github.com/line/lbm-sdk/pull/954) Remove duplicated events in x/collection Msg/Modify * (x/collection) [\#955](https://github.com/line/lbm-sdk/pull/955) Return nil where the parent not exists in x/collection Query/Parent +* (x/collection) [\#957](https://github.com/line/lbm-sdk/pull/957) Do not check existence of collection in Query/Balance ### Removed From d4dbad6ff150742578ed4707cc635a5412066ee1 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 12 Apr 2023 10:40:52 +0900 Subject: [PATCH 03/10] Revert "Update CHANGELOG.md" This reverts commit 00b8bc24f5c86a1f1dfcb8c16118f977c99e7236. --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1036f44e2..209c622739 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/collection) [\#953](https://github.com/line/lbm-sdk/pull/953) Allow zero amount of coin in x/collection Query/Balance * (x/collection) [\#954](https://github.com/line/lbm-sdk/pull/954) Remove duplicated events in x/collection Msg/Modify * (x/collection) [\#955](https://github.com/line/lbm-sdk/pull/955) Return nil where the parent not exists in x/collection Query/Parent -* (x/collection) [\#957](https://github.com/line/lbm-sdk/pull/957) Do not check existence of collection in Query/Balance ### Removed From a9a433206b9b55d79249614ba943c3180efd6798 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 12 Apr 2023 10:40:54 +0900 Subject: [PATCH 04/10] Revert "Do not check existence of collection in Query/Balance" This reverts commit 029c26a68040d288344b46233df614ba685f96dc. --- x/collection/keeper/grpc_query.go | 4 ++++ x/collection/keeper/grpc_query_test.go | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/x/collection/keeper/grpc_query.go b/x/collection/keeper/grpc_query.go index a29bf710ce..131fb14485 100644 --- a/x/collection/keeper/grpc_query.go +++ b/x/collection/keeper/grpc_query.go @@ -99,6 +99,10 @@ func (s queryServer) Balance(c context.Context, req *collection.QueryBalanceRequ return nil, err } + if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { + return nil, err + } + balance := s.keeper.GetBalance(ctx, req.ContractId, addr, req.TokenId) coin := collection.Coin{ TokenId: req.TokenId, diff --git a/x/collection/keeper/grpc_query_test.go b/x/collection/keeper/grpc_query_test.go index 1a66316bec..a5c6f25bc6 100644 --- a/x/collection/keeper/grpc_query_test.go +++ b/x/collection/keeper/grpc_query_test.go @@ -64,6 +64,11 @@ func (s *KeeperTestSuite) TestQueryBalance() { address: sdk.AccAddress("notfound"), tokenID: tokenID, }, + "collection not found": { + contractID: "deadbeef", + address: s.vendor, + tokenID: tokenID, + }, } for name, tc := range testCases { From 2156936b2f0be6b6be697ae5afe1fad45453a17d Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 12 Apr 2023 13:21:57 +0900 Subject: [PATCH 05/10] Revert #956 on x/auth relevant checks --- simapp/app.go | 4 +-- x/collection/expected_keepers.go | 4 --- x/collection/keeper/grpc_query.go | 36 +++----------------------- x/collection/keeper/grpc_query_test.go | 26 +------------------ x/collection/keeper/keeper_test.go | 6 +---- x/collection/module/module.go | 10 +++---- x/token/expected_keepers.go | 4 --- x/token/keeper/grpc_query.go | 32 +++-------------------- x/token/keeper/grpc_query_test.go | 18 ------------- x/token/keeper/keeper_test.go | 5 +--- x/token/module/module.go | 10 +++---- 11 files changed, 19 insertions(+), 136 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index 7ba8b69ba5..d564f165b7 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -384,8 +384,8 @@ func NewSimApp( upgrade.NewAppModule(app.UpgradeKeeper), evidence.NewAppModule(app.EvidenceKeeper), params.NewAppModule(app.ParamsKeeper), - tokenmodule.NewAppModule(appCodec, app.TokenKeeper, app.AccountKeeper), - collectionmodule.NewAppModule(appCodec, app.CollectionKeeper, app.AccountKeeper), + tokenmodule.NewAppModule(appCodec, app.TokenKeeper), + collectionmodule.NewAppModule(appCodec, app.CollectionKeeper), authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry), ) diff --git a/x/collection/expected_keepers.go b/x/collection/expected_keepers.go index f39f7109c6..3de4c66e52 100644 --- a/x/collection/expected_keepers.go +++ b/x/collection/expected_keepers.go @@ -10,8 +10,4 @@ type ( NewID(ctx sdk.Context) string HasID(ctx sdk.Context, id string) bool } - - AuthKeeper interface { - HasAccount(sdk.Context, sdk.AccAddress) bool - } ) diff --git a/x/collection/keeper/grpc_query.go b/x/collection/keeper/grpc_query.go index 131fb14485..28cac27372 100644 --- a/x/collection/keeper/grpc_query.go +++ b/x/collection/keeper/grpc_query.go @@ -17,27 +17,17 @@ import ( ) type queryServer struct { - keeper Keeper - authKeeper collection.AuthKeeper + keeper Keeper } // NewQueryServer returns an implementation of the token QueryServer interface // for the provided Keeper. -func NewQueryServer(keeper Keeper, authKeeper collection.AuthKeeper) collection.QueryServer { +func NewQueryServer(keeper Keeper) collection.QueryServer { return &queryServer{ - keeper: keeper, - authKeeper: authKeeper, + keeper: keeper, } } -func (s queryServer) validateExistenceOfAccountGRPC(ctx sdk.Context, addr sdk.AccAddress) error { - if !s.authKeeper.HasAccount(ctx, addr) { - return status.Error(codes.NotFound, sdkerrors.ErrUnknownAddress.Wrap(addr.String()).Error()) - } - - return nil -} - 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()) @@ -94,15 +84,6 @@ func (s queryServer) Balance(c context.Context, req *collection.QueryBalanceRequ } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfAccountGRPC(ctx, addr); err != nil { - return nil, err - } - - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { - return nil, err - } - balance := s.keeper.GetBalance(ctx, req.ContractId, addr, req.TokenId) coin := collection.Coin{ TokenId: req.TokenId, @@ -604,10 +585,6 @@ func (s queryServer) GranteeGrants(c context.Context, req *collection.QueryGrant ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfAccountGRPC(ctx, granteeAddr); err != nil { - return nil, err - } - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { return nil, err } @@ -650,13 +627,6 @@ func (s queryServer) IsOperatorFor(c context.Context, req *collection.QueryIsOpe ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfAccountGRPC(ctx, operator); err != nil { - return nil, err - } - if err := s.validateExistenceOfAccountGRPC(ctx, holder); err != nil { - return nil, err - } - if err := s.validateExistenceOfCollectionGRPC(ctx, req.ContractId); err != nil { return nil, err } diff --git a/x/collection/keeper/grpc_query_test.go b/x/collection/keeper/grpc_query_test.go index a5c6f25bc6..339ec7f030 100644 --- a/x/collection/keeper/grpc_query_test.go +++ b/x/collection/keeper/grpc_query_test.go @@ -55,20 +55,10 @@ func (s *KeeperTestSuite) TestQueryBalance() { contractID: s.contractID, tokenID: tokenID, }, - "invalid token id": { + "valid token id": { contractID: s.contractID, address: s.vendor, }, - "address not found": { - contractID: s.contractID, - address: sdk.AccAddress("notfound"), - tokenID: tokenID, - }, - "collection not found": { - contractID: "deadbeef", - address: s.vendor, - tokenID: tokenID, - }, } for name, tc := range testCases { @@ -935,10 +925,6 @@ func (s *KeeperTestSuite) TestQueryGranteeGrants() { contractID: "deadbeef", grantee: s.vendor, }, - "grantee not found": { - contractID: s.contractID, - grantee: sdk.AccAddress("notfound"), - }, } for name, tc := range testCases { @@ -997,16 +983,6 @@ func (s *KeeperTestSuite) TestQueryIsOperatorFor() { operator: s.operator, holder: s.vendor, }, - "operator not found": { - contractID: s.contractID, - operator: sdk.AccAddress("notfound"), - holder: s.customer, - }, - "holder not found": { - contractID: s.contractID, - operator: s.operator, - holder: sdk.AccAddress("notfound"), - }, } for name, tc := range testCases { diff --git a/x/collection/keeper/keeper_test.go b/x/collection/keeper/keeper_test.go index 248519197f..8fd829d868 100644 --- a/x/collection/keeper/keeper_test.go +++ b/x/collection/keeper/keeper_test.go @@ -64,7 +64,7 @@ func (s *KeeperTestSuite) SetupTest() { s.goCtx = sdk.WrapSDKContext(s.ctx) s.keeper = app.CollectionKeeper - s.queryServer = keeper.NewQueryServer(s.keeper, app.AccountKeeper) + s.queryServer = keeper.NewQueryServer(s.keeper) s.msgServer = keeper.NewMsgServer(s.keeper) s.depthLimit = 4 @@ -81,10 +81,6 @@ func (s *KeeperTestSuite) SetupTest() { } for i, address := range createRandomAccounts(len(addresses)) { *addresses[i] = address - - // create account - acc := app.AccountKeeper.NewAccountWithAddress(s.ctx, address) - app.AccountKeeper.SetAccount(s.ctx, acc) } s.balance = sdk.NewInt(1000000) diff --git a/x/collection/module/module.go b/x/collection/module/module.go index e2bb57dcf7..49c2f42529 100644 --- a/x/collection/module/module.go +++ b/x/collection/module/module.go @@ -78,15 +78,13 @@ func (b AppModuleBasic) RegisterInterfaces(registry codectypes.InterfaceRegistry type AppModule struct { AppModuleBasic - keeper keeper.Keeper - authKeeper collection.AuthKeeper + keeper keeper.Keeper } // NewAppModule creates a new AppModule object -func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, authKeeper collection.AuthKeeper) AppModule { +func NewAppModule(cdc codec.Codec, keeper keeper.Keeper) AppModule { return AppModule{ - keeper: keeper, - authKeeper: authKeeper, + keeper: keeper, } } @@ -108,7 +106,7 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd // module-specific GRPC queries. func (am AppModule) RegisterServices(cfg module.Configurator) { collection.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServer(am.keeper)) - collection.RegisterQueryServer(cfg.QueryServer(), keeper.NewQueryServer(am.keeper, am.authKeeper)) + collection.RegisterQueryServer(cfg.QueryServer(), keeper.NewQueryServer(am.keeper)) // m := keeper.NewMigrator(am.keeper) // migrations := map[uint64]func(sdk.Context) error{} diff --git a/x/token/expected_keepers.go b/x/token/expected_keepers.go index 295e6d8f9e..bc85027126 100644 --- a/x/token/expected_keepers.go +++ b/x/token/expected_keepers.go @@ -13,8 +13,4 @@ type ( InitGenesis(ctx sdk.Context, data *ClassGenesisState) ExportGenesis(ctx sdk.Context) *ClassGenesisState } - - AuthKeeper interface { - HasAccount(ctx sdk.Context, addr sdk.AccAddress) bool - } ) diff --git a/x/token/keeper/grpc_query.go b/x/token/keeper/grpc_query.go index ab58a09863..15eb102fb8 100644 --- a/x/token/keeper/grpc_query.go +++ b/x/token/keeper/grpc_query.go @@ -14,29 +14,19 @@ import ( ) type queryServer struct { - keeper Keeper - authKeeper token.AuthKeeper + keeper Keeper } // NewQueryServer returns an implementation of the token QueryServer interface // for the provided Keeper. -func NewQueryServer(keeper Keeper, authKeeper token.AuthKeeper) token.QueryServer { +func NewQueryServer(keeper Keeper) token.QueryServer { return &queryServer{ - keeper: keeper, - authKeeper: authKeeper, + keeper: keeper, } } var _ token.QueryServer = queryServer{} -func (s queryServer) validateExistenceOfAccountGRPC(ctx sdk.Context, addr sdk.AccAddress) error { - if !s.authKeeper.HasAccount(ctx, addr) { - return status.Error(codes.NotFound, sdkerrors.ErrUnknownAddress.Wrap(addr.String()).Error()) - } - - return nil -} - 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()) @@ -60,11 +50,6 @@ func (s queryServer) Balance(c context.Context, req *token.QueryBalanceRequest) } ctx := sdk.UnwrapSDKContext(c) - - if err := s.validateExistenceOfAccountGRPC(ctx, addr); err != nil { - return nil, err - } - balance := s.keeper.GetBalance(ctx, req.ContractId, addr) return &token.QueryBalanceResponse{Amount: balance}, nil @@ -167,10 +152,6 @@ func (s queryServer) GranteeGrants(c context.Context, req *token.QueryGranteeGra ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfAccountGRPC(ctx, grantee); err != nil { - return nil, err - } - if err := s.validateExistenceOfClassGRPC(ctx, req.ContractId); err != nil { return nil, err } @@ -212,13 +193,6 @@ func (s queryServer) IsOperatorFor(c context.Context, req *token.QueryIsOperator ctx := sdk.UnwrapSDKContext(c) - if err := s.validateExistenceOfAccountGRPC(ctx, operator); err != nil { - return nil, err - } - if err := s.validateExistenceOfAccountGRPC(ctx, holder); err != nil { - return nil, err - } - if err := s.validateExistenceOfClassGRPC(ctx, req.ContractId); err != nil { return nil, err } diff --git a/x/token/keeper/grpc_query_test.go b/x/token/keeper/grpc_query_test.go index d75ff371cb..a730b57a0e 100644 --- a/x/token/keeper/grpc_query_test.go +++ b/x/token/keeper/grpc_query_test.go @@ -31,10 +31,6 @@ func (s *KeeperTestSuite) TestQueryBalance() { "invalid address": { contractID: s.contractID, }, - "address not found": { - contractID: s.contractID, - address: sdk.AccAddress("notfound"), - }, } for name, tc := range testCases { @@ -245,10 +241,6 @@ func (s *KeeperTestSuite) TestQueryGranteeGrants() { contractID: "fee1dead", grantee: s.vendor, }, - "grantee not found": { - contractID: s.contractID, - grantee: sdk.AccAddress("notfound"), - }, } for name, tc := range testCases { @@ -307,16 +299,6 @@ func (s *KeeperTestSuite) TestQueryIsOperatorFor() { operator: s.operator, holder: s.vendor, }, - "operator not found": { - contractID: s.contractID, - operator: sdk.AccAddress("notfound"), - holder: s.customer, - }, - "holder not found": { - contractID: s.contractID, - operator: s.operator, - holder: sdk.AccAddress("notfound"), - }, } for name, tc := range testCases { diff --git a/x/token/keeper/keeper_test.go b/x/token/keeper/keeper_test.go index 4136ef48fe..02a610b3ea 100644 --- a/x/token/keeper/keeper_test.go +++ b/x/token/keeper/keeper_test.go @@ -57,7 +57,7 @@ func (s *KeeperTestSuite) SetupTest() { s.goCtx = sdk.WrapSDKContext(s.ctx) s.keeper = app.TokenKeeper - s.queryServer = keeper.NewQueryServer(s.keeper, app.AccountKeeper) + s.queryServer = keeper.NewQueryServer(s.keeper) s.msgServer = keeper.NewMsgServer(s.keeper) addresses := []*sdk.AccAddress{ @@ -68,9 +68,6 @@ func (s *KeeperTestSuite) SetupTest() { } for i, address := range createRandomAccounts(len(addresses)) { *addresses[i] = address - - acc := app.AccountKeeper.NewAccountWithAddress(s.ctx, address) - app.AccountKeeper.SetAccount(s.ctx, acc) } s.balance = sdk.NewInt(1000) diff --git a/x/token/module/module.go b/x/token/module/module.go index daf0346e1c..8740819d55 100644 --- a/x/token/module/module.go +++ b/x/token/module/module.go @@ -78,15 +78,13 @@ func (b AppModuleBasic) RegisterInterfaces(registry codectypes.InterfaceRegistry type AppModule struct { AppModuleBasic - keeper keeper.Keeper - authKeeper token.AuthKeeper + keeper keeper.Keeper } // NewAppModule creates a new AppModule object -func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, authKeeper token.AuthKeeper) AppModule { +func NewAppModule(cdc codec.Codec, keeper keeper.Keeper) AppModule { return AppModule{ - keeper: keeper, - authKeeper: authKeeper, + keeper: keeper, } } @@ -108,7 +106,7 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd // module-specific GRPC queries. func (am AppModule) RegisterServices(cfg module.Configurator) { token.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServer(am.keeper)) - token.RegisterQueryServer(cfg.QueryServer(), keeper.NewQueryServer(am.keeper, am.authKeeper)) + token.RegisterQueryServer(cfg.QueryServer(), keeper.NewQueryServer(am.keeper)) // m := keeper.NewMigrator(am.keeper) // migrations := map[uint64]func(sdk.Context) error{} From 5b81b138ddf130295c6af7d57bc83ab10ab0d940 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 12 Apr 2023 13:26:32 +0900 Subject: [PATCH 06/10] Add asset check on Query/HoldersByOperator --- x/collection/keeper/grpc_query.go | 5 +++++ x/collection/keeper/grpc_query_test.go | 4 ++++ x/token/keeper/grpc_query.go | 5 +++++ 3 files changed, 14 insertions(+) diff --git a/x/collection/keeper/grpc_query.go b/x/collection/keeper/grpc_query.go index 28cac27372..f29a292829 100644 --- a/x/collection/keeper/grpc_query.go +++ b/x/collection/keeper/grpc_query.go @@ -652,6 +652,11 @@ 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 339ec7f030..92c378cc43 100644 --- a/x/collection/keeper/grpc_query_test.go +++ b/x/collection/keeper/grpc_query_test.go @@ -1039,6 +1039,10 @@ func (s *KeeperTestSuite) TestQueryHoldersByOperator() { "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 15eb102fb8..7c972144a9 100644 --- a/x/token/keeper/grpc_query.go +++ b/x/token/keeper/grpc_query.go @@ -217,6 +217,11 @@ 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 From 6ce93af0ce7aa21f5c1a700d68c3f1e315130066 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 12 Apr 2023 14:26:54 +0900 Subject: [PATCH 07/10] Wrap non stateful errors with InvalidArgument --- x/collection/keeper/grpc_query.go | 99 +++++++++++++++++-------------- x/token/keeper/grpc_query.go | 41 ++++++++----- 2 files changed, 79 insertions(+), 61 deletions(-) diff --git a/x/collection/keeper/grpc_query.go b/x/collection/keeper/grpc_query.go index f29a292829..1dad6831df 100644 --- a/x/collection/keeper/grpc_query.go +++ b/x/collection/keeper/grpc_query.go @@ -62,6 +62,15 @@ func (s queryServer) validateExistenceOfNFTClassGRPC(ctx sdk.Context, contractID 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 +} + var _ collection.QueryServer = queryServer{} // Balance queries the number of tokens of a given token id owned by the owner. @@ -71,16 +80,16 @@ func (s queryServer) Balance(c context.Context, req *collection.QueryBalanceRequ } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } - addr, err := sdk.AccAddressFromBech32(req.Address) + addr, err := s.addressFromBech32GRPC(req.Address, "address") if err != nil { - return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid address: %s", req.Address) + return nil, err } if err := collection.ValidateTokenID(req.TokenId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -100,12 +109,12 @@ func (s queryServer) AllBalances(c context.Context, req *collection.QueryAllBala } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } - addr, err := sdk.AccAddressFromBech32(req.Address) + addr, err := s.addressFromBech32GRPC(req.Address, "address") if err != nil { - return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid address: %s", req.Address) + return nil, err } ctx := sdk.UnwrapSDKContext(c) @@ -125,7 +134,7 @@ func (s queryServer) AllBalances(c context.Context, req *collection.QueryAllBala return nil }) if err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } return &collection.QueryAllBalancesResponse{Balances: balances, Pagination: pageRes}, nil @@ -137,11 +146,11 @@ func (s queryServer) FTSupply(c context.Context, req *collection.QueryFTSupplyRe } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if err := collection.ValidateTokenID(req.TokenId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } classID := collection.SplitTokenID(req.TokenId) @@ -167,11 +176,11 @@ func (s queryServer) FTMinted(c context.Context, req *collection.QueryFTMintedRe } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if err := collection.ValidateTokenID(req.TokenId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } classID := collection.SplitTokenID(req.TokenId) @@ -197,11 +206,11 @@ func (s queryServer) FTBurnt(c context.Context, req *collection.QueryFTBurntRequ } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if err := collection.ValidateTokenID(req.TokenId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } classID := collection.SplitTokenID(req.TokenId) @@ -227,12 +236,12 @@ func (s queryServer) NFTSupply(c context.Context, req *collection.QueryNFTSupply } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } classID := req.TokenType if err := collection.ValidateClassID(classID); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -256,12 +265,12 @@ func (s queryServer) NFTMinted(c context.Context, req *collection.QueryNFTMinted } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } classID := req.TokenType if err := collection.ValidateClassID(classID); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -285,12 +294,12 @@ func (s queryServer) NFTBurnt(c context.Context, req *collection.QueryNFTBurntRe } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } classID := req.TokenType if err := collection.ValidateClassID(classID); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -314,7 +323,7 @@ func (s queryServer) Contract(c context.Context, req *collection.QueryContractRe } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -333,11 +342,11 @@ func (s queryServer) TokenClassTypeName(c context.Context, req *collection.Query } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if err := collection.ValidateClassID(req.ClassId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -361,12 +370,12 @@ func (s queryServer) TokenType(c context.Context, req *collection.QueryTokenType } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } classID := req.TokenType if err := collection.ValidateClassID(classID); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -437,11 +446,11 @@ func (s queryServer) Token(c context.Context, req *collection.QueryTokenRequest) } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if err := collection.ValidateTokenID(req.TokenId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -464,11 +473,11 @@ func (s queryServer) Root(c context.Context, req *collection.QueryRootRequest) ( } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if err := collection.ValidateNFTID(req.TokenId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -496,11 +505,11 @@ func (s queryServer) Parent(c context.Context, req *collection.QueryParentReques } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if err := collection.ValidateNFTID(req.TokenId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -532,11 +541,11 @@ func (s queryServer) Children(c context.Context, req *collection.QueryChildrenRe } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if err := collection.ValidateNFTID(req.TokenId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -563,7 +572,7 @@ func (s queryServer) Children(c context.Context, req *collection.QueryChildrenRe return nil }) if err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } return &collection.QueryChildrenResponse{Children: children, Pagination: pageRes}, nil @@ -575,12 +584,12 @@ func (s queryServer) GranteeGrants(c context.Context, req *collection.QueryGrant } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } - granteeAddr, err := sdk.AccAddressFromBech32(req.Grantee) + granteeAddr, err := s.addressFromBech32GRPC(req.Grantee, "grantee") if err != nil { - return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid grantee address: %s", req.Grantee) + return nil, err } ctx := sdk.UnwrapSDKContext(c) @@ -601,7 +610,7 @@ func (s queryServer) GranteeGrants(c context.Context, req *collection.QueryGrant return nil }) if err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } return &collection.QueryGranteeGrantsResponse{Grants: grants, Pagination: pageRes}, nil @@ -613,14 +622,14 @@ func (s queryServer) IsOperatorFor(c context.Context, req *collection.QueryIsOpe } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } - operator, err := sdk.AccAddressFromBech32(req.Operator) + operator, err := s.addressFromBech32GRPC(req.Operator, "operator") if err != nil { return nil, err } - holder, err := sdk.AccAddressFromBech32(req.Holder) + holder, err := s.addressFromBech32GRPC(req.Holder, "holder") if err != nil { return nil, err } @@ -643,12 +652,12 @@ func (s queryServer) HoldersByOperator(c context.Context, req *collection.QueryH } if err := collection.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } - operator, err := sdk.AccAddressFromBech32(req.Operator) + operator, err := s.addressFromBech32GRPC(req.Operator, "operator") if err != nil { - return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid operator address: %s", req.Operator) + return nil, err } ctx := sdk.UnwrapSDKContext(c) @@ -666,7 +675,7 @@ func (s queryServer) HoldersByOperator(c context.Context, req *collection.QueryH return nil }) if err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } return &collection.QueryHoldersByOperatorResponse{Holders: holders, Pagination: pageRes}, nil diff --git a/x/token/keeper/grpc_query.go b/x/token/keeper/grpc_query.go index 7c972144a9..f02a372238 100644 --- a/x/token/keeper/grpc_query.go +++ b/x/token/keeper/grpc_query.go @@ -35,6 +35,15 @@ func (s queryServer) validateExistenceOfClassGRPC(ctx sdk.Context, id string) er return nil } +func (s queryServer) addressFromBech32GRPC(address string, context string) (sdk.AccAddress, error) { + addr, err := sdk.AccAddressFromBech32(address) + if err != nil { + return nil, status.Error(codes.InvalidArgument, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress.Wrap(address), context).Error()) + } + + return addr, nil +} + // Balance queries the number of tokens of a given class owned by the owner. func (s queryServer) Balance(c context.Context, req *token.QueryBalanceRequest) (*token.QueryBalanceResponse, error) { if req == nil { @@ -42,11 +51,11 @@ func (s queryServer) Balance(c context.Context, req *token.QueryBalanceRequest) } if err := token.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } - addr, err := sdk.AccAddressFromBech32(req.Address) + addr, err := s.addressFromBech32GRPC(req.Address, "address") if err != nil { - return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid address: %s", req.Address) + return nil, err } ctx := sdk.UnwrapSDKContext(c) @@ -62,7 +71,7 @@ func (s queryServer) Supply(c context.Context, req *token.QuerySupplyRequest) (* } if err := token.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -83,7 +92,7 @@ func (s queryServer) Minted(c context.Context, req *token.QueryMintedRequest) (* } if err := token.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -104,7 +113,7 @@ func (s queryServer) Burnt(c context.Context, req *token.QueryBurntRequest) (*to } if err := token.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -125,7 +134,7 @@ func (s queryServer) Contract(c context.Context, req *token.QueryContractRequest } if err := token.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } ctx := sdk.UnwrapSDKContext(c) @@ -143,11 +152,11 @@ func (s queryServer) GranteeGrants(c context.Context, req *token.QueryGranteeGra } if err := token.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } - grantee, err := sdk.AccAddressFromBech32(req.Grantee) + grantee, err := s.addressFromBech32GRPC(req.Grantee, "grantee") if err != nil { - return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid grantee address: %s", req.Grantee) + return nil, err } ctx := sdk.UnwrapSDKContext(c) @@ -180,13 +189,13 @@ func (s queryServer) IsOperatorFor(c context.Context, req *token.QueryIsOperator } if err := token.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } - operator, err := sdk.AccAddressFromBech32(req.Operator) + operator, err := s.addressFromBech32GRPC(req.Operator, "operator") if err != nil { return nil, err } - holder, err := sdk.AccAddressFromBech32(req.Holder) + holder, err := s.addressFromBech32GRPC(req.Holder, "holder") if err != nil { return nil, err } @@ -209,11 +218,11 @@ func (s queryServer) HoldersByOperator(c context.Context, req *token.QueryHolder } if err := token.ValidateContractID(req.ContractId); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } - operator, err := sdk.AccAddressFromBech32(req.Operator) + operator, err := s.addressFromBech32GRPC(req.Operator, "operator") if err != nil { - return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid operator: %s", req.Operator) + return nil, err } ctx := sdk.UnwrapSDKContext(c) From 0750a72220fe390a7b1981062af73f1db90168f1 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 12 Apr 2023 17:41:32 +0900 Subject: [PATCH 08/10] Revert gRPC errors on page error --- x/collection/keeper/grpc_query.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/collection/keeper/grpc_query.go b/x/collection/keeper/grpc_query.go index fbf6ec80b4..4e5e836727 100644 --- a/x/collection/keeper/grpc_query.go +++ b/x/collection/keeper/grpc_query.go @@ -134,7 +134,7 @@ func (s queryServer) AllBalances(c context.Context, req *collection.QueryAllBala return nil }) if err != nil { - return nil, status.Error(codes.InvalidArgument, err.Error()) + return nil, err } return &collection.QueryAllBalancesResponse{Balances: balances, Pagination: pageRes}, nil @@ -596,7 +596,7 @@ func (s queryServer) Children(c context.Context, req *collection.QueryChildrenRe return nil }) if err != nil { - return nil, status.Error(codes.InvalidArgument, err.Error()) + return nil, err } return &collection.QueryChildrenResponse{Children: children, Pagination: pageRes}, nil @@ -634,7 +634,7 @@ func (s queryServer) GranteeGrants(c context.Context, req *collection.QueryGrant return nil }) if err != nil { - return nil, status.Error(codes.InvalidArgument, err.Error()) + return nil, err } return &collection.QueryGranteeGrantsResponse{Grants: grants, Pagination: pageRes}, nil @@ -699,7 +699,7 @@ func (s queryServer) HoldersByOperator(c context.Context, req *collection.QueryH return nil }) if err != nil { - return nil, status.Error(codes.InvalidArgument, err.Error()) + return nil, err } return &collection.QueryHoldersByOperatorResponse{Holders: holders, Pagination: pageRes}, nil From 46fa27b4e6c7f5f56a0da40d9b0748e178c960c3 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 12 Apr 2023 17:55:08 +0900 Subject: [PATCH 09/10] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d14dab9ede..52afcb610b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/collection) [\#959](https://github.com/line/lbm-sdk/pull/959) Revert #955 and add Query/HasParent into x/collection * (x/collection) [\#960](https://github.com/line/lbm-sdk/pull/960) Fix default next class ids of x/collection * (x/collection) [\#961](https://github.com/line/lbm-sdk/pull/961) Do not loop enum in x/collection +* (x/collection,token) [\#957](https://github.com/line/lbm-sdk/pull/957) Refactor queries of x/collection and x/token ### Removed From 80c52a64d06ccd39529b52a28b43de0bf6d60bd1 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 12 Apr 2023 18:11:18 +0900 Subject: [PATCH 10/10] Increase coverage --- x/collection/keeper/grpc_query.go | 12 ++++++++---- x/token/keeper/grpc_query_test.go | 4 ++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/x/collection/keeper/grpc_query.go b/x/collection/keeper/grpc_query.go index 4e5e836727..d0f24adbdf 100644 --- a/x/collection/keeper/grpc_query.go +++ b/x/collection/keeper/grpc_query.go @@ -500,6 +500,10 @@ func (s queryServer) Root(c context.Context, req *collection.QueryRootRequest) ( } func (s queryServer) HasParent(c context.Context, req *collection.QueryHasParentRequest) (*collection.QueryHasParentResponse, error) { + if req == nil { + return nil, status.Error(codes.InvalidArgument, "empty request") + } + ctx := sdk.UnwrapSDKContext(c) if err := s.validateGetParentVariants(ctx, req); err != nil { @@ -511,6 +515,10 @@ func (s queryServer) HasParent(c context.Context, req *collection.QueryHasParent } func (s queryServer) Parent(c context.Context, req *collection.QueryParentRequest) (*collection.QueryParentResponse, error) { + if req == nil { + return nil, status.Error(codes.InvalidArgument, "empty request") + } + ctx := sdk.UnwrapSDKContext(c) if err := s.validateGetParentVariants(ctx, req); err != nil { @@ -536,10 +544,6 @@ type parentRequest interface { } func (s queryServer) validateGetParentVariants(ctx sdk.Context, req parentRequest) error { - if req == nil { - return status.Error(codes.InvalidArgument, "empty request") - } - if err := collection.ValidateContractID(req.GetContractId()); err != nil { return status.Error(codes.InvalidArgument, err.Error()) } diff --git a/x/token/keeper/grpc_query_test.go b/x/token/keeper/grpc_query_test.go index a730b57a0e..003f960160 100644 --- a/x/token/keeper/grpc_query_test.go +++ b/x/token/keeper/grpc_query_test.go @@ -355,6 +355,10 @@ func (s *KeeperTestSuite) TestQueryHoldersByOperator() { "invalid operator": { contractID: s.contractID, }, + "class not found": { + contractID: "fee1dead", + operator: s.operator, + }, } for name, tc := range testCases {