Problem: signature verification result not cache between incarnations…

… of same tx (cosmos#565) * Problem: signature verification result not cache between incarnations of same tx Closes: cosmos#564 Solution: - introduce incarnation cache that's shared between incarnations of the same tx * Update CHANGELOG.md Signed-off-by: yihuang <[email protected]> * Update types/context.go Signed-off-by: yihuang <[email protected]> * Update types/context.go Signed-off-by: yihuang <[email protected]> * fix nil convert --------- Signed-off-by: yihuang <[email protected]>
mmsqe · Dec 12, 2024 · 78bfe02 · 78bfe02
1 parent baa3e40
commit 78bfe02
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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)
 }