From e7884547bdbd795089f8eceba397c25bf28684ab Mon Sep 17 00:00:00 2001 From: Michael Heuer <20623991+Michael-A-Heuer@users.noreply.github.com> Date: Wed, 31 Jan 2024 10:31:23 +0100 Subject: [PATCH] refactor: `PluginSetup` and deployment helpers (`Proxy`, `CloneFactory`) (#46) * refactor: deployment helpers and plugin setup * refactor: enforce setting the implementation through the setup constructor * chore: maintained changelog * style: run prettier * chore: bump package version * style: run eslint * fix: wrong variable name * style: run solhint * refactor: renaming * docs: improved natspec * refactor: added explicit function returning the implementation * refactor: provide default implementation for the initial build * build: clean typechain * refactor: remove default prepareUpdate implementation for the upgradeable setup * test: setups and deployment helpers * chore: maintained changelog * style: fix linting issues * test: use address constant * test: initialization * docs: typo * refactor: renaming * style: remove trailing underscore * style: improve library order * feat: added event to sdk * build: use sdk 0.0.1-alpha.5 * build: print coverage report in the console * test: use fixtures * fix: linter * refactor: fix names * refactor: reverting prepareUpdate default implementation * style: move fixtures to the end of the files * revert: remove default implementation * style: move fixture to the EOF * Apply suggestions from code review Co-authored-by: Mathias Scherer --------- Co-authored-by: Mathias Scherer --- contracts/.solcover.js | 2 +- contracts/CHANGELOG.md | 8 + contracts/package.json | 6 +- .../src/mocks/plugin/PluginCloneableMock.sol | 11 + .../mocks/plugin/PluginCloneableSetupMock.sol | 59 +++-- .../src/mocks/plugin/PluginSetupMock.sol | 15 +- .../plugin/PluginUUPSUpgradeableMock.sol | 17 +- .../plugin/PluginUUPSUpgradeableSetupMock.sol | 142 ++++++----- contracts/src/plugin/setup/IPluginSetup.sol | 6 +- contracts/src/plugin/setup/PluginSetup.sol | 52 ++-- .../plugin/setup/PluginUpgradeableSetup.sol | 45 ++++ .../src/utils/deployment/CloneFactory.sol | 21 -- contracts/src/utils/deployment/Proxy.sol | 15 -- .../src/utils/deployment/ProxyFactory.sol | 50 ++++ contracts/src/utils/deployment/ProxyLib.sol | 43 ++++ .../test/permission/auth/dao-authorizable.ts | 10 +- contracts/test/plugin/extensions/proposal.ts | 14 +- contracts/test/plugin/plugin-clonable.ts | 129 +++++++--- .../test/plugin/plugin-uups-upgradeable.ts | 236 +++++++++++------- contracts/test/plugin/plugin.ts | 42 +++- contracts/test/plugin/setup/plugin-setup.ts | 166 ++++++++++-- .../test/utils/deployment/clone-factory.ts | 1 - contracts/test/utils/deployment/proxy-lib.ts | 179 +++++++++++++ contracts/yarn.lock | 8 +- sdk/package.json | 2 +- sdk/src/events.ts | 4 + 26 files changed, 942 insertions(+), 341 deletions(-) create mode 100644 contracts/src/plugin/setup/PluginUpgradeableSetup.sol delete mode 100644 contracts/src/utils/deployment/CloneFactory.sol delete mode 100644 contracts/src/utils/deployment/Proxy.sol create mode 100644 contracts/src/utils/deployment/ProxyFactory.sol create mode 100644 contracts/src/utils/deployment/ProxyLib.sol delete mode 100644 contracts/test/utils/deployment/clone-factory.ts create mode 100644 contracts/test/utils/deployment/proxy-lib.ts diff --git a/contracts/.solcover.js b/contracts/.solcover.js index ada7b8d3..9f51a216 100644 --- a/contracts/.solcover.js +++ b/contracts/.solcover.js @@ -1,5 +1,5 @@ module.exports = { - istanbulReporter: ['html', 'lcov'], + istanbulReporter: ['html', 'lcov', 'text'], providerOptions: { privateKey: process.env.PRIVATE_KEY, }, diff --git a/contracts/CHANGELOG.md b/contracts/CHANGELOG.md index 859189ca..a813d4de 100644 --- a/contracts/CHANGELOG.md +++ b/contracts/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Added an `address internal immutable IMPLEMENTATION` variable to `PluginSetup` and `PluginSetupUpgradeable` and its initialization through the respective constructors. + +- Added an abstract `PluginUpgradeableSetup` base contract. + - Copied files from [aragon/osx commit e7ba46](https://github.com/aragon/osx/tree/e7ba46026db96931d3e4a585e8f30c585906e1fc) - interfaces `IDAO`, `IPermissionCondition`, `IPlugin`, `IMembership`, `IProposal`, `IPluginSetup`, `IProtocolVersion`, @@ -16,3 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - contracts `CloneFactory` - libraries `PermissionLib`, `VersionComparisonLib` - free functions `auth`, `Proxy`, `BitMap`, `Ratio`, `UncheckedMath` + +### Changed + +- Replaced `Proxy` and `CloneFactory` by `ProxyLib` and `ProxyFactory`. diff --git a/contracts/package.json b/contracts/package.json index 9a896ea1..c130b1a6 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -2,13 +2,13 @@ "name": "@aragon/osx-commons-contracts", "license": "AGPL-3.0-or-later", "description": "The Aragon OSx contracts package containing common utilities", - "version": "1.4.0-alpha.2", + "version": "1.4.0-alpha.3", "author": { "name": "aragon", "url": "https://github.com/aragon" }, "devDependencies": { - "@aragon/osx-commons-sdk": "0.0.1-alpha.3", + "@aragon/osx-commons-sdk": "0.0.1-alpha.5", "@aragon/osx-ethers-v1.0.0": "npm:@aragon/osx-ethers@1.0.0", "@aragon/osx-ethers-v1.3.0": "npm:@aragon/osx-ethers@1.3.0", "@ethersproject/abi": "^5.7.0", @@ -74,7 +74,7 @@ "lint:ts": "cd .. && yarn run lint:contracts:ts", "test": "hardhat test", "typechain": "cross-env TS_NODE_TRANSPILE_ONLY=true hardhat typechain", - "clean": "rimraf ./artifacts ./cache ./coverage ./types ./coverage.json && yarn typechain", + "clean": "rimraf ./artifacts ./cache ./coverage ./typechain ./types ./coverage.json && yarn typechain", "docgen": "hardhat docgen" } } diff --git a/contracts/src/mocks/plugin/PluginCloneableMock.sol b/contracts/src/mocks/plugin/PluginCloneableMock.sol index a5ecc5e8..71dfd8bf 100644 --- a/contracts/src/mocks/plugin/PluginCloneableMock.sol +++ b/contracts/src/mocks/plugin/PluginCloneableMock.sol @@ -31,3 +31,14 @@ contract PluginCloneableMockBuild2 is PluginCloneable { state2 = 2; } } + +/// @notice A mock cloneable plugin missing an initializer function. +/// @dev DO NOT USE IN PRODUCTION! +contract PluginCloneableMockBad is PluginCloneable { + uint256 public state1; + + function notAnInitializer(IDAO _dao) external { + __PluginCloneable_init(_dao); + state1 = 1; + } +} diff --git a/contracts/src/mocks/plugin/PluginCloneableSetupMock.sol b/contracts/src/mocks/plugin/PluginCloneableSetupMock.sol index f1cb1c7c..fbf41dd3 100644 --- a/contracts/src/mocks/plugin/PluginCloneableSetupMock.sol +++ b/contracts/src/mocks/plugin/PluginCloneableSetupMock.sol @@ -6,6 +6,7 @@ pragma solidity ^0.8.8; import {PermissionLib} from "../../permission/PermissionLib.sol"; import {IPluginSetup} from "../../plugin/setup/IPluginSetup.sol"; import {PluginSetup} from "../../plugin/setup/PluginSetup.sol"; +import {ProxyLib} from "../../utils/deployment/ProxyLib.sol"; import {IDAO} from "../../dao/IDAO.sol"; import {mockPermissions, mockHelpers} from "./PluginSetupMockData.sol"; import {PluginCloneableMockBuild1, PluginCloneableMockBuild2} from "./PluginCloneableMock.sol"; @@ -14,35 +15,34 @@ import {PluginCloneableMockBuild1, PluginCloneableMockBuild2} from "./PluginClon /// v1.1 (Release 1, Build 1) /// @dev DO NOT USE IN PRODUCTION! contract PluginCloneableSetupMockBuild1 is PluginSetup { - address internal pluginBase; + using ProxyLib for address; - constructor() { - pluginBase = address(new PluginCloneableMockBuild1()); - } + uint16 internal constant THIS_BUILD = 1; + + constructor() PluginSetup(address(new PluginCloneableMockBuild1())) {} /// @inheritdoc IPluginSetup function prepareInstallation( address _dao, bytes memory - ) external override returns (address plugin, PreparedSetupData memory preparedSetupData) { + ) external returns (address plugin, PreparedSetupData memory preparedSetupData) { bytes memory initData = abi.encodeCall(PluginCloneableMockBuild1.initialize, (IDAO(_dao))); - plugin = createERC1967Proxy(pluginBase, initData); // TODO createClone(pluginBase, initData); is missing! See task OS-794 and OS-675. - preparedSetupData.helpers = mockHelpers(1); - preparedSetupData.permissions = mockPermissions(0, 1, PermissionLib.Operation.Grant); + plugin = implementation().deployMinimalProxy(initData); + preparedSetupData.helpers = mockHelpers(THIS_BUILD); + preparedSetupData.permissions = mockPermissions( + 0, + THIS_BUILD, + PermissionLib.Operation.Grant + ); } /// @inheritdoc IPluginSetup function prepareUninstallation( address _dao, SetupPayload calldata _payload - ) external pure override returns (PermissionLib.MultiTargetPermission[] memory permissions) { + ) external pure returns (PermissionLib.MultiTargetPermission[] memory permissions) { (_dao, _payload); - permissions = mockPermissions(0, 1, PermissionLib.Operation.Revoke); - } - - /// @inheritdoc IPluginSetup - function implementation() external view override returns (address) { - return address(pluginBase); + permissions = mockPermissions(0, THIS_BUILD, PermissionLib.Operation.Revoke); } } @@ -50,34 +50,33 @@ contract PluginCloneableSetupMockBuild1 is PluginSetup { /// v1.2 (Release 1, Build 2) /// @dev DO NOT USE IN PRODUCTION! contract PluginCloneableSetupMockBuild2 is PluginSetup { - address internal pluginBase; + using ProxyLib for address; - constructor() { - pluginBase = address(new PluginCloneableMockBuild2()); - } + uint16 internal constant THIS_BUILD = 2; + + constructor() PluginSetup(address(new PluginCloneableMockBuild2())) {} /// @inheritdoc IPluginSetup function prepareInstallation( address _dao, bytes memory - ) external override returns (address plugin, PreparedSetupData memory preparedSetupData) { + ) external returns (address plugin, PreparedSetupData memory preparedSetupData) { bytes memory initData = abi.encodeCall(PluginCloneableMockBuild2.initialize, (IDAO(_dao))); - plugin = createERC1967Proxy(pluginBase, initData); // TODO createClone(pluginBase, initData); is missing! See task OS-794 and OS-675. - preparedSetupData.helpers = mockHelpers(2); - preparedSetupData.permissions = mockPermissions(0, 2, PermissionLib.Operation.Grant); + plugin = implementation().deployMinimalProxy(initData); + preparedSetupData.helpers = mockHelpers(THIS_BUILD); + preparedSetupData.permissions = mockPermissions( + 0, + THIS_BUILD, + PermissionLib.Operation.Grant + ); } /// @inheritdoc IPluginSetup function prepareUninstallation( address _dao, SetupPayload calldata _payload - ) external pure override returns (PermissionLib.MultiTargetPermission[] memory permissions) { + ) external pure returns (PermissionLib.MultiTargetPermission[] memory permissions) { (_dao, _payload); - permissions = mockPermissions(0, 2, PermissionLib.Operation.Revoke); - } - - /// @inheritdoc IPluginSetup - function implementation() external view override returns (address) { - return address(pluginBase); + permissions = mockPermissions(0, THIS_BUILD, PermissionLib.Operation.Revoke); } } diff --git a/contracts/src/mocks/plugin/PluginSetupMock.sol b/contracts/src/mocks/plugin/PluginSetupMock.sol index 0941f542..8af52dd6 100644 --- a/contracts/src/mocks/plugin/PluginSetupMock.sol +++ b/contracts/src/mocks/plugin/PluginSetupMock.sol @@ -14,17 +14,13 @@ import {PluginMockBuild1} from "./PluginMock.sol"; /// v1.1 (Release 1, Build 1) /// @dev DO NOT USE IN PRODUCTION! contract PluginSetupMockBuild1 is PluginSetup { - address internal pluginBase; - - constructor() { - pluginBase = address(new PluginMockBuild1(IDAO(address(0)))); - } + constructor() PluginSetup(address(new PluginMockBuild1(IDAO(address(0))))) {} /// @inheritdoc IPluginSetup function prepareInstallation( address _dao, bytes memory - ) external override returns (address plugin, PreparedSetupData memory preparedSetupData) { + ) external returns (address plugin, PreparedSetupData memory preparedSetupData) { plugin = address(new PluginMockBuild1(IDAO(_dao))); preparedSetupData.helpers = mockHelpers(1); preparedSetupData.permissions = mockPermissions(0, 1, PermissionLib.Operation.Grant); @@ -34,13 +30,8 @@ contract PluginSetupMockBuild1 is PluginSetup { function prepareUninstallation( address _dao, SetupPayload calldata _payload - ) external pure override returns (PermissionLib.MultiTargetPermission[] memory permissions) { + ) external pure returns (PermissionLib.MultiTargetPermission[] memory permissions) { (_dao, _payload); permissions = mockPermissions(0, 1, PermissionLib.Operation.Revoke); } - - /// @inheritdoc IPluginSetup - function implementation() external view override returns (address) { - return address(pluginBase); - } } diff --git a/contracts/src/mocks/plugin/PluginUUPSUpgradeableMock.sol b/contracts/src/mocks/plugin/PluginUUPSUpgradeableMock.sol index c486e59a..c04cebf4 100644 --- a/contracts/src/mocks/plugin/PluginUUPSUpgradeableMock.sol +++ b/contracts/src/mocks/plugin/PluginUUPSUpgradeableMock.sol @@ -6,7 +6,7 @@ pragma solidity ^0.8.8; import {PluginUUPSUpgradeable} from "../../plugin/PluginUUPSUpgradeable.sol"; import {IDAO} from "../../dao/IDAO.sol"; -/// @notice A mock cloneable plugin to be deployed via the UUPS proxy pattern. +/// @notice A mock upgradeable plugin to be deployed via the UUPS proxy pattern. /// v1.1 (Release 1, Build 1) /// @dev DO NOT USE IN PRODUCTION! contract PluginUUPSUpgradeableMockBuild1 is PluginUUPSUpgradeable { @@ -18,7 +18,7 @@ contract PluginUUPSUpgradeableMockBuild1 is PluginUUPSUpgradeable { } } -/// @notice A mock cloneable plugin to be deployed via the UUPS proxy pattern. +/// @notice A mock upgradeable plugin to be deployed via the UUPS proxy pattern. /// v1.1 (Release 1, Build 2) /// @dev DO NOT USE IN PRODUCTION! contract PluginUUPSUpgradeableMockBuild2 is PluginUUPSUpgradeable { @@ -38,7 +38,7 @@ contract PluginUUPSUpgradeableMockBuild2 is PluginUUPSUpgradeable { } } -/// @notice A mock cloneable plugin to be deployed via the UUPS proxy pattern. +/// @notice A mock upgradeable plugin to be deployed via the UUPS proxy pattern. /// v1.1 (Release 1, Build 3) /// @dev DO NOT USE IN PRODUCTION! contract PluginUUPSUpgradeableMockBuild3 is PluginUUPSUpgradeable { @@ -62,3 +62,14 @@ contract PluginUUPSUpgradeableMockBuild3 is PluginUUPSUpgradeable { } } } + +/// @notice A mock upgradeable plugin missing an initializer function. +/// @dev DO NOT USE IN PRODUCTION! +contract PluginUUPSUpgradeableMockBad is PluginUUPSUpgradeable { + uint256 public state1; + + function notAnInitializer(IDAO _dao) external { + __PluginUUPSUpgradeable_init(_dao); + state1 = 1; + } +} diff --git a/contracts/src/mocks/plugin/PluginUUPSUpgradeableSetupMock.sol b/contracts/src/mocks/plugin/PluginUUPSUpgradeableSetupMock.sol index 8aae74cf..9da148a2 100644 --- a/contracts/src/mocks/plugin/PluginUUPSUpgradeableSetupMock.sol +++ b/contracts/src/mocks/plugin/PluginUUPSUpgradeableSetupMock.sol @@ -5,7 +5,8 @@ pragma solidity ^0.8.8; import {PermissionLib} from "../../permission/PermissionLib.sol"; import {IPluginSetup} from "../../plugin/setup/IPluginSetup.sol"; -import {PluginSetup} from "../../plugin/setup/PluginSetup.sol"; +import {PluginUpgradeableSetup} from "../../plugin/setup/PluginUpgradeableSetup.sol"; +import {ProxyLib} from "../../utils/deployment/ProxyLib.sol"; import {IDAO} from "../../dao/IDAO.sol"; import {mockPermissions, mockHelpers} from "./PluginSetupMockData.sol"; import {PluginUUPSUpgradeableMockBuild1, PluginUUPSUpgradeableMockBuild2, PluginUUPSUpgradeableMockBuild3} from "./PluginUUPSUpgradeableMock.sol"; @@ -13,70 +14,80 @@ import {PluginUUPSUpgradeableMockBuild1, PluginUUPSUpgradeableMockBuild2, Plugin /// @notice A mock plugin setup of an upgradeable plugin to be deployed via the UUPS pattern. /// v1.1 (Release 1, Build 1) /// @dev DO NOT USE IN PRODUCTION! -contract PluginUUPSUpgradeableSetupMockBuild1 is PluginSetup { - address internal pluginBase; +contract PluginUUPSUpgradeableSetupMockBuild1 is PluginUpgradeableSetup { + using ProxyLib for address; - constructor() { - pluginBase = address(new PluginUUPSUpgradeableMockBuild1()); - } + uint16 internal constant THIS_BUILD = 1; + + constructor() PluginUpgradeableSetup(address(new PluginUUPSUpgradeableMockBuild1())) {} /// @inheritdoc IPluginSetup function prepareInstallation( address _dao, bytes memory - ) public virtual override returns (address plugin, PreparedSetupData memory preparedSetupData) { + ) public returns (address plugin, PreparedSetupData memory preparedSetupData) { bytes memory initData = abi.encodeCall( PluginUUPSUpgradeableMockBuild1.initialize, (IDAO(_dao)) ); - plugin = createERC1967Proxy(pluginBase, initData); + plugin = implementation().deployUUPSProxy(initData); preparedSetupData.helpers = mockHelpers(1); preparedSetupData.permissions = mockPermissions(0, 1, PermissionLib.Operation.Grant); } /// @inheritdoc IPluginSetup - function prepareUninstallation( + /// @dev The default implementation for the initial build 1 that reverts because no earlier build exists. + function prepareUpdate( address _dao, + uint16 _fromBuild, SetupPayload calldata _payload - ) external virtual override returns (PermissionLib.MultiTargetPermission[] memory permissions) { - (_dao, _payload); - permissions = mockPermissions(0, 1, PermissionLib.Operation.Revoke); + ) external pure virtual returns (bytes memory, PreparedSetupData memory) { + (_dao, _fromBuild, _payload); + revert InvalidUpdatePath({fromBuild: 0, thisBuild: THIS_BUILD}); } /// @inheritdoc IPluginSetup - function implementation() external view virtual override returns (address) { - return address(pluginBase); + function prepareUninstallation( + address _dao, + SetupPayload calldata _payload + ) external pure returns (PermissionLib.MultiTargetPermission[] memory permissions) { + (_dao, _payload); + permissions = mockPermissions(0, THIS_BUILD, PermissionLib.Operation.Revoke); } } /// @notice A mock plugin setup of an upgradeable plugin to be deployed via the UUPS pattern. /// v1.2 (Release 1, Build 2) /// @dev DO NOT USE IN PRODUCTION! -contract PluginUUPSUpgradeableSetupMockBuild2 is PluginSetup { - address internal pluginBase; +contract PluginUUPSUpgradeableSetupMockBuild2 is PluginUpgradeableSetup { + using ProxyLib for address; - constructor() { - pluginBase = address(new PluginUUPSUpgradeableMockBuild2()); - } + uint16 internal constant THIS_BUILD = 2; + + constructor() PluginUpgradeableSetup(address(new PluginUUPSUpgradeableMockBuild2())) {} /// @inheritdoc IPluginSetup function prepareInstallation( address _dao, bytes memory - ) external override returns (address plugin, PreparedSetupData memory preparedSetupData) { + ) external returns (address plugin, PreparedSetupData memory preparedSetupData) { bytes memory initData = abi.encodeCall( PluginUUPSUpgradeableMockBuild2.initialize, (IDAO(_dao)) ); - plugin = createERC1967Proxy(pluginBase, initData); - preparedSetupData.helpers = mockHelpers(2); - preparedSetupData.permissions = mockPermissions(0, 2, PermissionLib.Operation.Grant); + plugin = implementation().deployUUPSProxy(initData); + preparedSetupData.helpers = mockHelpers(THIS_BUILD); + preparedSetupData.permissions = mockPermissions( + 0, + THIS_BUILD, + PermissionLib.Operation.Grant + ); } /// @inheritdoc IPluginSetup function prepareUpdate( address _dao, - uint16 _currentBuild, + uint16 _fromBuild, SetupPayload calldata _payload ) external @@ -87,13 +98,14 @@ contract PluginUUPSUpgradeableSetupMockBuild2 is PluginSetup { (_dao, _payload); // Update from Build 1 - if (_currentBuild == 1) { - preparedSetupData.helpers = mockHelpers(2); - initData = abi.encodeCall( - PluginUUPSUpgradeableMockBuild2.initializeFrom, - (_currentBuild) + if (_fromBuild == 1) { + preparedSetupData.helpers = mockHelpers(THIS_BUILD); + initData = abi.encodeCall(PluginUUPSUpgradeableMockBuild2.initializeFrom, (_fromBuild)); + preparedSetupData.permissions = mockPermissions( + _fromBuild, + THIS_BUILD, + PermissionLib.Operation.Grant ); - preparedSetupData.permissions = mockPermissions(1, 2, PermissionLib.Operation.Grant); } } @@ -101,25 +113,20 @@ contract PluginUUPSUpgradeableSetupMockBuild2 is PluginSetup { function prepareUninstallation( address _dao, SetupPayload calldata _payload - ) external pure override returns (PermissionLib.MultiTargetPermission[] memory permissions) { + ) external pure returns (PermissionLib.MultiTargetPermission[] memory permissions) { (_dao, _payload); - permissions = mockPermissions(0, 2, PermissionLib.Operation.Revoke); - } - - /// @inheritdoc IPluginSetup - function implementation() external view override returns (address) { - return address(pluginBase); + permissions = mockPermissions(0, THIS_BUILD, PermissionLib.Operation.Revoke); } } /// @notice A mock plugin setup of an upgradeable plugin to be deployed via the UUPS pattern. /// v1.3 (Release 1, Build 3) -contract PluginUUPSUpgradeableSetupMockBuild3 is PluginSetup { - address internal pluginBase; +contract PluginUUPSUpgradeableSetupMockBuild3 is PluginUpgradeableSetup { + using ProxyLib for address; - constructor() { - pluginBase = address(new PluginUUPSUpgradeableMockBuild3()); - } + uint16 internal constant THIS_BUILD = 3; + + constructor() PluginUpgradeableSetup(address(new PluginUUPSUpgradeableMockBuild3())) {} /// @inheritdoc IPluginSetup function prepareInstallation( @@ -130,15 +137,20 @@ contract PluginUUPSUpgradeableSetupMockBuild3 is PluginSetup { PluginUUPSUpgradeableMockBuild3.initialize, (IDAO(_dao)) ); - plugin = createERC1967Proxy(pluginBase, initData); - preparedSetupData.helpers = mockHelpers(3); - preparedSetupData.permissions = mockPermissions(0, 3, PermissionLib.Operation.Grant); + plugin = implementation().deployUUPSProxy(initData); + preparedSetupData.helpers = mockHelpers(THIS_BUILD); + preparedSetupData.permissions = mockPermissions( + 0, + THIS_BUILD, + PermissionLib.Operation.Grant + ); } /// @inheritdoc IPluginSetup + function prepareUpdate( address _dao, - uint16 _currentBuild, + uint16 _fromBuild, SetupPayload calldata _payload ) external @@ -149,23 +161,28 @@ contract PluginUUPSUpgradeableSetupMockBuild3 is PluginSetup { (_dao, _payload); // Update from Build 1 - if (_currentBuild == 1) { - preparedSetupData.helpers = mockHelpers(3); - initData = abi.encodeCall( - PluginUUPSUpgradeableMockBuild3.initializeFrom, - (_currentBuild) + if (_fromBuild == 1) { + preparedSetupData.helpers = mockHelpers(THIS_BUILD); + initData = abi.encodeCall(PluginUUPSUpgradeableMockBuild3.initializeFrom, (_fromBuild)); + preparedSetupData.permissions = mockPermissions( + _fromBuild, + THIS_BUILD, + PermissionLib.Operation.Grant ); - preparedSetupData.permissions = mockPermissions(1, 3, PermissionLib.Operation.Grant); + + // If this update path should not be supported, you can revert with an error, e.g., + // revert InvalidUpdatePath({fromBuild: _fromBuild, thisBuild: THIS_BUILD}); } // Update from Build 2 - if (_currentBuild == 2) { - preparedSetupData.helpers = mockHelpers(3); - initData = abi.encodeCall( - PluginUUPSUpgradeableMockBuild3.initializeFrom, - (_currentBuild) + if (_fromBuild == 2) { + preparedSetupData.helpers = mockHelpers(THIS_BUILD); + initData = abi.encodeCall(PluginUUPSUpgradeableMockBuild3.initializeFrom, (_fromBuild)); + preparedSetupData.permissions = mockPermissions( + _fromBuild, + THIS_BUILD, + PermissionLib.Operation.Grant ); - preparedSetupData.permissions = mockPermissions(2, 3, PermissionLib.Operation.Grant); } } @@ -173,13 +190,8 @@ contract PluginUUPSUpgradeableSetupMockBuild3 is PluginSetup { function prepareUninstallation( address _dao, SetupPayload calldata _payload - ) external virtual override returns (PermissionLib.MultiTargetPermission[] memory permissions) { + ) external pure returns (PermissionLib.MultiTargetPermission[] memory permissions) { (_dao, _payload); - permissions = mockPermissions(0, 3, PermissionLib.Operation.Revoke); - } - - /// @inheritdoc IPluginSetup - function implementation() external view override returns (address) { - return address(pluginBase); + permissions = mockPermissions(0, THIS_BUILD, PermissionLib.Operation.Revoke); } } diff --git a/contracts/src/plugin/setup/IPluginSetup.sol b/contracts/src/plugin/setup/IPluginSetup.sol index ce510287..f9536652 100644 --- a/contracts/src/plugin/setup/IPluginSetup.sol +++ b/contracts/src/plugin/setup/IPluginSetup.sol @@ -42,13 +42,13 @@ interface IPluginSetup { /// @notice Prepares the update of a plugin. /// @param _dao The address of the updating DAO. - /// @param _currentBuild The build number of the plugin to update from. + /// @param _fromBuild The build number of the plugin to update from. /// @param _payload The relevant data necessary for the `prepareUpdate`. See above. /// @return initData The initialization data to be passed to upgradeable contracts when the update is applied in the `PluginSetupProcessor`. /// @return preparedSetupData The deployed plugin's relevant data which consists of helpers and permissions. function prepareUpdate( address _dao, - uint16 _currentBuild, + uint16 _fromBuild, SetupPayload calldata _payload ) external returns (bytes memory initData, PreparedSetupData memory preparedSetupData); @@ -63,6 +63,6 @@ interface IPluginSetup { /// @notice Returns the plugin implementation address. /// @return The address of the plugin implementation contract. - /// @dev The implementation can be instantiated via the `new` keyword, cloned via the minimal clones pattern (see [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167)), or proxied via the UUPS pattern (see [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822)). + /// @dev The implementation can be instantiated via the `new` keyword, cloned via the minimal proxy pattern (see [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167)), or proxied via the UUPS proxy pattern (see [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822)). function implementation() external view returns (address); } diff --git a/contracts/src/plugin/setup/PluginSetup.sol b/contracts/src/plugin/setup/PluginSetup.sol index 7ca76c7c..fde09491 100644 --- a/contracts/src/plugin/setup/PluginSetup.sol +++ b/contracts/src/plugin/setup/PluginSetup.sol @@ -4,43 +4,38 @@ pragma solidity ^0.8.8; import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; -// solhint-disable-next-line no-unused-import -import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; - import {IProtocolVersion} from "../../utils/versioning/IProtocolVersion.sol"; import {ProtocolVersion} from "../../utils/versioning/ProtocolVersion.sol"; -import {createERC1967Proxy as createERC1967} from "../../utils/deployment/Proxy.sol"; import {IPluginSetup} from "./IPluginSetup.sol"; /// @title PluginSetup -/// @author Aragon Association - 2022-2023 -/// @notice An abstract contract that developers have to inherit from to write the setup of a plugin. +/// @author Aragon Association - 2022-2024 +/// @notice An abstract contract to inherit from to implement the plugin setup for non-upgradeable plugins, i.e, +/// - `Plugin` being deployed via the `new` keyword +/// - `PluginCloneable` being deployed via the minimal proxy pattern (see [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167)). /// @custom:security-contact sirt@aragon.org abstract contract PluginSetup is ERC165, IPluginSetup, ProtocolVersion { + /// @notice The address of the plugin implementation contract for initial block explorer verification and, in the case of `PluginClonable` implementations, to create [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167) clones from. + address internal immutable IMPLEMENTATION; + + /// @notice Thrown when attempting to prepare an update on a non-upgradeable plugin. + error NonUpgradeablePlugin(); + + /// @notice The contract constructor, that setting the plugin implementation contract. + /// @param _implementation The address of the plugin implementation contract. + constructor(address _implementation) { + IMPLEMENTATION = _implementation; + } + /// @inheritdoc IPluginSetup + /// @dev Since the underlying plugin is non-upgradeable, this non-virtual function must always revert. function prepareUpdate( address _dao, - uint16 _currentBuild, + uint16 _fromBuild, SetupPayload calldata _payload - ) - external - virtual - override - returns (bytes memory initData, PreparedSetupData memory preparedSetupData) - // solhint-disable-next-line no-empty-blocks - { - // Empty to have a default implementation for non-upgradeable plugins. - } - - /// @notice A convenience function to create an [ERC-1967](https://eips.ethereum.org/EIPS/eip-1967) proxy contract pointing to an implementation and being associated to a DAO. - /// @param _implementation The address of the implementation contract to which the proxy is pointing to. - /// @param _data The data to initialize the storage of the proxy contract. - /// @return The address of the created proxy contract. - function createERC1967Proxy( - address _implementation, - bytes memory _data - ) internal returns (address) { - return createERC1967(_implementation, _data); + ) external pure returns (bytes memory, PreparedSetupData memory) { + (_dao, _fromBuild, _payload); + revert NonUpgradeablePlugin(); } /// @notice Checks if this or the parent contract supports an interface by its ID. @@ -52,4 +47,9 @@ abstract contract PluginSetup is ERC165, IPluginSetup, ProtocolVersion { _interfaceId == type(IProtocolVersion).interfaceId || super.supportsInterface(_interfaceId); } + + /// @inheritdoc IPluginSetup + function implementation() public view returns (address) { + return IMPLEMENTATION; + } } diff --git a/contracts/src/plugin/setup/PluginUpgradeableSetup.sol b/contracts/src/plugin/setup/PluginUpgradeableSetup.sol new file mode 100644 index 00000000..413cc500 --- /dev/null +++ b/contracts/src/plugin/setup/PluginUpgradeableSetup.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +pragma solidity ^0.8.8; + +import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; + +import {IProtocolVersion} from "../../utils/versioning/IProtocolVersion.sol"; +import {ProtocolVersion} from "../../utils/versioning/ProtocolVersion.sol"; +import {IPluginSetup} from "./IPluginSetup.sol"; + +/// @title PluginUpgradeableSetup +/// @author Aragon Association - 2022-2024 +/// @notice An abstract contract to inherit from to implement the plugin setup for upgradeable plugins, i.e, `PluginUUPSUpgradeable` being deployed via the UUPS pattern (see [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822) and [ERC-1967](https://eips.ethereum.org/EIPS/eip-1967)). +/// @custom:security-contact sirt@aragon.org +abstract contract PluginUpgradeableSetup is ERC165, IPluginSetup, ProtocolVersion { + /// @notice The address of the plugin implementation contract for initial block explorer verification + /// and to create [ERC-1967](https://eips.ethereum.org/EIPS/eip-1967) UUPS proxies from. + address internal immutable IMPLEMENTATION; + + /// @notice Thrown when an update path is not available, for example, if this is the initial build. + /// @param fromBuild The build number to update from. + /// @param thisBuild The build number of this setup to update to. + error InvalidUpdatePath(uint16 fromBuild, uint16 thisBuild); + + /// @notice The contract constructor, that setting the plugin implementation contract. + /// @param _implementation The address of the plugin implementation contract. + constructor(address _implementation) { + IMPLEMENTATION = _implementation; + } + + /// @notice Checks if this or the parent contract supports an interface by its ID. + /// @param _interfaceId The ID of the interface. + /// @return Returns `true` if the interface is supported. + function supportsInterface(bytes4 _interfaceId) public view virtual override returns (bool) { + return + _interfaceId == type(IPluginSetup).interfaceId || + _interfaceId == type(IProtocolVersion).interfaceId || + super.supportsInterface(_interfaceId); + } + + /// @inheritdoc IPluginSetup + function implementation() public view returns (address) { + return IMPLEMENTATION; + } +} diff --git a/contracts/src/utils/deployment/CloneFactory.sol b/contracts/src/utils/deployment/CloneFactory.sol deleted file mode 100644 index 3e13d8c2..00000000 --- a/contracts/src/utils/deployment/CloneFactory.sol +++ /dev/null @@ -1,21 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-or-later - -pragma solidity ^0.8.8; - -// TODO will be refactored as part of task OS-794 - -import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; - -contract CloneFactory { - using Clones for address; - - address private immutable IMPLEMENTATION; - - constructor(address _implementation) { - IMPLEMENTATION = _implementation; - } - - function deployClone() external returns (address clone) { - return IMPLEMENTATION.clone(); - } -} diff --git a/contracts/src/utils/deployment/Proxy.sol b/contracts/src/utils/deployment/Proxy.sol deleted file mode 100644 index 86365ec0..00000000 --- a/contracts/src/utils/deployment/Proxy.sol +++ /dev/null @@ -1,15 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-or-later - -pragma solidity ^0.8.8; - -import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; - -/// @notice Free function to create a [ERC-1967](https://eips.ethereum.org/EIPS/eip-1967) proxy contract based on the passed base contract address. -/// @param _logic The base contract address. -/// @param _data The constructor arguments for this contract. -/// @return The address of the proxy contract created. -/// @dev Initializes the upgradeable proxy with an initial implementation specified by _logic. If _data is non-empty, it’s used as data in a delegate call to _logic. This will typically be an encoded function call, and allows initializing the storage of the proxy like a Solidity constructor (see [OpenZeppelin ERC1967Proxy-constructor](https://docs.openzeppelin.com/contracts/4.x/api/proxy#ERC1967Proxy-constructor-address-bytes-)). -/// @custom:security-contact sirt@aragon.org -function createERC1967Proxy(address _logic, bytes memory _data) returns (address) { - return address(new ERC1967Proxy(_logic, _data)); -} diff --git a/contracts/src/utils/deployment/ProxyFactory.sol b/contracts/src/utils/deployment/ProxyFactory.sol new file mode 100644 index 00000000..289c1c63 --- /dev/null +++ b/contracts/src/utils/deployment/ProxyFactory.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +pragma solidity ^0.8.8; + +import {ProxyLib} from "./ProxyLib.sol"; + +/// @title ProxyFactory +/// @author Aragon Association - 2024 +/// @notice A factory to deploy proxies via the UUPS pattern (see [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822)) and minimal proxy pattern (see [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167)). +/// @custom:security-contact sirt@aragon.org +contract ProxyFactory { + using ProxyLib for address; + /// @notice The immutable logic contract address. + address internal immutable IMPLEMENTATION; + + /// @notice Emitted when an proxy contract is created. + /// @param proxy The proxy address. + event ProxyCreated(address proxy); + + /// @notice Initializes the contract with a logic contract address. + /// @param _implementation The logic contract address. + constructor(address _implementation) { + IMPLEMENTATION = _implementation; + } + + /// @notice Creates an [ERC-1967](https://eips.ethereum.org/EIPS/eip-1967) proxy contract pointing to the pre-set logic contract. + /// @param _data The initialization data for this contract. + /// @return proxy The address of the proxy contract created. + /// @dev If `_data` is non-empty, it is used in a delegate call to the `_implementation` contract. This will typically be an encoded function call initializing the proxy (see [OpenZeppelin ERC1967Proxy-constructor](https://docs.openzeppelin.com/contracts/4.x/api/proxy#ERC1967Proxy-constructor-address-bytes-)). + function deployUUPSProxy(bytes memory _data) external returns (address proxy) { + proxy = IMPLEMENTATION.deployUUPSProxy(_data); + emit ProxyCreated({proxy: proxy}); + } + + /// @notice Creates an [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167) minimal proxy contract pointing to the pre-set logic contract. + /// @param _data The initialization data for this contract. + /// @return proxy The address of the proxy contract created. + /// @dev If `_data` is non-empty, it is used in a call to the clone contract. This will typically be an encoded function call initializing the storage of the contract. + function deployMinimalProxy(bytes memory _data) external returns (address proxy) { + proxy = IMPLEMENTATION.deployMinimalProxy(_data); + emit ProxyCreated({proxy: proxy}); + } + + /// @notice Returns the implementation contract address. + /// @return The address of the implementation contract. + /// @dev The implementation can be cloned via the minimal proxy pattern (see [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167)), or proxied via the UUPS proxy pattern (see [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822)). + function implementation() public view returns (address) { + return IMPLEMENTATION; + } +} diff --git a/contracts/src/utils/deployment/ProxyLib.sol b/contracts/src/utils/deployment/ProxyLib.sol new file mode 100644 index 00000000..44a2c908 --- /dev/null +++ b/contracts/src/utils/deployment/ProxyLib.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +pragma solidity ^0.8.8; + +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; + +/// @title ProxyLib +/// @author Aragon Association - 2024 +/// @notice A library containing methods for the deployment of proxies via the UUPS pattern (see [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822)) and minimal proxy pattern (see [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167)). +/// @custom:security-contact sirt@aragon.org +library ProxyLib { + using Address for address; + using Clones for address; + + /// @notice Creates an [ERC-1967](https://eips.ethereum.org/EIPS/eip-1967) UUPS proxy contract pointing to a logic contract and allows to immediately initialize it. + /// @param _logic The logic contract the proxy is pointing to. + /// @param _initCalldata The initialization data for this contract. + /// @return uupsProxy The address of the UUPS proxy contract created. + /// @dev If `_initCalldata` is non-empty, it is used in a delegate call to the `_logic` contract. This will typically be an encoded function call initializing the storage of the proxy (see [OpenZeppelin ERC1967Proxy-constructor](https://docs.openzeppelin.com/contracts/4.x/api/proxy#ERC1967Proxy-constructor-address-bytes-)). + function deployUUPSProxy( + address _logic, + bytes memory _initCalldata + ) internal returns (address uupsProxy) { + uupsProxy = address(new ERC1967Proxy({_logic: _logic, _data: _initCalldata})); + } + + /// @notice Creates an [ERC-1167](https://eips.ethereum.org/EIPS/eip-1167) minimal proxy contract, also known as clones, pointing to a logic contract and allows to immediately initialize it. + /// @param _logic The logic contract the proxy is pointing to. + /// @param _initCalldata The initialization data for this contract. + /// @return minimalProxy The address of the minimal proxy contract created. + /// @dev If `_initCalldata` is non-empty, it is used in a call to the clone contract. This will typically be an encoded function call initializing the storage of the contract. + function deployMinimalProxy( + address _logic, + bytes memory _initCalldata + ) internal returns (address minimalProxy) { + minimalProxy = _logic.clone(); + if (_initCalldata.length > 0) { + minimalProxy.functionCall({data: _initCalldata}); + } + } +} diff --git a/contracts/test/permission/auth/dao-authorizable.ts b/contracts/test/permission/auth/dao-authorizable.ts index b06440b5..cf5abda3 100644 --- a/contracts/test/permission/auth/dao-authorizable.ts +++ b/contracts/test/permission/auth/dao-authorizable.ts @@ -51,9 +51,7 @@ describe('DaoAuthorizableUpgradeable', async () => { }); // Contains tests for functionality common for `DaoAuthorizableMock` and `DaoAuthorizableUpgradeableMock` to avoid duplication. -function daoAuthorizableBaseTests( - fixture: () => Promise -) { +function daoAuthorizableBaseTests(fixture: () => Promise) { const permissionId = ethers.utils.id('AUTHORIZED_FUNC_PERMISSION'); it('initializes the contract with a DAO address', async () => { @@ -91,14 +89,14 @@ function daoAuthorizableBaseTests( }); } -type DaoAuthorizableFixtureResult = { +type FixtureResult = { alice: SignerWithAddress; bob: SignerWithAddress; daoAuthorizableMock: DaoAuthorizableMock | DaoAuthorizableUpgradeableMock; daoMock: DAOMock; }; -async function daoAuthorizableFixture(): Promise { +async function daoAuthorizableFixture(): Promise { const [alice, bob] = await ethers.getSigners(); const daoMock = await new DAOMock__factory(alice).deploy(); const daoAuthorizableMock = await new DaoAuthorizableMock__factory( @@ -108,7 +106,7 @@ async function daoAuthorizableFixture(): Promise { return {alice, bob, daoAuthorizableMock, daoMock}; } -async function daoAuthorizableUpgradeableFixture(): Promise { +async function daoAuthorizableUpgradeableFixture(): Promise { const [alice, bob] = await ethers.getSigners(); const daoMock = await new DAOMock__factory(alice).deploy(); const daoAuthorizableMock = await new DaoAuthorizableUpgradeableMock__factory( diff --git a/contracts/test/plugin/extensions/proposal.ts b/contracts/test/plugin/extensions/proposal.ts index 184b21d3..ddc92d56 100644 --- a/contracts/test/plugin/extensions/proposal.ts +++ b/contracts/test/plugin/extensions/proposal.ts @@ -49,7 +49,7 @@ describe('ProposalUpgradeable', async () => { }); // Contains tests for functionality common for `ProposalMock` and `ProposalUpgradeableMock` to avoid duplication. -function proposalBaseTests(fixture: () => Promise) { +function proposalBaseTests(fixture: () => Promise) { it('counts proposals', async () => { const {alice, bob, proposalMock, exampleData} = await loadFixture(fixture); @@ -117,7 +117,7 @@ function proposalBaseTests(fixture: () => Promise) { exampleData.allowFailureMap ) ) - .to.emit(proposalMock, IPROPOSAL_EVENTS.PROPOSAL_CREATED) + .to.emit(proposalMock, IPROPOSAL_EVENTS.ProposalCreated) .withArgs( expectedProposalId, creator.address, @@ -168,7 +168,7 @@ function proposalBaseTests(fixture: () => Promise) { exampleData.allowFailureMap ) ) - .to.emit(proposalMock, IPROPOSAL_EVENTS.PROPOSAL_EXECUTED) + .to.emit(proposalMock, IPROPOSAL_EVENTS.ProposalExecuted) .withArgs(proposalId); }); @@ -198,7 +198,7 @@ function proposalBaseTests(fixture: () => Promise) { const event = await findEventTopicLog( tx, IDAO__factory.createInterface(), - IDAO_EVENTS.EXECUTED + IDAO_EVENTS.Executed ); expect(event.args.actor).to.equal(expectedActor); @@ -253,7 +253,7 @@ async function baseFixture(): Promise { return {alice, bob, daoMock, exampleData}; } -type ProposalFixtureResult = { +type FixtureResult = { proposalMock: ProposalMock | ProposalUpgradeableMock; alice: SignerWithAddress; bob: SignerWithAddress; @@ -261,7 +261,7 @@ type ProposalFixtureResult = { exampleData: ProposalData; }; -async function proposalFixture(): Promise { +async function proposalFixture(): Promise { const {alice, bob, daoMock, exampleData} = await baseFixture(); const proposalMock = await new ProposalMock__factory(alice).deploy(); @@ -269,7 +269,7 @@ async function proposalFixture(): Promise { return {alice, bob, proposalMock, daoMock, exampleData}; } -async function proposalUpgradeableFixture(): Promise { +async function proposalUpgradeableFixture(): Promise { const {alice, bob, daoMock, exampleData} = await baseFixture(); const proposalMock = await new ProposalUpgradeableMock__factory( diff --git a/contracts/test/plugin/plugin-clonable.ts b/contracts/test/plugin/plugin-clonable.ts index b459ed03..04acd0d9 100644 --- a/contracts/test/plugin/plugin-clonable.ts +++ b/contracts/test/plugin/plugin-clonable.ts @@ -1,37 +1,42 @@ import { - CloneFactory__factory, + DAOMock, + DAOMock__factory, IPlugin__factory, IProtocolVersion__factory, + PluginCloneableMockBad__factory, PluginCloneableMockBuild1, PluginCloneableMockBuild1__factory, + ProxyFactory__factory, } from '../../typechain'; +import { + ProxyCreatedEvent, + ProxyFactory, +} from '../../typechain/src/utils/deployment/ProxyFactory'; import {erc165ComplianceTests, getOzInitializedSlotValue} from '../helpers'; import {osxCommonsContractsVersion} from '../utils/versioning/protocol-version'; -import {getInterfaceId, PluginType} from '@aragon/osx-commons-sdk'; +import { + findEvent, + getInterfaceId, + PluginType, + PROXY_FACTORY_EVENTS, +} from '@aragon/osx-commons-sdk'; +import {loadFixture} from '@nomicfoundation/hardhat-network-helpers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; import {ethers} from 'hardhat'; describe('PluginCloneable', function () { - let deployer: SignerWithAddress; - let plugin: PluginCloneableMockBuild1; - - before(async () => { - [deployer] = await ethers.getSigners(); - plugin = await new PluginCloneableMockBuild1__factory(deployer).deploy(); - }); - describe('Initializable', async () => { it('initialize', async () => { - // Deploy a clone - const cloneFactory = await new CloneFactory__factory(deployer).deploy( - plugin.address - ); - const expectedAddress = await cloneFactory.callStatic.deployClone(); - await cloneFactory.deployClone(); - const clone = new PluginCloneableMockBuild1__factory(deployer).attach( - expectedAddress + const {proxyFactory, Build1Factory, daoMock} = await loadFixture(fixture); + + // Deploy an uninitialized clone + const tx = await proxyFactory.deployMinimalProxy([]); + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated ); + const clone = Build1Factory.attach(event.args.proxy); // Check the clone before initialization expect( @@ -40,28 +45,54 @@ describe('PluginCloneable', function () { expect(await clone.dao()).to.equal(ethers.constants.AddressZero); expect(await clone.state1()).to.equal(0); - // Initialize the clone - const dummyDaoAddress = plugin.address; - await clone.initialize(dummyDaoAddress); + await clone.initialize(daoMock.address); // Check the clone after initialization expect( await getOzInitializedSlotValue(ethers.provider, clone.address) ).to.equal(1); - expect(await clone.dao()).to.equal(dummyDaoAddress); + expect(await clone.dao()).to.equal(daoMock.address); expect(await clone.state1()).to.equal(1); }); + + it('disables initializers for the implementation', async () => { + const {implementation} = await loadFixture(fixture); + + // Check that the implementation is uninitialized. + expect(await implementation.dao()).to.equal(ethers.constants.AddressZero); + expect(await implementation.state1()).to.equal(0); + + // Check that the implementation initialization is disabled. + + expect( + await getOzInitializedSlotValue(ethers.provider, implementation.address) + ).to.equal(255); + }); + + it('reverts if an function tries to call `__PluginCloneable_init` without being an initializer', async () => { + const {deployer, daoMock} = await loadFixture(fixture); + + const badPlugin = await new PluginCloneableMockBad__factory( + deployer + ).deploy(); + + await expect( + badPlugin.notAnInitializer(daoMock.address) + ).to.be.revertedWith('Initializable: contract is not initializing'); + }); }); describe('PluginType', async () => { it('returns the current protocol version', async () => { - expect(await plugin.pluginType()).to.equal(PluginType.Cloneable); + const {implementation} = await loadFixture(fixture); + expect(await implementation.pluginType()).to.equal(PluginType.Cloneable); }); }); describe('ProtocolVersion', async () => { it('returns the current protocol version matching the semantic version of the `osx-contracts-commons` package', async () => { - expect(await plugin.protocolVersion()).to.deep.equal( + const {implementation} = await loadFixture(fixture); + expect(await implementation.protocolVersion()).to.deep.equal( osxCommonsContractsVersion() ); }); @@ -69,25 +100,67 @@ describe('PluginCloneable', function () { describe('ERC-165', async () => { it('supports the `ERC-165` standard', async () => { - await erc165ComplianceTests(plugin, deployer); + const {deployer, implementation} = await loadFixture(fixture); + await erc165ComplianceTests(implementation, deployer); }); it('supports the `IPlugin` interface', async () => { + const {implementation} = await loadFixture(fixture); const iface = IPlugin__factory.createInterface(); - expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; + expect(await implementation.supportsInterface(getInterfaceId(iface))).to + .be.true; }); it('supports the `IProtocolVersion` interface', async () => { + const {implementation} = await loadFixture(fixture); const iface = IProtocolVersion__factory.createInterface(); - expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; + expect(await implementation.supportsInterface(getInterfaceId(iface))).to + .be.true; }); }); describe('Protocol version', async () => { it('returns the current protocol version', async () => { - expect(await plugin.protocolVersion()).to.deep.equal( + const {implementation} = await loadFixture(fixture); + expect(await implementation.protocolVersion()).to.deep.equal( osxCommonsContractsVersion() ); }); }); }); + +type FixtureResult = { + deployer: SignerWithAddress; + implementation: PluginCloneableMockBuild1; + proxyFactory: ProxyFactory; + daoMock: DAOMock; + initCalldata: string; + Build1Factory: PluginCloneableMockBuild1__factory; +}; + +async function fixture(): Promise { + const [deployer] = await ethers.getSigners(); + + const Build1Factory = new PluginCloneableMockBuild1__factory(deployer); + const daoMock = await new DAOMock__factory(deployer).deploy(); + + const implementation = await Build1Factory.deploy(); + + const proxyFactory = await new ProxyFactory__factory(deployer).deploy( + implementation.address + ); + + const initCalldata = implementation.interface.encodeFunctionData( + 'initialize', + [daoMock.address] + ); + + return { + deployer, + implementation, + proxyFactory, + daoMock, + initCalldata, + Build1Factory, + }; +} diff --git a/contracts/test/plugin/plugin-uups-upgradeable.ts b/contracts/test/plugin/plugin-uups-upgradeable.ts index af29af41..0ec237cb 100644 --- a/contracts/test/plugin/plugin-uups-upgradeable.ts +++ b/contracts/test/plugin/plugin-uups-upgradeable.ts @@ -1,50 +1,45 @@ import { + DAOMock, + DAOMock__factory, IERC1822ProxiableUpgradeable__factory, IPlugin__factory, - DAOMock__factory, IProtocolVersion__factory, PluginUUPSUpgradeableMockBuild1, PluginUUPSUpgradeableMockBuild1__factory, PluginUUPSUpgradeableMockBuild2__factory, + PluginUUPSUpgradeableMockBad__factory, + ProxyFactory__factory, } from '../../typechain'; +import { + ProxyCreatedEvent, + ProxyFactory, +} from '../../typechain/src/utils/deployment/ProxyFactory'; import {erc165ComplianceTests, getOzInitializedSlotValue} from '../helpers'; import {osxCommonsContractsVersion} from '../utils/versioning/protocol-version'; import { + findEvent, getInterfaceId, PLUGIN_UUPS_UPGRADEABLE_PERMISSIONS, PluginType, + PROXY_FACTORY_EVENTS, } from '@aragon/osx-commons-sdk'; +import {loadFixture} from '@nomicfoundation/hardhat-network-helpers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; -import {ethers, upgrades} from 'hardhat'; +import {ethers} from 'hardhat'; describe('PluginUUPSUpgradeable', function () { - let Build1Factory: PluginUUPSUpgradeableMockBuild1__factory; - let Build2Factory: PluginUUPSUpgradeableMockBuild2__factory; - let DAOMockFactory: DAOMock__factory; - - let plugin: PluginUUPSUpgradeableMockBuild1; - let deployer: SignerWithAddress; - - before(async () => { - [deployer] = await ethers.getSigners(); - - Build1Factory = new PluginUUPSUpgradeableMockBuild1__factory(deployer); - Build2Factory = new PluginUUPSUpgradeableMockBuild2__factory(deployer); - DAOMockFactory = new DAOMock__factory(deployer); - - plugin = await Build1Factory.deploy(); - }); - describe('Initializable', async () => { it('initialize', async () => { - // Deploy a proxy - const proxy = await upgrades.deployProxy(Build1Factory, [], { - kind: 'uups', - initializer: false, // DO NOT USE THIS CODE IN PRODUCTION. This deploys an initialized proxy. - unsafeAllow: ['constructor'], - constructorArgs: [], - }); + const {proxyFactory, Build1Factory, daoMock} = await loadFixture(fixture); + + // Deploy an uninitialized proxy + const tx = await proxyFactory.deployUUPSProxy([]); + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated + ); + const proxy = Build1Factory.attach(event.args.proxy); // Check the clone before initialization expect( @@ -54,27 +49,52 @@ describe('PluginUUPSUpgradeable', function () { expect(await proxy.state1()).to.equal(0); // Initialize the clone - const dummyDaoAddress = plugin.address; - await proxy.initialize(dummyDaoAddress); + await proxy.initialize(daoMock.address); // Check the clone after initialization expect( await getOzInitializedSlotValue(ethers.provider, proxy.address) ).to.equal(1); - expect(await proxy.dao()).to.equal(dummyDaoAddress); + expect(await proxy.dao()).to.equal(daoMock.address); expect(await proxy.state1()).to.equal(1); }); + + it('disables initializers for the implementation', async () => { + const {implementation} = await loadFixture(fixture); + + // Check that the implementation is uninitialized. + expect(await implementation.dao()).to.equal(ethers.constants.AddressZero); + expect(await implementation.state1()).to.equal(0); + + // Check that the implementation initialization is disabled. + expect( + await getOzInitializedSlotValue(ethers.provider, implementation.address) + ).to.equal(255); + }); + + it('reverts if an function tries to call `__PluginUUPSUpgradeable_init` without being an initializer', async () => { + const {deployer, daoMock} = await loadFixture(fixture); + + const badPlugin = await new PluginUUPSUpgradeableMockBad__factory( + deployer + ).deploy(); + await expect( + badPlugin.notAnInitializer(daoMock.address) + ).to.be.revertedWith('Initializable: contract is not initializing'); + }); }); describe('PluginType', async () => { it('returns the current protocol version', async () => { - expect(await plugin.pluginType()).to.equal(PluginType.UUPS); + const {implementation} = await loadFixture(fixture); + expect(await implementation.pluginType()).to.equal(PluginType.UUPS); }); }); describe('ProtocolVersion', async () => { it('returns the current protocol version matching the semantic version of the `osx-contracts-commons` package', async () => { - expect(await plugin.protocolVersion()).to.deep.equal( + const {implementation} = await loadFixture(fixture); + expect(await implementation.protocolVersion()).to.deep.equal( osxCommonsContractsVersion() ); }); @@ -82,28 +102,36 @@ describe('PluginUUPSUpgradeable', function () { describe('ERC-165', async () => { it('supports the `ERC-165` standard', async () => { - await erc165ComplianceTests(plugin, deployer); + const {deployer, implementation} = await loadFixture(fixture); + await erc165ComplianceTests(implementation, deployer); }); it('supports the `IPlugin` interface', async () => { + const {implementation} = await loadFixture(fixture); const iface = IPlugin__factory.createInterface(); - expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; + expect(await implementation.supportsInterface(getInterfaceId(iface))).to + .be.true; }); it('supports the `IProtocolVersion` interface', async () => { + const {implementation} = await loadFixture(fixture); const iface = IProtocolVersion__factory.createInterface(); - expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; + expect(await implementation.supportsInterface(getInterfaceId(iface))).to + .be.true; }); it('supports the `IERC1822ProxiableUpgradeable` interface', async () => { + const {implementation} = await loadFixture(fixture); const iface = IERC1822ProxiableUpgradeable__factory.createInterface(); - expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; + expect(await implementation.supportsInterface(getInterfaceId(iface))).to + .be.true; }); }); describe('Protocol version', async () => { it('returns the current protocol version', async () => { - expect(await plugin.protocolVersion()).to.deep.equal( + const {implementation} = await loadFixture(fixture); + expect(await implementation.protocolVersion()).to.deep.equal( osxCommonsContractsVersion() ); }); @@ -111,41 +139,38 @@ describe('PluginUUPSUpgradeable', function () { describe('Upgradeability', async () => { it('returns the implementation', async () => { - const dummyDaoAddress = ethers.constants.AddressZero; - - const predeployedImplementation = await upgrades.deployImplementation( - Build1Factory + const {proxyFactory, Build1Factory, implementation} = await loadFixture( + fixture ); - const proxy = await upgrades.deployProxy( - Build1Factory, - [dummyDaoAddress], - { - useDeployedImplementation: true, - kind: 'uups', - initializer: 'initialize', - unsafeAllow: ['constructor'], - constructorArgs: [], - } + // Deploy an uninitialized build 1 proxy + const tx = await proxyFactory.deployUUPSProxy([]); + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated ); + const proxy = Build1Factory.attach(event.args.proxy); - expect(await proxy.implementation()).to.equal(predeployedImplementation); + expect(await proxy.implementation()).to.equal(implementation.address); }); it('reverts the upgrade if caller caller does not have the `UPGRADE_PLUGIN_PERMISSION_ID` permission on the plugin proxy', async () => { - const daoMock = await DAOMockFactory.deploy(); - - // Deploy a build 1 proxy - const proxy = await upgrades.deployProxy( + const { + deployer, + proxyFactory, + initCalldata, Build1Factory, - [daoMock.address], - { - kind: 'uups', - initializer: 'initialize', - unsafeAllow: ['constructor'], - constructorArgs: [], - } + Build2Factory, + daoMock, + } = await loadFixture(fixture); + + // Deploy an initialized build 1 proxy + const tx = await proxyFactory.deployUUPSProxy(initCalldata); + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated ); + const proxy = Build1Factory.attach(event.args.proxy); // Deploy the build 2 implementation const newImplementation = await Build2Factory.deploy(); @@ -165,19 +190,21 @@ describe('PluginUUPSUpgradeable', function () { }); it('upgrades if the caller has the `UPGRADE_PLUGIN_PERMISSION_ID` permission', async () => { - const daoMock = await DAOMockFactory.deploy(); - - // Create a build 1 proxy - const proxy = await upgrades.deployProxy( + const { + proxyFactory, + initCalldata, Build1Factory, - [daoMock.address], - { - kind: 'uups', - initializer: 'initialize', - unsafeAllow: ['constructor'], - constructorArgs: [], - } + Build2Factory, + daoMock, + } = await loadFixture(fixture); + + // Create an initialized build 1 proxy + const tx = await proxyFactory.deployUUPSProxy(initCalldata); + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated ); + const proxy = Build1Factory.attach(event.args.proxy); // Deploy the build 2 implementation const newImplementation = await Build2Factory.deploy(); @@ -198,19 +225,21 @@ describe('PluginUUPSUpgradeable', function () { }); it('can be reinitialzed', async () => { - const daoMock = await DAOMockFactory.deploy(); - - // Deploy a build 1 proxy - const proxy = await upgrades.deployProxy( + const { + proxyFactory, + initCalldata, Build1Factory, - [daoMock.address], - { - kind: 'uups', - initializer: 'initialize', - unsafeAllow: ['constructor'], - constructorArgs: [], - } + Build2Factory, + daoMock, + } = await loadFixture(fixture); + + // Deploy an initialized build 1 proxy + const tx = await proxyFactory.deployUUPSProxy(initCalldata); + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated ); + const proxy = Build1Factory.attach(event.args.proxy); // Deploy the build 2 implementation const newImplementation = await Build2Factory.deploy(); @@ -240,3 +269,42 @@ describe('PluginUUPSUpgradeable', function () { }); }); }); + +type FixtureResult = { + deployer: SignerWithAddress; + implementation: PluginUUPSUpgradeableMockBuild1; + proxyFactory: ProxyFactory; + daoMock: DAOMock; + initCalldata: string; + Build1Factory: PluginUUPSUpgradeableMockBuild1__factory; + Build2Factory: PluginUUPSUpgradeableMockBuild2__factory; +}; + +async function fixture(): Promise { + const [deployer] = await ethers.getSigners(); + + const Build1Factory = new PluginUUPSUpgradeableMockBuild1__factory(deployer); + const Build2Factory = new PluginUUPSUpgradeableMockBuild2__factory(deployer); + const daoMock = await new DAOMock__factory(deployer).deploy(); + + const implementation = await Build1Factory.deploy(); + + const proxyFactory = await new ProxyFactory__factory(deployer).deploy( + implementation.address + ); + + const initCalldata = implementation.interface.encodeFunctionData( + 'initialize', + [daoMock.address] + ); + + return { + deployer, + implementation, + proxyFactory, + initCalldata, + daoMock, + Build1Factory, + Build2Factory, + }; +} diff --git a/contracts/test/plugin/plugin.ts b/contracts/test/plugin/plugin.ts index 591028bb..d5d8342f 100644 --- a/contracts/test/plugin/plugin.ts +++ b/contracts/test/plugin/plugin.ts @@ -1,4 +1,6 @@ import { + DAOMock, + DAOMock__factory, IPlugin__factory, IProtocolVersion__factory, PluginMockBuild1, @@ -7,29 +9,29 @@ import { import {erc165ComplianceTests} from '../helpers'; import {osxCommonsContractsVersion} from '../utils/versioning/protocol-version'; import {getInterfaceId, PluginType} from '@aragon/osx-commons-sdk'; +import {loadFixture} from '@nomicfoundation/hardhat-network-helpers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; import {ethers} from 'hardhat'; -describe('Plugin', function () { - let deployer: SignerWithAddress; - let plugin: PluginMockBuild1; - - before(async () => { - [deployer] = await ethers.getSigners(); - plugin = await new PluginMockBuild1__factory(deployer).deploy( - ethers.constants.AddressZero - ); - }); +type FixtureResult = { + deployer: SignerWithAddress; + plugin: PluginMockBuild1; + daoMock: DAOMock; + Build1Factory: PluginMockBuild1__factory; +}; +describe('Plugin', function () { describe('PluginType', async () => { it('returns the current protocol version', async () => { + const {plugin} = await loadFixture(fixture); expect(await plugin.pluginType()).to.equal(PluginType.Constructable); }); }); describe('ProtocolVersion', async () => { it('returns the current protocol version matching the semantic version of the `osx-contracts-commons` package', async () => { + const {plugin} = await loadFixture(fixture); expect(await plugin.protocolVersion()).to.deep.equal( osxCommonsContractsVersion() ); @@ -38,21 +40,25 @@ describe('Plugin', function () { describe('ERC-165', async () => { it('supports the `ERC-165` standard', async () => { + const {deployer, plugin} = await loadFixture(fixture); await erc165ComplianceTests(plugin, deployer); }); it('supports the `IPlugin` interface', async () => { + const {plugin} = await loadFixture(fixture); const iface = IPlugin__factory.createInterface(); expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; }); it('supports the `IProtocolVersion` interface', async () => { + const {plugin} = await loadFixture(fixture); const iface = IProtocolVersion__factory.createInterface(); expect(await plugin.supportsInterface(getInterfaceId(iface))).to.be.true; }); }); describe('Protocol version', async () => { + const {plugin} = await loadFixture(fixture); it('returns the current protocol version', async () => { expect(await plugin.protocolVersion()).to.deep.equal( osxCommonsContractsVersion() @@ -60,3 +66,19 @@ describe('Plugin', function () { }); }); }); + +async function fixture(): Promise { + const [deployer] = await ethers.getSigners(); + + const Build1Factory = new PluginMockBuild1__factory(deployer); + const daoMock = await new DAOMock__factory(deployer).deploy(); + + const plugin = await Build1Factory.deploy(daoMock.address); + + return { + deployer, + plugin, + daoMock, + Build1Factory, + }; +} diff --git a/contracts/test/plugin/setup/plugin-setup.ts b/contracts/test/plugin/setup/plugin-setup.ts index 0f8eef9c..8c2abead 100644 --- a/contracts/test/plugin/setup/plugin-setup.ts +++ b/contracts/test/plugin/setup/plugin-setup.ts @@ -1,13 +1,22 @@ import { IPluginSetup__factory, + IPlugin__factory, IProtocolVersion__factory, - PluginSetupMockBuild1, - PluginSetupMockBuild1__factory, + PluginCloneableSetupMockBuild1, + PluginCloneableSetupMockBuild1__factory, + PluginUUPSUpgradeableSetupMockBuild1, + PluginUUPSUpgradeableSetupMockBuild1__factory, + PluginUUPSUpgradeableSetupMockBuild2__factory, } from '../../../typechain'; +import {IPluginSetup} from '../../../typechain/src/plugin/setup/PluginSetup'; import {erc165ComplianceTests} from '../../helpers'; import {osxCommonsContractsVersion} from '../../utils/versioning/protocol-version'; -import {getInterfaceId} from '@aragon/osx-commons-sdk'; -import {IPluginSetup__factory as IPluginSetup_V1_0_0__factory} from '@aragon/osx-ethers-v1.0.0'; +import {ADDRESS, getInterfaceId} from '@aragon/osx-commons-sdk'; +import { + IPluginSetup__factory as IPluginSetup_V1_0_0__factory, + Plugin__factory, +} from '@aragon/osx-ethers-v1.0.0'; +import {loadFixture} from '@nomicfoundation/hardhat-network-helpers'; import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; import {expect} from 'chai'; import {ethers} from 'hardhat'; @@ -23,42 +32,157 @@ describe('IPluginSetup', function () { }); describe('PluginSetup', async () => { - let deployer: SignerWithAddress; - let pluginSetup: PluginSetupMockBuild1; + pluginSetupBaseTests(pluginSetupFixture); + + describe('prepareUpdate', async () => { + it('reverts when called', async () => { + const {pluginSetupMock} = await loadFixture(pluginSetupFixture); + + const dummyDaoAddr = ADDRESS.ONE; + const dummyPluginAddr = ADDRESS.TWO; + const dummyFromBuildNumber = 123; + const setupPayload: IPluginSetup.SetupPayloadStruct = { + plugin: dummyPluginAddr, + currentHelpers: [], + data: [], + }; + + await expect( + pluginSetupMock.prepareUpdate( + dummyDaoAddr, + dummyFromBuildNumber, + setupPayload + ) + ) + .to.be.revertedWithCustomError(pluginSetupMock, 'NonUpgradeablePlugin') + .withArgs(); + }); + }); +}); + +describe('PluginUpgradeableSetup', async () => { + pluginSetupBaseTests(pluginUUPSUpgradeableSetupFixture); + + describe('prepareUpdate', async () => { + it('should revert when called on the initial build 1', async () => { + const {pluginSetupMock: setupBuild1} = await loadFixture( + pluginUUPSUpgradeableSetupFixture + ); + + const dummyDaoAddr = ADDRESS.ONE; + const dummyPluginAddr = ADDRESS.TWO; + const setupPayload: IPluginSetup.SetupPayloadStruct = { + plugin: dummyPluginAddr, + currentHelpers: [], + data: [], + }; + + const fromBuildNumber = 0; + const thisBuildNumber = 1; + + await expect( + setupBuild1.prepareUpdate(dummyDaoAddr, fromBuildNumber, setupPayload) + ) + .to.be.revertedWithCustomError( + setupBuild1 as PluginUUPSUpgradeableSetupMockBuild1, + 'InvalidUpdatePath' + ) + .withArgs(fromBuildNumber, thisBuildNumber); + }); + + it('can be called on builds that are not the initial build', async () => { + const {deployer} = await loadFixture(pluginSetupFixture); + const setupBuild2 = + await new PluginUUPSUpgradeableSetupMockBuild2__factory( + deployer + ).deploy(); - before(async () => { - [deployer] = await ethers.getSigners(); - pluginSetup = await new PluginSetupMockBuild1__factory(deployer).deploy(); + const dummyDaoAddr = ADDRESS.ONE; + const dummyPluginAddr = ADDRESS.TWO; + const dummyFromBuildNumber = 123; + const setupPayload: IPluginSetup.SetupPayloadStruct = { + plugin: dummyPluginAddr, + currentHelpers: [], + data: [], + }; + + await expect( + setupBuild2.prepareUpdate( + dummyDaoAddr, + dummyFromBuildNumber, + setupPayload + ) + ).to.be.not.reverted; + }); + }); +}); + +function pluginSetupBaseTests(fixture: () => Promise) { + it('returns the implementation contract', async () => { + const {deployer, pluginSetupMock} = await loadFixture(fixture); + + const implementation = await pluginSetupMock.implementation(); + + // Check that an address is returned + expect(implementation).to.not.equal(ADDRESS.ZERO); + + // Check that it supports the `IPlugin` interface + const plugin = Plugin__factory.connect(implementation, deployer); + const IPluginInterfaceId = getInterfaceId( + IPlugin__factory.createInterface() + ); + expect(await plugin.supportsInterface(IPluginInterfaceId)); }); describe('ProtocolVersion', async () => { it('returns the current protocol version matching the semantic version of the `osx-contracts-commons` package', async () => { - expect(await pluginSetup.protocolVersion()).to.deep.equal( + const {pluginSetupMock} = await loadFixture(fixture); + expect(await pluginSetupMock.protocolVersion()).to.deep.equal( osxCommonsContractsVersion() ); }); }); - it.skip('creates ERC1967 proxies', async () => { - // TODO this will likely be refactored with task OS-675 - expect(true).to.equal(false); - }); - describe('ERC-165', async () => { it('supports the `ERC-165` standard', async () => { - await erc165ComplianceTests(pluginSetup, deployer); + const {deployer, pluginSetupMock} = await loadFixture(fixture); + await erc165ComplianceTests(pluginSetupMock, deployer); }); it('supports the `IPluginSetup` interface', async () => { + const {pluginSetupMock} = await loadFixture(fixture); const iface = IPluginSetup__factory.createInterface(); - expect(await pluginSetup.supportsInterface(getInterfaceId(iface))).to.be - .true; + expect(await pluginSetupMock.supportsInterface(getInterfaceId(iface))).to + .be.true; }); it('supports the `IProtocolVersion` interface', async () => { + const {pluginSetupMock} = await loadFixture(fixture); const iface = IProtocolVersion__factory.createInterface(); - expect(await pluginSetup.supportsInterface(getInterfaceId(iface))).to.be - .true; + expect(await pluginSetupMock.supportsInterface(getInterfaceId(iface))).to + .be.true; }); }); -}); +} + +type FixtureResult = { + deployer: SignerWithAddress; + pluginSetupMock: + | PluginCloneableSetupMockBuild1 + | PluginUUPSUpgradeableSetupMockBuild1; +}; + +async function pluginSetupFixture(): Promise { + const [deployer] = await ethers.getSigners(); + const pluginSetupMock = await new PluginCloneableSetupMockBuild1__factory( + deployer + ).deploy(); + return {deployer, pluginSetupMock}; +} + +async function pluginUUPSUpgradeableSetupFixture(): Promise { + const [deployer] = await ethers.getSigners(); + const pluginSetupMock = + await new PluginUUPSUpgradeableSetupMockBuild1__factory(deployer).deploy(); + return {deployer, pluginSetupMock}; +} diff --git a/contracts/test/utils/deployment/clone-factory.ts b/contracts/test/utils/deployment/clone-factory.ts deleted file mode 100644 index 653ef3d2..00000000 --- a/contracts/test/utils/deployment/clone-factory.ts +++ /dev/null @@ -1 +0,0 @@ -// TODO CloneFactory will be refactored as part of Task OS-794. diff --git a/contracts/test/utils/deployment/proxy-lib.ts b/contracts/test/utils/deployment/proxy-lib.ts new file mode 100644 index 00000000..8c690d6c --- /dev/null +++ b/contracts/test/utils/deployment/proxy-lib.ts @@ -0,0 +1,179 @@ +import { + PluginCloneableMockBuild1, + PluginCloneableMockBuild1__factory, + PluginUUPSUpgradeableMockBuild1, + PluginUUPSUpgradeableMockBuild1__factory, + ProxyFactory, + ProxyFactory__factory, +} from '../../../typechain'; +import {ProxyCreatedEvent} from '../../../typechain/src/utils/deployment/ProxyFactory'; +import { + ADDRESS, + findEvent, + PROXY_FACTORY_EVENTS, +} from '@aragon/osx-commons-sdk'; +import {loadFixture} from '@nomicfoundation/hardhat-network-helpers'; +import {SignerWithAddress} from '@nomiclabs/hardhat-ethers/signers'; +import {expect} from 'chai'; +import {ethers} from 'hardhat'; + +describe('ProxyFactory', function () { + describe('deployUUPSProxy', function () { + it('deploys an initialized proxy if initialization data is provided', async () => { + const { + deployer, + proxyFactory, + implementation, + initCalldata, + daoMockAddr, + } = await loadFixture(uupsProxyFixture); + + // Deploy the proxy with initialization data + const tx = await proxyFactory.deployUUPSProxy(initCalldata); + + // Get the proxy address from the event + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated + ); + const proxy = PluginUUPSUpgradeableMockBuild1__factory.connect( + event.args.proxy, + deployer + ); + + // Check that the proxy and proxy factory both point to the implementation contract. + expect(await proxy.implementation()) + .to.equal(await proxyFactory.implementation()) + .to.equal(implementation.address); + + // Check that the proxy is initialized + expect(await proxy.dao()).to.equal(daoMockAddr); + expect(await proxy.state1()).to.equal(1); + }); + + it('deploys an uninitialized proxy if no initialization data is provided', async () => { + const {deployer, proxyFactory, implementation} = await loadFixture( + uupsProxyFixture + ); + + // Deploy the proxy without initialization data + const tx = await proxyFactory.deployUUPSProxy([]); + + // Get the proxy address from the event + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated + ); + const proxy = PluginUUPSUpgradeableMockBuild1__factory.connect( + event.args.proxy, + deployer + ); + + // Check that the proxy and proxy factory both point to the implementation contract. + expect(await proxy.implementation()) + .to.equal(await proxyFactory.implementation()) + .to.equal(implementation.address); + + // Check that the proxy is not initialized + expect(await proxy.dao()).to.equal(ADDRESS.ZERO); + expect(await proxy.state1()).to.equal(0); + }); + }); + + describe('deployMinimalProxy', function () { + it('deploys an initialized proxy if initialization data is provided', async () => { + const {deployer, proxyFactory, initCalldata, daoMockAddr} = + await loadFixture(minimalProxyFixture); + + // Deploy the proxy with initialization data + const tx = await proxyFactory.deployMinimalProxy(initCalldata); + + // Get the proxy address from the event + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated + ); + const proxy = PluginCloneableMockBuild1__factory.connect( + event.args.proxy, + deployer + ); + + // Check that the proxy is initialized + expect(await proxy.dao()).to.equal(daoMockAddr); + expect(await proxy.state1()).to.equal(1); + }); + + it('deploys an uninitialized proxy if no initialization data is provided', async () => { + const {deployer, proxyFactory} = await loadFixture(minimalProxyFixture); + + // Deploy the proxy without initialization data + const tx = await proxyFactory.deployMinimalProxy([]); + + // Get the proxy address from the event + const event = await findEvent( + tx, + PROXY_FACTORY_EVENTS.ProxyCreated + ); + const proxy = PluginCloneableMockBuild1__factory.connect( + event.args.proxy, + deployer + ); + + // Check that the proxy is not initialized + expect(await proxy.dao()).to.equal(ADDRESS.ZERO); + expect(await proxy.state1()).to.equal(0); + }); + }); +}); + +type FixtureResult = { + deployer: SignerWithAddress; + implementation: PluginUUPSUpgradeableMockBuild1 | PluginCloneableMockBuild1; + proxyFactory: ProxyFactory; + daoMockAddr: string; + initCalldata: string; +}; + +async function uupsProxyFixture(): Promise { + const [deployer] = await ethers.getSigners(); + + const implementation = await new PluginUUPSUpgradeableMockBuild1__factory( + deployer + ).deploy(); + + const proxyFactory = await new ProxyFactory__factory(deployer).deploy( + implementation.address + ); + + // Create a mock address with a valid checksum + const daoMockAddr = ethers.utils.getAddress(ADDRESS.LAST); + + const initCalldata = implementation.interface.encodeFunctionData( + 'initialize', + [daoMockAddr] + ); + + return {deployer, implementation, proxyFactory, daoMockAddr, initCalldata}; +} + +async function minimalProxyFixture(): Promise { + const [deployer] = await ethers.getSigners(); + + const implementation = await new PluginCloneableMockBuild1__factory( + deployer + ).deploy(); + + const proxyFactory = await new ProxyFactory__factory(deployer).deploy( + implementation.address + ); + + // Create a mock address with a valid checksum + const daoMockAddr = ethers.utils.getAddress(ADDRESS.LAST); + + const initCalldata = implementation.interface.encodeFunctionData( + 'initialize', + [daoMockAddr] + ); + + return {deployer, implementation, proxyFactory, daoMockAddr, initCalldata}; +} diff --git a/contracts/yarn.lock b/contracts/yarn.lock index f47cde12..f3ab164b 100644 --- a/contracts/yarn.lock +++ b/contracts/yarn.lock @@ -2,10 +2,10 @@ # yarn lockfile v1 -"@aragon/osx-commons-sdk@0.0.1-alpha.3": - version "0.0.1-alpha.3" - resolved "https://registry.yarnpkg.com/@aragon/osx-commons-sdk/-/osx-commons-sdk-0.0.1-alpha.3.tgz#c8ca71702cc404f7ad82cc327fb2355807a84801" - integrity sha512-7ibaTyNouYiPOQuMYFvfonl4Ts/87XPh2XerrqkdwQAEcdRGEkQjmwAd4XO4cHXC8yJh8u9JCTt604muuRJehA== +"@aragon/osx-commons-sdk@0.0.1-alpha.5": + version "0.0.1-alpha.5" + resolved "https://registry.yarnpkg.com/@aragon/osx-commons-sdk/-/osx-commons-sdk-0.0.1-alpha.5.tgz#9808e7c6a441af6459f96016abe07812706d97bf" + integrity sha512-GyErC61lMckZyG17BZPgPWMkY3phZGdzZHMP17mUyE6vFhth9at5HKhNBi7bWB0eNkcM9RrQksIWHjmSHbW3ug== dependencies: "@aragon/osx-ethers" "^1.3.0-rc0.4" "@aragon/osx-ethers-v1.0.0" "npm:@aragon/osx-ethers@1.2.1" diff --git a/sdk/package.json b/sdk/package.json index 8fda9ebf..34af4411 100644 --- a/sdk/package.json +++ b/sdk/package.json @@ -1,7 +1,7 @@ { "name": "@aragon/osx-commons-sdk", "author": "Aragon Association", - "version": "0.0.1-alpha.4", + "version": "0.0.1-alpha.5", "license": "MIT", "main": "dist/index.js", "module": "dist/osx-commons-sdk.esm.js", diff --git a/sdk/src/events.ts b/sdk/src/events.ts index 17fa2eef..f43d5b07 100644 --- a/sdk/src/events.ts +++ b/sdk/src/events.ts @@ -49,6 +49,10 @@ export const CALLBACK_HANDLER_EVENTS = { CallbackReceived: 'CallbackReceived', }; +export const PROXY_FACTORY_EVENTS = { + ProxyCreated: 'ProxyCreated', +}; + export const DAO_EVENTS = { NewURI: 'NewURI', };