From c5d8db675827a1df85247d6ede5100bcdeeaf59b Mon Sep 17 00:00:00 2001 From: Giorgi Lagidze Date: Sun, 29 Sep 2024 08:34:21 +0400 Subject: [PATCH] fix tests and typechain + loop (#613) * fix tests and typechain + loop * remove comments * add back vscode files * interface fixed --- .../scripts/generate-typechain-osx.ts | 7 +++++- packages/contracts/src/core/dao/DAO.sol | 23 +++++++++++++++---- packages/contracts/test/core/dao/dao.ts | 3 +-- .../plugin/plugin-setup-processor.ts | 8 ++++--- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/packages/contracts/scripts/generate-typechain-osx.ts b/packages/contracts/scripts/generate-typechain-osx.ts index 40bb38b2b..1678fce80 100644 --- a/packages/contracts/scripts/generate-typechain-osx.ts +++ b/packages/contracts/scripts/generate-typechain-osx.ts @@ -28,7 +28,12 @@ async function generateTypechain(): Promise { } const dirPath = path.dirname(filePath); - if (!excludedDirs.has(dirPath)) { + + // Only arrange the contracts' paths in the current + // directory without previous versions. + const containsPrefix = Array.from(excludedDirs).some(prefix => dirPath.startsWith(prefix)); + + if (!containsPrefix) { jsonFiles.push(filePath); } }); diff --git a/packages/contracts/src/core/dao/DAO.sol b/packages/contracts/src/core/dao/DAO.sol index ec0ff99d2..4d8d1235b 100644 --- a/packages/contracts/src/core/dao/DAO.sol +++ b/packages/contracts/src/core/dao/DAO.sol @@ -120,6 +120,9 @@ contract DAO is /// @notice Thrown when a function is removed but left to not corrupt the interface ID. error FunctionRemoved(); + /// @notice Thrown when initialize is called after it has already been executed. + error AlreadyInitialized(); + /// @notice Emitted when a new DAO URI is set. /// @param daoURI The new URI. event NewURI(string daoURI); @@ -137,6 +140,15 @@ contract DAO is _reentrancyStatus = _NOT_ENTERED; } + /// @notice This ensures that the initialize function cannot be called during the upgrade process. + modifier onlyCallAtInitialization() { + if (_getInitializedVersion() != 0) { + revert AlreadyInitialized(); + } + + _; + } + /// @notice Disables the initializers on the implementation contract to prevent it from being left uninitialized. constructor() { _disableInitializers(); @@ -157,9 +169,12 @@ contract DAO is address _initialOwner, address _trustedForwarder, string calldata daoURI_ - ) external reinitializer(3) { + ) external onlyCallAtInitialization reinitializer(3) { _reentrancyStatus = _NOT_ENTERED; // added in v1.3.0 + // In addition to the current interfaceId, also support previous version of the interfaceId. + _registerInterface(type(IDAO).interfaceId ^ IExecutor.execute.selector); + _registerInterface(type(IDAO).interfaceId); _registerInterface(type(IExecutor).interfaceId); _registerInterface(type(IERC1271).interfaceId); @@ -202,6 +217,7 @@ contract DAO is _permissionId: keccak256("SET_SIGNATURE_VALIDATOR_PERMISSION") }); + _registerInterface(type(IDAO).interfaceId); _registerInterface(type(IExecutor).interfaceId); } } @@ -274,8 +290,7 @@ contract DAO is msg.data ); - for (uint256 i = 0; i < _actions.length; i++) { - // TODO: Merge this loop with the below one. + for (uint256 i = 0; i < _actions.length; ) { Action calldata action = _actions[i]; bool isAllowed = hasExecutePermission; @@ -298,9 +313,7 @@ contract DAO is if (!isAllowed) { revert Unauthorized(action.to, msg.sender, permissionId); } - } - for (uint256 i = 0; i < _actions.length; ) { bool success; bytes memory data; diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts index 81d24a70c..84636e971 100644 --- a/packages/contracts/test/core/dao/dao.ts +++ b/packages/contracts/test/core/dao/dao.ts @@ -147,7 +147,7 @@ describe('DAO', function () { dummyAddress1, daoExampleURI ) - ).to.be.revertedWith('Initializable: contract is already initialized'); + ).to.be.revertedWithCustomError(dao, 'AlreadyInitialized'); }); it('initializes with the correct trusted forwarder', async () => { @@ -449,7 +449,6 @@ describe('DAO', function () { it('supports the `IDAO` interface', async () => { const iface = IDAO__factory.createInterface(); - expect(getInterfaceId(iface)).to.equal('0x9385547e'); // the interfaceID from IDAO v1.0.0 expect(await dao.supportsInterface(getInterfaceId(iface))).to.be.true; }); diff --git a/packages/contracts/test/framework/plugin/plugin-setup-processor.ts b/packages/contracts/test/framework/plugin/plugin-setup-processor.ts index d241940e5..c68fa1b23 100644 --- a/packages/contracts/test/framework/plugin/plugin-setup-processor.ts +++ b/packages/contracts/test/framework/plugin/plugin-setup-processor.ts @@ -109,6 +109,8 @@ const EMPTY_DATA = '0x'; const ADDRESS_TWO = `0x${'00'.repeat(19)}02`; +const APPLY_TARGET_PERMISSION_ID = ethers.utils.id('APPLY_TARGET_PERMISSION'); + describe('PluginSetupProcessor', function () { let signers: SignerWithAddress[]; let psp: PluginSetupProcessor; @@ -579,7 +581,7 @@ describe('PluginSetupProcessor', function () { .withArgs( targetDao.address, psp.address, - '0xf40ef0af1ef5603c5d084dc17659d339ea90c8ff31545c1ffcadc8207aefa8af' + APPLY_TARGET_PERMISSION_ID ); }); @@ -1165,7 +1167,7 @@ describe('PluginSetupProcessor', function () { .withArgs( targetDao.address, psp.address, - '0xf40ef0af1ef5603c5d084dc17659d339ea90c8ff31545c1ffcadc8207aefa8af' + APPLY_TARGET_PERMISSION_ID ); }); @@ -1827,7 +1829,7 @@ describe('PluginSetupProcessor', function () { .withArgs( targetDao.address, psp.address, - '0xf40ef0af1ef5603c5d084dc17659d339ea90c8ff31545c1ffcadc8207aefa8af' + APPLY_TARGET_PERMISSION_ID ); });