diff --git a/contracts/proxy/UpgradeableProxy.sol b/contracts/proxy/UpgradeableProxy.sol deleted file mode 100644 index 8dd86464026..00000000000 --- a/contracts/proxy/UpgradeableProxy.sol +++ /dev/null @@ -1,78 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.0; - -import "./Proxy.sol"; -import "../utils/Address.sol"; - -/** - * @dev This contract implements an upgradeable proxy. It is upgradeable because calls are delegated to an - * implementation address that can be changed. This address is stored in storage in the location specified by - * https://eips.ethereum.org/EIPS/eip-1967[EIP1967], so that it doesn't conflict with the storage layout of the - * implementation behind the proxy. - * - * Upgradeability is only provided internally through {_upgradeTo}. For an externally upgradeable proxy see - * {TransparentUpgradeableProxy}. - */ -contract UpgradeableProxy is Proxy { - /** - * @dev Initializes the upgradeable proxy with an initial implementation specified by `_logic`. - * - * If `_data` is nonempty, it's used as data in a delegate call to `_logic`. This will typically be an encoded - * function call, and allows initializating the storage of the proxy like a Solidity constructor. - */ - constructor(address _logic, bytes memory _data) payable { - assert(_IMPLEMENTATION_SLOT == bytes32(uint256(keccak256("eip1967.proxy.implementation")) - 1)); - _setImplementation(_logic); - if(_data.length > 0) { - Address.functionDelegateCall(_logic, _data); - } - } - - /** - * @dev Emitted when the implementation is upgraded. - */ - event Upgraded(address indexed implementation); - - /** - * @dev Storage slot with the address of the current implementation. - * This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1, and is - * validated in the constructor. - */ - bytes32 private constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; - - /** - * @dev Returns the current implementation address. - */ - function _implementation() internal view virtual override returns (address impl) { - bytes32 slot = _IMPLEMENTATION_SLOT; - // solhint-disable-next-line no-inline-assembly - assembly { - impl := sload(slot) - } - } - - /** - * @dev Upgrades the proxy to a new implementation. - * - * Emits an {Upgraded} event. - */ - function _upgradeTo(address newImplementation) internal virtual { - _setImplementation(newImplementation); - emit Upgraded(newImplementation); - } - - /** - * @dev Stores a new address in the EIP1967 implementation slot. - */ - function _setImplementation(address newImplementation) private { - require(Address.isContract(newImplementation), "UpgradeableProxy: new implementation is not a contract"); - - bytes32 slot = _IMPLEMENTATION_SLOT; - - // solhint-disable-next-line no-inline-assembly - assembly { - sstore(slot, newImplementation) - } - } -} diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol index 4ce4eec36d5..120762d0dbd 100644 --- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol +++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; -import "../UpgradeableProxy.sol"; +import "../uups/UUPSProxy.sol"; /** * @dev This contract implements a proxy that is upgradeable by an admin. @@ -25,12 +25,12 @@ import "../UpgradeableProxy.sol"; * Our recommendation is for the dedicated account to be an instance of the {ProxyAdmin} contract. If set up this way, * you should think of the `ProxyAdmin` instance as the real administrative interface of your proxy. */ -contract TransparentUpgradeableProxy is UpgradeableProxy { +contract TransparentUpgradeableProxy is UUPSProxy { /** * @dev Initializes an upgradeable proxy managed by `_admin`, backed by the implementation at `_logic`, and * optionally initialized with `_data` as explained in {UpgradeableProxy-constructor}. */ - constructor(address _logic, address admin_, bytes memory _data) payable UpgradeableProxy(_logic, _data) { + constructor(address _logic, address admin_, bytes memory _data) payable UUPSProxy(_logic, _data) { assert(_ADMIN_SLOT == bytes32(uint256(keccak256("eip1967.proxy.admin")) - 1)); _setAdmin(admin_); } diff --git a/contracts/proxy/uups/UUPS.sol b/contracts/proxy/uups/UUPS.sol index 5b0f690b4c6..7eb2df9a772 100644 --- a/contracts/proxy/uups/UUPS.sol +++ b/contracts/proxy/uups/UUPS.sol @@ -2,22 +2,49 @@ pragma solidity ^0.8.0; -library UUPS { - // bytes32 private constant _UUPS_SLOT = keccak256("PROXIABLE"); - bytes32 private constant _UUPS_SLOT = 0xc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf7; +import "../../utils/Address.sol"; - struct UUPSStore { - address implementation; +abstract contract UUPS { + /** + * @dev Storage slot with the address of the current implementation. + * This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1, and is + * validated in the constructor. + */ + bytes32 private constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; + + /** + * @dev Emitted when the implementation is upgraded. + */ + event Upgraded(address indexed implementation); + + /** + * + */ + function _uuid() internal view virtual returns (bytes32) { + return _IMPLEMENTATION_SLOT; } - function instance() internal pure returns (UUPSStore storage uups) { - bytes32 position = _UUPS_SLOT; - assembly { - uups.slot := position - } + /** + * @dev Upgrades the proxy to a new implementation. + * + * Emits an {Upgraded} event. + */ + function _upgradeTo(address newImplementation) internal virtual { + _setImplementation(newImplementation); + emit Upgraded(newImplementation); } - function uuid() internal pure returns (bytes32) { - return _UUPS_SLOT; + /** + * @dev Stores a new address in the EIP1967 implementation slot. + */ + function _setImplementation(address newImplementation) private { + require(Address.isContract(newImplementation), "UUPS: new implementation is not a contract"); + + bytes32 slot = _IMPLEMENTATION_SLOT; + + // solhint-disable-next-line no-inline-assembly + assembly { + sstore(slot, newImplementation) + } } } diff --git a/contracts/proxy/uups/UUPSProxiable.sol b/contracts/proxy/uups/UUPSProxiable.sol index d6a79da8462..c2af91af540 100644 --- a/contracts/proxy/uups/UUPSProxiable.sol +++ b/contracts/proxy/uups/UUPSProxiable.sol @@ -4,13 +4,13 @@ pragma solidity ^0.8.0; import "./UUPS.sol"; -abstract contract UUPSProxiable { - function proxiableUUID() public pure returns (bytes32) { - return UUPS.uuid(); +abstract contract UUPSProxiable is UUPS { + function proxiableUUID() public view returns (bytes32) { + return _uuid(); } function _updateCodeAddress(address newAddress) internal { - require(UUPS.uuid() == UUPSProxiable(newAddress).proxiableUUID(), "Not compatible"); - UUPS.instance().implementation = newAddress; + require(_uuid() == UUPSProxiable(newAddress).proxiableUUID(), "UUPS: new impelmentation is not compatible"); + _upgradeTo(newAddress); } } diff --git a/contracts/proxy/uups/UUPSProxy.sol b/contracts/proxy/uups/UUPSProxy.sol index 67228c09ac5..9529f35920f 100644 --- a/contracts/proxy/uups/UUPSProxy.sol +++ b/contracts/proxy/uups/UUPSProxy.sol @@ -3,20 +3,40 @@ pragma solidity ^0.8.0; import "./UUPS.sol"; -import "./UUPSProxiable.sol"; import "../Proxy.sol"; -import "../../utils/Address.sol"; -contract UUPSProxy is Proxy { +/** + * @dev This contract implements an upgradeable proxy. It is upgradeable because calls are delegated to an + * implementation address that can be changed. This address is stored in storage in the location specified by + * https://eips.ethereum.org/EIPS/eip-1967[EIP1967], so that it doesn't conflict with the storage layout of the + * implementation behind the proxy. + * + * Upgradeability is only provided internally through {_upgradeTo}. For an externally upgradeable proxy see + * {TransparentUpgradeableProxy}. + */ +contract UUPSProxy is Proxy, UUPS { + /** + * @dev Initializes the upgradeable proxy with an initial implementation specified by `_logic`. + * + * If `_data` is nonempty, it's used as data in a delegate call to `_logic`. This will typically be an encoded + * function call, and allows initializating the storage of the proxy like a Solidity constructor. + */ constructor(address _logic, bytes memory _data) payable { - require(UUPS.uuid() == UUPSProxiable(_logic).proxiableUUID(), "Not compatible"); - UUPS.instance().implementation = _logic; - if (_data.length > 0) { + assert(_uuid() == bytes32(uint256(keccak256("eip1967.proxy.implementation")) - 1)); + _upgradeTo(_logic); + if(_data.length > 0) { Address.functionDelegateCall(_logic, _data); } } - function _implementation() internal view virtual override returns (address) { - return UUPS.instance().implementation; + /** + * @dev Returns the current implementation address. + */ + function _implementation() internal view virtual override returns (address impl) { + bytes32 slot = _uuid(); + // solhint-disable-next-line no-inline-assembly + assembly { + impl := sload(slot) + } } } diff --git a/test/proxy/Proxy.behaviour.js b/test/proxy/Proxy.behaviour.js index e31590d2726..8cb457b06c6 100644 --- a/test/proxy/Proxy.behaviour.js +++ b/test/proxy/Proxy.behaviour.js @@ -11,13 +11,7 @@ function toChecksumAddress (address) { return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); } -module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, proxyCreator, opts = {}) { - const { artefact, slot } = Object.assign({ - artefact: DummyImplementation, - slot: '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16), - }, opts); - - +module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, proxyCreator) { it('cannot be initialized with a non-contract address', async function () { const nonContractAddress = proxyCreator; const initializeData = Buffer.from(''); @@ -29,17 +23,18 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, }); before('deploy implementation', async function () { - this.implementation = web3.utils.toChecksumAddress((await artefact.new()).address); + this.implementation = web3.utils.toChecksumAddress((await DummyImplementation.new()).address); }); const assertProxyInitialization = function ({ value, balance }) { it('sets the implementation address', async function () { + 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)).substr(-40)); expect(implementation).to.be.equal(this.implementation); }); it('initializes the proxy', async function () { - const dummy = new artefact(this.proxy); + const dummy = new DummyImplementation(this.proxy); expect(await dummy.value()).to.be.bignumber.equal(value.toString()); }); @@ -82,7 +77,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, describe('initialization without parameters', function () { describe('non payable', function () { const expectedInitializedValue = 10; - const initializeData = new artefact('').contract.methods['initializeNonPayable()']().encodeABI(); + const initializeData = new DummyImplementation('').contract.methods['initializeNonPayable()']().encodeABI(); describe('when not sending balance', function () { beforeEach('creating proxy', async function () { @@ -112,7 +107,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, describe('payable', function () { const expectedInitializedValue = 100; - const initializeData = new artefact('').contract.methods['initializePayable()']().encodeABI(); + const initializeData = new DummyImplementation('').contract.methods['initializePayable()']().encodeABI(); describe('when not sending balance', function () { beforeEach('creating proxy', async function () { @@ -152,7 +147,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, describe('initialization with parameters', function () { describe('non payable', function () { const expectedInitializedValue = 10; - const initializeData = new artefact('').contract + const initializeData = new DummyImplementation('').contract .methods.initializeNonPayableWithValue(expectedInitializedValue).encodeABI(); describe('when not sending balance', function () { @@ -183,7 +178,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, describe('payable', function () { const expectedInitializedValue = 42; - const initializeData = new artefact('').contract + const initializeData = new DummyImplementation('').contract .methods.initializePayableWithValue(expectedInitializedValue).encodeABI(); describe('when not sending balance', function () { @@ -221,7 +216,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, }); describe('reverting initialization', function () { - const initializeData = new artefact('').contract + const initializeData = new DummyImplementation('').contract .methods.reverts().encodeABI(); it('reverts', async function () { diff --git a/test/proxy/UpgradeableProxy.test.js b/test/proxy/UpgradeableProxy.test.js deleted file mode 100644 index 274e632cb28..00000000000 --- a/test/proxy/UpgradeableProxy.test.js +++ /dev/null @@ -1,13 +0,0 @@ -const shouldBehaveLikeProxy = require('./Proxy.behaviour'); - -const UpgradeableProxy = artifacts.require('UpgradeableProxy'); - -contract('UpgradeableProxy', function (accounts) { - const [proxyAdminOwner] = accounts; - - const createProxy = async function (implementation, _admin, initData, opts) { - return UpgradeableProxy.new(implementation, initData, opts); - }; - - shouldBehaveLikeProxy(createProxy, undefined, proxyAdminOwner); -}); diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js index 287c407d6b6..2b66c00ee17 100644 --- a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js +++ b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js @@ -80,7 +80,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy (createPro it('reverts', async function () { await expectRevert( this.proxy.upgradeTo(ZERO_ADDRESS, { from }), - 'UpgradeableProxy: new implementation is not a contract', + 'UUPS: new implementation is not a contract', ); }); }); diff --git a/test/proxy/uups/UUPSProxiable.test.js b/test/proxy/uups/UUPSProxiable.test.js new file mode 100644 index 00000000000..e62c24e07b2 --- /dev/null +++ b/test/proxy/uups/UUPSProxiable.test.js @@ -0,0 +1,53 @@ +const { expectRevert } = require('@openzeppelin/test-helpers'); +const ethereumjsUtil = require('ethereumjs-util'); + +const DummyImplementation = artifacts.require('DummyImplementation'); +const DummyImplementationProxiable = artifacts.require('DummyImplementationProxiable'); +const DummyImplementationV2Proxiable = artifacts.require('DummyImplementationV2Proxiable'); +const UUPSProxy = artifacts.require('UUPSProxy'); + +const UUPS_UUID = '0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc'; + +function toChecksumAddress (address) { + return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); +} + +contract('UUPSProxiable', function (accounts) { + const [ proxyCreator ] = accounts; + + const createProxy = async function (implementation, _admin, initData, opts) { + return UUPSProxy.new(implementation, initData, opts); + }; + + context('UUPS', function () { + beforeEach(async function () { + this.impl = await DummyImplementation.new(); + this.implproxiable = await DummyImplementationProxiable.new(); + this.implproxiablev2 = await DummyImplementationV2Proxiable.new(); + const { address } = await createProxy(this.implproxiable.address, undefined, '0x', { from: proxyCreator }); + this.dummy = new DummyImplementationProxiable(address); + }); + + it('check uuid', async function () { + expect(await this.dummy.proxiableUUID()).to.be.equal(UUPS_UUID); + }); + + it('can update to proxiable', async function () { + await this.dummy.updateCodeAddress(this.implproxiablev2.address); + const implementation = toChecksumAddress((await web3.eth.getStorageAt( + this.dummy.address, + UUPS_UUID, + )).substr(-40)); + expect(implementation).to.be.equal(this.implproxiablev2.address); + }); + + it('does not deploy with non-proxiable', async function () { + this.skip(); // NOT SUPPORTED YET + await expectRevert.unspecified(createProxy(this.impl.address, undefined, '0x', { from: proxyCreator })); + }); + + it('does not update to non-proxiable', async function () { + await expectRevert.unspecified(this.dummy.updateCodeAddress(this.impl.address)); + }); + }); +}); diff --git a/test/proxy/uups/UUPSProxy.test.js b/test/proxy/uups/UUPSProxy.test.js index ae142ca5a57..a960c0d8a2b 100644 --- a/test/proxy/uups/UUPSProxy.test.js +++ b/test/proxy/uups/UUPSProxy.test.js @@ -1,56 +1,13 @@ -const { expectRevert } = require('@openzeppelin/test-helpers'); -const ethereumjsUtil = require('ethereumjs-util'); - const shouldBehaveLikeProxy = require('../Proxy.behaviour'); -const DummyImplementation = artifacts.require('DummyImplementation') -const DummyImplementationProxiable = artifacts.require('DummyImplementationProxiable') -const DummyImplementationV2Proxiable = artifacts.require('DummyImplementationV2Proxiable') const UUPSProxy = artifacts.require('UUPSProxy'); -const UUPS_UUID = '0xc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf7'; - -function toChecksumAddress (address) { - return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); -} - contract('UUPSProxy', function (accounts) { - const [ proxyCreator ] = accounts; + const [proxyAdminOwner] = accounts; const createProxy = async function (implementation, _admin, initData, opts) { return UUPSProxy.new(implementation, initData, opts); }; - shouldBehaveLikeProxy(createProxy, undefined, proxyCreator, { - artefact: DummyImplementationProxiable, - slot: UUPS_UUID, - }); - - context('UUPS', function () { - beforeEach(async function () { - this.implementation = await DummyImplementation.new(); - this.implementationproxiable = await DummyImplementationProxiable.new(); - this.implementationproxiablev2 = await DummyImplementationV2Proxiable.new(); - const { address } = await createProxy(this.implementationproxiable.address, undefined, '0x', { from: proxyCreator }); - this.dummy = new DummyImplementationProxiable(address); - }); - - it('check uuid', async function() { - expect(await this.dummy.proxiableUUID()).to.be.equal(UUPS_UUID); - }); - - it('can update to proxiable', async function() { - await this.dummy.updateCodeAddress(this.implementationproxiablev2.address); - const implementation = toChecksumAddress((await web3.eth.getStorageAt(this.dummy.address, UUPS_UUID)).substr(-40)); - expect(implementation).to.be.equal(this.implementationproxiablev2.address); - }); - - it('does not deploy with non-proxiable', async function() { - await expectRevert.unspecified(createProxy(this.implementation.address, undefined, '0x', { from: proxyCreator })); - }); - - it('does not update to non-proxiable', async function() { - await expectRevert.unspecified(this.dummy.updateCodeAddress(this.implementation.address)); - }); - }); + shouldBehaveLikeProxy(createProxy, undefined, proxyAdminOwner); });