Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: signature verification cache #41

Merged
merged 8 commits into from
Jan 7, 2021
Merged

feat: signature verification cache #41

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bdac6d6
feat: signature verification cache
jinsan-line Dec 8, 2020
3ac69bd
chore: recover to skip to sigverify
jinsan-line Jan 5, 2021
dd56ff5
chore: rename `ok` to `exist` of `checkCache()` return var
jinsan-line Jan 5, 2021
cfc5dca
Merge remote-tracking branch 'upstream/feat/perf' into sig-cache
jinsan-line Jan 6, 2021
865bc14
chore: remove txHashCash if got an error
jinsan-line Jan 6, 2021
f369205
chore: revise `verifySignatureWithCache()` for readability
jinsan-line Jan 7, 2021
9531633
chore: remove naked return
jinsan-line Jan 7, 2021
a77c79a
fix: lint error
jinsan-line Jan 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 33 additions & 10 deletions x/auth/ante/sigverify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,16 @@ func (svd *SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
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 {
jinsan-line marked this conversation as resolved.
Show resolved Hide resolved
for _, sigKey := range newSigKeys {
svd.txHashCache.Delete(sigKey)
}
}
}()

for i, sig := range sigs {
signerAcc, err := GetSignerAcc(ctx, svd.ak, signerAddrs[i])
if err != nil {
Expand All @@ -211,8 +221,15 @@ func (svd *SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
}

txHash := sha256.Sum256(ctx.TxBytes())
sigKey := fmt.Sprintf("%d:%d", signerAcc.GetAccountNumber(), signerAcc.GetSequence())

if !svd.verifySignatureWithCache(ctx, sigTx, txHash[:], signerAcc, pubKey, sig) {
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)", signerAcc.GetSequence(), ctx.ChainID())
Expand All @@ -222,21 +239,27 @@ func (svd *SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
return next(ctx, tx, simulate)
}

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

// NOTE assume that `accountNumber` and `pubKey` are immutable
sigKey := fmt.Sprintf("%d:%d", signerAcc.GetAccountNumber(), signerAcc.GetSequence())

switch {
case ctx.IsCheckTx() && !ctx.IsReCheckTx(): // CheckTx
if !pubKey.VerifyBytes(sigTx.GetSignBytes(ctx, signerAcc), sig) {
return false
return false, false
}
svd.txHashCache.Store(sigKey, txHash)
Copy link

@egonspace egonspace Dec 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like some tx's signature will not be removed from the cache if it does not meet DeliverTx. If the mempool is full, the tx will not be able to enter the mempool after passing checkTx, so it will not be able to meet DeliverTx.

Copy link
Contributor Author

@jinsan-line jinsan-line Dec 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're correct. A tx couldn't be removed from the cache if the tx is not added to mempool. But I'd like to care it w/ missing transaction issue. Currently, I assume that the tx must be added to mempool if it passes checkTx.

Copy link
Contributor Author

@jinsan-line jinsan-line Dec 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As another approach, we could remove a tx from the cache when we fail to add a tx to mempool.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thought, even though we fail to add a tx to mempool after adding it to the cache, it could be removed when same account w/ the same sequence broadcasts a tx again. So, finally after thinking deeply, I have a conclusion that it's not a big deal. Could you agree on it?

stored = true

case ctx.IsReCheckTx(): // ReCheckTx
jinsan-line marked this conversation as resolved.
Show resolved Hide resolved
verified, exist := svd.checkCache(sigKey, txHash)
Expand All @@ -245,7 +268,7 @@ func (svd *SigVerificationDecorator) verifySignatureWithCache(ctx sdk.Context, s
if exist {
svd.txHashCache.Delete(sigKey)
}
return false
return false, false
}

default: // DeliverTx
Expand All @@ -255,11 +278,11 @@ func (svd *SigVerificationDecorator) verifySignatureWithCache(ctx sdk.Context, s
}

if !verified && !pubKey.VerifyBytes(sigTx.GetSignBytes(ctx, signerAcc), sig) {
return false
return false, false
}
}

return true
return true, stored
}

func (svd *SigVerificationDecorator) checkCache(sigKey string, txHash []byte) (verified, exist bool) {
Expand Down