From 5f4350d41859e5cbefd7514abe1b6461dc637c24 Mon Sep 17 00:00:00 2001 From: Dreamer <745124335@qq.com> Date: Tue, 15 Nov 2022 20:47:38 +0800 Subject: [PATCH] refactor(x/nft): remove class&nft id validation (#13836) --- docs/architecture/adr-043-nft-module.md | 7 ++- tests/e2e/nft/grpc.go | 80 +------------------------ tests/e2e/nft/query.go | 44 +------------- x/nft/client/cli/query.go | 6 -- x/nft/errors.go | 15 +++-- x/nft/genesis.go | 8 +-- x/nft/keeper/grpc_query.go | 33 +++++----- x/nft/keeper/grpc_query_test.go | 14 ++--- x/nft/msgs.go | 13 ++-- x/nft/validation.go | 35 ----------- 10 files changed, 49 insertions(+), 206 deletions(-) delete mode 100644 x/nft/validation.go diff --git a/docs/architecture/adr-043-nft-module.md b/docs/architecture/adr-043-nft-module.md index d3fb11dde6..87b4dbb5ee 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 6b6d84943f..3175069e71 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: "invalid class id", - expectValue: 0, - }, { name: "fail not exist owner", args: struct { @@ -87,19 +74,6 @@ func (s *IntegrationTestSuite) TestQueryOwnerGRPC() { errMsg string expectResult string }{ - { - name: "class id is invalid", - args: struct { - ClassID string - ID string - }{ - ClassID: "invalid_class_id", - ID: ExpNFT.Id, - }, - expectErr: true, - errMsg: "invalid class id", - expectResult: "", - }, { name: "class id does not exist", args: struct { @@ -112,18 +86,6 @@ func (s *IntegrationTestSuite) TestQueryOwnerGRPC() { expectErr: false, expectResult: "", }, - { - name: "nft id is invalid", - args: struct { - ClassID string - ID string - }{ - ClassID: ExpNFT.ClassId, - ID: "invalid_nft_id", - }, - expectErr: true, - expectResult: "", - }, { name: "nft id does not exist", args: struct { @@ -180,14 +142,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: "invalid class id", + errMsg: nft.ErrEmptyClassID.Error(), expectResult: 0, }, { @@ -337,42 +299,6 @@ func (s *IntegrationTestSuite) TestQueryNFTGRPC() { expectErr bool errorMsg string }{ - { - name: "class id is invalid", - args: struct { - ClassID string - ID string - }{ - ClassID: "invalid_class_id", - ID: ExpNFT.Id, - }, - expectErr: true, - errorMsg: "invalid class id", - }, - { - 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 invalid", - args: struct { - ClassID string - ID string - }{ - ClassID: ExpNFT.ClassId, - ID: "invalid_nft_id", - }, - expectErr: true, - errorMsg: "invalid nft id", - }, { name: "nft id does not exist", args: struct { diff --git a/tests/e2e/nft/query.go b/tests/e2e/nft/query.go index 56e0265fbf..6b041632cc 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: "invalid 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/client/cli/query.go b/x/nft/client/cli/query.go index 559a36f14e..52416d4a32 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/errors.go b/x/nft/errors.go index c7d6836596..771146f39e 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, 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") ) diff --git a/x/nft/genesis.go b/x/nft/genesis.go index 43be6ffbdd..75124ea354 100644 --- a/x/nft/genesis.go +++ b/x/nft/genesis.go @@ -7,14 +7,14 @@ import ( // 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 ErrEmptyClassID } } 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 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 e22fdf0b4a..e409a78c09 100644 --- a/x/nft/keeper/grpc_query.go +++ b/x/nft/keeper/grpc_query.go @@ -18,8 +18,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, nft.ErrEmptyClassID } owner, err := sdk.AccAddressFromBech32(r.Owner) @@ -38,12 +38,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, nft.ErrEmptyClassID } - if err := nft.ValidateNFTID(r.Id); err != nil { - return nil, err + if len(r.Id) == 0 { + return nil, nft.ErrEmptyNFTID } ctx := sdk.UnwrapSDKContext(goCtx) @@ -57,8 +57,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, nft.ErrEmptyClassID } ctx := sdk.UnwrapSDKContext(goCtx) supply := k.GetTotalSupply(ctx, r.ClassId) @@ -73,11 +73,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 +133,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, nft.ErrEmptyClassID } - if err := nft.ValidateNFTID(r.Id); err != nil { - return nil, err + if len(r.Id) == 0 { + return nil, nft.ErrEmptyNFTID } ctx := sdk.UnwrapSDKContext(goCtx) @@ -159,8 +154,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, 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 8afcf1c26e..ae8e68d7b7 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", + 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, } }, - "invalid class id", + nft.ErrEmptyClassID.Error(), func(index int, require *require.Assertions, res *nft.QueryOwnerResponse) {}, }, { @@ -105,7 +105,7 @@ func (s *TestSuite) TestOwner() { ClassId: testClassID, } }, - "invalid 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{} }, - "invalid 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{} }, - "invalid class id", + nft.ErrEmptyClassID.Error(), func(index int, require *require.Assertions, res *nft.QueryNFTResponse) {}, }, { @@ -403,7 +403,7 @@ func (s *TestSuite) TestNFT() { ClassId: testClassID, } }, - "invalid 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{} }, - "invalid 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 ec648e5f49..57926287bb 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 ErrEmptyClassID } - if err := ValidateNFTID(m.Id); err != nil { - return sdkerrors.Wrapf(ErrInvalidID, "Invalid nft id (%s)", m.Id) + if len(m.Id) == 0 { + return ErrEmptyNFTID } _, 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 65d6847166..0000000000 --- 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 -}