feegrant filtered msgs (cosmos#8604)

* WIP: add filtered message * updated `Accept` interface method * fix tests * add cli tests for filtered fee allowance * fix tests * review changes * rename `filteredFeeAllowance` message * review changes * review changes * review changes * review changes * review changes * update errors * remove validation * fix conflicts * add `ctx` to `Accept` method * fix test * add gas consumption * review changes * review changes * revert * review changes * improve error handling * fix test * Merge branch 'master' of github.com:cosmos/cosmos-sdk into atheesh/feegrant-filtered-msgs * review changes * update gas * update type * review changes Co-authored-by: Alessio Treglia <[email protected]> Co-authored-by: SaReN <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
dtkong · Apr 9, 2021 · 7a3a156 · 7a3a156
1 parent bb7b2a6
commit 7a3a156
Show file tree

Hide file tree

Showing 25 changed files with 729 additions and 101 deletions.
diff --git a/docs/core/proto-docs.md b/docs/core/proto-docs.md
@@ -307,6 +307,7 @@
     - [Msg](#cosmos.evidence.v1beta1.Msg)
 
 - [cosmos/feegrant/v1beta1/feegrant.proto](#cosmos/feegrant/v1beta1/feegrant.proto)
+    - [AllowedMsgFeeAllowance](#cosmos.feegrant.v1beta1.AllowedMsgFeeAllowance)
     - [BasicFeeAllowance](#cosmos.feegrant.v1beta1.BasicFeeAllowance)
     - [Duration](#cosmos.feegrant.v1beta1.Duration)
     - [ExpiresAt](#cosmos.feegrant.v1beta1.ExpiresAt)
@@ -4543,6 +4544,22 @@ Msg defines the evidence Msg service.
 
 
 
+<a name="cosmos.feegrant.v1beta1.AllowedMsgFeeAllowance"></a>
+
+### AllowedMsgFeeAllowance
+AllowedMsgFeeAllowance creates allowance only for specified message types.
+
+
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `allowance` | [google.protobuf.Any](#google.protobuf.Any) |  | allowance can be any of basic and filtered fee allowance. |
+| `allowed_messages` | [string](#string) | repeated | allowed_messages are the messages for which the grantee has the access. |
+
+
+
+
+
+
 <a name="cosmos.feegrant.v1beta1.BasicFeeAllowance"></a>
 
 ### BasicFeeAllowance

diff --git a/proto/cosmos/feegrant/v1beta1/feegrant.proto b/proto/cosmos/feegrant/v1beta1/feegrant.proto
@@ -52,6 +52,18 @@ message PeriodicFeeAllowance {
   ExpiresAt period_reset = 5 [(gogoproto.nullable) = false];
 }
 
+// AllowedMsgFeeAllowance creates allowance only for specified message types.
+message AllowedMsgFeeAllowance {
+  option (gogoproto.goproto_getters)         = false;
+  option (cosmos_proto.implements_interface) = "FeeAllowanceI";
+
+  // allowance can be any of basic and filtered fee allowance.
+  google.protobuf.Any allowance = 1 [(cosmos_proto.accepts_interface) = "FeeAllowanceI"];
+
+  // allowed_messages are the messages for which the grantee has the access.
+  repeated string allowed_messages = 2;
+}
+
 // Duration is a span of a clock time or number of blocks.
 // This is designed to be added to an ExpiresAt struct.
 message Duration {

diff --git a/x/auth/ante/expected_keepers.go b/x/auth/ante/expected_keepers.go
@@ -16,5 +16,5 @@ type AccountKeeper interface {
 
 // FeegrantKeeper defines the expected feegrant keeper.
 type FeegrantKeeper interface {
-	UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins) error
+	UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, msgs []sdk.Msg) error
 }
diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go
@@ -94,7 +94,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo
 		if dfd.feegrantKeeper == nil {
 			return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee grants are not enabled")
 		} else if !feeGranter.Equals(feePayer) {
-			err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee)
+			err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, tx.GetMsgs())
 
 			if err != nil {
 				return ctx, sdkerrors.Wrapf(err, "%s not allowed to pay fees from %s", feeGranter, feePayer)

diff --git a/x/feegrant/client/cli/cli_test.go b/x/feegrant/client/cli/cli_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/crypto/hd"
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
+	"github.com/cosmos/cosmos-sdk/testutil"
 	clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
 	"github.com/cosmos/cosmos-sdk/testutil/network"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -40,7 +41,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
 	}
 
 	cfg := network.DefaultConfig()
-	cfg.NumValidators = 2
+	cfg.NumValidators = 3
 
 	s.cfg = cfg
 	s.network = network.New(s.T(), cfg)
@@ -137,7 +138,7 @@ func (s *IntegrationTestSuite) TestCmdGetFeeGrant() {
 				grantee.String(),
 				fmt.Sprintf("--%s=json", tmcli.OutputFlag),
 			},
-			"no fee allowance found",
+			"no allowance",
 			true, nil, nil,
 		},
 		{
@@ -169,9 +170,13 @@ func (s *IntegrationTestSuite) TestCmdGetFeeGrant() {
 				s.Require().NoError(clientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), tc.respType), out.String())
 				s.Require().Equal(tc.respType.Grantee, tc.respType.Grantee)
 				s.Require().Equal(tc.respType.Granter, tc.respType.Granter)
+				grant, err := tc.respType.GetFeeGrant()
+				s.Require().NoError(err)
+				grant1, err1 := tc.resp.GetFeeGrant()
+				s.Require().NoError(err1)
 				s.Require().Equal(
-					tc.respType.GetFeeGrant().(*types.BasicFeeAllowance).SpendLimit,
-					tc.resp.GetFeeGrant().(*types.BasicFeeAllowance).SpendLimit,
+					grant.(*types.BasicFeeAllowance).SpendLimit,
+					grant1.(*types.BasicFeeAllowance).SpendLimit,
 				)
 			}
 		})
@@ -617,6 +622,181 @@ func (s *IntegrationTestSuite) TestTxWithFeeGrant() {
 	s.Require().Equal(uint32(0), resp.Code)
 }
 
+func (s *IntegrationTestSuite) TestFilteredFeeAllowance() {
+	val := s.network.Validators[0]
+
+	granter := val.Address
+	info, _, err := val.ClientCtx.Keyring.NewMnemonic("grantee1", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
+	s.Require().NoError(err)
+	grantee := sdk.AccAddress(info.GetPubKey().Address())
+
+	clientCtx := val.ClientCtx
+
+	commonFlags := []string{
+		fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
+		fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
+		fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
+	}
+	spendLimit := sdk.NewCoin("stake", sdk.NewInt(1000))
+
+	allowMsgs := "/cosmos.gov.v1beta1.Msg/SubmitProposal"
+
+	testCases := []struct {
+		name         string
+		args         []string
+		expectErr    bool
+		respType     proto.Message
+		expectedCode uint32
+	}{
+		{
+			"wrong granter",
+			append(
+				[]string{
+					"wrong granter",
+					"cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl",
+					fmt.Sprintf("--%s=%s", cli.FlagAllowedMsgs, allowMsgs),
+					fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, spendLimit.String()),
+					fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
+				},
+				commonFlags...,
+			),
+			true, &sdk.TxResponse{}, 0,
+		},
+		{
+			"wrong grantee",
+			append(
+				[]string{
+					granter.String(),
+					"wrong grantee",
+					fmt.Sprintf("--%s=%s", cli.FlagAllowedMsgs, allowMsgs),
+					fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, spendLimit.String()),
+					fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
+				},
+				commonFlags...,
+			),
+			true, &sdk.TxResponse{}, 0,
+		},
+		{
+			"valid filter fee grant",
+			append(
+				[]string{
+					granter.String(),
+					grantee.String(),
+					fmt.Sprintf("--%s=%s", cli.FlagAllowedMsgs, allowMsgs),
+					fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, spendLimit.String()),
+					fmt.Sprintf("--%s=%s", flags.FlagFrom, granter),
+				},
+				commonFlags...,
+			),
+			false, &sdk.TxResponse{}, 0,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		s.Run(tc.name, func() {
+			cmd := cli.NewCmdFeeGrant()
+			out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args)
+
+			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())
+			}
+		})
+	}
+
+	args := []string{
+		granter.String(),
+		grantee.String(),
+		fmt.Sprintf("--%s=json", tmcli.OutputFlag),
+	}
+
+	// get filtered fee allowance and check info
+	cmd := cli.GetCmdQueryFeeGrant()
+	out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
+	s.Require().NoError(err)
+
+	resp := &types.FeeAllowanceGrant{}
+
+	s.Require().NoError(clientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), resp), out.String())
+	s.Require().Equal(resp.Grantee, resp.Grantee)
+	s.Require().Equal(resp.Granter, resp.Granter)
+
+	grant, err := resp.GetFeeGrant()
+	s.Require().NoError(err)
+
+	filteredFeeGrant, err := grant.(*types.AllowedMsgFeeAllowance).GetAllowance()
+	s.Require().NoError(err)
+
+	s.Require().Equal(
+		filteredFeeGrant.(*types.BasicFeeAllowance).SpendLimit.String(),
+		spendLimit.String(),
+	)
+
+	// exec filtered fee allowance
+	cases := []struct {
+		name         string
+		malleate     func() (testutil.BufferWriter, error)
+		expectErr    bool
+		respType     proto.Message
+		expectedCode uint32
+	}{
+		{
+			"valid 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,
+		},
+		{
+			"should fail with unauthorized msgs",
+			func() (testutil.BufferWriter, error) {
+				args := append(
+					[]string{
+						grantee.String(),
+						"cosmos14cm33pvnrv2497tyt8sp9yavhmw83nwej3m0e8",
+						fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"),
+						fmt.Sprintf("--%s=%s", flags.FlagFeeAccount, granter),
+					},
+					commonFlags...,
+				)
+				cmd := cli.NewCmdFeeGrant()
+				return clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
+			},
+			false, &sdk.TxResponse{}, 7,
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+
+		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())
+			}
+		})
+	}
+}
+
 func TestIntegrationTestSuite(t *testing.T) {
 	suite.Run(t, new(IntegrationTestSuite))
 }
diff --git a/x/feegrant/client/cli/tx.go b/x/feegrant/client/cli/tx.go
@@ -22,6 +22,7 @@ const (
 	FlagPeriod      = "period"
 	FlagPeriodLimit = "period-limit"
 	FlagSpendLimit  = "spend-limit"
+	FlagAllowedMsgs = "allowed-messages"
 )
 
 // GetTxCmd returns the transaction commands for this module
@@ -55,8 +56,10 @@ func NewCmdFeeGrant() *cobra.Command {
 
 Examples:
 %s tx %s grant cosmos1skjw... cosmos1skjw... --spend-limit 100stake --expiration 36000 or
-%s tx %s grant cosmos1skjw... cosmos1skjw... --spend-limit 100stake --period 3600 --period-limit 10stake --expiration 36000
-				`, version.AppName, types.ModuleName, version.AppName, types.ModuleName,
+%s tx %s grant cosmos1skjw... cosmos1skjw... --spend-limit 100stake --period 3600 --period-limit 10stake --expiration 36000 or
+%s tx %s grant cosmos1skjw... cosmos1skjw... --spend-limit 100stake --expiration 36000 
+	--allowed-messages "/cosmos.gov.v1beta1.Msg/SubmitProposal,/cosmos.gov.v1beta1.Msg/Vote"
+				`, version.AppName, types.ModuleName, version.AppName, types.ModuleName, version.AppName, types.ModuleName,
 			),
 		),
 		Args: cobra.ExactArgs(2),
@@ -143,6 +146,18 @@ Examples:
 				}
 			}
 
+			allowedMsgs, err := cmd.Flags().GetStringSlice(FlagAllowedMsgs)
+			if err != nil {
+				return err
+			}
+
+			if len(allowedMsgs) > 0 {
+				grant, err = types.NewAllowedMsgFeeAllowance(grant, allowedMsgs)
+				if err != nil {
+					return err
+				}
+			}
+
 			msg, err := types.NewMsgGrantFeeAllowance(grant, granter, grantee)
 			if err != nil {
 				return err
@@ -160,10 +175,11 @@ Examples:
 	}
 
 	flags.AddTxFlagsToCmd(cmd)
+	cmd.Flags().StringSlice(FlagAllowedMsgs, []string{}, "Set of allowed messages for fee allowance")
 	cmd.Flags().Int64(FlagExpiration, 0, "The second unit of time duration which the grant is active for the user")
 	cmd.Flags().String(FlagSpendLimit, "", "Spend limit specifies the max limit can be used, if not mentioned there is no limit")
 	cmd.Flags().Int64(FlagPeriod, 0, "period specifies the time duration in which period_spend_limit coins can be spent before that allowance is reset")
-	cmd.Flags().String(FlagPeriodLimit, "", "// period limit specifies the maximum number of coins that can be spent in the period")
+	cmd.Flags().String(FlagPeriodLimit, "", "period limit specifies the maximum number of coins that can be spent in the period")
 
 	return cmd
 }

diff --git a/x/feegrant/client/rest/grpc_query_test.go b/x/feegrant/client/rest/grpc_query_test.go
@@ -92,7 +92,7 @@ func (s *IntegrationTestSuite) TestQueryFeeAllowance() {
 			"fail: no grants",
 			fmt.Sprintf("%s/cosmos/feegrant/v1beta1/fee_allowance/%s/%s", baseURL, val.Address.String(), s.grantee.String()),
 			true,
-			"no fee allowance found",
+			"no allowance",
 			func() {},
 			func(types.QueryFeeAllowanceResponse) {},
 		},

diff --git a/x/feegrant/genesis.go b/x/feegrant/genesis.go
@@ -12,7 +12,11 @@ type GenesisState []types.FeeAllowanceGrant
 // ValidateBasic ensures all grants in the genesis state are valid
 func (g GenesisState) ValidateBasic() error {
 	for _, f := range g {
-		err := f.GetFeeGrant().ValidateBasic()
+		grant, err := f.GetFeeGrant()
+		if err != nil {
+			return err
+		}
+		err = grant.ValidateBasic()
 		if err != nil {
 			return err
 		}
@@ -32,7 +36,12 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, data *types.GenesisState) {
 			panic(err)
 		}
 
-		err = k.GrantFeeAllowance(ctx, granter, grantee, f.GetFeeGrant())
+		grant, err := f.GetFeeGrant()
+		if err != nil {
+			panic(err)
+		}
+
+		err = k.GrantFeeAllowance(ctx, granter, grantee, grant)
 		if err != nil {
 			panic(err)
 		}

diff --git a/x/feegrant/keeper/grpc_query.go b/x/feegrant/keeper/grpc_query.go
@@ -34,9 +34,9 @@ func (q Keeper) FeeAllowance(c context.Context, req *types.QueryFeeAllowanceRequ
 
 	ctx := sdk.UnwrapSDKContext(c)
 
-	feeAllowance := q.GetFeeAllowance(ctx, granterAddr, granteeAddr)
-	if feeAllowance == nil {
-		return nil, status.Errorf(codes.NotFound, "no fee allowance found")
+	feeAllowance, err := q.GetFeeAllowance(ctx, granterAddr, granteeAddr)
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, err.Error())
 	}
 
 	msg, ok := feeAllowance.(proto.Message)