From 1e245aa54bd4bd0cccdb63524b45010ce043f114 Mon Sep 17 00:00:00 2001
From: Yamen Merhi <yamennmerhi@gmail.com>
Date: Tue, 21 Feb 2023 17:16:44 +0200
Subject: [PATCH] Add `isValidERC1271SignatureNow` to SignatureChecker library
 (#3932)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco <fg@frang.io>
---
 .changeset/slimy-knives-hug.md                |  5 ++
 .../utils/cryptography/SignatureChecker.sol   | 19 ++++-
 .../cryptography/SignatureChecker.test.js     | 76 ++++++++++---------
 3 files changed, 61 insertions(+), 39 deletions(-)
 create mode 100644 .changeset/slimy-knives-hug.md

diff --git a/.changeset/slimy-knives-hug.md b/.changeset/slimy-knives-hug.md
new file mode 100644
index 000000000..94099eea7
--- /dev/null
+++ b/.changeset/slimy-knives-hug.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`SignatureChecker`: Add `isValidERC1271SignatureNow` for checking a signature directly against a smart contract using ERC-1271.
diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol
index e06778deb..c3a724dd0 100644
--- a/contracts/utils/cryptography/SignatureChecker.sol
+++ b/contracts/utils/cryptography/SignatureChecker.sol
@@ -23,10 +23,23 @@ library SignatureChecker {
      */
     function isValidSignatureNow(address signer, bytes32 hash, bytes memory signature) internal view returns (bool) {
         (address recovered, ECDSA.RecoverError error) = ECDSA.tryRecover(hash, signature);
-        if (error == ECDSA.RecoverError.NoError && recovered == signer) {
-            return true;
-        }
+        return
+            (error == ECDSA.RecoverError.NoError && recovered == signer) ||
+            isValidERC1271SignatureNow(signer, hash, signature);
+    }
 
+    /**
+     * @dev Checks if a signature is valid for a given signer and data hash. The signature is validated
+     * against the signer smart contract using ERC1271.
+     *
+     * NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus
+     * change through time. It could return true at block N and false at block N+1 (or the opposite).
+     */
+    function isValidERC1271SignatureNow(
+        address signer,
+        bytes32 hash,
+        bytes memory signature
+    ) internal view returns (bool) {
         (bool success, bytes memory result) = signer.staticcall(
             abi.encodeWithSelector(IERC1271.isValidSignature.selector, hash, signature)
         );
diff --git a/test/utils/cryptography/SignatureChecker.test.js b/test/utils/cryptography/SignatureChecker.test.js
index 11054c3e1..ba8b100d1 100644
--- a/test/utils/cryptography/SignatureChecker.test.js
+++ b/test/utils/cryptography/SignatureChecker.test.js
@@ -40,44 +40,48 @@ contract('SignatureChecker (ERC1271)', function (accounts) {
   });
 
   context('ERC1271 wallet', function () {
-    it('with matching signer and signature', async function () {
-      expect(
-        await this.signaturechecker.$isValidSignatureNow(
-          this.wallet.address,
-          toEthSignedMessageHash(TEST_MESSAGE),
-          this.signature,
-        ),
-      ).to.equal(true);
-    });
+    for (const signature of ['isValidERC1271SignatureNow', 'isValidSignatureNow']) {
+      context(signature, function () {
+        it('with matching signer and signature', async function () {
+          expect(
+            await this.signaturechecker[`$${signature}`](
+              this.wallet.address,
+              toEthSignedMessageHash(TEST_MESSAGE),
+              this.signature,
+            ),
+          ).to.equal(true);
+        });
 
-    it('with invalid signer', async function () {
-      expect(
-        await this.signaturechecker.$isValidSignatureNow(
-          this.signaturechecker.address,
-          toEthSignedMessageHash(TEST_MESSAGE),
-          this.signature,
-        ),
-      ).to.equal(false);
-    });
+        it('with invalid signer', async function () {
+          expect(
+            await this.signaturechecker[`$${signature}`](
+              this.signaturechecker.address,
+              toEthSignedMessageHash(TEST_MESSAGE),
+              this.signature,
+            ),
+          ).to.equal(false);
+        });
 
-    it('with invalid signature', async function () {
-      expect(
-        await this.signaturechecker.$isValidSignatureNow(
-          this.wallet.address,
-          toEthSignedMessageHash(WRONG_MESSAGE),
-          this.signature,
-        ),
-      ).to.equal(false);
-    });
+        it('with invalid signature', async function () {
+          expect(
+            await this.signaturechecker[`$${signature}`](
+              this.wallet.address,
+              toEthSignedMessageHash(WRONG_MESSAGE),
+              this.signature,
+            ),
+          ).to.equal(false);
+        });
 
-    it('with malicious wallet', async function () {
-      expect(
-        await this.signaturechecker.$isValidSignatureNow(
-          this.malicious.address,
-          toEthSignedMessageHash(TEST_MESSAGE),
-          this.signature,
-        ),
-      ).to.equal(false);
-    });
+        it('with malicious wallet', async function () {
+          expect(
+            await this.signaturechecker[`$${signature}`](
+              this.malicious.address,
+              toEthSignedMessageHash(TEST_MESSAGE),
+              this.signature,
+            ),
+          ).to.equal(false);
+        });
+      });
+    }
   });
 });