diff --git a/.changeset/chilled-walls-develop.md b/.changeset/chilled-walls-develop.md new file mode 100644 index 000000000..4108feb61 --- /dev/null +++ b/.changeset/chilled-walls-develop.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`Clones`: Add version of `clone` and `cloneDeterministic` that support sending value at creation. diff --git a/.changeset/strong-singers-talk.md b/.changeset/strong-singers-talk.md new file mode 100644 index 000000000..7897980cb --- /dev/null +++ b/.changeset/strong-singers-talk.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`Errors`: New library of common custom errors. diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ffaa1b9b..147af99fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +### Custom error changes + +This version comes with changes to the custom error identifiers. Contracts previously depending on the following errors should be replaced accordingly: + +- Replace `Address.FailedInnerCall` with `Errors.FailedCall` +- Replace `Address.AddressInsufficientBalance` with `Errors.InsufficientBalance` +- Replace `Clones.Create2InsufficientBalance` with `Errors.InsufficientBalance` +- Replace `Clones.ERC1167FailedCreateClone` with `Errors.FailedDeployment` +- Replace `Clones.Create2FailedDeployment` with `Errors.FailedDeployment` ## 5.0.2 (2024-02-29) diff --git a/README.md b/README.md index 2f4609f00..fa7b4e31e 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ $ npm install @openzeppelin/contracts $ forge install OpenZeppelin/openzeppelin-contracts ``` -Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.` +Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.` ### Usage diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol index 221dabb62..fe2c9d1d5 100644 --- a/contracts/metatx/ERC2771Forwarder.sol +++ b/contracts/metatx/ERC2771Forwarder.sol @@ -8,6 +8,7 @@ import {ECDSA} from "../utils/cryptography/ECDSA.sol"; import {EIP712} from "../utils/cryptography/EIP712.sol"; import {Nonces} from "../utils/Nonces.sol"; import {Address} from "../utils/Address.sol"; +import {Errors} from "../utils/Errors.sol"; /** * @dev A forwarder compatible with ERC-2771 contracts. See {ERC2771Context}. @@ -132,7 +133,7 @@ contract ERC2771Forwarder is EIP712, Nonces { } if (!_execute(request, true)) { - revert Address.FailedInnerCall(); + revert Errors.FailedCall(); } } diff --git a/contracts/proxy/Clones.sol b/contracts/proxy/Clones.sol index ffbc8f10c..43532dcab 100644 --- a/contracts/proxy/Clones.sol +++ b/contracts/proxy/Clones.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.20; +import {Errors} from "../utils/Errors.sol"; + /** * @dev https://eips.ethereum.org/EIPS/eip-1167[ERC-1167] is a standard for * deploying minimal proxy contracts, also known as "clones". @@ -15,17 +17,26 @@ pragma solidity ^0.8.20; * deterministic method. */ library Clones { - /** - * @dev A clone instance deployment failed. - */ - error ERC1167FailedCreateClone(); - /** * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`. * * This function uses the create opcode, which should never revert. */ function clone(address implementation) internal returns (address instance) { + return clone(implementation, 0); + } + + /** + * @dev Same as {xref-Clones-clone-address-}[clone], but with a `value` parameter to send native currency + * to the new contract. + * + * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) + * to always have enough balance for new deployments. Consider exposing this function under a payable method. + */ + function clone(address implementation, uint256 value) internal returns (address instance) { + if (address(this).balance < value) { + revert Errors.InsufficientBalance(address(this).balance, value); + } /// @solidity memory-safe-assembly assembly { // Stores the bytecode after address @@ -34,10 +45,10 @@ library Clones { mstore(0x11, implementation) // Packs the first 3 bytes of the `implementation` address with the bytecode before the address. mstore(0x00, or(shr(0x88, implementation), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000)) - instance := create(0, 0x09, 0x37) + instance := create(value, 0x09, 0x37) } if (instance == address(0)) { - revert ERC1167FailedCreateClone(); + revert Errors.FailedDeployment(); } } @@ -49,6 +60,24 @@ library Clones { * the clones cannot be deployed twice at the same address. */ function cloneDeterministic(address implementation, bytes32 salt) internal returns (address instance) { + return cloneDeterministic(implementation, salt, 0); + } + + /** + * @dev Same as {xref-Clones-cloneDeterministic-address-bytes32-}[cloneDeterministic], but with + * a `value` parameter to send native currency to the new contract. + * + * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) + * to always have enough balance for new deployments. Consider exposing this function under a payable method. + */ + function cloneDeterministic( + address implementation, + bytes32 salt, + uint256 value + ) internal returns (address instance) { + if (address(this).balance < value) { + revert Errors.InsufficientBalance(address(this).balance, value); + } /// @solidity memory-safe-assembly assembly { // Stores the bytecode after address @@ -57,10 +86,10 @@ library Clones { mstore(0x11, implementation) // Packs the first 3 bytes of the `implementation` address with the bytecode before the address. mstore(0x00, or(shr(0x88, implementation), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000)) - instance := create2(0, 0x09, 0x37, salt) + instance := create2(value, 0x09, 0x37, salt) } if (instance == address(0)) { - revert ERC1167FailedCreateClone(); + revert Errors.FailedDeployment(); } } diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index b7e305952..53a3c4420 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -3,25 +3,17 @@ pragma solidity ^0.8.20; +import {Errors} from "./Errors.sol"; + /** * @dev Collection of functions related to the address type */ library Address { - /** - * @dev The ETH balance of the account is not enough to perform the operation. - */ - error AddressInsufficientBalance(address account); - /** * @dev There's no code at `target` (it is not a contract). */ error AddressEmptyCode(address target); - /** - * @dev A call to an address target failed. The target may have reverted. - */ - error FailedInnerCall(); - /** * @dev Replacement for Solidity's `transfer`: sends `amount` wei to * `recipient`, forwarding all available gas and reverting on errors. @@ -40,12 +32,12 @@ library Address { */ function sendValue(address payable recipient, uint256 amount) internal { if (address(this).balance < amount) { - revert AddressInsufficientBalance(address(this)); + revert Errors.InsufficientBalance(address(this).balance, amount); } (bool success, ) = recipient.call{value: amount}(""); if (!success) { - revert FailedInnerCall(); + revert Errors.FailedCall(); } } @@ -57,7 +49,7 @@ library Address { * If `target` reverts with a revert reason or custom error, it is bubbled * up by this function (like regular Solidity function calls). However, if * the call reverted with no returned reason, this function reverts with a - * {FailedInnerCall} error. + * {Errors.FailedCall} error. * * Returns the raw returned data. To convert to the expected return value, * use https://solidity.readthedocs.io/en/latest/units-and-global-variables.html?highlight=abi.decode#abi-encoding-and-decoding-functions[`abi.decode`]. @@ -82,7 +74,7 @@ library Address { */ function functionCallWithValue(address target, bytes memory data, uint256 value) internal returns (bytes memory) { if (address(this).balance < value) { - revert AddressInsufficientBalance(address(this)); + revert Errors.InsufficientBalance(address(this).balance, value); } (bool success, bytes memory returndata) = target.call{value: value}(data); return verifyCallResultFromTarget(target, success, returndata); @@ -108,8 +100,8 @@ library Address { /** * @dev Tool to verify that a low level call to smart-contract was successful, and reverts if the target - * was not a contract or bubbling up the revert reason (falling back to {FailedInnerCall}) in case of an - * unsuccessful call. + * was not a contract or bubbling up the revert reason (falling back to {Errors.FailedCall}) in case + * of an unsuccessful call. */ function verifyCallResultFromTarget( address target, @@ -130,7 +122,7 @@ library Address { /** * @dev Tool to verify that a low level call was successful, and reverts if it wasn't, either by bubbling the - * revert reason or with a default {FailedInnerCall} error. + * revert reason or with a default {Errors.FailedCall} error. */ function verifyCallResult(bool success, bytes memory returndata) internal pure returns (bytes memory) { if (!success) { @@ -141,7 +133,7 @@ library Address { } /** - * @dev Reverts with returndata if present. Otherwise reverts with {FailedInnerCall}. + * @dev Reverts with returndata if present. Otherwise reverts with {Errors.FailedCall}. */ function _revert(bytes memory returndata) private pure { // Look for revert reason and bubble it up if present @@ -153,7 +145,7 @@ library Address { revert(add(32, returndata), returndata_size) } } else { - revert FailedInnerCall(); + revert Errors.FailedCall(); } } } diff --git a/contracts/utils/Create2.sol b/contracts/utils/Create2.sol index ad1cd5f4d..b98253bf3 100644 --- a/contracts/utils/Create2.sol +++ b/contracts/utils/Create2.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.20; +import {Errors} from "./Errors.sol"; + /** * @dev Helper to make usage of the `CREATE2` EVM opcode easier and safer. * `CREATE2` can be used to compute in advance the address where a smart @@ -13,21 +15,11 @@ pragma solidity ^0.8.20; * information. */ library Create2 { - /** - * @dev Not enough balance for performing a CREATE2 deploy. - */ - error Create2InsufficientBalance(uint256 balance, uint256 needed); - /** * @dev There's no code to deploy. */ error Create2EmptyBytecode(); - /** - * @dev The deployment failed. - */ - error Create2FailedDeployment(); - /** * @dev Deploys a contract using `CREATE2`. The address where the contract * will be deployed can be known in advance via {computeAddress}. @@ -44,7 +36,7 @@ library Create2 { */ function deploy(uint256 amount, bytes32 salt, bytes memory bytecode) internal returns (address addr) { if (address(this).balance < amount) { - revert Create2InsufficientBalance(address(this).balance, amount); + revert Errors.InsufficientBalance(address(this).balance, amount); } if (bytecode.length == 0) { revert Create2EmptyBytecode(); @@ -54,7 +46,7 @@ library Create2 { addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt) } if (addr == address(0)) { - revert Create2FailedDeployment(); + revert Errors.FailedDeployment(); } } diff --git a/contracts/utils/Errors.sol b/contracts/utils/Errors.sol new file mode 100644 index 000000000..024109444 --- /dev/null +++ b/contracts/utils/Errors.sol @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +/** + * @dev Collection of common custom errors used in multiple contracts + * + * IMPORTANT: Backwards compatibility is not guaranteed in future versions of the library. + * It is recommended to avoid relying on the error API for critical functionality. + */ +library Errors { + /** + * @dev The ETH balance of the account is not enough to perform the operation. + */ + error InsufficientBalance(uint256 balance, uint256 needed); + + /** + * @dev A call to an address target failed. The target may have reverted. + */ + error FailedCall(); + + /** + * @dev The deployment failed. + */ + error FailedDeployment(); +} diff --git a/scripts/upgradeable/upgradeable.patch b/scripts/upgradeable/upgradeable.patch index 46893d7d2..458ecd435 100644 --- a/scripts/upgradeable/upgradeable.patch +++ b/scripts/upgradeable/upgradeable.patch @@ -59,7 +59,7 @@ index ff596b0c3..000000000 - - diff --git a/README.md b/README.md -index 35083bc6e..05cf4fc27 100644 +index fa7b4e31e..4799b6376 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,9 @@ @@ -89,8 +89,8 @@ index 35083bc6e..05cf4fc27 100644 +$ forge install OpenZeppelin/openzeppelin-contracts-upgradeable ``` --Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.` -+Add `@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/` in `remappings.txt.` +-Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.` ++Add `@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/` in `remappings.txt.` ### Usage @@ -110,7 +110,7 @@ index 35083bc6e..05cf4fc27 100644 } ``` diff --git a/contracts/package.json b/contracts/package.json -index 6ab89138a..ece834a44 100644 +index 845e8c403..8dc181b91 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -1,5 +1,5 @@ @@ -118,7 +118,7 @@ index 6ab89138a..ece834a44 100644 - "name": "@openzeppelin/contracts", + "name": "@openzeppelin/contracts-upgradeable", "description": "Secure Smart Contract library for Solidity", - "version": "5.0.1", + "version": "5.0.2", "files": [ @@ -13,7 +13,7 @@ }, @@ -307,7 +307,7 @@ index 77c4c8990..602467f40 100644 } } diff --git a/package.json b/package.json -index ec2c44ced..46eedc98f 100644 +index c4b358e10..96ab2559c 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,7 @@ @@ -328,7 +328,7 @@ 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 166038b36..268e0d29d 100644 +index 2b6e7fa97..268e0d29d 100644 --- a/test/utils/cryptography/EIP712.test.js +++ b/test/utils/cryptography/EIP712.test.js @@ -47,27 +47,6 @@ describe('EIP712', function () { @@ -346,7 +346,7 @@ index 166038b36..268e0d29d 100644 - 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(receipt => receipt.logs.find(ev => ev.fragment.name == 'return$clone_address').args.instance) - .then(address => ethers.getContractAt('$EIP712Verifier', address)); - - const expectedDomain = { ...this.domain, verifyingContract: clone.target }; diff --git a/test/finance/VestingWallet.behavior.js b/test/finance/VestingWallet.behavior.js index 819a47ff6..b45ffeecf 100644 --- a/test/finance/VestingWallet.behavior.js +++ b/test/finance/VestingWallet.behavior.js @@ -12,7 +12,7 @@ async function envSetup(mock, beneficiary, token) { const beneficiaryMock = await ethers.deployContract('EtherReceiverMock'); await beneficiaryMock.setAcceptEther(false); await mock.connect(beneficiary).transferOwnership(beneficiaryMock); - return { args: [], error: [mock, 'FailedInnerCall'] }; + return { args: [], error: [mock, 'FailedCall'] }; }, releasedEvent: 'EtherReleased', args: [], diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index e097ef0ef..3e48ccfee 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -445,7 +445,7 @@ describe('Governor', function () { await this.helper.waitForSnapshot(); await this.helper.connect(this.voter1).vote({ support: VoteType.For }); await this.helper.waitForDeadline(); - await expect(this.helper.execute()).to.be.revertedWithCustomError(this.mock, 'FailedInnerCall'); + await expect(this.helper.execute()).to.be.revertedWithCustomError(this.mock, 'FailedCall'); }); it('if receiver revert with reason', async function () { diff --git a/test/governance/TimelockController.test.js b/test/governance/TimelockController.test.js index f7ba96c82..a62cbd444 100644 --- a/test/governance/TimelockController.test.js +++ b/test/governance/TimelockController.test.js @@ -893,7 +893,7 @@ describe('TimelockController', function () { operation.predecessor, operation.salt, ), - ).to.be.revertedWithCustomError(this.mock, 'FailedInnerCall'); + ).to.be.revertedWithCustomError(this.mock, 'FailedCall'); }); }); }); @@ -1099,7 +1099,7 @@ describe('TimelockController', function () { this.mock .connect(this.executor) .execute(operation.target, operation.value, operation.data, operation.predecessor, operation.salt), - ).to.be.revertedWithCustomError(this.mock, 'FailedInnerCall'); + ).to.be.revertedWithCustomError(this.mock, 'FailedCall'); }); it('call throw', async function () { @@ -1146,7 +1146,7 @@ describe('TimelockController', function () { .execute(operation.target, operation.value, operation.data, operation.predecessor, operation.salt, { gasLimit: '100000', }), - ).to.be.revertedWithCustomError(this.mock, 'FailedInnerCall'); + ).to.be.revertedWithCustomError(this.mock, 'FailedCall'); }); it('call payable with eth', async function () { @@ -1199,7 +1199,7 @@ describe('TimelockController', function () { this.mock .connect(this.executor) .execute(operation.target, operation.value, operation.data, operation.predecessor, operation.salt), - ).to.be.revertedWithCustomError(this.mock, 'FailedInnerCall'); + ).to.be.revertedWithCustomError(this.mock, 'FailedCall'); expect(await ethers.provider.getBalance(this.mock)).to.equal(0n); expect(await ethers.provider.getBalance(this.callreceivermock)).to.equal(0n); @@ -1227,7 +1227,7 @@ describe('TimelockController', function () { this.mock .connect(this.executor) .execute(operation.target, operation.value, operation.data, operation.predecessor, operation.salt), - ).to.be.revertedWithCustomError(this.mock, 'FailedInnerCall'); + ).to.be.revertedWithCustomError(this.mock, 'FailedCall'); expect(await ethers.provider.getBalance(this.mock)).to.equal(0n); expect(await ethers.provider.getBalance(this.callreceivermock)).to.equal(0n); diff --git a/test/metatx/ERC2771Forwarder.test.js b/test/metatx/ERC2771Forwarder.test.js index 8a2a09233..8653ad708 100644 --- a/test/metatx/ERC2771Forwarder.test.js +++ b/test/metatx/ERC2771Forwarder.test.js @@ -122,7 +122,7 @@ describe('ERC2771Forwarder', function () { data: this.receiver.interface.encodeFunctionData('mockFunctionRevertsNoReason'), }); - await expect(this.forwarder.execute(request)).to.be.revertedWithCustomError(this.forwarder, 'FailedInnerCall'); + await expect(this.forwarder.execute(request)).to.be.revertedWithCustomError(this.forwarder, 'FailedCall'); }); }); diff --git a/test/proxy/Clones.behaviour.js b/test/proxy/Clones.behaviour.js index 861fae8a2..dcc620665 100644 --- a/test/proxy/Clones.behaviour.js +++ b/test/proxy/Clones.behaviour.js @@ -13,6 +13,25 @@ module.exports = function shouldBehaveLikeClone() { }); }; + describe('construct with value', function () { + const value = 10n; + + it('factory has enough balance', async function () { + await this.deployer.sendTransaction({ to: this.factory, value }); + + const instance = await this.createClone({ deployValue: value }); + await expect(instance.deploymentTransaction()).to.changeEtherBalances([this.factory, instance], [-value, value]); + + expect(await ethers.provider.getBalance(instance)).to.equal(value); + }); + + it('factory does not have enough balance', async function () { + await expect(this.createClone({ deployValue: value })) + .to.be.revertedWithCustomError(this.factory, 'InsufficientBalance') + .withArgs(0n, value); + }); + }); + describe('initialization without parameters', function () { describe('non payable', function () { const expectedInitializedValue = 10n; @@ -23,7 +42,7 @@ module.exports = function shouldBehaveLikeClone() { describe('when not sending balance', function () { beforeEach('creating proxy', async function () { - this.proxy = await this.createClone(this.initializeData); + this.proxy = await this.createClone({ initData: this.initializeData }); }); assertProxyInitialization({ @@ -36,7 +55,7 @@ module.exports = function shouldBehaveLikeClone() { const value = 10n ** 6n; it('reverts', async function () { - await expect(this.createClone(this.initializeData, { value })).to.be.reverted; + await expect(this.createClone({ initData: this.initializeData, initValue: value })).to.be.reverted; }); }); }); @@ -50,7 +69,7 @@ module.exports = function shouldBehaveLikeClone() { describe('when not sending balance', function () { beforeEach('creating proxy', async function () { - this.proxy = await this.createClone(this.initializeData); + this.proxy = await this.createClone({ initData: this.initializeData }); }); assertProxyInitialization({ @@ -63,7 +82,7 @@ module.exports = function shouldBehaveLikeClone() { const value = 10n ** 6n; beforeEach('creating proxy', async function () { - this.proxy = await this.createClone(this.initializeData, { value }); + this.proxy = await this.createClone({ initData: this.initializeData, initValue: value }); }); assertProxyInitialization({ @@ -86,7 +105,7 @@ module.exports = function shouldBehaveLikeClone() { describe('when not sending balance', function () { beforeEach('creating proxy', async function () { - this.proxy = await this.createClone(this.initializeData); + this.proxy = await this.createClone({ initData: this.initializeData }); }); assertProxyInitialization({ @@ -99,7 +118,7 @@ module.exports = function shouldBehaveLikeClone() { const value = 10n ** 6n; it('reverts', async function () { - await expect(this.createClone(this.initializeData, { value })).to.be.reverted; + await expect(this.createClone({ initData: this.initializeData, initValue: value })).to.be.reverted; }); }); }); @@ -115,7 +134,7 @@ module.exports = function shouldBehaveLikeClone() { describe('when not sending balance', function () { beforeEach('creating proxy', async function () { - this.proxy = await this.createClone(this.initializeData); + this.proxy = await this.createClone({ initData: this.initializeData }); }); assertProxyInitialization({ @@ -128,7 +147,7 @@ module.exports = function shouldBehaveLikeClone() { const value = 10n ** 6n; beforeEach('creating proxy', async function () { - this.proxy = await this.createClone(this.initializeData, { value }); + this.proxy = await this.createClone({ initData: this.initializeData, initValue: value }); }); assertProxyInitialization({ diff --git a/test/proxy/Clones.test.js b/test/proxy/Clones.test.js index 626b1e564..70220fbf7 100644 --- a/test/proxy/Clones.test.js +++ b/test/proxy/Clones.test.js @@ -10,21 +10,29 @@ async function fixture() { const factory = await ethers.deployContract('$Clones'); const implementation = await ethers.deployContract('DummyImplementation'); - const newClone = async (initData, opts = {}) => { + const newClone = async (opts = {}) => { const clone = await factory.$clone.staticCall(implementation).then(address => implementation.attach(address)); - await factory.$clone(implementation); - await deployer.sendTransaction({ to: clone, value: opts.value ?? 0n, data: initData ?? '0x' }); - return clone; + const tx = await (opts.deployValue + ? factory.$clone(implementation, ethers.Typed.uint256(opts.deployValue)) + : factory.$clone(implementation)); + if (opts.initData || opts.initValue) { + await deployer.sendTransaction({ to: clone, value: opts.initValue ?? 0n, data: opts.initData ?? '0x' }); + } + return Object.assign(clone, { deploymentTransaction: () => tx }); }; - const newCloneDeterministic = async (initData, opts = {}) => { + const newCloneDeterministic = async (opts = {}) => { const salt = opts.salt ?? ethers.randomBytes(32); const clone = await factory.$cloneDeterministic .staticCall(implementation, salt) .then(address => implementation.attach(address)); - await factory.$cloneDeterministic(implementation, salt); - await deployer.sendTransaction({ to: clone, value: opts.value ?? 0n, data: initData ?? '0x' }); - return clone; + const tx = await (opts.deployValue + ? factory.$cloneDeterministic(implementation, salt, ethers.Typed.uint256(opts.deployValue)) + : factory.$cloneDeterministic(implementation, salt)); + if (opts.initData || opts.initValue) { + await deployer.sendTransaction({ to: clone, value: opts.initValue ?? 0n, data: opts.initData ?? '0x' }); + } + return Object.assign(clone, { deploymentTransaction: () => tx }); }; return { deployer, factory, implementation, newClone, newCloneDeterministic }; @@ -56,13 +64,13 @@ describe('Clones', function () { // deploy once await expect(this.factory.$cloneDeterministic(this.implementation, salt)).to.emit( this.factory, - 'return$cloneDeterministic', + 'return$cloneDeterministic_address_bytes32', ); // deploy twice await expect(this.factory.$cloneDeterministic(this.implementation, salt)).to.be.revertedWithCustomError( this.factory, - 'ERC1167FailedCreateClone', + 'FailedDeployment', ); }); @@ -80,7 +88,7 @@ describe('Clones', function () { expect(predicted).to.equal(expected); await expect(this.factory.$cloneDeterministic(this.implementation, salt)) - .to.emit(this.factory, 'return$cloneDeterministic') + .to.emit(this.factory, 'return$cloneDeterministic_address_bytes32') .withArgs(predicted); }); }); diff --git a/test/utils/Address.test.js b/test/utils/Address.test.js index bdfd64bf9..21775397a 100644 --- a/test/utils/Address.test.js +++ b/test/utils/Address.test.js @@ -23,13 +23,13 @@ describe('Address', function () { describe('sendValue', function () { describe('when sender contract has no funds', function () { it('sends 0 wei', async function () { - await expect(this.mock.$sendValue(this.other, 0)).to.changeEtherBalance(this.recipient, 0); + await expect(this.mock.$sendValue(this.other, 0n)).to.changeEtherBalance(this.recipient, 0n); }); it('reverts when sending non-zero amounts', async function () { - await expect(this.mock.$sendValue(this.other, 1)) - .to.be.revertedWithCustomError(this.mock, 'AddressInsufficientBalance') - .withArgs(this.mock); + await expect(this.mock.$sendValue(this.other, 1n)) + .to.be.revertedWithCustomError(this.mock, 'InsufficientBalance') + .withArgs(0n, 1n); }); }); @@ -42,7 +42,7 @@ describe('Address', function () { describe('with EOA recipient', function () { it('sends 0 wei', async function () { - await expect(this.mock.$sendValue(this.recipient, 0)).to.changeEtherBalance(this.recipient, 0); + await expect(this.mock.$sendValue(this.recipient, 0n)).to.changeEtherBalance(this.recipient, 0n); }); it('sends non-zero amounts', async function () { @@ -59,8 +59,8 @@ describe('Address', function () { it('reverts when sending more than the balance', async function () { await expect(this.mock.$sendValue(this.recipient, funds + 1n)) - .to.be.revertedWithCustomError(this.mock, 'AddressInsufficientBalance') - .withArgs(this.mock); + .to.be.revertedWithCustomError(this.mock, 'InsufficientBalance') + .withArgs(funds, funds + 1n); }); }); @@ -74,7 +74,7 @@ describe('Address', function () { await this.targetEther.setAcceptEther(false); await expect(this.mock.$sendValue(this.targetEther, funds)).to.be.revertedWithCustomError( this.mock, - 'FailedInnerCall', + 'FailedCall', ); }); }); @@ -101,10 +101,7 @@ describe('Address', function () { it('reverts when the called function reverts with no reason', async function () { const call = this.target.interface.encodeFunctionData('mockFunctionRevertsNoReason'); - await expect(this.mock.$functionCall(this.target, call)).to.be.revertedWithCustomError( - this.mock, - 'FailedInnerCall', - ); + await expect(this.mock.$functionCall(this.target, call)).to.be.revertedWithCustomError(this.mock, 'FailedCall'); }); it('reverts when the called function reverts, bubbling up the revert reason', async function () { @@ -118,7 +115,7 @@ describe('Address', function () { await expect(this.mock.$functionCall(this.target, call, { gasLimit: 120_000n })).to.be.revertedWithCustomError( this.mock, - 'FailedInnerCall', + 'FailedCall', ); }); @@ -132,10 +129,7 @@ describe('Address', function () { const interface = new ethers.Interface(['function mockFunctionDoesNotExist()']); const call = interface.encodeFunctionData('mockFunctionDoesNotExist'); - await expect(this.mock.$functionCall(this.target, call)).to.be.revertedWithCustomError( - this.mock, - 'FailedInnerCall', - ); + await expect(this.mock.$functionCall(this.target, call)).to.be.revertedWithCustomError(this.mock, 'FailedCall'); }); }); @@ -155,7 +149,7 @@ describe('Address', function () { it('calls the requested function', async function () { const call = this.target.interface.encodeFunctionData('mockFunction'); - await expect(this.mock.$functionCallWithValue(this.target, call, 0)) + await expect(this.mock.$functionCallWithValue(this.target, call, 0n)) .to.emit(this.target, 'MockFunctionCalled') .to.emit(this.mock, 'return$functionCallWithValue') .withArgs(coder.encode(['string'], ['0x1234'])); @@ -169,8 +163,8 @@ describe('Address', function () { const call = this.target.interface.encodeFunctionData('mockFunction'); await expect(this.mock.$functionCallWithValue(this.target, call, value)) - .to.be.revertedWithCustomError(this.mock, 'AddressInsufficientBalance') - .withArgs(this.mock); + .to.be.revertedWithCustomError(this.mock, 'InsufficientBalance') + .withArgs(0n, value); }); it('calls the requested function with existing value', async function () { @@ -207,7 +201,7 @@ describe('Address', function () { await expect(this.mock.$functionCallWithValue(this.target, call, value)).to.be.revertedWithCustomError( this.mock, - 'FailedInnerCall', + 'FailedCall', ); }); }); @@ -225,7 +219,7 @@ describe('Address', function () { await expect(this.mock.$functionStaticCall(this.target, call)).to.be.revertedWithCustomError( this.mock, - 'FailedInnerCall', + 'FailedCall', ); }); diff --git a/test/utils/Create2.test.js b/test/utils/Create2.test.js index 86d00e131..aba4deeb0 100644 --- a/test/utils/Create2.test.js +++ b/test/utils/Create2.test.js @@ -114,7 +114,7 @@ describe('Create2', function () { await expect(this.factory.$deploy(0n, saltHex, this.constructorByteCode)).to.be.revertedWithCustomError( this.factory, - 'Create2FailedDeployment', + 'FailedDeployment', ); }); @@ -127,7 +127,7 @@ describe('Create2', function () { it('fails deploying a contract if factory contract does not have sufficient balance', async function () { await expect(this.factory.$deploy(1n, saltHex, this.constructorByteCode)) - .to.be.revertedWithCustomError(this.factory, 'Create2InsufficientBalance') + .to.be.revertedWithCustomError(this.factory, 'InsufficientBalance') .withArgs(0n, 1n); }); }); diff --git a/test/utils/cryptography/EIP712.test.js b/test/utils/cryptography/EIP712.test.js index 166038b36..2b6e7fa97 100644 --- a/test/utils/cryptography/EIP712.test.js +++ b/test/utils/cryptography/EIP712.test.js @@ -58,7 +58,7 @@ describe('EIP712', function () { 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(receipt => receipt.logs.find(ev => ev.fragment.name == 'return$clone_address').args.instance) .then(address => ethers.getContractAt('$EIP712Verifier', address)); const expectedDomain = { ...this.domain, verifyingContract: clone.target };