Skip to content

Commit

Permalink
Problem: signature verification result not cache between incarnations of
Browse files Browse the repository at this point in the history
same tx

Closes: cosmos#564

Solution:
- introduce incarnation cache that's shared between incarnations of the
  same tx
  • Loading branch information
yihuang committed Jul 12, 2024
1 parent 52a97a8 commit 45b23bd
Show file tree
Hide file tree
Showing 6 changed files with 59 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) [#]() 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)
25 changes: 25 additions & 0 deletions types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ 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 incranations of the same transaction.
incarnationCache map[string]any
}

// Proposed rename, not done to avoid API breakage
Expand Down Expand Up @@ -107,6 +110,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 +372,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
34 changes: 24 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,24 @@ 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 {
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 45b23bd

Please sign in to comment.