Skip to content

Commit

Permalink
update sig cache logic
Browse files Browse the repository at this point in the history
  • Loading branch information
pythonberg1997 committed Jun 27, 2023
1 parent d98d111 commit 4b2068d
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 47 deletions.
5 changes: 2 additions & 3 deletions x/auth/ante/sigverify.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,10 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul

// no need to verify signatures on recheck tx
if !simulate && !ctx.IsReCheckTx() {
err := authsigning.VerifySignature(pubKey, signerData, sig.Data, svd.signModeHandler, tx, ctx.SigCache())
err := authsigning.VerifySignature(pubKey, signerData, sig.Data, svd.signModeHandler, tx, ctx.SigCache(), ctx.TxBytes())
if err != nil {
errMsg := fmt.Sprintf("signature verification failed; please verify account (%s) and chain-id (%s)", pubKey.Address(), chainID)
errMsg := fmt.Sprintf("signature verification failed; please verify account (%s); err: %s", pubKey.Address(), err)
return ctx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, errMsg)

}
}
}
Expand Down
4 changes: 2 additions & 2 deletions x/auth/client/cli/tx_multisign.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) (err error) {
PubKey: sig.PubKey,
}

err = signing.VerifySignature(sig.PubKey, signingData, sig.Data, txCfg.SignModeHandler(), txBuilder.GetTx(), nil)
err = signing.VerifySignature(sig.PubKey, signingData, sig.Data, txCfg.SignModeHandler(), txBuilder.GetTx(), nil, nil)
if err != nil {
addr, _ := sdk.AccAddressFromHexUnsafe(sig.PubKey.Address().String())
return fmt.Errorf("couldn't verify signature for address %s", addr)
Expand Down Expand Up @@ -329,7 +329,7 @@ func makeBatchMultisignCmd() func(cmd *cobra.Command, args []string) error {
}

for _, sig := range signatureBatch {
err = signing.VerifySignature(sig[i].PubKey, signingData, sig[i].Data, txCfg.SignModeHandler(), txBldr.GetTx(), nil)
err = signing.VerifySignature(sig[i].PubKey, signingData, sig[i].Data, txCfg.SignModeHandler(), txBldr.GetTx(), nil, nil)
if err != nil {
return fmt.Errorf("couldn't verify signature: %w %v", err, sig)
}
Expand Down
2 changes: 1 addition & 1 deletion x/auth/client/cli/validate_sigs.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func printAndValidateSigs(
Sequence: accSeq,
PubKey: pubKey,
}
err = authsigning.VerifySignature(pubKey, signingData, sig.Data, signModeHandler, sigTx, nil)
err = authsigning.VerifySignature(pubKey, signingData, sig.Data, signModeHandler, sigTx, nil, nil)
if err != nil {
return false
}
Expand Down
66 changes: 25 additions & 41 deletions x/auth/signing/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,59 +17,38 @@ import (

// VerifySignature verifies a transaction signature contained in SignatureData abstracting over different signing modes
// and single vs multi-signatures.
func VerifySignature(pubKey cryptotypes.PubKey, signerData SignerData, sigData signing.SignatureData, handler SignModeHandler, tx sdk.Tx, sigCache *lru.ARCCache) error {
func VerifySignature(pubKey cryptotypes.PubKey, signerData SignerData, sigData signing.SignatureData, handler SignModeHandler, tx sdk.Tx, sigCache *lru.ARCCache, txBytes []byte) error {
switch data := sigData.(type) {
case *signing.SingleSignatureData:
if data.SignMode == signing.SignMode_SIGN_MODE_EIP_712 {
// skip signature verification if we have a cache and the tx is already in it
if sigCache != nil && txBytes != nil {
if _, known := sigCache.Get(string(txBytes)); known {
return nil
}
}

sig := data.Signature

// check signature length
if len(sig) != ethcrypto.SignatureLength {
return errorsmod.Wrap(sdkerrors.ErrorInvalidSigner, "signature length doesn't match typical [R||S||V] signature 65 bytes")
}

var feePayerPubkey []byte
if sigCache != nil {
var key [ethcrypto.SignatureLength]byte
copy(key[:], sig)

if pubkey, known := sigCache.Get(key); known {
feePayerPubkey = pubkey.([]byte)
} else {
sigHash, err := handler.GetSignBytes(data.SignMode, signerData, tx)
if err != nil {
return err
}

// remove the recovery offset if needed (ie. Metamask eip712 signature)
if sig[ethcrypto.RecoveryIDOffset] == 27 || sig[ethcrypto.RecoveryIDOffset] == 28 {
sig[ethcrypto.RecoveryIDOffset] -= 27
}

// recover the pubkey from the signature
feePayerPubkey, err = secp256k1.RecoverPubkey(sigHash, sig)
if err != nil {
return errorsmod.Wrap(err, "failed to recover fee payer from sig")
}

sigCache.Add(key, feePayerPubkey)
}
} else {
sigHash, err := handler.GetSignBytes(data.SignMode, signerData, tx)
if err != nil {
return err
}
sigHash, err := handler.GetSignBytes(data.SignMode, signerData, tx)
if err != nil {
return err
}

// remove the recovery offset if needed (ie. Metamask eip712 signature)
if sig[ethcrypto.RecoveryIDOffset] == 27 || sig[ethcrypto.RecoveryIDOffset] == 28 {
sig[ethcrypto.RecoveryIDOffset] -= 27
}
// remove the recovery offset if needed (ie. Metamask eip712 signature)
if sig[ethcrypto.RecoveryIDOffset] == 27 || sig[ethcrypto.RecoveryIDOffset] == 28 {
sig[ethcrypto.RecoveryIDOffset] -= 27
}

// recover the pubkey from the signature
feePayerPubkey, err = secp256k1.RecoverPubkey(sigHash, sig)
if err != nil {
return errorsmod.Wrap(err, "failed to recover fee payer from sig")
}
// recover the pubkey from the signature
feePayerPubkey, err := secp256k1.RecoverPubkey(sigHash, sig)
if err != nil {
return errorsmod.Wrap(err, "failed to recover fee payer from sig")
}

ecPubKey, err := ethcrypto.UnmarshalPubkey(feePayerPubkey)
Expand All @@ -84,6 +63,11 @@ func VerifySignature(pubKey cryptotypes.PubKey, signerData SignerData, sigData s
if !pubKey.Equals(pk) {
return errorsmod.Wrapf(sdkerrors.ErrorInvalidSigner, "feePayer's pubkey %s is different from signature's pubkey %s", pubKey, pk)
}

// add the tx to the cache if needed
if sigCache != nil && txBytes != nil {
sigCache.Add(string(txBytes), struct{}{})
}
return nil
}
// This should never happen, but we add it just for test cases.
Expand Down

0 comments on commit 4b2068d

Please sign in to comment.