From 39afaa2627823fd010a3aa426be8f152476d5198 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 4 Jan 2021 11:36:27 +0100 Subject: [PATCH 01/17] Starting work on a cloning library --- contracts/drafts/Clones.sol | 54 +++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 contracts/drafts/Clones.sol diff --git a/contracts/drafts/Clones.sol b/contracts/drafts/Clones.sol new file mode 100644 index 00000000000..c8599f7c33a --- /dev/null +++ b/contracts/drafts/Clones.sol @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: MIT + +pragma solidity >=0.6.0 <0.8.0; + +/** + * @dev https://eips.ethereum.org/EIPS/eip-1167[EIP 1167] is a standard for + * deploying minimal clone contracts. + * + * This contract provide tooling to deploy proxies following the EIP 1167 + * proposed bytecode. This is possible using both create and create2. + */ +library Clones +{ + function clone(address master) internal returns (address instance) { + // solhint-disable-next-line no-inline-assembly + assembly { + let ptr := mload(0x40) + mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000) + mstore(add(ptr, 0x14), shl(0x60, master)) + mstore(add(ptr, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000) + instance := create(0, ptr, 0x37) + } + } + + function clone2(address master, bytes32 salt) internal returns (address instance) { + // solhint-disable-next-line no-inline-assembly + assembly { + let ptr := mload(0x40) + mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000) + mstore(add(ptr, 0x14), shl(0x60, master)) + mstore(add(ptr, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000) + instance := create2(0, ptr, 0x37, salt) + } + require(instance != address(0), "ERC1167: create2 failled"); + } + + function predict2(address master, bytes32 salt, address deployer) internal pure returns (address predicted) { + // solhint-disable-next-line no-inline-assembly + assembly { + let ptr := mload(0x40) + mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000) + mstore(add(ptr, 0x14), shl(0x60, master)) + mstore(add(ptr, 0x28), 0x5af43d82803e903d91602b57fd5bf3ff00000000000000000000000000000000) + mstore(add(ptr, 0x38), shl(0x60, deployer)) + mstore(add(ptr, 0x4c), salt) + mstore(add(ptr, 0x6c), keccak256(ptr, 0x37)) + predicted := keccak256(add(ptr, 0x37), 0x55) + } + } + + function predict2(address master, bytes32 salt) internal view returns (address predicted) { + return predict2(master, salt, address(this)); + } +} From 571438762264f61c9ebaebd9c1e0eac708b0f323 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 4 Jan 2021 16:54:03 +0100 Subject: [PATCH 02/17] Clones: linting --- contracts/drafts/Clones.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/drafts/Clones.sol b/contracts/drafts/Clones.sol index c8599f7c33a..204213b49f1 100644 --- a/contracts/drafts/Clones.sol +++ b/contracts/drafts/Clones.sol @@ -9,8 +9,7 @@ pragma solidity >=0.6.0 <0.8.0; * This contract provide tooling to deploy proxies following the EIP 1167 * proposed bytecode. This is possible using both create and create2. */ -library Clones -{ +library Clones { function clone(address master) internal returns (address instance) { // solhint-disable-next-line no-inline-assembly assembly { From 9bc1cc699fe7bdd48fee50b1604f74ea66bc8b08 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 4 Jan 2021 19:56:08 +0100 Subject: [PATCH 03/17] Clones: typo --- contracts/drafts/Clones.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/drafts/Clones.sol b/contracts/drafts/Clones.sol index 204213b49f1..8b0fc99a075 100644 --- a/contracts/drafts/Clones.sol +++ b/contracts/drafts/Clones.sol @@ -30,7 +30,7 @@ library Clones { mstore(add(ptr, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000) instance := create2(0, ptr, 0x37, salt) } - require(instance != address(0), "ERC1167: create2 failled"); + require(instance != address(0), "ERC1167: create2 failed"); } function predict2(address master, bytes32 salt, address deployer) internal pure returns (address predicted) { From 09a48aa092537f90d6b34a2e83e744408a55ad53 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 15 Jan 2021 10:14:15 +0100 Subject: [PATCH 04/17] Update contracts/drafts/Clones.sol Co-authored-by: Francisco Giordano --- contracts/drafts/Clones.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/drafts/Clones.sol b/contracts/drafts/Clones.sol index 8b0fc99a075..9fbd866f55e 100644 --- a/contracts/drafts/Clones.sol +++ b/contracts/drafts/Clones.sol @@ -4,7 +4,7 @@ pragma solidity >=0.6.0 <0.8.0; /** * @dev https://eips.ethereum.org/EIPS/eip-1167[EIP 1167] is a standard for - * deploying minimal clone contracts. + * deploying minimal proxy contracts, also known as "clones". * * This contract provide tooling to deploy proxies following the EIP 1167 * proposed bytecode. This is possible using both create and create2. From d7a65da78114db444fb3f50a8e5b547ca87e31f6 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 15 Jan 2021 10:14:36 +0100 Subject: [PATCH 05/17] Update contracts/drafts/Clones.sol Co-authored-by: Francisco Giordano --- contracts/drafts/Clones.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/drafts/Clones.sol b/contracts/drafts/Clones.sol index 9fbd866f55e..5e09273abbf 100644 --- a/contracts/drafts/Clones.sol +++ b/contracts/drafts/Clones.sol @@ -6,6 +6,9 @@ pragma solidity >=0.6.0 <0.8.0; * @dev https://eips.ethereum.org/EIPS/eip-1167[EIP 1167] is a standard for * deploying minimal proxy contracts, also known as "clones". * + * > To simply and cheaply clone contract functionality in an immutable way, this standard specifie + * > a minimal bytecode implementation that delegates all calls to a known, fixed address. + * * This contract provide tooling to deploy proxies following the EIP 1167 * proposed bytecode. This is possible using both create and create2. */ From a195900a7eae662d21323b7a61b69469acb7ab30 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 15 Jan 2021 10:14:54 +0100 Subject: [PATCH 06/17] Update contracts/drafts/Clones.sol Co-authored-by: Francisco Giordano --- contracts/drafts/Clones.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/drafts/Clones.sol b/contracts/drafts/Clones.sol index 5e09273abbf..8ff6dbbcab2 100644 --- a/contracts/drafts/Clones.sol +++ b/contracts/drafts/Clones.sol @@ -24,7 +24,7 @@ library Clones { } } - function clone2(address master, bytes32 salt) internal returns (address instance) { + function cloneDeterministic(address master, bytes32 salt) internal returns (address instance) { // solhint-disable-next-line no-inline-assembly assembly { let ptr := mload(0x40) From 53ece070e5bb650218819ede1149d769568128aa Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Sat, 16 Jan 2021 18:24:47 +0100 Subject: [PATCH 07/17] moving Clones to proxy & add tests --- contracts/mocks/ClonesMock.sol | 32 +++++ contracts/{drafts => proxy}/Clones.sol | 6 +- test/proxy/Clones.behaviour.js | 159 +++++++++++++++++++++++++ test/proxy/Clones.test.js | 58 +++++++++ 4 files changed, 252 insertions(+), 3 deletions(-) create mode 100644 contracts/mocks/ClonesMock.sol rename contracts/{drafts => proxy}/Clones.sol (87%) create mode 100644 test/proxy/Clones.behaviour.js create mode 100644 test/proxy/Clones.test.js diff --git a/contracts/mocks/ClonesMock.sol b/contracts/mocks/ClonesMock.sol new file mode 100644 index 00000000000..b6926c663bd --- /dev/null +++ b/contracts/mocks/ClonesMock.sol @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: MIT + +pragma solidity >=0.6.0 <0.8.0; + +import "../proxy/Clones.sol"; +import "../utils/Address.sol"; + +contract ClonesMock { + using Address for address; + using Clones for address; + + event NewInstance(address instance); + + function clone(address master, bytes calldata initdata) public payable { + _initAndEmit(master.clone(), initdata); + } + + function cloneDeterministic(address master, bytes32 salt, bytes calldata initdata) public payable { + _initAndEmit(master.cloneDeterministic(salt), initdata); + } + + function predictDeterministicAddress(address master, bytes32 salt) public view returns (address predicted) { + return master.predictDeterministicAddress(salt); + } + + function _initAndEmit(address instance, bytes memory initdata) private { + if (initdata.length > 0) { + instance.functionCallWithValue(initdata, msg.value); + } + emit NewInstance(instance); + } +} diff --git a/contracts/drafts/Clones.sol b/contracts/proxy/Clones.sol similarity index 87% rename from contracts/drafts/Clones.sol rename to contracts/proxy/Clones.sol index 8ff6dbbcab2..20f5298c96b 100644 --- a/contracts/drafts/Clones.sol +++ b/contracts/proxy/Clones.sol @@ -36,7 +36,7 @@ library Clones { require(instance != address(0), "ERC1167: create2 failed"); } - function predict2(address master, bytes32 salt, address deployer) internal pure returns (address predicted) { + function predictDeterministicAddress(address master, bytes32 salt, address deployer) internal pure returns (address predicted) { // solhint-disable-next-line no-inline-assembly assembly { let ptr := mload(0x40) @@ -50,7 +50,7 @@ library Clones { } } - function predict2(address master, bytes32 salt) internal view returns (address predicted) { - return predict2(master, salt, address(this)); + function predictDeterministicAddress(address master, bytes32 salt) internal view returns (address predicted) { + return predictDeterministicAddress(master, salt, address(this)); } } diff --git a/test/proxy/Clones.behaviour.js b/test/proxy/Clones.behaviour.js new file mode 100644 index 00000000000..029c6d86236 --- /dev/null +++ b/test/proxy/Clones.behaviour.js @@ -0,0 +1,159 @@ +const { BN, expectRevert } = require('@openzeppelin/test-helpers'); +const { toChecksumAddress, keccak256 } = require('ethereumjs-util'); + +const { expect } = require('chai'); + +const DummyImplementation = artifacts.require('DummyImplementation'); + +module.exports = function shouldBehaveLikeClone (createClone) { + // it('cannot be initialized with a non-contract address', async function () { + // const nonContractAddress = proxyCreator; + // const initializeData = Buffer.from(''); + // await expectRevert.unspecified( + // createClone(nonContractAddress), + // ); + // }); + + before('deploy implementation', async function () { + this.implementation = web3.utils.toChecksumAddress((await DummyImplementation.new()).address); + }); + + const assertProxyInitialization = function ({ value, balance }) { + it('initializes the proxy', async function () { + const dummy = new DummyImplementation(this.proxy); + expect(await dummy.value()).to.be.bignumber.equal(value.toString()); + }); + + it('has expected balance', async function () { + expect(await web3.eth.getBalance(this.proxy)).to.be.bignumber.equal(balance.toString()); + }); + }; + + describe('initialization without parameters', function () { + describe('non payable', function () { + const expectedInitializedValue = 10; + const initializeData = new DummyImplementation('').contract.methods['initializeNonPayable()']().encodeABI(); + + describe('when not sending balance', function () { + beforeEach('creating proxy', async function () { + this.proxy = ( + await createClone(this.implementation, initializeData) + ).address; + }); + + assertProxyInitialization({ + value: expectedInitializedValue, + balance: 0, + }); + }); + + describe('when sending some balance', function () { + const value = 10e5; + + it('reverts', async function () { + await expectRevert.unspecified( + createClone(this.implementation, initializeData, { value }), + ); + }); + }); + }); + + describe('payable', function () { + const expectedInitializedValue = 100; + const initializeData = new DummyImplementation('').contract.methods['initializePayable()']().encodeABI(); + + describe('when not sending balance', function () { + beforeEach('creating proxy', async function () { + this.proxy = ( + await createClone(this.implementation, initializeData) + ).address; + }); + + assertProxyInitialization({ + value: expectedInitializedValue, + balance: 0, + }); + }); + + describe('when sending some balance', function () { + const value = 10e5; + + beforeEach('creating proxy', async function () { + this.proxy = ( + await createClone(this.implementation, initializeData, { value }) + ).address; + }); + + assertProxyInitialization({ + value: expectedInitializedValue, + balance: value, + }); + }); + }); + }); + + describe('initialization with parameters', function () { + describe('non payable', function () { + const expectedInitializedValue = 10; + const initializeData = new DummyImplementation('').contract + .methods.initializeNonPayableWithValue(expectedInitializedValue).encodeABI(); + + describe('when not sending balance', function () { + beforeEach('creating proxy', async function () { + this.proxy = ( + await createClone(this.implementation, initializeData) + ).address; + }); + + assertProxyInitialization({ + value: expectedInitializedValue, + balance: 0, + }); + }); + + describe('when sending some balance', function () { + const value = 10e5; + + it('reverts', async function () { + await expectRevert.unspecified( + createClone(this.implementation, initializeData, { value }), + ); + }); + }); + }); + + describe('payable', function () { + const expectedInitializedValue = 42; + const initializeData = new DummyImplementation('').contract + .methods.initializePayableWithValue(expectedInitializedValue).encodeABI(); + + describe('when not sending balance', function () { + beforeEach('creating proxy', async function () { + this.proxy = ( + await createClone(this.implementation, initializeData) + ).address; + }); + + assertProxyInitialization({ + value: expectedInitializedValue, + balance: 0, + }); + }); + + describe('when sending some balance', function () { + const value = 10e5; + + beforeEach('creating proxy', async function () { + this.proxy = ( + await createClone(this.implementation, initializeData, { value }) + ).address; + }); + + assertProxyInitialization({ + value: expectedInitializedValue, + balance: value, + }); + }); + }); + }); +}; diff --git a/test/proxy/Clones.test.js b/test/proxy/Clones.test.js new file mode 100644 index 00000000000..f4921714318 --- /dev/null +++ b/test/proxy/Clones.test.js @@ -0,0 +1,58 @@ +const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); +const { assert } = require('chai'); + +const shouldBehaveLikeClone = require('./Clones.behaviour'); + +const ClonesMock = artifacts.require('ClonesMock'); + +contract('Clones', function (accounts) { + beforeEach('deploying', async function () { + }); + + describe('clone', function () { + shouldBehaveLikeClone(async (implementation, initData, opts = {}) => { + const factory = await ClonesMock.new(); + const receipt = await factory.clone(implementation, initData, { value: opts.value }); + const address = receipt.logs.find(({ event }) => event == 'NewInstance').args.instance; + return { address }; + }); + }); + + describe('cloneDeterministic', function () { + shouldBehaveLikeClone(async (implementation, initData, opts = {}) => { + const salt = web3.utils.randomHex(32); + const factory = await ClonesMock.new(); + const receipt = await factory.cloneDeterministic(implementation, salt, initData, { value: opts.value }); + const address = receipt.logs.find(({ event }) => event == 'NewInstance').args.instance; + return { address }; + }); + + it ('address already used', async function () { + const implementation = web3.utils.randomHex(20); + const salt = web3.utils.randomHex(32); + const factory = await ClonesMock.new(); + // deploy once + expectEvent( + await factory.cloneDeterministic(implementation, salt, '0x'), + 'NewInstance', + ); + // deploy twice + await expectRevert( + factory.cloneDeterministic(implementation, salt, '0x'), + 'ERC1167: create2 failed', + ); + }); + + it ('address prediction', async function () { + const implementation = web3.utils.randomHex(20); + const salt = web3.utils.randomHex(32); + const factory = await ClonesMock.new(); + const predicted = await factory.predictDeterministicAddress(implementation, salt); + expectEvent( + await factory.cloneDeterministic(implementation, salt, "0x"), + 'NewInstance', + { instance: predicted }, + ); + }); + }); +}); From d492bd6277960fbb81c439fd58479d64a4a0d6d4 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Sat, 16 Jan 2021 18:27:44 +0100 Subject: [PATCH 08/17] fix lint --- test/proxy/Clones.behaviour.js | 3 +-- test/proxy/Clones.test.js | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/test/proxy/Clones.behaviour.js b/test/proxy/Clones.behaviour.js index 029c6d86236..723c95c5d05 100644 --- a/test/proxy/Clones.behaviour.js +++ b/test/proxy/Clones.behaviour.js @@ -1,5 +1,4 @@ -const { BN, expectRevert } = require('@openzeppelin/test-helpers'); -const { toChecksumAddress, keccak256 } = require('ethereumjs-util'); +const { expectRevert } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); diff --git a/test/proxy/Clones.test.js b/test/proxy/Clones.test.js index f4921714318..9edbc1e056d 100644 --- a/test/proxy/Clones.test.js +++ b/test/proxy/Clones.test.js @@ -1,5 +1,4 @@ const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); -const { assert } = require('chai'); const shouldBehaveLikeClone = require('./Clones.behaviour'); @@ -13,24 +12,24 @@ contract('Clones', function (accounts) { shouldBehaveLikeClone(async (implementation, initData, opts = {}) => { const factory = await ClonesMock.new(); const receipt = await factory.clone(implementation, initData, { value: opts.value }); - const address = receipt.logs.find(({ event }) => event == 'NewInstance').args.instance; + const address = receipt.logs.find(({ event }) => event === 'NewInstance').args.instance; return { address }; }); }); describe('cloneDeterministic', function () { shouldBehaveLikeClone(async (implementation, initData, opts = {}) => { - const salt = web3.utils.randomHex(32); + const salt = web3.utils.randomHex(32); const factory = await ClonesMock.new(); const receipt = await factory.cloneDeterministic(implementation, salt, initData, { value: opts.value }); - const address = receipt.logs.find(({ event }) => event == 'NewInstance').args.instance; + const address = receipt.logs.find(({ event }) => event === 'NewInstance').args.instance; return { address }; }); - it ('address already used', async function () { + it('address already used', async function () { const implementation = web3.utils.randomHex(20); - const salt = web3.utils.randomHex(32); - const factory = await ClonesMock.new(); + const salt = web3.utils.randomHex(32); + const factory = await ClonesMock.new(); // deploy once expectEvent( await factory.cloneDeterministic(implementation, salt, '0x'), @@ -43,13 +42,13 @@ contract('Clones', function (accounts) { ); }); - it ('address prediction', async function () { + it('address prediction', async function () { const implementation = web3.utils.randomHex(20); - const salt = web3.utils.randomHex(32); - const factory = await ClonesMock.new(); - const predicted = await factory.predictDeterministicAddress(implementation, salt); + const salt = web3.utils.randomHex(32); + const factory = await ClonesMock.new(); + const predicted = await factory.predictDeterministicAddress(implementation, salt); expectEvent( - await factory.cloneDeterministic(implementation, salt, "0x"), + await factory.cloneDeterministic(implementation, salt, '0x'), 'NewInstance', { instance: predicted }, ); From 6c0206346a66eaa03406e6c4f9c17e9f358b4e93 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Sat, 16 Jan 2021 19:15:40 +0100 Subject: [PATCH 09/17] fix toChecksumAddress for addressed starting with 0x00 --- test/proxy/TransparentUpgradeableProxy.behaviour.js | 10 +++++++--- test/proxy/UpgradeableProxy.behaviour.js | 8 ++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/test/proxy/TransparentUpgradeableProxy.behaviour.js b/test/proxy/TransparentUpgradeableProxy.behaviour.js index fb8dd74efb9..800c0918565 100644 --- a/test/proxy/TransparentUpgradeableProxy.behaviour.js +++ b/test/proxy/TransparentUpgradeableProxy.behaviour.js @@ -1,6 +1,6 @@ const { BN, expectRevert, expectEvent, constants } = require('@openzeppelin/test-helpers'); const { ZERO_ADDRESS } = constants; -const { toChecksumAddress, keccak256 } = require('ethereumjs-util'); +const ethereumjsUtil = require('ethereumjs-util'); const { expect } = require('chai'); @@ -19,6 +19,10 @@ const ClashingImplementation = artifacts.require('ClashingImplementation'); const IMPLEMENTATION_LABEL = 'eip1967.proxy.implementation'; const ADMIN_LABEL = 'eip1967.proxy.admin'; +function toChecksumAddress (address) { + return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); +} + module.exports = function shouldBehaveLikeTransparentUpgradeableProxy (createProxy, accounts) { const [proxyAdminAddress, proxyAdminOwner, anotherAccount] = accounts; @@ -308,13 +312,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy (createPro describe('storage', function () { it('should store the implementation address in specified location', async function () { - const slot = '0x' + new BN(keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16); + const slot = '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16); const implementation = toChecksumAddress(await web3.eth.getStorageAt(this.proxyAddress, slot)); expect(implementation).to.be.equal(this.implementationV0); }); it('should store the admin proxy in specified location', async function () { - const slot = '0x' + new BN(keccak256(Buffer.from(ADMIN_LABEL))).subn(1).toString(16); + const slot = '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(ADMIN_LABEL))).subn(1).toString(16); const proxyAdmin = toChecksumAddress(await web3.eth.getStorageAt(this.proxyAddress, slot)); expect(proxyAdmin).to.be.equal(proxyAdminAddress); }); diff --git a/test/proxy/UpgradeableProxy.behaviour.js b/test/proxy/UpgradeableProxy.behaviour.js index 047451508ad..d23b5662327 100644 --- a/test/proxy/UpgradeableProxy.behaviour.js +++ b/test/proxy/UpgradeableProxy.behaviour.js @@ -1,5 +1,5 @@ const { BN, expectRevert } = require('@openzeppelin/test-helpers'); -const { toChecksumAddress, keccak256 } = require('ethereumjs-util'); +const ethereumjsUtil = require('ethereumjs-util'); const { expect } = require('chai'); @@ -7,6 +7,10 @@ const DummyImplementation = artifacts.require('DummyImplementation'); const IMPLEMENTATION_LABEL = 'eip1967.proxy.implementation'; +function toChecksumAddress (address) { + return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); +} + module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAdminAddress, proxyCreator) { it('cannot be initialized with a non-contract address', async function () { const nonContractAddress = proxyCreator; @@ -24,7 +28,7 @@ module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAd const assertProxyInitialization = function ({ value, balance }) { it('sets the implementation address', async function () { - const slot = '0x' + new BN(keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16); + const slot = '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16); const implementation = toChecksumAddress(await web3.eth.getStorageAt(this.proxy, slot)); expect(implementation).to.be.equal(this.implementation); }); From 1508d3121ae54648006757f1e2b8a2d449e6eb27 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 19 Jan 2021 11:54:38 +0100 Subject: [PATCH 10/17] documentation --- contracts/proxy/Clones.sol | 18 ++++++++++++++++++ contracts/proxy/README.adoc | 10 ++++++++++ 2 files changed, 28 insertions(+) diff --git a/contracts/proxy/Clones.sol b/contracts/proxy/Clones.sol index 20f5298c96b..a4b4032ba55 100644 --- a/contracts/proxy/Clones.sol +++ b/contracts/proxy/Clones.sol @@ -13,6 +13,11 @@ pragma solidity >=0.6.0 <0.8.0; * proposed bytecode. This is possible using both create and create2. */ library Clones { + /** + * @dev Deploys and return the address of a clone that mimics the behaviour of `master`. + * + * This function uses the create opcode, which should never revert. + */ function clone(address master) internal returns (address instance) { // solhint-disable-next-line no-inline-assembly assembly { @@ -24,6 +29,13 @@ library Clones { } } + /** + * @dev Deploys and return the address of a clone that mimics the behaviour of `master`. + * + * This function uses the create2 opcode and a `salt` to deterministically deploy + * the clone. Using the same `master` and `salt` multiple time will revert, since + * the clones cannot be deployed twice at the same address. + */ function cloneDeterministic(address master, bytes32 salt) internal returns (address instance) { // solhint-disable-next-line no-inline-assembly assembly { @@ -36,6 +48,9 @@ library Clones { require(instance != address(0), "ERC1167: create2 failed"); } + /** + * @dev Computes the address of a clone deployed using {Clones-cloneDeterministic}. + */ function predictDeterministicAddress(address master, bytes32 salt, address deployer) internal pure returns (address predicted) { // solhint-disable-next-line no-inline-assembly assembly { @@ -50,6 +65,9 @@ library Clones { } } + /** + * @dev Computes the address of a clone deployed using {Clones-cloneDeterministic}. + */ function predictDeterministicAddress(address master, bytes32 salt) internal view returns (address predicted) { return predictDeterministicAddress(master, salt, address(this)); } diff --git a/contracts/proxy/README.adoc b/contracts/proxy/README.adoc index 255faeb43cd..b06109c9156 100644 --- a/contracts/proxy/README.adoc +++ b/contracts/proxy/README.adoc @@ -29,6 +29,16 @@ CAUTION: Using upgradeable proxies correctly and securely is a difficult task th {{UpgradeableBeacon}} +== Clones (ERC1167) + +The clone library provides a way to deploy minimal, non-upgradeable, proxies for cheap. This can be useful for applications that require deploying many instances of the same contract (for example one per user, or one per task). + +These instances are designed to be both cheap to deploy, and cheap to call. The drawback being that they are not upgradeable. If upgradeability is necessary, it is possible to use this library to clone an updradeable proxy logic. + +The clone library includes functions to deploy proxy using either `create` (traditional deployment) or `create2` (salted deterministic deployment). It also includes tools to predict the addresses of clones deployed using the deterministic method. + +{{Clones}} + == Utilities {{Initializable}} From 006646a72e9e4cb2fcc9353a92e22808d9954197 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 19 Jan 2021 17:20:27 +0100 Subject: [PATCH 11/17] check out of gas faillures in Clones.clone --- contracts/proxy/Clones.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/proxy/Clones.sol b/contracts/proxy/Clones.sol index a4b4032ba55..7f46d67adb2 100644 --- a/contracts/proxy/Clones.sol +++ b/contracts/proxy/Clones.sol @@ -27,6 +27,7 @@ library Clones { mstore(add(ptr, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000) instance := create(0, ptr, 0x37) } + require(instance != address(0), "ERC1167: create failed"); } /** From 98bdd56cfbf76a1f27749267cb0ed88da5ac9cf2 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Tue, 19 Jan 2021 15:13:18 -0300 Subject: [PATCH 12/17] fix beacon heading --- contracts/proxy/README.adoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/proxy/README.adoc b/contracts/proxy/README.adoc index b06109c9156..755b90c10f0 100644 --- a/contracts/proxy/README.adoc +++ b/contracts/proxy/README.adoc @@ -9,7 +9,7 @@ The abstract {Proxy} contract implements the core delegation functionality. If t Upgradeability is implemented in the {UpgradeableProxy} contract, although it provides only an internal upgrade interface. For an upgrade interface exposed externally to an admin, we provide {TransparentUpgradeableProxy}. Both of these contracts use the storage slots specified in https://eips.ethereum.org/EIPS/eip-1967[EIP1967] to avoid clashes with the storage of the implementation contract behind the proxy. -An alternative upgradeability mechanism is provided in <>. This pattern, popularized by Dharma, allows multiple proxies to be upgraded to a different implementation in a single transaction. In this pattern, the proxy contract doesn't hold the implementation address in storage like {UpgradeableProxy}, but the address of a {UpgradeableBeacon} contract, which is where the implementation address is actually stored and retrieved from. The `upgrade` operations that change the implementation contract address are then sent to the beacon instead of to the proxy contract, and all proxies that follow that beacon are automatically upgraded. +An alternative upgradeability mechanism is provided in <>. This pattern, popularized by Dharma, allows multiple proxies to be upgraded to a different implementation in a single transaction. In this pattern, the proxy contract doesn't hold the implementation address in storage like {UpgradeableProxy}, but the address of a {UpgradeableBeacon} contract, which is where the implementation address is actually stored and retrieved from. The `upgrade` operations that change the implementation contract address are then sent to the beacon instead of to the proxy contract, and all proxies that follow that beacon are automatically upgraded. CAUTION: Using upgradeable proxies correctly and securely is a difficult task that requires deep knowledge of the proxy pattern, Solidity, and the EVM. Unless you want a lot of low level control, we recommend using the xref:upgrades-plugins::index.adoc[OpenZeppelin Upgrades Plugins] for Truffle and Buidler. @@ -21,7 +21,7 @@ CAUTION: Using upgradeable proxies correctly and securely is a difficult task th {{TransparentUpgradeableProxy}} -== UpgradeableBeacon +== Beacon {{BeaconProxy}} From 9a928619ddbeff30b4bc99cc88f8fcd9d245b8dd Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Tue, 19 Jan 2021 15:13:40 -0300 Subject: [PATCH 13/17] reorganize content about minimal proxies --- contracts/proxy/Clones.sol | 7 ++++--- contracts/proxy/README.adoc | 12 ++++-------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/contracts/proxy/Clones.sol b/contracts/proxy/Clones.sol index 7f46d67adb2..55b541ccbb5 100644 --- a/contracts/proxy/Clones.sol +++ b/contracts/proxy/Clones.sol @@ -6,11 +6,12 @@ pragma solidity >=0.6.0 <0.8.0; * @dev https://eips.ethereum.org/EIPS/eip-1167[EIP 1167] is a standard for * deploying minimal proxy contracts, also known as "clones". * - * > To simply and cheaply clone contract functionality in an immutable way, this standard specifie + * > To simply and cheaply clone contract functionality in an immutable way, this standard specifies * > a minimal bytecode implementation that delegates all calls to a known, fixed address. * - * This contract provide tooling to deploy proxies following the EIP 1167 - * proposed bytecode. This is possible using both create and create2. + * The clone library includes functions to deploy proxy using either `create` (traditional deployment) or `create2` + * (salted deterministic deployment). It also includes functions to predict the addresses of clones deployed using the + * deterministic method. */ library Clones { /** diff --git a/contracts/proxy/README.adoc b/contracts/proxy/README.adoc index 755b90c10f0..452ad91333f 100644 --- a/contracts/proxy/README.adoc +++ b/contracts/proxy/README.adoc @@ -3,7 +3,7 @@ [.readme-notice] NOTE: This document is better viewed at https://docs.openzeppelin.com/contracts/api/proxy -This is a low-level set of contracts implementing the proxy pattern for upgradeability. For an in-depth overview of this pattern check out the xref:upgrades-plugins::proxies.adoc[Proxy Upgrade Pattern] page. +This is a low-level set of contracts implementing different proxy patterns with and without upgradeability. For an in-depth overview of this pattern check out the xref:upgrades-plugins::proxies.adoc[Proxy Upgrade Pattern] page. The abstract {Proxy} contract implements the core delegation functionality. If the concrete proxies that we provide below are not suitable, we encourage building on top of this base contract since it contains an assembly block that may be hard to get right. @@ -11,6 +11,8 @@ Upgradeability is implemented in the {UpgradeableProxy} contract, although it pr An alternative upgradeability mechanism is provided in <>. This pattern, popularized by Dharma, allows multiple proxies to be upgraded to a different implementation in a single transaction. In this pattern, the proxy contract doesn't hold the implementation address in storage like {UpgradeableProxy}, but the address of a {UpgradeableBeacon} contract, which is where the implementation address is actually stored and retrieved from. The `upgrade` operations that change the implementation contract address are then sent to the beacon instead of to the proxy contract, and all proxies that follow that beacon are automatically upgraded. +The {Clones} library provides a way to deploy minimal non-upgradeable proxies for cheap. This can be useful for applications that require deploying many instances of the same contract (for example one per user, or one per task). These instances are designed to be both cheap to deploy, and cheap to call. The drawback being that they are not upgradeable. + CAUTION: Using upgradeable proxies correctly and securely is a difficult task that requires deep knowledge of the proxy pattern, Solidity, and the EVM. Unless you want a lot of low level control, we recommend using the xref:upgrades-plugins::index.adoc[OpenZeppelin Upgrades Plugins] for Truffle and Buidler. == Core @@ -29,13 +31,7 @@ CAUTION: Using upgradeable proxies correctly and securely is a difficult task th {{UpgradeableBeacon}} -== Clones (ERC1167) - -The clone library provides a way to deploy minimal, non-upgradeable, proxies for cheap. This can be useful for applications that require deploying many instances of the same contract (for example one per user, or one per task). - -These instances are designed to be both cheap to deploy, and cheap to call. The drawback being that they are not upgradeable. If upgradeability is necessary, it is possible to use this library to clone an updradeable proxy logic. - -The clone library includes functions to deploy proxy using either `create` (traditional deployment) or `create2` (salted deterministic deployment). It also includes tools to predict the addresses of clones deployed using the deterministic method. +== Minimal Clones {{Clones}} From 972432b5721f36a7ab5da30a64c422d70c3cce21 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Tue, 19 Jan 2021 15:14:59 -0300 Subject: [PATCH 14/17] tweak wording --- contracts/proxy/Clones.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/proxy/Clones.sol b/contracts/proxy/Clones.sol index 55b541ccbb5..a4089b81021 100644 --- a/contracts/proxy/Clones.sol +++ b/contracts/proxy/Clones.sol @@ -9,7 +9,7 @@ pragma solidity >=0.6.0 <0.8.0; * > To simply and cheaply clone contract functionality in an immutable way, this standard specifies * > a minimal bytecode implementation that delegates all calls to a known, fixed address. * - * The clone library includes functions to deploy proxy using either `create` (traditional deployment) or `create2` + * The library includes functions to deploy a proxy using either `create` (traditional deployment) or `create2` * (salted deterministic deployment). It also includes functions to predict the addresses of clones deployed using the * deterministic method. */ From 7decf975f07cc1f9ff20c134dd1579f7f8a6b6e7 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Tue, 19 Jan 2021 15:21:10 -0300 Subject: [PATCH 15/17] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 678f75d671f..ff6ca7c638f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * `ERC20Permit`: added an implementation of the ERC20 permit extension for gasless token approvals. ([#2237](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2237)) * Presets: added token presets with preminted fixed supply `ERC20PresetFixedSupply` and `ERC777PresetFixedSupply`. ([#2399](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2399)) * `Address`: added `functionDelegateCall`, similar to the existing `functionCall`. ([#2333](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2333)) + * `Clones`: added a library for deploying EIP 1167 minimal proxies. ([#2449](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2449)) ## 3.3.0 (2020-11-26) From 80ead17d1271276d0515653396863aa544131228 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Tue, 19 Jan 2021 15:21:37 -0300 Subject: [PATCH 16/17] remove commented lines and empty beforeEach block --- test/proxy/Clones.behaviour.js | 8 -------- test/proxy/Clones.test.js | 3 --- 2 files changed, 11 deletions(-) diff --git a/test/proxy/Clones.behaviour.js b/test/proxy/Clones.behaviour.js index 723c95c5d05..81c5ee35f94 100644 --- a/test/proxy/Clones.behaviour.js +++ b/test/proxy/Clones.behaviour.js @@ -5,14 +5,6 @@ const { expect } = require('chai'); const DummyImplementation = artifacts.require('DummyImplementation'); module.exports = function shouldBehaveLikeClone (createClone) { - // it('cannot be initialized with a non-contract address', async function () { - // const nonContractAddress = proxyCreator; - // const initializeData = Buffer.from(''); - // await expectRevert.unspecified( - // createClone(nonContractAddress), - // ); - // }); - before('deploy implementation', async function () { this.implementation = web3.utils.toChecksumAddress((await DummyImplementation.new()).address); }); diff --git a/test/proxy/Clones.test.js b/test/proxy/Clones.test.js index 9edbc1e056d..0f6a5de9762 100644 --- a/test/proxy/Clones.test.js +++ b/test/proxy/Clones.test.js @@ -5,9 +5,6 @@ const shouldBehaveLikeClone = require('./Clones.behaviour'); const ClonesMock = artifacts.require('ClonesMock'); contract('Clones', function (accounts) { - beforeEach('deploying', async function () { - }); - describe('clone', function () { shouldBehaveLikeClone(async (implementation, initData, opts = {}) => { const factory = await ClonesMock.new(); From e57cccc3f85cc6dc98a67b9c51bcc14cb538bfe6 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Tue, 19 Jan 2021 15:23:42 -0300 Subject: [PATCH 17/17] typo --- contracts/proxy/Clones.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/proxy/Clones.sol b/contracts/proxy/Clones.sol index a4089b81021..9ad5c223f56 100644 --- a/contracts/proxy/Clones.sol +++ b/contracts/proxy/Clones.sol @@ -15,7 +15,7 @@ pragma solidity >=0.6.0 <0.8.0; */ library Clones { /** - * @dev Deploys and return the address of a clone that mimics the behaviour of `master`. + * @dev Deploys and returns the address of a clone that mimics the behaviour of `master`. * * This function uses the create opcode, which should never revert. */ @@ -32,7 +32,7 @@ library Clones { } /** - * @dev Deploys and return the address of a clone that mimics the behaviour of `master`. + * @dev Deploys and returns the address of a clone that mimics the behaviour of `master`. * * This function uses the create2 opcode and a `salt` to deterministically deploy * the clone. Using the same `master` and `salt` multiple time will revert, since