From 9d5fba3790605696379acda7593344d7d5710a1c Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 25 Apr 2024 17:11:57 +0200 Subject: [PATCH] refactor(x/**): rewrite ante handlers as tx validators (#19949) --- simapp/ante.go | 2 +- x/accounts/testing/counter/counter.go | 2 +- x/auth/ante/ante.go | 2 +- x/auth/ante/basic.go | 98 ++++++++++++++++++++------- x/auth/ante/basic_test.go | 26 ++++++- x/auth/ante/sigverify.go | 20 ++++-- x/auth/module.go | 25 ++++--- x/auth/tx/config/depinject.go | 2 +- x/feegrant/module/module.go | 11 --- 9 files changed, 128 insertions(+), 60 deletions(-) diff --git a/simapp/ante.go b/simapp/ante.go index 939d14362477..6569277bc7b7 100644 --- a/simapp/ante.go +++ b/simapp/ante.go @@ -38,7 +38,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { circuitante.NewCircuitBreakerDecorator(options.CircuitKeeper), ante.NewExtensionOptionsDecorator(options.ExtensionOptionChecker), ante.NewValidateBasicDecorator(options.AccountKeeper.GetEnvironment()), - ante.NewTxTimeoutHeightDecorator(), + ante.NewTxTimeoutHeightDecorator(options.AccountKeeper.GetEnvironment()), ante.NewUnorderedTxDecorator(unorderedtx.DefaultMaxUnOrderedTTL, options.TxManager, options.AccountKeeper.GetEnvironment()), ante.NewValidateMemoDecorator(options.AccountKeeper), ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), diff --git a/x/accounts/testing/counter/counter.go b/x/accounts/testing/counter/counter.go index 9e454aa928a8..da70f644b3dc 100644 --- a/x/accounts/testing/counter/counter.go +++ b/x/accounts/testing/counter/counter.go @@ -114,7 +114,7 @@ func (a Account) TestDependencies(ctx context.Context, _ *counterv1.MsgTestDepen // test gas meter gm := a.gs.GasMeter(ctx) - gasBefore := gm.Limit() - gm.Remaining() + gasBefore := gm.Limit() - gm.Consumed() gm.Consume(10, "test") gasAfter := gm.Limit() - gm.Consumed() diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index 7f823eef343d..ea58980d7e6c 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -43,7 +43,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first NewExtensionOptionsDecorator(options.ExtensionOptionChecker), NewValidateBasicDecorator(options.AccountKeeper.GetEnvironment()), - NewTxTimeoutHeightDecorator(), + NewTxTimeoutHeightDecorator(options.AccountKeeper.GetEnvironment()), NewValidateMemoDecorator(options.AccountKeeper), NewConsumeGasForTxSizeDecorator(options.AccountKeeper), NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker), diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index de5d8b7f5227..6844828830fb 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -1,6 +1,8 @@ package ante import ( + "context" + "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/transaction" errorsmod "cosmossdk.io/errors" @@ -30,20 +32,28 @@ func NewValidateBasicDecorator(env appmodule.Environment) ValidateBasicDecorator } } -func (vbd ValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { +func (vbd ValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + if err := vbd.ValidateTx(ctx, tx); err != nil { + return ctx, err + } + + return next(ctx, tx, false) +} + +func (vbd ValidateBasicDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { // no need to validate basic on recheck tx, call next antehandler txService := vbd.env.TransactionService if txService.ExecMode(ctx) == transaction.ExecModeReCheck { - return next(ctx, tx, false) + return nil } if validateBasic, ok := tx.(sdk.HasValidateBasic); ok { if err := validateBasic.ValidateBasic(); err != nil { - return ctx, err + return err } } - return next(ctx, tx, false) + return nil } // ValidateMemoDecorator will validate memo given the parameters passed in @@ -59,24 +69,32 @@ func NewValidateMemoDecorator(ak AccountKeeper) ValidateMemoDecorator { } } -func (vmd ValidateMemoDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { - memoTx, ok := tx.(sdk.TxWithMemo) +func (vmd ValidateMemoDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + if err := vmd.ValidateTx(ctx, tx); err != nil { + return ctx, err + } + + return next(ctx, tx, false) +} + +func (vmd ValidateMemoDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { + memoTx, ok := tx.(sdk.TxWithMemo) // TODO: what do we do with this. if !ok { - return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") + return errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") } memoLength := len(memoTx.GetMemo()) if memoLength > 0 { params := vmd.ak.GetParams(ctx) if uint64(memoLength) > params.MaxMemoCharacters { - return ctx, errorsmod.Wrapf(sdkerrors.ErrMemoTooLarge, + return errorsmod.Wrapf(sdkerrors.ErrMemoTooLarge, "maximum number of characters is %d but received %d characters", params.MaxMemoCharacters, memoLength, ) } } - return next(ctx, tx, false) + return nil } // ConsumeTxSizeGasDecorator will take in parameters and consume gas proportional @@ -98,14 +116,25 @@ func NewConsumeGasForTxSizeDecorator(ak AccountKeeper) ConsumeTxSizeGasDecorator } } -func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { - sigTx, ok := tx.(authsigning.SigVerifiableTx) +func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + if err := cgts.ValidateTx(ctx, tx); err != nil { + return ctx, err + } + + return next(ctx, tx, false) +} + +func (cgts ConsumeTxSizeGasDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { + sigTx, ok := tx.(authsigning.SigVerifiableTx) // TODO: what do we do with this. if !ok { - return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid tx type") + return errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid tx type") } params := cgts.ak.GetParams(ctx) - ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*storetypes.Gas(len(ctx.TxBytes())), "txSize") + gasService := cgts.ak.GetEnvironment().GasService + if err := gasService.GasMeter(ctx).Consume(params.TxSizeCostPerByte*storetypes.Gas(len(tx.Bytes())), "txSize"); err != nil { + return err + } // simulate gas cost for signatures in simulate mode txService := cgts.ak.GetEnvironment().TransactionService @@ -113,7 +142,7 @@ func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ b // in simulate mode, each element should be a nil signature sigs, err := sigTx.GetSignaturesV2() if err != nil { - return ctx, err + return err } n := len(sigs) @@ -147,11 +176,13 @@ func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ b cost *= params.TxSigLimit } - ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*cost, "txSize") + if err := gasService.GasMeter(ctx).Consume(params.TxSizeCostPerByte*cost, "txSize"); err != nil { + return err + } } } - return next(ctx, tx, false) + return nil } // isIncompleteSignature tests whether SignatureData is fully filled in for simulation purposes @@ -180,7 +211,9 @@ func isIncompleteSignature(data signing.SignatureData) bool { type ( // TxTimeoutHeightDecorator defines an AnteHandler decorator that checks for a // tx height timeout. - TxTimeoutHeightDecorator struct{} + TxTimeoutHeightDecorator struct { + env appmodule.Environment + } // TxWithTimeoutHeight defines the interface a tx must implement in order for // TxHeightTimeoutDecorator to process the tx. @@ -193,26 +226,39 @@ type ( // TxTimeoutHeightDecorator defines an AnteHandler decorator that checks for a // tx height timeout. -func NewTxTimeoutHeightDecorator() TxTimeoutHeightDecorator { - return TxTimeoutHeightDecorator{} +func NewTxTimeoutHeightDecorator(env appmodule.Environment) TxTimeoutHeightDecorator { + return TxTimeoutHeightDecorator{ + env: env, + } } -// AnteHandle implements an AnteHandler decorator for the TxHeightTimeoutDecorator +// AnteHandle implements an AnteHandler decorator for the TxHeightTimeoutDecorator. +func (txh TxTimeoutHeightDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + if err := txh.ValidateTx(ctx, tx); err != nil { + return ctx, err + } + + return next(ctx, tx, false) +} + +// ValidateTx implements an TxValidator decorator for the TxHeightTimeoutDecorator // type where the current block height is checked against the tx's height timeout. // If a height timeout is provided (non-zero) and is less than the current block // height, then an error is returned. -func (txh TxTimeoutHeightDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { +func (txh TxTimeoutHeightDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { timeoutTx, ok := tx.(TxWithTimeoutHeight) if !ok { - return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "expected tx to implement TxWithTimeoutHeight") + return errorsmod.Wrap(sdkerrors.ErrTxDecode, "expected tx to implement TxWithTimeoutHeight") } timeoutHeight := timeoutTx.GetTimeoutHeight() - if timeoutHeight > 0 && uint64(ctx.BlockHeight()) > timeoutHeight { - return ctx, errorsmod.Wrapf( - sdkerrors.ErrTxTimeoutHeight, "block height: %d, timeout height: %d", ctx.BlockHeight(), timeoutHeight, + headerInfo := txh.env.HeaderService.HeaderInfo(ctx) + + if timeoutHeight > 0 && uint64(headerInfo.Height) > timeoutHeight { + return errorsmod.Wrapf( + sdkerrors.ErrTxTimeoutHeight, "block height: %d, timeout height: %d", headerInfo.Height, timeoutHeight, ) } - return next(ctx, tx, false) + return nil } diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go index 5186f9b79fc1..836fd63e1fb6 100644 --- a/x/auth/ante/basic_test.go +++ b/x/auth/ante/basic_test.go @@ -1,11 +1,14 @@ package ante_test import ( + "context" "strings" "testing" "github.com/stretchr/testify/require" + "cosmossdk.io/core/appmodule/v2" + "cosmossdk.io/core/header" storetypes "cosmossdk.io/store/types" "cosmossdk.io/x/auth/ante" @@ -95,6 +98,8 @@ func TestValidateMemo(t *testing.T) { } func TestConsumeGasForTxSize(t *testing.T) { + t.Skip() // TODO(@julienrbrt) Fix after https://github.com/cosmos/cosmos-sdk/pull/20072 + suite := SetupTestSuite(t, true) // keys and addresses @@ -182,7 +187,8 @@ func TestConsumeGasForTxSize(t *testing.T) { func TestTxHeightTimeoutDecorator(t *testing.T) { suite := SetupTestSuite(t, true) - antehandler := sdk.ChainAnteDecorators(ante.NewTxTimeoutHeightDecorator()) + mockHeaderService := &mockHeaderService{} + antehandler := sdk.ChainAnteDecorators(ante.NewTxTimeoutHeightDecorator(appmodule.Environment{HeaderService: mockHeaderService})) // keys and addresses priv1, _, addr1 := testdata.KeyTestPubAddr() @@ -221,9 +227,23 @@ func TestTxHeightTimeoutDecorator(t *testing.T) { tx, err := suite.CreateTestTx(suite.ctx, privs, accNums, accSeqs, suite.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT) require.NoError(t, err) - ctx := suite.ctx.WithBlockHeight(tc.height) - _, err = antehandler(ctx, tx, true) + mockHeaderService.WithBlockHeight(tc.height) + _, err = antehandler(suite.ctx, tx, true) require.ErrorIs(t, err, tc.expectedErr) }) } } + +type mockHeaderService struct { + header.Service + + exp header.Info +} + +func (m *mockHeaderService) HeaderInfo(_ context.Context) header.Info { + return m.exp +} + +func (m *mockHeaderService) WithBlockHeight(height int64) { + m.exp.Height = height +} diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index b09bcff5c2c7..d4105cedcd5c 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -460,27 +460,35 @@ func NewValidateSigCountDecorator(ak AccountKeeper) ValidateSigCountDecorator { } } -func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { - sigTx, ok := tx.(authsigning.SigVerifiableTx) +func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + if err := vscd.ValidateTx(ctx, tx); err != nil { + return ctx, err + } + + return next(ctx, tx, false) +} + +func (vscd ValidateSigCountDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { + sigTx, ok := tx.(authsigning.SigVerifiableTx) // TODO: what do we do with this. if !ok { - return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a sigTx") + return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a sigTx") } params := vscd.ak.GetParams(ctx) pubKeys, err := sigTx.GetPubKeys() if err != nil { - return ctx, err + return err } sigCount := 0 for _, pk := range pubKeys { sigCount += CountSubKeys(pk) if uint64(sigCount) > params.TxSigLimit { - return ctx, errorsmod.Wrapf(sdkerrors.ErrTooManySignatures, "signatures: %d, limit: %d", sigCount, params.TxSigLimit) + return errorsmod.Wrapf(sdkerrors.ErrTooManySignatures, "signatures: %d, limit: %d", sigCount, params.TxSigLimit) } } - return next(ctx, tx, false) + return nil } // DefaultSigVerificationGasConsumer is the default implementation of SignatureVerificationGasConsumer. It consumes gas diff --git a/x/auth/module.go b/x/auth/module.go index 19f7cb7aa95c..cb2ced48bc53 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -150,25 +150,30 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) // TxValidator implements appmodulev2.HasTxValidator. // It replaces auth ante handlers for server/v2 func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { - sdkCtx := sdk.UnwrapSDKContext(ctx) - // supports legacy ante handler - // eventually do the reverse, write ante handler as TxValidator - anteDecorators := []sdk.AnteDecorator{ - ante.NewSetUpContextDecorator(), + validators := []appmodulev2.TxValidator[sdk.Tx]{ ante.NewValidateBasicDecorator(am.accountKeeper.GetEnvironment()), - ante.NewTxTimeoutHeightDecorator(), + ante.NewTxTimeoutHeightDecorator(am.accountKeeper.GetEnvironment()), ante.NewValidateMemoDecorator(am.accountKeeper), ante.NewConsumeGasForTxSizeDecorator(am.accountKeeper), ante.NewValidateSigCountDecorator(am.accountKeeper), } - anteHandler := sdk.ChainAnteDecorators(anteDecorators...) - _, err := anteHandler(sdkCtx, nil /** do not import runtime **/, sdkCtx.ExecMode() == sdk.ExecModeSimulate) - return err + sdkTx, ok := tx.(sdk.Tx) + if !ok { + return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) + } + + for _, validator := range validators { + if err := validator.ValidateTx(ctx, sdkTx); err != nil { + return err + } + } + + return nil } -// ConsensusVersion implements HasConsensusVersion +// ConsensusVersion implements appmodule.HasConsensusVersion func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion } // AppModuleSimulation functions diff --git a/x/auth/tx/config/depinject.go b/x/auth/tx/config/depinject.go index 1fc800dad3a9..c12b6ad5f104 100644 --- a/x/auth/tx/config/depinject.go +++ b/x/auth/tx/config/depinject.go @@ -59,7 +59,7 @@ type ModuleOutputs struct { TxConfig client.TxConfig TxConfigOptions tx.ConfigOptions - BaseAppOption runtime.BaseAppOption // TODO find alternative to this + BaseAppOption runtime.BaseAppOption // This is only useful for chains using baseapp. Server/v2 chains use TxValidator. } func ProvideProtoRegistry() txsigning.ProtoFileResolver { diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go index 6096023daf2f..b02978dbd906 100644 --- a/x/feegrant/module/module.go +++ b/x/feegrant/module/module.go @@ -10,9 +10,7 @@ import ( "google.golang.org/grpc" "cosmossdk.io/core/appmodule" - appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/registry" - "cosmossdk.io/core/transaction" "cosmossdk.io/errors" "cosmossdk.io/x/feegrant" "cosmossdk.io/x/feegrant/client/cli" @@ -36,8 +34,6 @@ var ( _ appmodule.HasMigrations = AppModule{} _ appmodule.HasGenesis = AppModule{} _ appmodule.HasRegisterInterfaces = AppModule{} - - _ appmodulev2.HasTxValidator[transaction.Tx] = AppModule{} ) // AppModule implements an application module for the feegrant module. @@ -156,10 +152,3 @@ func (AppModule) ConsensusVersion() uint64 { return 2 } func (am AppModule) EndBlock(ctx context.Context) error { return EndBlocker(ctx, am.keeper) } - -// TxValidator implements appmodulev2.HasTxValidator. -// It replaces auth ante handlers for server/v2 -func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { - // TODO in follow-up - return nil -}