From c5ccdfe1fa7368db2abdf224d89ab13670ca4d21 Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Fri, 11 Nov 2022 14:34:06 +0800 Subject: [PATCH 1/6] remove class&nft id validation --- x/nft/client/cli/query.go | 6 ------ x/nft/genesis.go | 9 +++++---- x/nft/keeper/grpc_query.go | 34 ++++++++++++++------------------ x/nft/keeper/grpc_query_test.go | 14 ++++++------- x/nft/msgs.go | 13 ++++++------ x/nft/validation.go | 35 --------------------------------- 6 files changed, 34 insertions(+), 77 deletions(-) delete mode 100644 x/nft/validation.go diff --git a/x/nft/client/cli/query.go b/x/nft/client/cli/query.go index 559a36f14e6e..52416d4a32a9 100644 --- a/x/nft/client/cli/query.go +++ b/x/nft/client/cli/query.go @@ -166,12 +166,6 @@ $ %s query %s nfts --owner= return err } - if len(classID) > 0 { - if err := nft.ValidateClassID(classID); err != nil { - return err - } - } - if len(owner) == 0 && len(classID) == 0 { return errors.ErrInvalidRequest.Wrap("must provide at least one of classID or owner") } diff --git a/x/nft/genesis.go b/x/nft/genesis.go index 43be6ffbddd1..fb433b06ba68 100644 --- a/x/nft/genesis.go +++ b/x/nft/genesis.go @@ -1,20 +1,21 @@ package nft import ( + "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" ) // ValidateGenesis check the given genesis state has no integrity issues func ValidateGenesis(data GenesisState) error { for _, class := range data.Classes { - if err := ValidateClassID(class.Id); err != nil { - return err + if len(class.Id) == 0 { + return errors.Wrapf(ErrInvalidID, "Empty class id (%s)", class.Id) } } for _, entry := range data.Entries { for _, nft := range entry.Nfts { - if err := ValidateNFTID(nft.Id); err != nil { - return err + if len(nft.Id) == 0 { + return errors.Wrapf(ErrInvalidID, "Empty nft id (%s)", nft.Id) } if _, err := sdk.AccAddressFromBech32(entry.Owner); err != nil { return err diff --git a/x/nft/keeper/grpc_query.go b/x/nft/keeper/grpc_query.go index e22fdf0b4a22..8c1ef685a8dd 100644 --- a/x/nft/keeper/grpc_query.go +++ b/x/nft/keeper/grpc_query.go @@ -3,6 +3,7 @@ package keeper import ( "context" + "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -18,8 +19,8 @@ func (k Keeper) Balance(goCtx context.Context, r *nft.QueryBalanceRequest) (*nft return nil, sdkerrors.ErrInvalidRequest.Wrap("empty request") } - if err := nft.ValidateClassID(r.ClassId); err != nil { - return nil, err + if len(r.ClassId) == 0 { + return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) } owner, err := sdk.AccAddressFromBech32(r.Owner) @@ -38,12 +39,12 @@ func (k Keeper) Owner(goCtx context.Context, r *nft.QueryOwnerRequest) (*nft.Que return nil, sdkerrors.ErrInvalidRequest.Wrap("empty request") } - if err := nft.ValidateClassID(r.ClassId); err != nil { - return nil, err + if len(r.ClassId) == 0 { + return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) } - if err := nft.ValidateNFTID(r.Id); err != nil { - return nil, err + if len(r.Id) == 0 { + return nil, errors.Wrapf(nft.ErrInvalidID, "Empty nft id (%s)", r.Id) } ctx := sdk.UnwrapSDKContext(goCtx) @@ -57,8 +58,8 @@ func (k Keeper) Supply(goCtx context.Context, r *nft.QuerySupplyRequest) (*nft.Q return nil, sdkerrors.ErrInvalidRequest.Wrap("empty request") } - if err := nft.ValidateClassID(r.ClassId); err != nil { - return nil, err + if len(r.ClassId) == 0 { + return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) } ctx := sdk.UnwrapSDKContext(goCtx) supply := k.GetTotalSupply(ctx, r.ClassId) @@ -73,11 +74,6 @@ func (k Keeper) NFTs(goCtx context.Context, r *nft.QueryNFTsRequest) (*nft.Query var err error var owner sdk.AccAddress - if len(r.ClassId) > 0 { - if err := nft.ValidateClassID(r.ClassId); err != nil { - return nil, err - } - } if len(r.Owner) > 0 { owner, err = sdk.AccAddressFromBech32(r.Owner) @@ -138,11 +134,11 @@ func (k Keeper) NFT(goCtx context.Context, r *nft.QueryNFTRequest) (*nft.QueryNF return nil, sdkerrors.ErrInvalidRequest.Wrap("empty request") } - if err := nft.ValidateClassID(r.ClassId); err != nil { - return nil, err + if len(r.ClassId) == 0 { + return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) } - if err := nft.ValidateNFTID(r.Id); err != nil { - return nil, err + if len(r.Id) == 0 { + return nil, errors.Wrapf(nft.ErrInvalidID, "Empty nft id (%s)", r.Id) } ctx := sdk.UnwrapSDKContext(goCtx) @@ -159,8 +155,8 @@ func (k Keeper) Class(goCtx context.Context, r *nft.QueryClassRequest) (*nft.Que return nil, sdkerrors.ErrInvalidRequest.Wrap("empty request") } - if err := nft.ValidateClassID(r.ClassId); err != nil { - return nil, err + if len(r.ClassId) == 0 { + return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) } ctx := sdk.UnwrapSDKContext(goCtx) diff --git a/x/nft/keeper/grpc_query_test.go b/x/nft/keeper/grpc_query_test.go index 8afcf1c26e30..892d45ed1174 100644 --- a/x/nft/keeper/grpc_query_test.go +++ b/x/nft/keeper/grpc_query_test.go @@ -29,7 +29,7 @@ func (s *TestSuite) TestBalance() { func(index int, require *require.Assertions) { req = &nft.QueryBalanceRequest{} }, - "invalid class id", + "Empty class id", 0, func(index int, require *require.Assertions, res *nft.QueryBalanceResponse, expBalance uint64) {}, }, @@ -95,7 +95,7 @@ func (s *TestSuite) TestOwner() { Id: testID, } }, - "invalid class id", + "Empty class id", func(index int, require *require.Assertions, res *nft.QueryOwnerResponse) {}, }, { @@ -105,7 +105,7 @@ func (s *TestSuite) TestOwner() { ClassId: testClassID, } }, - "invalid nft id", + "Empty nft id", func(index int, require *require.Assertions, res *nft.QueryOwnerResponse) {}, }, { @@ -180,7 +180,7 @@ func (s *TestSuite) TestSupply() { func(index int, require *require.Assertions) { req = &nft.QuerySupplyRequest{} }, - "invalid class id", + "Empty class id", 0, func(index int, require *require.Assertions, res *nft.QuerySupplyResponse, supply uint64) {}, }, @@ -393,7 +393,7 @@ func (s *TestSuite) TestNFT() { func(index int, require *require.Assertions) { req = &nft.QueryNFTRequest{} }, - "invalid class id", + "Empty class id", func(index int, require *require.Assertions, res *nft.QueryNFTResponse) {}, }, { @@ -403,7 +403,7 @@ func (s *TestSuite) TestNFT() { ClassId: testClassID, } }, - "invalid nft id", + "Empty nft id", func(index int, require *require.Assertions, res *nft.QueryNFTResponse) {}, }, { @@ -480,7 +480,7 @@ func (s *TestSuite) TestClass() { func(index int, require *require.Assertions) { req = &nft.QueryClassRequest{} }, - "invalid class id", + "Empty class id", func(index int, require *require.Assertions, res *nft.QueryClassResponse) {}, }, { diff --git a/x/nft/msgs.go b/x/nft/msgs.go index ec648e5f498c..10e5c23dafe7 100644 --- a/x/nft/msgs.go +++ b/x/nft/msgs.go @@ -1,6 +1,7 @@ package nft import ( + "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) @@ -14,22 +15,22 @@ var _ sdk.Msg = &MsgSend{} // ValidateBasic implements the Msg.ValidateBasic method. func (m MsgSend) ValidateBasic() error { - if err := ValidateClassID(m.ClassId); err != nil { - return sdkerrors.Wrapf(ErrInvalidID, "Invalid class id (%s)", m.ClassId) + if len(m.ClassId) == 0 { + return errors.Wrapf(ErrInvalidID, "Empty class id (%s)", m.ClassId) } - if err := ValidateNFTID(m.Id); err != nil { - return sdkerrors.Wrapf(ErrInvalidID, "Invalid nft id (%s)", m.Id) + if len(m.Id) == 0 { + return errors.Wrapf(ErrInvalidID, "Empty nft id (%s)", m.Id) } _, err := sdk.AccAddressFromBech32(m.Sender) if err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", m.Sender) + return errors.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", m.Sender) } _, err = sdk.AccAddressFromBech32(m.Receiver) if err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid receiver address (%s)", m.Receiver) + return errors.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid receiver address (%s)", m.Receiver) } return nil } diff --git a/x/nft/validation.go b/x/nft/validation.go deleted file mode 100644 index 65d684716608..000000000000 --- a/x/nft/validation.go +++ /dev/null @@ -1,35 +0,0 @@ -package nft - -import ( - "fmt" - "regexp" - - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" -) - -var ( - // reClassIDString can be 3 ~ 100 characters long and support letters, followed by either - // a letter, a number or a slash ('/') or a colon (':') or ('-'). - reClassIDString = `[a-zA-Z][a-zA-Z0-9/:-]{2,100}` - reClassID = regexp.MustCompile(fmt.Sprintf(`^%s$`, reClassIDString)) - - // reNFTIDString can be 3 ~ 100 characters long and support letters, followed by either - // a letter, a number or a slash ('/') or a colon (':') or ('-'). - reNFTID = reClassID -) - -// ValidateClassID returns whether the class id is valid -func ValidateClassID(id string) error { - if !reClassID.MatchString(id) { - return sdkerrors.Wrapf(ErrInvalidClassID, "invalid class id: %s", id) - } - return nil -} - -// ValidateNFTID returns whether the nft id is valid -func ValidateNFTID(id string) error { - if !reNFTID.MatchString(id) { - return sdkerrors.Wrapf(ErrInvalidID, "invalid nft id: %s", id) - } - return nil -} From 1a5b780df1e0f1024fe1c9f5438db2b543acf60a Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Fri, 11 Nov 2022 14:43:52 +0800 Subject: [PATCH 2/6] clean code --- x/nft/genesis.go | 4 ++-- x/nft/keeper/grpc_query.go | 14 +++++++------- x/nft/msgs.go | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/x/nft/genesis.go b/x/nft/genesis.go index fb433b06ba68..1301b1f3a76a 100644 --- a/x/nft/genesis.go +++ b/x/nft/genesis.go @@ -9,13 +9,13 @@ import ( func ValidateGenesis(data GenesisState) error { for _, class := range data.Classes { if len(class.Id) == 0 { - return errors.Wrapf(ErrInvalidID, "Empty class id (%s)", class.Id) + return errors.Wrap(ErrInvalidID, "Empty class id") } } for _, entry := range data.Entries { for _, nft := range entry.Nfts { if len(nft.Id) == 0 { - return errors.Wrapf(ErrInvalidID, "Empty nft id (%s)", nft.Id) + return errors.Wrap(ErrInvalidID, "Empty nft id") } if _, err := sdk.AccAddressFromBech32(entry.Owner); err != nil { return err diff --git a/x/nft/keeper/grpc_query.go b/x/nft/keeper/grpc_query.go index 8c1ef685a8dd..406936d93985 100644 --- a/x/nft/keeper/grpc_query.go +++ b/x/nft/keeper/grpc_query.go @@ -20,7 +20,7 @@ func (k Keeper) Balance(goCtx context.Context, r *nft.QueryBalanceRequest) (*nft } if len(r.ClassId) == 0 { - return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) + return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") } owner, err := sdk.AccAddressFromBech32(r.Owner) @@ -40,11 +40,11 @@ func (k Keeper) Owner(goCtx context.Context, r *nft.QueryOwnerRequest) (*nft.Que } if len(r.ClassId) == 0 { - return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) + return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") } if len(r.Id) == 0 { - return nil, errors.Wrapf(nft.ErrInvalidID, "Empty nft id (%s)", r.Id) + return nil, errors.Wrap(nft.ErrInvalidID, "Empty nft id") } ctx := sdk.UnwrapSDKContext(goCtx) @@ -59,7 +59,7 @@ func (k Keeper) Supply(goCtx context.Context, r *nft.QuerySupplyRequest) (*nft.Q } if len(r.ClassId) == 0 { - return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) + return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") } ctx := sdk.UnwrapSDKContext(goCtx) supply := k.GetTotalSupply(ctx, r.ClassId) @@ -135,10 +135,10 @@ func (k Keeper) NFT(goCtx context.Context, r *nft.QueryNFTRequest) (*nft.QueryNF } if len(r.ClassId) == 0 { - return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) + return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") } if len(r.Id) == 0 { - return nil, errors.Wrapf(nft.ErrInvalidID, "Empty nft id (%s)", r.Id) + return nil, errors.Wrap(nft.ErrInvalidID, "Empty nft id") } ctx := sdk.UnwrapSDKContext(goCtx) @@ -156,7 +156,7 @@ func (k Keeper) Class(goCtx context.Context, r *nft.QueryClassRequest) (*nft.Que } if len(r.ClassId) == 0 { - return nil, errors.Wrapf(nft.ErrInvalidID, "Empty class id (%s)", r.ClassId) + return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") } ctx := sdk.UnwrapSDKContext(goCtx) diff --git a/x/nft/msgs.go b/x/nft/msgs.go index 10e5c23dafe7..330e71641def 100644 --- a/x/nft/msgs.go +++ b/x/nft/msgs.go @@ -16,11 +16,11 @@ var _ sdk.Msg = &MsgSend{} // ValidateBasic implements the Msg.ValidateBasic method. func (m MsgSend) ValidateBasic() error { if len(m.ClassId) == 0 { - return errors.Wrapf(ErrInvalidID, "Empty class id (%s)", m.ClassId) + return errors.Wrap(ErrInvalidID, "Empty class id") } if len(m.Id) == 0 { - return errors.Wrapf(ErrInvalidID, "Empty nft id (%s)", m.Id) + return errors.Wrap(ErrInvalidID, "Empty nft id") } _, err := sdk.AccAddressFromBech32(m.Sender) From 42a360f5125b4ea91ab0d22a3ecd97bc67288ca5 Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Fri, 11 Nov 2022 15:53:02 +0800 Subject: [PATCH 3/6] fix e2e test&update adr-043 docs --- docs/architecture/adr-043-nft-module.md | 7 ++++--- tests/e2e/nft/grpc.go | 8 ++++---- tests/e2e/nft/query.go | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/architecture/adr-043-nft-module.md b/docs/architecture/adr-043-nft-module.md index d3fb11dde6e9..87b4dbb5ee20 100644 --- a/docs/architecture/adr-043-nft-module.md +++ b/docs/architecture/adr-043-nft-module.md @@ -5,6 +5,7 @@ * 2021-05-01: Initial Draft * 2021-07-02: Review updates * 2022-06-15: Add batch operation +* 2022-11-11: Remove strict validation of classID and tokenID ## Status @@ -75,7 +76,7 @@ message Class { } ``` -* `id` is an alphanumeric identifier of the NFT class; it is used as the primary index for storing the class; _required_ +* `id` is used as the primary index for storing the class; _required_ * `name` is a descriptive name of the NFT class; _optional_ * `symbol` is the symbol usually shown on exchanges for the NFT class; _optional_ * `description` is a detailed description of the NFT class; _optional_ @@ -97,8 +98,8 @@ message NFT { } ``` -* `class_id` is the identifier of the NFT class where the NFT belongs; _required_,`[a-zA-Z][a-zA-Z0-9/:-]{2,100}` -* `id` is an alphanumeric identifier of the NFT, unique within the scope of its class. It is specified by the creator of the NFT and may be expanded to use DID in the future. `class_id` combined with `id` uniquely identifies an NFT and is used as the primary index for storing the NFT; _required_,`[a-zA-Z][a-zA-Z0-9/:-]{2,100}` +* `class_id` is the identifier of the NFT class where the NFT belongs; _required_ +* `id` is an identifier of the NFT, unique within the scope of its class. It is specified by the creator of the NFT and may be expanded to use DID in the future. `class_id` combined with `id` uniquely identifies an NFT and is used as the primary index for storing the NFT; _required_ ```text {class_id}/{id} --> NFT (bytes) diff --git a/tests/e2e/nft/grpc.go b/tests/e2e/nft/grpc.go index 6b6d84943fc4..738f22386100 100644 --- a/tests/e2e/nft/grpc.go +++ b/tests/e2e/nft/grpc.go @@ -29,7 +29,7 @@ func (s *IntegrationTestSuite) TestQueryBalanceGRPC() { Owner: s.owner.String(), }, expectErr: true, - errMsg: "invalid class id", + errMsg: "Empty class id", expectValue: 0, }, { @@ -97,7 +97,7 @@ func (s *IntegrationTestSuite) TestQueryOwnerGRPC() { ID: ExpNFT.Id, }, expectErr: true, - errMsg: "invalid class id", + errMsg: "Empty class id", expectResult: "", }, { @@ -187,7 +187,7 @@ func (s *IntegrationTestSuite) TestQuerySupplyGRPC() { ClassID: "invalid_class_id", }, expectErr: true, - errMsg: "invalid class id", + errMsg: "Empty class id", expectResult: 0, }, { @@ -371,7 +371,7 @@ func (s *IntegrationTestSuite) TestQueryNFTGRPC() { ID: "invalid_nft_id", }, expectErr: true, - errorMsg: "invalid nft id", + errorMsg: "Empty nft id", }, { name: "nft id does not exist", diff --git a/tests/e2e/nft/query.go b/tests/e2e/nft/query.go index 56e0265fbfe9..b5c134af7a76 100644 --- a/tests/e2e/nft/query.go +++ b/tests/e2e/nft/query.go @@ -231,7 +231,7 @@ func (s *IntegrationTestSuite) TestQueryOwner() { ID: testID, }, expectErr: true, - errorMsg: "invalid class id", + errorMsg: "Empty class id", expectResult: "", }, { From 191d3ea275d4f43be34953887af5fa8ed2fbaead Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Mon, 14 Nov 2022 09:48:30 +0800 Subject: [PATCH 4/6] refactor error code --- tests/e2e/nft/cli_test.go | 3 --- tests/e2e/nft/grpc.go | 42 +++++++++++-------------------- tests/e2e/nft/query.go | 44 +++------------------------------ x/nft/errors.go | 15 ++++++----- x/nft/genesis.go | 5 ++-- x/nft/keeper/grpc_query.go | 15 ++++++----- x/nft/keeper/grpc_query_test.go | 14 +++++------ x/nft/msgs.go | 4 +-- 8 files changed, 43 insertions(+), 99 deletions(-) diff --git a/tests/e2e/nft/cli_test.go b/tests/e2e/nft/cli_test.go index 0a76010a640f..ba95eed45038 100644 --- a/tests/e2e/nft/cli_test.go +++ b/tests/e2e/nft/cli_test.go @@ -1,6 +1,3 @@ -//go:build e2e -// +build e2e - package nft import ( diff --git a/tests/e2e/nft/grpc.go b/tests/e2e/nft/grpc.go index 738f22386100..60afa5dfe5f2 100644 --- a/tests/e2e/nft/grpc.go +++ b/tests/e2e/nft/grpc.go @@ -19,19 +19,6 @@ func (s *IntegrationTestSuite) TestQueryBalanceGRPC() { errMsg string expectValue uint64 }{ - { - name: "fail not exist class id", - args: struct { - ClassID string - Owner string - }{ - ClassID: "invalid_class_id", - Owner: s.owner.String(), - }, - expectErr: true, - errMsg: "Empty class id", - expectValue: 0, - }, { name: "fail not exist owner", args: struct { @@ -88,16 +75,16 @@ func (s *IntegrationTestSuite) TestQueryOwnerGRPC() { expectResult string }{ { - name: "class id is invalid", + name: "class id is empty", args: struct { ClassID string ID string }{ - ClassID: "invalid_class_id", + ClassID: "", ID: ExpNFT.Id, }, expectErr: true, - errMsg: "Empty class id", + errMsg: nft.ErrEmptyClassID.Error(), expectResult: "", }, { @@ -113,15 +100,16 @@ func (s *IntegrationTestSuite) TestQueryOwnerGRPC() { expectResult: "", }, { - name: "nft id is invalid", + name: "nft id is empty", args: struct { ClassID string ID string }{ ClassID: ExpNFT.ClassId, - ID: "invalid_nft_id", + ID: "", }, expectErr: true, + errMsg: nft.ErrEmptyNFTID.Error(), expectResult: "", }, { @@ -180,14 +168,14 @@ func (s *IntegrationTestSuite) TestQuerySupplyGRPC() { expectResult uint64 }{ { - name: "class id is invalid", + name: "class id is empty", args: struct { ClassID string }{ - ClassID: "invalid_class_id", + ClassID: "", }, expectErr: true, - errMsg: "Empty class id", + errMsg: nft.ErrEmptyClassID.Error(), expectResult: 0, }, { @@ -338,16 +326,16 @@ func (s *IntegrationTestSuite) TestQueryNFTGRPC() { errorMsg string }{ { - name: "class id is invalid", + name: "class id is empty", args: struct { ClassID string ID string }{ - ClassID: "invalid_class_id", + ClassID: "", ID: ExpNFT.Id, }, expectErr: true, - errorMsg: "invalid class id", + errorMsg: nft.ErrEmptyClassID.Error(), }, { name: "class id does not exist", @@ -362,16 +350,16 @@ func (s *IntegrationTestSuite) TestQueryNFTGRPC() { errorMsg: "not found nft", }, { - name: "nft id is invalid", + name: "nft id is empty", args: struct { ClassID string ID string }{ ClassID: ExpNFT.ClassId, - ID: "invalid_nft_id", + ID: "", }, expectErr: true, - errorMsg: "Empty nft id", + errorMsg: nft.ErrEmptyNFTID.Error(), }, { name: "nft id does not exist", diff --git a/tests/e2e/nft/query.go b/tests/e2e/nft/query.go index b5c134af7a76..6b041632cce8 100644 --- a/tests/e2e/nft/query.go +++ b/tests/e2e/nft/query.go @@ -221,19 +221,6 @@ func (s *IntegrationTestSuite) TestQueryOwner() { errorMsg string expectResult string }{ - { - name: "class id is invalid", - args: struct { - ClassID string - ID string - }{ - ClassID: "invalid_class_id", - ID: testID, - }, - expectErr: true, - errorMsg: "Empty class id", - expectResult: "", - }, { name: "class id does not exist", args: struct { @@ -246,18 +233,6 @@ func (s *IntegrationTestSuite) TestQueryOwner() { expectErr: false, expectResult: "", }, - { - name: "nft id is invalid", - args: struct { - ClassID string - ID string - }{ - ClassID: testClassID, - ID: "invalid_nft_id", - }, - expectErr: true, - expectResult: "", - }, { name: "nft id does not exist", args: struct { @@ -311,19 +286,6 @@ func (s *IntegrationTestSuite) TestQueryBalance() { errorMsg string expectResult uint64 }{ - { - name: "class id is invalid", - args: struct { - ClassID string - Owner string - }{ - ClassID: "invalid_class_id", - Owner: val.Address.String(), - }, - expectErr: true, - errorMsg: "invalid class id", - expectResult: 0, - }, { name: "class id does not exist", args: struct { @@ -389,14 +351,14 @@ func (s *IntegrationTestSuite) TestQuerySupply() { expectResult uint64 }{ { - name: "class id is invalid", + name: "class id is empty", args: struct { ClassID string }{ - ClassID: "invalid_class_id", + ClassID: "", }, expectErr: true, - errorMsg: "invalid class id", + errorMsg: nft.ErrEmptyClassID.Error(), expectResult: 0, }, { diff --git a/x/nft/errors.go b/x/nft/errors.go index c7d6836596f7..ccc2ae8b2c92 100644 --- a/x/nft/errors.go +++ b/x/nft/errors.go @@ -1,16 +1,15 @@ package nft import ( - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "cosmossdk.io/errors" ) // x/nft module sentinel errors var ( - ErrInvalidNFT = sdkerrors.Register(ModuleName, 2, "invalid nft") - ErrClassExists = sdkerrors.Register(ModuleName, 3, "nft class already exist") - ErrClassNotExists = sdkerrors.Register(ModuleName, 4, "nft class does not exist") - ErrNFTExists = sdkerrors.Register(ModuleName, 5, "nft already exist") - ErrNFTNotExists = sdkerrors.Register(ModuleName, 6, "nft does not exist") - ErrInvalidID = sdkerrors.Register(ModuleName, 7, "invalid id") - ErrInvalidClassID = sdkerrors.Register(ModuleName, 8, "invalid class id") + ErrClassExists = errors.Register(ModuleName, 2, "nft class already exist") + ErrClassNotExists = errors.Register(ModuleName, 3, "nft class does not exist") + ErrNFTExists = errors.Register(ModuleName, 4, "nft already exist") + ErrNFTNotExists = errors.Register(ModuleName, 5, "nft does not exist") + ErrEmptyClassID = errors.Register(ModuleName, 6, "empty class id") + ErrEmptyNFTID = errors.Register(ModuleName, 7, "empty nft id") ) diff --git a/x/nft/genesis.go b/x/nft/genesis.go index 1301b1f3a76a..75124ea354e5 100644 --- a/x/nft/genesis.go +++ b/x/nft/genesis.go @@ -1,7 +1,6 @@ package nft import ( - "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -9,13 +8,13 @@ import ( func ValidateGenesis(data GenesisState) error { for _, class := range data.Classes { if len(class.Id) == 0 { - return errors.Wrap(ErrInvalidID, "Empty class id") + return ErrEmptyClassID } } for _, entry := range data.Entries { for _, nft := range entry.Nfts { if len(nft.Id) == 0 { - return errors.Wrap(ErrInvalidID, "Empty nft id") + return ErrEmptyNFTID } if _, err := sdk.AccAddressFromBech32(entry.Owner); err != nil { return err diff --git a/x/nft/keeper/grpc_query.go b/x/nft/keeper/grpc_query.go index 406936d93985..e409a78c0923 100644 --- a/x/nft/keeper/grpc_query.go +++ b/x/nft/keeper/grpc_query.go @@ -3,7 +3,6 @@ package keeper import ( "context" - "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -20,7 +19,7 @@ func (k Keeper) Balance(goCtx context.Context, r *nft.QueryBalanceRequest) (*nft } if len(r.ClassId) == 0 { - return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") + return nil, nft.ErrEmptyClassID } owner, err := sdk.AccAddressFromBech32(r.Owner) @@ -40,11 +39,11 @@ func (k Keeper) Owner(goCtx context.Context, r *nft.QueryOwnerRequest) (*nft.Que } if len(r.ClassId) == 0 { - return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") + return nil, nft.ErrEmptyClassID } if len(r.Id) == 0 { - return nil, errors.Wrap(nft.ErrInvalidID, "Empty nft id") + return nil, nft.ErrEmptyNFTID } ctx := sdk.UnwrapSDKContext(goCtx) @@ -59,7 +58,7 @@ func (k Keeper) Supply(goCtx context.Context, r *nft.QuerySupplyRequest) (*nft.Q } if len(r.ClassId) == 0 { - return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") + return nil, nft.ErrEmptyClassID } ctx := sdk.UnwrapSDKContext(goCtx) supply := k.GetTotalSupply(ctx, r.ClassId) @@ -135,10 +134,10 @@ func (k Keeper) NFT(goCtx context.Context, r *nft.QueryNFTRequest) (*nft.QueryNF } if len(r.ClassId) == 0 { - return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") + return nil, nft.ErrEmptyClassID } if len(r.Id) == 0 { - return nil, errors.Wrap(nft.ErrInvalidID, "Empty nft id") + return nil, nft.ErrEmptyNFTID } ctx := sdk.UnwrapSDKContext(goCtx) @@ -156,7 +155,7 @@ func (k Keeper) Class(goCtx context.Context, r *nft.QueryClassRequest) (*nft.Que } if len(r.ClassId) == 0 { - return nil, errors.Wrap(nft.ErrInvalidID, "Empty class id") + return nil, nft.ErrEmptyClassID } ctx := sdk.UnwrapSDKContext(goCtx) diff --git a/x/nft/keeper/grpc_query_test.go b/x/nft/keeper/grpc_query_test.go index 892d45ed1174..ae8e68d7b74b 100644 --- a/x/nft/keeper/grpc_query_test.go +++ b/x/nft/keeper/grpc_query_test.go @@ -29,7 +29,7 @@ func (s *TestSuite) TestBalance() { func(index int, require *require.Assertions) { req = &nft.QueryBalanceRequest{} }, - "Empty class id", + nft.ErrEmptyClassID.Error(), 0, func(index int, require *require.Assertions, res *nft.QueryBalanceResponse, expBalance uint64) {}, }, @@ -95,7 +95,7 @@ func (s *TestSuite) TestOwner() { Id: testID, } }, - "Empty class id", + nft.ErrEmptyClassID.Error(), func(index int, require *require.Assertions, res *nft.QueryOwnerResponse) {}, }, { @@ -105,7 +105,7 @@ func (s *TestSuite) TestOwner() { ClassId: testClassID, } }, - "Empty nft id", + nft.ErrEmptyNFTID.Error(), func(index int, require *require.Assertions, res *nft.QueryOwnerResponse) {}, }, { @@ -180,7 +180,7 @@ func (s *TestSuite) TestSupply() { func(index int, require *require.Assertions) { req = &nft.QuerySupplyRequest{} }, - "Empty class id", + nft.ErrEmptyClassID.Error(), 0, func(index int, require *require.Assertions, res *nft.QuerySupplyResponse, supply uint64) {}, }, @@ -393,7 +393,7 @@ func (s *TestSuite) TestNFT() { func(index int, require *require.Assertions) { req = &nft.QueryNFTRequest{} }, - "Empty class id", + nft.ErrEmptyClassID.Error(), func(index int, require *require.Assertions, res *nft.QueryNFTResponse) {}, }, { @@ -403,7 +403,7 @@ func (s *TestSuite) TestNFT() { ClassId: testClassID, } }, - "Empty nft id", + nft.ErrEmptyNFTID.Error(), func(index int, require *require.Assertions, res *nft.QueryNFTResponse) {}, }, { @@ -480,7 +480,7 @@ func (s *TestSuite) TestClass() { func(index int, require *require.Assertions) { req = &nft.QueryClassRequest{} }, - "Empty class id", + nft.ErrEmptyClassID.Error(), func(index int, require *require.Assertions, res *nft.QueryClassResponse) {}, }, { diff --git a/x/nft/msgs.go b/x/nft/msgs.go index 330e71641def..57926287bb1f 100644 --- a/x/nft/msgs.go +++ b/x/nft/msgs.go @@ -16,11 +16,11 @@ var _ sdk.Msg = &MsgSend{} // ValidateBasic implements the Msg.ValidateBasic method. func (m MsgSend) ValidateBasic() error { if len(m.ClassId) == 0 { - return errors.Wrap(ErrInvalidID, "Empty class id") + return ErrEmptyClassID } if len(m.Id) == 0 { - return errors.Wrap(ErrInvalidID, "Empty nft id") + return ErrEmptyNFTID } _, err := sdk.AccAddressFromBech32(m.Sender) From e0142650369a372241c4490c5e6cce52a72e3505 Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Mon, 14 Nov 2022 11:41:52 +0800 Subject: [PATCH 5/6] remove invalid test cases --- tests/e2e/nft/grpc.go | 62 ------------------------------------------- 1 file changed, 62 deletions(-) diff --git a/tests/e2e/nft/grpc.go b/tests/e2e/nft/grpc.go index 60afa5dfe5f2..3175069e71d1 100644 --- a/tests/e2e/nft/grpc.go +++ b/tests/e2e/nft/grpc.go @@ -74,19 +74,6 @@ func (s *IntegrationTestSuite) TestQueryOwnerGRPC() { errMsg string expectResult string }{ - { - name: "class id is empty", - args: struct { - ClassID string - ID string - }{ - ClassID: "", - ID: ExpNFT.Id, - }, - expectErr: true, - errMsg: nft.ErrEmptyClassID.Error(), - expectResult: "", - }, { name: "class id does not exist", args: struct { @@ -99,19 +86,6 @@ func (s *IntegrationTestSuite) TestQueryOwnerGRPC() { expectErr: false, expectResult: "", }, - { - name: "nft id is empty", - args: struct { - ClassID string - ID string - }{ - ClassID: ExpNFT.ClassId, - ID: "", - }, - expectErr: true, - errMsg: nft.ErrEmptyNFTID.Error(), - expectResult: "", - }, { name: "nft id does not exist", args: struct { @@ -325,42 +299,6 @@ func (s *IntegrationTestSuite) TestQueryNFTGRPC() { expectErr bool errorMsg string }{ - { - name: "class id is empty", - args: struct { - ClassID string - ID string - }{ - ClassID: "", - ID: ExpNFT.Id, - }, - expectErr: true, - errorMsg: nft.ErrEmptyClassID.Error(), - }, - { - name: "class id does not exist", - args: struct { - ClassID string - ID string - }{ - ClassID: "class", - ID: ExpNFT.Id, - }, - expectErr: true, - errorMsg: "not found nft", - }, - { - name: "nft id is empty", - args: struct { - ClassID string - ID string - }{ - ClassID: ExpNFT.ClassId, - ID: "", - }, - expectErr: true, - errorMsg: nft.ErrEmptyNFTID.Error(), - }, { name: "nft id does not exist", args: struct { From 7108e161ff2cd03e680f2f13a150d535a420b671 Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Tue, 15 Nov 2022 09:47:18 +0800 Subject: [PATCH 6/6] update error code --- tests/e2e/nft/cli_test.go | 3 +++ x/nft/errors.go | 12 ++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/e2e/nft/cli_test.go b/tests/e2e/nft/cli_test.go index ba95eed45038..0a76010a640f 100644 --- a/tests/e2e/nft/cli_test.go +++ b/tests/e2e/nft/cli_test.go @@ -1,3 +1,6 @@ +//go:build e2e +// +build e2e + package nft import ( diff --git a/x/nft/errors.go b/x/nft/errors.go index ccc2ae8b2c92..771146f39e69 100644 --- a/x/nft/errors.go +++ b/x/nft/errors.go @@ -6,10 +6,10 @@ import ( // x/nft module sentinel errors var ( - ErrClassExists = errors.Register(ModuleName, 2, "nft class already exist") - ErrClassNotExists = errors.Register(ModuleName, 3, "nft class does not exist") - ErrNFTExists = errors.Register(ModuleName, 4, "nft already exist") - ErrNFTNotExists = errors.Register(ModuleName, 5, "nft does not exist") - ErrEmptyClassID = errors.Register(ModuleName, 6, "empty class id") - ErrEmptyNFTID = errors.Register(ModuleName, 7, "empty nft id") + ErrClassExists = errors.Register(ModuleName, 3, "nft class already exist") + ErrClassNotExists = errors.Register(ModuleName, 4, "nft class does not exist") + ErrNFTExists = errors.Register(ModuleName, 5, "nft already exist") + ErrNFTNotExists = errors.Register(ModuleName, 6, "nft does not exist") + ErrEmptyClassID = errors.Register(ModuleName, 7, "empty class id") + ErrEmptyNFTID = errors.Register(ModuleName, 8, "empty nft id") )