Merge upstream openzeppelin-contracts into upstream-patched

.github/workflows/test.yml

@@ -29,6 +29,6 @@ jobs:
       - run: npm run test
         env:
           FORCE_COLOR: 1
-          ENABLE_GAS_REPORT: 1
+          ENABLE_GAS_REPORT: true
       - name: Print gas report
         run: cat gas-report.txt

CHANGELOG.md

@@ -8,6 +8,7 @@
  * `ERC20FlashMint`: add an implementation of the ERC3156 extension for flash-minting ERC20 tokens. ([#2543](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2543))
  * `SignatureChecker`: add a signature verification library that supports both EOA and ERC1271 compliant contracts as signers. ([#2532](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2532))
  * `Multicall`: add abstract contract with `multicall(bytes[] calldata data)` function to bundle multiple calls together ([#2608](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2608))
+ * `ECDSA`: add support for ERC2098 short-signatures. ([#2582](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2582))
 
 ## 4.0.0 (2021-03-23)
 

contracts/utils/cryptography/ECDSA.sol

@@ -24,16 +24,15 @@ library ECDSA {
      * be too long), and then calling {toEthSignedMessageHash} on it.
      */
     function recover(bytes32 hash, bytes memory signature) internal pure returns (address) {
-        // Check the signature length
-        if (signature.length != 65) {
-            revert("ECDSA: invalid signature length");
-        }
-
         // Divide the signature in r, s and v variables
         bytes32 r;
         bytes32 s;
         uint8 v;
 
+        // Check the signature length
+        // - case 65: r,s,v signature (standard)
+        // - case 64: r,vs signature (cf https://eips.ethereum.org/EIPS/eip-2098)
+        if (signature.length == 65) {
             // ecrecover takes the signature parameters, and the only way to get them
             // currently is to use assembly.
             // solhint-disable-next-line no-inline-assembly
@@ -42,6 +41,19 @@ library ECDSA {
                 s := mload(add(signature, 0x40))
                 v := byte(0, mload(add(signature, 0x60)))
             }
+        } else if (signature.length == 64) {
+            // ecrecover takes the signature parameters, and the only way to get them
+            // currently is to use assembly.
+            // solhint-disable-next-line no-inline-assembly
+            assembly {
+                let vs := mload(add(signature, 0x40))
+                r := mload(add(signature, 0x20))
+                s := and(vs, 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
+                v := add(shr(255, vs), 27)
+            }
+        } else {
+            revert("ECDSA: invalid signature length");
+        }
 
         return recover(hash, v, r, s);
     }

hardhat.config.js

+/// ENVVAR
+// - ENABLE_GAS_REPORT
+// - CI
+// - COMPILE_MODE
+
 const fs = require('fs');
 const path = require('path');
+const argv = require('yargs/yargs')()
+  .env('')
+  .boolean('enableGasReport')
+  .boolean('ci')
+  .string('compileMode')
+  .argv;
 
 require('@nomiclabs/hardhat-truffle5');
 require('@nomiclabs/hardhat-solhint');
 require('solidity-coverage');
-require('hardhat-gas-reporter');
+
+if (argv.enableGasReport) {
+  require('hardhat-gas-reporter');
+}
 
 for (const f of fs.readdirSync(path.join(__dirname, 'hardhat'))) {
   require(path.join(__dirname, 'hardhat', f));
 }
 
-const enableGasReport = !!process.env.ENABLE_GAS_REPORT;
-const enableProduction = process.env.COMPILE_MODE === 'production';
-
 /**
  * @type import('hardhat/config').HardhatUserConfig
  */
 module.exports = {
   solidity: {
-    version: '0.8.0',
+    version: '0.8.3',
     settings: {
       optimizer: {
-        enabled: enableGasReport || enableProduction,
+        enabled: argv.enableGasReport || argv.compileMode === 'production',
         runs: 200,
       },
     },
@@ -32,8 +43,7 @@ module.exports = {
     },
   },
   gasReporter: {
-    enable: enableGasReport,
     currency: 'USD',
-    outputFile: process.env.CI ? 'gas-report.txt' : undefined,
+    outputFile: argv.ci ? 'gas-report.txt' : undefined,
   },
 };

package.json

@@ -28,7 +28,7 @@
     "release": "scripts/release/release.sh",
     "version": "scripts/release/version.sh",
     "test": "hardhat test",
-    "gas-report": "env ENABLE_GAS_REPORT=1 npm run test"
+    "gas-report": "env ENABLE_GAS_REPORT=true npm run test"
   },
   "repository": {
     "type": "git",
@@ -77,6 +77,7 @@
     "solhint": "^3.2.0",
     "solidity-coverage": "^0.7.11",
     "solidity-docgen": "^0.5.3",
-    "web3": "^1.3.0"
+    "web3": "^1.3.0",
+    "yargs": "^16.2.0"
   }
 }

test/utils/cryptography/ECDSA.test.js

@@ -8,6 +8,22 @@ const ECDSAMock = artifacts.require('ECDSAMock');
 const TEST_MESSAGE = web3.utils.sha3('OpenZeppelin');
 const WRONG_MESSAGE = web3.utils.sha3('Nope');
 
+function to2098Format (signature) {
+  const long = web3.utils.hexToBytes(signature);
+  expect(long.length).to.be.equal(65);
+  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);
+  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);
+}
+
 contract('ECDSA', function (accounts) {
   const [ other ] = accounts;
 
@@ -36,30 +52,31 @@ contract('ECDSA', function (accounts) {
       // eslint-disable-next-line max-len
       const signatureWithoutVersion = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be892';
 
-      context('with 00 as version value', function () {
-        it('reverts', async function () {
+      it('reverts with 00 as version value', async function () {
         const version = '00';
         const signature = signatureWithoutVersion + version;
         await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
       });
-      });
 
-      context('with 27 as version value', function () {
-        it('works', async function () {
+      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);
       });
-      });
 
-      context('with wrong version', function () {
-        it('reverts', async function () {
+      it('reverts with wrong version', async function () {
         // The last two hex digits are the signature version.
         // The only valid values are 0, 1, 27 and 28.
         const version = '02';
         const signature = signatureWithoutVersion + version;
         await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
       });
+
+      it('works with short EIP2098 format', async function () {
+        const version = '1b'; // 27 = 1b.
+        const signature = signatureWithoutVersion + version;
+        expect(await this.ecdsa.recover(TEST_MESSAGE, to2098Format(signature))).to.equal(signer);
+        expect(await this.ecdsa.recover(TEST_MESSAGE, from2098Format(to2098Format(signature)))).to.equal(signer);
       });
     });
 
@@ -68,46 +85,44 @@ contract('ECDSA', function (accounts) {
       // eslint-disable-next-line max-len
       const signatureWithoutVersion = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e0';
 
-      context('with 01 as version value', function () {
-        it('reverts', async function () {
+      it('reverts with 01 as version value', async function () {
         const version = '01';
         const signature = signatureWithoutVersion + version;
         await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
       });
-      });
 
-      context('with 28 as version value', function () {
-        it('works', async function () {
+      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);
       });
-      });
 
-      context('with wrong version', function () {
-        it('reverts', async function () {
+      it('reverts with wrong version', async function () {
         // The last two hex digits are the signature version.
         // The only valid values are 0, 1, 27 and 28.
         const version = '02';
         const signature = signatureWithoutVersion + version;
         await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature \'v\' value');
       });
+
+      it('works with short EIP2098 format', async function () {
+        const version = '1c'; // 27 = 1b.
+        const signature = signatureWithoutVersion + version;
+        expect(await this.ecdsa.recover(TEST_MESSAGE, to2098Format(signature))).to.equal(signer);
+        expect(await this.ecdsa.recover(TEST_MESSAGE, from2098Format(to2098Format(signature)))).to.equal(signer);
       });
     });
 
-    context('with high-s value signature', function () {
-      it('reverts', async function () {
+    it('reverts with high-s value signature', async function () {
       const message = '0xb94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9';
       // eslint-disable-next-line max-len
       const highSSignature = '0xe742ff452d41413616a5bf43fe15dd88294e983d3d36206c2712f39083d638bde0a0fc89be718fbc1033e1d30d78be1c68081562ed2e97af876f286f3453231d1b';
 
       await expectRevert(this.ecdsa.recover(message, highSSignature), 'ECDSA: invalid signature \'s\' value');
     });
-    });
 
     context('using web3.eth.sign', function () {
-      context('with correct signature', function () {
-        it('returns signer address', async function () {
+      it('returns signer address with correct signature', async function () {
         // Create the signature
         const signature = fixSignature(await web3.eth.sign(TEST_MESSAGE, other));
 
@@ -117,27 +132,22 @@ contract('ECDSA', function (accounts) {
           signature,
         )).to.equal(other);
       });
-      });
 
-      context('with wrong message', function () {
       it('returns a different address', async function () {
         const signature = fixSignature(await web3.eth.sign(TEST_MESSAGE, other));
         expect(await this.ecdsa.recover(WRONG_MESSAGE, signature)).to.not.equal(other);
       });
-      });
 
-      context('with invalid signature', function () {
-        it('reverts', async function () {
+      it('reverts with invalid signature', async function () {
         // eslint-disable-next-line max-len
         const signature = '0x332ce75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e01c';
         await expectRevert(this.ecdsa.recover(TEST_MESSAGE, signature), 'ECDSA: invalid signature');
       });
     });
   });
-  });
 
   context('toEthSignedMessage', function () {
-    it('should prefix hashes correctly', async function () {
+    it('prefixes hashes correctly', async function () {
       expect(await this.ecdsa.toEthSignedMessageHash(TEST_MESSAGE)).to.equal(toEthSignedMessageHash(TEST_MESSAGE));
     });
   });