From cb7faaf4db9d1ea443b507311487625220e5e215 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 4 Sep 2024 09:41:40 +0200 Subject: [PATCH] Add clone variant with per-instance immutable arguments (#5109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto García --- .changeset/four-chairs-help.md | 5 + contracts/proxy/Clones.sol | 141 +++++++++++++++++++++ test/proxy/Clones.t.sol | 36 +++++- test/proxy/Clones.test.js | 222 ++++++++++++++++++++++----------- 4 files changed, 331 insertions(+), 73 deletions(-) create mode 100644 .changeset/four-chairs-help.md diff --git a/.changeset/four-chairs-help.md b/.changeset/four-chairs-help.md new file mode 100644 index 00000000000..cbd0076075e --- /dev/null +++ b/.changeset/four-chairs-help.md @@ -0,0 +1,5 @@ +--- +"openzeppelin-solidity": minor +--- + +`Clones`: Add `cloneWithImmutableArgs` and `cloneDeterministicWithImmutableArgs` variants that create clones with per-instance immutable arguments. The immutable arguments can be retrieved using `fetchCloneArgs`. The corresponding `predictDeterministicWithImmutableArgs` function is also included. diff --git a/contracts/proxy/Clones.sol b/contracts/proxy/Clones.sol index 097b43b43b3..454d7fb97d2 100644 --- a/contracts/proxy/Clones.sol +++ b/contracts/proxy/Clones.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; +import {Create2} from "../utils/Create2.sol"; import {Errors} from "../utils/Errors.sol"; /** @@ -17,6 +18,8 @@ import {Errors} from "../utils/Errors.sol"; * deterministic method. */ library Clones { + error CloneArgumentsTooLong(); + /** * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`. * @@ -118,4 +121,142 @@ library Clones { ) internal view returns (address predicted) { return predictDeterministicAddress(implementation, salt, address(this)); } + + /** + * @dev Deploys and returns the address of a clone that mimics the behavior of `implementation` with custom + * immutable arguments. These are provided through `args` and cannot be changed after deployment. To + * access the arguments within the implementation, use {fetchCloneArgs}. + * + * This function uses the create opcode, which should never revert. + */ + function cloneWithImmutableArgs(address implementation, bytes memory args) internal returns (address instance) { + return cloneWithImmutableArgs(implementation, args, 0); + } + + /** + * @dev Same as {xref-Clones-cloneWithImmutableArgs-address-bytes-}[cloneWithImmutableArgs], 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 cloneWithImmutableArgs( + address implementation, + bytes memory args, + uint256 value + ) internal returns (address instance) { + if (address(this).balance < value) { + revert Errors.InsufficientBalance(address(this).balance, value); + } + bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args); + assembly ("memory-safe") { + instance := create(value, add(bytecode, 0x20), mload(bytecode)) + } + if (instance == address(0)) { + revert Errors.FailedDeployment(); + } + } + + /** + * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation` with custom + * immutable arguments. These are provided through `args` and cannot be changed after deployment. To + * access the arguments within the implementation, use {fetchCloneArgs}. + * + * This function uses the create2 opcode and a `salt` to deterministically deploy the clone. Using the same + * `implementation` and `salt` multiple time will revert, since the clones cannot be deployed twice at the same + * address. + */ + function cloneDeterministicWithImmutableArgs( + address implementation, + bytes memory args, + bytes32 salt + ) internal returns (address instance) { + return cloneDeterministicWithImmutableArgs(implementation, args, salt, 0); + } + + /** + * @dev Same as {xref-Clones-cloneDeterministicWithImmutableArgs-address-bytes-bytes32-}[cloneDeterministicWithImmutableArgs], + * 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 cloneDeterministicWithImmutableArgs( + address implementation, + bytes memory args, + bytes32 salt, + uint256 value + ) internal returns (address instance) { + bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args); + return Create2.deploy(value, salt, bytecode); + } + + /** + * @dev Computes the address of a clone deployed using {Clones-cloneDeterministicWithImmutableArgs}. + */ + function predictDeterministicAddressWithImmutableArgs( + address implementation, + bytes memory args, + bytes32 salt, + address deployer + ) internal pure returns (address predicted) { + bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args); + return Create2.computeAddress(salt, keccak256(bytecode), deployer); + } + + /** + * @dev Computes the address of a clone deployed using {Clones-cloneDeterministicWithImmutableArgs}. + */ + function predictDeterministicAddressWithImmutableArgs( + address implementation, + bytes memory args, + bytes32 salt + ) internal view returns (address predicted) { + return predictDeterministicAddressWithImmutableArgs(implementation, args, salt, address(this)); + } + + /** + * @dev Get the immutable args attached to a clone. + * + * - If `instance` is a clone that was deployed using `clone` or `cloneDeterministic`, this + * function will return an empty array. + * - If `instance` is a clone that was deployed using `cloneWithImmutableArgs` or + * `cloneDeterministicWithImmutableArgs`, this function will return the args array used at + * creation. + * - If `instance` is NOT a clone deployed using this library, the behavior is undefined. This + * function should only be used to check addresses that are known to be clones. + */ + function fetchCloneArgs(address instance) internal view returns (bytes memory) { + bytes memory result = new bytes(instance.code.length - 0x2d); // revert if length is too short + assembly ("memory-safe") { + extcodecopy(instance, add(result, 0x20), 0x2d, mload(result)) + } + return result; + } + + /** + * @dev Helper that prepares the initcode of the proxy with immutable args. + * + * An assembly variant of this function requires copying the `args` array, which can be efficiently done using + * `mcopy`. Unfortunately, that opcode is not available before cancun. A pure solidity implementation using + * abi.encodePacked is more expensive but also more portable and easier to review. + * + * NOTE: https://eips.ethereum.org/EIPS/eip-170[EIP-170] limits the length of the contract code to 24576 bytes. + * With the proxy code taking 45 bytes, that limits the length of the immutable args to 24531 bytes. + */ + function _cloneCodeWithImmutableArgs( + address implementation, + bytes memory args + ) private pure returns (bytes memory) { + if (args.length > 0x5fd3) revert CloneArgumentsTooLong(); + return + abi.encodePacked( + hex"61", + uint16(args.length + 0x2d), + hex"3d81600a3d39f3363d3d373d3d3d363d73", + implementation, + hex"5af43d82803e903d91602b57fd5bf3", + args + ); + } } diff --git a/test/proxy/Clones.t.sol b/test/proxy/Clones.t.sol index 31b072b98de..e589ba90678 100644 --- a/test/proxy/Clones.t.sol +++ b/test/proxy/Clones.t.sol @@ -19,12 +19,28 @@ contract ClonesTest is Test { assertEq(spillage, bytes32(0)); } + function testSymbolicPredictDeterministicAddressWithImmutableArgsSpillage( + address implementation, + bytes32 salt, + bytes memory args + ) public { + vm.assume(args.length < 0xbfd3); + + address predicted = Clones.predictDeterministicAddressWithImmutableArgs(implementation, args, salt); + bytes32 spillage; + /// @solidity memory-safe-assembly + assembly { + spillage := and(predicted, 0xffffffffffffffffffffffff0000000000000000000000000000000000000000) + } + assertEq(spillage, bytes32(0)); + } + function testCloneDirty() external { address cloneClean = Clones.clone(address(this)); address cloneDirty = Clones.clone(_dirty(address(this))); // both clones have the same code - assertEq(keccak256(cloneClean.code), keccak256(cloneDirty.code)); + assertEq(cloneClean.code, cloneDirty.code); // both clones behave as expected assertEq(ClonesTest(cloneClean).getNumber(), this.getNumber()); @@ -36,7 +52,7 @@ contract ClonesTest is Test { address cloneDirty = Clones.cloneDeterministic(_dirty(address(this)), ~salt); // both clones have the same code - assertEq(keccak256(cloneClean.code), keccak256(cloneDirty.code)); + assertEq(cloneClean.code, cloneDirty.code); // both clones behave as expected assertEq(ClonesTest(cloneClean).getNumber(), this.getNumber()); @@ -51,6 +67,22 @@ contract ClonesTest is Test { assertEq(predictClean, predictDirty); } + function testFetchCloneArgs(bytes memory args, bytes32 salt) external { + vm.assume(args.length < 0xbfd3); + + address instance1 = Clones.cloneWithImmutableArgs(address(this), args); + address instance2 = Clones.cloneDeterministicWithImmutableArgs(address(this), args, salt); + + // both clones have the same code + assertEq(instance1.code, instance2.code); + + // both clones behave as expected and args can be fetched + assertEq(ClonesTest(instance1).getNumber(), this.getNumber()); + assertEq(ClonesTest(instance2).getNumber(), this.getNumber()); + assertEq(Clones.fetchCloneArgs(instance1), args); + assertEq(Clones.fetchCloneArgs(instance2), args); + } + function _dirty(address input) private pure returns (address output) { assembly ("memory-safe") { output := or(input, shl(160, not(0))) diff --git a/test/proxy/Clones.test.js b/test/proxy/Clones.test.js index 70220fbf7a0..0706c6f834c 100644 --- a/test/proxy/Clones.test.js +++ b/test/proxy/Clones.test.js @@ -2,38 +2,77 @@ const { ethers } = require('hardhat'); const { expect } = require('chai'); const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { generators } = require('../helpers/random'); + const shouldBehaveLikeClone = require('./Clones.behaviour'); +const cloneInitCode = (instance, args = undefined) => + args + ? ethers.concat([ + '0x61', + ethers.toBeHex(0x2d + ethers.getBytes(args).length, 2), + '0x3d81600a3d39f3363d3d373d3d3d363d73', + instance.target ?? instance.address ?? instance, + '0x5af43d82803e903d91602b57fd5bf3', + args, + ]) + : ethers.concat([ + '0x3d602d80600a3d3981f3363d3d373d3d3d363d73', + instance.target ?? instance.address ?? instance, + '0x5af43d82803e903d91602b57fd5bf3', + ]); + async function fixture() { const [deployer] = await ethers.getSigners(); const factory = await ethers.deployContract('$Clones'); const implementation = await ethers.deployContract('DummyImplementation'); - const newClone = async (opts = {}) => { - const clone = await factory.$clone.staticCall(implementation).then(address => implementation.attach(address)); - 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 (opts = {}) => { - const salt = opts.salt ?? ethers.randomBytes(32); - const clone = await factory.$cloneDeterministic - .staticCall(implementation, salt) - .then(address => implementation.attach(address)); - 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 }); - }; + const newClone = + args => + async (opts = {}) => { + const clone = await (args + ? factory.$cloneWithImmutableArgs.staticCall(implementation, args) + : factory.$clone.staticCall(implementation) + ).then(address => implementation.attach(address)); + const tx = await (args + ? opts.deployValue + ? factory.$cloneWithImmutableArgs(implementation, args, ethers.Typed.uint256(opts.deployValue)) + : factory.$cloneWithImmutableArgs(implementation, args) + : 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 = + args => + async (opts = {}) => { + const salt = opts.salt ?? ethers.randomBytes(32); + const clone = await (args + ? factory.$cloneDeterministicWithImmutableArgs.staticCall(implementation, args, salt) + : factory.$cloneDeterministic.staticCall(implementation, salt) + ).then(address => implementation.attach(address)); + const tx = await (args + ? opts.deployValue + ? factory.$cloneDeterministicWithImmutableArgs( + implementation, + args, + salt, + ethers.Typed.uint256(opts.deployValue), + ) + : factory.$cloneDeterministicWithImmutableArgs(implementation, args, salt) + : 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 }; } @@ -43,53 +82,94 @@ describe('Clones', function () { Object.assign(this, await loadFixture(fixture)); }); - describe('clone', function () { - beforeEach(async function () { - this.createClone = this.newClone; - }); - - shouldBehaveLikeClone(); - }); - - describe('cloneDeterministic', function () { - beforeEach(async function () { - this.createClone = this.newCloneDeterministic; - }); - - shouldBehaveLikeClone(); - - it('revert if address already used', async function () { - const salt = ethers.randomBytes(32); - - // deploy once - await expect(this.factory.$cloneDeterministic(this.implementation, salt)).to.emit( - this.factory, - 'return$cloneDeterministic_address_bytes32', - ); - - // deploy twice - await expect(this.factory.$cloneDeterministic(this.implementation, salt)).to.be.revertedWithCustomError( - this.factory, - 'FailedDeployment', - ); - }); - - it('address prediction', async function () { - const salt = ethers.randomBytes(32); - - const creationCode = ethers.concat([ - '0x3d602d80600a3d3981f3363d3d373d3d3d363d73', - this.implementation.target, - '0x5af43d82803e903d91602b57fd5bf3', - ]); - - const predicted = await this.factory.$predictDeterministicAddress(this.implementation, salt); - const expected = ethers.getCreate2Address(this.factory.target, salt, ethers.keccak256(creationCode)); - expect(predicted).to.equal(expected); - - await expect(this.factory.$cloneDeterministic(this.implementation, salt)) - .to.emit(this.factory, 'return$cloneDeterministic_address_bytes32') - .withArgs(predicted); + for (const args of [undefined, '0x', '0x11223344']) { + describe(args ? `with immutable args: ${args}` : 'without immutable args', function () { + describe('clone', function () { + beforeEach(async function () { + this.createClone = this.newClone(args); + }); + + shouldBehaveLikeClone(); + + it('get immutable arguments', async function () { + const instance = await this.createClone(); + expect(await this.factory.$fetchCloneArgs(instance)).to.equal(args ?? '0x'); + }); + }); + + describe('cloneDeterministic', function () { + beforeEach(async function () { + this.createClone = this.newCloneDeterministic(args); + }); + + shouldBehaveLikeClone(); + + it('get immutable arguments', async function () { + const instance = await this.createClone(); + expect(await this.factory.$fetchCloneArgs(instance)).to.equal(args ?? '0x'); + }); + + it('revert if address already used', async function () { + const salt = ethers.randomBytes(32); + + const deployClone = () => + args + ? this.factory.$cloneDeterministicWithImmutableArgs(this.implementation, args, salt) + : this.factory.$cloneDeterministic(this.implementation, salt); + + // deploy once + await expect(deployClone()).to.not.be.reverted; + + // deploy twice + await expect(deployClone()).to.be.revertedWithCustomError(this.factory, 'FailedDeployment'); + }); + + it('address prediction', async function () { + const salt = ethers.randomBytes(32); + + const expected = ethers.getCreate2Address( + this.factory.target, + salt, + ethers.keccak256(cloneInitCode(this.implementation, args)), + ); + + if (args) { + const predicted = await this.factory.$predictDeterministicAddressWithImmutableArgs( + this.implementation, + args, + salt, + ); + expect(predicted).to.equal(expected); + + await expect(this.factory.$cloneDeterministicWithImmutableArgs(this.implementation, args, salt)) + .to.emit(this.factory, 'return$cloneDeterministicWithImmutableArgs_address_bytes_bytes32') + .withArgs(predicted); + } else { + const predicted = await this.factory.$predictDeterministicAddress(this.implementation, salt); + expect(predicted).to.equal(expected); + + await expect(this.factory.$cloneDeterministic(this.implementation, salt)) + .to.emit(this.factory, 'return$cloneDeterministic_address_bytes32') + .withArgs(predicted); + } + }); + }); }); + } + + it('EIP-170 limit on immutable args', async function () { + // EIP-170 limits the contract code size to 0x6000 + // This limits the length of immutable args to 0x5fd3 + const args = generators.hexBytes(0x5fd4); + const salt = ethers.randomBytes(32); + + await expect( + this.factory.$predictDeterministicAddressWithImmutableArgs(this.implementation, args, salt), + ).to.be.revertedWithCustomError(this.factory, 'CloneArgumentsTooLong'); + + await expect(this.factory.$cloneWithImmutableArgs(this.implementation, args)).to.be.revertedWithCustomError( + this.factory, + 'CloneArgumentsTooLong', + ); }); });