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]>
  • Loading branch information
yihuang authored Jul 15, 2024
1 parent 52a97a8 commit 5a1594f
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [#252](https://github.com/crypto-org-chain/cosmos-sdk/pull/252) Add `BlockGasWanted` to `Context` to support feemarket module.
* [#269](https://github.com/crypto-org-chain/cosmos-sdk/pull/269) Add `StreamingManager` to baseapp to extend the abci listeners.
* (crypto/keyring) [#20212](https://github.com/cosmos/cosmos-sdk/pull/20212) Expose the db keyring used in the keystore.
* (baseapp) [#565](https://github.com/crypto-org-chain/cosmos-sdk/pull/565) Support incarnation cache when executed in block-stm.

### Improvements

Expand Down
4 changes: 2 additions & 2 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -842,8 +842,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 @@ -765,10 +765,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 @@ -781,7 +781,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 @@ -839,16 +839,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)
26 changes: 26 additions & 0 deletions types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,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 @@ -107,6 +111,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 @@ -352,6 +373,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
37 changes: 27 additions & 10 deletions x/auth/ante/sigverify.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ var (
key = make([]byte, secp256k1.PubKeySize)
simSecp256k1Pubkey = &secp256k1.PubKey{Key: key}
simSecp256k1Sig [64]byte

SigVerificationResultCacheKey = "ante:SigVerificationResult"
)

func init() {
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}

Expand Down

0 comments on commit 5a1594f

Please sign in to comment.