diff --git a/scripts/upgradeable/upgradeable.patch b/scripts/upgradeable/upgradeable.patch index 522f82ca4..c2a5732d9 100644 --- a/scripts/upgradeable/upgradeable.patch +++ b/scripts/upgradeable/upgradeable.patch @@ -59,10 +59,10 @@ index ff596b0c3..000000000 - - 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,7 +110,7 @@ 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 @@ @@ -118,7 +118,7 @@ index 9017953ca..f51c1d38b 100644 - "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,12 +328,12 @@ 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') { @@ -341,17 +341,18 @@ index faf01f1a3..b25171a56 100644 - // 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 index 45cceba9a..b62160eec 100644 --- 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 index 35da3a0f3..da392b3ea 100644 --- 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 index 5836cc351..0aea208a9 100644 --- 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 new file mode 100644 index 000000000..8aacf5325 --- /dev/null +++ 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 index f09272b28..295c1b953 100644 --- 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 index 388716d53..db2363cd2 100644 --- 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 index faf1a15ad..96d6c4e77 100644 --- 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'), diff --git a/test/utils/cryptography/EIP712.test.js b/test/utils/cryptography/EIP712.test.js index 9d0bdae9a..2b88f5f8e 100644 --- a/test/utils/cryptography/EIP712.test.js +++ b/test/utils/cryptography/EIP712.test.js @@ -1,38 +1,39 @@ const { ethers } = require('hardhat'); const { expect } = require('chai'); -const { getDomain, domainType, domainSeparator, hashTypedData } = require('../../helpers/eip712'); -const { getChainId } = require('../../helpers/chainid'); -const { mapValues } = require('../../helpers/iterate'); - -const EIP712Verifier = artifacts.require('$EIP712Verifier'); -const Clones = artifacts.require('$Clones'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); -contract('EIP712', function (accounts) { - const [mailTo] = accounts; - - const shortName = 'A Name'; - const shortVersion = '1'; +const { getDomain, domainSeparator, hashTypedData } = require('../../helpers/eip712'); +const { getChainId } = require('../../helpers/chainid'); - const longName = 'A'.repeat(40); - const longVersion = 'B'.repeat(40); +const LENGTHS = { + 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 = [ - ['short', shortName, shortVersion], - ['long', longName, longVersion], - ]; + return { from, to, lengths }; +}; - 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 () { beforeEach('deploying', async function () { - this.eip712 = await EIP712Verifier.new(name, version); - - this.domain = { - name, - version, - chainId: await getChainId(), - verifyingContract: this.eip712.address, - }; - this.domainType = domainType(this.domain); + Object.assign(this, await loadFixture(fixture)); + Object.assign(this, this.lengths[shortOrLong]); }); describe('domain separator', function () { @@ -44,7 +45,7 @@ contract('EIP712', function (accounts) { it("can be rebuilt using EIP-5267's eip712Domain", async 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') { @@ -52,33 +53,29 @@ contract('EIP712', function (accounts) { // 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); }); } }); it('hash digest', async function () { - const structhash = web3.utils.randomHex(32); - expect(await this.eip712.$_hashTypedDataV4(structhash)).to.be.equal(hashTypedData(this.domain, structhash)); + const structhash = ethers.hexlify(ethers.randomBytes(32)); + expect(await this.eip712.$_hashTypedDataV4(structhash)).to.equal(hashTypedData(this.domain, structhash)); }); it('digest', async function () { - const message = { - to: mailTo, - contents: 'very interesting', - }; - const types = { Mail: [ { name: 'to', type: 'address' }, @@ -86,19 +83,22 @@ contract('EIP712', function (accounts) { ], }; - const signer = ethers.Wallet.createRandom(); - const address = await signer.getAddress(); - const signature = await signer.signTypedData(this.domain, types, message); + const message = { + to: this.to.address, + 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 () { - expect(await this.eip712.$_EIP712Name()).to.be.equal(name); + expect(await this.eip712.$_EIP712Name()).to.equal(name); }); it('version', async function () { - expect(await this.eip712.$_EIP712Version()).to.be.equal(version); + expect(await this.eip712.$_EIP712Version()).to.equal(version); }); }); }