#890: Add ECDSA.toEthSignedMessageHash(bytes) for abritrary length me…

…ssage hashing (#2865) * #890: Add ECDSA#toEthSignedMessage for bytes type * refactor * add test, refactor * select overloaded function explicitly * use short test message string * add changelog entry Co-authored-by: Francisco Giordano <frangio.1@gmail.com> Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
OpenZeppelin · Oct 11, 2021 · 5b28259 · 5b28259
1 parent efb5b0a
commit 5b28259
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
  * Add internal `_setApprovalForAll` to `ERC721` and `ERC1155`. ([#2834](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2834))
  * `Governor`: shift vote start and end by one block to better match Compound's GovernorBravo and prevent voting at the Governor level if the voting snapshot is not ready. ([#2892](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2892))
  * `PaymentSplitter`: now supports ERC20 assets in addition to Ether. ([#2858](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2858))
+ * `ECDSA`: add a variant of `toEthSignedMessageHash` for arbitrary length message hashing. ([#2865](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2865))
 
 ## 4.3.2 (2021-09-14)
 

diff --git a/contracts/mocks/ECDSAMock.sol b/contracts/mocks/ECDSAMock.sol
@@ -6,6 +6,7 @@ import "../utils/cryptography/ECDSA.sol";
 
 contract ECDSAMock {
     using ECDSA for bytes32;
+    using ECDSA for bytes;
 
     function recover(bytes32 hash, bytes memory signature) public pure returns (address) {
         return hash.recover(signature);
@@ -33,4 +34,8 @@ contract ECDSAMock {
     function toEthSignedMessageHash(bytes32 hash) public pure returns (bytes32) {
         return hash.toEthSignedMessageHash();
     }
+
+    function toEthSignedMessageHash(bytes memory s) public pure returns (bytes32) {
+        return s.toEthSignedMessageHash();
+    }
 }
diff --git a/contracts/utils/cryptography/ECDSA.sol b/contracts/utils/cryptography/ECDSA.sol
@@ -2,6 +2,8 @@
 
 pragma solidity ^0.8.0;
 
+import "../Strings.sol";
+
 /**
  * @dev Elliptic Curve Digital Signature Algorithm (ECDSA) operations.
  *
@@ -204,6 +206,18 @@ library ECDSA {
         return keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", hash));
     }
 
+    /**
+     * @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}.
+     */
+    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

diff --git a/test/utils/cryptography/ECDSA.test.js b/test/utils/cryptography/ECDSA.test.js
@@ -7,6 +7,7 @@ const ECDSAMock = artifacts.require('ECDSAMock');
 
 const TEST_MESSAGE = web3.utils.sha3('OpenZeppelin');
 const WRONG_MESSAGE = web3.utils.sha3('Nope');
+const NON_HASH_MESSAGE = '0x' + Buffer.from('abcd').toString('hex');
 
 function to2098Format (signature) {
   const long = web3.utils.hexToBytes(signature);
@@ -84,6 +85,17 @@ contract('ECDSA', function (accounts) {
         )).to.equal(other);
       });
 
+      it('returns signer address with correct signature for arbitrary length message', async function () {
+        // Create the signature
+        const signature = await web3.eth.sign(NON_HASH_MESSAGE, other);
+
+        // Recover the signer address from the generated message and signature.
+        expect(await this.ecdsa.recover(
+          toEthSignedMessageHash(NON_HASH_MESSAGE),
+          signature,
+        )).to.equal(other);
+      });
+
       it('returns a different address', async function () {
         const signature = await web3.eth.sign(TEST_MESSAGE, other);
         expect(await this.ecdsa.recover(WRONG_MESSAGE, signature)).to.not.equal(other);
@@ -196,9 +208,15 @@ contract('ECDSA', function (accounts) {
     });
   });
 
-  context('toEthSignedMessage', function () {
-    it('prefixes hashes correctly', async function () {
-      expect(await this.ecdsa.toEthSignedMessageHash(TEST_MESSAGE)).to.equal(toEthSignedMessageHash(TEST_MESSAGE));
+  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));
     });
   });
 });