diff --git a/contracts/mocks/BadBeacon.sol b/contracts/mocks/BadBeacon.sol new file mode 100644 index 000000000..c7a97b3c7 --- /dev/null +++ b/contracts/mocks/BadBeacon.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: MIT + +pragma solidity >=0.6.0 <0.8.0; + +import "../proxy/IBeacon.sol"; + +contract BadBeaconNoImpl { +} + +contract BadBeaconNotContract is IBeacon { + function implementation() external view override returns (address) { + return address(0x1); + } +} diff --git a/contracts/mocks/DummyImplementation.sol b/contracts/mocks/DummyImplementation.sol index 38569ebd8..df6a7d890 100644 --- a/contracts/mocks/DummyImplementation.sol +++ b/contracts/mocks/DummyImplementation.sol @@ -19,11 +19,11 @@ contract DummyImplementation { value = 100; } - function initializeNonPayable(uint256 _value) public { + function initializeNonPayableWithValue(uint256 _value) public { value = _value; } - function initializePayable(uint256 _value) public payable { + function initializePayableWithValue(uint256 _value) public payable { value = _value; } @@ -42,7 +42,7 @@ contract DummyImplementation { } function reverts() public pure { - require(false); + require(false, "DummyImplementation reverted"); } } diff --git a/contracts/proxy/BeaconProxy.sol b/contracts/proxy/BeaconProxy.sol new file mode 100644 index 000000000..8054c4847 --- /dev/null +++ b/contracts/proxy/BeaconProxy.sol @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: MIT + +pragma solidity >=0.6.0 <0.8.0; + +import "./Proxy.sol"; +import "../utils/Address.sol"; +import "./IBeacon.sol"; + +/** + * @dev This contract implements a proxy that gets the implementation address for each call from a {UpgradeableBeacon}. + * + * The beacon address is stored in storage slot `uint256(keccak256('eip1967.proxy.beacon')) - 1`, so that it doesn't + * conflict with the storage layout of the implementation behind the proxy. + */ +contract BeaconProxy is Proxy { + /** + * @dev The storage slot of the UpgradeableBeacon contract which defines the implementation for this proxy. + * This is bytes32(uint256(keccak256('eip1967.proxy.beacon')) - 1)) and is validated in the constructor. + */ + bytes32 private constant _BEACON_SLOT = 0xa3f0ad74e5423aebfd80d3ef4346578335a9a72aeaee59ff6cb3582b35133d50; + + /** + * @dev Initializes the proxy with `beacon`. + * + * If `data` is nonempty, it's used as data in a delegate call to the implementation returned by the beacon. This + * will typically be an encoded function call, and allows initializating the storage of the proxy like a Solidity + * constructor. + * + * Requirements: + * + * - `beacon` must be a contract with the interface {IBeacon}. + */ + constructor(address beacon, bytes memory data) public payable { + assert(_BEACON_SLOT == bytes32(uint256(keccak256("eip1967.proxy.beacon")) - 1)); + _setBeacon(beacon, data); + } + + /** + * @dev Returns the current beacon address. + */ + function _beacon() internal view returns (address beacon) { + bytes32 slot = _BEACON_SLOT; + // solhint-disable-next-line no-inline-assembly + assembly { + beacon := sload(slot) + } + } + + /** + * @dev Returns the current implementation address of the associated beacon. + */ + function _implementation() internal view override returns (address) { + return IBeacon(_beacon()).implementation(); + } + + /** + * @dev Changes the proxy to use a new beacon. + * + * If `data` is nonempty, it's used as data in a delegate call to the implementation returned by the beacon. + * + * Requirements: + * + * - `beacon` must be a contract. + * - The implementation returned by `beacon` must be a contract. + */ + function _setBeacon(address beacon, bytes memory data) internal { + require( + Address.isContract(beacon), + "BeaconProxy: beacon is not a contract" + ); + require( + Address.isContract(IBeacon(beacon).implementation()), + "BeaconProxy: beacon implementation is not a contract" + ); + bytes32 slot = _BEACON_SLOT; + + // solhint-disable-next-line no-inline-assembly + assembly { + sstore(slot, beacon) + } + + if (data.length > 0) { + Address.functionDelegateCall(_implementation(), data, "BeaconProxy: function call failed"); + } + } +} diff --git a/contracts/proxy/IBeacon.sol b/contracts/proxy/IBeacon.sol new file mode 100644 index 000000000..7d08b7b17 --- /dev/null +++ b/contracts/proxy/IBeacon.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: MIT + +pragma solidity >=0.6.0 <0.8.0; + +/** + * @dev This is the interface that {BeaconProxy} expects of its beacon. + */ +interface IBeacon { + /** + * @dev Must return an address that can be used as a delegate call target. + * + * {BeaconProxy} will check that this address is a contract. + */ + function implementation() external view returns (address); +} diff --git a/contracts/proxy/README.adoc b/contracts/proxy/README.adoc index 68d55854e..255faeb43 100644 --- a/contracts/proxy/README.adoc +++ b/contracts/proxy/README.adoc @@ -9,6 +9,8 @@ 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. + 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 @@ -19,6 +21,14 @@ CAUTION: Using upgradeable proxies correctly and securely is a difficult task th {{TransparentUpgradeableProxy}} +== UpgradeableBeacon + +{{BeaconProxy}} + +{{IBeacon}} + +{{UpgradeableBeacon}} + == Utilities {{Initializable}} diff --git a/contracts/proxy/UpgradeableBeacon.sol b/contracts/proxy/UpgradeableBeacon.sol new file mode 100644 index 000000000..e1350ce36 --- /dev/null +++ b/contracts/proxy/UpgradeableBeacon.sol @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: MIT + +pragma solidity >=0.6.0 <0.8.0; + +import "./IBeacon.sol"; +import "../access/Ownable.sol"; +import "../utils/Address.sol"; + +/** + * @dev This contract is used in conjunction with one or more instances of {BeaconProxy} to determine their + * implementation contract, which is where they will delegate all function calls. + * + * An owner is able to change the implementation the beacon points to, thus upgrading the proxies that use this beacon. + */ +contract UpgradeableBeacon is IBeacon, Ownable { + address private _implementation; + + /** + * @dev Emitted when the implementation returned by the beacon is changed. + */ + event Upgraded(address indexed implementation); + + /** + * @dev Sets the address of the initial implementation, and the deployer account as the owner who can upgrade the + * beacon. + */ + constructor(address implementation_) public { + _setImplementation(implementation_); + } + + /** + * @dev Returns the current implementation address. + */ + function implementation() public view override returns (address) { + return _implementation; + } + + /** + * @dev Upgrades the beacon to a new implementation. + * + * Emits an {Upgraded} event. + * + * Requirements: + * + * - msg.sender must be the owner of the contract. + * - `newImplementation` must be a contract. + */ + function upgradeTo(address newImplementation) public onlyOwner { + _setImplementation(newImplementation); + emit Upgraded(newImplementation); + } + + /** + * @dev Sets the implementation contract address for this beacon + * + * Requirements: + * + * - `newImplementation` must be a contract. + */ + function _setImplementation(address newImplementation) private { + require(Address.isContract(newImplementation), "UpgradeableBeacon: implementation is not a contract"); + _implementation = newImplementation; + } +} diff --git a/test/proxy/BeaconProxy.test.js b/test/proxy/BeaconProxy.test.js new file mode 100644 index 000000000..7a1a0d0fd --- /dev/null +++ b/test/proxy/BeaconProxy.test.js @@ -0,0 +1,163 @@ +const { BN, expectRevert } = require('@openzeppelin/test-helpers'); +const ethereumjsUtil = require('ethereumjs-util'); +const { keccak256 } = ethereumjsUtil; + +const { expect } = require('chai'); + +const UpgradeableBeacon = artifacts.require('UpgradeableBeacon'); +const BeaconProxy = artifacts.require('BeaconProxy'); +const DummyImplementation = artifacts.require('DummyImplementation'); +const DummyImplementationV2 = artifacts.require('DummyImplementationV2'); +const BadBeaconNoImpl = artifacts.require('BadBeaconNoImpl'); +const BadBeaconNotContract = artifacts.require('BadBeaconNotContract'); + +function toChecksumAddress (address) { + return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); +} + +const BEACON_LABEL = 'eip1967.proxy.beacon'; +const BEACON_SLOT = '0x' + new BN(keccak256(Buffer.from(BEACON_LABEL))).subn(1).toString(16); + +contract('BeaconProxy', function (accounts) { + const [anotherAccount] = accounts; + + describe('bad beacon is not accepted', async function () { + it('non-contract beacon', async function () { + await expectRevert( + BeaconProxy.new(anotherAccount, '0x'), + 'BeaconProxy: beacon is not a contract', + ); + }); + + it('non-compliant beacon', async function () { + const beacon = await BadBeaconNoImpl.new(); + await expectRevert.unspecified( + BeaconProxy.new(beacon.address, '0x'), + ); + }); + + it('non-contract implementation', async function () { + const beacon = await BadBeaconNotContract.new(); + await expectRevert( + BeaconProxy.new(beacon.address, '0x'), + 'BeaconProxy: beacon implementation is not a contract', + ); + }); + }); + + before('deploy implementation', async function () { + this.implementationV0 = await DummyImplementation.new(); + this.implementationV1 = await DummyImplementationV2.new(); + }); + + describe('initialization', function () { + before(function () { + this.assertInitialized = async ({ value, balance }) => { + const beaconAddress = toChecksumAddress(await web3.eth.getStorageAt(this.proxy.address, BEACON_SLOT)); + expect(beaconAddress).to.equal(this.beacon.address); + + const dummy = new DummyImplementation(this.proxy.address); + expect(await dummy.value()).to.bignumber.eq(value); + + expect(await web3.eth.getBalance(this.proxy.address)).to.bignumber.eq(balance); + }; + }); + + beforeEach('deploy beacon', async function () { + this.beacon = await UpgradeableBeacon.new(this.implementationV0.address); + }); + + it('no initialization', async function () { + const data = Buffer.from(''); + const balance = '10'; + this.proxy = await BeaconProxy.new(this.beacon.address, data, { value: balance }); + await this.assertInitialized({ value: '0', balance }); + }); + + it('non-payable initialization', async function () { + const value = '55'; + const data = this.implementationV0.contract.methods + .initializeNonPayableWithValue(value) + .encodeABI(); + this.proxy = await BeaconProxy.new(this.beacon.address, data); + await this.assertInitialized({ value, balance: '0' }); + }); + + it('payable initialization', async function () { + const value = '55'; + const data = this.implementationV0.contract.methods + .initializePayableWithValue(value) + .encodeABI(); + const balance = '100'; + this.proxy = await BeaconProxy.new(this.beacon.address, data, { value: balance }); + await this.assertInitialized({ value, balance }); + }); + + it('reverting initialization', async function () { + const data = this.implementationV0.contract.methods.reverts().encodeABI(); + await expectRevert( + BeaconProxy.new(this.beacon.address, data), + 'DummyImplementation reverted', + ); + }); + }); + + it('upgrade a proxy by upgrading its beacon', async function () { + const beacon = await UpgradeableBeacon.new(this.implementationV0.address); + + const value = '10'; + const data = this.implementationV0.contract.methods + .initializeNonPayableWithValue(value) + .encodeABI(); + const proxy = await BeaconProxy.new(beacon.address, data); + + const dummy = new DummyImplementation(proxy.address); + + // test initial values + expect(await dummy.value()).to.bignumber.eq(value); + + // test initial version + expect(await dummy.version()).to.eq('V1'); + + // upgrade beacon + await beacon.upgradeTo(this.implementationV1.address); + + // test upgraded version + expect(await dummy.version()).to.eq('V2'); + }); + + it('upgrade 2 proxies by upgrading shared beacon', async function () { + const value1 = '10'; + const value2 = '42'; + + const beacon = await UpgradeableBeacon.new(this.implementationV0.address); + + const proxy1InitializeData = this.implementationV0.contract.methods + .initializeNonPayableWithValue(value1) + .encodeABI(); + const proxy1 = await BeaconProxy.new(beacon.address, proxy1InitializeData); + + const proxy2InitializeData = this.implementationV0.contract.methods + .initializeNonPayableWithValue(value2) + .encodeABI(); + const proxy2 = await BeaconProxy.new(beacon.address, proxy2InitializeData); + + const dummy1 = new DummyImplementation(proxy1.address); + const dummy2 = new DummyImplementation(proxy2.address); + + // test initial values + expect(await dummy1.value()).to.bignumber.eq(value1); + expect(await dummy2.value()).to.bignumber.eq(value2); + + // test initial version + expect(await dummy1.version()).to.eq('V1'); + expect(await dummy2.version()).to.eq('V1'); + + // upgrade beacon + await beacon.upgradeTo(this.implementationV1.address); + + // test upgraded version + expect(await dummy1.version()).to.eq('V2'); + expect(await dummy2.version()).to.eq('V2'); + }); +}); diff --git a/test/proxy/ProxyAdmin.test.js b/test/proxy/ProxyAdmin.test.js index c0bd61c9a..c761022eb 100644 --- a/test/proxy/ProxyAdmin.test.js +++ b/test/proxy/ProxyAdmin.test.js @@ -80,7 +80,7 @@ contract('ProxyAdmin', function (accounts) { describe('#upgradeAndCall', function () { context('with unauthorized account', function () { it('fails to upgrade', async function () { - const callData = new ImplV1('').contract.methods['initializeNonPayable(uint256)'](1337).encodeABI(); + const callData = new ImplV1('').contract.methods.initializeNonPayableWithValue(1337).encodeABI(); await expectRevert( this.proxyAdmin.upgradeAndCall(this.proxy.address, this.implementationV2.address, callData, { from: anotherAccount }, @@ -104,7 +104,7 @@ contract('ProxyAdmin', function (accounts) { context('with valid callData', function () { it('upgrades implementation', async function () { - const callData = new ImplV1('').contract.methods['initializeNonPayable(uint256)'](1337).encodeABI(); + const callData = new ImplV1('').contract.methods.initializeNonPayableWithValue(1337).encodeABI(); await this.proxyAdmin.upgradeAndCall(this.proxy.address, this.implementationV2.address, callData, { from: proxyAdminOwner }, ); diff --git a/test/proxy/UpgradeableBeacon.test.js b/test/proxy/UpgradeableBeacon.test.js new file mode 100644 index 000000000..0c49906d6 --- /dev/null +++ b/test/proxy/UpgradeableBeacon.test.js @@ -0,0 +1,50 @@ +const { expectRevert, expectEvent } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); + +const UpgradeableBeacon = artifacts.require('UpgradeableBeacon'); +const Implementation1 = artifacts.require('Implementation1'); +const Implementation2 = artifacts.require('Implementation2'); + +contract('UpgradeableBeacon', function (accounts) { + const [owner, other] = accounts; + + it('cannot be created with non-contract implementation', async function () { + await expectRevert( + UpgradeableBeacon.new(accounts[0]), + 'UpgradeableBeacon: implementation is not a contract', + ); + }); + + context('once deployed', async function () { + beforeEach('deploying beacon', async function () { + this.v1 = await Implementation1.new(); + this.beacon = await UpgradeableBeacon.new(this.v1.address, { from: owner }); + }); + + it('returns implementation', async function () { + expect(await this.beacon.implementation()).to.equal(this.v1.address); + }); + + it('can be upgraded by the owner', async function () { + const v2 = await Implementation2.new(); + const receipt = await this.beacon.upgradeTo(v2.address, { from: owner }); + expectEvent(receipt, 'Upgraded', { implementation: v2.address }); + expect(await this.beacon.implementation()).to.equal(v2.address); + }); + + it('cannot be upgraded to a non-contract', async function () { + await expectRevert( + this.beacon.upgradeTo(other, { from: owner }), + 'UpgradeableBeacon: implementation is not a contract', + ); + }); + + it('cannot be upgraded by other account', async function () { + const v2 = await Implementation2.new(); + await expectRevert( + this.beacon.upgradeTo(v2.address, { from: other }), + 'Ownable: caller is not the owner', + ); + }); + }); +}); diff --git a/test/proxy/UpgradeableProxy.behaviour.js b/test/proxy/UpgradeableProxy.behaviour.js index 923db198c..047451508 100644 --- a/test/proxy/UpgradeableProxy.behaviour.js +++ b/test/proxy/UpgradeableProxy.behaviour.js @@ -144,7 +144,7 @@ module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAd describe('non payable', function () { const expectedInitializedValue = 10; const initializeData = new DummyImplementation('').contract - .methods['initializeNonPayable(uint256)'](expectedInitializedValue).encodeABI(); + .methods.initializeNonPayableWithValue(expectedInitializedValue).encodeABI(); describe('when not sending balance', function () { beforeEach('creating proxy', async function () { @@ -175,7 +175,7 @@ module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAd describe('payable', function () { const expectedInitializedValue = 42; const initializeData = new DummyImplementation('').contract - .methods['initializePayable(uint256)'](expectedInitializedValue).encodeABI(); + .methods.initializePayableWithValue(expectedInitializedValue).encodeABI(); describe('when not sending balance', function () { beforeEach('creating proxy', async function () {