diff --git a/x/feegrant/CHANGELOG.md b/x/feegrant/CHANGELOG.md index 9dabfb6fa9..3b671d3bf3 100644 --- a/x/feegrant/CHANGELOG.md +++ b/x/feegrant/CHANGELOG.md @@ -34,3 +34,4 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#15606](https://github.com/cosmos/cosmos-sdk/pull/15606) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey` and methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context`. * [#15347](https://github.com/cosmos/cosmos-sdk/pull/15347) Remove global bech32 usage in keeper. * [#15347](https://github.com/cosmos/cosmos-sdk/pull/15347) `ValidateBasic` is treated as a no op now with with acceptance of RFC001 +* [#17869](https://github.com/cosmos/cosmos-sdk/pull/17869) `NewGrant`, `NewMsgGrantAllowance` & `NewMsgRevokeAllowance` takes strings instead of `sdk.AccAddress` diff --git a/x/feegrant/client/cli/tx.go b/x/feegrant/client/cli/tx.go index a1040facbd..5e892baf59 100644 --- a/x/feegrant/client/cli/tx.go +++ b/x/feegrant/client/cli/tx.go @@ -74,12 +74,16 @@ Examples: return err } - grantee, err := ac.StringToBytes(args[1]) + _, err = ac.StringToBytes(args[1]) if err != nil { return err } granter := clientCtx.GetFromAddress() + granterStr, err := ac.BytesToString(granter) + if err != nil { + return err + } sl, err := cmd.Flags().GetString(FlagSpendLimit) if err != nil { return err @@ -167,7 +171,7 @@ Examples: } } - msg, err := feegrant.NewMsgGrantAllowance(grant, granter, grantee) + msg, err := feegrant.NewMsgGrantAllowance(grant, granterStr, args[1]) if err != nil { return err } @@ -209,12 +213,17 @@ Example: return err } - grantee, err := ac.StringToBytes(args[1]) + _, err = ac.StringToBytes(args[1]) if err != nil { return err } - msg := feegrant.NewMsgRevokeAllowance(clientCtx.GetFromAddress(), grantee) + granter, err := ac.BytesToString(clientCtx.GetFromAddress()) + if err != nil { + return err + } + + msg := feegrant.NewMsgRevokeAllowance(granter, args[1]) return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), &msg) }, diff --git a/x/feegrant/client/cli/tx_test.go b/x/feegrant/client/cli/tx_test.go index f19e401a53..6ce3ff1d71 100644 --- a/x/feegrant/client/cli/tx_test.go +++ b/x/feegrant/client/cli/tx_test.go @@ -96,7 +96,12 @@ func (s *CLITestSuite) SetupSuite() { s.createGrant(granter, grantee) - grant, err := feegrant.NewGrant(granter, grantee, &feegrant.BasicAllowance{ + granteeStr, err := s.baseCtx.AddressCodec.BytesToString(grantee) + s.Require().NoError(err) + granterStr, err := s.baseCtx.AddressCodec.BytesToString(granter) + s.Require().NoError(err) + + grant, err := feegrant.NewGrant(granterStr, granteeStr, &feegrant.BasicAllowance{ SpendLimit: sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100))), }) s.Require().NoError(err) diff --git a/x/feegrant/filtered_fee_test.go b/x/feegrant/filtered_fee_test.go index c2cf736ae0..8f59ad334c 100644 --- a/x/feegrant/filtered_fee_test.go +++ b/x/feegrant/filtered_fee_test.go @@ -12,6 +12,7 @@ import ( "cosmossdk.io/x/feegrant" "cosmossdk.io/x/feegrant/module" + addresscodec "github.com/cosmos/cosmos-sdk/codec/address" "github.com/cosmos/cosmos-sdk/testutil" sdk "github.com/cosmos/cosmos-sdk/types" moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" @@ -33,6 +34,8 @@ func TestFilteredFeeValidAllow(t *testing.T) { now := ctx.HeaderInfo().Time oneHour := now.Add(1 * time.Hour) + ac := addresscodec.NewBech32Codec("cosmos") + // msg we will call in the all cases call := banktypes.MsgSend{} cases := map[string]struct { @@ -146,7 +149,9 @@ func TestFilteredFeeValidAllow(t *testing.T) { var granter, grantee sdk.AccAddress allowance, err := feegrant.NewAllowedMsgAllowance(tc.allowance, tc.msgs) require.NoError(t, err) - grant, err := feegrant.NewGrant(granter, grantee, allowance) + granterStr, err := ac.BytesToString(granter) + require.NoError(t, err) + granteeStr, err := ac.BytesToString(grantee) require.NoError(t, err) // now try to deduct @@ -166,8 +171,8 @@ func TestFilteredFeeValidAllow(t *testing.T) { // create a new updated grant newGrant, err := feegrant.NewGrant( - sdk.AccAddress(grant.Granter), - sdk.AccAddress(grant.Grantee), + granterStr, + granteeStr, allowance) require.NoError(t, err) diff --git a/x/feegrant/grant.go b/x/feegrant/grant.go index 56029b9ae2..0fb85dd5a2 100644 --- a/x/feegrant/grant.go +++ b/x/feegrant/grant.go @@ -6,14 +6,13 @@ import ( errorsmod "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/codec/types" - sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) var _ types.UnpackInterfacesMessage = &Grant{} // NewGrant creates a new FeeAllowanceGrant. -func NewGrant(granter, grantee sdk.AccAddress, feeAllowance FeeAllowanceI) (Grant, error) { +func NewGrant(granter, grantee string, feeAllowance FeeAllowanceI) (Grant, error) { msg, ok := feeAllowance.(proto.Message) if !ok { return Grant{}, errorsmod.Wrapf(sdkerrors.ErrPackAny, "cannot proto marshal %T", feeAllowance) @@ -25,8 +24,8 @@ func NewGrant(granter, grantee sdk.AccAddress, feeAllowance FeeAllowanceI) (Gran } return Grant{ - Granter: granter.String(), - Grantee: grantee.String(), + Granter: granter, + Grantee: grantee, Allowance: any, }, nil } diff --git a/x/feegrant/grant_test.go b/x/feegrant/grant_test.go index 52482dfa7f..a6be9ff267 100644 --- a/x/feegrant/grant_test.go +++ b/x/feegrant/grant_test.go @@ -82,7 +82,11 @@ func TestGrant(t *testing.T) { for name, tc := range cases { tc := tc t.Run(name, func(t *testing.T) { - grant, err := feegrant.NewGrant(tc.granter, tc.grantee, &feegrant.BasicAllowance{ + granterStr, err := addressCodec.BytesToString(tc.granter) + require.NoError(t, err) + granteeStr, err := addressCodec.BytesToString(tc.grantee) + require.NoError(t, err) + grant, err := feegrant.NewGrant(granterStr, granteeStr, &feegrant.BasicAllowance{ SpendLimit: tc.limit, Expiration: &tc.expires, }) diff --git a/x/feegrant/keeper/genesis_test.go b/x/feegrant/keeper/genesis_test.go index 0f4ddbe8eb..f48b9f0125 100644 --- a/x/feegrant/keeper/genesis_test.go +++ b/x/feegrant/keeper/genesis_test.go @@ -59,7 +59,6 @@ func TestImportExportGenesis(t *testing.T) { f := initFixture(t) f.accountKeeper.EXPECT().GetAccount(gomock.Any(), granteeAddr).Return(authtypes.NewBaseAccountWithAddress(granteeAddr)).AnyTimes() - f.accountKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() coins := sdk.NewCoins(sdk.NewCoin("foo", math.NewInt(1_000))) now := f.ctx.HeaderInfo().Time @@ -73,10 +72,15 @@ func TestImportExportGenesis(t *testing.T) { genesis, err := f.feegrantKeeper.ExportGenesis(f.ctx) assert.NilError(t, err) + granter, err := f.accountKeeper.AddressCodec().BytesToString(granterAddr.Bytes()) + assert.NilError(t, err) + grantee, err := f.accountKeeper.AddressCodec().BytesToString(granteeAddr.Bytes()) + assert.NilError(t, err) + // revoke fee allowance _, err = msgSrvr.RevokeAllowance(f.ctx, &feegrant.MsgRevokeAllowance{ - Granter: granterAddr.String(), - Grantee: granteeAddr.String(), + Granter: granter, + Grantee: grantee, }) assert.NilError(t, err) @@ -92,6 +96,13 @@ func TestInitGenesis(t *testing.T) { any, err := codectypes.NewAnyWithValue(&testdata.Dog{}) assert.NilError(t, err) + ac := address.NewBech32Codec("cosmos") + + granter, err := ac.BytesToString(granterAddr.Bytes()) + assert.NilError(t, err) + grantee, err := ac.BytesToString(granteeAddr.Bytes()) + assert.NilError(t, err) + testCases := []struct { name string feeAllowances []feegrant.Grant @@ -102,7 +113,7 @@ func TestInitGenesis(t *testing.T) { []feegrant.Grant{ { Granter: "invalid granter", - Grantee: granteeAddr.String(), + Grantee: grantee, }, }, true, @@ -111,7 +122,7 @@ func TestInitGenesis(t *testing.T) { "invalid grantee", []feegrant.Grant{ { - Granter: granterAddr.String(), + Granter: granter, Grantee: "invalid grantee", }, }, @@ -121,8 +132,8 @@ func TestInitGenesis(t *testing.T) { "invalid allowance", []feegrant.Grant{ { - Granter: granterAddr.String(), - Grantee: granteeAddr.String(), + Granter: granter, + Grantee: grantee, Allowance: any, }, }, diff --git a/x/feegrant/keeper/grpc_query_test.go b/x/feegrant/keeper/grpc_query_test.go index edb7208180..7959a5f604 100644 --- a/x/feegrant/keeper/grpc_query_test.go +++ b/x/feegrant/keeper/grpc_query_test.go @@ -30,7 +30,7 @@ func (suite *KeeperTestSuite) TestFeeAllowance() { "fail: invalid granter", &feegrant.QueryAllowanceRequest{ Granter: invalidGranter, - Grantee: suite.addrs[0].String(), + Grantee: suite.encodedAddrs[0], }, true, func() {}, @@ -39,7 +39,7 @@ func (suite *KeeperTestSuite) TestFeeAllowance() { { "fail: invalid grantee", &feegrant.QueryAllowanceRequest{ - Granter: suite.addrs[0].String(), + Granter: suite.encodedAddrs[0], Grantee: invalidGrantee, }, true, @@ -49,8 +49,8 @@ func (suite *KeeperTestSuite) TestFeeAllowance() { { "fail: no grants", &feegrant.QueryAllowanceRequest{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], }, true, func() {}, @@ -69,16 +69,16 @@ func (suite *KeeperTestSuite) TestFeeAllowance() { { "valid query: expect single grant", &feegrant.QueryAllowanceRequest{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], }, false, func() { suite.grantFeeAllowance(suite.addrs[0], suite.addrs[1]) }, func(response *feegrant.QueryAllowanceResponse) { - suite.Require().Equal(response.Allowance.Granter, suite.addrs[0].String()) - suite.Require().Equal(response.Allowance.Grantee, suite.addrs[1].String()) + suite.Require().Equal(response.Allowance.Granter, suite.encodedAddrs[0]) + suite.Require().Equal(response.Allowance.Grantee, suite.encodedAddrs[1]) }, }, } @@ -124,7 +124,7 @@ func (suite *KeeperTestSuite) TestFeeAllowances() { { "no grants", &feegrant.QueryAllowancesRequest{ - Grantee: suite.addrs[1].String(), + Grantee: suite.encodedAddrs[1], }, false, func() {}, @@ -135,7 +135,7 @@ func (suite *KeeperTestSuite) TestFeeAllowances() { { "valid query: expect single grant", &feegrant.QueryAllowancesRequest{ - Grantee: suite.addrs[1].String(), + Grantee: suite.encodedAddrs[1], }, false, func() { @@ -143,8 +143,8 @@ func (suite *KeeperTestSuite) TestFeeAllowances() { }, func(resp *feegrant.QueryAllowancesResponse) { suite.Require().Equal(len(resp.Allowances), 1) - suite.Require().Equal(resp.Allowances[0].Granter, suite.addrs[0].String()) - suite.Require().Equal(resp.Allowances[0].Grantee, suite.addrs[1].String()) + suite.Require().Equal(resp.Allowances[0].Granter, suite.encodedAddrs[0]) + suite.Require().Equal(resp.Allowances[0].Grantee, suite.encodedAddrs[1]) }, }, } @@ -190,7 +190,7 @@ func (suite *KeeperTestSuite) TestFeeAllowancesByGranter() { { "no grants", &feegrant.QueryAllowancesByGranterRequest{ - Granter: suite.addrs[0].String(), + Granter: suite.encodedAddrs[0], }, false, func() {}, @@ -201,7 +201,7 @@ func (suite *KeeperTestSuite) TestFeeAllowancesByGranter() { { "valid query: expect single grant", &feegrant.QueryAllowancesByGranterRequest{ - Granter: suite.addrs[0].String(), + Granter: suite.encodedAddrs[0], }, false, func() { @@ -212,8 +212,8 @@ func (suite *KeeperTestSuite) TestFeeAllowancesByGranter() { }, func(resp *feegrant.QueryAllowancesByGranterResponse) { suite.Require().Equal(len(resp.Allowances), 1) - suite.Require().Equal(resp.Allowances[0].Granter, suite.addrs[0].String()) - suite.Require().Equal(resp.Allowances[0].Grantee, suite.addrs[1].String()) + suite.Require().Equal(resp.Allowances[0].Granter, suite.encodedAddrs[0]) + suite.Require().Equal(resp.Allowances[0].Grantee, suite.encodedAddrs[1]) suite.Require().Equal(resp.Pagination.Total, uint64(1)) }, }, diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index e18dbfdeaa..fa2bbcc94a 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -97,7 +97,16 @@ func (k Keeper) GrantAllowance(ctx context.Context, granter, grantee sdk.AccAddr } } - grant, err := feegrant.NewGrant(granter, grantee, feeAllowance) + granterStr, err := k.authKeeper.AddressCodec().BytesToString(granter) + if err != nil { + return err + } + granteeStr, err := k.authKeeper.AddressCodec().BytesToString(grantee) + if err != nil { + return err + } + + grant, err := feegrant.NewGrant(granterStr, granteeStr, feeAllowance) if err != nil { return err } @@ -124,7 +133,16 @@ func (k Keeper) UpdateAllowance(ctx context.Context, granter, grantee sdk.AccAdd return err } - grant, err := feegrant.NewGrant(granter, grantee, feeAllowance) + granterStr, err := k.authKeeper.AddressCodec().BytesToString(granter) + if err != nil { + return err + } + granteeStr, err := k.authKeeper.AddressCodec().BytesToString(grantee) + if err != nil { + return err + } + + grant, err := feegrant.NewGrant(granterStr, granteeStr, feeAllowance) if err != nil { return err } @@ -166,11 +184,20 @@ func (k Keeper) revokeAllowance(ctx context.Context, granter, grantee sdk.AccAdd } } + granterStr, err := k.authKeeper.AddressCodec().BytesToString(granter) + if err != nil { + return err + } + granteeStr, err := k.authKeeper.AddressCodec().BytesToString(grantee) + if err != nil { + return err + } + sdk.UnwrapSDKContext(ctx).EventManager().EmitEvent( sdk.NewEvent( feegrant.EventTypeRevokeFeeGrant, - sdk.NewAttribute(feegrant.AttributeKeyGranter, granter.String()), - sdk.NewAttribute(feegrant.AttributeKeyGrantee, grantee.String()), + sdk.NewAttribute(feegrant.AttributeKeyGranter, granterStr), + sdk.NewAttribute(feegrant.AttributeKeyGrantee, granteeStr), ), ) return nil @@ -228,6 +255,15 @@ func (k Keeper) UseGrantedFees(ctx context.Context, granter, grantee sdk.AccAddr return err } + granterStr, err := k.authKeeper.AddressCodec().BytesToString(granter) + if err != nil { + return err + } + granteeStr, err := k.authKeeper.AddressCodec().BytesToString(grantee) + if err != nil { + return err + } + remove, err := grant.Accept(ctx, fee, msgs) if remove { // Ignoring the `revokeFeeAllowance` error, because the user has enough grants to perform this transaction. @@ -235,14 +271,14 @@ func (k Keeper) UseGrantedFees(ctx context.Context, granter, grantee sdk.AccAddr if err != nil { return err } - emitUseGrantEvent(ctx, granter.String(), grantee.String()) + emitUseGrantEvent(ctx, granterStr, granteeStr) return nil } if err != nil { return err } - emitUseGrantEvent(ctx, granter.String(), grantee.String()) + emitUseGrantEvent(ctx, granterStr, granteeStr) // if fee allowance is accepted, store the updated state of the allowance return k.UpdateAllowance(ctx, granter, grantee, grant) diff --git a/x/feegrant/keeper/keeper_test.go b/x/feegrant/keeper/keeper_test.go index 65edbd9166..f043a4df8f 100644 --- a/x/feegrant/keeper/keeper_test.go +++ b/x/feegrant/keeper/keeper_test.go @@ -28,6 +28,7 @@ type KeeperTestSuite struct { ctx sdk.Context addrs []sdk.AccAddress + encodedAddrs []string msgSrvr feegrant.MsgServer atom sdk.Coins feegrantKeeper keeper.Keeper @@ -52,7 +53,13 @@ func (suite *KeeperTestSuite) SetupTest() { suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[2]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[2])).AnyTimes() suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[3]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[3])).AnyTimes() - suite.accountKeeper.EXPECT().AddressCodec().Return(codecaddress.NewBech32Codec("cosmos")).AnyTimes() + ac := codecaddress.NewBech32Codec("cosmos") + suite.accountKeeper.EXPECT().AddressCodec().Return(ac).AnyTimes() + for _, addr := range suite.addrs { + str, err := ac.BytesToString(addr) + suite.Require().NoError(err) + suite.encodedAddrs = append(suite.encodedAddrs, str) + } suite.feegrantKeeper = keeper.NewKeeper(encCfg.Codec, runtime.NewKVStoreService(key), suite.accountKeeper) suite.ctx = testCtx.Ctx @@ -107,21 +114,21 @@ func (suite *KeeperTestSuite) TestKeeperCrud() { suite.Require().Error(err) // remove some, overwrite other - _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.addrs[0].String(), Grantee: suite.addrs[1].String()}) + _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.encodedAddrs[0], Grantee: suite.encodedAddrs[1]}) suite.Require().NoError(err) - _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.addrs[0].String(), Grantee: suite.addrs[2].String()}) + _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.encodedAddrs[0], Grantee: suite.encodedAddrs[2]}) suite.Require().NoError(err) // revoke non-exist fee allowance - _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.addrs[0].String(), Grantee: suite.addrs[2].String()}) + _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.encodedAddrs[0], Grantee: suite.encodedAddrs[2]}) suite.Require().Error(err) err = suite.feegrantKeeper.GrantAllowance(suite.ctx, suite.addrs[0], suite.addrs[2], basic) suite.Require().NoError(err) // revoke an existing grant and grant again with different allowance. - _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.addrs[1].String(), Grantee: suite.addrs[2].String()}) + _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.encodedAddrs[1], Grantee: suite.encodedAddrs[2]}) suite.Require().NoError(err) err = suite.feegrantKeeper.GrantAllowance(suite.ctx, suite.addrs[1], suite.addrs[2], basic3) @@ -183,7 +190,7 @@ func (suite *KeeperTestSuite) TestKeeperCrud() { _, err = suite.feegrantKeeper.GetAllowance(suite.ctx, suite.addrs[3], accAddr) suite.Require().NoError(err) - _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.addrs[3].String(), Grantee: address}) + _, err = suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{Granter: suite.encodedAddrs[3], Grantee: address}) suite.Require().NoError(err) } @@ -230,8 +237,8 @@ func (suite *KeeperTestSuite) TestUseGrantedFee() { final: future, postRun: func() { _, err := suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], }) suite.Require().NoError(err) }, @@ -244,8 +251,8 @@ func (suite *KeeperTestSuite) TestUseGrantedFee() { final: futureAfterSmall, postRun: func() { _, err := suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], }) suite.Require().NoError(err) }, @@ -314,8 +321,8 @@ func (suite *KeeperTestSuite) TestIterateGrants() { err = suite.feegrantKeeper.GrantAllowance(suite.ctx, suite.addrs[2], suite.addrs[1], allowance1) suite.Require().NoError(err) err = suite.feegrantKeeper.IterateAllFeeAllowances(suite.ctx, func(grant feegrant.Grant) bool { - suite.Require().Equal(suite.addrs[1].String(), grant.Grantee) - suite.Require().Contains([]string{suite.addrs[0].String(), suite.addrs[2].String()}, grant.Granter) + suite.Require().Equal(suite.encodedAddrs[1], grant.Grantee) + suite.Require().Contains([]string{suite.encodedAddrs[0], suite.encodedAddrs[2]}, grant.Granter) return true }) suite.Require().NoError(err) diff --git a/x/feegrant/keeper/msg_server_test.go b/x/feegrant/keeper/msg_server_test.go index ceac419a57..707b4264d8 100644 --- a/x/feegrant/keeper/msg_server_test.go +++ b/x/feegrant/keeper/msg_server_test.go @@ -34,7 +34,7 @@ func (suite *KeeperTestSuite) TestGrantAllowance() { invalid := "invalid-granter" return &feegrant.MsgGrantAllowance{ Granter: invalid, - Grantee: suite.addrs[1].String(), + Grantee: suite.encodedAddrs[1], Allowance: any, } }, @@ -48,7 +48,7 @@ func (suite *KeeperTestSuite) TestGrantAllowance() { suite.Require().NoError(err) invalid := "invalid-grantee" return &feegrant.MsgGrantAllowance{ - Granter: suite.addrs[0].String(), + Granter: suite.encodedAddrs[0], Grantee: invalid, Allowance: any, } @@ -79,7 +79,7 @@ func (suite *KeeperTestSuite) TestGrantAllowance() { suite.Require().NoError(err) return &feegrant.MsgGrantAllowance{ - Granter: suite.addrs[0].String(), + Granter: suite.encodedAddrs[0], Grantee: grantee, Allowance: any, } @@ -96,8 +96,8 @@ func (suite *KeeperTestSuite) TestGrantAllowance() { }) suite.Require().NoError(err) return &feegrant.MsgGrantAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], Allowance: any, } }, @@ -113,8 +113,8 @@ func (suite *KeeperTestSuite) TestGrantAllowance() { }) suite.Require().NoError(err) return &feegrant.MsgGrantAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], Allowance: any, } }, @@ -130,8 +130,8 @@ func (suite *KeeperTestSuite) TestGrantAllowance() { }) suite.Require().NoError(err) return &feegrant.MsgGrantAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], Allowance: any, } }, @@ -150,8 +150,8 @@ func (suite *KeeperTestSuite) TestGrantAllowance() { }) suite.Require().NoError(err) return &feegrant.MsgGrantAllowance{ - Granter: suite.addrs[1].String(), - Grantee: suite.addrs[2].String(), + Granter: suite.encodedAddrs[1], + Grantee: suite.encodedAddrs[2], Allowance: any, } }, @@ -170,8 +170,8 @@ func (suite *KeeperTestSuite) TestGrantAllowance() { }) suite.Require().NoError(err) return &feegrant.MsgGrantAllowance{ - Granter: suite.addrs[1].String(), - Grantee: suite.addrs[2].String(), + Granter: suite.encodedAddrs[1], + Grantee: suite.encodedAddrs[2], Allowance: any, } }, @@ -205,7 +205,7 @@ func (suite *KeeperTestSuite) TestRevokeAllowance() { "error: invalid granter", &feegrant.MsgRevokeAllowance{ Granter: invalidGranter, - Grantee: suite.addrs[1].String(), + Grantee: suite.encodedAddrs[1], }, func() {}, true, @@ -214,7 +214,7 @@ func (suite *KeeperTestSuite) TestRevokeAllowance() { { "error: invalid grantee", &feegrant.MsgRevokeAllowance{ - Granter: suite.addrs[0].String(), + Granter: suite.encodedAddrs[0], Grantee: invalidGrantee, }, func() {}, @@ -224,8 +224,8 @@ func (suite *KeeperTestSuite) TestRevokeAllowance() { { "error: fee allowance not found", &feegrant.MsgRevokeAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], }, func() {}, true, @@ -234,14 +234,14 @@ func (suite *KeeperTestSuite) TestRevokeAllowance() { { "success: revoke fee allowance", &feegrant.MsgRevokeAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], }, func() { // removing fee allowance from previous tests if exists _, err := suite.msgSrvr.RevokeAllowance(suite.ctx, &feegrant.MsgRevokeAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], }) suite.Require().Error(err) any, err := codectypes.NewAnyWithValue(&feegrant.PeriodicAllowance{ @@ -253,8 +253,8 @@ func (suite *KeeperTestSuite) TestRevokeAllowance() { }) suite.Require().NoError(err) req := &feegrant.MsgGrantAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], Allowance: any, } _, err = suite.msgSrvr.GrantAllowance(suite.ctx, req) @@ -266,8 +266,8 @@ func (suite *KeeperTestSuite) TestRevokeAllowance() { { "error: check fee allowance revoked", &feegrant.MsgRevokeAllowance{ - Granter: suite.addrs[0].String(), - Grantee: suite.addrs[1].String(), + Granter: suite.encodedAddrs[0], + Grantee: suite.encodedAddrs[1], }, func() {}, true, diff --git a/x/feegrant/migrations/v2/store_test.go b/x/feegrant/migrations/v2/store_test.go index 8dbafea7f9..ab8ad26fe0 100644 --- a/x/feegrant/migrations/v2/store_test.go +++ b/x/feegrant/migrations/v2/store_test.go @@ -13,6 +13,7 @@ import ( v2 "cosmossdk.io/x/feegrant/migrations/v2" "cosmossdk.io/x/feegrant/module" + addresscodec "github.com/cosmos/cosmos-sdk/codec/address" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" "github.com/cosmos/cosmos-sdk/runtime" "github.com/cosmos/cosmos-sdk/testutil" @@ -23,6 +24,7 @@ import ( func TestMigration(t *testing.T) { encodingConfig := moduletestutil.MakeTestEncodingConfig(module.AppModuleBasic{}) cdc := encodingConfig.Codec + ac := addresscodec.NewBech32Codec("cosmos") feegrantKey := storetypes.NewKVStoreKey(v2.ModuleName) ctx := testutil.DefaultContext(feegrantKey, storetypes.NewTransientStoreKey("transient_test")) @@ -68,7 +70,11 @@ func TestMigration(t *testing.T) { store := ctx.KVStore(feegrantKey) for _, grant := range grants { - newGrant, err := feegrant.NewGrant(grant.granter, grant.grantee, &feegrant.BasicAllowance{ + granterStr, err := ac.BytesToString(grant.granter) + require.NoError(t, err) + granteeStr, err := ac.BytesToString(grant.grantee) + require.NoError(t, err) + newGrant, err := feegrant.NewGrant(granterStr, granteeStr, &feegrant.BasicAllowance{ SpendLimit: grant.spendLimit, Expiration: grant.expiration, }) diff --git a/x/feegrant/module/abci_test.go b/x/feegrant/module/abci_test.go index 5af790303f..ff9c5b0fb4 100644 --- a/x/feegrant/module/abci_test.go +++ b/x/feegrant/module/abci_test.go @@ -44,8 +44,8 @@ func TestFeegrantPruning(t *testing.T) { accountKeeper.EXPECT().GetAccount(gomock.Any(), granter1).Return(authtypes.NewBaseAccountWithAddress(granter1)).AnyTimes() accountKeeper.EXPECT().GetAccount(gomock.Any(), granter2).Return(authtypes.NewBaseAccountWithAddress(granter2)).AnyTimes() accountKeeper.EXPECT().GetAccount(gomock.Any(), granter3).Return(authtypes.NewBaseAccountWithAddress(granter3)).AnyTimes() - - accountKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() + ac := address.NewBech32Codec("cosmos") + accountKeeper.EXPECT().AddressCodec().Return(ac).AnyTimes() feegrantKeeper := keeper.NewKeeper(encCfg.Codec, runtime.NewKVStoreService(key), accountKeeper) @@ -85,8 +85,10 @@ func TestFeegrantPruning(t *testing.T) { module.EndBlocker(testCtx.Ctx, feegrantKeeper) + granteeStr, err := ac.BytesToString(grantee) + require.NoError(t, err) res, err := queryClient.Allowances(testCtx.Ctx.Context(), &feegrant.QueryAllowancesRequest{ - Grantee: grantee.String(), + Grantee: granteeStr, }) require.NoError(t, err) require.NotNil(t, res) @@ -96,7 +98,7 @@ func TestFeegrantPruning(t *testing.T) { module.EndBlocker(testCtx.Ctx, feegrantKeeper) res, err = queryClient.Allowances(testCtx.Ctx.Context(), &feegrant.QueryAllowancesRequest{ - Grantee: grantee.String(), + Grantee: granteeStr, }) require.NoError(t, err) require.NotNil(t, res) diff --git a/x/feegrant/msgs.go b/x/feegrant/msgs.go index b0c4fcf09d..c049c73d4f 100644 --- a/x/feegrant/msgs.go +++ b/x/feegrant/msgs.go @@ -16,7 +16,7 @@ var ( ) // NewMsgGrantAllowance creates a new MsgGrantAllowance. -func NewMsgGrantAllowance(feeAllowance FeeAllowanceI, granter, grantee sdk.AccAddress) (*MsgGrantAllowance, error) { +func NewMsgGrantAllowance(feeAllowance FeeAllowanceI, granter, grantee string) (*MsgGrantAllowance, error) { msg, ok := feeAllowance.(proto.Message) if !ok { return nil, errorsmod.Wrapf(sdkerrors.ErrPackAny, "cannot proto marshal %T", msg) @@ -27,8 +27,8 @@ func NewMsgGrantAllowance(feeAllowance FeeAllowanceI, granter, grantee sdk.AccAd } return &MsgGrantAllowance{ - Granter: granter.String(), - Grantee: grantee.String(), + Granter: granter, + Grantee: grantee, Allowance: any, }, nil } @@ -51,6 +51,6 @@ func (msg MsgGrantAllowance) UnpackInterfaces(unpacker types.AnyUnpacker) error // NewMsgRevokeAllowance returns a message to revoke a fee allowance for a given // granter and grantee -func NewMsgRevokeAllowance(granter, grantee sdk.AccAddress) MsgRevokeAllowance { - return MsgRevokeAllowance{Granter: granter.String(), Grantee: grantee.String()} +func NewMsgRevokeAllowance(granter, grantee string) MsgRevokeAllowance { + return MsgRevokeAllowance{Granter: granter, Grantee: grantee} } diff --git a/x/feegrant/simulation/decoder_test.go b/x/feegrant/simulation/decoder_test.go index e1f1bfddb4..6033f9e751 100644 --- a/x/feegrant/simulation/decoder_test.go +++ b/x/feegrant/simulation/decoder_test.go @@ -11,6 +11,7 @@ import ( "cosmossdk.io/x/feegrant/module" "cosmossdk.io/x/feegrant/simulation" + addresscodec "github.com/cosmos/cosmos-sdk/codec/address" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/kv" @@ -27,8 +28,14 @@ func TestDecodeStore(t *testing.T) { encodingConfig := moduletestutil.MakeTestEncodingConfig(module.AppModuleBasic{}) cdc := encodingConfig.Codec dec := simulation.NewDecodeStore(cdc) + ac := addresscodec.NewBech32Codec("cosmos") - grant, err := feegrant.NewGrant(granterAddr, granteeAddr, &feegrant.BasicAllowance{ + granterStr, err := ac.BytesToString(granterAddr) + require.NoError(t, err) + granteeStr, err := ac.BytesToString(granteeAddr) + require.NoError(t, err) + + grant, err := feegrant.NewGrant(granterStr, granteeStr, &feegrant.BasicAllowance{ SpendLimit: sdk.NewCoins(sdk.NewCoin("foo", sdkmath.NewInt(100))), }) diff --git a/x/feegrant/simulation/genesis.go b/x/feegrant/simulation/genesis.go index 607111bc97..14066e8033 100644 --- a/x/feegrant/simulation/genesis.go +++ b/x/feegrant/simulation/genesis.go @@ -18,12 +18,12 @@ func genFeeGrants(r *rand.Rand, accounts []simtypes.Account) []feegrant.Grant { for i := 0; i < len(accounts)-1; i++ { granter := accounts[i].Address grantee := accounts[i+1].Address - allowances[i] = generateRandomAllowances(granter, grantee, r) + allowances[i] = generateRandomAllowances(granter.String(), grantee.String(), r) // TODO decouple this from call .String() } return allowances } -func generateRandomAllowances(granter, grantee sdk.AccAddress, r *rand.Rand) feegrant.Grant { +func generateRandomAllowances(granter, grantee string, r *rand.Rand) feegrant.Grant { allowances := make([]feegrant.Grant, 3) spendLimit := sdk.NewCoins(sdk.NewCoin("stake", math.NewInt(100))) periodSpendLimit := sdk.NewCoins(sdk.NewCoin("stake", math.NewInt(10))) diff --git a/x/feegrant/simulation/operations.go b/x/feegrant/simulation/operations.go index 38f2ef9f0c..78bb2950f3 100644 --- a/x/feegrant/simulation/operations.go +++ b/x/feegrant/simulation/operations.go @@ -83,7 +83,16 @@ func SimulateMsgGrantAllowance( ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { granter, _ := simtypes.RandomAcc(r, accs) grantee, _ := simtypes.RandomAcc(r, accs) - if grantee.Address.String() == granter.Address.String() { + granterStr, err := ak.AddressCodec().BytesToString(granter.Address) + if err != nil { + return simtypes.OperationMsg{}, nil, err + } + granteeStr, err := ak.AddressCodec().BytesToString(grantee.Address) + if err != nil { + return simtypes.OperationMsg{}, nil, err + } + + if granteeStr == granterStr { return simtypes.NoOpMsg(feegrant.ModuleName, TypeMsgGrantAllowance, "grantee and granter cannot be same"), nil, nil } @@ -102,7 +111,7 @@ func SimulateMsgGrantAllowance( msg, err := feegrant.NewMsgGrantAllowance(&feegrant.BasicAllowance{ SpendLimit: spendableCoins, Expiration: &oneYear, - }, granter.Address, grantee.Address) + }, granterStr, granteeStr) if err != nil { return simtypes.NoOpMsg(feegrant.ModuleName, TypeMsgGrantAllowance, err.Error()), nil, err } @@ -171,7 +180,15 @@ func SimulateMsgRevokeAllowance( account := ak.GetAccount(ctx, granter.Address) spendableCoins := bk.SpendableCoins(ctx, account.GetAddress()) - msg := feegrant.NewMsgRevokeAllowance(granterAddr, granteeAddr) + granterStr, err := ak.AddressCodec().BytesToString(granterAddr) + if err != nil { + return simtypes.NoOpMsg(feegrant.ModuleName, TypeMsgRevokeAllowance, err.Error()), nil, err + } + granteeStr, err := ak.AddressCodec().BytesToString(granteeAddr) + if err != nil { + return simtypes.NoOpMsg(feegrant.ModuleName, TypeMsgRevokeAllowance, err.Error()), nil, err + } + msg := feegrant.NewMsgRevokeAllowance(granterStr, granteeStr) txCtx := simulation.OperationInput{ R: r, diff --git a/x/feegrant/simulation/operations_test.go b/x/feegrant/simulation/operations_test.go index 8c193a2a82..008517e7d0 100644 --- a/x/feegrant/simulation/operations_test.go +++ b/x/feegrant/simulation/operations_test.go @@ -147,9 +147,13 @@ func (suite *SimTestSuite) TestSimulateMsgGrantAllowance() { s := rand.NewSource(1) r := rand.New(s) accounts := suite.getTestingAccounts(r, 3) + addr1, err := suite.accountKeeper.AddressCodec().BytesToString(accounts[1].Address) + require.NoError(err) + addr2, err := suite.accountKeeper.AddressCodec().BytesToString(accounts[2].Address) + require.NoError(err) // new block - _, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: app.LastBlockHeight() + 1}) + _, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: app.LastBlockHeight() + 1}) require.NoError(err) // execute operation @@ -161,8 +165,8 @@ func (suite *SimTestSuite) TestSimulateMsgGrantAllowance() { err = proto.Unmarshal(operationMsg.Msg, &msg) require.NoError(err) require.True(operationMsg.OK) - require.Equal(accounts[2].Address.String(), msg.Granter) - require.Equal(accounts[1].Address.String(), msg.Grantee) + require.Equal(addr2, msg.Granter) + require.Equal(addr1, msg.Grantee) require.Len(futureOperations, 0) } @@ -194,6 +198,11 @@ func (suite *SimTestSuite) TestSimulateMsgRevokeAllowance() { ) require.NoError(err) + granterStr, err := suite.accountKeeper.AddressCodec().BytesToString(accounts[0].Address) + require.NoError(err) + granteeStr, err := suite.accountKeeper.AddressCodec().BytesToString(accounts[1].Address) + require.NoError(err) + // execute operation op := simulation.SimulateMsgRevokeAllowance(codec.NewProtoCodec(suite.interfaceRegistry), suite.txConfig, suite.accountKeeper, suite.bankKeeper, suite.feegrantKeeper, codecaddress.NewBech32Codec("cosmos")) operationMsg, futureOperations, err := op(r, app.BaseApp, ctx, accounts, "") @@ -203,8 +212,8 @@ func (suite *SimTestSuite) TestSimulateMsgRevokeAllowance() { err = proto.Unmarshal(operationMsg.Msg, &msg) require.NoError(err) require.True(operationMsg.OK) - require.Equal(granter.Address.String(), msg.Granter) - require.Equal(grantee.Address.String(), msg.Grantee) + require.Equal(granterStr, msg.Granter) + require.Equal(granteeStr, msg.Grantee) require.Len(futureOperations, 0) }