Add isValidERC1271SignatureNow to SignatureChecker library (#3932)

Co-authored-by: Hadrien Croubois <[email protected]> Co-authored-by: Francisco <[email protected]>
OpenZeppelin · Feb 21, 2023 · 1e245aa · 1e245aa
1 parent 815bfbf
commit 1e245aa
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 39 deletions.
diff --git a/.changeset/slimy-knives-hug.md 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
@@ -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
@@ -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);
+        });
+      });
+    }
   });
 });