x/feegrant audit: clean up / add test coverage to types package (#9193)

* Squashed commit of the following:

commit 58dc50051226a9eeb8d0ebea5bb0908fe5b9637f
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Fri Apr 23 15:09:27 2021 -0700

    remove comments

commit a84107e1b3eaa31324cb0f4f097b49f02af79c69
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Fri Apr 23 15:02:41 2021 -0700

    add tests for msgs.go

commit 2ad16869237e9631b402c93cde650c3fc554daf2
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Fri Apr 23 13:20:21 2021 -0700

    specify test name

commit b7121277c9be586a7c80d010ec401e50b510e02a
Merge: c0c134d971 3c65c3dacd
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Fri Apr 23 12:54:55 2021 -0700

    Merge branch 'master' into ty-9115-types_tests

commit c0c134d97107194dc4f9d3c501a15d023ae083e5
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Thu Apr 22 19:59:11 2021 -0700

    -add test case to cli_test.go for filtered fee
    -clean up identifiers
    -remove redundant import alias from filtered_fee.go

commit f7ab3699da39be3ab886f96962d28d23438d2e8e
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Thu Apr 22 09:57:31 2021 -0700

    remove unecessary constant

commit 9db59a82a7337cf5a7a3569c6900a44a6c81e8b4
Merge: a3e75ceb8a e28271b8e6
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Thu Apr 22 09:19:20 2021 -0700

    Merge branch 'master' into ty-9115-types_tests

commit a3e75ceb8a510ad9db43dd96073c43b7a8b062b0
Merge: 4d3dafab85 bffcae54a1
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Wed Apr 21 12:04:39 2021 -0700

    Merge branch 'master' into ty-9115-types_tests

commit 4d3dafab85d85526a7c94b045289605289ee6b0d
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Wed Apr 21 12:03:41 2021 -0700

    cleanup id's / add test case

commit e8f6924931ba95e592bfc3057ba167700458da41
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Wed Apr 21 10:55:29 2021 -0700

    add test for 0 allowance / remove unused field on exp test

commit 516b6ef89a4582ad681cc6c5c101cf50a9a54fb5
Author: technicallyty <48813565+tytech3@users.noreply.github.com>
Date:   Tue Apr 20 15:22:48 2021 -0700

    make names more clear, remove unused field

* fix imports

* fix tests

* rename test field

Co-authored-by: technicallyty <48813565+tytech3@users.noreply.github.com>
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: MD Aleem <72057206+aleem1314@users.noreply.github.com>

x/feegrant/client/testutil/suite.go

@@ -649,7 +649,7 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() {
 	}
 	spendLimit := sdk.NewCoin("stake", sdk.NewInt(1000))
 
-	allowMsgs := "/cosmos.gov.v1beta1.Msg/SubmitProposal"
+	allowMsgs := "/cosmos.gov.v1beta1.Msg/SubmitProposal,weighted_vote"
 
 	testCases := []struct {
 		name         string
@@ -659,10 +659,10 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() {
 		expectedCode uint32
 	}{
 		{
-			"wrong granter",
+			"invalid granter address",
 			append(
 				[]string{
-					"wrong granter",
+					"not an address",
 					"cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl",
 					fmt.Sprintf("--%s=%s", cli.FlagAllowedMsgs, allowMsgs),
 					fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, spendLimit.String()),
@@ -673,11 +673,11 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() {
 			true, &sdk.TxResponse{}, 0,
 		},
 		{
-			"wrong grantee",
+			"invalid grantee address",
 			append(
 				[]string{
 					granter.String(),
-					"wrong grantee",
+					"not an address",
 					fmt.Sprintf("--%s=%s", cli.FlagAllowedMsgs, allowMsgs),
 					fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, spendLimit.String()),
 					fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
@@ -753,22 +753,30 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() {
 	cases := []struct {
 		name         string
 		malleate     func() (testutil.BufferWriter, error)
-		expectErr    bool
 		respType     proto.Message
 		expectedCode uint32
 	}{
 		{
-			"valid tx",
+			"valid proposal tx",
 			func() (testutil.BufferWriter, error) {
 				return govtestutil.MsgSubmitProposal(val.ClientCtx, grantee.String(),
 					"Text Proposal", "No desc", govtypes.ProposalTypeText,
 					fmt.Sprintf("--%s=%s", flags.FlagFeeAccount, granter.String()),
 				)
 			},
-			false,
 			&sdk.TxResponse{},
 			0,
 		},
+		{
+			"valid weighted_vote tx",
+			func() (testutil.BufferWriter, error) {
+				return govtestutil.MsgVote(val.ClientCtx, grantee.String(), "0", "yes",
+					fmt.Sprintf("--%s=%s", flags.FlagFeeAccount, granter.String()),
+				)
+			},
+			&sdk.TxResponse{},
+			2,
+		},
 		{
 			"should fail with unauthorized msgs",
 			func() (testutil.BufferWriter, error) {
@@ -784,7 +792,8 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() {
 				cmd := cli.NewCmdFeeGrant()
 				return clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
 			},
-			false, &sdk.TxResponse{}, 7,
+			&sdk.TxResponse{},
+			7,
 		},
 	}
 
@@ -793,16 +802,10 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() {
 
 		s.Run(tc.name, func() {
 			out, err := tc.malleate()
-
+			s.Require().NoError(err)
-			if tc.expectErr {
+			s.Require().NoError(clientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), tc.respType), out.String())
-				s.Require().Error(err)
+			txResp := tc.respType.(*sdk.TxResponse)
-			} else {
+			s.Require().Equal(tc.expectedCode, txResp.Code, out.String())
-				s.Require().NoError(err)
-				s.Require().NoError(clientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), tc.respType), out.String())
-
-				txResp := tc.respType.(*sdk.TxResponse)
-				s.Require().Equal(tc.expectedCode, txResp.Code, out.String())
-			}
 		})
 	}
 }

x/feegrant/types/basic_fee_test.go

@@ -22,53 +22,47 @@ func TestBasicFeeValidAllow(t *testing.T) {
 	leftAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 512))
 
 	cases := map[string]struct {
-		allow *types.BasicFeeAllowance
+		allowance *types.BasicFeeAllowance
 		// all other checks are ignored if valid=false
 		fee         sdk.Coins
 		blockHeight int64
-		valid       bool
 		accept      bool
-		remove      bool
+		remove     bool
 		remains     sdk.Coins
 	}{
 		"empty": {
-			allow:  &types.BasicFeeAllowance{},
+			allowance:  &types.BasicFeeAllowance{},
-			valid:  true,
 			accept: true,
 		},
 		"small fee without expire": {
-			allow: &types.BasicFeeAllowance{
+			allowance: &types.BasicFeeAllowance{
 				SpendLimit: atom,
 			},
-			valid:   true,
 			fee:     smallAtom,
 			accept:  true,
 			remove:  false,
 			remains: leftAtom,
 		},
 		"all fee without expire": {
-			allow: &types.BasicFeeAllowance{
+			allowance: &types.BasicFeeAllowance{
 				SpendLimit: smallAtom,
 			},
-			valid:  true,
 			fee:    smallAtom,
 			accept: true,
 			remove: true,
 		},
 		"wrong fee": {
-			allow: &types.BasicFeeAllowance{
+			allowance: &types.BasicFeeAllowance{
 				SpendLimit: smallAtom,
 			},
-			valid:  true,
 			fee:    eth,
 			accept: false,
 		},
 		"non-expired": {
-			allow: &types.BasicFeeAllowance{
+			allowance: &types.BasicFeeAllowance{
 				SpendLimit: atom,
 				Expiration: types.ExpiresAtHeight(100),
 			},
-			valid:       true,
 			fee:         smallAtom,
 			blockHeight: 85,
 			accept:      true,
@@ -76,40 +70,36 @@ func TestBasicFeeValidAllow(t *testing.T) {
 			remains:     leftAtom,
 		},
 		"expired": {
-			allow: &types.BasicFeeAllowance{
+			allowance: &types.BasicFeeAllowance{
 				SpendLimit: atom,
 				Expiration: types.ExpiresAtHeight(100),
 			},
-			valid:       true,
 			fee:         smallAtom,
 			blockHeight: 121,
 			accept:      false,
 			remove:      true,
 		},
 		"fee more than allowed": {
-			allow: &types.BasicFeeAllowance{
+			allowance: &types.BasicFeeAllowance{
 				SpendLimit: atom,
 				Expiration: types.ExpiresAtHeight(100),
 			},
-			valid:       true,
 			fee:         bigAtom,
 			blockHeight: 85,
 			accept:      false,
 		},
 		"with out spend limit": {
-			allow: &types.BasicFeeAllowance{
+			allowance: &types.BasicFeeAllowance{
 				Expiration: types.ExpiresAtHeight(100),
 			},
-			valid:       true,
 			fee:         bigAtom,
 			blockHeight: 85,
 			accept:      true,
 		},
 		"expired no spend limit": {
-			allow: &types.BasicFeeAllowance{
+			allowance: &types.BasicFeeAllowance{
 				Expiration: types.ExpiresAtHeight(100),
 			},
-			valid:       true,
 			fee:         bigAtom,
 			blockHeight: 120,
 			accept:      false,
@@ -119,26 +109,22 @@ func TestBasicFeeValidAllow(t *testing.T) {
 	for name, stc := range cases {
 		tc := stc // to make scopelint happy
 		t.Run(name, func(t *testing.T) {
-			err := tc.allow.ValidateBasic()
+			err := tc.allowance.ValidateBasic()
-			if !tc.valid {
-				require.Error(t, err)
-				return
-			}
 			require.NoError(t, err)
 
 			ctx := app.BaseApp.NewContext(false, tmproto.Header{}).WithBlockHeight(tc.blockHeight)
 
 			// now try to deduct
-			remove, err := tc.allow.Accept(ctx, tc.fee, []sdk.Msg{})
+			removed, err := tc.allowance.Accept(ctx, tc.fee, []sdk.Msg{})
 			if !tc.accept {
 				require.Error(t, err)
 				return
 			}
 			require.NoError(t, err)
 
-			require.Equal(t, tc.remove, remove)
+			require.Equal(t, tc.remove, removed)
-			if !remove {
+			if !removed {
-				assert.Equal(t, tc.allow.SpendLimit, tc.remains)
+				assert.Equal(t, tc.allowance.SpendLimit, tc.remains)
 			}
 		})
 	}

x/feegrant/types/expiration_test.go

@@ -14,33 +14,28 @@ func TestExpiresAt(t *testing.T) {
 	now := time.Now()
 
 	cases := map[string]struct {
-		example types.ExpiresAt
+		expires types.ExpiresAt
-		valid   bool
 		zero    bool
 		before  types.ExpiresAt
 		after   types.ExpiresAt
 	}{
 		"basic": {
-			example: types.ExpiresAtHeight(100),
+			expires: types.ExpiresAtHeight(100),
-			valid:   true,
 			before:  types.ExpiresAtHeight(50),
 			after:   types.ExpiresAtHeight(122),
 		},
 		"zero": {
-			example: types.ExpiresAt{},
+			expires: types.ExpiresAt{},
 			zero:    true,
-			valid:   true,
 			before:  types.ExpiresAtHeight(1),
 		},
 		"match height": {
-			example: types.ExpiresAtHeight(1000),
+			expires: types.ExpiresAtHeight(1000),
-			valid:   true,
 			before:  types.ExpiresAtHeight(999),
 			after:   types.ExpiresAtHeight(1000),
 		},
 		"match time": {
-			example: types.ExpiresAtTime(now),
+			expires: types.ExpiresAtTime(now),
-			valid:   true,
 			before:  types.ExpiresAtTime(now.Add(-1 * time.Second)),
 			after:   types.ExpiresAtTime(now.Add(1 * time.Second)),
 		},
@@ -49,19 +44,15 @@ func TestExpiresAt(t *testing.T) {
 	for name, stc := range cases {
 		tc := stc // to make scopelint happy
 		t.Run(name, func(t *testing.T) {
-			err := tc.example.ValidateBasic()
+			err := tc.expires.ValidateBasic()
-			assert.Equal(t, tc.zero, tc.example.Undefined())
+			assert.Equal(t, tc.zero, tc.expires.Undefined())
-			if !tc.valid {
-				require.Error(t, err)
-				return
-			}
 			require.NoError(t, err)
 
 			if !tc.before.Undefined() {
-				assert.Equal(t, false, tc.example.IsExpired(tc.before.GetTime(), tc.before.GetHeight()))
+				assert.Equal(t, false, tc.expires.IsExpired(tc.before.GetTime(), tc.before.GetHeight()))
 			}
 			if !tc.after.Undefined() {
-				assert.Equal(t, true, tc.example.IsExpired(tc.after.GetTime(), tc.after.GetHeight()))
+				assert.Equal(t, true, tc.expires.IsExpired(tc.after.GetTime(), tc.after.GetHeight()))
 			}
 		})
 	}

x/feegrant/types/filtered_fee.go

@@ -3,11 +3,10 @@ package types
 import (
 	"time"
 
-	proto "github.com/gogo/protobuf/proto"
-
 	"github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/gogo/protobuf/proto"
 )
 
 // TODO: Revisit this once we have propoer gas fee framework.

x/feegrant/types/grant_test.go

@@ -2,8 +2,8 @@ package types_test
 
 import (
 	"testing"
+	"time"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/cosmos/cosmos-sdk/simapp"
@@ -18,66 +18,70 @@ func TestGrant(t *testing.T) {
 	addr2, err := sdk.AccAddressFromBech32("cosmos1p9qh4ldfd6n0qehujsal4k7g0e37kel90rc4ts")
 	require.NoError(t, err)
 	atom := sdk.NewCoins(sdk.NewInt64Coin("atom", 555))
-
+	zeroAtoms := sdk.NewCoins(sdk.NewInt64Coin("atom", 0))
-	goodGrant, err := types.NewFeeAllowanceGrant(addr2, addr, &types.BasicFeeAllowance{
-		SpendLimit: atom,
-		Expiration: types.ExpiresAtHeight(100),
-	})
-	require.NoError(t, err)
-
-	noGranteeGrant, err := types.NewFeeAllowanceGrant(addr2, nil, &types.BasicFeeAllowance{
-		SpendLimit: atom,
-		Expiration: types.ExpiresAtHeight(100),
-	})
-	require.NoError(t, err)
-
-	noGranterGrant, err := types.NewFeeAllowanceGrant(nil, addr, &types.BasicFeeAllowance{
-		SpendLimit: atom,
-		Expiration: types.ExpiresAtHeight(100),
-	})
-	require.NoError(t, err)
-
-	selfGrant, err := types.NewFeeAllowanceGrant(addr2, addr2, &types.BasicFeeAllowance{
-		SpendLimit: atom,
-		Expiration: types.ExpiresAtHeight(100),
-	})
-	require.NoError(t, err)
-
-	badAllowanceGrant, err := types.NewFeeAllowanceGrant(addr2, addr, &types.BasicFeeAllowance{
-		SpendLimit: atom,
-		Expiration: types.ExpiresAtHeight(-1),
-	})
-	require.NoError(t, err)
-
 	cdc := app.AppCodec()
-	// RegisterLegacyAminoCodec(cdc)
 
 	cases := map[string]struct {
-		grant types.FeeAllowanceGrant
+		granter sdk.AccAddress
+		grantee sdk.AccAddress
+		limit sdk.Coins
+		expires types.ExpiresAt
 		valid bool
 	}{
 		"good": {
-			grant: goodGrant,
+			granter: addr2,
+			grantee: addr,
+			limit: atom,
+			expires: types.ExpiresAtHeight(100),
 			valid: true,
 		},
 		"no grantee": {
-			grant: noGranteeGrant,
+			granter: addr2,
+			grantee: nil,
+			limit: atom,
+			expires: types.ExpiresAtHeight(100),
+			valid: false,
 		},
 		"no granter": {
-			grant: noGranterGrant,
+			granter: nil,
+			grantee: addr,
+			limit: atom,
+			expires: types.ExpiresAtHeight(100),
+			valid: false,
 		},
 		"self-grant": {
-			grant: selfGrant,
+			granter: addr2,
+			grantee: addr2,
+			limit: atom,
+			expires: types.ExpiresAtHeight(100),
+			valid: false,
 		},
-		"bad allowance": {
+		"bad height": {
-			grant: badAllowanceGrant,
+			granter: addr2,
+			grantee: addr,
+			limit: atom,
+			expires: types.ExpiresAtHeight(-100),
+			valid: false,
+		},
+		"zero allowance": {
+			granter: addr2,
+			grantee: addr,
+			limit: zeroAtoms,
+			expires: types.ExpiresAtTime(time.Now().Add(3 * time.Hour)),
+			valid: false,
 		},
 	}
 
 	for name, tc := range cases {
 		tc := tc
 		t.Run(name, func(t *testing.T) {
-			err := tc.grant.ValidateBasic()
+			grant, err := types.NewFeeAllowanceGrant(tc.granter, tc.grantee, &types.BasicFeeAllowance{
+				SpendLimit: tc.limit,
+				Expiration: tc.expires,
+			})
+			require.NoError(t, err)
+			err = grant.ValidateBasic()
+
 			if !tc.valid {
 				require.Error(t, err)
 				return
@@ -85,7 +89,7 @@ func TestGrant(t *testing.T) {
 			require.NoError(t, err)
 
 			// if it is valid, let's try to serialize, deserialize, and make sure it matches
-			bz, err := cdc.MarshalBinaryBare(&tc.grant)
+			bz, err := cdc.MarshalBinaryBare(&grant)
 			require.NoError(t, err)
 			var loaded types.FeeAllowanceGrant
 			err = cdc.UnmarshalBinaryBare(bz, &loaded)
@@ -93,8 +97,7 @@ func TestGrant(t *testing.T) {
 
 			err = loaded.ValidateBasic()
 			require.NoError(t, err)
-
+			require.Equal(t, grant, loaded)
-			assert.Equal(t, tc.grant, loaded)
 		})
 	}
 }

x/feegrant/types/msgs.go

@@ -52,6 +52,7 @@ func (msg MsgGrantFeeAllowance) ValidateBasic() error {
 	return allowance.ValidateBasic()
 }
 
+// GetSigners gets the granter account associated with an allowance
 func (msg MsgGrantFeeAllowance) GetSigners() []sdk.AccAddress {
 	granter, err := sdk.AccAddressFromBech32(msg.Granter)
 	if err != nil {
@@ -76,6 +77,8 @@ func (msg MsgGrantFeeAllowance) UnpackInterfaces(unpacker types.AnyUnpacker) err
 	return unpacker.UnpackAny(msg.Allowance, &allowance)
 }
 
+// NewMsgRevokeFeeAllowance returns a message to revoke a fee allowance for a given
+// granter and grantee
 //nolint:interfacer
 func NewMsgRevokeFeeAllowance(granter sdk.AccAddress, grantee sdk.AccAddress) MsgRevokeFeeAllowance {
 	return MsgRevokeFeeAllowance{Granter: granter.String(), Grantee: grantee.String()}
@@ -88,10 +91,15 @@ func (msg MsgRevokeFeeAllowance) ValidateBasic() error {
 	if msg.Grantee == "" {
 		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "missing grantee address")
 	}
+	if msg.Grantee == msg.Granter {
+		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "addresses must be different")
+	}
 
 	return nil
 }
 
+// GetSigners gets the granter address associated with an Allowance
+// to revoke.
 func (msg MsgRevokeFeeAllowance) GetSigners() []sdk.AccAddress {
 	granter, err := sdk.AccAddressFromBech32(msg.Granter)
 	if err != nil {

x/feegrant/types/msgs_test.go

@@ -0,0 +1,128 @@
+package types_test
+
+import (
+	"github.com/cosmos/cosmos-sdk/codec"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/feegrant/types"
+	"github.com/stretchr/testify/require"
+	"testing"
+	"time"
+)
+
+func TestMsgGrantFeeAllowance(t *testing.T) {
+	cdc := codec.NewProtoCodec(codectypes.NewInterfaceRegistry())
+	addr, _ := sdk.AccAddressFromBech32("cosmos1aeuqja06474dfrj7uqsvukm6rael982kk89mqr")
+	addr2, _ := sdk.AccAddressFromBech32("cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl")
+	atom := sdk.NewCoins(sdk.NewInt64Coin("atom", 555))
+	basic := &types.BasicFeeAllowance{
+		SpendLimit: atom,
+		Expiration: types.ExpiresAtTime(time.Now().Add(3 * time.Hour)),
+	}
+	cases := map[string]struct {
+		grantee sdk.AccAddress
+		granter sdk.AccAddress
+		grant 	*types.BasicFeeAllowance
+		valid 	bool
+	}{
+		"valid":{
+			grantee: addr,
+			granter: addr2,
+			grant: basic,
+			valid: true,
+		},
+		"no grantee": {
+			granter: addr2,
+			grantee: sdk.AccAddress{},
+			grant: basic,
+			valid: false,
+		},
+		"no granter": {
+			granter: sdk.AccAddress{},
+			grantee: addr,
+			grant: basic,
+			valid: false,
+		},
+		"grantee == granter":{
+			grantee: addr,
+			granter: addr,
+			grant: basic,
+			valid: false,
+		},
+	}
+
+	for _,tc := range cases {
+		msg, err := types.NewMsgGrantFeeAllowance(tc.grant, tc.granter, tc.grantee)
+		require.NoError(t, err)
+		err = msg.ValidateBasic()
+
+		if tc.valid {
+			require.NoError(t, err)
+
+			addrSlice := msg.GetSigners()
+			require.Equal(t, tc.granter.String(), addrSlice[0].String())
+
+			allowance, err := msg.GetFeeAllowanceI()
+			require.NoError(t, err)
+			require.Equal(t, tc.grant, allowance)
+
+			err = msg.UnpackInterfaces(cdc)
+			require.NoError(t, err)
+		} else {
+			require.Error(t, err)
+		}
+	}
+}
+
+func TestMsgRevokeFeeAllowance(t *testing.T) {
+	addr, _ := sdk.AccAddressFromBech32("cosmos1aeuqja06474dfrj7uqsvukm6rael982kk89mqr")
+	addr2, _ := sdk.AccAddressFromBech32("cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl")
+	atom := sdk.NewCoins(sdk.NewInt64Coin("atom", 555))
+	basic := &types.BasicFeeAllowance{
+		SpendLimit: atom,
+		Expiration: types.ExpiresAtTime(time.Now().Add(3 * time.Hour)),
+	}
+	cases := map[string]struct {
+		grantee sdk.AccAddress
+		granter sdk.AccAddress
+		grant 	*types.BasicFeeAllowance
+		valid 	bool
+	}{
+		"valid":{
+			grantee: addr,
+			granter: addr2,
+			grant: basic,
+			valid: true,
+		},
+		"no grantee": {
+			granter: addr2,
+			grantee: sdk.AccAddress{},
+			grant: basic,
+			valid: false,
+		},
+		"no granter": {
+			granter: sdk.AccAddress{},
+			grantee: addr,
+			grant: basic,
+			valid: false,
+		},
+		"grantee == granter":{
+			grantee: addr,
+			granter: addr,
+			grant: basic,
+			valid: false,
+		},
+	}
+
+	for _,tc := range cases {
+		msg := types.NewMsgRevokeFeeAllowance(tc.granter, tc.grantee)
+		err := msg.ValidateBasic()
+		if tc.valid {
+			require.NoError(t, err)
+			addrSlice := msg.GetSigners()
+			require.Equal(t, tc.granter.String(), addrSlice[0].String())
+		} else {
+			require.Error(t, err)
+		}
+	}
+}

x/feegrant/types/periodic_fee_test.go

@@ -21,7 +21,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 	eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 1))
 
 	cases := map[string]struct {
-		allow types.PeriodicFeeAllowance
+		allowance types.PeriodicFeeAllowance
 		// all other checks are ignored if valid=false
 		fee           sdk.Coins
 		blockHeight   int64
@@ -33,11 +33,11 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 		periodReset   types.ExpiresAt
 	}{
 		"empty": {
-			allow: types.PeriodicFeeAllowance{},
+			allowance: types.PeriodicFeeAllowance{},
 			valid: false,
 		},
 		"only basic": {
-			allow: types.PeriodicFeeAllowance{
+			allowance: types.PeriodicFeeAllowance{
 				Basic: types.BasicFeeAllowance{
 					SpendLimit: atom,
 					Expiration: types.ExpiresAtHeight(100),
@@ -46,7 +46,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			valid: false,
 		},
 		"empty basic": {
-			allow: types.PeriodicFeeAllowance{
+			allowance: types.PeriodicFeeAllowance{
 				Period:           types.BlockDuration(10),
 				PeriodSpendLimit: smallAtom,
 				PeriodReset:      types.ExpiresAtHeight(70),
@@ -58,7 +58,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			periodReset: types.ExpiresAtHeight(80),
 		},
 		"mismatched currencies": {
-			allow: types.PeriodicFeeAllowance{
+			allowance: types.PeriodicFeeAllowance{
 				Basic: types.BasicFeeAllowance{
 					SpendLimit: atom,
 					Expiration: types.ExpiresAtHeight(100),
@@ -69,7 +69,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			valid: false,
 		},
 		"first time": {
-			allow: types.PeriodicFeeAllowance{
+			allowance: types.PeriodicFeeAllowance{
 				Basic: types.BasicFeeAllowance{
 					SpendLimit: atom,
 					Expiration: types.ExpiresAtHeight(100),
@@ -87,7 +87,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			periodReset:   types.ExpiresAtHeight(85),
 		},
 		"same period": {
-			allow: types.PeriodicFeeAllowance{
+			allowance: types.PeriodicFeeAllowance{
 				Basic: types.BasicFeeAllowance{
 					SpendLimit: atom,
 					Expiration: types.ExpiresAtHeight(100),
@@ -107,7 +107,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			periodReset:   types.ExpiresAtHeight(80),
 		},
 		"step one period": {
-			allow: types.PeriodicFeeAllowance{
+			allowance: types.PeriodicFeeAllowance{
 				Basic: types.BasicFeeAllowance{
 					SpendLimit: atom,
 					Expiration: types.ExpiresAtHeight(100),
@@ -126,7 +126,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			periodReset:   types.ExpiresAtHeight(80), // one step from last reset, not now
 		},
 		"step limited by global allowance": {
-			allow: types.PeriodicFeeAllowance{
+			allowance: types.PeriodicFeeAllowance{
 				Basic: types.BasicFeeAllowance{
 					SpendLimit: smallAtom,
 					Expiration: types.ExpiresAtHeight(100),
@@ -145,7 +145,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			periodReset:   types.ExpiresAtHeight(80), // one step from last reset, not now
 		},
 		"expired": {
-			allow: types.PeriodicFeeAllowance{
+			allowance: types.PeriodicFeeAllowance{
 				Basic: types.BasicFeeAllowance{
 					SpendLimit: atom,
 					Expiration: types.ExpiresAtHeight(100),
@@ -160,7 +160,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			remove:      true,
 		},
 		"over period limit": {
-			allow: types.PeriodicFeeAllowance{
+			allowance: types.PeriodicFeeAllowance{
 				Basic: types.BasicFeeAllowance{
 					SpendLimit: atom,
 					Expiration: types.ExpiresAtHeight(100),
@@ -181,7 +181,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 	for name, stc := range cases {
 		tc := stc // to make scopelint happy
 		t.Run(name, func(t *testing.T) {
-			err := tc.allow.ValidateBasic()
+			err := tc.allowance.ValidateBasic()
 			if !tc.valid {
 				require.Error(t, err)
 				return
@@ -190,18 +190,18 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 
 			ctx := app.BaseApp.NewContext(false, tmproto.Header{}).WithBlockHeight(tc.blockHeight)
 			// now try to deduct
-			remove, err := tc.allow.Accept(ctx, tc.fee, []sdk.Msg{})
+			removed, err := tc.allowance.Accept(ctx, tc.fee, []sdk.Msg{})
 			if !tc.accept {
 				require.Error(t, err)
 				return
 			}
 			require.NoError(t, err)
 
-			require.Equal(t, tc.remove, remove)
+			require.Equal(t, tc.remove, removed)
-			if !remove {
+			if !removed {
-				assert.Equal(t, tc.remains, tc.allow.Basic.SpendLimit)
+				assert.Equal(t, tc.remains, tc.allowance.Basic.SpendLimit)
-				assert.Equal(t, tc.remainsPeriod, tc.allow.PeriodCanSpend)
+				assert.Equal(t, tc.remainsPeriod, tc.allowance.PeriodCanSpend)
-				assert.Equal(t, tc.periodReset, tc.allow.PeriodReset)
+				assert.Equal(t, tc.periodReset, tc.allowance.PeriodReset)
 			}
 		})
 	}