Migrate EIP712 to ethersjs (#4750)

Co-authored-by: Hadrien Croubois <[email protected]> Co-authored-by: ernestognw <[email protected]>
OpenZeppelin · Nov 23, 2023 · 6a56b3b · 6a56b3b
1 parent 9702b67
commit 6a56b3b
Show file tree

Hide file tree

Showing 9 changed files with 147 additions and 125 deletions.
diff --git a/scripts/upgradeable/upgradeable.patch b/scripts/upgradeable/upgradeable.patch
@@ -59,10 +59,10 @@ index ff596b0c3..000000000
 -<!-- Make sure that you have reviewed the OpenZeppelin Contracts Contributor Guidelines. -->
 -<!-- https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/CONTRIBUTING.md -->
 diff --git a/README.md b/README.md
-index 549891e3f..a6b24078e 100644
+index 9ca41573f..57d6e3b5b 100644
 --- a/README.md
 +++ b/README.md
-@@ -23,6 +23,9 @@
+@@ -19,6 +19,9 @@
  > [!IMPORTANT]
  > OpenZeppelin Contracts uses semantic versioning to communicate backwards compatibility of its API and storage layout. For upgradeable contracts, the storage layout of different major versions should be assumed incompatible, for example, it is unsafe to upgrade from 4.9.3 to 5.0.0. Learn more at [Backwards Compatibility](https://docs.openzeppelin.com/contracts/backwards-compatibility).
 
@@ -72,7 +72,7 @@ index 549891e3f..a6b24078e 100644
  ## Overview
 
  ### Installation
-@@ -30,7 +33,7 @@
+@@ -26,7 +29,7 @@
  #### Hardhat, Truffle (npm)
 
  ```
@@ -81,7 +81,7 @@ index 549891e3f..a6b24078e 100644
  ```
 
  #### Foundry (git)
-@@ -42,10 +45,10 @@ $ npm install @openzeppelin/contracts
+@@ -38,10 +41,10 @@ $ npm install @openzeppelin/contracts
  > Foundry installs the latest version initially, but subsequent `forge update` commands will use the `master` branch.
 
  ```
@@ -94,7 +94,7 @@ index 549891e3f..a6b24078e 100644
 
  ### Usage
 
-@@ -54,10 +57,11 @@ Once installed, you can use the contracts in the library by importing them:
+@@ -50,10 +53,11 @@ Once installed, you can use the contracts in the library by importing them:
  ```solidity
  pragma solidity ^0.8.20;
 
@@ -110,15 +110,15 @@ index 549891e3f..a6b24078e 100644
  }
  ```
 diff --git a/contracts/package.json b/contracts/package.json
-index 9017953ca..f51c1d38b 100644
+index be3e741e3..877e942c2 100644
 --- a/contracts/package.json
 +++ b/contracts/package.json
 @@ -1,5 +1,5 @@
  {
 -  "name": "@openzeppelin/contracts",
 +  "name": "@openzeppelin/contracts-upgradeable",
    "description": "Secure Smart Contract library for Solidity",
-   "version": "4.9.2",
+   "version": "5.0.0",
    "files": [
 @@ -13,7 +13,7 @@
    },
@@ -140,7 +140,7 @@ index 9017953ca..f51c1d38b 100644
 +  }
  }
 diff --git a/contracts/utils/cryptography/EIP712.sol b/contracts/utils/cryptography/EIP712.sol
-index 644f6f531..ab8ba05ff 100644
+index 8e548cdd8..a60ee74fd 100644
 --- a/contracts/utils/cryptography/EIP712.sol
 +++ b/contracts/utils/cryptography/EIP712.sol
 @@ -4,7 +4,6 @@
@@ -307,7 +307,7 @@ index 644f6f531..ab8ba05ff 100644
      }
  }
 diff --git a/package.json b/package.json
-index 3a1617c09..97e59c2d9 100644
+index c2c3a2675..3301b213d 100644
 --- a/package.json
 +++ b/package.json
 @@ -32,7 +32,7 @@
@@ -328,30 +328,31 @@ index 304d1386a..a1cd63bee 100644
 +@openzeppelin/contracts-upgradeable/=contracts/
 +@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/
 diff --git a/test/utils/cryptography/EIP712.test.js b/test/utils/cryptography/EIP712.test.js
-index faf01f1a3..b25171a56 100644
+index 75ca00b12..265e6c909 100644
 --- a/test/utils/cryptography/EIP712.test.js
 +++ b/test/utils/cryptography/EIP712.test.js
-@@ -47,26 +47,6 @@ contract('EIP712', function (accounts) {
+@@ -40,27 +40,6 @@ describe('EIP712', function () {
            const rebuildDomain = await getDomain(this.eip712);
-           expect(mapValues(rebuildDomain, String)).to.be.deep.equal(mapValues(this.domain, String));
+           expect(rebuildDomain).to.be.deep.equal(this.domain);
          });
 -
 -        if (shortOrLong === 'short') {
 -          // Long strings are in storage, and the proxy will not be properly initialized unless
 -          // the upgradeable contract variant is used and the initializer is invoked.
 -
 -          it('adjusts when behind proxy', async function () {
--            const factory = await Clones.new();
--            const cloneReceipt = await factory.$clone(this.eip712.address);
--            const cloneAddress = cloneReceipt.logs.find(({ event }) => event === 'return$clone').args.instance;
--            const clone = new EIP712Verifier(cloneAddress);
+-            const factory = await ethers.deployContract('$Clones');
 -
--            const cloneDomain = { ...this.domain, verifyingContract: clone.address };
+-            const clone = await factory
+-              .$clone(this.eip712)
+-              .then(tx => tx.wait())
+-              .then(receipt => receipt.logs.find(ev => ev.fragment.name == 'return$clone').args.instance)
+-              .then(address => ethers.getContractAt('$EIP712Verifier', address));
 -
--            const reportedDomain = await getDomain(clone);
--            expect(mapValues(reportedDomain, String)).to.be.deep.equal(mapValues(cloneDomain, String));
+-            const expectedDomain = { ...this.domain, verifyingContract: clone.target };
+-            expect(await getDomain(clone)).to.be.deep.equal(expectedDomain);
 -
--            const expectedSeparator = await domainSeparator(cloneDomain);
+-            const expectedSeparator = await domainSeparator(expectedDomain);
 -            expect(await clone.$_domainSeparatorV4()).to.equal(expectedSeparator);
 -          });
 -        }

diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js
@@ -4,7 +4,11 @@ const ethSigUtil = require('eth-sig-util');
 const Wallet = require('ethereumjs-wallet').default;
 
 const Enums = require('../helpers/enums');
-const { getDomain, domainType } = require('../helpers/eip712');
+const {
+  getDomain,
+  domainType,
+  types: { Ballot },
+} = require('../helpers/eip712');
 const { GovernorHelper, proposalStatesToBitMap } = require('../helpers/governance');
 const { clockFromReceipt } = require('../helpers/time');
 const { expectRevertCustomError } = require('../helpers/customError');
@@ -209,12 +213,7 @@ contract('Governor', function (accounts) {
               primaryType: 'Ballot',
               types: {
                 EIP712Domain: domainType(domain),
-                Ballot: [
-                  { name: 'proposalId', type: 'uint256' },
-                  { name: 'support', type: 'uint8' },
-                  { name: 'voter', type: 'address' },
-                  { name: 'nonce', type: 'uint256' },
-                ],
+                Ballot,
               },
               domain,
               message,
@@ -384,12 +383,7 @@ contract('Governor', function (accounts) {
                 primaryType: 'Ballot',
                 types: {
                   EIP712Domain: domainType(domain),
-                  Ballot: [
-                    { name: 'proposalId', type: 'uint256' },
-                    { name: 'support', type: 'uint8' },
-                    { name: 'voter', type: 'address' },
-                    { name: 'nonce', type: 'uint256' },
-                  ],
+                  Ballot,
                 },
                 domain,
                 message,

diff --git a/test/governance/extensions/GovernorWithParams.test.js b/test/governance/extensions/GovernorWithParams.test.js
@@ -4,7 +4,11 @@ const ethSigUtil = require('eth-sig-util');
 const Wallet = require('ethereumjs-wallet').default;
 
 const Enums = require('../../helpers/enums');
-const { getDomain, domainType } = require('../../helpers/eip712');
+const {
+  getDomain,
+  domainType,
+  types: { ExtendedBallot },
+} = require('../../helpers/eip712');
 const { GovernorHelper } = require('../../helpers/governance');
 const { expectRevertCustomError } = require('../../helpers/customError');
 
@@ -130,14 +134,7 @@ contract('GovernorWithParams', function (accounts) {
               primaryType: 'ExtendedBallot',
               types: {
                 EIP712Domain: domainType(domain),
-                ExtendedBallot: [
-                  { name: 'proposalId', type: 'uint256' },
-                  { name: 'support', type: 'uint8' },
-                  { name: 'voter', type: 'address' },
-                  { name: 'nonce', type: 'uint256' },
-                  { name: 'reason', type: 'string' },
-                  { name: 'params', type: 'bytes' },
-                ],
+                ExtendedBallot,
               },
               domain,
               message,

diff --git a/test/governance/utils/Votes.behavior.js b/test/governance/utils/Votes.behavior.js
@@ -7,16 +7,14 @@ const ethSigUtil = require('eth-sig-util');
 const Wallet = require('ethereumjs-wallet').default;
 
 const { shouldBehaveLikeEIP6372 } = require('./EIP6372.behavior');
-const { getDomain, domainType } = require('../../helpers/eip712');
+const {
+  getDomain,
+  domainType,
+  types: { Delegation },
+} = require('../../helpers/eip712');
 const { clockFromReceipt } = require('../../helpers/time');
 const { expectRevertCustomError } = require('../../helpers/customError');
 
-const Delegation = [
-  { name: 'delegatee', type: 'address' },
-  { name: 'nonce', type: 'uint256' },
-  { name: 'expiry', type: 'uint256' },
-];
-
 const buildAndSignDelegation = (contract, message, pk) =>
   getDomain(contract)
     .then(domain => ({

diff --git a/test/helpers/eip712-types.js b/test/helpers/eip712-types.js
@@ -0,0 +1,44 @@
+module.exports = {
+  EIP712Domain: [
+    { name: 'name', type: 'string' },
+    { name: 'version', type: 'string' },
+    { name: 'chainId', type: 'uint256' },
+    { name: 'verifyingContract', type: 'address' },
+    { name: 'salt', type: 'bytes32' },
+  ],
+  Permit: [
+    { name: 'owner', type: 'address' },
+    { name: 'spender', type: 'address' },
+    { name: 'value', type: 'uint256' },
+    { name: 'nonce', type: 'uint256' },
+    { name: 'deadline', type: 'uint256' },
+  ],
+  Ballot: [
+    { name: 'proposalId', type: 'uint256' },
+    { name: 'support', type: 'uint8' },
+    { name: 'voter', type: 'address' },
+    { name: 'nonce', type: 'uint256' },
+  ],
+  ExtendedBallot: [
+    { name: 'proposalId', type: 'uint256' },
+    { name: 'support', type: 'uint8' },
+    { name: 'voter', type: 'address' },
+    { name: 'nonce', type: 'uint256' },
+    { name: 'reason', type: 'string' },
+    { name: 'params', type: 'bytes' },
+  ],
+  Delegation: [
+    { name: 'delegatee', type: 'address' },
+    { name: 'nonce', type: 'uint256' },
+    { name: 'expiry', type: 'uint256' },
+  ],
+  ForwardRequest: [
+    { name: 'from', type: 'address' },
+    { name: 'to', type: 'address' },
+    { name: 'value', type: 'uint256' },
+    { name: 'gas', type: 'uint256' },
+    { name: 'nonce', type: 'uint256' },
+    { name: 'deadline', type: 'uint48' },
+    { name: 'data', type: 'bytes' },
+  ],
+};
diff --git a/test/helpers/eip712.js b/test/helpers/eip712.js
@@ -1,20 +1,5 @@
 const { ethers } = require('ethers');
-
-const EIP712Domain = [
-  { name: 'name', type: 'string' },
-  { name: 'version', type: 'string' },
-  { name: 'chainId', type: 'uint256' },
-  { name: 'verifyingContract', type: 'address' },
-  { name: 'salt', type: 'bytes32' },
-];
-
-const Permit = [
-  { name: 'owner', type: 'address' },
-  { name: 'spender', type: 'address' },
-  { name: 'value', type: 'uint256' },
-  { name: 'nonce', type: 'uint256' },
-  { name: 'deadline', type: 'uint256' },
-];
+const types = require('./eip712-types');
 
 async function getDomain(contract) {
   const { fields, name, version, chainId, verifyingContract, salt, extensions } = await contract.eip712Domain();
@@ -32,7 +17,7 @@ async function getDomain(contract) {
     salt,
   };
 
-  for (const [i, { name }] of EIP712Domain.entries()) {
+  for (const [i, { name }] of types.EIP712Domain.entries()) {
     if (!(fields & (1 << i))) {
       delete domain[name];
     }
@@ -42,7 +27,7 @@ async function getDomain(contract) {
 }
 
 function domainType(domain) {
-  return EIP712Domain.filter(({ name }) => domain[name] !== undefined);
+  return types.EIP712Domain.filter(({ name }) => domain[name] !== undefined);
 }
 
 function hashTypedData(domain, structHash) {
@@ -53,7 +38,7 @@ function hashTypedData(domain, structHash) {
 }
 
 module.exports = {
-  Permit,
+  types,
   getDomain,
   domainType,
   domainSeparator: ethers.TypedDataEncoder.hashDomain,

diff --git a/test/token/ERC20/extensions/ERC20Permit.test.js b/test/token/ERC20/extensions/ERC20Permit.test.js
@@ -10,7 +10,12 @@ const Wallet = require('ethereumjs-wallet').default;
 
 const ERC20Permit = artifacts.require('$ERC20Permit');
 
-const { Permit, getDomain, domainType, domainSeparator } = require('../../../helpers/eip712');
+const {
+  types: { Permit },
+  getDomain,
+  domainType,
+  domainSeparator,
+} = require('../../../helpers/eip712');
 const { getChainId } = require('../../../helpers/chainid');
 const { expectRevertCustomError } = require('../../../helpers/customError');
 

diff --git a/test/token/ERC20/extensions/ERC20Votes.test.js b/test/token/ERC20/extensions/ERC20Votes.test.js
@@ -10,16 +10,14 @@ const ethSigUtil = require('eth-sig-util');
 const Wallet = require('ethereumjs-wallet').default;
 
 const { batchInBlock } = require('../../../helpers/txpool');
-const { getDomain, domainType } = require('../../../helpers/eip712');
+const {
+  getDomain,
+  domainType,
+  types: { Delegation },
+} = require('../../../helpers/eip712');
 const { clock, clockFromReceipt } = require('../../../helpers/time');
 const { expectRevertCustomError } = require('../../../helpers/customError');
 
-const Delegation = [
-  { name: 'delegatee', type: 'address' },
-  { name: 'nonce', type: 'uint256' },
-  { name: 'expiry', type: 'uint256' },
-];
-
 const MODES = {
   blocknumber: artifacts.require('$ERC20Votes'),
   timestamp: artifacts.require('$ERC20VotesTimestampMock'),