From 78bfe02f81067863a6038524fd96fd944fdae418 Mon Sep 17 00:00:00 2001 From: yihuang Date: Mon, 15 Jul 2024 11:15:29 +0800 Subject: [PATCH] Problem: signature verification result not cache between incarnations of same tx (#565) * Problem: signature verification result not cache between incarnations of same tx Closes: #564 Solution: - introduce incarnation cache that's shared between incarnations of the same tx * Update CHANGELOG.md Signed-off-by: yihuang * Update types/context.go Signed-off-by: yihuang * Update types/context.go Signed-off-by: yihuang * fix nil convert --------- Signed-off-by: yihuang --- baseapp/abci.go | 4 ++-- baseapp/baseapp.go | 11 ++++++----- baseapp/txexecutor.go | 2 +- types/context.go | 26 ++++++++++++++++++++++++++ x/auth/ante/sigverify.go | 37 +++++++++++++++++++++++++++---------- 5 files changed, 62 insertions(+), 18 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 2eeef9da2993..0cdfef2ff2c7 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -845,8 +845,8 @@ func (app *BaseApp) internalFinalizeBlock(ctx context.Context, req *abci.Request func (app *BaseApp) executeTxs(ctx context.Context, txs [][]byte) ([]*abci.ExecTxResult, error) { if app.txExecutor != nil { - return app.txExecutor(ctx, len(txs), app.finalizeBlockState.ms, func(i int, ms storetypes.MultiStore) *abci.ExecTxResult { - return app.deliverTxWithMultiStore(txs[i], i, ms) + return app.txExecutor(ctx, len(txs), app.finalizeBlockState.ms, func(i int, ms storetypes.MultiStore, incarnationCache map[string]any) *abci.ExecTxResult { + return app.deliverTxWithMultiStore(txs[i], i, ms, incarnationCache) }) } diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 13afc822c536..ed84bb5c6684 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -771,10 +771,10 @@ func (app *BaseApp) beginBlock(_ *abci.RequestFinalizeBlock) (sdk.BeginBlock, er } func (app *BaseApp) deliverTx(tx []byte, txIndex int) *abci.ExecTxResult { - return app.deliverTxWithMultiStore(tx, txIndex, nil) + return app.deliverTxWithMultiStore(tx, txIndex, nil, nil) } -func (app *BaseApp) deliverTxWithMultiStore(tx []byte, txIndex int, txMultiStore storetypes.MultiStore) *abci.ExecTxResult { +func (app *BaseApp) deliverTxWithMultiStore(tx []byte, txIndex int, txMultiStore storetypes.MultiStore, incarnationCache map[string]any) *abci.ExecTxResult { gInfo := sdk.GasInfo{} resultStr := "successful" @@ -787,7 +787,7 @@ func (app *BaseApp) deliverTxWithMultiStore(tx []byte, txIndex int, txMultiStore telemetry.SetGauge(float32(gInfo.GasWanted), "tx", "gas", "wanted") }() - gInfo, result, anteEvents, err := app.runTxWithMultiStore(execModeFinalize, tx, txIndex, txMultiStore) + gInfo, result, anteEvents, err := app.runTxWithMultiStore(execModeFinalize, tx, txIndex, txMultiStore, incarnationCache) if err != nil { resultStr = "failed" resp = sdkerrors.ResponseExecTxResultWithEvents( @@ -845,16 +845,17 @@ func (app *BaseApp) endBlock(_ context.Context) (sdk.EndBlock, error) { // returned if the tx does not run out of gas and if all the messages are valid // and execute successfully. An error is returned otherwise. func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) { - return app.runTxWithMultiStore(mode, txBytes, -1, nil) + return app.runTxWithMultiStore(mode, txBytes, -1, nil, nil) } -func (app *BaseApp) runTxWithMultiStore(mode execMode, txBytes []byte, txIndex int, txMultiStore storetypes.MultiStore) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) { +func (app *BaseApp) runTxWithMultiStore(mode execMode, txBytes []byte, txIndex int, txMultiStore storetypes.MultiStore, incarnationCache map[string]any) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) { // NOTE: GasWanted should be returned by the AnteHandler. GasUsed is // determined by the GasMeter. We need access to the context to get the gas // meter, so we initialize upfront. var gasWanted uint64 ctx := app.getContextForTx(mode, txBytes, txIndex) + ctx = ctx.WithIncarnationCache(incarnationCache) if txMultiStore != nil { ctx = ctx.WithMultiStore(txMultiStore) } diff --git a/baseapp/txexecutor.go b/baseapp/txexecutor.go index 230250ce9992..82649036dba0 100644 --- a/baseapp/txexecutor.go +++ b/baseapp/txexecutor.go @@ -12,5 +12,5 @@ type TxExecutor func( ctx context.Context, blockSize int, cms types.MultiStore, - deliverTxWithMultiStore func(int, types.MultiStore) *abci.ExecTxResult, + deliverTxWithMultiStore func(int, types.MultiStore, map[string]any) *abci.ExecTxResult, ) ([]*abci.ExecTxResult, error) diff --git a/types/context.go b/types/context.go index e07f4e7de160..ef58584af661 100644 --- a/types/context.go +++ b/types/context.go @@ -76,6 +76,10 @@ type Context struct { blockGasUsed uint64 // sum the gas wanted by all the transactions in the current block, only accessible by end blocker blockGasWanted uint64 + + // incarnationCache is shared between multiple incarnations of the same transaction, + // it must only cache stateless computation results that only depends on tx body and block level information that don't change during block execution, like the result of tx signature verification. + incarnationCache map[string]any } // Proposed rename, not done to avoid API breakage @@ -109,6 +113,23 @@ func (c Context) MsgIndex() int { return c.msgIn func (c Context) TxCount() int { return c.txCount } func (c Context) BlockGasUsed() uint64 { return c.blockGasUsed } func (c Context) BlockGasWanted() uint64 { return c.blockGasWanted } +func (c Context) IncarnationCache() map[string]any { return c.incarnationCache } + +func (c Context) GetIncarnationCache(key string) (any, bool) { + if c.incarnationCache == nil { + return nil, false + } + val, ok := c.incarnationCache[key] + return val, ok +} + +func (c Context) SetIncarnationCache(key string, value any) { + if c.incarnationCache == nil { + // noop if cache is not initialized + return + } + c.incarnationCache[key] = value +} // BlockHeader returns the header by value (shallow copy). func (c Context) BlockHeader() cmtproto.Header { @@ -361,6 +382,11 @@ func (c Context) WithBlockGasWanted(gasWanted uint64) Context { return c } +func (c Context) WithIncarnationCache(cache map[string]any) Context { + c.incarnationCache = cache + return c +} + // TODO: remove??? func (c Context) IsZero() bool { return c.ms == nil diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index c588f2b78276..75d82bd021e2 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -31,6 +31,8 @@ var ( key = make([]byte, secp256k1.PubKeySize) simSecp256k1Pubkey = &secp256k1.PubKey{Key: key} simSecp256k1Sig [64]byte + + SigVerificationResultCacheKey = "ante:SigVerificationResult" ) func init() { @@ -250,44 +252,44 @@ func OnlyLegacyAminoSigners(sigData signing.SignatureData) bool { } } -func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { +func (svd SigVerificationDecorator) anteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool) error { sigTx, ok := tx.(authsigning.Tx) if !ok { - return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") + return errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") } // stdSigs contains the sequence number, account number, and signatures. // When simulating, this would just be a 0-length slice. sigs, err := sigTx.GetSignaturesV2() if err != nil { - return ctx, err + return err } signers, err := sigTx.GetSigners() if err != nil { - return ctx, err + return err } // check that signer length and signature length are the same if len(sigs) != len(signers) { - return ctx, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "invalid number of signer; expected: %d, got %d", len(signers), len(sigs)) + return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "invalid number of signer; expected: %d, got %d", len(signers), len(sigs)) } for i, sig := range sigs { acc, err := GetSignerAcc(ctx, svd.ak, signers[i]) if err != nil { - return ctx, err + return err } // retrieve pubkey pubKey := acc.GetPubKey() if !simulate && pubKey == nil { - return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") } // Check account sequence number. if sig.Sequence != acc.GetSequence() { - return ctx, errorsmod.Wrapf( + return errorsmod.Wrapf( sdkerrors.ErrWrongSequence, "account sequence mismatch, expected %d, got %d", acc.GetSequence(), sig.Sequence, ) @@ -317,7 +319,7 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul } adaptableTx, ok := tx.(authsigning.V2AdaptableTx) if !ok { - return ctx, fmt.Errorf("expected tx to implement V2AdaptableTx, got %T", tx) + return fmt.Errorf("expected tx to implement V2AdaptableTx, got %T", tx) } txData := adaptableTx.GetSigningTxData() err = authsigning.VerifySignature(ctx, pubKey, signerData, sig.Data, svd.signModeHandler, txData) @@ -330,12 +332,27 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul } else { errMsg = fmt.Sprintf("signature verification failed; please verify account number (%d) and chain-id (%s): (%s)", accNum, chainID, err.Error()) } - return ctx, errorsmod.Wrap(sdkerrors.ErrUnauthorized, errMsg) + return errorsmod.Wrap(sdkerrors.ErrUnauthorized, errMsg) } } } + return nil +} +func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + if v, ok := ctx.GetIncarnationCache(SigVerificationResultCacheKey); ok { + // can't convert `nil` to interface + if v != nil { + err = v.(error) + } + } else { + err = svd.anteHandle(ctx, tx, simulate) + ctx.SetIncarnationCache(SigVerificationResultCacheKey, err) + } + if err != nil { + return ctx, err + } return next(ctx, tx, simulate) }