Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use immutable beacon address in BeaconProxy #4435

Merged
merged 11 commits into from
Jul 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/proud-seals-complain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

`BeaconProxy`: Use an immutable variable to store the address of the beacon. It is no longer possible for a `BeaconProxy` to upgrade by changing to another beacon.
7 changes: 5 additions & 2 deletions contracts/proxy/ERC1967/ERC1967Utils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,13 @@ library ERC1967Utils {
}

/**
* @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).
* @dev Change the beacon and trigger a setup call.
*
* Emits an {IERC1967-BeaconUpgraded} event.
*
* CAUTION: Invoking this function has no effect on an instance of {BeaconProxy} since v5, since
* 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 {
_setBeacon(newBeacon);
Expand Down
16 changes: 15 additions & 1 deletion contracts/proxy/beacon/BeaconProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ import {ERC1967Utils} from "../ERC1967/ERC1967Utils.sol";
*
* The beacon address is stored in storage slot `uint256(keccak256('eip1967.proxy.beacon')) - 1`, so that it doesn't
* conflict with the storage layout of the implementation behind the proxy.
*
* CAUTION: The beacon address can only be set once during construction, and cannot be changed afterwards.
* You must ensure that you either control the beacon, or trust the beacon to not upgrade the implementation maliciously.
*/
contract BeaconProxy is Proxy {
// An immutable address for the beacon to avoid unnecessary SLOADs before each delegate call.
address private immutable _beacon;

/**
* @dev Initializes the proxy with `beacon`.
*
Expand All @@ -27,12 +33,20 @@ contract BeaconProxy is Proxy {
*/
constructor(address beacon, bytes memory data) payable {
ERC1967Utils.upgradeBeaconToAndCall(beacon, data, false);
_beacon = beacon;
}

/**
* @dev Returns the current implementation address of the associated beacon.
*/
function _implementation() internal view virtual override returns (address) {
return IBeacon(ERC1967Utils.getBeacon()).implementation();
return IBeacon(_getBeacon()).implementation();
}

/**
* @dev Returns the beacon.
*/
function _getBeacon() internal view virtual returns (address) {
return _beacon;
}
}