diff --git a/.changeset/short-roses-judge.md b/.changeset/short-roses-judge.md new file mode 100644 index 00000000000..002aebb11e3 --- /dev/null +++ b/.changeset/short-roses-judge.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`EIP712`: add EIP-5267 support for better domain discovery. diff --git a/CHANGELOG.md b/CHANGELOG.md index e60161db0ea..ccd279d53b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +### Breaking changes + +- `EIP712`: Addition of ERC5267 support requires support for user defined value types, which was released in Solidity version 0.8.8. This requires a pragma change from `^0.8.0` to `^0.8.8`. +- `EIP712`: Optimization of the cache for the upgradeable version affects the way `name` and `version` are set. This is no longer done through an initializer, and is instead part of the implementation's constructor. As a consequence, all proxies using the same implementation will necessarily share the same `name` and `version`. Additionally, an implementation upgrade risks changing the EIP712 domain unless the same `name` and `version` are used when deploying the new implementation contract. + ### Deprecations - `ERC20Permit`: Added the file `IERC20Permit.sol` and `ERC20Permit.sol` and deprecated `draft-IERC20Permit.sol` and `draft-ERC20Permit.sol` since [EIP-2612](https://eips.ethereum.org/EIPS/eip-2612) is no longer a Draft. Developers are encouraged to update their imports. ([#3793](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3793)) diff --git a/contracts/interfaces/IERC5267.sol b/contracts/interfaces/IERC5267.sol new file mode 100644 index 00000000000..3adc4a70355 --- /dev/null +++ b/contracts/interfaces/IERC5267.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +interface IERC5267 { + /** + * @dev MAY be emitted to signal that the domain could have changed. + */ + event EIP712DomainChanged(); + + /** + * @dev returns the fields and values that describe the domain separator used by this contract for EIP-712 + * signature. + */ + function eip712Domain() + external + view + returns ( + bytes1 fields, + string memory name, + string memory version, + uint256 chainId, + address verifyingContract, + bytes32 salt, + uint256[] memory extensions + ); +} diff --git a/contracts/utils/cryptography/EIP712.sol b/contracts/utils/cryptography/EIP712.sol index eb211a7e27f..d0e52c39658 100644 --- a/contracts/utils/cryptography/EIP712.sol +++ b/contracts/utils/cryptography/EIP712.sol @@ -1,9 +1,11 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v4.8.0) (utils/cryptography/EIP712.sol) -pragma solidity ^0.8.0; +pragma solidity ^0.8.8; import "./ECDSA.sol"; +import "../ShortStrings.sol"; +import "../../interfaces/IERC5267.sol"; /** * @dev https://eips.ethereum.org/EIPS/eip-712[EIP 712] is a standard for hashing and signing of typed structured data. @@ -22,19 +24,34 @@ import "./ECDSA.sol"; * NOTE: This contract implements the version of the encoding known as "v4", as implemented by the JSON RPC method * https://docs.metamask.io/guide/signing-data.html[`eth_signTypedDataV4` in MetaMask]. * + * NOTE: In the upgradeable version of this contract, the cached values will correspond to the address, and the domain + * separator of the implementation contract. This will cause the `_domainSeparatorV4` function to always rebuild the + * separator from the immutable values, which is cheaper than accessing a cached version in cold storage. + * * _Available since v3.4._ + * + * @custom:oz-upgrades-unsafe-allow state-variable-immutable state-variable-assignment */ -abstract contract EIP712 { +abstract contract EIP712 is IERC5267 { + using ShortStrings for *; + + bytes32 private constant _TYPE_HASH = + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + /* solhint-disable var-name-mixedcase */ // Cache the domain separator as an immutable value, but also store the chain id that it corresponds to, in order to // invalidate the cached domain separator if the chain id changes. - bytes32 private immutable _CACHED_DOMAIN_SEPARATOR; - uint256 private immutable _CACHED_CHAIN_ID; - address private immutable _CACHED_THIS; + bytes32 private immutable _cachedDomainSeparator; + uint256 private immutable _cachedChainId; + address private immutable _cachedThis; + + ShortString private immutable _name; + ShortString private immutable _version; + string private _nameFallback; + string private _versionFallback; - bytes32 private immutable _HASHED_NAME; - bytes32 private immutable _HASHED_VERSION; - bytes32 private immutable _TYPE_HASH; + bytes32 private immutable _hashedName; + bytes32 private immutable _hashedVersion; /* solhint-enable var-name-mixedcase */ @@ -51,36 +68,29 @@ abstract contract EIP712 { * contract upgrade]. */ constructor(string memory name, string memory version) { - bytes32 hashedName = keccak256(bytes(name)); - bytes32 hashedVersion = keccak256(bytes(version)); - bytes32 typeHash = keccak256( - "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" - ); - _HASHED_NAME = hashedName; - _HASHED_VERSION = hashedVersion; - _CACHED_CHAIN_ID = block.chainid; - _CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(typeHash, hashedName, hashedVersion); - _CACHED_THIS = address(this); - _TYPE_HASH = typeHash; + _name = name.toShortStringWithFallback(_nameFallback); + _version = version.toShortStringWithFallback(_versionFallback); + _hashedName = keccak256(bytes(name)); + _hashedVersion = keccak256(bytes(version)); + + _cachedChainId = block.chainid; + _cachedDomainSeparator = _buildDomainSeparator(); + _cachedThis = address(this); } /** * @dev Returns the domain separator for the current chain. */ function _domainSeparatorV4() internal view returns (bytes32) { - if (address(this) == _CACHED_THIS && block.chainid == _CACHED_CHAIN_ID) { - return _CACHED_DOMAIN_SEPARATOR; + if (address(this) == _cachedThis && block.chainid == _cachedChainId) { + return _cachedDomainSeparator; } else { - return _buildDomainSeparator(_TYPE_HASH, _HASHED_NAME, _HASHED_VERSION); + return _buildDomainSeparator(); } } - function _buildDomainSeparator( - bytes32 typeHash, - bytes32 nameHash, - bytes32 versionHash - ) private view returns (bytes32) { - return keccak256(abi.encode(typeHash, nameHash, versionHash, block.chainid, address(this))); + function _buildDomainSeparator() private view returns (bytes32) { + return keccak256(abi.encode(_TYPE_HASH, _hashedName, _hashedVersion, block.chainid, address(this))); } /** @@ -101,4 +111,33 @@ abstract contract EIP712 { function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) { return ECDSA.toTypedDataHash(_domainSeparatorV4(), structHash); } + + /** + * @dev See {EIP-5267}. + */ + function eip712Domain() + public + view + virtual + override + returns ( + bytes1 fields, + string memory name, + string memory version, + uint256 chainId, + address verifyingContract, + bytes32 salt, + uint256[] memory extensions + ) + { + return ( + hex"0f", // 01111 + _name.toStringWithFallback(_nameFallback), + _version.toStringWithFallback(_versionFallback), + block.chainid, + address(this), + bytes32(0), + new uint256[](0) + ); + } } diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index 4c083ed92c4..2018959348c 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -4,7 +4,7 @@ const ethSigUtil = require('eth-sig-util'); const Wallet = require('ethereumjs-wallet').default; const { fromRpcSig } = require('ethereumjs-util'); const Enums = require('../helpers/enums'); -const { EIP712Domain } = require('../helpers/eip712'); +const { getDomain, domainType } = require('../helpers/eip712'); const { GovernorHelper } = require('../helpers/governance'); const { shouldSupportInterfaces } = require('../utils/introspection/SupportsInterface.behavior'); @@ -20,7 +20,6 @@ contract('Governor', function (accounts) { const empty = web3.utils.toChecksumAddress(web3.utils.randomHex(20)); const name = 'OZ-Governor'; - const version = '1'; const tokenName = 'MockToken'; const tokenSymbol = 'MTKN'; const tokenSupply = web3.utils.toWei('100'); @@ -148,24 +147,22 @@ contract('Governor', function (accounts) { const voterBySig = Wallet.generate(); const voterBySigAddress = web3.utils.toChecksumAddress(voterBySig.getAddressString()); - const signature = async message => { - return fromRpcSig( - ethSigUtil.signTypedMessage(voterBySig.getPrivateKey(), { - data: { - types: { - EIP712Domain, - Ballot: [ - { name: 'proposalId', type: 'uint256' }, - { name: 'support', type: 'uint8' }, - ], - }, - domain: { name, version, chainId: this.chainId, verifyingContract: this.mock.address }, - primaryType: 'Ballot', - message, + const signature = (contract, message) => + getDomain(contract) + .then(domain => ({ + primaryType: 'Ballot', + types: { + EIP712Domain: domainType(domain), + Ballot: [ + { name: 'proposalId', type: 'uint256' }, + { name: 'support', type: 'uint8' }, + ], }, - }), - ); - }; + domain, + message, + })) + .then(data => ethSigUtil.signTypedMessage(voterBySig.getPrivateKey(), { data })) + .then(fromRpcSig); await this.token.delegate(voterBySigAddress, { from: voter1 }); diff --git a/test/governance/extensions/GovernorWithParams.test.js b/test/governance/extensions/GovernorWithParams.test.js index 26e94854439..86ebacc087b 100644 --- a/test/governance/extensions/GovernorWithParams.test.js +++ b/test/governance/extensions/GovernorWithParams.test.js @@ -4,7 +4,7 @@ const ethSigUtil = require('eth-sig-util'); const Wallet = require('ethereumjs-wallet').default; const { fromRpcSig } = require('ethereumjs-util'); const Enums = require('../../helpers/enums'); -const { EIP712Domain } = require('../../helpers/eip712'); +const { getDomain, domainType } = require('../../helpers/eip712'); const { GovernorHelper } = require('../../helpers/governance'); const Token = artifacts.require('$ERC20VotesComp'); @@ -22,7 +22,6 @@ contract('GovernorWithParams', function (accounts) { const [owner, proposer, voter1, voter2, voter3, voter4] = accounts; const name = 'OZ-Governor'; - const version = '1'; const tokenName = 'MockToken'; const tokenSymbol = 'MTKN'; const tokenSupply = web3.utils.toWei('100'); @@ -116,26 +115,24 @@ contract('GovernorWithParams', function (accounts) { const voterBySig = Wallet.generate(); const voterBySigAddress = web3.utils.toChecksumAddress(voterBySig.getAddressString()); - const signature = async message => { - return fromRpcSig( - ethSigUtil.signTypedMessage(voterBySig.getPrivateKey(), { - data: { - types: { - EIP712Domain, - ExtendedBallot: [ - { name: 'proposalId', type: 'uint256' }, - { name: 'support', type: 'uint8' }, - { name: 'reason', type: 'string' }, - { name: 'params', type: 'bytes' }, - ], - }, - domain: { name, version, chainId: this.chainId, verifyingContract: this.mock.address }, - primaryType: 'ExtendedBallot', - message, + const signature = (contract, message) => + getDomain(contract) + .then(domain => ({ + primaryType: 'ExtendedBallot', + types: { + EIP712Domain: domainType(domain), + ExtendedBallot: [ + { name: 'proposalId', type: 'uint256' }, + { name: 'support', type: 'uint8' }, + { name: 'reason', type: 'string' }, + { name: 'params', type: 'bytes' }, + ], }, - }), - ); - }; + domain, + message, + })) + .then(data => ethSigUtil.signTypedMessage(voterBySig.getPrivateKey(), { data })) + .then(fromRpcSig); await this.token.delegate(voterBySigAddress, { from: voter2 }); diff --git a/test/governance/utils/Votes.behavior.js b/test/governance/utils/Votes.behavior.js index 5062a9c9701..8fe34dc47e5 100644 --- a/test/governance/utils/Votes.behavior.js +++ b/test/governance/utils/Votes.behavior.js @@ -6,7 +6,7 @@ const { fromRpcSig } = require('ethereumjs-util'); const ethSigUtil = require('eth-sig-util'); const Wallet = require('ethereumjs-wallet').default; -const { EIP712Domain, domainSeparator } = require('../../helpers/eip712'); +const { getDomain, domainType, domainSeparator } = require('../../helpers/eip712'); const Delegation = [ { name: 'delegatee', type: 'address' }, @@ -14,8 +14,6 @@ const Delegation = [ { name: 'expiry', type: 'uint256' }, ]; -const version = '1'; - function shouldBehaveLikeVotes() { describe('run votes workflow', function () { it('initial nonce is 0', async function () { @@ -23,14 +21,7 @@ function shouldBehaveLikeVotes() { }); it('domain separator', async function () { - expect(await this.votes.DOMAIN_SEPARATOR()).to.equal( - await domainSeparator({ - name: this.name, - version, - chainId: this.chainId, - verifyingContract: this.votes.address, - }), - ); + expect(await this.votes.DOMAIN_SEPARATOR()).to.equal(domainSeparator(await getDomain(this.votes))); }); describe('delegation with signature', function () { @@ -38,29 +29,29 @@ function shouldBehaveLikeVotes() { const delegatorAddress = web3.utils.toChecksumAddress(delegator.getAddressString()); const nonce = 0; - const buildData = (chainId, verifyingContract, name, message) => ({ - data: { + const buildAndSignData = async (contract, message, pk) => { + const data = await getDomain(contract).then(domain => ({ primaryType: 'Delegation', - types: { EIP712Domain, Delegation }, - domain: { name, version, chainId, verifyingContract }, + types: { EIP712Domain: domainType(domain), Delegation }, + domain, message, - }, - }); + })); + return fromRpcSig(ethSigUtil.signTypedMessage(pk, { data })); + }; beforeEach(async function () { await this.votes.$_mint(delegatorAddress, this.NFT0); }); it('accept signed delegation', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.votes.address, this.name, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), + const { v, r, s } = await buildAndSignData( + this.votes, + { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }, + delegator.getPrivateKey(), ); expect(await this.votes.delegates(delegatorAddress)).to.be.equal(ZERO_ADDRESS); @@ -86,15 +77,14 @@ function shouldBehaveLikeVotes() { }); it('rejects reused signature', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.votes.address, this.name, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), + const { v, r, s } = await buildAndSignData( + this.votes, + { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }, + delegator.getPrivateKey(), ); await this.votes.delegateBySig(delegatorAddress, nonce, MAX_UINT256, v, r, s); @@ -106,15 +96,14 @@ function shouldBehaveLikeVotes() { }); it('rejects bad delegatee', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.votes.address, this.name, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), + const { v, r, s } = await buildAndSignData( + this.votes, + { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }, + delegator.getPrivateKey(), ); const receipt = await this.votes.delegateBySig(this.account1Delegatee, nonce, MAX_UINT256, v, r, s); @@ -125,16 +114,16 @@ function shouldBehaveLikeVotes() { }); it('rejects bad nonce', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.votes.address, this.name, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), + const { v, r, s } = await buildAndSignData( + this.votes, + { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }, + delegator.getPrivateKey(), ); + await expectRevert( this.votes.delegateBySig(delegatorAddress, nonce + 1, MAX_UINT256, v, r, s), 'Votes: invalid nonce', @@ -143,15 +132,15 @@ function shouldBehaveLikeVotes() { it('rejects expired permit', async function () { const expiry = (await time.latest()) - time.duration.weeks(1); - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.votes.address, this.name, { - delegatee: delegatorAddress, - nonce, - expiry, - }), - ), + + const { v, r, s } = await buildAndSignData( + this.votes, + { + delegatee: delegatorAddress, + nonce, + expiry, + }, + delegator.getPrivateKey(), ); await expectRevert( diff --git a/test/helpers/eip712.js b/test/helpers/eip712.js index 9851df66205..b12a6233ec9 100644 --- a/test/helpers/eip712.js +++ b/test/helpers/eip712.js @@ -6,6 +6,7 @@ const EIP712Domain = [ { name: 'version', type: 'string' }, { name: 'chainId', type: 'uint256' }, { name: 'verifyingContract', type: 'address' }, + { name: 'salt', type: 'bytes32' }, ]; const Permit = [ @@ -24,25 +25,43 @@ function hexStringToBuffer(hexstr) { return Buffer.from(hexstr.replace(/^0x/, ''), 'hex'); } -async function domainSeparator({ name, version, chainId, verifyingContract }) { +async function getDomain(contract) { + const { fields, name, version, chainId, verifyingContract, salt, extensions } = await contract.eip712Domain(); + + if (extensions.length > 0) { + throw Error('Extensions not implemented'); + } + + const domain = { name, version, chainId, verifyingContract, salt }; + for (const [i, { name }] of EIP712Domain.entries()) { + if (!(fields & (1 << i))) { + delete domain[name]; + } + } + + return domain; +} + +function domainType(domain) { + return EIP712Domain.filter(({ name }) => domain[name] !== undefined); +} + +function domainSeparator(domain) { return bufferToHexString( - ethSigUtil.TypedDataUtils.hashStruct( - 'EIP712Domain', - { name, version, chainId, verifyingContract }, - { EIP712Domain }, - ), + ethSigUtil.TypedDataUtils.hashStruct('EIP712Domain', domain, { EIP712Domain: domainType(domain) }), ); } -async function hashTypedData(domain, structHash) { - return domainSeparator(domain).then(separator => - bufferToHexString(keccak256(Buffer.concat(['0x1901', separator, structHash].map(str => hexStringToBuffer(str))))), +function hashTypedData(domain, structHash) { + return bufferToHexString( + keccak256(Buffer.concat(['0x1901', domainSeparator(domain), structHash].map(str => hexStringToBuffer(str)))), ); } module.exports = { - EIP712Domain, Permit, + getDomain, + domainType, domainSeparator, hashTypedData, }; diff --git a/test/helpers/governance.js b/test/helpers/governance.js index f933f389d0a..b945a16e045 100644 --- a/test/helpers/governance.js +++ b/test/helpers/governance.js @@ -84,7 +84,7 @@ class GovernorHelper { ? // if signature, and either params or reason → vote.params || vote.reason ? vote - .signature({ + .signature(this.governor, { proposalId: proposal.id, support: vote.support, reason: vote.reason || '', @@ -96,7 +96,7 @@ class GovernorHelper { ), ) : vote - .signature({ + .signature(this.governor, { proposalId: proposal.id, support: vote.support, }) diff --git a/test/metatx/ERC2771Context.test.js b/test/metatx/ERC2771Context.test.js index 78877772663..6c298d3d9f0 100644 --- a/test/metatx/ERC2771Context.test.js +++ b/test/metatx/ERC2771Context.test.js @@ -1,6 +1,6 @@ const ethSigUtil = require('eth-sig-util'); const Wallet = require('ethereumjs-wallet').default; -const { EIP712Domain } = require('../helpers/eip712'); +const { getDomain, domainType } = require('../helpers/eip712'); const { expectEvent } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); @@ -10,24 +10,15 @@ const MinimalForwarder = artifacts.require('MinimalForwarder'); const ContextMockCaller = artifacts.require('ContextMockCaller'); const { shouldBehaveLikeRegularContext } = require('../utils/Context.behavior'); -const { getChainId } = require('../helpers/chainid'); - -const name = 'MinimalForwarder'; -const version = '0.0.1'; contract('ERC2771Context', function (accounts) { beforeEach(async function () { this.forwarder = await MinimalForwarder.new(); this.recipient = await ERC2771ContextMock.new(this.forwarder.address); - this.domain = { - name, - version, - chainId: await getChainId(), - verifyingContract: this.forwarder.address, - }; + this.domain = await getDomain(this.forwarder); this.types = { - EIP712Domain, + EIP712Domain: domainType(this.domain), ForwardRequest: [ { name: 'from', type: 'address' }, { name: 'to', type: 'address' }, diff --git a/test/metatx/MinimalForwarder.test.js b/test/metatx/MinimalForwarder.test.js index 24de1719d31..4884cc760bb 100644 --- a/test/metatx/MinimalForwarder.test.js +++ b/test/metatx/MinimalForwarder.test.js @@ -1,29 +1,20 @@ const ethSigUtil = require('eth-sig-util'); const Wallet = require('ethereumjs-wallet').default; -const { EIP712Domain } = require('../helpers/eip712'); +const { getDomain, domainType } = require('../helpers/eip712'); const { expectRevert, constants } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); -const { getChainId } = require('../helpers/chainid'); - const MinimalForwarder = artifacts.require('MinimalForwarder'); const CallReceiverMock = artifacts.require('CallReceiverMock'); -const name = 'MinimalForwarder'; -const version = '0.0.1'; - contract('MinimalForwarder', function (accounts) { beforeEach(async function () { this.forwarder = await MinimalForwarder.new(); - this.domain = { - name, - version, - chainId: await getChainId(), - verifyingContract: this.forwarder.address, - }; + + this.domain = await getDomain(this.forwarder); this.types = { - EIP712Domain, + EIP712Domain: domainType(this.domain), ForwardRequest: [ { name: 'from', type: 'address' }, { name: 'to', type: 'address' }, diff --git a/test/token/ERC20/extensions/ERC20Votes.test.js b/test/token/ERC20/extensions/ERC20Votes.test.js index a340e71a775..9759c11629b 100644 --- a/test/token/ERC20/extensions/ERC20Votes.test.js +++ b/test/token/ERC20/extensions/ERC20Votes.test.js @@ -11,7 +11,7 @@ const Wallet = require('ethereumjs-wallet').default; const ERC20Votes = artifacts.require('$ERC20Votes'); const { batchInBlock } = require('../../../helpers/txpool'); -const { EIP712Domain, domainSeparator } = require('../../../helpers/eip712'); +const { getDomain, domainType, domainSeparator } = require('../../../helpers/eip712'); const { getChainId } = require('../../../helpers/chainid'); const Delegation = [ @@ -38,9 +38,7 @@ contract('ERC20Votes', function (accounts) { }); it('domain separator', async function () { - expect(await this.token.DOMAIN_SEPARATOR()).to.equal( - await domainSeparator({ name, version, chainId: this.chainId, verifyingContract: this.token.address }), - ); + expect(await this.token.DOMAIN_SEPARATOR()).to.equal(await getDomain(this.token).then(domainSeparator)); }); it('minting restriction', async function () { @@ -107,30 +105,26 @@ contract('ERC20Votes', function (accounts) { const delegatorAddress = web3.utils.toChecksumAddress(delegator.getAddressString()); const nonce = 0; - const buildData = (chainId, verifyingContract, message) => ({ - data: { + const buildData = (contract, message) => + getDomain(contract).then(domain => ({ primaryType: 'Delegation', - types: { EIP712Domain, Delegation }, - domain: { name, version, chainId, verifyingContract }, + types: { EIP712Domain: domainType(domain), Delegation }, + domain, message, - }, - }); + })); beforeEach(async function () { await this.token.$_mint(delegatorAddress, supply); }); it('accept signed delegation', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), - ); + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); expect(await this.token.delegates(delegatorAddress)).to.be.equal(ZERO_ADDRESS); @@ -155,16 +149,13 @@ contract('ERC20Votes', function (accounts) { }); it('rejects reused signature', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), - ); + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); await this.token.delegateBySig(delegatorAddress, nonce, MAX_UINT256, v, r, s); @@ -175,16 +166,13 @@ contract('ERC20Votes', function (accounts) { }); it('rejects bad delegatee', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), - ); + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); const receipt = await this.token.delegateBySig(holderDelegatee, nonce, MAX_UINT256, v, r, s); const { args } = receipt.logs.find(({ event }) => event == 'DelegateChanged'); @@ -194,16 +182,14 @@ contract('ERC20Votes', function (accounts) { }); it('rejects bad nonce', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), - ); + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); + await expectRevert( this.token.delegateBySig(delegatorAddress, nonce + 1, MAX_UINT256, v, r, s), 'ERC20Votes: invalid nonce', @@ -212,16 +198,14 @@ contract('ERC20Votes', function (accounts) { it('rejects expired permit', async function () { const expiry = (await time.latest()) - time.duration.weeks(1); - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry, - }), - ), - ); + + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); await expectRevert( this.token.delegateBySig(delegatorAddress, nonce, expiry, v, r, s), diff --git a/test/token/ERC20/extensions/ERC20VotesComp.test.js b/test/token/ERC20/extensions/ERC20VotesComp.test.js index 6a7c0001344..660ed2124ce 100644 --- a/test/token/ERC20/extensions/ERC20VotesComp.test.js +++ b/test/token/ERC20/extensions/ERC20VotesComp.test.js @@ -11,7 +11,7 @@ const Wallet = require('ethereumjs-wallet').default; const ERC20VotesComp = artifacts.require('$ERC20VotesComp'); const { batchInBlock } = require('../../../helpers/txpool'); -const { EIP712Domain, domainSeparator } = require('../../../helpers/eip712'); +const { getDomain, domainType, domainSeparator } = require('../../../helpers/eip712'); const { getChainId } = require('../../../helpers/chainid'); const Delegation = [ @@ -38,9 +38,7 @@ contract('ERC20VotesComp', function (accounts) { }); it('domain separator', async function () { - expect(await this.token.DOMAIN_SEPARATOR()).to.equal( - await domainSeparator({ name, version, chainId: this.chainId, verifyingContract: this.token.address }), - ); + expect(await this.token.DOMAIN_SEPARATOR()).to.equal(await getDomain(this.token).then(domainSeparator)); }); it('minting restriction', async function () { @@ -94,30 +92,26 @@ contract('ERC20VotesComp', function (accounts) { const delegatorAddress = web3.utils.toChecksumAddress(delegator.getAddressString()); const nonce = 0; - const buildData = (chainId, verifyingContract, message) => ({ - data: { + const buildData = (contract, message) => + getDomain(contract).then(domain => ({ primaryType: 'Delegation', - types: { EIP712Domain, Delegation }, - domain: { name, version, chainId, verifyingContract }, + types: { EIP712Domain: domainType(domain), Delegation }, + domain, message, - }, - }); + })); beforeEach(async function () { await this.token.$_mint(delegatorAddress, supply); }); it('accept signed delegation', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), - ); + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); expect(await this.token.delegates(delegatorAddress)).to.be.equal(ZERO_ADDRESS); @@ -142,16 +136,13 @@ contract('ERC20VotesComp', function (accounts) { }); it('rejects reused signature', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), - ); + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); await this.token.delegateBySig(delegatorAddress, nonce, MAX_UINT256, v, r, s); @@ -162,16 +153,13 @@ contract('ERC20VotesComp', function (accounts) { }); it('rejects bad delegatee', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), - ); + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); const receipt = await this.token.delegateBySig(holderDelegatee, nonce, MAX_UINT256, v, r, s); const { args } = receipt.logs.find(({ event }) => event == 'DelegateChanged'); @@ -181,16 +169,14 @@ contract('ERC20VotesComp', function (accounts) { }); it('rejects bad nonce', async function () { - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry: MAX_UINT256, - }), - ), - ); + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry: MAX_UINT256, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); + await expectRevert( this.token.delegateBySig(delegatorAddress, nonce + 1, MAX_UINT256, v, r, s), 'ERC20Votes: invalid nonce', @@ -199,16 +185,14 @@ contract('ERC20VotesComp', function (accounts) { it('rejects expired permit', async function () { const expiry = (await time.latest()) - time.duration.weeks(1); - const { v, r, s } = fromRpcSig( - ethSigUtil.signTypedMessage( - delegator.getPrivateKey(), - buildData(this.chainId, this.token.address, { - delegatee: delegatorAddress, - nonce, - expiry, - }), - ), - ); + + const { v, r, s } = await buildData(this.token, { + delegatee: delegatorAddress, + nonce, + expiry, + }) + .then(data => ethSigUtil.signTypedMessage(delegator.getPrivateKey(), { data })) + .then(fromRpcSig); await expectRevert( this.token.delegateBySig(delegatorAddress, nonce, expiry, v, r, s), diff --git a/test/token/ERC20/extensions/draft-ERC20Permit.test.js b/test/token/ERC20/extensions/draft-ERC20Permit.test.js index eb673782699..33c43c479fd 100644 --- a/test/token/ERC20/extensions/draft-ERC20Permit.test.js +++ b/test/token/ERC20/extensions/draft-ERC20Permit.test.js @@ -10,7 +10,7 @@ const Wallet = require('ethereumjs-wallet').default; const ERC20Permit = artifacts.require('$ERC20Permit'); -const { EIP712Domain, Permit, domainSeparator } = require('../../../helpers/eip712'); +const { Permit, getDomain, domainType, domainSeparator } = require('../../../helpers/eip712'); const { getChainId } = require('../../../helpers/chainid'); contract('ERC20Permit', function (accounts) { @@ -34,9 +34,7 @@ contract('ERC20Permit', function (accounts) { }); it('domain separator', async function () { - expect(await this.token.DOMAIN_SEPARATOR()).to.equal( - await domainSeparator({ name, version, chainId: this.chainId, verifyingContract: this.token.address }), - ); + expect(await this.token.DOMAIN_SEPARATOR()).to.equal(await getDomain(this.token).then(domainSeparator)); }); describe('permit', function () { @@ -47,17 +45,18 @@ contract('ERC20Permit', function (accounts) { const nonce = 0; const maxDeadline = MAX_UINT256; - const buildData = (chainId, verifyingContract, deadline = maxDeadline) => ({ - primaryType: 'Permit', - types: { EIP712Domain, Permit }, - domain: { name, version, chainId, verifyingContract }, - message: { owner, spender, value, nonce, deadline }, - }); + const buildData = (contract, deadline = maxDeadline) => + getDomain(contract).then(domain => ({ + primaryType: 'Permit', + types: { EIP712Domain: domainType(domain), Permit }, + domain, + message: { owner, spender, value, nonce, deadline }, + })); it('accepts owner signature', async function () { - const data = buildData(this.chainId, this.token.address); - const signature = ethSigUtil.signTypedMessage(wallet.getPrivateKey(), { data }); - const { v, r, s } = fromRpcSig(signature); + const { v, r, s } = await buildData(this.token) + .then(data => ethSigUtil.signTypedMessage(wallet.getPrivateKey(), { data })) + .then(fromRpcSig); await this.token.permit(owner, spender, value, maxDeadline, v, r, s); @@ -66,9 +65,9 @@ contract('ERC20Permit', function (accounts) { }); it('rejects reused signature', async function () { - const data = buildData(this.chainId, this.token.address); - const signature = ethSigUtil.signTypedMessage(wallet.getPrivateKey(), { data }); - const { v, r, s } = fromRpcSig(signature); + const { v, r, s } = await buildData(this.token) + .then(data => ethSigUtil.signTypedMessage(wallet.getPrivateKey(), { data })) + .then(fromRpcSig); await this.token.permit(owner, spender, value, maxDeadline, v, r, s); @@ -80,9 +79,10 @@ contract('ERC20Permit', function (accounts) { it('rejects other signature', async function () { const otherWallet = Wallet.generate(); - const data = buildData(this.chainId, this.token.address); - const signature = ethSigUtil.signTypedMessage(otherWallet.getPrivateKey(), { data }); - const { v, r, s } = fromRpcSig(signature); + + const { v, r, s } = await buildData(this.token) + .then(data => ethSigUtil.signTypedMessage(otherWallet.getPrivateKey(), { data })) + .then(fromRpcSig); await expectRevert( this.token.permit(owner, spender, value, maxDeadline, v, r, s), @@ -93,9 +93,9 @@ contract('ERC20Permit', function (accounts) { it('rejects expired permit', async function () { const deadline = (await time.latest()) - time.duration.weeks(1); - const data = buildData(this.chainId, this.token.address, deadline); - const signature = ethSigUtil.signTypedMessage(wallet.getPrivateKey(), { data }); - const { v, r, s } = fromRpcSig(signature); + const { v, r, s } = await buildData(this.token, deadline) + .then(data => ethSigUtil.signTypedMessage(wallet.getPrivateKey(), { data })) + .then(fromRpcSig); await expectRevert(this.token.permit(owner, spender, value, deadline, v, r, s), 'ERC20Permit: expired deadline'); }); diff --git a/test/token/ERC20/utils/SafeERC20.test.js b/test/token/ERC20/utils/SafeERC20.test.js index 878989cb171..a6b10fae3a4 100644 --- a/test/token/ERC20/utils/SafeERC20.test.js +++ b/test/token/ERC20/utils/SafeERC20.test.js @@ -6,8 +6,7 @@ const ERC20ReturnTrueMock = artifacts.require('ERC20ReturnTrueMock'); const ERC20NoReturnMock = artifacts.require('ERC20NoReturnMock'); const ERC20PermitNoRevertMock = artifacts.require('ERC20PermitNoRevertMock'); -const { EIP712Domain, Permit } = require('../../../helpers/eip712'); -const { getChainId } = require('../../../helpers/chainid'); +const { getDomain, domainType, Permit } = require('../../../helpers/eip712'); const { fromRpcSig } = require('ethereumjs-util'); const ethSigUtil = require('eth-sig-util'); @@ -58,16 +57,15 @@ contract('SafeERC20', function (accounts) { const spender = hasNoCode; beforeEach(async function () { - const chainId = await getChainId(); - this.token = await ERC20PermitNoRevertMock.new(); - this.data = { + this.data = await getDomain(this.token).then(domain => ({ primaryType: 'Permit', - types: { EIP712Domain, Permit }, - domain: { name: 'ERC20PermitNoRevertMock', version: '1', chainId, verifyingContract: this.token.address }, + types: { EIP712Domain: domainType(domain), Permit }, + domain, message: { owner, spender, value: '42', nonce: '0', deadline: constants.MAX_UINT256 }, - }; + })); + this.signature = fromRpcSig(ethSigUtil.signTypedMessage(wallet.getPrivateKey(), { data: this.data })); }); diff --git a/test/utils/cryptography/EIP712.test.js b/test/utils/cryptography/EIP712.test.js index 8c20b91badc..f12f226735b 100644 --- a/test/utils/cryptography/EIP712.test.js +++ b/test/utils/cryptography/EIP712.test.js @@ -1,8 +1,9 @@ const ethSigUtil = require('eth-sig-util'); const Wallet = require('ethereumjs-wallet').default; -const { EIP712Domain, domainSeparator, hashTypedData } = require('../../helpers/eip712'); +const { getDomain, domainType, domainSeparator, hashTypedData } = require('../../helpers/eip712'); const { getChainId } = require('../../helpers/chainid'); +const { mapValues } = require('../../helpers/map-values'); const EIP712Verifier = artifacts.require('$EIP712Verifier'); @@ -21,19 +22,25 @@ contract('EIP712', function (accounts) { chainId: await getChainId(), verifyingContract: this.eip712.address, }; + this.domainType = domainType(this.domain); }); - it('domain separator', async function () { - const expected = await domainSeparator(this.domain); + describe('domain separator', function () { + it('is internally available', async function () { + const expected = await domainSeparator(this.domain); - expect(await this.eip712.$_domainSeparatorV4()).to.equal(expected); + expect(await this.eip712.$_domainSeparatorV4()).to.equal(expected); + }); + + 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)); + }); }); it('hash digest', async function () { const structhash = web3.utils.randomHex(32); - const expected = await hashTypedData(this.domain, structhash); - - expect(await this.eip712.$_hashTypedDataV4(structhash)).to.be.equal(expected); + expect(await this.eip712.$_hashTypedDataV4(structhash)).to.be.equal(hashTypedData(this.domain, structhash)); }); it('digest', async function () { @@ -44,7 +51,7 @@ contract('EIP712', function (accounts) { const data = { types: { - EIP712Domain, + EIP712Domain: this.domainType, Mail: [ { name: 'to', type: 'address' }, { name: 'contents', type: 'string' },