Optimize EOA signature verification

CHANGELOG.md

@@ -6,6 +6,10 @@
  * `EnumerableSet`: add `values()` functions that returns an array containing all values in a single call. ([#2768](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2768))
  * `Governor`: added a modular system of `Governor` contracts based on `GovernorAlpha` and `GovernorBravo`. ([#2672](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2672))
  * Add an `interface` folder containing solidity interfaces to final ERCs. ([#2517](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2517))
+ * `ECDSA`: add `tryRecover` functions that will not throw if the signature is invalid, and will return an error flag instead. ([#2661](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2661))
+ * `SignatureChecker`: Reduce gas usage of the `isValidSignatureNow` function for the "signature by EOA" case. ([#2661](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2661))
+ * `ECDSA`: add `tryRecover` functions that will not throw if the signature is invalid, and will return an error flag instead. ([#2661](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2661))
+ * `SignatureChecker`: Reduce gas usage of the `isValidSignatureNow` function for the "signature by EOA" case. ([#2661](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2661))
 
 ## 4.2.0 (2021-06-30)
 

contracts/mocks/ECDSAMock.sol

@@ -11,6 +11,25 @@ contract ECDSAMock {
         return hash.recover(signature);
     }
 
+    // solhint-disable-next-line func-name-mixedcase
+    function recover_v_r_s(
+        bytes32 hash,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public pure returns (address) {
+        return hash.recover(v, r, s);
+    }
+
+    // solhint-disable-next-line func-name-mixedcase
+    function recover_r_vs(
+        bytes32 hash,
+        bytes32 r,
+        bytes32 vs
+    ) public pure returns (address) {
+        return hash.recover(r, vs);
+    }
+
     function toEthSignedMessageHash(bytes32 hash) public pure returns (bytes32) {
         return hash.toEthSignedMessageHash();
     }

contracts/utils/cryptography/ECDSA.sol

@@ -9,9 +9,31 @@ pragma solidity ^0.8.0;
  * of the private keys of a given address.
  */
 library ECDSA {
+    enum RecoverError {
+        NoError,
+        InvalidSignature,
+        InvalidSignatureLength,
+        InvalidSignatureS,
+        InvalidSignatureV
+    }
+
+    function _throwError(RecoverError error) private pure {
+        if (error == RecoverError.NoError) {
+            return; // no error: do nothing
+        } else if (error == RecoverError.InvalidSignature) {
+            revert("ECDSA: invalid signature");
+        } else if (error == RecoverError.InvalidSignatureLength) {
+            revert("ECDSA: invalid signature length");
+        } else if (error == RecoverError.InvalidSignatureS) {
+            revert("ECDSA: invalid signature 's' value");
+        } else if (error == RecoverError.InvalidSignatureV) {
+            revert("ECDSA: invalid signature 'v' value");
+        }
+    }
+
     /**
      * @dev Returns the address that signed a hashed message (`hash`) with
-     * `signature`. This address can then be used for verification purposes.
+     * `signature` or error string. This address can then be used for verification purposes.
      *
      * The `ecrecover` EVM opcode allows for malleable (non-unique) signatures:
      * this function rejects them by requiring the `s` value to be in the lower
@@ -26,8 +48,10 @@ library ECDSA {
      * Documentation for signature generation:
      * - with https://web3js.readthedocs.io/en/v1.3.4/web3-eth-accounts.html#sign[Web3.js]
      * - with https://docs.ethers.io/v5/api/signer/#Signer-signMessage[ethers]
+     *
+     * _Available since v4.3._
      */
-    function recover(bytes32 hash, bytes memory signature) internal pure returns (address) {
+    function tryRecover(bytes32 hash, bytes memory signature) internal pure returns (address, RecoverError) {
         // Check the signature length
         // - case 65: r,s,v signature (standard)
         // - case 64: r,vs signature (cf https://eips.ethereum.org/EIPS/eip-2098) _Available since v4.1._
@@ -42,7 +66,7 @@ library ECDSA {
                 s := mload(add(signature, 0x40))
                 v := byte(0, mload(add(signature, 0x60)))
             }
-            return recover(hash, v, r, s);
+            return tryRecover(hash, v, r, s);
         } else if (signature.length == 64) {
             bytes32 r;
             bytes32 vs;
@@ -52,42 +76,80 @@ library ECDSA {
                 r := mload(add(signature, 0x20))
                 vs := mload(add(signature, 0x40))
             }
-            return recover(hash, r, vs);
+            return tryRecover(hash, r, vs);
         } else {
-            revert("ECDSA: invalid signature length");
+            return (address(0), RecoverError.InvalidSignatureLength);
         }
     }
 
     /**
-     * @dev Overload of {ECDSA-recover} that receives the `r` and `vs` short-signature fields separately.
+     * @dev Returns the address that signed a hashed message (`hash`) with
+     * `signature`. This address can then be used for verification purposes.
+     *
+     * The `ecrecover` EVM opcode allows for malleable (non-unique) signatures:
+     * this function rejects them by requiring the `s` value to be in the lower
+     * half order, and the `v` value to be either 27 or 28.
+     *
+     * IMPORTANT: `hash` _must_ be the result of a hash operation for the
+     * verification to be secure: it is possible to craft signatures that
+     * recover to arbitrary addresses for non-hashed data. A safe way to ensure
+     * this is by receiving a hash of the original message (which may otherwise
+     * be too long), and then calling {toEthSignedMessageHash} on it.
+     */
+    function recover(bytes32 hash, bytes memory signature) internal pure returns (address) {
+        (address recovered, RecoverError error) = tryRecover(hash, signature);
+        _throwError(error);
+        return recovered;
+    }
+
+    /**
+     * @dev Overload of {ECDSA-tryRecover} that receives the `r` and `vs` short-signature fields separately.
      *
      * See https://eips.ethereum.org/EIPS/eip-2098[EIP-2098 short signatures]
      *
-     * _Available since v4.2._
+     * _Available since v4.3._
      */
-    function recover(
+    function tryRecover(
         bytes32 hash,
         bytes32 r,
         bytes32 vs
-    ) internal pure returns (address) {
+    ) internal pure returns (address, RecoverError) {
         bytes32 s;
         uint8 v;
         assembly {
             s := and(vs, 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
             v := add(shr(255, vs), 27)
         }
-        return recover(hash, v, r, s);
+        return tryRecover(hash, v, r, s);
     }
 
     /**
-     * @dev Overload of {ECDSA-recover} that receives the `v`, `r` and `s` signature fields separately.
+     * @dev Overload of {ECDSA-recover} that receives the `r and `vs` short-signature fields separately.
+     *
+     * _Available since v4.2._
      */
     function recover(
+        bytes32 hash,
+        bytes32 r,
+        bytes32 vs
+    ) internal pure returns (address) {
+        (address recovered, RecoverError error) = tryRecover(hash, r, vs);
+        _throwError(error);
+        return recovered;
+    }
+
+    /**
+     * @dev Overload of {ECDSA-tryRecover} that receives the `v`,
+     * `r` and `s` signature fields separately.
+     *
+     * _Available since v4.3._
+     */
+    function tryRecover(
         bytes32 hash,
         uint8 v,
         bytes32 r,
         bytes32 s
-    ) internal pure returns (address) {
+    ) internal pure returns (address, RecoverError) {
         // EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature
         // unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines
         // the valid range for s in (301): 0 < s < secp256k1n ÷ 2 + 1, and for v in (302): v ∈ {27, 28}. Most
@@ -97,17 +159,35 @@ library ECDSA {
         // with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or
         // vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept
         // these malleable signatures as well.
-        require(
-            uint256(s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0,
-            "ECDSA: invalid signature 's' value"
-        );
-        require(v == 27 || v == 28, "ECDSA: invalid signature 'v' value");
+        if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
+            return (address(0), RecoverError.InvalidSignatureS);
+        }
+        if (v != 27 && v != 28) {
+            return (address(0), RecoverError.InvalidSignatureV);
+        }
 
         // If the signature is valid (and not malleable), return the signer address
         address signer = ecrecover(hash, v, r, s);
-        require(signer != address(0), "ECDSA: invalid signature");
+        if (signer == address(0)) {
+            return (address(0), RecoverError.InvalidSignature);
+        }
 
-        return signer;
+        return (signer, RecoverError.NoError);
+    }
+
+    /**
+     * @dev Overload of {ECDSA-recover} that receives the `v`,
+     * `r` and `s` signature fields separately.
+     */
+    function recover(
+        bytes32 hash,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) internal pure returns (address) {
+        (address recovered, RecoverError error) = tryRecover(hash, v, r, s);
+        _throwError(error);
+        return recovered;
     }
 
     /**

contracts/utils/cryptography/SignatureChecker.sol

@@ -22,14 +22,14 @@ library SignatureChecker {
         bytes32 hash,
         bytes memory signature
     ) internal view returns (bool) {
-        if (Address.isContract(signer)) {
-            try IERC1271(signer).isValidSignature(hash, signature) returns (bytes4 magicValue) {
-                return magicValue == IERC1271.isValidSignature.selector;
-            } catch {
-                return false;
-            }
-        } else {
-            return ECDSA.recover(hash, signature) == signer;
+        (address recovered, ECDSA.RecoverError error) = ECDSA.tryRecover(hash, signature);
+        if (error == ECDSA.RecoverError.NoError && recovered == signer) {
+            return true;
         }
+
+        (bool success, bytes memory result) = signer.staticcall(
+            abi.encodeWithSelector(IERC1271.isValidSignature.selector, hash, signature)
+        );
+        return (success && result.length == 32 && abi.decode(result, (bytes4)) == IERC1271.isValidSignature.selector);
     }
 }

test/utils/cryptography/ECDSA.test.js

@@ -10,20 +10,46 @@ const WRONG_MESSAGE = web3.utils.sha3('Nope');
 
 function to2098Format (signature) {
   const long = web3.utils.hexToBytes(signature);
-  expect(long.length).to.be.equal(65);
+  if (long.length !== 65) {
+    throw new Error('invalid signature length (expected long format)');
+  }
+  if (long[32] >> 7 === 1) {
+    throw new Error('invalid signature \'s\' value');
+  }
   const short = long.slice(0, 64);
   short[32] |= (long[64] % 27) << 7; // set the first bit of the 32nd byte to the v parity bit
   return web3.utils.bytesToHex(short);
 }
 
 function from2098Format (signature) {
   const short = web3.utils.hexToBytes(signature);
-  expect(short.length).to.be.equal(64);
+  if (short.length !== 64) {
+    throw new Error('invalid signature length (expected short format)');
+  }
   short.push((short[32] >> 7) + 27);
   short[32] &= (1 << 7) - 1; // zero out the first bit of 1 the 32nd byte
   return web3.utils.bytesToHex(short);
 }
 
+function split (signature) {
+  const raw = web3.utils.hexToBytes(signature);
+  switch (raw.length) {
+  case 64:
+    return [
+      web3.utils.bytesToHex(raw.slice(0, 32)), // r
+      web3.utils.bytesToHex(raw.slice(32, 64)), // vs
+    ];
+  case 65:
+    return [
+      raw[64], // v
+      web3.utils.bytesToHex(raw.slice(0, 32)), // r
+      web3.utils.bytesToHex(raw.slice(32, 64)), // s
+    ];
+  default:
+    expect.fail('Invalid siganture length, cannot split');
+  }
+}
+
 contract('ECDSA', function (accounts) {
   const [ other ] = accounts;
 
@@ -80,12 +106,18 @@ contract('ECDSA', function (accounts) {
         const version = '00';
         const signature = signatureWithoutVersion + version;
         await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
+        await expectRevert(
+          this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)),
+          'ECDSA: invalid signature \'v\' value',
+        );
       });
 
       it('works with 27 as version value', async function () {
         const version = '1b'; // 27 = 1b.
         const signature = signatureWithoutVersion + version;
         expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.equal(signer);
+        expect(await this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature))).to.equal(signer);
+        expect(await this.ecdsa.recover_r_vs(TEST_MESSAGE, ...split(to2098Format(signature)))).to.equal(signer);
       });
 
       it('reverts with wrong version', async function () {
@@ -94,6 +126,10 @@ contract('ECDSA', function (accounts) {
         const version = '02';
         const signature = signatureWithoutVersion + version;
         await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
+        await expectRevert(
+          this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)),
+          'ECDSA: invalid signature \'v\' value',
+        );
       });
 
       it('works with short EIP2098 format', async function () {
@@ -113,12 +149,18 @@ contract('ECDSA', function (accounts) {
         const version = '01';
         const signature = signatureWithoutVersion + version;
         await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
+        await expectRevert(
+          this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)),
+          'ECDSA: invalid signature \'v\' value',
+        );
       });
 
       it('works with 28 as version value', async function () {
         const version = '1c'; // 28 = 1c.
         const signature = signatureWithoutVersion + version;
         expect(await this.ecdsa.recover(TEST_MESSAGE, signature)).to.equal(signer);
+        expect(await this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature))).to.equal(signer);
+        expect(await this.ecdsa.recover_r_vs(TEST_MESSAGE, ...split(to2098Format(signature)))).to.equal(signer);
       });
 
       it('reverts with wrong version', async function () {
@@ -127,6 +169,10 @@ contract('ECDSA', function (accounts) {
         const version = '02';
         const signature = signatureWithoutVersion + version;
         await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
+        await expectRevert(
+          this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(signature)),
+          'ECDSA: invalid signature \'v\' value',
+        );
       });
 
       it('works with short EIP2098 format', async function () {
@@ -141,8 +187,12 @@ contract('ECDSA', function (accounts) {
       const message = '0xb94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9';
       // eslint-disable-next-line max-len
       const highSSignature = '0xe742ff452d41413616a5bf43fe15dd88294e983d3d36206c2712f39083d638bde0a0fc89be718fbc1033e1d30d78be1c68081562ed2e97af876f286f3453231d1b';
-
       await expectRevert(this.ecdsa.recover(message, highSSignature), 'ECDSA: invalid signature \'s\' value');
+      await expectRevert(
+        this.ecdsa.recover_v_r_s(TEST_MESSAGE, ...split(highSSignature)),
+        'ECDSA: invalid signature \'s\' value',
+      );
+      expect(() => to2098Format(highSSignature)).to.throw('invalid signature \'s\' value');
     });
   });