Преглед изворни кода

Add clone variant with per-instance immutable arguments (#5109)

Co-authored-by: Ernesto García <ernestognw@gmail.com>
Hadrien Croubois пре 1 година
родитељ
комит
cb7faaf4db
4 измењених фајлова са 331 додато и 73 уклоњено
  1. 5 0
      .changeset/four-chairs-help.md
  2. 141 0
      contracts/proxy/Clones.sol
  3. 34 2
      test/proxy/Clones.t.sol
  4. 151 71
      test/proxy/Clones.test.js

+ 5 - 0
.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.

+ 141 - 0
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
+            );
+    }
 }

+ 34 - 2
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)))

+ 151 - 71
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',
+    );
   });
 });