Skip to content

Commit

Permalink
Problem: signature verification result not cache between incarnations…
Browse files Browse the repository at this point in the history
… 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]>

Problem: gas consumed differs after enabled cache (cosmos#570)

* Problem: gas consumed differs after enabled cache

* fix test

* Revert "Problem: gas consumed differs after enabled cache"

This reverts commit f33944e.

* Revert "Problem: signature verification result not cache between incarnations of same tx (cosmos#565)"

This reverts commit 5a1594f.

* keep interface
  • Loading branch information
yihuang authored and mmsqe committed Dec 18, 2024
1 parent 2202573 commit cd5b28d
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 9 deletions.
4 changes: 2 additions & 2 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

Expand Down
11 changes: 6 additions & 5 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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(
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion baseapp/txexecutor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
1 change: 0 additions & 1 deletion core/coins/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -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], ",") {
Expand Down
26 changes: 26 additions & 0 deletions types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit cd5b28d

Please sign in to comment.