From cd5b28d06b078e5c3ce463270ff62d19a78fd0f6 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 Problem: gas consumed differs after enabled cache (#570) * Problem: gas consumed differs after enabled cache * fix test * Revert "Problem: gas consumed differs after enabled cache" This reverts commit f33944ee849c66f1b51d5d29e6e4f3b6ce41797a. * Revert "Problem: signature verification result not cache between incarnations of same tx (#565)" This reverts commit 5a1594f17924388dc2530bd8143ca2b04655fad4. * keep interface --- baseapp/abci.go | 4 ++-- baseapp/baseapp.go | 11 ++++++----- baseapp/txexecutor.go | 2 +- core/coins/format.go | 1 - types/context.go | 26 ++++++++++++++++++++++++++ 5 files changed, 35 insertions(+), 9 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 b626c64813f1..edabaf1fd689 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/core/coins/format.go b/core/coins/format.go index c23a25dc2612..7b1e49f2b7e2 100644 --- a/core/coins/format.go +++ b/core/coins/format.go @@ -84,7 +84,6 @@ func FormatCoins(coins []*basev1beta1.Coin, metadata []*bankv1beta1.Metadata) (s if err != nil { return "", err } - // If a coin contains a comma, return an error given that the output // could be misinterpreted by the user as 2 different coins. if strings.Contains(formatted[i], ",") { 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