From 1e133aee0ee7bb68ed940c16457eb71302316d58 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 9 Mar 2021 15:06:58 +0100 Subject: [PATCH] refactor --- contracts/proxy/ERC1967/ERC1967Proxy.sol | 4 +- .../{ERC1967Utils.sol => ERC1967Storage.sol} | 115 ++++++++---------- contracts/proxy/ERC1967/ERC1967Upgrade.sol | 4 +- .../proxy/ERC1967/ERC1967UpgradeSecure.sol | 2 +- contracts/proxy/beacon/BeaconProxy.sol | 4 +- .../TransparentUpgradeableProxy.sol | 9 +- 6 files changed, 57 insertions(+), 81 deletions(-) rename contracts/proxy/ERC1967/{ERC1967Utils.sol => ERC1967Storage.sol} (86%) diff --git a/contracts/proxy/ERC1967/ERC1967Proxy.sol b/contracts/proxy/ERC1967/ERC1967Proxy.sol index dcffe0f3a4f..595257bc096 100644 --- a/contracts/proxy/ERC1967/ERC1967Proxy.sol +++ b/contracts/proxy/ERC1967/ERC1967Proxy.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; -import "./ERC1967Utils.sol"; +import "./ERC1967Storage.sol"; import "../Proxy.sol"; import "../../utils/Address.sol"; @@ -15,7 +15,7 @@ import "../../utils/Address.sol"; * Upgradeability is only provided internally through {_upgradeTo}. For an externally upgradeable proxy see * {TransparentUpgradeableProxy}. */ -contract ERC1967Proxy is Proxy, ERC1967Utils { +contract ERC1967Proxy is Proxy, ERC1967Storage { /** * @dev Initializes the upgradeable proxy with an initial implementation specified by `_logic`. * diff --git a/contracts/proxy/ERC1967/ERC1967Utils.sol b/contracts/proxy/ERC1967/ERC1967Storage.sol similarity index 86% rename from contracts/proxy/ERC1967/ERC1967Utils.sol rename to contracts/proxy/ERC1967/ERC1967Storage.sol index c06a303801a..d99f8f1c6f8 100644 --- a/contracts/proxy/ERC1967/ERC1967Utils.sol +++ b/contracts/proxy/ERC1967/ERC1967Storage.sol @@ -6,7 +6,7 @@ import "../beacon/IBeacon.sol"; import "../../utils/Address.sol"; import "../../utils/StorageSlot.sol"; -abstract contract ERC1967ImplementationUtils { +abstract contract ERC1967Storage { /** * @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 @@ -17,15 +17,10 @@ abstract contract ERC1967ImplementationUtils { // This is the keccak-256 hash of "eip1967.proxy.upgradePending" subtracted by 1 bytes32 internal constant _UPGRADE_PENDING_SLOT = 0x39c07022fef61edd40345eccc814df883dce06b1b65a92ff48ae275074d292ee; - /** - * @dev Emitted when the implementation is upgraded. - */ - event Upgraded(address indexed implementation); - /** * @dev Returns the current implementation address. */ - function _getImplementation() internal view virtual returns (address) { + function _getImplementation() internal view returns (address) { return StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value; } @@ -37,17 +32,22 @@ abstract contract ERC1967ImplementationUtils { StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation; } + /** + * @dev Emitted when the implementation is upgraded. + */ + event Upgraded(address indexed implementation); + /** * @dev Perform implementation upgrade (with additional setup call) * * Emits an {Upgraded} event. */ - function _upgradeToAndCall(address newImplementation, bytes memory data) internal virtual { + function _upgradeToAndCall(address newImplementation, bytes memory data) internal { _setImplementation(newImplementation); + emit Upgraded(newImplementation); if (data.length > 0) { Address.functionDelegateCall(newImplementation, data); } - emit Upgraded(newImplementation); } /** @@ -55,7 +55,7 @@ abstract contract ERC1967ImplementationUtils { * * Emits an {Upgraded} event. */ - function _upgradeToAndCallSecure(address newImplementation, bytes memory data) internal virtual { + function _upgradeToAndCallSecure(address newImplementation, bytes memory data) internal { address oldImplementation = _getImplementation(); // check if nested in an upgrade check StorageSlot.BooleanSlot storage upgradePending = StorageSlot.getBooleanSlot(_UPGRADE_PENDING_SLOT); @@ -85,63 +85,35 @@ abstract contract ERC1967ImplementationUtils { emit Upgraded(newImplementation); } } -} - -abstract contract ERC1967AdminUtils { - /** - * @dev Storage slot with the admin of the contract. - * 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; /** - * @dev Emitted when the admin account has changed. - */ - event AdminChanged(address previousAdmin, address newAdmin); - - /** - * @dev Returns the current admin. - */ - function _getAdmin() internal view virtual returns (address) { - return StorageSlot.getAddressSlot(_ADMIN_SLOT).value; - } - - /** - * @dev Stores a new address in the EIP1967 admin slot. + * @dev Emitted when the beacon is upgraded. */ - function _setAdmin(address newAdmin) private { - require(newAdmin != address(0), "ERC1967: new admin is the zero address"); - StorageSlot.getAddressSlot(_ADMIN_SLOT).value = newAdmin; - } + event BeaconUpgraded(address indexed beacon); /** - * @dev Changes the admin of the proxy. + * @dev Perform implementation upgrade (with addition delegate call) * - * Emits an {AdminChanged} event. + * Emits an {Upgraded} event. */ - function _changeAdmin(address newAdmin) internal { - emit AdminChanged(_getAdmin(), newAdmin); - _setAdmin(newAdmin); + function _upgradeBeaconToAndCall(address newBeacon, bytes memory data) internal { + _setBeacon(newBeacon); + emit BeaconUpgraded(newBeacon); + if (data.length > 0) { + Address.functionDelegateCall(IBeacon(newBeacon).implementation(), data); + } } -} -abstract contract ERC1967BeaconUtils { /** * @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 internal constant _BEACON_SLOT = 0xa3f0ad74e5423aebfd80d3ef4346578335a9a72aeaee59ff6cb3582b35133d50; - /** - * @dev Emitted when the beacon is upgraded. - */ - event BeaconUpgraded(address indexed beacon); - /** * @dev Returns the current beacon. */ - function _getBeacon() internal view virtual returns (address) { + function _getBeacon() internal view returns (address) { return StorageSlot.getAddressSlot(_BEACON_SLOT).value; } @@ -161,28 +133,39 @@ abstract contract ERC1967BeaconUtils { } /** - * @dev Perform implementation upgrade - * - * Emits an {Upgraded} event. + * @dev Storage slot with the admin of the contract. + * This is the keccak-256 hash of "eip1967.proxy.admin" subtracted by 1, and is + * validated in the constructor. */ - function _upgradeBeaconTo(address newBeacon) internal virtual { - _upgradeBeaconToAndCall(newBeacon, bytes("")); + bytes32 internal constant _ADMIN_SLOT = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103; + + /** + * @dev Returns the current admin. + */ + function _getAdmin() internal view returns (address) { + return StorageSlot.getAddressSlot(_ADMIN_SLOT).value; } /** - * @dev Perform implementation upgrade (with addition delegate call) + * @dev Stores a new address in the EIP1967 admin slot. + */ + function _setAdmin(address newAdmin) private { + require(newAdmin != address(0), "ERC1967: new admin is the zero address"); + StorageSlot.getAddressSlot(_ADMIN_SLOT).value = newAdmin; + } + + /** + * @dev Emitted when the admin account has changed. + */ + event AdminChanged(address previousAdmin, address newAdmin); + + /** + * @dev Changes the admin of the proxy. * - * Emits an {Upgraded} event. + * Emits an {AdminChanged} event. */ - function _upgradeBeaconToAndCall(address newBeacon, bytes memory data) internal virtual { - // TODO: additional security checks ? - _setBeacon(newBeacon); - // TODO: change event to match AdminChanged ? - emit BeaconUpgraded(newBeacon); - if (data.length > 0) { - Address.functionDelegateCall(IBeacon(newBeacon).implementation(), data); - } + function _changeAdmin(address newAdmin) internal { + emit AdminChanged(_getAdmin(), newAdmin); + _setAdmin(newAdmin); } } - -abstract contract ERC1967Utils is ERC1967ImplementationUtils, ERC1967AdminUtils, ERC1967BeaconUtils {} diff --git a/contracts/proxy/ERC1967/ERC1967Upgrade.sol b/contracts/proxy/ERC1967/ERC1967Upgrade.sol index a061dad5bc9..01efb7519f1 100644 --- a/contracts/proxy/ERC1967/ERC1967Upgrade.sol +++ b/contracts/proxy/ERC1967/ERC1967Upgrade.sol @@ -2,9 +2,9 @@ pragma solidity ^0.8.0; -import "./ERC1967Utils.sol"; +import "./ERC1967Storage.sol"; -abstract contract ERC1967Upgrade is ERC1967Utils { +abstract contract ERC1967Upgrade is ERC1967Storage { /** * @dev Upgrade the implementation of the proxy. */ diff --git a/contracts/proxy/ERC1967/ERC1967UpgradeSecure.sol b/contracts/proxy/ERC1967/ERC1967UpgradeSecure.sol index 3d558976d9d..c3f0610ed7a 100644 --- a/contracts/proxy/ERC1967/ERC1967UpgradeSecure.sol +++ b/contracts/proxy/ERC1967/ERC1967UpgradeSecure.sol @@ -12,6 +12,6 @@ abstract contract ERC1967UpgradeSecure is ERC1967Upgrade { */ function upgradeToAndCall(address newImplementation, bytes memory data) public payable virtual override { beforeUpgrade(newImplementation); - _upgradeToAndCallSecure(newImplementation, data); + ERC1967Storage._upgradeToAndCallSecure(newImplementation, data); } } diff --git a/contracts/proxy/beacon/BeaconProxy.sol b/contracts/proxy/beacon/BeaconProxy.sol index 3017f31d525..fef670864a3 100644 --- a/contracts/proxy/beacon/BeaconProxy.sol +++ b/contracts/proxy/beacon/BeaconProxy.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.0; import "./IBeacon.sol"; import "../Proxy.sol"; -import "../ERC1967/ERC1967Utils.sol"; +import "../ERC1967/ERC1967Storage.sol"; /** * @dev This contract implements a proxy that gets the implementation address for each call from a {UpgradeableBeacon}. @@ -14,7 +14,7 @@ import "../ERC1967/ERC1967Utils.sol"; * * _Available since v3.4._ */ -contract BeaconProxy is Proxy, ERC1967Utils { +contract BeaconProxy is Proxy, ERC1967Storage { /** * @dev Initializes the proxy with `beacon`. * diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol index c36cd7c5c43..a53fdfde83d 100644 --- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol +++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol @@ -70,14 +70,7 @@ contract TransparentUpgradeableProxy is ERC1967Proxy, ERC1967Upgrade { * `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc` */ function implementation() external ifAdmin returns (address implementation_) { - implementation_ = _getImplementation(); - } - - /** - * @dev Returns the current implementation address. - */ - function _implementation() internal view virtual override returns (address) { - return _getImplementation(); + return _implementation(); } /**