From 16312fcfb9b4e7bcfa3235b8aac770521a91376c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 2 Mar 2021 15:20:59 +0100 Subject: [PATCH] Rename UpgradeableProxy to ERC1967Proxy (#2547) Co-authored-by: Francisco Giordano (cherry picked from commit c789941d76dd713333bdeafe5b4484f6d9543c4e) --- CHANGELOG.md | 1 + .../ERC1967Proxy.sol} | 8 ++++---- contracts/proxy/README.adoc | 6 ++++-- .../transparent/TransparentUpgradeableProxy.sol | 6 +++--- test/proxy/ERC1967/ERC1967Proxy.test.js | 13 +++++++++++++ ...adeableProxy.behaviour.js => Proxy.behaviour.js} | 2 +- test/proxy/UpgradeableProxy.test.js | 13 ------------- test/proxy/{ => transparent}/ProxyAdmin.test.js | 0 .../TransparentUpgradeableProxy.behaviour.js | 2 +- .../transparent/TransparentUpgradeableProxy.test.js | 4 ++-- 10 files changed, 29 insertions(+), 26 deletions(-) rename contracts/proxy/{UpgradeableProxy.sol => ERC1967/ERC1967Proxy.sol} (92%) create mode 100644 test/proxy/ERC1967/ERC1967Proxy.test.js rename test/proxy/{UpgradeableProxy.behaviour.js => Proxy.behaviour.js} (98%) delete mode 100644 test/proxy/UpgradeableProxy.test.js rename test/proxy/{ => transparent}/ProxyAdmin.test.js (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 945904af5c6..3903fb65c73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * Meta Transactions: add `ERC2771Context` and a `MinimalForwarder` for meta-transactions. ([#2508](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2508)) * Overall reorganisation of the contract folder to improve clarity and discoverability. ([#2503](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2503)) * `ERC20Capped`: optimize gas usage of by enforcing te check directly in `_mint`. ([#2524](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2524)) + * Rename `UpgradeableProxy` to `ERC1967Proxy`. ([#2547](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2547)) ### How to upgrade from 3.x diff --git a/contracts/proxy/UpgradeableProxy.sol b/contracts/proxy/ERC1967/ERC1967Proxy.sol similarity index 92% rename from contracts/proxy/UpgradeableProxy.sol rename to contracts/proxy/ERC1967/ERC1967Proxy.sol index 8dd86464026..5528e608dee 100644 --- a/contracts/proxy/UpgradeableProxy.sol +++ b/contracts/proxy/ERC1967/ERC1967Proxy.sol @@ -2,8 +2,8 @@ pragma solidity ^0.8.0; -import "./Proxy.sol"; -import "../utils/Address.sol"; +import "../Proxy.sol"; +import "../../utils/Address.sol"; /** * @dev This contract implements an upgradeable proxy. It is upgradeable because calls are delegated to an @@ -14,7 +14,7 @@ import "../utils/Address.sol"; * Upgradeability is only provided internally through {_upgradeTo}. For an externally upgradeable proxy see * {TransparentUpgradeableProxy}. */ -contract UpgradeableProxy is Proxy { +contract ERC1967Proxy is Proxy { /** * @dev Initializes the upgradeable proxy with an initial implementation specified by `_logic`. * @@ -66,7 +66,7 @@ contract UpgradeableProxy is Proxy { * @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"); + require(Address.isContract(newImplementation), "ERC1967Proxy: new implementation is not a contract"); bytes32 slot = _IMPLEMENTATION_SLOT; diff --git a/contracts/proxy/README.adoc b/contracts/proxy/README.adoc index 0a2ecdf7b56..14a246d5eaa 100644 --- a/contracts/proxy/README.adoc +++ b/contracts/proxy/README.adoc @@ -7,7 +7,7 @@ This is a low-level set of contracts implementing different proxy patterns with 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. -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. +{ERC1967Proxy} provides a simple, fully functioning, proxy. While this proxy is not by itself upgradeable, it includes 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. @@ -19,7 +19,9 @@ CAUTION: Using upgradeable proxies correctly and securely is a difficult task th {{Proxy}} -{{UpgradeableProxy}} +== ERC1967 + +{{ERC1967Proxy}} == Transparent Proxy diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol index 4ce4eec36d5..486c473de8d 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 "../ERC1967/ERC1967Proxy.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 ERC1967Proxy { /** * @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 ERC1967Proxy(_logic, _data) { assert(_ADMIN_SLOT == bytes32(uint256(keccak256("eip1967.proxy.admin")) - 1)); _setAdmin(admin_); } diff --git a/test/proxy/ERC1967/ERC1967Proxy.test.js b/test/proxy/ERC1967/ERC1967Proxy.test.js new file mode 100644 index 00000000000..22df960ffbd --- /dev/null +++ b/test/proxy/ERC1967/ERC1967Proxy.test.js @@ -0,0 +1,13 @@ +const shouldBehaveLikeProxy = require('../Proxy.behaviour'); + +const ERC1967Proxy = artifacts.require('ERC1967Proxy'); + +contract('ERC1967Proxy', function (accounts) { + const [proxyAdminOwner] = accounts; + + const createProxy = async function (implementation, _admin, initData, opts) { + return ERC1967Proxy.new(implementation, initData, opts); + }; + + shouldBehaveLikeProxy(createProxy, undefined, proxyAdminOwner); +}); diff --git a/test/proxy/UpgradeableProxy.behaviour.js b/test/proxy/Proxy.behaviour.js similarity index 98% rename from test/proxy/UpgradeableProxy.behaviour.js rename to test/proxy/Proxy.behaviour.js index dc6ffb1fbe5..8cb457b06c6 100644 --- a/test/proxy/UpgradeableProxy.behaviour.js +++ b/test/proxy/Proxy.behaviour.js @@ -11,7 +11,7 @@ function toChecksumAddress (address) { return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); } -module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAdminAddress, proxyCreator) { +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(''); diff --git a/test/proxy/UpgradeableProxy.test.js b/test/proxy/UpgradeableProxy.test.js deleted file mode 100644 index 12d54bdbd42..00000000000 --- a/test/proxy/UpgradeableProxy.test.js +++ /dev/null @@ -1,13 +0,0 @@ -const shouldBehaveLikeUpgradeableProxy = require('./UpgradeableProxy.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); - }; - - shouldBehaveLikeUpgradeableProxy(createProxy, undefined, proxyAdminOwner); -}); diff --git a/test/proxy/ProxyAdmin.test.js b/test/proxy/transparent/ProxyAdmin.test.js similarity index 100% rename from test/proxy/ProxyAdmin.test.js rename to test/proxy/transparent/ProxyAdmin.test.js diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js index 287c407d6b6..7c702fb3ff9 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', + 'ERC1967Proxy: new implementation is not a contract', ); }); }); diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.test.js b/test/proxy/transparent/TransparentUpgradeableProxy.test.js index cf8206996bc..86dd55d32c1 100644 --- a/test/proxy/transparent/TransparentUpgradeableProxy.test.js +++ b/test/proxy/transparent/TransparentUpgradeableProxy.test.js @@ -1,4 +1,4 @@ -const shouldBehaveLikeUpgradeableProxy = require('../UpgradeableProxy.behaviour'); +const shouldBehaveLikeProxy = require('../Proxy.behaviour'); const shouldBehaveLikeTransparentUpgradeableProxy = require('./TransparentUpgradeableProxy.behaviour'); const TransparentUpgradeableProxy = artifacts.require('TransparentUpgradeableProxy'); @@ -10,6 +10,6 @@ contract('TransparentUpgradeableProxy', function (accounts) { return TransparentUpgradeableProxy.new(logic, admin, initData, opts); }; - shouldBehaveLikeUpgradeableProxy(createProxy, proxyAdminAddress, proxyAdminOwner); + shouldBehaveLikeProxy(createProxy, proxyAdminAddress, proxyAdminOwner); shouldBehaveLikeTransparentUpgradeableProxy(createProxy, accounts); });