Skip to content

Commit

Permalink
Use immutable beacon address in BeaconProxy (#4435)
Browse files Browse the repository at this point in the history
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco Giordano <fg@frang.io>
  • Loading branch information
3 people committed Jul 8, 2023
1 parent 6d74b91 commit 5229b75
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
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;
}
}

0 comments on commit 5229b75

Please sign in to comment.