Make TransparentUpgradeableProxy deploy its ProxyAdmin and optimize proxy interfaces (#4382)

This commit is contained in:
Francisco Giordano
2023-07-09 16:54:32 -03:00
parent e47b53bce4
commit 8fff875589
24 changed files with 300 additions and 308 deletions

View File

@ -9,7 +9,7 @@ pragma solidity ^0.8.19;
contract ClashingImplementation {
event ClashingImplementationCall();
function upgradeTo(address) external payable {
function upgradeToAndCall(address, bytes calldata) external payable {
emit ClashingImplementationCall();
}

View File

@ -23,12 +23,8 @@ contract UUPSUpgradeableMock is NonUpgradeableMock, UUPSUpgradeable {
}
contract UUPSUpgradeableUnsafeMock is UUPSUpgradeableMock {
function upgradeTo(address newImplementation) public override {
ERC1967Utils.upgradeToAndCall(newImplementation, bytes(""), false);
}
function upgradeToAndCall(address newImplementation, bytes memory data) public payable override {
ERC1967Utils.upgradeToAndCall(newImplementation, data, false);
ERC1967Utils.upgradeToAndCall(newImplementation, data);
}
}

View File

@ -20,7 +20,7 @@ contract ERC1967Proxy is Proxy {
* function call, and allows initializing the storage of the proxy like a Solidity constructor.
*/
constructor(address _logic, bytes memory _data) payable {
ERC1967Utils.upgradeToAndCall(_logic, _data, false);
ERC1967Utils.upgradeToAndCall(_logic, _data);
}
/**

View File

@ -52,6 +52,11 @@ library ERC1967Utils {
*/
error ERC1967InvalidBeacon(address beacon);
/**
* @dev An upgrade function sees `msg.value > 0` that may be lost.
*/
error ERC1967NonPayable();
/**
* @dev Returns the current implementation address.
*/
@ -69,25 +74,19 @@ library ERC1967Utils {
StorageSlot.getAddressSlot(IMPLEMENTATION_SLOT).value = newImplementation;
}
/**
* @dev Perform implementation upgrade
*
* Emits an {IERC1967-Upgraded} event.
*/
function upgradeTo(address newImplementation) internal {
_setImplementation(newImplementation);
emit Upgraded(newImplementation);
}
/**
* @dev Perform implementation upgrade with additional setup call.
*
* Emits an {IERC1967-Upgraded} event.
*/
function upgradeToAndCall(address newImplementation, bytes memory data, bool forceCall) internal {
upgradeTo(newImplementation);
if (data.length > 0 || forceCall) {
function upgradeToAndCall(address newImplementation, bytes memory data) internal {
_setImplementation(newImplementation);
emit Upgraded(newImplementation);
if (data.length > 0) {
Address.functionDelegateCall(newImplementation, data);
} else {
_checkNonPayable();
}
}
@ -161,7 +160,7 @@ library ERC1967Utils {
}
/**
* @dev Change the beacon and trigger a setup call.
* @dev Change the beacon and trigger a setup call if data is nonempty.
*
* Emits an {IERC1967-BeaconUpgraded} event.
*
@ -169,11 +168,20 @@ library ERC1967Utils {
* it uses an immutable beacon without looking at the value of the ERC-1967 beacon slot for
* efficiency.
*/
function upgradeBeaconToAndCall(address newBeacon, bytes memory data, bool forceCall) internal {
function upgradeBeaconToAndCall(address newBeacon, bytes memory data) internal {
_setBeacon(newBeacon);
emit BeaconUpgraded(newBeacon);
if (data.length > 0 || forceCall) {
if (data.length > 0) {
Address.functionDelegateCall(IBeacon(newBeacon).implementation(), data);
} else {
_checkNonPayable();
}
}
function _checkNonPayable() private {
if (msg.value > 0) {
revert ERC1967NonPayable();
}
}
}

View File

@ -36,7 +36,7 @@ contract BeaconProxy is Proxy {
* - `beacon` must be a contract with the interface {IBeacon}.
*/
constructor(address beacon, bytes memory data) payable {
ERC1967Utils.upgradeBeaconToAndCall(beacon, data, false);
ERC1967Utils.upgradeBeaconToAndCall(beacon, data);
_beacon = beacon;
}

View File

@ -3,7 +3,7 @@
pragma solidity ^0.8.19;
import {ITransparentUpgradeableProxy, TransparentUpgradeableProxy} from "./TransparentUpgradeableProxy.sol";
import {ITransparentUpgradeableProxy} from "./TransparentUpgradeableProxy.sol";
import {Ownable} from "../../access/Ownable.sol";
/**
@ -11,25 +11,25 @@ import {Ownable} from "../../access/Ownable.sol";
* explanation of why you would want to use this see the documentation for {TransparentUpgradeableProxy}.
*/
contract ProxyAdmin is Ownable {
/**
* @dev The version of the upgrade interface of the contract. If this getter is missing, both `upgrade(address)`
* and `upgradeAndCall(address,bytes)` are present, and `upgradeTo` must be used if no function should be called,
* while `upgradeAndCall` will invoke the `receive` function if the second argument is the empty byte string.
* If the getter returns `"5.0.0"`, only `upgradeAndCall(address,bytes)` is present, and the second argument must
* be the empty byte string if no function should be called, being impossible to invoke the `receive` function
* during an upgrade.
*/
// solhint-disable-next-line private-vars-leading-underscore
string internal constant UPGRADE_INTERFACE_VERSION = "5.0.0";
/**
* @dev Sets the initial owner who can perform upgrades.
*/
constructor(address initialOwner) Ownable(initialOwner) {}
/**
* @dev Upgrades `proxy` to `implementation`. See {TransparentUpgradeableProxy-upgradeTo}.
*
* Requirements:
*
* - This contract must be the admin of `proxy`.
*/
function upgrade(ITransparentUpgradeableProxy proxy, address implementation) public virtual onlyOwner {
proxy.upgradeTo(implementation);
}
/**
* @dev Upgrades `proxy` to `implementation` and calls a function on the new implementation. See
* {TransparentUpgradeableProxy-upgradeToAndCall}.
* @dev Upgrades `proxy` to `implementation` and calls a function on the new implementation.
* See {TransparentUpgradeableProxy-_dispatchUpgradeToAndCall}.
*
* Requirements:
*

View File

@ -6,21 +6,20 @@ pragma solidity ^0.8.19;
import {ERC1967Utils} from "../ERC1967/ERC1967Utils.sol";
import {ERC1967Proxy} from "../ERC1967/ERC1967Proxy.sol";
import {IERC1967} from "../../interfaces/IERC1967.sol";
import {ProxyAdmin} from "./ProxyAdmin.sol";
/**
* @dev Interface for {TransparentUpgradeableProxy}. In order to implement transparency, {TransparentUpgradeableProxy}
* does not implement this interface directly, and some of its functions are implemented by an internal dispatch
* does not implement this interface directly, and its upgradeability mechanism is implemented by an internal dispatch
* mechanism. The compiler is unaware that these functions are implemented by {TransparentUpgradeableProxy} and will not
* include them in the ABI so this interface must be used to interact with it.
*/
interface ITransparentUpgradeableProxy is IERC1967 {
function upgradeTo(address) external;
function upgradeToAndCall(address, bytes memory) external payable;
function upgradeToAndCall(address, bytes calldata) external payable;
}
/**
* @dev This contract implements a proxy that is upgradeable by an immutable admin.
* @dev This contract implements a proxy that is upgradeable through an associated {ProxyAdmin} instance.
*
* To avoid https://medium.com/nomic-labs-blog/malicious-backdoors-in-ethereum-proxies-62629adf3357[proxy selector
* clashing], which can potentially be used in an attack, this contract uses the
@ -28,23 +27,22 @@ interface ITransparentUpgradeableProxy is IERC1967 {
* things that go hand in hand:
*
* 1. If any account other than the admin calls the proxy, the call will be forwarded to the implementation, even if
* that call matches one of the admin functions exposed by the proxy itself.
* 2. If the admin calls the proxy, it can access the admin functions, but its calls will never be forwarded to the
* that call matches the {ITransparentUpgradeableProxy-upgradeToAndCall} function exposed by the proxy itself.
* 2. If the admin calls the proxy, it can call the `upgradeToAndCall` function but any other call won't be forwarded to the
* implementation. If the admin tries to call a function on the implementation it will fail with an error indicating the
* proxy admin cannot fallback to the target implementation.
*
* These properties mean that the admin account can only be used for upgrading the proxy, so it's best if it's a dedicated
* account that is not used for anything else. This will avoid headaches due to sudden errors when trying to call a function
* from the proxy implementation.
*
* 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, which extends from the
* {Ownable} contract to allow for changing the proxy's admin owner.
* from the proxy implementation. For this reason, the proxy deploys an instance of {ProxyAdmin} and allows upgrades
* only if they come through it.
* You should think of the `ProxyAdmin` instance as the administrative interface of the proxy, including the ability to
* change who can trigger upgrades by transferring ownership.
*
* NOTE: The real interface of this proxy is that defined in `ITransparentUpgradeableProxy`. This contract does not
* inherit from that interface, and instead the admin functions are implicitly implemented using a custom dispatch
* mechanism in `_fallback`. Consequently, the compiler will not produce an ABI for this contract. This is necessary to
* fully implement transparency without decoding reverts caused by selector clashes between the proxy and the
* inherit from that interface, and instead `upgradeToAndCall` is implicitly implemented using a custom dispatch mechanism
* in `_fallback`. Consequently, the compiler will not produce an ABI for this contract. This is necessary to fully
* implement transparency without decoding reverts caused by selector clashes between the proxy and the
* implementation.
*
* IMPORTANT: This contract avoids unnecessary storage reads by setting the admin only during construction as an immutable variable,
@ -55,10 +53,10 @@ interface ITransparentUpgradeableProxy is IERC1967 {
* WARNING: It is not recommended to extend this contract to add additional external functions. If you do so, the compiler
* will not check that there are no selector conflicts, due to the note above. A selector clash between any new function
* and the functions declared in {ITransparentUpgradeableProxy} will be resolved in favor of the new one. This could
* render the admin operations inaccessible, which could prevent upgradeability. Transparency may also be compromised.
* render the `upgradeToAndCall` function inaccessible, preventing upgradeability and compromising transparency.
*/
contract TransparentUpgradeableProxy is ERC1967Proxy {
// An immutable address for the admin avoid unnecessary SLOADs before each call
// An immutable address for the admin to avoid unnecessary SLOADs before each call
// at the expense of removing the ability to change the admin once it's set.
// This is acceptable if the admin is always a ProxyAdmin instance or similar contract
// with its own ability to transfer the permissions to another account.
@ -69,19 +67,14 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
*/
error ProxyDeniedAdminAccess();
/**
* @dev msg.value is not 0.
*/
error ProxyNonPayableFunction();
/**
* @dev Initializes an upgradeable proxy managed by `_admin`, backed by the implementation at `_logic`, and
* optionally initialized with `_data` as explained in {ERC1967Proxy-constructor}.
*/
constructor(address _logic, address admin_, bytes memory _data) payable ERC1967Proxy(_logic, _data) {
_admin = admin_;
constructor(address _logic, address initialOwner, bytes memory _data) payable ERC1967Proxy(_logic, _data) {
_admin = address(new ProxyAdmin(initialOwner));
// Set the storage value and emit an event for ERC-1967 compatibility
ERC1967Utils.changeAdmin(admin_);
ERC1967Utils.changeAdmin(_admin);
}
/**
@ -89,18 +82,11 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
*/
function _fallback() internal virtual override {
if (msg.sender == _admin) {
bytes memory ret;
bytes4 selector = msg.sig;
if (selector == ITransparentUpgradeableProxy.upgradeTo.selector) {
ret = _dispatchUpgradeTo();
} else if (selector == ITransparentUpgradeableProxy.upgradeToAndCall.selector) {
ret = _dispatchUpgradeToAndCall();
if (msg.sig == ITransparentUpgradeableProxy.upgradeToAndCall.selector) {
_dispatchUpgradeToAndCall();
} else {
revert ProxyDeniedAdminAccess();
}
assembly {
return(add(ret, 0x20), mload(ret))
}
} else {
super._fallback();
}
@ -109,34 +95,8 @@ contract TransparentUpgradeableProxy is ERC1967Proxy {
/**
* @dev Upgrade the implementation of the proxy.
*/
function _dispatchUpgradeTo() private returns (bytes memory) {
_requireZeroValue();
address newImplementation = abi.decode(msg.data[4:], (address));
ERC1967Utils.upgradeToAndCall(newImplementation, bytes(""), false);
return "";
}
/**
* @dev Upgrade the implementation of the proxy, and then call a function from the new implementation as specified
* by `data`, which should be an encoded function call. This is useful to initialize new storage variables in the
* proxied contract.
*/
function _dispatchUpgradeToAndCall() private returns (bytes memory) {
function _dispatchUpgradeToAndCall() private {
(address newImplementation, bytes memory data) = abi.decode(msg.data[4:], (address, bytes));
ERC1967Utils.upgradeToAndCall(newImplementation, data, true);
return "";
}
/**
* @dev To keep this contract fully transparent, the fallback is payable. This helper is here to enforce
* non-payability of function implemented through dispatchers while still allowing value to pass through.
*/
function _requireZeroValue() private {
if (msg.value != 0) {
revert ProxyNonPayableFunction();
}
ERC1967Utils.upgradeToAndCall(newImplementation, data);
}
}

View File

@ -20,6 +20,17 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
/// @custom:oz-upgrades-unsafe-allow state-variable-immutable state-variable-assignment
address private immutable __self = address(this);
/**
* @dev The version of the upgrade interface of the contract. If this getter is missing, both `upgradeTo(address)`
* and `upgradeToAndCall(address,bytes)` are present, and `upgradeTo` must be used if no function should be called,
* while `upgradeToAndCall` will invoke the `receive` function if the second argument is the empty byte string.
* If the getter returns `"5.0.0"`, only `upgradeToAndCall(address,bytes)` is present, and the second argument must
* be the empty byte string if no function should be called, being impossible to invoke the `receive` function
* during an upgrade.
*/
// solhint-disable-next-line private-vars-leading-underscore
string internal constant UPGRADE_INTERFACE_VERSION = "5.0.0";
/**
* @dev The call is from an unauthorized context.
*/
@ -71,20 +82,6 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
return ERC1967Utils.IMPLEMENTATION_SLOT;
}
/**
* @dev Upgrade the implementation of the proxy to `newImplementation`.
*
* Calls {_authorizeUpgrade}.
*
* Emits an {Upgraded} event.
*
* @custom:oz-upgrades-unsafe-allow-reachable delegatecall
*/
function upgradeTo(address newImplementation) public virtual onlyProxy {
_authorizeUpgrade(newImplementation);
_upgradeToAndCallUUPS(newImplementation, new bytes(0), false);
}
/**
* @dev Upgrade the implementation of the proxy to `newImplementation`, and subsequently execute the function call
* encoded in `data`.
@ -97,17 +94,17 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
*/
function upgradeToAndCall(address newImplementation, bytes memory data) public payable virtual onlyProxy {
_authorizeUpgrade(newImplementation);
_upgradeToAndCallUUPS(newImplementation, data, true);
_upgradeToAndCallUUPS(newImplementation, data);
}
/**
* @dev Function that should revert when `msg.sender` is not authorized to upgrade the contract. Called by
* {upgradeTo} and {upgradeToAndCall}.
* {upgradeToAndCall}.
*
* Normally, this function will use an xref:access.adoc[access control] modifier such as {Ownable-onlyOwner}.
*
* ```solidity
* function _authorizeUpgrade(address) internal onlyOwner {}
* function _authorizeUpgrade(address) internal onlyOwner {}
* ```
*/
function _authorizeUpgrade(address newImplementation) internal virtual;
@ -117,12 +114,12 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable {
*
* Emits an {IERC1967-Upgraded} event.
*/
function _upgradeToAndCallUUPS(address newImplementation, bytes memory data, bool forceCall) private {
function _upgradeToAndCallUUPS(address newImplementation, bytes memory data) private {
try IERC1822Proxiable(newImplementation).proxiableUUID() returns (bytes32 slot) {
if (slot != ERC1967Utils.IMPLEMENTATION_SLOT) {
revert UUPSUnsupportedProxiableUUID(slot);
}
ERC1967Utils.upgradeToAndCall(newImplementation, data, forceCall);
ERC1967Utils.upgradeToAndCall(newImplementation, data);
} catch {
// The implementation is not UUPS
revert ERC1967Utils.ERC1967InvalidImplementation(newImplementation);