Move ECDSA message hash methods to its own MessageHashUtils libra…

…ry (#4430) Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com> Co-authored-by: Francisco <fg@frang.io>
OpenZeppelin · Jul 7, 2023 · 0053ee0 · 0053ee0
1 parent 996168f
commit 0053ee0
Show file tree

Hide file tree

Showing 9 changed files with 179 additions and 111 deletions.
diff --git a/.changeset/hot-dingos-kiss.md b/.changeset/hot-dingos-kiss.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`MessageHashUtils`: Add a new library for creating message digest to be used along with signing or recovery such as  ECDSA or ERC-1271. These functions are moved from the `ECDSA` library.
diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc
@@ -34,6 +34,8 @@ Finally, {Create2} contains all necessary utilities to safely use the https://bl
 
 {{ECDSA}}
 
+{{MessageHashUtils}}
+
 {{SignatureChecker}}
 
 {{MerkleProof}}

diff --git a/contracts/utils/cryptography/ECDSA.sol b/contracts/utils/cryptography/ECDSA.sol
@@ -3,8 +3,6 @@
 
 pragma solidity ^0.8.19;
 
-import {Strings} from "../Strings.sol";
-
 /**
  * @dev Elliptic Curve Digital Signature Algorithm (ECDSA) operations.
  *
@@ -34,18 +32,6 @@ library ECDSA {
      */
     error ECDSAInvalidSignatureS(bytes32 s);
 
-    function _throwError(RecoverError error, bytes32 errorArg) private pure {
-        if (error == RecoverError.NoError) {
-            return; // no error: do nothing
-        } else if (error == RecoverError.InvalidSignature) {
-            revert ECDSAInvalidSignature();
-        } else if (error == RecoverError.InvalidSignatureLength) {
-            revert ECDSAInvalidSignatureLength(uint256(errorArg));
-        } else if (error == RecoverError.InvalidSignatureS) {
-            revert ECDSAInvalidSignatureS(errorArg);
-        }
-    }
-
     /**
      * @dev Returns the address that signed a hashed message (`hash`) with
      * `signature` or error string. This address can then be used for verification purposes.
@@ -58,7 +44,7 @@ library ECDSA {
      * 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.
+     * be too long), and then calling {MessageHashUtils-toEthSignedMessageHash} on it.
      *
      * Documentation for signature generation:
      * - with https://web3js.readthedocs.io/en/v1.3.4/web3-eth-accounts.html#sign[Web3.js]
@@ -95,7 +81,7 @@ library ECDSA {
      * 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.
+     * be too long), and then calling {MessageHashUtils-toEthSignedMessageHash} on it.
      */
     function recover(bytes32 hash, bytes memory signature) internal pure returns (address) {
         (address recovered, RecoverError error, bytes32 errorArg) = tryRecover(hash, signature);
@@ -169,63 +155,17 @@ library ECDSA {
     }
 
     /**
-     * @dev Returns an Ethereum Signed Message, created from a `hash`. This
-     * produces hash corresponding to the one signed with the
-     * https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`]
-     * JSON-RPC method as part of EIP-191.
-     *
-     * See {recover}.
-     */
-    function toEthSignedMessageHash(bytes32 hash) internal pure returns (bytes32 message) {
-        // 32 is the length in bytes of hash,
-        // enforced by the type signature above
-        /// @solidity memory-safe-assembly
-        assembly {
-            mstore(0x00, "\x19Ethereum Signed Message:\n32")
-            mstore(0x1c, hash)
-            message := keccak256(0x00, 0x3c)
-        }
-    }
-
-    /**
-     * @dev Returns an Ethereum Signed Message, created from `s`. This
-     * produces hash corresponding to the one signed with the
-     * https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`]
-     * JSON-RPC method as part of EIP-191.
-     *
-     * See {recover}.
+     * @dev Optionally reverts with the corresponding custom error according to the `error` argument provided.
      */
-    function toEthSignedMessageHash(bytes memory s) internal pure returns (bytes32) {
-        return keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n", Strings.toString(s.length), s));
-    }
-
-    /**
-     * @dev Returns an Ethereum Signed Typed Data, created from a
-     * `domainSeparator` and a `structHash`. This produces hash corresponding
-     * to the one signed with the
-     * https://eips.ethereum.org/EIPS/eip-712[`eth_signTypedData`]
-     * JSON-RPC method as part of EIP-712.
-     *
-     * See {recover}.
-     */
-    function toTypedDataHash(bytes32 domainSeparator, bytes32 structHash) internal pure returns (bytes32 data) {
-        /// @solidity memory-safe-assembly
-        assembly {
-            let ptr := mload(0x40)
-            mstore(ptr, hex"19_01")
-            mstore(add(ptr, 0x02), domainSeparator)
-            mstore(add(ptr, 0x22), structHash)
-            data := keccak256(ptr, 0x42)
+    function _throwError(RecoverError error, bytes32 errorArg) private pure {
+        if (error == RecoverError.NoError) {
+            return; // no error: do nothing
+        } else if (error == RecoverError.InvalidSignature) {
+            revert ECDSAInvalidSignature();
+        } else if (error == RecoverError.InvalidSignatureLength) {
+            revert ECDSAInvalidSignatureLength(uint256(errorArg));
+        } else if (error == RecoverError.InvalidSignatureS) {
+            revert ECDSAInvalidSignatureS(errorArg);
         }
     }
-
-    /**
-     * @dev Returns an Ethereum Signed Data with intended validator, created from a
-     * `validator` and `data` according to the version 0 of EIP-191.
-     *
-     * See {recover}.
-     */
-    function toDataWithIntendedValidatorHash(address validator, bytes memory data) internal pure returns (bytes32) {
-        return keccak256(abi.encodePacked(hex"19_00", validator, data));
-    }
 }
diff --git a/contracts/utils/cryptography/EIP712.sol b/contracts/utils/cryptography/EIP712.sol
@@ -3,16 +3,17 @@
 
 pragma solidity ^0.8.19;
 
-import {ECDSA} from "./ECDSA.sol";
+import {MessageHashUtils} from "./MessageHashUtils.sol";
 import {ShortStrings, ShortString} from "../ShortStrings.sol";
 import {IERC5267} from "../../interfaces/IERC5267.sol";
 
 /**
  * @dev https://eips.ethereum.org/EIPS/eip-712[EIP 712] is a standard for hashing and signing of typed structured data.
  *
- * The encoding specified in the EIP is very generic, and such a generic implementation in Solidity is not feasible,
- * thus this contract does not implement the encoding itself. Protocols need to implement the type-specific encoding
- * they need in their contracts using a combination of `abi.encode` and `keccak256`.
+ * The encoding scheme specified in the EIP requires a domain separator and a hash of the typed structured data, whose
+ * encoding is very generic and therefore its implementation in Solidity is not feasible, thus this contract
+ * does not implement the encoding itself. Protocols need to implement the type-specific encoding they need in order to
+ * produce the hash of their typed data using a combination of `abi.encode` and `keccak256`.
  *
  * This contract implements the EIP 712 domain separator ({_domainSeparatorV4}) that is used as part of the encoding
  * scheme, and the final step of the encoding to obtain the message digest that is then signed via ECDSA
@@ -25,7 +26,7 @@ import {IERC5267} from "../../interfaces/IERC5267.sol";
  * https://docs.metamask.io/guide/signing-data.html[`eth_signTypedDataV4` in MetaMask].
  *
  * NOTE: In the upgradeable version of this contract, the cached values will correspond to the address, and the domain
- * separator of the implementation contract. This will cause the `_domainSeparatorV4` function to always rebuild the
+ * separator of the implementation contract. This will cause the {_domainSeparatorV4} function to always rebuild the
  * separator from the immutable values, which is cheaper than accessing a cached version in cold storage.
  *
  * @custom:oz-upgrades-unsafe-allow state-variable-immutable state-variable-assignment
@@ -104,7 +105,7 @@ abstract contract EIP712 is IERC5267 {
      * ```
      */
     function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) {
-        return ECDSA.toTypedDataHash(_domainSeparatorV4(), structHash);
+        return MessageHashUtils.toTypedDataHash(_domainSeparatorV4(), structHash);
     }
 
     /**

diff --git a/contracts/utils/cryptography/MessageHashUtils.sol b/contracts/utils/cryptography/MessageHashUtils.sol
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.19;
+
+import {Strings} from "../Strings.sol";
+
+/**
+ * @dev Signature message hash utilities for producing digests to be consumed by {ECDSA} recovery or signing.
+ *
+ * The library provides methods for generating a hash of a message that conforms to the
+ * https://eips.ethereum.org/EIPS/eip-191[EIP 191] and https://eips.ethereum.org/EIPS/eip-712[EIP 712]
+ * specifications.
+ */
+library MessageHashUtils {
+    /**
+     * @dev Returns the keccak256 digest of an EIP-191 signed data with version
+     * `0x45` (`personal_sign` messages).
+     *
+     * The digest is calculated by prefixing a bytes32 `messageHash` with
+     * `"\x19Ethereum Signed Message:\n32"` and hashing the result. It corresponds with the
+     * hash signed when using the https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`] JSON-RPC method.
+     *
+     * NOTE: The `hash` parameter is intended to be the result of hashing a raw message with
+     * keccak256, althoguh any bytes32 value can be safely used because the final digest will
+     * be re-hashed.
+     *
+     * See {ECDSA-recover}.
+     */
+    function toEthSignedMessageHash(bytes32 messageHash) internal pure returns (bytes32 digest) {
+        /// @solidity memory-safe-assembly
+        assembly {
+            mstore(0x00, "\x19Ethereum Signed Message:\n32") // 32 is the bytes-length of messageHash
+            mstore(0x1c, messageHash) // 0x1c (28) is the length of the prefix
+            digest := keccak256(0x00, 0x3c) // 0x3c is the length of the prefix (0x1c) + messageHash (0x20)
+        }
+    }
+
+    /**
+     * @dev Returns the keccak256 digest of an EIP-191 signed data with version
+     * `0x45` (`personal_sign` messages).
+     *
+     * The digest is calculated by prefixing an arbitrary `message` with
+     * `"\x19Ethereum Signed Message:\n" + len(message)` and hashing the result. It corresponds with the
+     * hash signed when using the https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`] JSON-RPC method.
+     *
+     * See {ECDSA-recover}.
+     */
+    function toEthSignedMessageHash(bytes memory message) internal pure returns (bytes32 digest) {
+        return keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n", Strings.toString(message.length), message));
+    }
+
+    /**
+     * @dev Returns the keccak256 digest of an EIP-191 signed data with version
+     * `0x00` (data with intended validator).
+     *
+     * The digest is calculated by prefixing an arbitrary `data` with `"\x19\x00"` and the intended
+     * `validator` address. Then hashing the result.
+     *
+     * See {ECDSA-recover}.
+     */
+    function toDataWithIntendedValidatorHash(
+        address validator,
+        bytes memory data
+    ) internal pure returns (bytes32 digest) {
+        return keccak256(abi.encodePacked(hex"19_00", validator, data));
+    }
+
+    /**
+     * @dev Returns the keccak256 digest of an EIP-712 typed data (EIP-191 version `0x01`).
+     *
+     * The digest is calculated from a `domainSeparator` and a `structHash`, by prefixing them with
+     * `\x19\x01` and hashing the result. It corresponds to the hash signed by the
+     * https://eips.ethereum.org/EIPS/eip-712[`eth_signTypedData`] JSON-RPC method as part of EIP-712.
+     *
+     * See {ECDSA-recover}.
+     */
+    function toTypedDataHash(bytes32 domainSeparator, bytes32 structHash) internal pure returns (bytes32 digest) {
+        /// @solidity memory-safe-assembly
+        assembly {
+            let ptr := mload(0x40)
+            mstore(ptr, hex"19_01")
+            mstore(add(ptr, 0x02), domainSeparator)
+            mstore(add(ptr, 0x22), structHash)
+            digest := keccak256(ptr, 0x42)
+        }
+    }
+}
diff --git a/docs/modules/ROOT/pages/utilities.adoc b/docs/modules/ROOT/pages/utilities.adoc
@@ -9,11 +9,12 @@ The OpenZeppelin Contracts provide a ton of useful utilities that you can use in
 
 xref:api:utils.adoc#ECDSA[`ECDSA`] provides functions for recovering and managing Ethereum account ECDSA signatures. These are often generated via https://web3js.readthedocs.io/en/v1.7.3/web3-eth.html#sign[`web3.eth.sign`], and are a 65 byte array (of type `bytes` in Solidity) arranged the following way: `[[v (1)], [r (32)], [s (32)]]`.
 
-The data signer can be recovered with xref:api:utils.adoc#ECDSA-recover-bytes32-bytes-[`ECDSA.recover`], and its address compared to verify the signature. Most wallets will hash the data to sign and add the prefix '\x19Ethereum Signed Message:\n', so when attempting to recover the signer of an Ethereum signed message hash, you'll want to use xref:api:utils.adoc#ECDSA-toEthSignedMessageHash-bytes32-[`toEthSignedMessageHash`].
+The data signer can be recovered with xref:api:utils.adoc#ECDSA-recover-bytes32-bytes-[`ECDSA.recover`], and its address compared to verify the signature. Most wallets will hash the data to sign and add the prefix '\x19Ethereum Signed Message:\n', so when attempting to recover the signer of an Ethereum signed message hash, you'll want to use xref:api:utils.adoc#MessageHashUtils-toEthSignedMessageHash-bytes32-[`toEthSignedMessageHash`].
 
 [source,solidity]
 ----
 using ECDSA for bytes32;
+using MessageHashUtils for bytes32;
 
 function _verify(bytes32 data, bytes memory signature, address account) internal pure returns (bool) {
     return data
@@ -22,7 +23,7 @@ function _verify(bytes32 data, bytes memory signature, address account) internal
 }
 ----
 
-WARNING: Getting signature verification right is not trivial: make sure you fully read and understand xref:api:utils.adoc#ECDSA[`ECDSA`]'s documentation.
+WARNING: Getting signature verification right is not trivial: make sure you fully read and understand xref:api:utils.adoc#MessageHashUtils[`MessageHashUtils`]'s and xref:api:utils.adoc#ECDSA[`ECDSA`]'s documentation.
 
 === Verifying Merkle Proofs
 

diff --git a/scripts/upgradeable/upgradeable.patch b/scripts/upgradeable/upgradeable.patch
@@ -126,20 +126,20 @@ index df141192..1cf90ad1 100644
    "keywords": [
      "solidity",
 diff --git a/contracts/utils/cryptography/EIP712.sol b/contracts/utils/cryptography/EIP712.sol
-index ff34e814..a9d08d5c 100644
+index 36f076e5..90c1db78 100644
 --- a/contracts/utils/cryptography/EIP712.sol
 +++ b/contracts/utils/cryptography/EIP712.sol
 @@ -4,7 +4,6 @@
  pragma solidity ^0.8.19;
 
- import {ECDSA} from "./ECDSA.sol";
+ import {MessageHashUtils} from "./MessageHashUtils.sol";
 -import {ShortStrings, ShortString} from "../ShortStrings.sol";
  import {IERC5267} from "../../interfaces/IERC5267.sol";
 
  /**
-@@ -27,28 +26,18 @@ import {IERC5267} from "../../interfaces/IERC5267.sol";
+@@ -28,28 +27,18 @@ import {IERC5267} from "../../interfaces/IERC5267.sol";
   * NOTE: In the upgradeable version of this contract, the cached values will correspond to the address, and the domain
-  * separator of the implementation contract. This will cause the `_domainSeparatorV4` function to always rebuild the
+  * separator of the implementation contract. This will cause the {_domainSeparatorV4} function to always rebuild the
   * separator from the immutable values, which is cheaper than accessing a cached version in cold storage.
 - *
 - * @custom:oz-upgrades-unsafe-allow state-variable-immutable state-variable-assignment
@@ -170,7 +170,7 @@ index ff34e814..a9d08d5c 100644
 
      /**
       * @dev Initializes the domain separator and parameter caches.
-@@ -63,29 +52,23 @@ abstract contract EIP712 is IERC5267 {
+@@ -64,29 +53,23 @@ abstract contract EIP712 is IERC5267 {
       * contract upgrade].
       */
      constructor(string memory name, string memory version) {
@@ -208,7 +208,7 @@ index ff34e814..a9d08d5c 100644
      }
 
      /**
-@@ -124,6 +107,10 @@ abstract contract EIP712 is IERC5267 {
+@@ -125,6 +108,10 @@ abstract contract EIP712 is IERC5267 {
              uint256[] memory extensions
          )
      {
@@ -219,7 +219,7 @@ index ff34e814..a9d08d5c 100644
          return (
              hex"0f", // 01111
              _EIP712Name(),
-@@ -138,22 +125,62 @@ abstract contract EIP712 is IERC5267 {
+@@ -139,22 +126,62 @@ abstract contract EIP712 is IERC5267 {
      /**
       * @dev The name parameter for the EIP712 domain.
       *

diff --git a/test/utils/cryptography/ECDSA.test.js b/test/utils/cryptography/ECDSA.test.js
@@ -1,6 +1,6 @@
 require('@openzeppelin/test-helpers');
 const { expectRevertCustomError } = require('../../helpers/customError');
-const { toEthSignedMessageHash, toDataWithIntendedValidatorHash } = require('../../helpers/sign');
+const { toEthSignedMessageHash } = require('../../helpers/sign');
 
 const { expect } = require('chai');
 
@@ -9,7 +9,6 @@ const ECDSA = artifacts.require('$ECDSA');
 const TEST_MESSAGE = web3.utils.sha3('OpenZeppelin');
 const WRONG_MESSAGE = web3.utils.sha3('Nope');
 const NON_HASH_MESSAGE = '0x' + Buffer.from('abcd').toString('hex');
-const RANDOM_ADDRESS = web3.utils.toChecksumAddress(web3.utils.randomHex(20));
 
 function to2098Format(signature) {
   const long = web3.utils.hexToBytes(signature);
@@ -243,26 +242,4 @@ contract('ECDSA', function (accounts) {
       expect(() => to2098Format(highSSignature)).to.throw("invalid signature 's' value");
     });
   });
-
-  context('toEthSignedMessageHash', function () {
-    it('prefixes bytes32 data correctly', async function () {
-      expect(await this.ecdsa.methods['$toEthSignedMessageHash(bytes32)'](TEST_MESSAGE)).to.equal(
-        toEthSignedMessageHash(TEST_MESSAGE),
-      );
-    });
-
-    it('prefixes dynamic length data correctly', async function () {
-      expect(await this.ecdsa.methods['$toEthSignedMessageHash(bytes)'](NON_HASH_MESSAGE)).to.equal(
-        toEthSignedMessageHash(NON_HASH_MESSAGE),
-      );
-    });
-  });
-
-  context('toDataWithIntendedValidatorHash', function () {
-    it('returns the hash correctly', async function () {
-      expect(
-        await this.ecdsa.methods['$toDataWithIntendedValidatorHash(address,bytes)'](RANDOM_ADDRESS, NON_HASH_MESSAGE),
-      ).to.equal(toDataWithIntendedValidatorHash(RANDOM_ADDRESS, NON_HASH_MESSAGE));
-    });
-  });
 });