Skip to content

Commit

Permalink
feat: signature verification cache (#41)
Browse files Browse the repository at this point in the history 
* feat: signature verification cache

* chore: recover to skip to sigverify

* chore: rename `ok` to `exist` of `checkCache()` return var

* chore: remove txHashCash if got an error

* chore: revise `verifySignatureWithCache()` for readability

* chore: remove naked return

* fix: lint error
  • Loading branch information
@jinsan-line
jinsan-line authored Jan 7, 2021
1 parent 900473c commit 4946a48
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 34 deletions.
34 changes: 18 additions & 16 deletions x/auth/ante/ante_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,10 @@ func TestAnteHandlerReCheck(t *testing.T) {
app, ctx := createTestApp(true)
// set blockheight and recheck=true
ctx = ctx.WithBlockHeight(1)
ctx = ctx.WithIsReCheckTx(true)

checkCtx, _ := ctx.CacheContext()
recheckCtx, _ := ctx.CacheContext()
recheckCtx = recheckCtx.WithIsReCheckTx(true)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand All @@ -760,18 +763,16 @@ func TestAnteHandlerReCheck(t *testing.T) {
privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0}
tx := types.NewTestTxWithMemo(ctx, msgs, privs, accnums, seqs, fee, "thisisatestmemo")

// make signature array empty which would normally cause ValidateBasicDecorator and SigVerificationDecorator fail
// since these decorators don't run on recheck, the tx should pass the antehandler
stdTx := tx.(types.StdTx)
stdTx.Signatures = []types.StdSignature{}
_, err := antehandler(checkCtx, tx, false)
require.Nil(t, err, "AnteHandler errored on check unexpectedly: %v", err)

_, err := antehandler(ctx, stdTx, false)
_, err = antehandler(recheckCtx, tx, false)
require.Nil(t, err, "AnteHandler errored on recheck unexpectedly: %v", err)

tx = types.NewTestTxWithMemo(ctx, msgs, privs, accnums, seqs, fee, "thisisatestmemo")
tx = types.NewTestTxWithMemo(recheckCtx, msgs, privs, accnums, seqs, fee, "thisisatestmemo")
txBytes, err := json.Marshal(tx)
require.Nil(t, err, "Error marshalling tx: %v", err)
ctx = ctx.WithTxBytes(txBytes)
recheckCtx = recheckCtx.WithTxBytes(txBytes)

// require that state machine param-dependent checking is still run on recheck since parameters can change between check and recheck
testCases := []struct {
Expand All @@ -784,31 +785,32 @@ func TestAnteHandlerReCheck(t *testing.T) {
}
for _, tc := range testCases {
// set testcase parameters
app.AccountKeeper.SetParams(ctx, tc.params)
app.AccountKeeper.SetParams(recheckCtx, tc.params)

_, err := antehandler(ctx, tx, false)
_, err := antehandler(recheckCtx, tx, false)

require.NotNil(t, err, "tx does not fail on recheck with updated params in test case: %s", tc.name)

// reset parameters to default values
app.AccountKeeper.SetParams(ctx, types.DefaultParams())
app.AccountKeeper.SetParams(recheckCtx, types.DefaultParams())
}

// require that local mempool fee check is still run on recheck since validator may change minFee between check and recheck
// create new minimum gas price so antehandler fails on recheck
ctx = ctx.WithMinGasPrices([]sdk.DecCoin{{
recheckCtx = recheckCtx.WithMinGasPrices([]sdk.DecCoin{{
Denom: "dnecoin", // fee does not have this denom
Amount: sdk.NewDec(5),
}})
_, err = antehandler(ctx, tx, false)

_, err = antehandler(recheckCtx, tx, false)
require.NotNil(t, err, "antehandler on recheck did not fail when mingasPrice was changed")
// reset min gasprice
ctx = ctx.WithMinGasPrices(sdk.DecCoins{})
recheckCtx = recheckCtx.WithMinGasPrices(sdk.DecCoins{})

// remove funds for account so antehandler fails on recheck
acc1.SetCoins(sdk.Coins{})
app.AccountKeeper.SetAccount(ctx, acc1)
app.AccountKeeper.SetAccount(recheckCtx, acc1)

_, err = antehandler(ctx, tx, false)
_, err = antehandler(recheckCtx, tx, false)
require.NotNil(t, err, "antehandler on recheck did not fail once feePayer no longer has sufficient funds")
}
105 changes: 88 additions & 17 deletions x/auth/ante/sigverify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package ante

import (
"bytes"
"crypto/sha256"
"encoding/hex"
"fmt"
"sync"

"github.com/tendermint/tendermint/crypto"
"github.com/tendermint/tendermint/crypto/ed25519"
Expand Down Expand Up @@ -159,16 +162,19 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
// CONTRACT: Pubkeys are set in context for all signers before this decorator runs
// CONTRACT: Tx must implement SigVerifiableTx interface
type SigVerificationDecorator struct {
ak keeper.AccountKeeper
ak keeper.AccountKeeper
txHashCache sync.Map
}

func NewSigVerificationDecorator(ak keeper.AccountKeeper) SigVerificationDecorator {
return SigVerificationDecorator{
ak: ak,
func NewSigVerificationDecorator(ak keeper.AccountKeeper) *SigVerificationDecorator {
return &SigVerificationDecorator{
ak: ak,
txHashCache: sync.Map{},
}
}

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, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
// TODO https://github.com/line/link/issues/1136
// no need to verify signatures on recheck tx
if ctx.IsReCheckTx() {
return next(ctx, tx, simulate)
Expand All @@ -181,43 +187,108 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul
// stdSigs contains the sequence number, account number, and signatures.
// When simulating, this would just be a 0-length slice.
sigs := sigTx.GetSignatures()

// stdSigs contains the sequence number, account number, and signatures.
// When simulating, this would just be a 0-length slice.
signerAddrs := sigTx.GetSigners()
signerAccs := make([]exported.Account, len(signerAddrs))

// check that signer length and signature length are the same
if len(sigs) != len(signerAddrs) {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "invalid number of signer; expected: %d, got %d", len(signerAddrs), len(sigs))
}

newSigKeys := make([]string, 0, len(sigs))
defer func() {
// remove txHashCash if got an error
if err != nil {
for _, sigKey := range newSigKeys {
svd.txHashCache.Delete(sigKey)
}
}
}()

for i, sig := range sigs {
signerAccs[i], err = GetSignerAcc(ctx, svd.ak, signerAddrs[i])
signerAcc, err := GetSignerAcc(ctx, svd.ak, signerAddrs[i])
if err != nil {
return ctx, err
}

// retrieve signBytes of tx
signBytes := sigTx.GetSignBytes(ctx, signerAccs[i])
if simulate {
continue
}

// retrieve pubkey
pubKey := signerAccs[i].GetPubKey()
if !simulate && pubKey == nil {
pubKey := signerAcc.GetPubKey()
if pubKey == nil {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set")
}

// verify signature
if !simulate && !pubKey.VerifyBytes(signBytes, sig) {
txHash := sha256.Sum256(ctx.TxBytes())
sigKey := fmt.Sprintf("%d:%d", signerAcc.GetAccountNumber(), signerAcc.GetSequence())

verified, stored := svd.verifySignatureWithCache(ctx, sigTx, sigKey, txHash[:], signerAcc, pubKey, sig)

if stored {
newSigKeys = append(newSigKeys, sigKey)
}

if !verified {
return ctx, sdkerrors.Wrapf(
sdkerrors.ErrUnauthorized,
"signature verification failed; verify correct account sequence (%d) and chain-id (%s)", signerAccs[i].GetSequence(), ctx.ChainID())
"signature verification failed; verify correct account sequence (%d) and chain-id (%s)", signerAcc.GetSequence(), ctx.ChainID())
}
}

return next(ctx, tx, simulate)
}

func (svd *SigVerificationDecorator) verifySignatureWithCache(
ctx sdk.Context,
sigTx SigVerifiableTx,
sigKey string,
txHash []byte,
signerAcc exported.Account,
pubKey crypto.PubKey,
sig []byte,
) (verified, stored bool) {
// #NOTE `genesis` transactions should not use `cache`
if ctx.BlockHeight() == 0 {
verified = pubKey.VerifyBytes(sigTx.GetSignBytes(ctx, signerAcc), sig)
return verified, stored
}

exist := false

switch {
case ctx.IsCheckTx() && !ctx.IsReCheckTx(): // CheckTx
verified = pubKey.VerifyBytes(sigTx.GetSignBytes(ctx, signerAcc), sig)
if verified {
svd.txHashCache.Store(sigKey, txHash)
stored = true
}

case ctx.IsReCheckTx(): // ReCheckTx
verified, exist = svd.checkCache(sigKey, txHash)
if !verified && exist {
svd.txHashCache.Delete(sigKey)
}

default: // DeliverTx
verified, exist = svd.checkCache(sigKey, txHash)
if exist {
svd.txHashCache.Delete(sigKey)
}
if !verified {
verified = pubKey.VerifyBytes(sigTx.GetSignBytes(ctx, signerAcc), sig)
}
}

return verified, stored
}

func (svd *SigVerificationDecorator) checkCache(sigKey string, txHash []byte) (verified, exist bool) {
cached, exist := svd.txHashCache.Load(sigKey)
verified = exist && bytes.Equal(cached.([]byte), txHash)
return verified, exist
}

// IncrementSequenceDecorator handles incrementing sequences of all signers.
// Use the IncrementSequenceDecorator decorator to prevent replay attacks. Note,
// there is no need to execute IncrementSequenceDecorator on RecheckTX since
Expand Down
2 changes: 1 addition & 1 deletion x/auth/ante/sigverify_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func TestSigVerification(t *testing.T) {
{"wrong accnums", []crypto.PrivKey{priv1, priv2, priv3}, []uint64{7, 8, 9}, []uint64{0, 0, 0}, false, true},
{"wrong sequences", []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{3, 4, 5}, false, true},
{"valid tx", []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0}, false, false},
{"no err on recheck", []crypto.PrivKey{}, []uint64{}, []uint64{}, true, false},
{"no err on recheck", []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0}, true, false},
}
for i, tc := range testCases {
ctx = ctx.WithIsReCheckTx(tc.recheck)
Expand Down

0 comments on commit 4946a48

Please sign in to comment.