diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index ccedb2ac06..af571b9d3d 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -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) - } } } diff --git a/x/auth/client/cli/tx_multisign.go b/x/auth/client/cli/tx_multisign.go index ae9844da8f..4cbc5051cd 100644 --- a/x/auth/client/cli/tx_multisign.go +++ b/x/auth/client/cli/tx_multisign.go @@ -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) @@ -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) } diff --git a/x/auth/client/cli/validate_sigs.go b/x/auth/client/cli/validate_sigs.go index 75b88eb9c4..61dc033a7c 100644 --- a/x/auth/client/cli/validate_sigs.go +++ b/x/auth/client/cli/validate_sigs.go @@ -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 } diff --git a/x/auth/signing/verify.go b/x/auth/signing/verify.go index e79e92ccb0..0a5c22eedb 100644 --- a/x/auth/signing/verify.go +++ b/x/auth/signing/verify.go @@ -17,10 +17,17 @@ 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 @@ -28,48 +35,20 @@ func VerifySignature(pubKey cryptotypes.PubKey, signerData SignerData, sigData s 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) @@ -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.