x/authz charge gas for expensive authorizations (cosmos#8995)

* WIP

* Add consume gas

* update authz.go

* add build flag

* Update x/staking/types/authz.go

* Update x/staking/types/authz.go

* add tests docs

* review changes

* fix operations

* Update x/authz/types/msgs.go

Co-authored-by: Amaury <[email protected]>

* review changes

* update gas cost

* review changes

Co-authored-by: SaReN <[email protected]>
Co-authored-by: Amaury <[email protected]>

types/msgservice/msg_service.go

@@ -3,6 +3,7 @@ package msgservice
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/gogo/protobuf/proto"
 	"google.golang.org/grpc"
@@ -42,3 +43,9 @@ func RegisterMsgServiceDesc(registry codectypes.InterfaceRegistry, sd *grpc.Serv
 func noopInterceptor(_ context.Context, _ interface{}, _ *grpc.UnaryServerInfo, _ grpc.UnaryHandler) (interface{}, error) {
 	return nil, nil
 }
+
+// IsServiceMsg checks if a type URL corresponds to a service method name,
+// i.e. /cosmos.bank.Msg/Send vs /cosmos.bank.MsgSend
+func IsServiceMsg(typeURL string) bool {
+	return strings.Count(typeURL, "/") >= 2
+}

types/tx/types.go

@@ -2,12 +2,12 @@ package tx
 
 import (
 	"fmt"
-	"strings"
 
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/types/msgservice"
 )
 
 // MaxGasWanted defines the max gas allowed.
@@ -27,7 +27,7 @@ func (t *Tx) GetMsgs() []sdk.Msg {
 	res := make([]sdk.Msg, len(anys))
 	for i, any := range anys {
 		var msg sdk.Msg
-		if isServiceMsg(any.TypeUrl) {
+		if msgservice.IsServiceMsg(any.TypeUrl) {
 			req := any.GetCachedValue()
 			if req == nil {
 				panic("Any cached value is nil. Transaction messages must be correctly packed Any values.")
@@ -183,7 +183,7 @@ func (m *TxBody) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error {
 	for _, any := range m.Messages {
 		// If the any's typeUrl contains 2 slashes, then we unpack the any into
 		// a ServiceMsg struct as per ADR-031.
-		if isServiceMsg(any.TypeUrl) {
+		if msgservice.IsServiceMsg(any.TypeUrl) {
 			var req sdk.MsgRequest
 			err := unpacker.UnpackAny(any, &req)
 			if err != nil {
@@ -222,9 +222,3 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
 	registry.RegisterInterface("cosmos.tx.v1beta1.Tx", (*sdk.Tx)(nil))
 	registry.RegisterImplementations((*sdk.Tx)(nil), &Tx{})
 }
-
-// isServiceMsg checks if a type URL corresponds to a service method name,
-// i.e. /cosmos.bank.Msg/Send vs /cosmos.bank.MsgSend
-func isServiceMsg(typeURL string) bool {
-	return strings.Count(typeURL, "/") >= 2
-}

x/authz/client/cli/tx_test.go

@@ -156,8 +156,8 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 			},
-			&sdk.TxResponse{}, 29,
-			false,
+			nil, 0,
+			true,
 		},
 		{
 			"failed with error both validators not allowed",

x/authz/exported/authorizations.go

@@ -3,8 +3,6 @@ package exported
 import (
 	"github.com/gogo/protobuf/proto"
 
-	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -17,5 +15,9 @@ type Authorization interface {
 
 	// Accept determines whether this grant permits the provided sdk.ServiceMsg to be performed, and if
 	// so provides an upgraded authorization instance.
-	Accept(msg sdk.ServiceMsg, block tmproto.Header) (updated Authorization, delete bool, err error)
+	Accept(ctx sdk.Context, msg sdk.ServiceMsg) (updated Authorization, delete bool, err error)
+
+	// ValidateBasic does a simple validation check that
+	// doesn't require access to any other information.
+	ValidateBasic() error
 }

x/authz/keeper/keeper.go

@@ -87,7 +87,7 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, service
 			if authorization == nil {
 				return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "authorization not found")
 			}
-			updated, del, err := authorization.Accept(serviceMsg, ctx.BlockHeader())
+			updated, del, err := authorization.Accept(ctx, serviceMsg)
 			if err != nil {
 				return nil, err
 			}

x/authz/simulation/operations.go

@@ -96,8 +96,12 @@ func SimulateMsgGrantAuthorization(ak types.AccountKeeper, bk types.BankKeeper,
 		}
 
 		blockTime := ctx.BlockTime()
+		spendLimit := spendableCoins.Sub(fees)
+		if spendLimit == nil {
+			return simtypes.NoOpMsg(types.ModuleName, TypeMsgGrantAuthorization, "spend limit is nil"), nil, nil
+		}
 		msg, err := types.NewMsgGrantAuthorization(granter.Address, grantee.Address,
-			banktype.NewSendAuthorization(spendableCoins.Sub(fees)), blockTime.AddDate(1, 0, 0))
+			banktype.NewSendAuthorization(spendLimit), blockTime.AddDate(1, 0, 0))
 
 		if err != nil {
 			return simtypes.NoOpMsg(types.ModuleName, TypeMsgGrantAuthorization, err.Error()), nil, err
@@ -249,7 +253,7 @@ func SimulateMsgExecuteAuthorized(ak types.AccountKeeper, bk types.BankKeeper, k
 
 		msg := types.NewMsgExecAuthorized(grantee.Address, []sdk.ServiceMsg{execMsg})
 		sendGrant := targetGrant.Authorization.GetCachedValue().(*banktype.SendAuthorization)
-		_, _, err = sendGrant.Accept(execMsg, ctx.BlockHeader())
+		_, _, err = sendGrant.Accept(ctx, execMsg)
 		if err != nil {
 			return simtypes.NoOpMsg(types.ModuleName, TypeMsgExecDelegated, err.Error()), nil, nil
 		}

x/authz/types/generic_authorization.go

@@ -1,9 +1,9 @@
 package types
 
 import (
-	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/types/msgservice"
 	"github.com/cosmos/cosmos-sdk/x/authz/exported"
 )
 
@@ -19,11 +19,19 @@ func NewGenericAuthorization(methodName string) *GenericAuthorization {
 }
 
 // MethodName implements Authorization.MethodName.
-func (cap GenericAuthorization) MethodName() string {
-	return cap.MessageName
+func (authorization GenericAuthorization) MethodName() string {
+	return authorization.MessageName
 }
 
 // Accept implements Authorization.Accept.
-func (cap GenericAuthorization) Accept(msg sdk.ServiceMsg, block tmproto.Header) (updated exported.Authorization, delete bool, err error) {
-	return &cap, false, nil
+func (authorization GenericAuthorization) Accept(ctx sdk.Context, msg sdk.ServiceMsg) (updated exported.Authorization, delete bool, err error) {
+	return &authorization, false, nil
+}
+
+// ValidateBasic implements Authorization.ValidateBasic.
+func (authorization GenericAuthorization) ValidateBasic() error {
+	if !msgservice.IsServiceMsg(authorization.MessageName) {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, " %s is not a valid service msg", authorization.MessageName)
+	}
+	return nil
 }

x/authz/types/generic_authorization_test.go

@@ -0,0 +1,20 @@
+package types_test
+
+import (
+	"testing"
+
+	"github.com/cosmos/cosmos-sdk/x/authz/types"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGenericAuthorization(t *testing.T) {
+	t.Log("verify ValidateBasic returns error for non-service msg")
+	authorization := types.NewGenericAuthorization(banktypes.TypeMsgSend)
+	require.Error(t, authorization.ValidateBasic())
+
+	t.Log("verify ValidateBasic returns nil for service msg")
+	authorization = types.NewGenericAuthorization(banktypes.SendAuthorization{}.MethodName())
+	require.NoError(t, authorization.ValidateBasic())
+	require.Equal(t, banktypes.SendAuthorization{}.MethodName(), authorization.MessageName)
+}

x/authz/types/msgs.go

@@ -63,7 +63,11 @@ func (msg MsgGrantAuthorizationRequest) ValidateBasic() error {
 		return sdkerrors.Wrap(ErrInvalidExpirationTime, "Time can't be in the past")
 	}
 
-	return nil
+	authorization, ok := msg.Authorization.GetCachedValue().(exported.Authorization)
+	if !ok {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", (exported.Authorization)(nil), msg.Authorization.GetCachedValue())
+	}
+	return authorization.ValidateBasic()
 }
 
 // GetGrantAuthorization returns the cache value from the MsgGrantAuthorization.Authorization if present.

x/bank/types/send_authorization.go

@@ -3,8 +3,6 @@ package types
 import (
 	"reflect"
 
-	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	authz "github.com/cosmos/cosmos-sdk/x/authz/exported"
@@ -27,7 +25,7 @@ func (authorization SendAuthorization) MethodName() string {
 }
 
 // Accept implements Authorization.Accept.
-func (authorization SendAuthorization) Accept(msg sdk.ServiceMsg, block tmproto.Header) (updated authz.Authorization, delete bool, err error) {
+func (authorization SendAuthorization) Accept(ctx sdk.Context, msg sdk.ServiceMsg) (updated authz.Authorization, delete bool, err error) {
 	if reflect.TypeOf(msg.Request) == reflect.TypeOf(&MsgSend{}) {
 		msg, ok := msg.Request.(*MsgSend)
 		if ok {
@@ -44,3 +42,14 @@ func (authorization SendAuthorization) Accept(msg sdk.ServiceMsg, block tmproto.
 	}
 	return nil, false, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "type mismatch")
 }
+
+// ValidateBasic implements Authorization.ValidateBasic.
+func (authorization SendAuthorization) ValidateBasic() error {
+	if authorization.SpendLimit == nil {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "spend limit cannot be nil")
+	}
+	if !authorization.SpendLimit.IsAllPositive() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "spend limit cannot be negitive")
+	}
+	return nil
+}

x/bank/types/send_authorization_test.go

@@ -0,0 +1,64 @@
+package types_test
+
+import (
+	"testing"
+
+	"github.com/cosmos/cosmos-sdk/simapp"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/bank/types"
+	"github.com/stretchr/testify/require"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
+)
+
+var (
+	coins1000 = sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000)))
+	coins500  = sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(500)))
+	fromAddr  = sdk.AccAddress("_____from _____")
+	toAddr    = sdk.AccAddress("_______to________")
+)
+
+func TestSendAuthorization(t *testing.T) {
+	app := simapp.Setup(false)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+	authorization := types.NewSendAuthorization(coins1000)
+
+	t.Log("verify authorization returns valid method name")
+	require.Equal(t, authorization.MethodName(), "/cosmos.bank.v1beta1.Msg/Send")
+	require.NoError(t, authorization.ValidateBasic())
+	send := types.NewMsgSend(fromAddr, toAddr, coins1000)
+	srvMsg := sdk.ServiceMsg{
+		MethodName: "/cosmos.bank.v1beta1.Msg/Send",
+		Request:    send,
+	}
+	require.NoError(t, authorization.ValidateBasic())
+
+	t.Log("verify updated authorization returns nil")
+	updated, del, err := authorization.Accept(ctx, srvMsg)
+	require.NoError(t, err)
+	require.True(t, del)
+	require.Nil(t, updated)
+
+	authorization = types.NewSendAuthorization(coins1000)
+	require.Equal(t, authorization.MethodName(), "/cosmos.bank.v1beta1.Msg/Send")
+	require.NoError(t, authorization.ValidateBasic())
+	send = types.NewMsgSend(fromAddr, toAddr, coins500)
+	srvMsg = sdk.ServiceMsg{
+		MethodName: "/cosmos.bank.v1beta1.Msg/Send",
+		Request:    send,
+	}
+	require.NoError(t, authorization.ValidateBasic())
+	updated, del, err = authorization.Accept(ctx, srvMsg)
+
+	t.Log("verify updated authorization returns remaining spent limit")
+	require.NoError(t, err)
+	require.False(t, del)
+	require.NotNil(t, updated)
+	sendAuth := types.NewSendAuthorization(coins500)
+	require.Equal(t, sendAuth.String(), updated.String())
+
+	t.Log("expect updated authorization nil after spending remaining amount")
+	updated, del, err = updated.Accept(ctx, srvMsg)
+	require.NoError(t, err)
+	require.True(t, del)
+	require.Nil(t, updated)
+}

x/staking/types/authz.go

@@ -1,18 +1,21 @@
 package types
 
 import (
-	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	authz "github.com/cosmos/cosmos-sdk/x/authz/exported"
 )
 
+// TODO: Revisit this once we have propoer gas fee framework.
+// Tracking issues https://github.com/cosmos/cosmos-sdk/issues/9054, https://github.com/cosmos/cosmos-sdk/discussions/9072
+const gasCostPerIteration = uint64(10)
+
 var (
-	_                   authz.Authorization = &StakeAuthorization{}
-	TypeDelegate                            = "/cosmos.staking.v1beta1.Msg/Delegate"
-	TypeUndelegate                          = "/cosmos.staking.v1beta1.Msg/Undelegate"
-	TypeBeginRedelegate                     = "/cosmos.staking.v1beta1.Msg/BeginRedelegate"
+	_ authz.Authorization = &StakeAuthorization{}
+
+	TypeDelegate        = "/cosmos.staking.v1beta1.Msg/Delegate"
+	TypeUndelegate      = "/cosmos.staking.v1beta1.Msg/Undelegate"
+	TypeBeginRedelegate = "/cosmos.staking.v1beta1.Msg/BeginRedelegate"
 )
 
 // NewStakeAuthorization creates a new StakeAuthorization object.
@@ -46,8 +49,19 @@ func (authorization StakeAuthorization) MethodName() string {
 	return authzType
 }
 
+func (authorization StakeAuthorization) ValidateBasic() error {
+	if authorization.MaxTokens != nil && authorization.MaxTokens.IsNegative() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "negative coin amount: %v", authorization.MaxTokens)
+	}
+	if authorization.AuthorizationType == AuthorizationType_AUTHORIZATION_TYPE_UNSPECIFIED {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "unknown authorization type")
+	}
+
+	return nil
+}
+
 // Accept implements Authorization.Accept.
-func (authorization StakeAuthorization) Accept(msg sdk.ServiceMsg, block tmproto.Header) (updated authz.Authorization, delete bool, err error) {
+func (authorization StakeAuthorization) Accept(ctx sdk.Context, msg sdk.ServiceMsg) (updated authz.Authorization, delete bool, err error) {
 	var validatorAddress string
 	var amount sdk.Coin
 
@@ -68,13 +82,16 @@ func (authorization StakeAuthorization) Accept(msg sdk.ServiceMsg, block tmproto
 	isValidatorExists := false
 	allowedList := authorization.GetAllowList().GetAddress()
 	for _, validator := range allowedList {
+		ctx.GasMeter().ConsumeGas(gasCostPerIteration, "stake authorization")
 		if validator == validatorAddress {
 			isValidatorExists = true
 			break
 		}
 	}
+
 	denyList := authorization.GetDenyList().GetAddress()
 	for _, validator := range denyList {
+		ctx.GasMeter().ConsumeGas(gasCostPerIteration, "stake authorization")
 		if validator == validatorAddress {
 			return nil, false, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, " cannot delegate/undelegate to %s validator", validator)
 		}

x/staking/types/authz_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/cosmos/cosmos-sdk/simapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 )
@@ -21,13 +22,21 @@ var (
 )
 
 func TestAuthzAuthorizations(t *testing.T) {
+	app := simapp.Setup(false)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+
+	// verify ValidateBasic returns error for the AUTHORIZATION_TYPE_UNSPECIFIED authorization type
+	delAuth, err := stakingtypes.NewStakeAuthorization([]sdk.ValAddress{val1, val2}, []sdk.ValAddress{}, stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_UNSPECIFIED, &coin100)
+	require.NoError(t, err)
+	require.Error(t, delAuth.ValidateBasic())
 
 	// verify MethodName
-	delAuth, _ := stakingtypes.NewStakeAuthorization([]sdk.ValAddress{val1, val2}, []sdk.ValAddress{}, stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_DELEGATE, &coin100)
+	delAuth, err = stakingtypes.NewStakeAuthorization([]sdk.ValAddress{val1, val2}, []sdk.ValAddress{}, stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_DELEGATE, &coin100)
+	require.NoError(t, err)
 	require.Equal(t, delAuth.MethodName(), stakingtypes.TypeDelegate)
 
 	// error both allow & deny list
-	_, err := stakingtypes.NewStakeAuthorization([]sdk.ValAddress{val1, val2}, []sdk.ValAddress{val1}, stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_DELEGATE, &coin100)
+	_, err = stakingtypes.NewStakeAuthorization([]sdk.ValAddress{val1, val2}, []sdk.ValAddress{val1}, stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_DELEGATE, &coin100)
 	require.Error(t, err)
 
 	// verify MethodName
@@ -243,7 +252,7 @@ func TestAuthzAuthorizations(t *testing.T) {
 		t.Run(tc.msg, func(t *testing.T) {
 			delAuth, err := stakingtypes.NewStakeAuthorization(tc.allowed, tc.denied, tc.msgType, tc.limit)
 			require.NoError(t, err)
-			updated, del, err := delAuth.Accept(tc.srvMsg, tmproto.Header{})
+			updated, del, err := delAuth.Accept(ctx, tc.srvMsg)
 			if tc.expectErr {
 				require.Error(t, err)
 				require.Equal(t, tc.isDelete, del)