x/feegrant audit: clean up / add test coverage to types package

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()
-
-			if tc.expectErr {
-				s.Require().Error(err)
-			} else {
-				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())
-			}
+			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,94 +22,84 @@ 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{},
-			valid:  true,
+			allowance:  &types.BasicFeeAllowance{},
 			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,
 			remove:      false,
 			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()
-			if !tc.valid {
-				require.Error(t, err)
-				return
-			}
+			err := tc.allowance.ValidateBasic()
 			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)
-			if !remove {
-				assert.Equal(t, tc.allow.SpendLimit, tc.remains)
+			require.Equal(t, tc.remove, removed)
+			if !removed {
+				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
-		valid   bool
+		expires types.ExpiresAt
 		zero    bool
 		before  types.ExpiresAt
 		after   types.ExpiresAt
 	}{
 		"basic": {
-			example: types.ExpiresAtHeight(100),
-			valid:   true,
+			expires: types.ExpiresAtHeight(100),
 			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),
-			valid:   true,
+			expires: types.ExpiresAtHeight(1000),
 			before:  types.ExpiresAtHeight(999),
 			after:   types.ExpiresAtHeight(1000),
 		},
 		"match time": {
-			example: types.ExpiresAtTime(now),
-			valid:   true,
+			expires: types.ExpiresAtTime(now),
 			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()
-			assert.Equal(t, tc.zero, tc.example.Undefined())
-			if !tc.valid {
-				require.Error(t, err)
-				return
-			}
+			err := tc.expires.ValidateBasic()
+			assert.Equal(t, tc.zero, tc.expires.Undefined())
 			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.