From d732e379f8f4ea96be9190639bcd1e7230c55650 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 10 Apr 2023 15:03:53 +0200 Subject: [PATCH] refactor(nft): move `ValidateBasic` logic to `msgServer` (#15759) --- x/nft/client/cli/tx.go | 4 ++++ x/nft/client/cli/tx_test.go | 10 +++++----- x/nft/go.mod | 2 +- x/nft/go.sum | 4 ++-- x/nft/keeper/msg_server.go | 24 ++++++++++++++++-------- x/nft/keeper/msg_server_test.go | 22 ++++++++++++++++++++++ x/nft/msgs.go | 25 ------------------------- 7 files changed, 50 insertions(+), 41 deletions(-) diff --git a/x/nft/client/cli/tx.go b/x/nft/client/cli/tx.go index 2a314fe979..e6cdd2b771 100644 --- a/x/nft/client/cli/tx.go +++ b/x/nft/client/cli/tx.go @@ -46,6 +46,10 @@ func NewCmdSend() *cobra.Command { return err } + if args[0] == "" || args[1] == "" || args[2] == "" { + return fmt.Errorf("class-id, nft-id and receiver cannot be empty") + } + msg := nft.MsgSend{ ClassId: args[0], Id: args[1], diff --git a/x/nft/client/cli/tx_test.go b/x/nft/client/cli/tx_test.go index ebf87e9456..a4834ffaea 100644 --- a/x/nft/client/cli/tx_test.go +++ b/x/nft/client/cli/tx_test.go @@ -149,7 +149,7 @@ func (s *CLITestSuite) TestCLITxSend() { }, 0, true, - "empty class id", + "class-id, nft-id and receiver cannot be empty", }, { "nft id is empty", @@ -160,18 +160,18 @@ func (s *CLITestSuite) TestCLITxSend() { }, 0, true, - "empty nft id", + "class-id, nft-id and receiver cannot be empty", }, { - "invalid receiver address", + "empty receiver address", []string{ testClassID, testID, - "invalid receiver", + "", }, 0, true, - "Invalid receiver address", + "class-id, nft-id and receiver cannot be empty", }, { "valid transaction", diff --git a/x/nft/go.mod b/x/nft/go.mod index 081abf50b9..135862cb3a 100644 --- a/x/nft/go.mod +++ b/x/nft/go.mod @@ -11,7 +11,7 @@ require ( cosmossdk.io/store v0.1.0-alpha.1.0.20230328185921-37ba88872dbc github.com/cometbft/cometbft v0.37.0 github.com/cosmos/cosmos-proto v1.0.0-beta.3 - github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330094838-d21f58c638d5 + github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230407193120-0a70647deaec github.com/cosmos/gogoproto v1.4.7 github.com/golang/mock v1.6.0 github.com/golang/protobuf v1.5.3 diff --git a/x/nft/go.sum b/x/nft/go.sum index 8a7d50ce4c..f19e42b247 100644 --- a/x/nft/go.sum +++ b/x/nft/go.sum @@ -181,8 +181,8 @@ github.com/cosmos/cosmos-db v1.0.0-rc.1 h1:SjnT8B6WKMW9WEIX32qMhnEEKcI7ZP0+G1Sa9 github.com/cosmos/cosmos-db v1.0.0-rc.1/go.mod h1:Dnmk3flSf5lkwCqvvjNpoxjpXzhxnCAFzKHlbaForso= github.com/cosmos/cosmos-proto v1.0.0-beta.3 h1:VitvZ1lPORTVxkmF2fAp3IiA61xVwArQYKXTdEcpW6o= github.com/cosmos/cosmos-proto v1.0.0-beta.3/go.mod h1:t8IASdLaAq+bbHbjq4p960BvcTqtwuAxid3b/2rOD6I= -github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330094838-d21f58c638d5 h1:zO3mov9MaHWNnYZyQ8Wz/CZhrjfizMKvvLH41Ro/FYk= -github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330094838-d21f58c638d5/go.mod h1:aKJRE3RjbwJUFGKG+kTDLhrST9vzFr8FlsTlv4eD+80= +github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230407193120-0a70647deaec h1:BE559vEyhAjljq+iyCGJsnVnpKl7QgYrJuzP4Ax1QDc= +github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230407193120-0a70647deaec/go.mod h1:lD11e/GdgJ5z2KCSN0DkXr0LFLXUrYUGIoF9cVvPU28= github.com/cosmos/go-bip39 v0.0.0-20180819234021-555e2067c45d/go.mod h1:tSxLoYXyBmiFeKpvmq4dzayMdCjCnu8uqmCysIGBT2Y= github.com/cosmos/go-bip39 v1.0.0 h1:pcomnQdrdH22njcAatO0yWojsUnCO3y2tNoV1cb6hHY= github.com/cosmos/go-bip39 v1.0.0/go.mod h1:RNJv0H/pOIVgxw6KS7QeX2a0Uo0aKUlfhZ4xuwvCdJw= diff --git a/x/nft/keeper/msg_server.go b/x/nft/keeper/msg_server.go index 93fdef5287..2f48298714 100644 --- a/x/nft/keeper/msg_server.go +++ b/x/nft/keeper/msg_server.go @@ -15,20 +15,28 @@ var _ nft.MsgServer = Keeper{} // Send implements Send method of the types.MsgServer. func (k Keeper) Send(goCtx context.Context, msg *nft.MsgSend) (*nft.MsgSendResponse, error) { - ctx := sdk.UnwrapSDKContext(goCtx) - sender, err := k.ac.StringToBytes(msg.Sender) - if err != nil { - return nil, err + if len(msg.ClassId) == 0 { + return nil, nft.ErrEmptyClassID } - owner := k.GetOwner(ctx, msg.ClassId, msg.Id) - if !bytes.Equal(owner, sender) { - return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not the owner of nft %s", msg.Sender, msg.Id) + if len(msg.Id) == 0 { + return nil, nft.ErrEmptyNFTID + } + + sender, err := k.ac.StringToBytes(msg.Sender) + if err != nil { + return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", msg.Sender) } receiver, err := k.ac.StringToBytes(msg.Receiver) if err != nil { - return nil, err + return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid receiver address (%s)", msg.Receiver) + } + + ctx := sdk.UnwrapSDKContext(goCtx) + owner := k.GetOwner(ctx, msg.ClassId, msg.Id) + if !bytes.Equal(owner, sender) { + return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not the owner of nft %s", msg.Sender, msg.Id) } if err := k.Transfer(ctx, msg.ClassId, msg.Id, receiver); err != nil { diff --git a/x/nft/keeper/msg_server_test.go b/x/nft/keeper/msg_server_test.go index 14f90f7c06..fc044a5f99 100644 --- a/x/nft/keeper/msg_server_test.go +++ b/x/nft/keeper/msg_server_test.go @@ -50,6 +50,28 @@ func (s *TestSuite) TestSend() { expErr bool errMsg string }{ + { + name: "empty nft id", + req: &nft.MsgSend{ + ClassId: testClassID, + Id: "", + Sender: s.addrs[0].String(), + Receiver: s.addrs[1].String(), + }, + expErr: true, + errMsg: "empty nft id", + }, + { + name: "empty class id", + req: &nft.MsgSend{ + ClassId: "", + Id: testID, + Sender: s.addrs[0].String(), + Receiver: s.addrs[1].String(), + }, + expErr: true, + errMsg: "empty class id", + }, { name: "invalid class id", req: &nft.MsgSend{ diff --git a/x/nft/msgs.go b/x/nft/msgs.go index c15b99cd8e..081737eb97 100644 --- a/x/nft/msgs.go +++ b/x/nft/msgs.go @@ -1,10 +1,7 @@ package nft import ( - errorsmod "cosmossdk.io/errors" - sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) const ( @@ -14,28 +11,6 @@ const ( var _ sdk.Msg = &MsgSend{} -// ValidateBasic implements the Msg.ValidateBasic method. -func (m MsgSend) ValidateBasic() error { - if len(m.ClassId) == 0 { - return ErrEmptyClassID - } - - if len(m.Id) == 0 { - return ErrEmptyNFTID - } - - _, err := sdk.AccAddressFromBech32(m.Sender) - if err != nil { - return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", m.Sender) - } - - _, err = sdk.AccAddressFromBech32(m.Receiver) - if err != nil { - return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid receiver address (%s)", m.Receiver) - } - return nil -} - // GetSigners returns the expected signers for MsgSend. func (m MsgSend) GetSigners() []sdk.AccAddress { signer, _ := sdk.AccAddressFromBech32(m.Sender)