Skip to content

Commit

Permalink
Make ERC1967Upgrades a library instead of an abstract contract (#4325)
Browse files Browse the repository at this point in the history
  • Loading branch information
Amxx committed Jun 15, 2023
1 parent 05ef692 commit ff85c7b
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 61 deletions.
5 changes: 5 additions & 0 deletions .changeset/grumpy-worms-tease.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': major
---

`ERC1967Utils`: Refactor the `ERC1967Upgrade` abstract contract as a library.
15 changes: 9 additions & 6 deletions contracts/mocks/proxy/UUPSLegacy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@ import "./UUPSUpgradeableMock.sol";
// This contract implements the pre-4.5 UUPS upgrade function with a rollback test.
// It's used to test that newer UUPS contracts are considered valid upgrades by older UUPS contracts.
contract UUPSUpgradeableLegacyMock is UUPSUpgradeableMock {
// Inlined from ERC1967Upgrade
// Inlined from ERC1967Utils
bytes32 private constant _ROLLBACK_SLOT = 0x4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd9143;

// ERC1967Upgrade._setImplementation is private so we reproduce it here.
// ERC1967Utils._setImplementation is private so we reproduce it here.
// An extra underscore prevents a name clash error.
function __setImplementation(address newImplementation) private {
require(newImplementation.code.length > 0, "ERC1967: new implementation is not a contract");
StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation;
StorageSlot.getAddressSlot(ERC1967Utils.IMPLEMENTATION_SLOT).value = newImplementation;
}

function _upgradeToAndCallSecureLegacyV1(address newImplementation, bytes memory data, bool forceCall) internal {
address oldImplementation = _getImplementation();
address oldImplementation = ERC1967Utils.getImplementation();

// Initial upgrade and setup call
__setImplementation(newImplementation);
Expand All @@ -34,9 +34,12 @@ contract UUPSUpgradeableLegacyMock is UUPSUpgradeableMock {
Address.functionDelegateCall(newImplementation, abi.encodeCall(this.upgradeTo, (oldImplementation)));
rollbackTesting.value = false;
// Check rollback was effective
require(oldImplementation == _getImplementation(), "ERC1967Upgrade: upgrade breaks further upgrades");
require(
oldImplementation == ERC1967Utils.getImplementation(),
"ERC1967Utils: upgrade breaks further upgrades"
);
// Finally reset to the new implementation and log the upgrade
_upgradeTo(newImplementation);
ERC1967Utils.upgradeTo(newImplementation);
}
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/mocks/proxy/UUPSUpgradeableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ contract UUPSUpgradeableMock is NonUpgradeableMock, UUPSUpgradeable {

contract UUPSUpgradeableUnsafeMock is UUPSUpgradeableMock {
function upgradeTo(address newImplementation) public override {
_upgradeToAndCall(newImplementation, bytes(""), false);
ERC1967Utils.upgradeToAndCall(newImplementation, bytes(""), false);
}

function upgradeToAndCall(address newImplementation, bytes memory data) public payable override {
_upgradeToAndCall(newImplementation, data, false);
ERC1967Utils.upgradeToAndCall(newImplementation, data, false);
}
}
8 changes: 4 additions & 4 deletions contracts/proxy/ERC1967/ERC1967Proxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,23 @@
pragma solidity ^0.8.19;

import "../Proxy.sol";
import "./ERC1967Upgrade.sol";
import "./ERC1967Utils.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.
*/
contract ERC1967Proxy is Proxy, ERC1967Upgrade {
contract ERC1967Proxy 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 initializing the storage of the proxy like a Solidity constructor.
*/
constructor(address _logic, bytes memory _data) payable {
_upgradeToAndCall(_logic, _data, false);
ERC1967Utils.upgradeToAndCall(_logic, _data, false);
}

/**
Expand All @@ -31,6 +31,6 @@ contract ERC1967Proxy is Proxy, ERC1967Upgrade {
* `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc`
*/
function _implementation() internal view virtual override returns (address impl) {
return _getImplementation();
return ERC1967Utils.getImplementation();
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v4.9.0) (proxy/ERC1967/ERC1967Upgrade.sol)
// OpenZeppelin Contracts (last updated v4.9.0) (proxy/ERC1967/ERC1967Utils.sol)

pragma solidity ^0.8.19;
pragma solidity ^0.8.20;

import "../beacon/IBeacon.sol";
import "../../interfaces/IERC1967.sol";
Expand All @@ -15,7 +15,24 @@ import "../../utils/StorageSlot.sol";
*
* _Available since v4.1._
*/
abstract contract ERC1967Upgrade is IERC1967 {
library ERC1967Utils {
// We re-declare ERC-1967 events here because they can't be used directly from IERC1967.
// This will be fixed in Solidity 0.8.21. At that point we should remove these events.
/**
* @dev Emitted when the implementation is upgraded.
*/
event Upgraded(address indexed implementation);

/**
* @dev Emitted when the admin account has changed.
*/
event AdminChanged(address previousAdmin, address newAdmin);

/**
* @dev Emitted when the beacon is changed.
*/
event BeaconUpgraded(address indexed beacon);

// This is the keccak-256 hash of "eip1967.proxy.rollback" subtracted by 1
bytes32 private constant _ROLLBACK_SLOT = 0x4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd9143;

Expand All @@ -24,7 +41,8 @@ abstract contract ERC1967Upgrade is IERC1967 {
* This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1, and is
* validated in the constructor.
*/
bytes32 internal constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
// solhint-disable-next-line private-vars-leading-underscore
bytes32 internal constant IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;

/**
* @dev The `implementation` of the proxy is invalid.
Expand All @@ -49,8 +67,8 @@ abstract contract ERC1967Upgrade is IERC1967 {
/**
* @dev Returns the current implementation address.
*/
function _getImplementation() internal view returns (address) {
return StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value;
function getImplementation() internal view returns (address) {
return StorageSlot.getAddressSlot(IMPLEMENTATION_SLOT).value;
}

/**
Expand All @@ -60,26 +78,26 @@ abstract contract ERC1967Upgrade is IERC1967 {
if (newImplementation.code.length == 0) {
revert ERC1967InvalidImplementation(newImplementation);
}
StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation;
StorageSlot.getAddressSlot(IMPLEMENTATION_SLOT).value = newImplementation;
}

/**
* @dev Perform implementation upgrade
*
* Emits an {Upgraded} event.
* Emits an {IERC1967-Upgraded} event.
*/
function _upgradeTo(address newImplementation) internal {
function upgradeTo(address newImplementation) internal {
_setImplementation(newImplementation);
emit Upgraded(newImplementation);
}

/**
* @dev Perform implementation upgrade with additional setup call.
*
* Emits an {Upgraded} event.
* Emits an {IERC1967-Upgraded} event.
*/
function _upgradeToAndCall(address newImplementation, bytes memory data, bool forceCall) internal {
_upgradeTo(newImplementation);
function upgradeToAndCall(address newImplementation, bytes memory data, bool forceCall) internal {
upgradeTo(newImplementation);
if (data.length > 0 || forceCall) {
Address.functionDelegateCall(newImplementation, data);
}
Expand All @@ -88,24 +106,24 @@ abstract contract ERC1967Upgrade is IERC1967 {
/**
* @dev Perform implementation upgrade with security checks for UUPS proxies, and additional setup call.
*
* Emits an {Upgraded} event.
* Emits an {IERC1967-Upgraded} event.
*/
function _upgradeToAndCallUUPS(address newImplementation, bytes memory data, bool forceCall) internal {
function upgradeToAndCallUUPS(address newImplementation, bytes memory data, bool forceCall) internal {
// Upgrades from old implementations will perform a rollback test. This test requires the new
// implementation to upgrade back to the old, non-ERC1822 compliant, implementation. Removing
// this special case will break upgrade paths from old UUPS implementation to new ones.
if (StorageSlot.getBooleanSlot(_ROLLBACK_SLOT).value) {
_setImplementation(newImplementation);
} else {
try IERC1822Proxiable(newImplementation).proxiableUUID() returns (bytes32 slot) {
if (slot != _IMPLEMENTATION_SLOT) {
if (slot != IMPLEMENTATION_SLOT) {
revert ERC1967UnsupportedProxiableUUID(slot);
}
} catch {
// The implementation is not UUPS
revert ERC1967InvalidImplementation(newImplementation);
}
_upgradeToAndCall(newImplementation, data, forceCall);
upgradeToAndCall(newImplementation, data, forceCall);
}
}

Expand All @@ -114,7 +132,8 @@ abstract contract ERC1967Upgrade is IERC1967 {
* This is the keccak-256 hash of "eip1967.proxy.admin" subtracted by 1, and is
* validated in the constructor.
*/
bytes32 internal constant _ADMIN_SLOT = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;
// solhint-disable-next-line private-vars-leading-underscore
bytes32 internal constant ADMIN_SLOT = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;

/**
* @dev Returns the current admin.
Expand All @@ -123,8 +142,8 @@ abstract contract ERC1967Upgrade is IERC1967 {
* https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call.
* `0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103`
*/
function _getAdmin() internal view returns (address) {
return StorageSlot.getAddressSlot(_ADMIN_SLOT).value;
function getAdmin() internal view returns (address) {
return StorageSlot.getAddressSlot(ADMIN_SLOT).value;
}

/**
Expand All @@ -134,30 +153,31 @@ abstract contract ERC1967Upgrade is IERC1967 {
if (newAdmin == address(0)) {
revert ERC1967InvalidAdmin(address(0));
}
StorageSlot.getAddressSlot(_ADMIN_SLOT).value = newAdmin;
StorageSlot.getAddressSlot(ADMIN_SLOT).value = newAdmin;
}

/**
* @dev Changes the admin of the proxy.
*
* Emits an {AdminChanged} event.
* Emits an {IERC1967-AdminChanged} event.
*/
function _changeAdmin(address newAdmin) internal {
emit AdminChanged(_getAdmin(), newAdmin);
function changeAdmin(address newAdmin) internal {
emit AdminChanged(getAdmin(), newAdmin);
_setAdmin(newAdmin);
}

/**
* @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.
* This is bytes32(uint256(keccak256('eip1967.proxy.beacon')) - 1) and is validated in the constructor.
*/
bytes32 internal constant _BEACON_SLOT = 0xa3f0ad74e5423aebfd80d3ef4346578335a9a72aeaee59ff6cb3582b35133d50;
// solhint-disable-next-line private-vars-leading-underscore
bytes32 internal constant BEACON_SLOT = 0xa3f0ad74e5423aebfd80d3ef4346578335a9a72aeaee59ff6cb3582b35133d50;

/**
* @dev Returns the current beacon.
*/
function _getBeacon() internal view returns (address) {
return StorageSlot.getAddressSlot(_BEACON_SLOT).value;
function getBeacon() internal view returns (address) {
return StorageSlot.getAddressSlot(BEACON_SLOT).value;
}

/**
Expand All @@ -173,16 +193,16 @@ abstract contract ERC1967Upgrade is IERC1967 {
revert ERC1967InvalidImplementation(beaconImplementation);
}

StorageSlot.getAddressSlot(_BEACON_SLOT).value = newBeacon;
StorageSlot.getAddressSlot(BEACON_SLOT).value = newBeacon;
}

/**
* @dev Perform beacon upgrade with additional setup call. Note: This upgrades the address of the beacon, it does
* not upgrade the implementation contained in the beacon (see {UpgradeableBeacon-_setImplementation} for that).
*
* Emits a {BeaconUpgraded} event.
* Emits an {IERC1967-BeaconUpgraded} event.
*/
function _upgradeBeaconToAndCall(address newBeacon, bytes memory data, bool forceCall) internal {
function upgradeBeaconToAndCall(address newBeacon, bytes memory data, bool forceCall) internal {
_setBeacon(newBeacon);
emit BeaconUpgraded(newBeacon);
if (data.length > 0 || forceCall) {
Expand Down
4 changes: 2 additions & 2 deletions contracts/proxy/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Most of the proxies below are built on an abstract base contract.
In order to avoid clashes with the storage variables of the implementation contract behind a proxy, we use https://eips.ethereum.org/EIPS/eip-1967[EIP1967] storage slots.

- {ERC1967Upgrade}: Internal functions to get and set the storage slots defined in EIP1967.
- {ERC1967Utils}: Internal functions to get and set the storage slots defined in EIP1967.
- {ERC1967Proxy}: A proxy using EIP1967 storage slots. Not upgradeable by default.
There are two alternative ways to add upgradeability to an ERC1967 proxy. Their differences are explained below in <<transparent-vs-uups>>.
Expand Down Expand Up @@ -60,7 +60,7 @@ The current implementation of this security mechanism uses https://eips.ethereum

{{ERC1967Proxy}}

{{ERC1967Upgrade}}
{{ERC1967Utils}}

== Transparent Proxy

Expand Down
8 changes: 4 additions & 4 deletions contracts/proxy/beacon/BeaconProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity ^0.8.19;

import "./IBeacon.sol";
import "../Proxy.sol";
import "../ERC1967/ERC1967Upgrade.sol";
import "../ERC1967/ERC1967Utils.sol";

/**
* @dev This contract implements a proxy that gets the implementation address for each call from an {UpgradeableBeacon}.
Expand All @@ -15,7 +15,7 @@ import "../ERC1967/ERC1967Upgrade.sol";
*
* _Available since v3.4._
*/
contract BeaconProxy is Proxy, ERC1967Upgrade {
contract BeaconProxy is Proxy {
/**
* @dev Initializes the proxy with `beacon`.
*
Expand All @@ -28,13 +28,13 @@ contract BeaconProxy is Proxy, ERC1967Upgrade {
* - `beacon` must be a contract with the interface {IBeacon}.
*/
constructor(address beacon, bytes memory data) payable {
_upgradeBeaconToAndCall(beacon, data, false);
ERC1967Utils.upgradeBeaconToAndCall(beacon, data, false);
}

/**
* @dev Returns the current implementation address of the associated beacon.
*/
function _implementation() internal view virtual override returns (address) {
return IBeacon(_getBeacon()).implementation();
return IBeacon(ERC1967Utils.getBeacon()).implementation();
}
}
10 changes: 5 additions & 5 deletions contracts/proxy/transparent/TransparentUpgradeableProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
* optionally initialized with `_data` as explained in {ERC1967Proxy-constructor}.
*/
constructor(address _logic, address admin_, bytes memory _data) payable ERC1967Proxy(_logic, _data) {
_changeAdmin(admin_);
ERC1967Utils.changeAdmin(admin_);
}

/**
* @dev If caller is the admin process the call internally, otherwise transparently fallback to the proxy behavior
*/
function _fallback() internal virtual override {
if (msg.sender == _getAdmin()) {
if (msg.sender == ERC1967Utils.getAdmin()) {
bytes memory ret;
bytes4 selector = msg.sig;
if (selector == ITransparentUpgradeableProxy.upgradeTo.selector) {
Expand Down Expand Up @@ -103,7 +103,7 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
_requireZeroValue();

address newAdmin = abi.decode(msg.data[4:], (address));
_changeAdmin(newAdmin);
ERC1967Utils.changeAdmin(newAdmin);

return "";
}
Expand All @@ -115,7 +115,7 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
_requireZeroValue();

address newImplementation = abi.decode(msg.data[4:], (address));
_upgradeToAndCall(newImplementation, bytes(""), false);
ERC1967Utils.upgradeToAndCall(newImplementation, bytes(""), false);

return "";
}
Expand All @@ -127,7 +127,7 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
*/
function _dispatchUpgradeToAndCall() private returns (bytes memory) {
(address newImplementation, bytes memory data) = abi.decode(msg.data[4:], (address, bytes));
_upgradeToAndCall(newImplementation, data, true);
ERC1967Utils.upgradeToAndCall(newImplementation, data, true);

return "";
}
Expand Down

0 comments on commit ff85c7b

Please sign in to comment.