From 88a48e3e74167d4a3926fd5a04b1ea095347c59e Mon Sep 17 00:00:00 2001 From: Andrei Kozlov Date: Thu, 25 May 2023 13:59:54 +0300 Subject: [PATCH 1/5] simple ImmutableBeaconProxy implementation --- .../ImmutableBeaconProxy.sol | 36 ++++++++++++++++ .../transparent-proxy/interfaces/IBeacon.sol | 16 +++++++ test/ImmutableBeaconProxy.t.sol | 43 +++++++++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 src/contracts/transparent-proxy/ImmutableBeaconProxy.sol create mode 100644 src/contracts/transparent-proxy/interfaces/IBeacon.sol create mode 100644 test/ImmutableBeaconProxy.t.sol diff --git a/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol b/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol new file mode 100644 index 0000000..454dbba --- /dev/null +++ b/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {IBeacon} from './interfaces/IBeacon.sol'; +import {Proxy} from './Proxy.sol'; +import {Address} from '../oz-common/Address.sol'; + +/** + * @dev This contract implements a proxy that gets the implementation address for each call from an {UpgradeableBeacon}. + * The beacon address is immutable. The purpose of it, is to be able to access this proxy via delegatecall + + * !!! IMPORTANT CONSIDERATION !!! + * We expect that that implementation will not have any storage associated, + * because it will not be accessible via delegatecall, and will not work as expected + */ +contract ImmutableBeaconProxy is Proxy { + address internal immutable _beacon; + + constructor(address beacon) { + require(Address.isContract(beacon), 'beacon is not a contract'); + require( + Address.isContract(IBeacon(beacon).implementation()), + 'beacon implementation is not a contract' + ); + + // there is no initialization call, because we expect that implementation should have no storage + _beacon = beacon; + } + + /** + * @dev Returns the current implementation address of the associated beacon. + */ + function _implementation() internal view virtual override returns (address) { + return IBeacon(_beacon).implementation(); + } +} diff --git a/src/contracts/transparent-proxy/interfaces/IBeacon.sol b/src/contracts/transparent-proxy/interfaces/IBeacon.sol new file mode 100644 index 0000000..b29ed70 --- /dev/null +++ b/src/contracts/transparent-proxy/interfaces/IBeacon.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.1 (proxy/beacon/IBeacon.sol) + +pragma solidity ^0.8.0; + +/** + * @dev This is the interface that {BeaconProxy} expects of its beacon. + */ +interface IBeacon { + /** + * @dev Must return an address that can be used as a delegate call target. + * + * {BeaconProxy} will check that this address is a contract. + */ + function implementation() external view returns (address); +} diff --git a/test/ImmutableBeaconProxy.t.sol b/test/ImmutableBeaconProxy.t.sol new file mode 100644 index 0000000..95f6e6b --- /dev/null +++ b/test/ImmutableBeaconProxy.t.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; +import 'forge-std/Test.sol'; +import {ImmutableBeaconProxy, IBeacon} from '../src/contracts/transparent-proxy/ImmutableBeaconProxy.sol'; + +contract ImplementationMock {} + +contract BeaconMock is IBeacon { + address public implementation; + + constructor(address newImplementation) { + implementation = newImplementation; + } +} + +contract ImmutableBeaconProxyMock is ImmutableBeaconProxy { + constructor(address beacon) ImmutableBeaconProxy(beacon) {} + + function implementation() public view returns (address) { + return _implementation(); + } +} + +contract ImmutableBeaconProxyTest is Test { + function testResolvesImplementationCorrectly() public { + address implementation = address(new ImplementationMock()); + address beacon = address(new BeaconMock(implementation)); + + assertEq(implementation, (new ImmutableBeaconProxyMock(beacon)).implementation()); + } + + function testBeaconNotAContract() public { + vm.expectRevert(bytes('beacon is not a contract')); + new ImmutableBeaconProxy(address(1)); + } + + function testImplementationNotAContract() public { + address beacon = address(new BeaconMock(address(1))); + + vm.expectRevert(bytes('beacon implementation is not a contract')); + new ImmutableBeaconProxy(beacon); + } +} From e5c63399ed10c9a703028577048aa678e01c8fe4 Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 25 May 2023 16:09:46 +0200 Subject: [PATCH 2/5] Update src/contracts/transparent-proxy/ImmutableBeaconProxy.sol Co-authored-by: Ernesto Boado --- src/contracts/transparent-proxy/ImmutableBeaconProxy.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol b/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol index 454dbba..946f75d 100644 --- a/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol +++ b/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol @@ -10,8 +10,8 @@ import {Address} from '../oz-common/Address.sol'; * The beacon address is immutable. The purpose of it, is to be able to access this proxy via delegatecall * !!! IMPORTANT CONSIDERATION !!! - * We expect that that implementation will not have any storage associated, - * because it will not be accessible via delegatecall, and will not work as expected + * We expect that the implementation will not have any storage associated, + * because it when accessed via delegatecall, will not work as expected creating dangerous side-effects. Preferable, the implementation should be declared as a library */ contract ImmutableBeaconProxy is Proxy { address internal immutable _beacon; From 895d4f1a7261ff2556574daa78c1b65fe670e0f7 Mon Sep 17 00:00:00 2001 From: Andrei Kozlov Date: Thu, 25 May 2023 17:24:38 +0300 Subject: [PATCH 3/5] update error messages --- src/contracts/transparent-proxy/ImmutableBeaconProxy.sol | 7 ++----- test/ImmutableBeaconProxy.t.sol | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol b/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol index 946f75d..3c2f157 100644 --- a/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol +++ b/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol @@ -17,11 +17,8 @@ contract ImmutableBeaconProxy is Proxy { address internal immutable _beacon; constructor(address beacon) { - require(Address.isContract(beacon), 'beacon is not a contract'); - require( - Address.isContract(IBeacon(beacon).implementation()), - 'beacon implementation is not a contract' - ); + require(Address.isContract(beacon), 'INVALID_BEACON'); + require(Address.isContract(IBeacon(beacon).implementation()), 'INVALID_IMPLEMENTATION'); // there is no initialization call, because we expect that implementation should have no storage _beacon = beacon; diff --git a/test/ImmutableBeaconProxy.t.sol b/test/ImmutableBeaconProxy.t.sol index 95f6e6b..acc6eaf 100644 --- a/test/ImmutableBeaconProxy.t.sol +++ b/test/ImmutableBeaconProxy.t.sol @@ -30,14 +30,14 @@ contract ImmutableBeaconProxyTest is Test { } function testBeaconNotAContract() public { - vm.expectRevert(bytes('beacon is not a contract')); + vm.expectRevert(bytes('INVALID_BEACON')); new ImmutableBeaconProxy(address(1)); } function testImplementationNotAContract() public { address beacon = address(new BeaconMock(address(1))); - vm.expectRevert(bytes('beacon implementation is not a contract')); + vm.expectRevert(bytes('INVALID_IMPLEMENTATION')); new ImmutableBeaconProxy(beacon); } } From e343ae3e48264323fa61649395ac1b2f1256e01f Mon Sep 17 00:00:00 2001 From: Andrei Kozlov Date: Fri, 26 May 2023 15:01:11 +0300 Subject: [PATCH 4/5] event added to ImmutableBeaconProfy, small refactoring --- .../ImmutableBeaconProxy.sol | 5 +- .../UpgradeableBeacon.sol | 68 +++++++++++++++++++ .../interfaces/IBeacon.sol | 0 .../Proxy.sol | 0 .../transparent-proxy/ERC1967Proxy.sol | 2 +- test/ImmutableBeaconProxy.t.sol | 6 +- 6 files changed, 78 insertions(+), 3 deletions(-) rename src/contracts/{transparent-proxy => immutable-beacon-proxy}/ImmutableBeaconProxy.sol (90%) create mode 100644 src/contracts/immutable-beacon-proxy/UpgradeableBeacon.sol rename src/contracts/{transparent-proxy => immutable-beacon-proxy}/interfaces/IBeacon.sol (100%) rename src/contracts/{transparent-proxy => oz-common}/Proxy.sol (100%) diff --git a/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol b/src/contracts/immutable-beacon-proxy/ImmutableBeaconProxy.sol similarity index 90% rename from src/contracts/transparent-proxy/ImmutableBeaconProxy.sol rename to src/contracts/immutable-beacon-proxy/ImmutableBeaconProxy.sol index 3c2f157..0e701ec 100644 --- a/src/contracts/transparent-proxy/ImmutableBeaconProxy.sol +++ b/src/contracts/immutable-beacon-proxy/ImmutableBeaconProxy.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; import {IBeacon} from './interfaces/IBeacon.sol'; -import {Proxy} from './Proxy.sol'; +import {Proxy} from '../oz-common/Proxy.sol'; import {Address} from '../oz-common/Address.sol'; /** @@ -16,12 +16,15 @@ import {Address} from '../oz-common/Address.sol'; contract ImmutableBeaconProxy is Proxy { address internal immutable _beacon; + event ImmutableBeaconSet(address indexed beacon); + constructor(address beacon) { require(Address.isContract(beacon), 'INVALID_BEACON'); require(Address.isContract(IBeacon(beacon).implementation()), 'INVALID_IMPLEMENTATION'); // there is no initialization call, because we expect that implementation should have no storage _beacon = beacon; + emit ImmutableBeaconSet(beacon); } /** diff --git a/src/contracts/immutable-beacon-proxy/UpgradeableBeacon.sol b/src/contracts/immutable-beacon-proxy/UpgradeableBeacon.sol new file mode 100644 index 0000000..da67f47 --- /dev/null +++ b/src/contracts/immutable-beacon-proxy/UpgradeableBeacon.sol @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.4.1 (proxy/beacon/UpgradeableBeacon.sol) + +pragma solidity ^0.8.0; + +import './interfaces/IBeacon.sol'; +import {Ownable} from '../oz-common/Ownable.sol'; +import {Address} from '../oz-common/Address.sol'; + +/** + * @dev This contract is used in conjunction with one or more instances of {BeaconProxy} to determine their + * implementation contract, which is where they will delegate all function calls. + * + * An owner is able to change the implementation the beacon points to, thus upgrading the proxies that use this beacon. + */ +contract UpgradeableBeacon is IBeacon, Ownable { + address private _implementation; + + /** + * @dev Emitted when the implementation returned by the beacon is changed. + */ + event Upgraded(address indexed implementation); + + /** + * @dev Sets the address of the initial implementation, and the deployer account as the owner who can upgrade the + * beacon. + */ + constructor(address implementation_) { + _setImplementation(implementation_); + } + + /** + * @dev Returns the current implementation address. + */ + function implementation() public view virtual override returns (address) { + return _implementation; + } + + /** + * @dev Upgrades the beacon to a new implementation. + * + * Emits an {Upgraded} event. + * + * Requirements: + * + * - msg.sender must be the owner of the contract. + * - `newImplementation` must be a contract. + */ + function upgradeTo(address newImplementation) public virtual onlyOwner { + _setImplementation(newImplementation); + emit Upgraded(newImplementation); + } + + /** + * @dev Sets the implementation contract address for this beacon + * + * Requirements: + * + * - `newImplementation` must be a contract. + */ + function _setImplementation(address newImplementation) private { + require( + Address.isContract(newImplementation), + 'UpgradeableBeacon: implementation is not a contract' + ); + _implementation = newImplementation; + } +} diff --git a/src/contracts/transparent-proxy/interfaces/IBeacon.sol b/src/contracts/immutable-beacon-proxy/interfaces/IBeacon.sol similarity index 100% rename from src/contracts/transparent-proxy/interfaces/IBeacon.sol rename to src/contracts/immutable-beacon-proxy/interfaces/IBeacon.sol diff --git a/src/contracts/transparent-proxy/Proxy.sol b/src/contracts/oz-common/Proxy.sol similarity index 100% rename from src/contracts/transparent-proxy/Proxy.sol rename to src/contracts/oz-common/Proxy.sol diff --git a/src/contracts/transparent-proxy/ERC1967Proxy.sol b/src/contracts/transparent-proxy/ERC1967Proxy.sol index 409395c..c7f74c5 100644 --- a/src/contracts/transparent-proxy/ERC1967Proxy.sol +++ b/src/contracts/transparent-proxy/ERC1967Proxy.sol @@ -9,7 +9,7 @@ pragma solidity ^0.8.0; -import './Proxy.sol'; +import '../oz-common/Proxy.sol'; import './ERC1967Upgrade.sol'; /** diff --git a/test/ImmutableBeaconProxy.t.sol b/test/ImmutableBeaconProxy.t.sol index acc6eaf..9af6726 100644 --- a/test/ImmutableBeaconProxy.t.sol +++ b/test/ImmutableBeaconProxy.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; import 'forge-std/Test.sol'; -import {ImmutableBeaconProxy, IBeacon} from '../src/contracts/transparent-proxy/ImmutableBeaconProxy.sol'; +import {ImmutableBeaconProxy, IBeacon} from '../src/contracts/immutable-beacon-proxy/ImmutableBeaconProxy.sol'; contract ImplementationMock {} @@ -22,10 +22,14 @@ contract ImmutableBeaconProxyMock is ImmutableBeaconProxy { } contract ImmutableBeaconProxyTest is Test { + event ImmutableBeaconSet(address indexed beacon); + function testResolvesImplementationCorrectly() public { address implementation = address(new ImplementationMock()); address beacon = address(new BeaconMock(implementation)); + vm.expectEmit(true, false, false, true); + emit ImmutableBeaconSet(beacon); assertEq(implementation, (new ImmutableBeaconProxyMock(beacon)).implementation()); } From b1e140229ed52deddcca735b881d91c2cad707be Mon Sep 17 00:00:00 2001 From: Andrei Kozlov Date: Tue, 30 May 2023 15:51:22 +0200 Subject: [PATCH 5/5] typo in IBeacon import --- src/contracts/immutable-beacon-proxy/UpgradeableBeacon.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/contracts/immutable-beacon-proxy/UpgradeableBeacon.sol b/src/contracts/immutable-beacon-proxy/UpgradeableBeacon.sol index da67f47..7766f01 100644 --- a/src/contracts/immutable-beacon-proxy/UpgradeableBeacon.sol +++ b/src/contracts/immutable-beacon-proxy/UpgradeableBeacon.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; -import './interfaces/IBeacon.sol'; +import {IBeacon} from './interfaces/IBeacon.sol'; import {Ownable} from '../oz-common/Ownable.sol'; import {Address} from '../oz-common/Address.sol';