update sig cache logic

bnb-chain · Jun 27, 2023 · 6d8b7fc · 6d8b7fc
1 parent 33f8bd7
commit 6d8b7fc
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 50 deletions.
diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
@@ -727,9 +727,18 @@ func (app *BaseApp) runTxOnContext(ctx sdk.Context, mode runTxMode, txBytes []by
 		defer consumeBlockGas()
 	}
 
-	tx, err := app.txDecoder(txBytes)
-	if err != nil {
-		return sdk.GasInfo{}, nil, nil, 0, err
+	var tx sdk.Tx
+	if ctx.SigCache() != nil && ctx.TxBytes() != nil {
+		if txCache, known := ctx.SigCache().Get(ctx.TxBytes()); known {
+			tx = txCache.(sdk.Tx)
+		}
+	}
+
+	if tx == nil {
+		tx, err = app.txDecoder(txBytes)
+		if err != nil {
+			return sdk.GasInfo{}, nil, nil, 0, err
+		}
 	}
 
 	msgs := tx.GetMsgs()

diff --git 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
@@ -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
@@ -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
@@ -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)
@@ -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), tx)
+			}
 			return nil
 		}
 		// This should never happen, but we add it just for test cases.