LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

refactor(x/nft): remove class&nft id validation (#13836)

Browse Source
This commit is contained in:
Dreamer 2022-11-15 20:47:38 +08:00 committed by GitHub
parent bcff22a376
commit 5f4350d418
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 49 additions and 206 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
docs/architecture/adr-043-nft-module.md
View File

@ -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)

80
tests/e2e/nft/grpc.go
View File

@ -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 {

44
tests/e2e/nft/query.go
View File

@ -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,
},
{

6
x/nft/client/cli/query.go
View File

@ -166,12 +166,6 @@ $ %s query %s nfts <class-id> --owner=<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")
}

15
x/nft/errors.go
View File

@ -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")
)

8
x/nft/genesis.go
View File

@ -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

33
x/nft/keeper/grpc_query.go
View File

@ -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)

14
x/nft/keeper/grpc_query_test.go
View File

@ -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) {},
},
{

13
x/nft/msgs.go
View File

@ -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
}

35
x/nft/validation.go
View File

@ -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
}