Migrate EIP712 to ethersjs (#4750)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: ernestognw <ernestognw@gmail.com>
pull/4730/head^2
Renan Souza 1 year ago committed by GitHub
parent 9702b67ce1
commit 6a56b3b08d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 41
      scripts/upgradeable/upgradeable.patch
  2. 20
      test/governance/Governor.test.js
  3. 15
      test/governance/extensions/GovernorWithParams.test.js
  4. 12
      test/governance/utils/Votes.behavior.js
  5. 44
      test/helpers/eip712-types.js
  6. 23
      test/helpers/eip712.js
  7. 7
      test/token/ERC20/extensions/ERC20Permit.test.js
  8. 12
      test/token/ERC20/extensions/ERC20Votes.test.js
  9. 98
      test/utils/cryptography/EIP712.test.js

@ -59,10 +59,10 @@ index ff596b0c3..000000000
-<!-- Make sure that you have reviewed the OpenZeppelin Contracts Contributor Guidelines. --> -<!-- Make sure that you have reviewed the OpenZeppelin Contracts Contributor Guidelines. -->
-<!-- https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/CONTRIBUTING.md --> -<!-- https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/CONTRIBUTING.md -->
diff --git a/README.md b/README.md diff --git a/README.md b/README.md
index 549891e3f..a6b24078e 100644 index 9ca41573f..57d6e3b5b 100644
--- a/README.md --- a/README.md
+++ b/README.md +++ b/README.md
@@ -23,6 +23,9 @@ @@ -19,6 +19,9 @@
> [!IMPORTANT] > [!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). > 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 ## Overview
### Installation ### Installation
@@ -30,7 +33,7 @@ @@ -26,7 +29,7 @@
#### Hardhat, Truffle (npm) #### Hardhat, Truffle (npm)
``` ```
@ -81,7 +81,7 @@ index 549891e3f..a6b24078e 100644
``` ```
#### Foundry (git) #### 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. > 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 ### 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 ```solidity
pragma solidity ^0.8.20; pragma solidity ^0.8.20;
@ -110,7 +110,7 @@ index 549891e3f..a6b24078e 100644
} }
``` ```
diff --git a/contracts/package.json b/contracts/package.json diff --git a/contracts/package.json b/contracts/package.json
index 9017953ca..f51c1d38b 100644 index be3e741e3..877e942c2 100644
--- a/contracts/package.json --- a/contracts/package.json
+++ b/contracts/package.json +++ b/contracts/package.json
@@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
@ -118,7 +118,7 @@ index 9017953ca..f51c1d38b 100644
- "name": "@openzeppelin/contracts", - "name": "@openzeppelin/contracts",
+ "name": "@openzeppelin/contracts-upgradeable", + "name": "@openzeppelin/contracts-upgradeable",
"description": "Secure Smart Contract library for Solidity", "description": "Secure Smart Contract library for Solidity",
"version": "4.9.2", "version": "5.0.0",
"files": [ "files": [
@@ -13,7 +13,7 @@ @@ -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 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 --- a/contracts/utils/cryptography/EIP712.sol
+++ b/contracts/utils/cryptography/EIP712.sol +++ b/contracts/utils/cryptography/EIP712.sol
@@ -4,7 +4,6 @@ @@ -4,7 +4,6 @@
@ -307,7 +307,7 @@ index 644f6f531..ab8ba05ff 100644
} }
} }
diff --git a/package.json b/package.json diff --git a/package.json b/package.json
index 3a1617c09..97e59c2d9 100644 index c2c3a2675..3301b213d 100644
--- a/package.json --- a/package.json
+++ b/package.json +++ b/package.json
@@ -32,7 +32,7 @@ @@ -32,7 +32,7 @@
@ -328,12 +328,12 @@ index 304d1386a..a1cd63bee 100644
+@openzeppelin/contracts-upgradeable/=contracts/ +@openzeppelin/contracts-upgradeable/=contracts/
+@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/ +@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/
diff --git a/test/utils/cryptography/EIP712.test.js b/test/utils/cryptography/EIP712.test.js 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 --- a/test/utils/cryptography/EIP712.test.js
+++ b/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); 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') { - if (shortOrLong === 'short') {
@ -341,17 +341,18 @@ index faf01f1a3..b25171a56 100644
- // the upgradeable contract variant is used and the initializer is invoked. - // the upgradeable contract variant is used and the initializer is invoked.
- -
- it('adjusts when behind proxy', async function () { - it('adjusts when behind proxy', async function () {
- const factory = await Clones.new(); - const factory = await ethers.deployContract('$Clones');
- 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 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); - const expectedDomain = { ...this.domain, verifyingContract: clone.target };
- expect(mapValues(reportedDomain, String)).to.be.deep.equal(mapValues(cloneDomain, String)); - 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); - expect(await clone.$_domainSeparatorV4()).to.equal(expectedSeparator);
- }); - });
- } - }

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

@ -4,7 +4,11 @@ const ethSigUtil = require('eth-sig-util');
const Wallet = require('ethereumjs-wallet').default; const Wallet = require('ethereumjs-wallet').default;
const Enums = require('../../helpers/enums'); 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 { GovernorHelper } = require('../../helpers/governance');
const { expectRevertCustomError } = require('../../helpers/customError'); const { expectRevertCustomError } = require('../../helpers/customError');
@ -130,14 +134,7 @@ contract('GovernorWithParams', function (accounts) {
primaryType: 'ExtendedBallot', primaryType: 'ExtendedBallot',
types: { types: {
EIP712Domain: domainType(domain), EIP712Domain: domainType(domain),
ExtendedBallot: [ 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' },
],
}, },
domain, domain,
message, message,

@ -7,16 +7,14 @@ const ethSigUtil = require('eth-sig-util');
const Wallet = require('ethereumjs-wallet').default; const Wallet = require('ethereumjs-wallet').default;
const { shouldBehaveLikeEIP6372 } = require('./EIP6372.behavior'); 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 { clockFromReceipt } = require('../../helpers/time');
const { expectRevertCustomError } = require('../../helpers/customError'); const { expectRevertCustomError } = require('../../helpers/customError');
const Delegation = [
{ name: 'delegatee', type: 'address' },
{ name: 'nonce', type: 'uint256' },
{ name: 'expiry', type: 'uint256' },
];
const buildAndSignDelegation = (contract, message, pk) => const buildAndSignDelegation = (contract, message, pk) =>
getDomain(contract) getDomain(contract)
.then(domain => ({ .then(domain => ({

@ -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' },
],
};

@ -1,20 +1,5 @@
const { ethers } = require('ethers'); const { ethers } = require('ethers');
const types = require('./eip712-types');
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' },
];
async function getDomain(contract) { async function getDomain(contract) {
const { fields, name, version, chainId, verifyingContract, salt, extensions } = await contract.eip712Domain(); const { fields, name, version, chainId, verifyingContract, salt, extensions } = await contract.eip712Domain();
@ -32,7 +17,7 @@ async function getDomain(contract) {
salt, salt,
}; };
for (const [i, { name }] of EIP712Domain.entries()) { for (const [i, { name }] of types.EIP712Domain.entries()) {
if (!(fields & (1 << i))) { if (!(fields & (1 << i))) {
delete domain[name]; delete domain[name];
} }
@ -42,7 +27,7 @@ async function getDomain(contract) {
} }
function domainType(domain) { function domainType(domain) {
return EIP712Domain.filter(({ name }) => domain[name] !== undefined); return types.EIP712Domain.filter(({ name }) => domain[name] !== undefined);
} }
function hashTypedData(domain, structHash) { function hashTypedData(domain, structHash) {
@ -53,7 +38,7 @@ function hashTypedData(domain, structHash) {
} }
module.exports = { module.exports = {
Permit, types,
getDomain, getDomain,
domainType, domainType,
domainSeparator: ethers.TypedDataEncoder.hashDomain, domainSeparator: ethers.TypedDataEncoder.hashDomain,

@ -10,7 +10,12 @@ const Wallet = require('ethereumjs-wallet').default;
const ERC20Permit = artifacts.require('$ERC20Permit'); 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 { getChainId } = require('../../../helpers/chainid');
const { expectRevertCustomError } = require('../../../helpers/customError'); const { expectRevertCustomError } = require('../../../helpers/customError');

@ -10,16 +10,14 @@ const ethSigUtil = require('eth-sig-util');
const Wallet = require('ethereumjs-wallet').default; const Wallet = require('ethereumjs-wallet').default;
const { batchInBlock } = require('../../../helpers/txpool'); 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 { clock, clockFromReceipt } = require('../../../helpers/time');
const { expectRevertCustomError } = require('../../../helpers/customError'); const { expectRevertCustomError } = require('../../../helpers/customError');
const Delegation = [
{ name: 'delegatee', type: 'address' },
{ name: 'nonce', type: 'uint256' },
{ name: 'expiry', type: 'uint256' },
];
const MODES = { const MODES = {
blocknumber: artifacts.require('$ERC20Votes'), blocknumber: artifacts.require('$ERC20Votes'),
timestamp: artifacts.require('$ERC20VotesTimestampMock'), timestamp: artifacts.require('$ERC20VotesTimestampMock'),

@ -1,38 +1,39 @@
const { ethers } = require('hardhat'); const { ethers } = require('hardhat');
const { expect } = require('chai'); const { expect } = require('chai');
const { getDomain, domainType, domainSeparator, hashTypedData } = require('../../helpers/eip712'); const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
const { getChainId } = require('../../helpers/chainid');
const { mapValues } = require('../../helpers/iterate');
const EIP712Verifier = artifacts.require('$EIP712Verifier');
const Clones = artifacts.require('$Clones');
contract('EIP712', function (accounts) { const { getDomain, domainSeparator, hashTypedData } = require('../../helpers/eip712');
const [mailTo] = accounts; const { getChainId } = require('../../helpers/chainid');
const shortName = 'A Name';
const shortVersion = '1';
const longName = 'A'.repeat(40); const LENGTHS = {
const longVersion = 'B'.repeat(40); short: ['A Name', '1'],
long: ['A'.repeat(40), 'B'.repeat(40)],
};
const fixture = async () => {
const [from, to] = await ethers.getSigners();
const lengths = {};
for (const [shortOrLong, [name, version]] of Object.entries(LENGTHS)) {
lengths[shortOrLong] = { name, version };
lengths[shortOrLong].eip712 = await ethers.deployContract('$EIP712Verifier', [name, version]);
lengths[shortOrLong].domain = {
name,
version,
chainId: await getChainId(),
verifyingContract: lengths[shortOrLong].eip712.target,
};
}
const cases = [ return { from, to, lengths };
['short', shortName, shortVersion], };
['long', longName, longVersion],
];
for (const [shortOrLong, name, version] of cases) { describe('EIP712', function () {
for (const [shortOrLong, [name, version]] of Object.entries(LENGTHS)) {
describe(`with ${shortOrLong} name and version`, function () { describe(`with ${shortOrLong} name and version`, function () {
beforeEach('deploying', async function () { beforeEach('deploying', async function () {
this.eip712 = await EIP712Verifier.new(name, version); Object.assign(this, await loadFixture(fixture));
Object.assign(this, this.lengths[shortOrLong]);
this.domain = {
name,
version,
chainId: await getChainId(),
verifyingContract: this.eip712.address,
};
this.domainType = domainType(this.domain);
}); });
describe('domain separator', function () { describe('domain separator', function () {
@ -44,7 +45,7 @@ contract('EIP712', function (accounts) {
it("can be rebuilt using EIP-5267's eip712Domain", async function () { it("can be rebuilt using EIP-5267's eip712Domain", async function () {
const rebuildDomain = await getDomain(this.eip712); 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') { if (shortOrLong === 'short') {
@ -52,33 +53,29 @@ contract('EIP712', function (accounts) {
// the upgradeable contract variant is used and the initializer is invoked. // the upgradeable contract variant is used and the initializer is invoked.
it('adjusts when behind proxy', async function () { it('adjusts when behind proxy', async function () {
const factory = await Clones.new(); const factory = await ethers.deployContract('$Clones');
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 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); const expectedDomain = { ...this.domain, verifyingContract: clone.target };
expect(mapValues(reportedDomain, String)).to.be.deep.equal(mapValues(cloneDomain, String)); 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); expect(await clone.$_domainSeparatorV4()).to.equal(expectedSeparator);
}); });
} }
}); });
it('hash digest', async function () { it('hash digest', async function () {
const structhash = web3.utils.randomHex(32); const structhash = ethers.hexlify(ethers.randomBytes(32));
expect(await this.eip712.$_hashTypedDataV4(structhash)).to.be.equal(hashTypedData(this.domain, structhash)); expect(await this.eip712.$_hashTypedDataV4(structhash)).to.equal(hashTypedData(this.domain, structhash));
}); });
it('digest', async function () { it('digest', async function () {
const message = {
to: mailTo,
contents: 'very interesting',
};
const types = { const types = {
Mail: [ Mail: [
{ name: 'to', type: 'address' }, { name: 'to', type: 'address' },
@ -86,19 +83,22 @@ contract('EIP712', function (accounts) {
], ],
}; };
const signer = ethers.Wallet.createRandom(); const message = {
const address = await signer.getAddress(); to: this.to.address,
const signature = await signer.signTypedData(this.domain, types, message); contents: 'very interesting',
};
const signature = await this.from.signTypedData(this.domain, types, message);
await this.eip712.verify(signature, address, message.to, message.contents); await expect(this.eip712.verify(signature, this.from.address, message.to, message.contents)).to.not.be.reverted;
}); });
it('name', async function () { it('name', async function () {
expect(await this.eip712.$_EIP712Name()).to.be.equal(name); expect(await this.eip712.$_EIP712Name()).to.equal(name);
}); });
it('version', async function () { it('version', async function () {
expect(await this.eip712.$_EIP712Version()).to.be.equal(version); expect(await this.eip712.$_EIP712Version()).to.equal(version);
}); });
}); });
} }

Loading…
Cancel
Save