From 6a804fd794c10cd57362be12d557ca7ef3aee849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Wed, 6 Nov 2024 15:11:51 +0700 Subject: [PATCH] Signer List tests WIP --- README.md | 1 - TEST_TREE.md | 3 +- src/EncryptionRegistry.sol | 10 +- src/SignerList.sol | 11 +- test/SignerListTree.t.sol | 472 ++++++++++++++++++++++++++++++++++-- test/SignerListTree.t.yaml | 14 +- test/helpers/DaoBuilder.sol | 15 +- 7 files changed, 483 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 2e85300..14c8cdb 100644 --- a/README.md +++ b/README.md @@ -306,7 +306,6 @@ Then use `make` to automatically sync the described branches into solidity test ```sh $ make Available targets: -Available targets: - make all Builds all tree files and updates the test tree markdown - make sync Scaffold or sync tree files into solidity tests - make check Checks if solidity files are out of sync diff --git a/TEST_TREE.md b/TEST_TREE.md index 0bd36cc..9be0ba0 100644 --- a/TEST_TREE.md +++ b/TEST_TREE.md @@ -451,6 +451,7 @@ SignerListTest │ ├── Given duplicate addresses on initialize │ │ └── It should revert │ ├── It should append the new addresses to the list +│ ├── It should return true on isListed │ ├── It should emit the SignersAddedEvent │ ├── When encryptionRegistry is not compatible on initialize │ │ └── It should revert @@ -482,6 +483,7 @@ SignerListTest │ ├── Given duplicate addresses on updateSettings │ │ └── It should revert │ ├── It should append the new addresses to the list +│ ├── It should return true on isListed │ └── It should emit the SignersAddedEvent ├── When calling removeSigners │ ├── When removing without the permission @@ -543,4 +545,3 @@ SignerListTest ├── It result does not contain unlisted addresses └── It result does not contain non appointed addresses ``` - diff --git a/src/EncryptionRegistry.sol b/src/EncryptionRegistry.sol index 4454ba8..abdf7fb 100644 --- a/src/EncryptionRegistry.sol +++ b/src/EncryptionRegistry.sol @@ -4,13 +4,14 @@ pragma solidity ^0.8.17; import {Addresslist} from "@aragon/osx/plugins/utils/Addresslist.sol"; import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; +import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {IEncryptionRegistry} from "./interfaces/IEncryptionRegistry.sol"; /// @title EncryptionRegistry - Release 1, Build 1 /// @author Aragon Association - 2024 /// @notice A smart contract where accounts can register their libsodium public key for encryption purposes, as well as appointing an EOA -contract EncryptionRegistry is IEncryptionRegistry { +contract EncryptionRegistry is IEncryptionRegistry, ERC165 { struct AccountEntry { address appointedWallet; bytes32 publicKey; @@ -110,6 +111,13 @@ contract EncryptionRegistry is IEncryptionRegistry { return _member; } + /// @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(IEncryptionRegistry).interfaceId || super.supportsInterface(_interfaceId); + } + // Internal helpers function _setPublicKey(address _account, bytes32 _publicKey) internal { diff --git a/src/SignerList.sol b/src/SignerList.sol index 228ec02..d909d3b 100644 --- a/src/SignerList.sol +++ b/src/SignerList.sol @@ -129,8 +129,11 @@ contract SignerList is ISignerList, Addresslist, ERC165Upgradeable, DaoAuthoriza function resolveEncryptionOwner(address _address) public view returns (address owner) { (bool ownerIsListed, bool isAppointed) = resolveEncryptionAccountStatus(_address); - if (!ownerIsListed) return address(0); - else if (isAppointed) return settings.encryptionRegistry.appointedBy(_address); + if (!ownerIsListed) { + return address(0); + } else if (isAppointed) { + return settings.encryptionRegistry.appointedBy(_address); + } return _address; } @@ -212,9 +215,7 @@ contract SignerList is ISignerList, Addresslist, ERC165Upgradeable, DaoAuthoriza && _newSettings.minSignerListLength == settings.minSignerListLength ) { return; - } else if ( - !IERC165(address(_newSettings.encryptionRegistry)).supportsInterface(type(IEncryptionRegistry).interfaceId) - ) { + } else if (!_newSettings.encryptionRegistry.supportsInterface(type(IEncryptionRegistry).interfaceId)) { revert InvalidEncryptionRegitry(address(_newSettings.encryptionRegistry)); } diff --git a/test/SignerListTree.t.sol b/test/SignerListTree.t.sol index 5a8b575..ca53fdc 100644 --- a/test/SignerListTree.t.sol +++ b/test/SignerListTree.t.sol @@ -1,17 +1,78 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.17; -import {Test} from "forge-std/Test.sol"; +import {AragonTest} from "./base/AragonTest.sol"; +import {Addresslist} from "@aragon/osx/plugins/utils/Addresslist.sol"; +import {EncryptionRegistry} from "../src/EncryptionRegistry.sol"; +import { + SignerList, + ISignerList, + UPDATE_SIGNER_LIST_PERMISSION_ID, + UPDATE_SIGNER_LIST_SETTINGS_PERMISSION_ID +} from "../src/SignerList.sol"; +import {DaoBuilder} from "./helpers/DaoBuilder.sol"; +import {DAO} from "@aragon/osx/core/dao/DAO.sol"; +import {Multisig} from "../src/Multisig.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; +import {IERC165Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/introspection/IERC165Upgradeable.sol"; +import {createProxyAndCall} from "../src/helpers/proxy.sol"; + +contract SignerListTest is AragonTest { + SignerList signerList; + EncryptionRegistry encryptionRegistry; + DaoBuilder builder; + DAO dao; + Multisig multisig; + address[] signers; + + address immutable SIGNER_LIST_BASE = address(new SignerList()); + + // Events/errors to be tested here (duplicate) + error SignerListLengthOutOfBounds(uint16 limit, uint256 actual); + error InvalidEncryptionRegitry(address givenAddress); + error DaoUnauthorized(address dao, address where, address who, bytes32 permissionId); + + event SignerListSettingsUpdated(EncryptionRegistry encryptionRegistry, uint16 minSignerListLength); + event SignersAdded(address[] signers); + event SignersRemoved(address[] signers); + + function setUp() public { + vm.startPrank(alice); + + builder = new DaoBuilder(); + (dao,, multisig,,, signerList, encryptionRegistry,) = builder.withMultisigMember(alice).withMultisigMember(bob) + .withMultisigMember(carol).withMultisigMember(david).build(); + + signers = new address[](4); + signers[0] = alice; + signers[1] = bob; + signers[2] = carol; + signers[3] = david; + } -contract SignerListTest is Test { function test_WhenDeployingTheContract() external { // It should initialize normally - vm.skip(true); + encryptionRegistry = EncryptionRegistry(address(0)); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 0))) + ) + ); } function test_GivenADeployedContract() external { // It should refuse to initialize again - vm.skip(true); + encryptionRegistry = EncryptionRegistry(address(0)); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 0))) + ) + ); + + vm.expectRevert(bytes("Initializable: contract is already initialized")); + signerList.initialize(dao, signers, SignerList.Settings(EncryptionRegistry(address(0)), 0)); } modifier givenANewInstance() { @@ -23,13 +84,119 @@ contract SignerListTest is Test { } function test_GivenCallingInitialize() external givenANewInstance givenCallingInitialize { + encryptionRegistry = EncryptionRegistry(address(0)); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 0))) + ) + ); + // It should set the DAO address + vm.assertEq(address(signerList.dao()), address(dao), "Incorrect DAO addres"); + + (EncryptionRegistry reg, uint16 minSignerListLength) = signerList.settings(); + // It should append the new addresses to the list - // It should emit the SignersAddedEvent + // It should return true on isListed + vm.assertEq(signerList.addresslistLength(), 4, "Incorrect length"); + vm.assertEq(signerList.isListed(alice), true, "Should be a signer"); + vm.assertEq(signerList.isListed(bob), true, "Should be a signer"); + vm.assertEq(signerList.isListed(carol), true, "Should be a signer"); + vm.assertEq(signerList.isListed(david), true, "Should be a signer"); + vm.assertEq(signerList.isListed(address(100)), false, "Should not be a signer"); + vm.assertEq(signerList.isListed(address(200)), false, "Should not be a signer"); + // It sets the new encryption registry + vm.assertEq(address(reg), address(encryptionRegistry), "Incorrect address"); + // It sets the new minSignerListLength + vm.assertEq(minSignerListLength, 0); + + // It should emit the SignersAdded event + + vm.expectEmit(); + emit SignersAdded({signers: signers}); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 0))) + ) + ); + // It should emit a SignerListSettingsUpdated event - vm.skip(true); + + vm.expectEmit(); + emit SignerListSettingsUpdated({encryptionRegistry: encryptionRegistry, minSignerListLength: 0}); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 0))) + ) + ); + + // It should set the right values in general + + // 2 + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 0))) + ) + ); + + (reg, minSignerListLength) = signerList.settings(); + vm.assertEq(address(reg), address(encryptionRegistry), "Incorrect address"); + vm.assertEq(minSignerListLength, 0); + vm.assertEq(signerList.addresslistLength(), 4, "Incorrect length"); + vm.assertEq(signerList.isListed(alice), true, "Should be a signer"); + vm.assertEq(signerList.isListed(bob), true, "Should be a signer"); + vm.assertEq(signerList.isListed(carol), true, "Should be a signer"); + vm.assertEq(signerList.isListed(david), true, "Should be a signer"); + vm.assertEq(signerList.isListed(address(100)), false, "Should not be a signer"); + vm.assertEq(signerList.isListed(address(200)), false, "Should not be a signer"); + + // 3 + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 2))) + ) + ); + + (reg, minSignerListLength) = signerList.settings(); + vm.assertEq(address(reg), address(encryptionRegistry), "Incorrect address"); + vm.assertEq(minSignerListLength, 2); + vm.assertEq(signerList.addresslistLength(), 4, "Incorrect length"); + vm.assertEq(signerList.isListed(alice), true, "Should be a signer"); + vm.assertEq(signerList.isListed(bob), true, "Should be a signer"); + vm.assertEq(signerList.isListed(carol), true, "Should be a signer"); + vm.assertEq(signerList.isListed(david), true, "Should be a signer"); + vm.assertEq(signerList.isListed(address(100)), false, "Should not be a signer"); + vm.assertEq(signerList.isListed(address(200)), false, "Should not be a signer"); + + // 4 + signers = new address[](2); + signers[0] = address(100); + signers[0] = address(200); + + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 1))) + ) + ); + + (reg, minSignerListLength) = signerList.settings(); + vm.assertEq(address(reg), address(encryptionRegistry), "Incorrect address"); + vm.assertEq(minSignerListLength, 1); + vm.assertEq(signerList.addresslistLength(), 2, "Incorrect length"); + vm.assertEq(signerList.isListed(alice), false, "Should not be a signer"); + vm.assertEq(signerList.isListed(bob), false, "Should not be a signer"); + vm.assertEq(signerList.isListed(carol), false, "Should not be a signer"); + vm.assertEq(signerList.isListed(david), false, "Should not be a signer"); + vm.assertEq(signerList.isListed(address(100)), true, "Should be a signer"); + vm.assertEq(signerList.isListed(address(200)), true, "Should be a signer"); } function test_RevertGiven_PassingMoreAddressesThanSupportedOnInitialize() @@ -38,12 +205,43 @@ contract SignerListTest is Test { givenCallingInitialize { // It should revert - vm.skip(true); + + // 1 + signers = new address[](type(uint16).max + 1); + + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 0))) + ) + ); + + // 2 + signers = new address[](type(uint16).max + 10); + + vm.expectRevert( + abi.encodeWithSelector(SignerListLengthOutOfBounds.selector, type(uint16).max, type(uint16).max + 10) + ); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 0))) + ) + ); } function test_RevertGiven_DuplicateAddressesOnInitialize() external givenANewInstance givenCallingInitialize { // It should revert - vm.skip(true); + + // 1 + signers[2] = signers[1]; + vm.expectRevert(); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 2))) + ) + ); } function test_RevertWhen_EncryptionRegistryIsNotCompatibleOnInitialize() @@ -52,53 +250,197 @@ contract SignerListTest is Test { givenCallingInitialize { // It should revert - vm.skip(true); - } - function test_RevertWhen_MinSignerListLengthIsLowerThanTheListSizeOnInitialize() + // 1 + vm.expectRevert(InvalidEncryptionRegitry.selector); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall( + SignerList.initialize, (dao, signers, SignerList.Settings(EncryptionRegistry(address(alice)), 2)) + ) + ) + ); + + // 2 + vm.expectRevert(InvalidEncryptionRegitry.selector); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall( + SignerList.initialize, (dao, signers, SignerList.Settings(EncryptionRegistry(address(bob)), 3)) + ) + ) + ); + + // OK + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall( + SignerList.initialize, + (dao, signers, SignerList.Settings(EncryptionRegistry(encryptionRegistry), 2)) + ) + ) + ); + } + + function test_RevertWhen_MinSignerListLengthIsBiggerThanTheListSizeOnInitialize() external givenANewInstance givenCallingInitialize { // It should revert - vm.skip(true); + + // 1 + vm.expectRevert(abi.encodeWithSelector(SignerListLengthOutOfBounds.selector, 4, 5)); + + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall( + SignerList.initialize, (dao, signers, SignerList.Settings(EncryptionRegistry(address(0)), 5)) + ) + ) + ); + + // 2 + vm.expectRevert(abi.encodeWithSelector(SignerListLengthOutOfBounds.selector, 4, 10)); + + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall( + SignerList.initialize, (dao, signers, SignerList.Settings(EncryptionRegistry(address(0)), 10)) + ) + ) + ); + + // 3 + signers = new address[](5); + vm.expectRevert(abi.encodeWithSelector(SignerListLengthOutOfBounds.selector, 5, 15)); + + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall( + SignerList.initialize, (dao, signers, SignerList.Settings(EncryptionRegistry(address(0)), 15)) + ) + ) + ); } modifier whenCallingUpdateSettings() { + // Initialize + encryptionRegistry = EncryptionRegistry(address(0)); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall(SignerList.initialize, (dao, signers, SignerList.Settings(encryptionRegistry, 0))) + ) + ); + _; } function test_WhenCallingUpdateSettings() external whenCallingUpdateSettings { - // It set the new encryption registry - // It set the new minSignerListLength + encryptionRegistry = new EncryptionRegistry(Addresslist(address(0))); + + // 1 + signerList.updateSettings(SignerList.Settings(encryptionRegistry, 0)); + + (EncryptionRegistry reg, uint16 minSignerListLength) = signerList.settings(); + + // It sets the new encryption registry + vm.assertEq(address(reg), address(encryptionRegistry), "Incorrect encryptionRegistry"); + + // It sets the new minSignerListLength + vm.assertEq(minSignerListLength, 0); + // It should emit a SignerListSettingsUpdated event - vm.skip(true); + vm.expectEmit(); + emit SignerListSettingsUpdated({encryptionRegistry: encryptionRegistry, minSignerListLength: 0}); + signerList.updateSettings(SignerList.Settings(encryptionRegistry, 0)); + + // 2 + encryptionRegistry = new EncryptionRegistry(Addresslist(address(0))); + signerList.updateSettings(SignerList.Settings(encryptionRegistry, 3)); + + (reg, minSignerListLength) = signerList.settings(); + + // It sets the new encryption registry + vm.assertEq(address(reg), address(encryptionRegistry), "Incorrect encryptionRegistry"); + + // It sets the new minSignerListLength + vm.assertEq(minSignerListLength, 3); + + // It should emit a SignerListSettingsUpdated event + vm.expectEmit(); + emit SignerListSettingsUpdated({encryptionRegistry: encryptionRegistry, minSignerListLength: 4}); + signerList.updateSettings(SignerList.Settings(encryptionRegistry, 4)); } function test_RevertWhen_UpdateSettingsWithoutThePermission() external whenCallingUpdateSettings { // It should revert - vm.skip(true); + + encryptionRegistry = EncryptionRegistry(address(0)); + + vm.startPrank(bob); + vm.expectRevert( + abi.encodeWithSelector( + DaoUnauthorized.selector, + address(dao), + address(signerList), + alice, + UPDATE_SIGNER_LIST_SETTINGS_PERMISSION_ID + ) + ); + signerList.updateSettings(SignerList.Settings(encryptionRegistry, 0)); } function test_RevertWhen_EncryptionRegistryIsNotCompatibleOnUpdateSettings() external whenCallingUpdateSettings { // It should revert - vm.skip(true); + + vm.expectRevert(InvalidEncryptionRegitry.selector); + signerList.updateSettings(SignerList.Settings(EncryptionRegistry(address(alice)), 0)); } - function test_RevertWhen_MinSignerListLengthIsLowerThanTheListSizeOnUpdateSettings() + function test_RevertWhen_MinSignerListLengthIsBiggerThanTheListSizeOnUpdateSettings() external whenCallingUpdateSettings { // It should revert - vm.skip(true); + + // 1 + vm.expectRevert(abi.encodeWithSelector(SignerListLengthOutOfBounds.selector, 4, 15)); + signerList.updateSettings(SignerList.Settings(EncryptionRegistry(address(0)), 15)); + + // 2 + vm.expectRevert(abi.encodeWithSelector(SignerListLengthOutOfBounds.selector, 4, 20)); + signerList.updateSettings(SignerList.Settings(EncryptionRegistry(address(0)), 20)); + + // 3 + signers = new address[](5); + vm.expectRevert(abi.encodeWithSelector(SignerListLengthOutOfBounds.selector, 5, 50)); + signerList.updateSettings(SignerList.Settings(EncryptionRegistry(address(0)), 50)); } - function test_WhenCallingSupportsInterface() external { + function test_WhenCallingSupportsInterface() external view { // It does not support the empty interface + bool supported = multisig.supportsInterface(bytes4(0xffffffff)); + assertEq(supported, true, "Should not support the empty interface"); + // It supports IERC165Upgradeable + supported = multisig.supportsInterface(type(IERC165Upgradeable).interfaceId); + assertEq(supported, true, "Should support IERC165Upgradeable"); + // It supports Addresslist + supported = multisig.supportsInterface(type(Addresslist).interfaceId); + assertEq(supported, true, "Should support Addresslist"); + // It supports ISignerList - vm.skip(true); + supported = multisig.supportsInterface(type(ISignerList).interfaceId); + assertEq(supported, true, "Should support ISignerList"); } modifier whenCallingAddSigners() { @@ -107,23 +449,101 @@ contract SignerListTest is Test { function test_WhenCallingAddSigners() external whenCallingAddSigners { // It should append the new addresses to the list - // It should emit the SignersAddedEvent - vm.skip(true); + // It should return true on isListed + + // 0 + vm.assertEq(signerList.addresslistLength(), 4, "Incorrect length"); + vm.assertEq(signerList.isListed(alice), true, "Should be a signer"); + vm.assertEq(signerList.isListed(bob), true, "Should be a signer"); + vm.assertEq(signerList.isListed(carol), true, "Should be a signer"); + vm.assertEq(signerList.isListed(david), true, "Should be a signer"); + vm.assertEq(signerList.isListed(address(100)), false, "Should not be a signer"); + vm.assertEq(signerList.isListed(address(200)), false, "Should not be a signer"); + + // 1 + address[] memory newSigners = new address[](1); + newSigners[0] = address(100); + signerList.addSigners(newSigners); + + vm.assertEq(signerList.addresslistLength(), 4, "Incorrect length"); + vm.assertEq(signerList.isListed(alice), true, "Should be a signer"); + vm.assertEq(signerList.isListed(bob), true, "Should be a signer"); + vm.assertEq(signerList.isListed(carol), true, "Should be a signer"); + vm.assertEq(signerList.isListed(david), true, "Should be a signer"); + vm.assertEq(signerList.isListed(address(100)), true, "Should be a signer"); + vm.assertEq(signerList.isListed(address(200)), false, "Should not be a signer"); + + // 2 + newSigners[0] = address(200); + signerList.addSigners(newSigners); + + vm.assertEq(signerList.addresslistLength(), 4, "Incorrect length"); + vm.assertEq(signerList.isListed(alice), true, "Should be a signer"); + vm.assertEq(signerList.isListed(bob), true, "Should be a signer"); + vm.assertEq(signerList.isListed(carol), true, "Should be a signer"); + vm.assertEq(signerList.isListed(david), true, "Should be a signer"); + vm.assertEq(signerList.isListed(address(100)), true, "Should be a signer"); + vm.assertEq(signerList.isListed(address(200)), true, "Should be a signer"); + + // It should emit the SignersAdded event + newSigners[0] = address(300); + vm.expectEmit(); + emit SignersAdded({signers: newSigners}); + signerList.addSigners(newSigners); } function test_RevertWhen_AddingWithoutThePermission() external whenCallingAddSigners { // It should revert - vm.skip(true); + + address[] memory newSigners = new address[](1); + newSigners[0] = address(100); + + vm.startPrank(bob); + + vm.expectRevert( + abi.encodeWithSelector( + DaoUnauthorized.selector, address(dao), address(signerList), alice, UPDATE_SIGNER_LIST_PERMISSION_ID + ) + ); + signerList.addSigners(newSigners); } function test_RevertGiven_PassingMoreAddressesThanSupportedOnUpdateSettings() external whenCallingAddSigners { // It should revert - vm.skip(true); + + // 1 + address[] memory newSigners = new address[](type(uint8).max + 1); + vm.expectRevert( + abi.encodeWithSelector(SignerListLengthOutOfBounds.selector, type(uint16).max, type(uint16).max + 1) + ); + signerList.addSigners(newSigners); + + // 2 + newSigners = new address[](type(uint8).max + 10); + vm.expectRevert( + abi.encodeWithSelector(SignerListLengthOutOfBounds.selector, type(uint16).max, type(uint16).max + 10) + ); + signerList.addSigners(newSigners); } function test_RevertGiven_DuplicateAddressesOnUpdateSettings() external whenCallingAddSigners { // It should revert - vm.skip(true); + + // 1 + address[] memory newSigners = new address[](1); + newSigners[0] = alice; // Alice is a signer already + vm.expectRevert(); + signerList.addSigners(newSigners); + + // 2 + newSigners[0] = bob; // Bob is a signer already + vm.expectRevert(); + signerList.addSigners(newSigners); + + // OK + newSigners[0] = address(1234); + vm.expectRevert(); + signerList.addSigners(newSigners); } modifier whenCallingRemoveSigners() { diff --git a/test/SignerListTree.t.yaml b/test/SignerListTree.t.yaml index 298c4ef..14acfa1 100644 --- a/test/SignerListTree.t.yaml +++ b/test/SignerListTree.t.yaml @@ -19,12 +19,13 @@ SignerListTest: then: - it: should revert - it: should append the new addresses to the list - - it: should emit the SignersAddedEvent + - it: should return true on isListed + - it: should emit the SignersAdded event # Same checks as updateSettings below - when: encryptionRegistry is not compatible [on initialize] then: - it: should revert - - when: minSignerListLength is lower than the list size [on initialize] + - when: minSignerListLength is bigger than the list size [on initialize] then: - it: should revert - it: sets the new encryption registry @@ -40,11 +41,11 @@ SignerListTest: - when: encryptionRegistry is not compatible [on updateSettings] then: - it: should revert - - when: minSignerListLength is lower than the list size [on updateSettings] + - when: minSignerListLength is bigger than the list size [on updateSettings] then: - it: should revert - - it: set the new encryption registry - - it: set the new minSignerListLength + - it: sets the new encryption registry + - it: sets the new minSignerListLength - it: should emit a SignerListSettingsUpdated event - when: calling supportsInterface @@ -67,7 +68,8 @@ SignerListTest: then: - it: should revert - it: should append the new addresses to the list - - it: should emit the SignersAddedEvent + - it: should return true on isListed + - it: should emit the SignersAdded event - when: calling removeSigners and: diff --git a/test/helpers/DaoBuilder.sol b/test/helpers/DaoBuilder.sol index f1896e1..af55966 100644 --- a/test/helpers/DaoBuilder.sol +++ b/test/helpers/DaoBuilder.sol @@ -6,7 +6,7 @@ import {DAO} from "@aragon/osx/core/dao/DAO.sol"; import {Multisig} from "../../src/Multisig.sol"; import {EmergencyMultisig} from "../../src/EmergencyMultisig.sol"; import {OptimisticTokenVotingPlugin} from "../../src/OptimisticTokenVotingPlugin.sol"; -import {SignerList} from "../../src/SignerList.sol"; +import {SignerList, UPDATE_SIGNER_LIST_SETTINGS_PERMISSION_ID} from "../../src/SignerList.sol"; import {EncryptionRegistry} from "../../src/EncryptionRegistry.sol"; import {createProxyAndCall} from "../../src/helpers/proxy.sol"; import {RATIO_BASE} from "@aragon/osx/plugins/utils/Ratio.sol"; @@ -20,6 +20,7 @@ contract DaoBuilder is Test { address immutable MULTISIG_BASE = address(new Multisig()); address immutable EMERGENCY_MULTISIG_BASE = address(new EmergencyMultisig()); address immutable OPTIMISTIC_BASE = address(new OptimisticTokenVotingPlugin()); + address immutable SIGNER_LIST_BASE = address(new SignerList()); enum TaikoL1Status { Standard, @@ -234,10 +235,18 @@ contract DaoBuilder is Test { signers[0] = owner; } - signerList = new SignerList(); - signerList.initialize(dao, signers, SignerList.Settings(EncryptionRegistry(address(0)), 0)); + signerList = SignerList( + createProxyAndCall( + address(SIGNER_LIST_BASE), + abi.encodeCall( + SignerList.initialize, (dao, signers, SignerList.Settings(EncryptionRegistry(address(0)), 0)) + ) + ) + ); encryptionRegistry = new EncryptionRegistry(signerList); + dao.grant(address(signerList), address(this), UPDATE_SIGNER_LIST_SETTINGS_PERMISSION_ID); signerList.updateSettings(SignerList.Settings(encryptionRegistry, uint16(signers.length))); + dao.revoke(address(signerList), address(this), UPDATE_SIGNER_LIST_SETTINGS_PERMISSION_ID); } // Standard multisig