From b4d1109efc2ed348b5a95c1974d8528ff47fe982 Mon Sep 17 00:00:00 2001 From: Marko Date: Mon, 3 Apr 2023 12:28:17 +0200 Subject: [PATCH] refactor(nft): remove global bech32 (#15654) --- tests/e2e/nft/test_helper.go | 3 ++- x/feegrant/expected_keepers.go | 8 +++---- x/nft/client/cli/query.go | 10 ++++---- x/nft/client/cli/query_test.go | 3 ++- x/nft/expected_keepers.go | 3 +++ x/nft/genesis.go | 6 ++--- x/nft/keeper/genesis.go | 5 +++- x/nft/keeper/grpc_query.go | 4 ++-- x/nft/keeper/grpc_query_test.go | 1 + x/nft/keeper/keeper.go | 3 +++ x/nft/keeper/keeper_test.go | 14 +++++++---- x/nft/keeper/msg_server.go | 9 +++---- x/nft/module/module.go | 12 ++++++---- x/nft/testutil/expected_keepers_mocks.go | 30 ++++++++++++++++++++++++ x/staking/types/expected_keepers.go | 8 +++---- 15 files changed, 83 insertions(+), 36 deletions(-) diff --git a/tests/e2e/nft/test_helper.go b/tests/e2e/nft/test_helper.go index 54756d5d10..ce34da3d67 100644 --- a/tests/e2e/nft/test_helper.go +++ b/tests/e2e/nft/test_helper.go @@ -5,6 +5,7 @@ import ( "cosmossdk.io/x/nft/client/cli" "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/cosmos/cosmos-sdk/codec/address" "github.com/cosmos/cosmos-sdk/testutil" clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" "github.com/cosmos/cosmos-sdk/testutil/network" @@ -40,7 +41,7 @@ func ExecQueryNFT(val *network.Validator, classID, nftID string) (testutil.Buffe } func ExecQueryNFTs(val *network.Validator, classID, owner string) (testutil.BufferWriter, error) { - cmd := cli.GetCmdQueryNFTs() + cmd := cli.GetCmdQueryNFTs(address.NewBech32Codec("cosmos")) var args []string args = append(args, fmt.Sprintf("--%s=%s", cli.FlagClassID, classID)) args = append(args, fmt.Sprintf("--%s=%s", cli.FlagOwner, owner)) diff --git a/x/feegrant/expected_keepers.go b/x/feegrant/expected_keepers.go index ae30f5830e..03e5bf0a8e 100644 --- a/x/feegrant/expected_keepers.go +++ b/x/feegrant/expected_keepers.go @@ -3,22 +3,20 @@ package feegrant import ( context "context" + "cosmossdk.io/core/address" sdk "github.com/cosmos/cosmos-sdk/types" ) // AccountKeeper defines the expected auth Account Keeper (noalias) type AccountKeeper interface { + address.Codec + GetModuleAddress(moduleName string) sdk.AccAddress GetModuleAccount(ctx context.Context, moduleName string) sdk.ModuleAccountI NewAccountWithAddress(ctx context.Context, addr sdk.AccAddress) sdk.AccountI GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI SetAccount(ctx context.Context, acc sdk.AccountI) - - // StringToBytes decodes text to bytes - StringToBytes(text string) ([]byte, error) - // BytesToString encodes bytes to text - BytesToString(bz []byte) (string, error) } // BankKeeper defines the expected supply Keeper (noalias) diff --git a/x/nft/client/cli/query.go b/x/nft/client/cli/query.go index 63f567e761..a32489a76e 100644 --- a/x/nft/client/cli/query.go +++ b/x/nft/client/cli/query.go @@ -6,10 +6,10 @@ import ( "github.com/spf13/cobra" + "cosmossdk.io/core/address" "cosmossdk.io/x/nft" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/version" ) @@ -21,7 +21,7 @@ const ( ) // GetQueryCmd returns the cli query commands for this module -func GetQueryCmd() *cobra.Command { +func GetQueryCmd(ac address.Codec) *cobra.Command { nftQueryCmd := &cobra.Command{ Use: nft.ModuleName, Short: "Querying commands for the nft module", @@ -35,7 +35,7 @@ func GetQueryCmd() *cobra.Command { GetCmdQueryClass(), GetCmdQueryClasses(), GetCmdQueryNFT(), - GetCmdQueryNFTs(), + GetCmdQueryNFTs(ac), GetCmdQueryOwner(), GetCmdQueryBalance(), GetCmdQuerySupply(), @@ -127,7 +127,7 @@ func GetCmdQueryNFT() *cobra.Command { } // GetCmdQueryNFTs implements the query nft command. -func GetCmdQueryNFTs() *cobra.Command { +func GetCmdQueryNFTs(ac address.Codec) *cobra.Command { cmd := &cobra.Command{ Use: "nfts", Short: "query all NFTs of a given class or owner address.", @@ -156,7 +156,7 @@ $ %s query %s nfts --owner= } if len(owner) > 0 { - if _, err := sdk.AccAddressFromBech32(owner); err != nil { + if _, err := ac.StringToBytes(owner); err != nil { return err } } diff --git a/x/nft/client/cli/query_test.go b/x/nft/client/cli/query_test.go index 8e7411982a..d2e019df2d 100644 --- a/x/nft/client/cli/query_test.go +++ b/x/nft/client/cli/query_test.go @@ -9,6 +9,7 @@ import ( "cosmossdk.io/x/nft/client/cli" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/cosmos/cosmos-sdk/codec/address" svrcmd "github.com/cosmos/cosmos-sdk/server/cmd" "github.com/cosmos/cosmos-sdk/testutil" clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" @@ -169,7 +170,7 @@ func (s *CLITestSuite) TestQueryNFTs() { for _, tc := range testCases { s.Run(tc.name, func() { - cmd := cli.GetCmdQueryNFTs() + cmd := cli.GetCmdQueryNFTs(address.NewBech32Codec("cosmos")) var args []string args = append(args, fmt.Sprintf("--%s=%s", cli.FlagClassID, tc.args.ClassID)) args = append(args, fmt.Sprintf("--%s=%s", cli.FlagOwner, tc.args.Owner)) diff --git a/x/nft/expected_keepers.go b/x/nft/expected_keepers.go index b96a8ba11b..98fd9890b7 100644 --- a/x/nft/expected_keepers.go +++ b/x/nft/expected_keepers.go @@ -3,6 +3,7 @@ package nft import ( context "context" + "cosmossdk.io/core/address" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -16,4 +17,6 @@ type BankKeeper interface { type AccountKeeper interface { GetModuleAddress(name string) sdk.AccAddress GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI + + address.Codec } diff --git a/x/nft/genesis.go b/x/nft/genesis.go index 30b743a686..14a71f7dbb 100644 --- a/x/nft/genesis.go +++ b/x/nft/genesis.go @@ -1,11 +1,11 @@ package nft import ( - sdk "github.com/cosmos/cosmos-sdk/types" + "cosmossdk.io/core/address" ) // ValidateGenesis checks that the given genesis state has no integrity issues -func ValidateGenesis(data GenesisState) error { +func ValidateGenesis(data GenesisState, ac address.Codec) error { for _, class := range data.Classes { if len(class.Id) == 0 { return ErrEmptyClassID @@ -16,7 +16,7 @@ func ValidateGenesis(data GenesisState) error { if len(nft.Id) == 0 { return ErrEmptyNFTID } - if _, err := sdk.AccAddressFromBech32(entry.Owner); err != nil { + if _, err := ac.StringToBytes(entry.Owner); err != nil { return err } } diff --git a/x/nft/keeper/genesis.go b/x/nft/keeper/genesis.go index 37b9acd402..9b57cd8b65 100644 --- a/x/nft/keeper/genesis.go +++ b/x/nft/keeper/genesis.go @@ -17,7 +17,10 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data *nft.GenesisState) { } for _, entry := range data.Entries { for _, nft := range entry.Nfts { - owner := sdk.MustAccAddressFromBech32(entry.Owner) + owner, err := k.ac.StringToBytes(entry.Owner) + if err != nil { + panic(err) + } if err := k.Mint(ctx, *nft, owner); err != nil { panic(err) diff --git a/x/nft/keeper/grpc_query.go b/x/nft/keeper/grpc_query.go index 84bf2efba4..2742382881 100644 --- a/x/nft/keeper/grpc_query.go +++ b/x/nft/keeper/grpc_query.go @@ -24,7 +24,7 @@ func (k Keeper) Balance(goCtx context.Context, r *nft.QueryBalanceRequest) (*nft return nil, nft.ErrEmptyClassID } - owner, err := sdk.AccAddressFromBech32(r.Owner) + owner, err := k.ac.StringToBytes(r.Owner) if err != nil { return nil, err } @@ -77,7 +77,7 @@ func (k Keeper) NFTs(goCtx context.Context, r *nft.QueryNFTsRequest) (*nft.Query var owner sdk.AccAddress if len(r.Owner) > 0 { - owner, err = sdk.AccAddressFromBech32(r.Owner) + owner, err = k.ac.StringToBytes(r.Owner) if err != nil { return nil, err } diff --git a/x/nft/keeper/grpc_query_test.go b/x/nft/keeper/grpc_query_test.go index 13f9db530c..3320a09c61 100644 --- a/x/nft/keeper/grpc_query_test.go +++ b/x/nft/keeper/grpc_query_test.go @@ -16,6 +16,7 @@ func TestGRPCQuery(t *testing.T) { } func (s *TestSuite) TestBalance() { + s.accountKeeper.EXPECT().StringToBytes("owner").Return(nil, fmt.Errorf("decoding bech32 failed")).AnyTimes() var req *nft.QueryBalanceRequest testCases := []struct { msg string diff --git a/x/nft/keeper/keeper.go b/x/nft/keeper/keeper.go index b68d0004b7..218230e05f 100644 --- a/x/nft/keeper/keeper.go +++ b/x/nft/keeper/keeper.go @@ -1,6 +1,7 @@ package keeper import ( + "cosmossdk.io/core/address" store "cosmossdk.io/core/store" "cosmossdk.io/x/nft" @@ -12,6 +13,7 @@ type Keeper struct { cdc codec.BinaryCodec storeService store.KVStoreService bk nft.BankKeeper + ac address.Codec } // NewKeeper creates a new nft Keeper instance @@ -27,5 +29,6 @@ func NewKeeper(storeService store.KVStoreService, cdc: cdc, storeService: storeService, bk: bk, + ac: ak, } } diff --git a/x/nft/keeper/keeper_test.go b/x/nft/keeper/keeper_test.go index 1c926e7482..6d7fda36dd 100644 --- a/x/nft/keeper/keeper_test.go +++ b/x/nft/keeper/keeper_test.go @@ -37,10 +37,11 @@ const ( type TestSuite struct { suite.Suite - ctx sdk.Context - addrs []sdk.AccAddress - queryClient nft.QueryClient - nftKeeper keeper.Keeper + ctx sdk.Context + addrs []sdk.AccAddress + queryClient nft.QueryClient + nftKeeper keeper.Keeper + accountKeeper *nfttestutil.MockAccountKeeper encCfg moduletestutil.TestEncodingConfig } @@ -60,6 +61,11 @@ func (s *TestSuite) SetupTest() { accountKeeper := nfttestutil.NewMockAccountKeeper(ctrl) bankKeeper := nfttestutil.NewMockBankKeeper(ctrl) accountKeeper.EXPECT().GetModuleAddress("nft").Return(s.addrs[0]).AnyTimes() + for _, addr := range s.addrs { + accountKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes() + } + + s.accountKeeper = accountKeeper nftKeeper := keeper.NewKeeper(storeService, s.encCfg.Codec, accountKeeper, bankKeeper) queryHelper := baseapp.NewQueryServerTestHelper(ctx, s.encCfg.InterfaceRegistry) diff --git a/x/nft/keeper/msg_server.go b/x/nft/keeper/msg_server.go index 87e8dc270c..93fdef5287 100644 --- a/x/nft/keeper/msg_server.go +++ b/x/nft/keeper/msg_server.go @@ -1,6 +1,7 @@ package keeper import ( + "bytes" "context" errorsmod "cosmossdk.io/errors" @@ -15,17 +16,17 @@ 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 := sdk.AccAddressFromBech32(msg.Sender) + sender, err := k.ac.StringToBytes(msg.Sender) if err != nil { return nil, err } owner := k.GetOwner(ctx, msg.ClassId, msg.Id) - if !owner.Equals(sender) { - return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not the owner of nft %s", sender, 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) } - receiver, err := sdk.AccAddressFromBech32(msg.Receiver) + receiver, err := k.ac.StringToBytes(msg.Receiver) if err != nil { return nil, err } diff --git a/x/nft/module/module.go b/x/nft/module/module.go index 4404f87d26..a277f5bbf6 100644 --- a/x/nft/module/module.go +++ b/x/nft/module/module.go @@ -9,6 +9,7 @@ import ( "github.com/spf13/cobra" "google.golang.org/grpc" + "cosmossdk.io/core/address" "cosmossdk.io/core/appmodule" "cosmossdk.io/core/store" "cosmossdk.io/depinject" @@ -38,6 +39,7 @@ var ( // AppModuleBasic defines the basic application module used by the nft module. type AppModuleBasic struct { cdc codec.Codec + ac address.Codec } // Name returns the nft module's name. @@ -68,13 +70,13 @@ func (AppModuleBasic) DefaultGenesis(cdc codec.JSONCodec) json.RawMessage { } // ValidateGenesis performs genesis state validation for the nft module. -func (AppModuleBasic) ValidateGenesis(cdc codec.JSONCodec, config sdkclient.TxEncodingConfig, bz json.RawMessage) error { +func (ab AppModuleBasic) ValidateGenesis(cdc codec.JSONCodec, config sdkclient.TxEncodingConfig, bz json.RawMessage) error { var data nft.GenesisState if err := cdc.UnmarshalJSON(bz, &data); err != nil { return errors.Wrapf(err, "failed to unmarshal %s genesis state", nft.ModuleName) } - return nft.ValidateGenesis(data) + return nft.ValidateGenesis(data, ab.ac) } // RegisterGRPCGatewayRoutes registers the gRPC Gateway routes for the nft module. @@ -85,8 +87,8 @@ func (a AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx sdkclient.Context, m } // GetQueryCmd returns the cli query commands for the nft module -func (AppModuleBasic) GetQueryCmd() *cobra.Command { - return cli.GetQueryCmd() +func (ab AppModuleBasic) GetQueryCmd() *cobra.Command { + return cli.GetQueryCmd(ab.ac) } // GetTxCmd returns the transaction commands for the nft module @@ -107,7 +109,7 @@ type AppModule struct { // NewAppModule creates a new AppModule object func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, ak nft.AccountKeeper, bk nft.BankKeeper, registry cdctypes.InterfaceRegistry) AppModule { return AppModule{ - AppModuleBasic: AppModuleBasic{cdc: cdc}, + AppModuleBasic: AppModuleBasic{cdc: cdc, ac: ak}, keeper: keeper, accountKeeper: ak, bankKeeper: bk, diff --git a/x/nft/testutil/expected_keepers_mocks.go b/x/nft/testutil/expected_keepers_mocks.go index 545212bdcf..a54ef82070 100644 --- a/x/nft/testutil/expected_keepers_mocks.go +++ b/x/nft/testutil/expected_keepers_mocks.go @@ -72,6 +72,21 @@ func (m *MockAccountKeeper) EXPECT() *MockAccountKeeperMockRecorder { return m.recorder } +// BytesToString mocks base method. +func (m *MockAccountKeeper) BytesToString(bz []byte) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BytesToString", bz) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// BytesToString indicates an expected call of BytesToString. +func (mr *MockAccountKeeperMockRecorder) BytesToString(bz interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BytesToString", reflect.TypeOf((*MockAccountKeeper)(nil).BytesToString), bz) +} + // GetAccount mocks base method. func (m *MockAccountKeeper) GetAccount(ctx context.Context, addr types.AccAddress) types.AccountI { m.ctrl.T.Helper() @@ -99,3 +114,18 @@ func (mr *MockAccountKeeperMockRecorder) GetModuleAddress(name interface{}) *gom mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleAddress", reflect.TypeOf((*MockAccountKeeper)(nil).GetModuleAddress), name) } + +// StringToBytes mocks base method. +func (m *MockAccountKeeper) StringToBytes(text string) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "StringToBytes", text) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// StringToBytes indicates an expected call of StringToBytes. +func (mr *MockAccountKeeperMockRecorder) StringToBytes(text interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StringToBytes", reflect.TypeOf((*MockAccountKeeper)(nil).StringToBytes), text) +} diff --git a/x/staking/types/expected_keepers.go b/x/staking/types/expected_keepers.go index 9f2bc78d6e..b70505d0ec 100644 --- a/x/staking/types/expected_keepers.go +++ b/x/staking/types/expected_keepers.go @@ -3,6 +3,7 @@ package types import ( context "context" + "cosmossdk.io/core/address" "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" @@ -16,6 +17,8 @@ type DistributionKeeper interface { // AccountKeeper defines the expected account keeper (noalias) type AccountKeeper interface { + address.Codec + IterateAccounts(ctx context.Context, process func(sdk.AccountI) (stop bool)) GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI // only used for simulation @@ -24,11 +27,6 @@ type AccountKeeper interface { // TODO remove with genesis 2-phases refactor https://github.com/cosmos/cosmos-sdk/issues/2862 SetModuleAccount(context.Context, sdk.ModuleAccountI) - - // StringToBytes decodes address strings to bytes - StringToBytes(text string) ([]byte, error) - // BytesToString encodes address bytes to text - BytesToString(bz []byte) (string, error) } // BankKeeper defines the expected interface needed to retrieve account balances.