From 69043627e1f0fb61a0dcca1ae42a481e9ac6c090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Tue, 21 May 2024 16:51:47 +0200 Subject: [PATCH] Adapting the Multisig tests --- src/Multisig.sol | 2 +- test/Multisig.t.sol | 1718 +++++++++++++++-------------------- test/helpers/DaoBuilder.sol | 5 + 3 files changed, 730 insertions(+), 995 deletions(-) diff --git a/src/Multisig.sol b/src/Multisig.sol index ee9638c..9064a9f 100644 --- a/src/Multisig.sol +++ b/src/Multisig.sol @@ -386,7 +386,7 @@ contract Multisig is IMultisig, IMembership, PluginUUPSUpgradeable, ProposalUpgr /// @return True if the proposal vote is open, false otherwise. function _isProposalOpen(Proposal storage proposal_) internal view returns (bool) { uint64 currentTimestamp64 = block.timestamp.toUint64(); - return !proposal_.executed && proposal_.parameters.expirationDate >= currentTimestamp64; + return !proposal_.executed && proposal_.parameters.expirationDate > currentTimestamp64; } /// @notice Internal function to update the plugin settings. diff --git a/test/Multisig.t.sol b/test/Multisig.t.sol index 58346de..4d3ddde 100644 --- a/test/Multisig.t.sol +++ b/test/Multisig.t.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.17; import {AragonTest} from "./base/AragonTest.sol"; +import {DaoBuilder} from "./helpers/DaoBuilder.sol"; import {StandardProposalCondition} from "../src/conditions/StandardProposalCondition.sol"; import {OptimisticTokenVotingPlugin} from "../src/OptimisticTokenVotingPlugin.sol"; import {Multisig} from "../src/Multisig.sol"; @@ -15,11 +16,13 @@ import {IMembership} from "@aragon/osx/core/plugin/membership/IMembership.sol"; import {Addresslist} from "@aragon/osx/plugins/utils/Addresslist.sol"; import {DaoUnauthorized} from "@aragon/osx/core/utils/auth.sol"; import {IERC165Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/introspection/IERC165Upgradeable.sol"; -import {createProxyAndCall} from "./helpers.sol"; +import {createProxyAndCall} from "./helpers/proxy.sol"; contract MultisigTest is AragonTest { + DaoBuilder builder; + DAO dao; - Multisig plugin; + Multisig multisig; OptimisticTokenVotingPlugin optimisticPlugin; // Events/errors to be tested here (duplicate) @@ -43,9 +46,11 @@ contract MultisigTest is AragonTest { event Upgraded(address indexed implementation); function setUp() public { - vm.startPrank(alice); + switchTo(alice); - (dao, plugin, optimisticPlugin) = makeDaoWithMultisigAndOptimistic(alice); + builder = new DaoBuilder(); + (dao, optimisticPlugin, multisig,,,) = builder.withMultisigMember(alice).withMultisigMember(bob) + .withMultisigMember(carol).withMultisigMember(david).withMinApprovals(3).build(); } function test_RevertsIfTryingToReinitialize() public { @@ -58,16 +63,16 @@ contract MultisigTest is AragonTest { signers[2] = carol; signers[3] = david; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); // Reinitialize should fail vm.expectRevert("Initializable: contract is already initialized"); - plugin.initialize(dao, signers, settings); + multisig.initialize(dao, signers, settings); } - function test_AddsInitialAddresses() public { + function test_InitializeAddsInitialAddresses() public { // Deploy with 4 signers Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 3, destinationProposalDuration: 4 days}); @@ -77,33 +82,39 @@ contract MultisigTest is AragonTest { signers[2] = carol; signers[3] = david; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - assertEq(plugin.isListed(alice), true, "Should be a member"); - assertEq(plugin.isListed(bob), true, "Should be a member"); - assertEq(plugin.isListed(carol), true, "Should be a member"); - assertEq(plugin.isListed(david), true, "Should be a member"); + assertEq(multisig.isListed(alice), true, "Should be a member"); + assertEq(multisig.isListed(bob), true, "Should be a member"); + assertEq(multisig.isListed(carol), true, "Should be a member"); + assertEq(multisig.isListed(david), true, "Should be a member"); + assertEq(multisig.isListed(randomWallet), false, "Should not be a member"); // Redeploy with just 2 signers - settings.minApprovals = 1; + builder = new DaoBuilder(); + (dao, optimisticPlugin, multisig,,,) = builder.withMultisigMember(alice).withMultisigMember(bob).build(); - signers = new address[](2); - signers[0] = alice; - signers[1] = bob; + assertEq(multisig.isListed(alice), true, "Should be a member"); + assertEq(multisig.isListed(bob), true, "Should be a member"); + assertEq(multisig.isListed(carol), false, "Should not be a member"); + assertEq(multisig.isListed(david), false, "Should not be a member"); + assertEq(multisig.isListed(randomWallet), false, "Should not be a member"); - plugin = Multisig( - createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) - ); + // Redeploy with 5 signers + builder = new DaoBuilder(); + (dao, optimisticPlugin, multisig,,,) = builder.withMultisigMember(alice).withMultisigMember(bob) + .withMultisigMember(carol).withMultisigMember(david).withMultisigMember(randomWallet).build(); - assertEq(plugin.isListed(alice), true, "Should be a member"); - assertEq(plugin.isListed(bob), true, "Should be a member"); - assertEq(plugin.isListed(carol), false, "Should not be a member"); - assertEq(plugin.isListed(david), false, "Should not be a member"); + assertEq(multisig.isListed(alice), true, "Should be a member"); + assertEq(multisig.isListed(bob), true, "Should be a member"); + assertEq(multisig.isListed(carol), true, "Should be a member"); + assertEq(multisig.isListed(david), true, "Should be a member"); + assertEq(multisig.isListed(randomWallet), true, "Should be a member"); } - function test_ShouldSetMinApprovals() public { + function test_InitializeSetsMinApprovals() public { // 2 Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 2, destinationProposalDuration: 4 days}); @@ -113,25 +124,25 @@ contract MultisigTest is AragonTest { signers[2] = carol; signers[3] = david; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - (, uint16 minApprovals,) = plugin.multisigSettings(); + (, uint16 minApprovals,) = multisig.multisigSettings(); assertEq(minApprovals, uint16(2), "Incorrect minApprovals"); // Redeploy with 1 settings.minApprovals = 1; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - (, minApprovals,) = plugin.multisigSettings(); + (, minApprovals,) = multisig.multisigSettings(); assertEq(minApprovals, uint16(1), "Incorrect minApprovals"); } - function test_ShouldSetOnlyListed() public { + function test_InitializeSetsOnlyListed() public { // Deploy with true Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 3, destinationProposalDuration: 4 days}); @@ -141,25 +152,25 @@ contract MultisigTest is AragonTest { signers[2] = carol; signers[3] = david; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - (bool onlyListed,,) = plugin.multisigSettings(); + (bool onlyListed,,) = multisig.multisigSettings(); assertEq(onlyListed, true, "Incorrect onlyListed"); // Redeploy with false settings.onlyListed = false; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - (onlyListed,,) = plugin.multisigSettings(); + (onlyListed,,) = multisig.multisigSettings(); assertEq(onlyListed, false, "Incorrect onlyListed"); } - function test_ShouldSetDestinationProposalDuration() public { + function test_InitializeSetsDestinationProposalDuration() public { // Deploy with 5 days Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 3, destinationProposalDuration: 5 days}); @@ -169,25 +180,25 @@ contract MultisigTest is AragonTest { signers[2] = carol; signers[3] = david; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - (,, uint64 minDuration) = plugin.multisigSettings(); + (,, uint64 minDuration) = multisig.multisigSettings(); assertEq(minDuration, 5 days, "Incorrect minDuration"); // Redeploy with 3 days settings.destinationProposalDuration = 3 days; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - (,, minDuration) = plugin.multisigSettings(); + (,, minDuration) = multisig.multisigSettings(); assertEq(minDuration, 3 days, "Incorrect minDuration"); } - function test_ShouldEmitMultisigSettingsUpdatedOnInstall() public { + function test_InitializeEmitsMultisigSettingsUpdatedOnInstall() public { // Deploy with true/3/2 Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 3, destinationProposalDuration: 4 days}); @@ -200,7 +211,7 @@ contract MultisigTest is AragonTest { vm.expectEmit(); emit MultisigSettingsUpdated(true, uint16(3), 4 days); - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); @@ -209,18 +220,18 @@ contract MultisigTest is AragonTest { vm.expectEmit(); emit MultisigSettingsUpdated(false, uint16(2), 7 days); - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); } - function test_ShouldRevertIfMembersListIsTooLong() public { + function test_InitializeRevertsIfMembersListIsTooLong() public { Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 3, destinationProposalDuration: 4 days}); address[] memory signers = new address[](65537); vm.expectRevert(abi.encodeWithSelector(Multisig.AddresslistLengthOutOfBounds.selector, 65535, 65537)); - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); } @@ -228,37 +239,37 @@ contract MultisigTest is AragonTest { // INTERFACES function test_DoesntSupportTheEmptyInterface() public view { - bool supported = plugin.supportsInterface(0); + bool supported = multisig.supportsInterface(0); assertEq(supported, false, "Should not support the empty interface"); } function test_SupportsIERC165Upgradeable() public view { - bool supported = plugin.supportsInterface(type(IERC165Upgradeable).interfaceId); + bool supported = multisig.supportsInterface(type(IERC165Upgradeable).interfaceId); assertEq(supported, true, "Should support IERC165Upgradeable"); } function test_SupportsIPlugin() public view { - bool supported = plugin.supportsInterface(type(IPlugin).interfaceId); + bool supported = multisig.supportsInterface(type(IPlugin).interfaceId); assertEq(supported, true, "Should support IPlugin"); } function test_SupportsIProposal() public view { - bool supported = plugin.supportsInterface(type(IProposal).interfaceId); + bool supported = multisig.supportsInterface(type(IProposal).interfaceId); assertEq(supported, true, "Should support IProposal"); } function test_SupportsIMembership() public view { - bool supported = plugin.supportsInterface(type(IMembership).interfaceId); + bool supported = multisig.supportsInterface(type(IMembership).interfaceId); assertEq(supported, true, "Should support IMembership"); } function test_SupportsAddresslist() public view { - bool supported = plugin.supportsInterface(type(Addresslist).interfaceId); + bool supported = multisig.supportsInterface(type(Addresslist).interfaceId); assertEq(supported, true, "Should support Addresslist"); } function test_SupportsIMultisig() public view { - bool supported = plugin.supportsInterface(type(IMultisig).interfaceId); + bool supported = multisig.supportsInterface(type(IMultisig).interfaceId); assertEq(supported, true, "Should support IMultisig"); } @@ -278,7 +289,7 @@ contract MultisigTest is AragonTest { vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 4, 5)); - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); @@ -289,7 +300,7 @@ contract MultisigTest is AragonTest { destinationProposalDuration: 4 days // Greater than 4 members below }); vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 4, 6)); - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); } @@ -305,20 +316,20 @@ contract MultisigTest is AragonTest { vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 1, 0)); - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); // Retry with onlyListed false settings = Multisig.MultisigSettings({onlyListed: false, minApprovals: 0, destinationProposalDuration: 4 days}); vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 1, 0)); - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); } - function test_ShouldEmitMultisigSettingsUpdated() public { - dao.grant(address(plugin), address(alice), plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + function test_EmitsMultisigSettingsUpdated() public { + dao.grant(address(multisig), address(alice), multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); // 1 Multisig.MultisigSettings memory settings = @@ -326,28 +337,28 @@ contract MultisigTest is AragonTest { vm.expectEmit(); emit MultisigSettingsUpdated(true, 1, 4 days); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); // 2 settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 2, destinationProposalDuration: 5 days}); vm.expectEmit(); emit MultisigSettingsUpdated(true, 2, 5 days); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); // 3 settings = Multisig.MultisigSettings({onlyListed: false, minApprovals: 3, destinationProposalDuration: 0}); vm.expectEmit(); emit MultisigSettingsUpdated(false, 3, 0); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); // 4 settings = Multisig.MultisigSettings({onlyListed: false, minApprovals: 4, destinationProposalDuration: 1 days}); vm.expectEmit(); emit MultisigSettingsUpdated(false, 4, 1 days); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); } function test_onlyWalletWithPermissionsCanUpdateSettings() public { @@ -357,19 +368,19 @@ contract MultisigTest is AragonTest { abi.encodeWithSelector( DaoUnauthorized.selector, address(dao), - address(plugin), + address(multisig), alice, - plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() + multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() ) ); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); // Retry with the permission - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); vm.expectEmit(); emit MultisigSettingsUpdated(true, 1, 3 days); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); } function test_IsMemberShouldReturnWhenApropriate() public { @@ -378,14 +389,14 @@ contract MultisigTest is AragonTest { address[] memory signers = new address[](1); signers[0] = alice; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - assertEq(plugin.isMember(alice), true, "Should be a member"); - assertEq(plugin.isMember(bob), false, "Should not be a member"); - assertEq(plugin.isMember(carol), false, "Should not be a member"); - assertEq(plugin.isMember(david), false, "Should not be a member"); + assertEq(multisig.isMember(alice), true, "Should be a member"); + assertEq(multisig.isMember(bob), false, "Should not be a member"); + assertEq(multisig.isMember(carol), false, "Should not be a member"); + assertEq(multisig.isMember(david), false, "Should not be a member"); // More members settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 1, destinationProposalDuration: 4 days}); @@ -394,14 +405,14 @@ contract MultisigTest is AragonTest { signers[1] = carol; signers[2] = david; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - assertEq(plugin.isMember(alice), false, "Should not be a member"); - assertEq(plugin.isMember(bob), true, "Should be a member"); - assertEq(plugin.isMember(carol), true, "Should be a member"); - assertEq(plugin.isMember(david), true, "Should be a member"); + assertEq(multisig.isMember(alice), false, "Should not be a member"); + assertEq(multisig.isMember(bob), true, "Should be a member"); + assertEq(multisig.isMember(carol), true, "Should be a member"); + assertEq(multisig.isMember(david), true, "Should be a member"); } function test_IsMemberIsListedShouldReturnTheSameValue() public { @@ -410,14 +421,14 @@ contract MultisigTest is AragonTest { address[] memory signers = new address[](1); signers[0] = alice; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - assertEq(plugin.isListed(alice), plugin.isMember(alice), "isMember isListed should be equal"); - assertEq(plugin.isListed(bob), plugin.isMember(bob), "isMember isListed should be equal"); - assertEq(plugin.isListed(carol), plugin.isMember(carol), "isMember isListed should be equal"); - assertEq(plugin.isListed(david), plugin.isMember(david), "isMember isListed should be equal"); + assertEq(multisig.isListed(alice), multisig.isMember(alice), "isMember isListed should be equal"); + assertEq(multisig.isListed(bob), multisig.isMember(bob), "isMember isListed should be equal"); + assertEq(multisig.isListed(carol), multisig.isMember(carol), "isMember isListed should be equal"); + assertEq(multisig.isListed(david), multisig.isMember(david), "isMember isListed should be equal"); // More members settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 1, destinationProposalDuration: 4 days}); @@ -426,14 +437,14 @@ contract MultisigTest is AragonTest { signers[1] = carol; signers[2] = david; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - assertEq(plugin.isListed(alice), plugin.isMember(alice), "isMember isListed should be equal"); - assertEq(plugin.isListed(bob), plugin.isMember(bob), "isMember isListed should be equal"); - assertEq(plugin.isListed(carol), plugin.isMember(carol), "isMember isListed should be equal"); - assertEq(plugin.isListed(david), plugin.isMember(david), "isMember isListed should be equal"); + assertEq(multisig.isListed(alice), multisig.isMember(alice), "isMember isListed should be equal"); + assertEq(multisig.isListed(bob), multisig.isMember(bob), "isMember isListed should be equal"); + assertEq(multisig.isListed(carol), multisig.isMember(carol), "isMember isListed should be equal"); + assertEq(multisig.isListed(david), multisig.isMember(david), "isMember isListed should be equal"); } function testFuzz_IsMemberIsFalseByDefault(uint256 _randomEntropy) public { @@ -441,31 +452,31 @@ contract MultisigTest is AragonTest { Multisig.MultisigSettings({onlyListed: true, minApprovals: 1, destinationProposalDuration: 4 days}); address[] memory signers = new address[](1); // 0x0... would be a member but the chance is negligible - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - assertEq(plugin.isListed(randomWallet), false, "Should be false"); + assertEq(multisig.isListed(randomWallet), false, "Should be false"); assertEq( - plugin.isListed(vm.addr(uint256(keccak256(abi.encodePacked(_randomEntropy))))), false, "Should be false" + multisig.isListed(vm.addr(uint256(keccak256(abi.encodePacked(_randomEntropy))))), false, "Should be false" ); } function test_AddsNewMembersAndEmits() public { - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); // No - assertEq(plugin.isMember(randomWallet), false, "Should not be a member"); + assertEq(multisig.isMember(randomWallet), false, "Should not be a member"); address[] memory addrs = new address[](1); addrs[0] = randomWallet; vm.expectEmit(); emit MembersAdded({members: addrs}); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // Yes - assertEq(plugin.isMember(randomWallet), true, "Should be a member"); + assertEq(multisig.isMember(randomWallet), true, "Should be a member"); // Next addrs = new address[](3); @@ -474,31 +485,31 @@ contract MultisigTest is AragonTest { addrs[2] = vm.addr(3456); // No - assertEq(plugin.isMember(addrs[0]), false, "Should not be a member"); - assertEq(plugin.isMember(addrs[1]), false, "Should not be a member"); - assertEq(plugin.isMember(addrs[2]), false, "Should not be a member"); + assertEq(multisig.isMember(addrs[0]), false, "Should not be a member"); + assertEq(multisig.isMember(addrs[1]), false, "Should not be a member"); + assertEq(multisig.isMember(addrs[2]), false, "Should not be a member"); vm.expectEmit(); emit MembersAdded({members: addrs}); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // Yes - assertEq(plugin.isMember(addrs[0]), true, "Should be a member"); - assertEq(plugin.isMember(addrs[1]), true, "Should be a member"); - assertEq(plugin.isMember(addrs[2]), true, "Should be a member"); + assertEq(multisig.isMember(addrs[0]), true, "Should be a member"); + assertEq(multisig.isMember(addrs[1]), true, "Should be a member"); + assertEq(multisig.isMember(addrs[2]), true, "Should be a member"); } function test_RemovesMembersAndEmits() public { - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 1, destinationProposalDuration: 4 days}); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); // Before - assertEq(plugin.isMember(alice), true, "Should be a member"); - assertEq(plugin.isMember(bob), true, "Should be a member"); - assertEq(plugin.isMember(carol), true, "Should be a member"); - assertEq(plugin.isMember(david), true, "Should be a member"); + assertEq(multisig.isMember(alice), true, "Should be a member"); + assertEq(multisig.isMember(bob), true, "Should be a member"); + assertEq(multisig.isMember(carol), true, "Should be a member"); + assertEq(multisig.isMember(david), true, "Should be a member"); address[] memory addrs = new address[](2); addrs[0] = alice; @@ -506,20 +517,20 @@ contract MultisigTest is AragonTest { vm.expectEmit(); emit MembersRemoved({members: addrs}); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // After - assertEq(plugin.isMember(alice), false, "Should not be a member"); - assertEq(plugin.isMember(bob), false, "Should not be a member"); - assertEq(plugin.isMember(carol), true, "Should be a member"); - assertEq(plugin.isMember(david), true, "Should be a member"); + assertEq(multisig.isMember(alice), false, "Should not be a member"); + assertEq(multisig.isMember(bob), false, "Should not be a member"); + assertEq(multisig.isMember(carol), true, "Should be a member"); + assertEq(multisig.isMember(david), true, "Should be a member"); // Next addrs = new address[](3); addrs[0] = vm.addr(1234); addrs[1] = vm.addr(2345); addrs[2] = vm.addr(3456); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // Remove addrs = new address[](2); @@ -528,122 +539,122 @@ contract MultisigTest is AragonTest { vm.expectEmit(); emit MembersRemoved({members: addrs}); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // Yes - assertEq(plugin.isMember(carol), false, "Should not be a member"); - assertEq(plugin.isMember(david), false, "Should not be a member"); + assertEq(multisig.isMember(carol), false, "Should not be a member"); + assertEq(multisig.isMember(david), false, "Should not be a member"); } function test_ShouldRevertIfEmptySignersList() public { - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 1, destinationProposalDuration: 4 days}); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); // Before - assertEq(plugin.isMember(alice), true, "Should be a member"); - assertEq(plugin.isMember(bob), true, "Should be a member"); - assertEq(plugin.isMember(carol), true, "Should be a member"); - assertEq(plugin.isMember(david), true, "Should be a member"); + assertEq(multisig.isMember(alice), true, "Should be a member"); + assertEq(multisig.isMember(bob), true, "Should be a member"); + assertEq(multisig.isMember(carol), true, "Should be a member"); + assertEq(multisig.isMember(david), true, "Should be a member"); // ok address[] memory addrs = new address[](1); addrs[0] = alice; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); addrs[0] = bob; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); addrs[0] = carol; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); - assertEq(plugin.isMember(alice), false, "Should not be a member"); - assertEq(plugin.isMember(bob), false, "Should not be a member"); - assertEq(plugin.isMember(carol), false, "Should not be a member"); - assertEq(plugin.isMember(david), true, "Should be a member"); + assertEq(multisig.isMember(alice), false, "Should not be a member"); + assertEq(multisig.isMember(bob), false, "Should not be a member"); + assertEq(multisig.isMember(carol), false, "Should not be a member"); + assertEq(multisig.isMember(david), true, "Should be a member"); // ko addrs[0] = david; vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 1, 0)); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // Next addrs = new address[](1); addrs[0] = vm.addr(1234); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // Retry removing David addrs = new address[](1); addrs[0] = david; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // Yes - assertEq(plugin.isMember(david), false, "Should not be a member"); + assertEq(multisig.isMember(david), false, "Should not be a member"); } function test_ShouldRevertIfLessThanMinApproval() public { - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); // Before - assertEq(plugin.isMember(alice), true, "Should be a member"); - assertEq(plugin.isMember(bob), true, "Should be a member"); - assertEq(plugin.isMember(carol), true, "Should be a member"); - assertEq(plugin.isMember(david), true, "Should be a member"); + assertEq(multisig.isMember(alice), true, "Should be a member"); + assertEq(multisig.isMember(bob), true, "Should be a member"); + assertEq(multisig.isMember(carol), true, "Should be a member"); + assertEq(multisig.isMember(david), true, "Should be a member"); // ok address[] memory addrs = new address[](1); addrs[0] = alice; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // ko addrs[0] = bob; vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 3, 2)); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // ko addrs[0] = carol; vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 3, 2)); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // ko addrs[0] = david; vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 3, 2)); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // Add and retry removing addrs = new address[](1); addrs[0] = vm.addr(1234); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); addrs = new address[](1); addrs[0] = bob; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // 2 addrs = new address[](1); addrs[0] = vm.addr(2345); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); addrs = new address[](1); addrs[0] = carol; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // 3 addrs = new address[](1); addrs[0] = vm.addr(3456); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); addrs = new address[](1); addrs[0] = david; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); } function test_MinApprovalsBiggerThanTheListReverts() public { // MinApprovals should be within the boundaries of the list - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({ onlyListed: true, @@ -651,16 +662,16 @@ contract MultisigTest is AragonTest { destinationProposalDuration: 4 days // More than 4 }); vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 4, 5)); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); // More signers address[] memory signers = new address[](1); signers[0] = randomWallet; - plugin.addAddresses(signers); + multisig.addAddresses(signers); // should not fail now - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); // More than that, should fail again settings = Multisig.MultisigSettings({ @@ -669,64 +680,64 @@ contract MultisigTest is AragonTest { destinationProposalDuration: 4 days // More than 5 }); vm.expectRevert(abi.encodeWithSelector(Multisig.MinApprovalsOutOfBounds.selector, 5, 6)); - plugin.updateMultisigSettings(settings); + multisig.updateMultisigSettings(settings); } function test_ShouldRevertIfDuplicatingAddresses() public { - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); // ok address[] memory addrs = new address[](1); addrs[0] = vm.addr(1234); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // ko vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // 1 addrs[0] = alice; vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // 2 addrs[0] = bob; vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // 3 addrs[0] = carol; vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // 4 addrs[0] = david; vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // ok addrs[0] = vm.addr(1234); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // ko vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); addrs[0] = vm.addr(2345); vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); addrs[0] = vm.addr(3456); vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); addrs[0] = vm.addr(4567); vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); addrs[0] = randomWallet; vm.expectRevert(abi.encodeWithSelector(InvalidAddresslistUpdate.selector, addrs[0])); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); } function test_onlyWalletWithPermissionsCanAddRemove() public { @@ -737,12 +748,12 @@ contract MultisigTest is AragonTest { abi.encodeWithSelector( DaoUnauthorized.selector, address(dao), - address(plugin), + address(multisig), alice, - plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() + multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() ) ); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); // ko addrs[0] = alice; @@ -750,108 +761,105 @@ contract MultisigTest is AragonTest { abi.encodeWithSelector( DaoUnauthorized.selector, address(dao), - address(plugin), + address(multisig), alice, - plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() + multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() ) ); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // Permission - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); // ok addrs[0] = vm.addr(1234); - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); addrs[0] = alice; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); } function testFuzz_PermissionedAddRemoveMembers(address randomAccount) public { - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); - assertEq(plugin.isMember(randomWallet), false, "Should be false"); + assertEq(multisig.isMember(randomWallet), false, "Should be false"); // in address[] memory addrs = new address[](1); addrs[0] = randomWallet; - plugin.addAddresses(addrs); - assertEq(plugin.isMember(randomWallet), true, "Should be true"); + multisig.addAddresses(addrs); + assertEq(multisig.isMember(randomWallet), true, "Should be true"); // out - plugin.removeAddresses(addrs); - assertEq(plugin.isMember(randomWallet), false, "Should be false"); + multisig.removeAddresses(addrs); + assertEq(multisig.isMember(randomWallet), false, "Should be false"); // someone else if (randomAccount != alice) { - vm.stopPrank(); - vm.startPrank(randomAccount); + switchTo(randomAccount); vm.expectRevert( abi.encodeWithSelector( DaoUnauthorized.selector, address(dao), - address(plugin), + address(multisig), randomAccount, - plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() + multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() ) ); - plugin.addAddresses(addrs); - assertEq(plugin.isMember(randomWallet), false, "Should be false"); + multisig.addAddresses(addrs); + assertEq(multisig.isMember(randomWallet), false, "Should be false"); addrs[0] = carol; - assertEq(plugin.isMember(carol), true, "Should be true"); + assertEq(multisig.isMember(carol), true, "Should be true"); vm.expectRevert( abi.encodeWithSelector( DaoUnauthorized.selector, address(dao), - address(plugin), + address(multisig), randomAccount, - plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() + multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() ) ); - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); - assertEq(plugin.isMember(carol), true, "Should be true"); + assertEq(multisig.isMember(carol), true, "Should be true"); } - vm.stopPrank(); - vm.startPrank(alice); + switchTo(alice); } function testFuzz_PermissionedUpdateSettings(address randomAccount) public { - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); - (bool onlyListed, uint16 minApprovals, uint64 destMinDuration) = plugin.multisigSettings(); + (bool onlyListed, uint16 minApprovals, uint64 destMinDuration) = multisig.multisigSettings(); assertEq(minApprovals, 3, "Should be 3"); assertEq(onlyListed, true, "Should be true"); - assertEq(destMinDuration, 4 days, "Incorrect destMinDuration"); + assertEq(destMinDuration, 10 days, "Incorrect destMinDuration A"); // in Multisig.MultisigSettings memory newSettings = Multisig.MultisigSettings({onlyListed: false, minApprovals: 2, destinationProposalDuration: 5 days}); - plugin.updateMultisigSettings(newSettings); + multisig.updateMultisigSettings(newSettings); - (onlyListed, minApprovals, destMinDuration) = plugin.multisigSettings(); + (onlyListed, minApprovals, destMinDuration) = multisig.multisigSettings(); assertEq(minApprovals, 2, "Should be 2"); assertEq(onlyListed, false, "Should be false"); - assertEq(destMinDuration, 5 days, "Incorrect destMinDuration"); + assertEq(destMinDuration, 5 days, "Incorrect destMinDuration B"); // out newSettings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 1, destinationProposalDuration: 6 days}); - plugin.updateMultisigSettings(newSettings); - (onlyListed, minApprovals, destMinDuration) = plugin.multisigSettings(); + multisig.updateMultisigSettings(newSettings); + (onlyListed, minApprovals, destMinDuration) = multisig.multisigSettings(); assertEq(minApprovals, 1, "Should be 1"); assertEq(onlyListed, true, "Should be true"); - assertEq(destMinDuration, 6 days, "Incorrect destMinDuration"); + assertEq(destMinDuration, 6 days, "Incorrect destMinDuration C"); blockForward(1); // someone else if (randomAccount != alice) { - vm.stopPrank(); - vm.startPrank(randomAccount); + switchTo(randomAccount); newSettings = Multisig.MultisigSettings({onlyListed: false, minApprovals: 4, destinationProposalDuration: 4 days}); @@ -860,67 +868,63 @@ contract MultisigTest is AragonTest { abi.encodeWithSelector( DaoUnauthorized.selector, address(dao), - address(plugin), + address(multisig), randomAccount, - plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() + multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID() ) ); - plugin.updateMultisigSettings(newSettings); + multisig.updateMultisigSettings(newSettings); - (onlyListed, minApprovals, destMinDuration) = plugin.multisigSettings(); + (onlyListed, minApprovals, destMinDuration) = multisig.multisigSettings(); assertEq(minApprovals, 1, "Should still be 1"); assertEq(onlyListed, true, "Should still be true"); assertEq(destMinDuration, 6 days, "Should still be 6 days"); } - vm.stopPrank(); - vm.startPrank(alice); + switchTo(alice); } // PROPOSAL CREATION function test_IncrementsTheProposalCounter() public { // increments the proposal counter - blockForward(1); - assertEq(plugin.proposalCount(), 0, "Should have no proposals"); + assertEq(multisig.proposalCount(), 0, "Should have no proposals"); // 1 IDAO.Action[] memory actions = new IDAO.Action[](0); - plugin.createProposal("", actions, optimisticPlugin, false); + multisig.createProposal("", actions, optimisticPlugin, false); - assertEq(plugin.proposalCount(), 1, "Should have 1 proposal"); + assertEq(multisig.proposalCount(), 1, "Should have 1 proposal"); // 2 - plugin.createProposal("ipfs://", actions, optimisticPlugin, true); + multisig.createProposal("ipfs://", actions, optimisticPlugin, true); - assertEq(plugin.proposalCount(), 2, "Should have 2 proposals"); + assertEq(multisig.proposalCount(), 2, "Should have 2 proposals"); } function test_CreatesAndReturnsUniqueProposalIds() public { // creates unique proposal IDs for each proposal - blockForward(1); // 1 IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); assertEq(pid, 0, "Should be 0"); // 2 - pid = plugin.createProposal("ipfs://", actions, optimisticPlugin, true); + pid = multisig.createProposal("ipfs://", actions, optimisticPlugin, true); assertEq(pid, 1, "Should be 1"); // 3 - pid = plugin.createProposal("ipfs://more", actions, optimisticPlugin, true); + pid = multisig.createProposal("ipfs://more", actions, optimisticPlugin, true); assertEq(pid, 2, "Should be 2"); } function test_EmitsProposalCreated() public { // emits the `ProposalCreated` event - blockForward(1); IDAO.Action[] memory actions = new IDAO.Action[](0); vm.expectEmit(); @@ -928,17 +932,15 @@ contract MultisigTest is AragonTest { proposalId: 0, creator: alice, metadata: "", - startDate: 0, - endDate: 0, + startDate: uint64(block.timestamp), + endDate: 10 days + 1, actions: actions, allowFailureMap: 0 }); - plugin.createProposal("", actions, optimisticPlugin, true); + multisig.createProposal("", actions, optimisticPlugin, true); // 2 - vm.stopPrank(); - vm.startPrank(bob); - blockForward(1); + switchTo(bob); actions = new IDAO.Action[](1); actions[0].to = carol; @@ -951,34 +953,17 @@ contract MultisigTest is AragonTest { proposalId: 1, creator: bob, metadata: "ipfs://", - startDate: 50 days, - endDate: 100 days, + startDate: uint64(block.timestamp), + endDate: 10 days + 1, actions: actions, allowFailureMap: 0 }); - plugin.createProposal("ipfs://", actions, optimisticPlugin, false); - - // undo - vm.stopPrank(); - vm.startPrank(alice); + multisig.createProposal("ipfs://", actions, optimisticPlugin, false); } function test_RevertsIfSettingsChangedInSameBlock() public { // reverts if the multisig settings have changed in the same block - // 1 - IDAO.Action[] memory actions = new IDAO.Action[](0); - vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalCreationForbidden.selector, alice)); - plugin.createProposal("", actions, optimisticPlugin, false); - - // Next block - blockForward(1); - plugin.createProposal("", actions, optimisticPlugin, false); - } - - function test_CreatesWhenUnlistedAccountsAllowed() public { - // creates a proposal when unlisted accounts are allowed - // Deploy a new multisig instance Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: false, minApprovals: 3, destinationProposalDuration: 4 days}); @@ -988,34 +973,40 @@ contract MultisigTest is AragonTest { signers[2] = carol; signers[3] = david; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings))) ); - vm.stopPrank(); - vm.startPrank(randomWallet); + // 1 + IDAO.Action[] memory actions = new IDAO.Action[](0); + vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalCreationForbidden.selector, alice)); + multisig.createProposal("", actions, optimisticPlugin, false); + + // Next block blockForward(1); + multisig.createProposal("", actions, optimisticPlugin, false); + } - IDAO.Action[] memory actions = new IDAO.Action[](0); - plugin.createProposal("", actions, optimisticPlugin, false); + function test_CreatesWhenUnlistedAccountsAllowed() public { + // creates a proposal when unlisted accounts are allowed + + builder = new DaoBuilder(); + (dao, optimisticPlugin, multisig,,,) = builder.withMultisigMember(alice).withoutOnlyListed().build(); - vm.stopPrank(); - vm.startPrank(alice); + switchTo(randomWallet); + + IDAO.Action[] memory actions = new IDAO.Action[](0); + multisig.createProposal("", actions, optimisticPlugin, false); } - function test_RevertsWhenOnlyListedAndAnotherWalletCreates() public { + function test_RevertsWhenOnlyListedAndTheWalletIsNotListed() public { // reverts if the user is not on the list and only listed accounts can create proposals - vm.stopPrank(); - vm.startPrank(randomWallet); - blockForward(1); + switchTo(randomWallet); IDAO.Action[] memory actions = new IDAO.Action[](0); vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalCreationForbidden.selector, randomWallet)); - plugin.createProposal("", actions, optimisticPlugin, false); - - vm.stopPrank(); - vm.startPrank(alice); + multisig.createProposal("", actions, optimisticPlugin, false); } function test_RevertsWhenCreatorWasListedBeforeButNotNow() public { @@ -1027,72 +1018,67 @@ contract MultisigTest is AragonTest { address[] memory addrs = new address[](1); addrs[0] = alice; - plugin = Multisig( + multisig = Multisig( createProxyAndCall(address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, addrs, settings))) ); - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); blockForward(1); // Add+remove addrs[0] = bob; - plugin.addAddresses(addrs); + multisig.addAddresses(addrs); addrs[0] = alice; - plugin.removeAddresses(addrs); + multisig.removeAddresses(addrs); // Alice cannot create now IDAO.Action[] memory actions = new IDAO.Action[](0); vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalCreationForbidden.selector, alice)); - plugin.createProposal("", actions, optimisticPlugin, false); + multisig.createProposal("", actions, optimisticPlugin, false); // Bob can create now - vm.stopPrank(); - vm.startPrank(bob); + switchTo(bob); - plugin.createProposal("", actions, optimisticPlugin, false); + multisig.createProposal("", actions, optimisticPlugin, false); - assertEq(plugin.isListed(alice), false, "Should not be listed"); - assertEq(plugin.isListed(bob), true, "Should be listed"); + assertEq(multisig.isListed(alice), false, "Should not be listed"); + assertEq(multisig.isListed(bob), true, "Should be listed"); } function test_CreatesProposalWithoutApprovingIfUnspecified() public { // creates a proposal successfully and does not approve if not specified - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal( + uint256 pid = multisig.createProposal( "", actions, optimisticPlugin, false // approveProposal ); - assertEq(plugin.hasApproved(pid, alice), false, "Should not have approved"); - (, uint16 approvals,,,,) = plugin.getProposal(pid); + assertEq(multisig.hasApproved(pid, alice), false, "Should not have approved"); + (, uint16 approvals,,,,) = multisig.getProposal(pid); assertEq(approvals, 0, "Should be 0"); - plugin.approve(pid, false); + multisig.approve(pid, false); - assertEq(plugin.hasApproved(pid, alice), true, "Should have approved"); - (, approvals,,,,) = plugin.getProposal(pid); + assertEq(multisig.hasApproved(pid, alice), true, "Should have approved"); + (, approvals,,,,) = multisig.getProposal(pid); assertEq(approvals, 1, "Should be 1"); } function test_CreatesAndApprovesWhenSpecified() public { // creates a proposal successfully and approves if specified - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal( + uint256 pid = multisig.createProposal( "", actions, optimisticPlugin, true // approveProposal ); - assertEq(plugin.hasApproved(pid, alice), true, "Should have approved"); - (, uint16 approvals,,,,) = plugin.getProposal(pid); + assertEq(multisig.hasApproved(pid, alice), true, "Should have approved"); + (, uint16 approvals,,,,) = multisig.getProposal(pid); assertEq(approvals, 1, "Should be 1"); } @@ -1102,13 +1088,13 @@ contract MultisigTest is AragonTest { // returns `false` if the approver is not listed { - // Deploy a new multisig instance + // Deploy a new multisig instance (more efficient than the builder for fuzz testing) Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: true, minApprovals: 1, destinationProposalDuration: 4 days}); address[] memory signers = new address[](1); signers[0] = alice; - plugin = Multisig( + multisig = Multisig( createProxyAndCall( address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings)) ) @@ -1117,183 +1103,149 @@ contract MultisigTest is AragonTest { } IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // ko if (_randomWallet != alice) { - assertEq(plugin.canApprove(pid, _randomWallet), false, "Should be false"); + assertEq(multisig.canApprove(pid, _randomWallet), false, "Should be false"); } // static ko - assertEq(plugin.canApprove(pid, randomWallet), false, "Should be false"); + assertEq(multisig.canApprove(pid, randomWallet), false, "Should be false"); // static ok - assertEq(plugin.canApprove(pid, alice), true, "Should be true"); + assertEq(multisig.canApprove(pid, alice), true, "Should be true"); } function test_CanApproveReturnsFalseIfApproved() public { // returns `false` if the approver has already approved - { - Multisig.MultisigSettings memory settings = - Multisig.MultisigSettings({onlyListed: true, minApprovals: 4, destinationProposalDuration: 4 days}); - address[] memory signers = new address[](4); - signers[0] = alice; - signers[1] = bob; - signers[2] = carol; - signers[3] = david; - plugin = Multisig( - createProxyAndCall( - address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings)) - ) - ); - } - - blockForward(1); + builder = new DaoBuilder(); + (dao, optimisticPlugin, multisig,,,) = builder.withMultisigMember(alice).withMultisigMember(bob) + .withMultisigMember(carol).withMultisigMember(david).withMinApprovals(4).build(); IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - assertEq(plugin.canApprove(pid, alice), true, "Should be true"); - plugin.approve(pid, false); - assertEq(plugin.canApprove(pid, alice), false, "Should be false"); + assertEq(multisig.canApprove(pid, alice), true, "Should be true"); + multisig.approve(pid, false); + assertEq(multisig.canApprove(pid, alice), false, "Should be false"); // Bob - assertEq(plugin.canApprove(pid, bob), true, "Should be true"); - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - assertEq(plugin.canApprove(pid, bob), false, "Should be false"); + assertEq(multisig.canApprove(pid, bob), true, "Should be true"); + switchTo(bob); + multisig.approve(pid, false); + assertEq(multisig.canApprove(pid, bob), false, "Should be false"); // Carol - assertEq(plugin.canApprove(pid, carol), true, "Should be true"); - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - assertEq(plugin.canApprove(pid, carol), false, "Should be false"); + assertEq(multisig.canApprove(pid, carol), true, "Should be true"); + switchTo(carol); + multisig.approve(pid, false); + assertEq(multisig.canApprove(pid, carol), false, "Should be false"); // David - assertEq(plugin.canApprove(pid, david), true, "Should be true"); - vm.stopPrank(); - vm.startPrank(david); - plugin.approve(pid, false); - assertEq(plugin.canApprove(pid, david), false, "Should be false"); - - vm.stopPrank(); - vm.startPrank(alice); + assertEq(multisig.canApprove(pid, david), true, "Should be true"); + switchTo(david); + multisig.approve(pid, false); + assertEq(multisig.canApprove(pid, david), false, "Should be false"); } function test_CanApproveReturnsFalseIfExpired() public { // returns `false` if the proposal has ended - blockForward(1); - vm.warp(0); // timestamp = 0 + uint64 startDate = 10; + setTime(startDate); IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); - assertEq(plugin.canApprove(pid, alice), true, "Should be true"); + (,, Multisig.ProposalParameters memory parameters,,,) = multisig.getProposal(pid); + assertEq(parameters.expirationDate, startDate + 10 days, "Incorrect expiration"); - vm.warp(10 days - 1); // multisig expiration time - 1 - assertEq(plugin.canApprove(pid, alice), true, "Should be true"); + assertEq(multisig.canApprove(pid, alice), true, "Should be true"); - vm.warp(10 days + 1); // multisig expiration time - assertEq(plugin.canApprove(pid, alice), false, "Should be false"); + setTime(startDate + 10 days - 1); // multisig expiration time - 1 + assertEq(multisig.canApprove(pid, alice), true, "Should be true"); + + setTime(startDate + 10 days); // multisig expiration time + assertEq(multisig.canApprove(pid, alice), false, "Should be false"); // Start later - vm.warp(1000); - pid = plugin.createProposal("", actions, optimisticPlugin, false); + startDate = 5 days; + setTime(startDate); + pid = multisig.createProposal("", actions, optimisticPlugin, false); - assertEq(plugin.canApprove(pid, alice), true, "Should be true"); + assertEq(multisig.canApprove(pid, alice), true, "Should be true"); - vm.warp(10 days + 1000); // expiration time - 1 - assertEq(plugin.canApprove(pid, alice), true, "Should be true"); + timeForward(10 days - 1); // expiration time - 1 + assertEq(multisig.canApprove(pid, alice), true, "Should be true"); - vm.warp(10 days + 1001); // expiration time - assertEq(plugin.canApprove(pid, alice), false, "Should be false"); + timeForward(1); // expiration time + assertEq(multisig.canApprove(pid, alice), false, "Should be false"); } function test_CanApproveReturnsFalseIfExecuted() public { // returns `false` if the proposal is already executed - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - - blockForward(1); + dao.grant(address(optimisticPlugin), address(multisig), optimisticPlugin.PROPOSER_PERMISSION_ID()); bool executed; IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, false); + multisig.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, true); // auto execute + switchTo(carol); + multisig.approve(pid, true); // auto execute - (executed,,,,,) = plugin.getProposal(pid); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, true, "Should be executed"); // David cannot approve - assertEq(plugin.canApprove(pid, david), false, "Should be false"); + assertEq(multisig.canApprove(pid, david), false, "Should be false"); - vm.stopPrank(); - vm.startPrank(alice); + switchTo(alice); } function test_CanApproveReturnsTrueIfListed() public { // returns `true` if the approver is listed - blockForward(1); - vm.warp(10); // timestamp = 10 - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); - assertEq(plugin.canApprove(pid, alice), true, "Should be true"); - assertEq(plugin.canApprove(pid, bob), true, "Should be true"); - assertEq(plugin.canApprove(pid, carol), true, "Should be true"); - assertEq(plugin.canApprove(pid, david), true, "Should be true"); + assertEq(multisig.canApprove(pid, alice), true, "Should be true"); + assertEq(multisig.canApprove(pid, bob), true, "Should be true"); + assertEq(multisig.canApprove(pid, carol), true, "Should be true"); + assertEq(multisig.canApprove(pid, david), true, "Should be true"); - { - // Deploy a new multisig instance - Multisig.MultisigSettings memory settings = - Multisig.MultisigSettings({onlyListed: false, minApprovals: 1, destinationProposalDuration: 4 days}); - address[] memory signers = new address[](1); - signers[0] = randomWallet; - - plugin = Multisig( - createProxyAndCall( - address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings)) - ) - ); - blockForward(1); - } + // new instance + builder = new DaoBuilder(); + (dao, optimisticPlugin, multisig,,,) = builder.withMultisigMember(randomWallet).withoutOnlyListed().build(); // now ko actions = new IDAO.Action[](0); - pid = plugin.createProposal("", actions, optimisticPlugin, false); + pid = multisig.createProposal("", actions, optimisticPlugin, false); - assertEq(plugin.canApprove(pid, alice), false, "Should be false"); - assertEq(plugin.canApprove(pid, bob), false, "Should be false"); - assertEq(plugin.canApprove(pid, carol), false, "Should be false"); - assertEq(plugin.canApprove(pid, david), false, "Should be false"); + assertEq(multisig.canApprove(pid, alice), false, "Should be false"); + assertEq(multisig.canApprove(pid, bob), false, "Should be false"); + assertEq(multisig.canApprove(pid, carol), false, "Should be false"); + assertEq(multisig.canApprove(pid, david), false, "Should be false"); // ok - assertEq(plugin.canApprove(pid, randomWallet), true, "Should be true"); + assertEq(multisig.canApprove(pid, randomWallet), true, "Should be true"); } // HAS APPROVED @@ -1301,54 +1253,44 @@ contract MultisigTest is AragonTest { function test_HasApprovedReturnsFalseWhenNotApproved() public { // returns `false` if user hasn't approved yet - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - assertEq(plugin.hasApproved(pid, alice), false, "Should be false"); - assertEq(plugin.hasApproved(pid, bob), false, "Should be false"); - assertEq(plugin.hasApproved(pid, carol), false, "Should be false"); - assertEq(plugin.hasApproved(pid, david), false, "Should be false"); + assertEq(multisig.hasApproved(pid, alice), false, "Should be false"); + assertEq(multisig.hasApproved(pid, bob), false, "Should be false"); + assertEq(multisig.hasApproved(pid, carol), false, "Should be false"); + assertEq(multisig.hasApproved(pid, david), false, "Should be false"); } function test_HasApprovedReturnsTrueWhenUserApproved() public { // returns `true` if user has approved - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - assertEq(plugin.hasApproved(pid, alice), false, "Should be false"); - plugin.approve(pid, false); - assertEq(plugin.hasApproved(pid, alice), true, "Should be true"); + assertEq(multisig.hasApproved(pid, alice), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.hasApproved(pid, alice), true, "Should be true"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - assertEq(plugin.hasApproved(pid, bob), false, "Should be false"); - plugin.approve(pid, false); - assertEq(plugin.hasApproved(pid, bob), true, "Should be true"); + switchTo(bob); + assertEq(multisig.hasApproved(pid, bob), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.hasApproved(pid, bob), true, "Should be true"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - assertEq(plugin.hasApproved(pid, carol), false, "Should be false"); - plugin.approve(pid, false); - assertEq(plugin.hasApproved(pid, carol), true, "Should be true"); + switchTo(carol); + assertEq(multisig.hasApproved(pid, carol), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.hasApproved(pid, carol), true, "Should be true"); // David - vm.stopPrank(); - vm.startPrank(david); - assertEq(plugin.hasApproved(pid, david), false, "Should be false"); - plugin.approve(pid, false); - assertEq(plugin.hasApproved(pid, david), true, "Should be true"); - - vm.stopPrank(); - vm.startPrank(alice); + switchTo(david); + assertEq(multisig.hasApproved(pid, david), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.hasApproved(pid, david), true, "Should be true"); } // APPROVE @@ -1356,346 +1298,254 @@ contract MultisigTest is AragonTest { function test_ApproveRevertsIfApprovingMultipleTimes() public { // reverts when approving multiple times - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, true); + multisig.approve(pid, true); vm.expectRevert(abi.encodeWithSelector(Multisig.ApprovalCastForbidden.selector, pid, alice)); - plugin.approve(pid, true); + multisig.approve(pid, true); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, true); + switchTo(bob); + multisig.approve(pid, true); vm.expectRevert(abi.encodeWithSelector(Multisig.ApprovalCastForbidden.selector, pid, bob)); - plugin.approve(pid, false); + multisig.approve(pid, false); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); vm.expectRevert(abi.encodeWithSelector(Multisig.ApprovalCastForbidden.selector, pid, carol)); - plugin.approve(pid, true); - - vm.stopPrank(); - vm.startPrank(alice); + multisig.approve(pid, true); } function test_ApprovesWithTheSenderAddress() public { // approves with the msg.sender address // Same as test_HasApprovedReturnsTrueWhenUserApproved() - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - assertEq(plugin.hasApproved(pid, alice), false, "Should be false"); - plugin.approve(pid, false); - assertEq(plugin.hasApproved(pid, alice), true, "Should be true"); + assertEq(multisig.hasApproved(pid, alice), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.hasApproved(pid, alice), true, "Should be true"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - assertEq(plugin.hasApproved(pid, bob), false, "Should be false"); - plugin.approve(pid, false); - assertEq(plugin.hasApproved(pid, bob), true, "Should be true"); + switchTo(bob); + assertEq(multisig.hasApproved(pid, bob), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.hasApproved(pid, bob), true, "Should be true"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - assertEq(plugin.hasApproved(pid, carol), false, "Should be false"); - plugin.approve(pid, false); - assertEq(plugin.hasApproved(pid, carol), true, "Should be true"); + switchTo(carol); + assertEq(multisig.hasApproved(pid, carol), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.hasApproved(pid, carol), true, "Should be true"); // David - vm.stopPrank(); - vm.startPrank(david); - assertEq(plugin.hasApproved(pid, david), false, "Should be false"); - plugin.approve(pid, false); - assertEq(plugin.hasApproved(pid, david), true, "Should be true"); - - vm.stopPrank(); - vm.startPrank(alice); + switchTo(david); + assertEq(multisig.hasApproved(pid, david), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.hasApproved(pid, david), true, "Should be true"); } function test_ApproveRevertsIfExpired() public { // reverts if the proposal has ended - blockForward(1); - vm.warp(0); // timestamp = 0 + uint64 expirationTime = uint64(block.timestamp) + 10 days; IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); - assertEq(plugin.canApprove(pid, alice), true, "Should be true"); + assertEq(multisig.canApprove(pid, alice), true, "Should be true"); - vm.warp(10 days + 1); + setTime(expirationTime); vm.expectRevert(abi.encodeWithSelector(Multisig.ApprovalCastForbidden.selector, pid, alice)); - plugin.approve(pid, false); + multisig.approve(pid, false); - vm.warp(15 days); + setTime(expirationTime + 15 days); vm.expectRevert(abi.encodeWithSelector(Multisig.ApprovalCastForbidden.selector, pid, alice)); - plugin.approve(pid, false); + multisig.approve(pid, false); // 2 - vm.warp(10); // timestamp = 10 - pid = plugin.createProposal("", actions, optimisticPlugin, false); + setTime(1000); + expirationTime = uint64(block.timestamp) + 10 days; + pid = multisig.createProposal("", actions, optimisticPlugin, false); - assertEq(plugin.canApprove(pid, alice), true, "Should be true"); + assertEq(multisig.canApprove(pid, alice), true, "Should be true"); - vm.warp(10 + 10 days + 1); + setTime(expirationTime); vm.expectRevert(abi.encodeWithSelector(Multisig.ApprovalCastForbidden.selector, pid, alice)); - plugin.approve(pid, true); + multisig.approve(pid, true); - vm.warp(10 + 10 days + 500); + setTime(expirationTime + 500); vm.expectRevert(abi.encodeWithSelector(Multisig.ApprovalCastForbidden.selector, pid, alice)); - plugin.approve(pid, true); + multisig.approve(pid, true); } function test_ApprovingProposalsEmits() public { // Approving a proposal emits the Approved event - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); vm.expectEmit(); emit Approved(pid, alice); - plugin.approve(pid, false); + multisig.approve(pid, false); // Bob - vm.stopPrank(); - vm.startPrank(bob); + switchTo(bob); vm.expectEmit(); emit Approved(pid, bob); - plugin.approve(pid, false); + multisig.approve(pid, false); // Carol - vm.stopPrank(); - vm.startPrank(carol); + switchTo(carol); vm.expectEmit(); emit Approved(pid, carol); - plugin.approve(pid, false); + multisig.approve(pid, false); // David (even if it already passed) - vm.stopPrank(); - vm.startPrank(david); + switchTo(david); vm.expectEmit(); emit Approved(pid, david); - plugin.approve(pid, false); - - vm.stopPrank(); - vm.startPrank(alice); + multisig.approve(pid, false); } // CAN EXECUTE function test_CanExecuteReturnsFalseIfBelowMinApprovals() public { // returns `false` if the proposal has not reached the minimum approvals yet + (dao, optimisticPlugin, multisig,,,) = builder.withMinApprovals(2).build(); - { - // Deploy a new multisig instance - Multisig.MultisigSettings memory settings = - Multisig.MultisigSettings({onlyListed: true, minApprovals: 2, destinationProposalDuration: 4 days}); - address[] memory signers = new address[](4); - signers[0] = alice; - signers[1] = bob; - signers[2] = carol; - signers[3] = david; - - plugin = Multisig( - createProxyAndCall( - address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings)) - ) - ); - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - blockForward(1); - } IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), false, "Should be false"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), true, "Should be true"); + switchTo(bob); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), true, "Should be true"); - vm.stopPrank(); - vm.startPrank(alice); + switchTo(alice); // More approvals required (4) + (dao, optimisticPlugin, multisig,,,) = builder.withMinApprovals(4).build(); - { - // Deploy a new multisig instance - Multisig.MultisigSettings memory settings = - Multisig.MultisigSettings({onlyListed: true, minApprovals: 4, destinationProposalDuration: 4 days}); - address[] memory signers = new address[](4); - signers[0] = alice; - signers[1] = bob; - signers[2] = carol; - signers[3] = david; - - plugin = Multisig( - createProxyAndCall( - address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings)) - ) - ); - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - blockForward(1); - } - - pid = plugin.createProposal("", actions, optimisticPlugin, false); + pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - vm.stopPrank(); - vm.startPrank(alice); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), false, "Should be false"); + switchTo(alice); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), false, "Should be false"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), false, "Should be false"); + switchTo(bob); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), false, "Should be false"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), false, "Should be false"); + switchTo(carol); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), false, "Should be false"); // David - vm.stopPrank(); - vm.startPrank(david); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), true, "Should be true"); - - vm.stopPrank(); - vm.startPrank(alice); + switchTo(david); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), true, "Should be true"); } function test_CanExecuteReturnsFalseIfExpired() public { // returns `false` if the proposal has ended - blockForward(1); - // 1 - vm.warp(0); // timestamp = 0 - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), true, "Should be true"); + multisig.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), true, "Should be true"); - vm.warp(10 days); - assertEq(plugin.canExecute(pid), true, "Should be true"); + timeForward(10 days - 1); + assertEq(multisig.canExecute(pid), true, "Should be true"); - vm.warp(10 days + 1); - assertEq(plugin.canExecute(pid), false, "Should be false"); + timeForward(1); + assertEq(multisig.canExecute(pid), false, "Should be false"); // 2 - vm.warp(0); // timestamp = 0 - + setTime(50 days); actions = new IDAO.Action[](0); - pid = plugin.createProposal("", actions, optimisticPlugin, false); - - vm.stopPrank(); - vm.startPrank(alice); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), true, "Should be true"); + pid = multisig.createProposal("", actions, optimisticPlugin, false); - vm.warp(10 days); - assertEq(plugin.canExecute(pid), true, "Should be true"); + switchTo(alice); + multisig.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), true, "Should be true"); - vm.warp(10 days + 1); - assertEq(plugin.canExecute(pid), false, "Should be false"); + timeForward(10 days - 1); + assertEq(multisig.canExecute(pid), true, "Should be true"); - vm.stopPrank(); - vm.startPrank(alice); + timeForward(1); + assertEq(multisig.canExecute(pid), false, "Should be false"); } function test_CanExecuteReturnsFalseIfExecuted() public { // returns `false` if the proposal is already executed - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, false); + multisig.approve(pid, false); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - - assertEq(plugin.canExecute(pid), true, "Should be true"); - plugin.execute(pid); + switchTo(carol); + multisig.approve(pid, false); - assertEq(plugin.canExecute(pid), false, "Should be false"); + assertEq(multisig.canExecute(pid), true, "Should be true"); + multisig.execute(pid); - vm.stopPrank(); - vm.startPrank(alice); + assertEq(multisig.canExecute(pid), false, "Should be false"); } function test_CanExecuteReturnsTrueWhenAllGood() public { // returns `true` if the proposal can be executed - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); - assertEq(plugin.canExecute(pid), false, "Should be false"); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); + assertEq(multisig.canExecute(pid), false, "Should be false"); // Alice - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), false, "Should be false"); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), false, "Should be false"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), false, "Should be false"); + switchTo(bob); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), false, "Should be false"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); - assertEq(plugin.canExecute(pid), true, "Should be true"); + assertEq(multisig.canExecute(pid), true, "Should be true"); } // EXECUTE @@ -1703,409 +1553,322 @@ contract MultisigTest is AragonTest { function test_ExecuteRevertsIfBelowMinApprovals() public { // reverts if minApprovals is not met yet - { - // Deploy a new multisig instance - Multisig.MultisigSettings memory settings = - Multisig.MultisigSettings({onlyListed: true, minApprovals: 2, destinationProposalDuration: 4 days}); - address[] memory signers = new address[](4); - signers[0] = alice; - signers[1] = bob; - signers[2] = carol; - signers[3] = david; + (dao, optimisticPlugin, multisig,,,) = builder.withMinApprovals(2).build(); - plugin = Multisig( - createProxyAndCall( - address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings)) - ) - ); - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - blockForward(1); - } IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, false); + multisig.approve(pid, false); vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalExecutionForbidden.selector, pid)); - plugin.execute(pid); + multisig.execute(pid); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - plugin.execute(pid); // ok + switchTo(bob); + multisig.approve(pid, false); + multisig.execute(pid); // ok - // More approvals required (4) - vm.stopPrank(); - vm.startPrank(alice); + switchTo(alice); - { - // Deploy a new multisig instance - Multisig.MultisigSettings memory settings = - Multisig.MultisigSettings({onlyListed: true, minApprovals: 4, destinationProposalDuration: 4 days}); - address[] memory signers = new address[](4); - signers[0] = alice; - signers[1] = bob; - signers[2] = carol; - signers[3] = david; - - plugin = Multisig( - createProxyAndCall( - address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings)) - ) - ); - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - blockForward(1); - } + // More approvals required (4) + (dao, optimisticPlugin, multisig,,,) = builder.withMinApprovals(4).build(); - pid = plugin.createProposal("", actions, optimisticPlugin, false); + pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - vm.stopPrank(); - vm.startPrank(alice); - plugin.approve(pid, false); + switchTo(alice); + multisig.approve(pid, false); vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalExecutionForbidden.selector, pid)); - plugin.execute(pid); + multisig.execute(pid); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalExecutionForbidden.selector, pid)); - plugin.execute(pid); + multisig.execute(pid); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalExecutionForbidden.selector, pid)); - plugin.execute(pid); + multisig.execute(pid); // David - vm.stopPrank(); - vm.startPrank(david); - plugin.approve(pid, false); - plugin.execute(pid); - - vm.stopPrank(); - vm.startPrank(alice); + switchTo(david); + multisig.approve(pid, false); + multisig.execute(pid); } function test_ExecuteRevertsIfExpired() public { // reverts if the proposal has expired - blockForward(1); - vm.warp(0); - // 1 IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), true, "Should be true"); + multisig.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), true, "Should be true"); - vm.warp(10 days + 1); + timeForward(10 days); vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalExecutionForbidden.selector, pid)); - plugin.execute(pid); + multisig.execute(pid); - vm.warp(100 days); + setTime(100 days); // 2 - pid = plugin.createProposal("", actions, optimisticPlugin, false); + pid = multisig.createProposal("", actions, optimisticPlugin, false); - vm.stopPrank(); - vm.startPrank(alice); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - assertEq(plugin.canExecute(pid), true, "Should be true"); + switchTo(alice); + multisig.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); + assertEq(multisig.canExecute(pid), true, "Should be true"); - vm.warp(100 days + 10 days + 1); + timeForward(10 days); vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalExecutionForbidden.selector, pid)); - plugin.execute(pid); + multisig.execute(pid); - vm.stopPrank(); - vm.startPrank(alice); + switchTo(alice); } function test_ExecuteRevertsWhenAlreadyExecuted() public { // executes if the minimum approval is met when multisig with the `tryExecution` option - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, false); + multisig.approve(pid, false); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); - assertEq(plugin.canExecute(pid), true, "Should be true"); - plugin.execute(pid); + assertEq(multisig.canExecute(pid), true, "Should be true"); + multisig.execute(pid); vm.expectRevert(abi.encodeWithSelector(Multisig.ProposalExecutionForbidden.selector, pid)); - plugin.execute(pid); - - vm.stopPrank(); - vm.startPrank(alice); + multisig.execute(pid); } function test_ExecuteEmitsEvents() public { // emits the `ProposalExecuted` and `ProposalCreated` events - blockForward(1); - - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - vm.warp(0); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, false); + multisig.approve(pid, false); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); // event vm.expectEmit(); emit Executed(pid); - uint256 targetPid = uint256(block.timestamp) << 128 | uint256(block.timestamp + 4 days) << 64; + uint256 targetPid = uint256(block.timestamp) << 128 | uint256(block.timestamp + 10 days) << 64; vm.expectEmit(); emit ProposalCreated( - targetPid, address(plugin), uint64(block.timestamp), uint64(block.timestamp) + 4 days, "", actions, 0 + targetPid, address(multisig), uint64(block.timestamp), uint64(block.timestamp) + 10 days, "", actions, 0 ); - plugin.execute(pid); + multisig.execute(pid); // 2 + (dao, optimisticPlugin, multisig,,,) = builder.withDuration(50 days).build(); + + setTime(5000); actions = new IDAO.Action[](1); actions[0].value = 1 ether; actions[0].to = address(bob); actions[0].data = hex"00112233"; - pid = plugin.createProposal("ipfs://", actions, optimisticPlugin, false); + pid = multisig.createProposal("ipfs://", actions, optimisticPlugin, false); // Alice - vm.stopPrank(); - vm.startPrank(alice); - plugin.approve(pid, false); + switchTo(alice); + multisig.approve(pid, false); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); // events vm.expectEmit(); emit Executed(pid); - targetPid = (uint256(block.timestamp + 10 days) << 128 | uint256(block.timestamp + 50 days) << 64) + 1; + targetPid = (uint256(block.timestamp) << 128 | uint256(block.timestamp + 50 days) << 64); vm.expectEmit(); - emit ProposalCreated(targetPid, address(plugin), uint64(block.timestamp), 50 days, "ipfs://", actions, 0); - plugin.execute(pid); + emit ProposalCreated( + targetPid, address(multisig), uint64(block.timestamp), 5000 + 50 days, "ipfs://", actions, 0 + ); + multisig.execute(pid); } function test_ExecutesWhenApprovingWithTryExecutionAndEnoughApprovals() public { // executes if the minimum approval is met when multisig with the `tryExecution` option - blockForward(1); - - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); - (bool executed,,,,,) = plugin.getProposal(pid); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); + (bool executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Alice - plugin.approve(pid, true); - (executed,,,,,) = plugin.getProposal(pid); + multisig.approve(pid, true); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, true); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(bob); + multisig.approve(pid, true); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, true); + switchTo(carol); + multisig.approve(pid, true); - (executed,,,,,) = plugin.getProposal(pid); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, true, "Should be executed"); - - vm.stopPrank(); - vm.startPrank(alice); } function test_ExecuteEmitsWhenAutoExecutedFromApprove() public { // emits the `Approved`, `ProposalExecuted`, and `ProposalCreated` events if execute is called inside the `approve` method - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, true); + multisig.approve(pid, true); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, true); + switchTo(bob); + multisig.approve(pid, true); // Carol - vm.stopPrank(); - vm.startPrank(carol); + switchTo(carol); vm.expectEmit(); emit Approved(pid, carol); vm.expectEmit(); emit Executed(pid); - uint256 targetPid = (uint256(block.timestamp) << 128 | uint256(block.timestamp + 4 days) << 64); + uint256 targetPid = (uint256(block.timestamp) << 128 | uint256(block.timestamp + 10 days) << 64); vm.expectEmit(); emit ProposalCreated( - targetPid, address(plugin), uint64(block.timestamp), uint64(block.timestamp) + 4 days, "", actions, 0 + targetPid, address(multisig), uint64(block.timestamp), uint64(block.timestamp) + 10 days, "", actions, 0 ); - plugin.approve(pid, true); + multisig.approve(pid, true); // 2 + setTime(5 days); actions = new IDAO.Action[](1); actions[0].value = 1 ether; actions[0].to = address(bob); actions[0].data = hex"00112233"; - pid = plugin.createProposal("ipfs://", actions, optimisticPlugin, false); + pid = multisig.createProposal("ipfs://", actions, optimisticPlugin, false); // Alice - vm.stopPrank(); - vm.startPrank(alice); - plugin.approve(pid, true); + switchTo(alice); + multisig.approve(pid, true); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, true); + switchTo(bob); + multisig.approve(pid, true); // Carol - vm.stopPrank(); - vm.startPrank(carol); + switchTo(carol); vm.expectEmit(); emit Approved(pid, carol); vm.expectEmit(); emit Executed(pid); - targetPid = (uint256(5 days) << 128 | uint256(20 days) << 64) + 1; + targetPid = (uint256(5 days) << 128 | uint256(5 days + 10 days) << 64) + 1; vm.expectEmit(); emit ProposalCreated( targetPid, // foreign pid - address(plugin), - 5 days, - 20 days, + address(multisig), + uint64(block.timestamp), + uint64(block.timestamp) + 10 days, "ipfs://", actions, 0 ); - plugin.approve(pid, true); + multisig.approve(pid, true); // 3 + (dao, optimisticPlugin, multisig,,,) = builder.withDuration(50 days).build(); + + setTime(7 days); actions = new IDAO.Action[](1); actions[0].value = 5 ether; actions[0].to = address(carol); actions[0].data = hex"44556677"; - pid = plugin.createProposal("ipfs://...", actions, optimisticPlugin, false); + pid = multisig.createProposal("ipfs://...", actions, optimisticPlugin, false); // Alice - vm.stopPrank(); - vm.startPrank(alice); - plugin.approve(pid, true); + switchTo(alice); + multisig.approve(pid, true); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, true); + switchTo(bob); + multisig.approve(pid, true); // Carol - vm.stopPrank(); - vm.startPrank(carol); + switchTo(carol); vm.expectEmit(); emit Approved(pid, carol); vm.expectEmit(); emit Executed(pid); - targetPid = (uint256(3 days) << 128 | uint256(500 days) << 64) + 2; + targetPid = (uint256(7 days) << 128 | uint256(7 days + 50 days) << 64); vm.expectEmit(); - emit ProposalCreated(targetPid, address(plugin), 3 days, 500 days, "ipfs://...", actions, 0); - plugin.approve(pid, true); - - vm.stopPrank(); - vm.startPrank(alice); + emit ProposalCreated(targetPid, address(multisig), 7 days, 7 days + 50 days, "ipfs://...", actions, 0); + multisig.approve(pid, true); } function test_ExecutesWithEnoughApprovalsOnTime() public { // executes if the minimum approval is met - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, false); - (bool executed,,,,,) = plugin.getProposal(pid); + multisig.approve(pid, false); + (bool executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(bob); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(carol); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); - plugin.execute(pid); - (executed,,,,,) = plugin.getProposal(pid); + multisig.execute(pid); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, true, "Should be executed"); // 2 @@ -2113,110 +1876,91 @@ contract MultisigTest is AragonTest { actions[0].value = 1 ether; actions[0].to = address(bob); actions[0].data = hex"00112233"; - pid = plugin.createProposal("ipfs://", actions, optimisticPlugin, false); + pid = multisig.createProposal("ipfs://", actions, optimisticPlugin, false); // Alice - vm.stopPrank(); - vm.startPrank(alice); - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(alice); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(bob); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(carol); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); - plugin.execute(pid); + multisig.execute(pid); - (executed,,,,,) = plugin.getProposal(pid); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, true, "Should be executed"); - - vm.stopPrank(); - vm.startPrank(alice); } function test_ExecuteWhenPassedAndCalledByAnyone() public { // executes if the minimum approval is met and can be called by an unlisted accounts - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - blockForward(1); - IDAO.Action[] memory actions = new IDAO.Action[](0); - uint256 pid = plugin.createProposal("", actions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, false); - (bool executed,,,,,) = plugin.getProposal(pid); + multisig.approve(pid, false); + (bool executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(bob); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(carol); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); - vm.stopPrank(); - vm.startPrank(randomWallet); - plugin.execute(pid); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(randomWallet); + multisig.execute(pid); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, true, "Should be executed"); // 2 - vm.stopPrank(); - vm.startPrank(alice); + switchTo(alice); actions = new IDAO.Action[](1); actions[0].value = 1 ether; actions[0].to = address(bob); actions[0].data = hex"00112233"; - pid = plugin.createProposal("ipfs://", actions, optimisticPlugin, false); + pid = multisig.createProposal("ipfs://", actions, optimisticPlugin, false); // Alice - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Bob - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(bob); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); // Carol - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - (executed,,,,,) = plugin.getProposal(pid); + switchTo(carol); + multisig.approve(pid, false); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); - vm.stopPrank(); - vm.startPrank(randomWallet); - plugin.execute(pid); + switchTo(randomWallet); + multisig.execute(pid); - (executed,,,,,) = plugin.getProposal(pid); + (executed,,,,,) = multisig.getProposal(pid); assertEq(executed, true, "Should be executed"); - - vm.stopPrank(); - vm.startPrank(alice); } function test_GetProposalReturnsTheRightValues() public { @@ -2229,9 +1973,7 @@ contract MultisigTest is AragonTest { IDAO.Action[] memory actions; OptimisticTokenVotingPlugin destPlugin; - blockForward(1); - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - vm.warp(10); // timestamp = 10 + setTime(10); IDAO.Action[] memory createActions = new IDAO.Action[](3); createActions[0].to = alice; @@ -2244,11 +1986,11 @@ contract MultisigTest is AragonTest { createActions[2].value = 3 ether; createActions[2].data = hex"223344556677"; - uint256 pid = plugin.createProposal("ipfs://metadata", createActions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("ipfs://metadata", createActions, optimisticPlugin, false); assertEq(pid, 0, "PID should be 0"); // Check round 1 - (executed, approvals, parameters, metadataURI, actions, destPlugin) = plugin.getProposal(pid); + (executed, approvals, parameters, metadataURI, actions, destPlugin) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); assertEq(approvals, 0, "Should be 0"); @@ -2273,10 +2015,10 @@ contract MultisigTest is AragonTest { assertEq(address(destPlugin), address(optimisticPlugin), "Incorrect destPlugin"); // Approve - plugin.approve(pid, false); + multisig.approve(pid, false); // Check round 2 - (executed, approvals, parameters, metadataURI, actions, destPlugin) = plugin.getProposal(pid); + (executed, approvals, parameters, metadataURI, actions, destPlugin) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); assertEq(approvals, 1, "Should be 1"); @@ -2301,15 +2043,13 @@ contract MultisigTest is AragonTest { assertEq(address(destPlugin), address(optimisticPlugin), "Incorrect destPlugin"); // Approve - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); // Check round 3 - (executed, approvals, parameters, metadataURI, actions, destPlugin) = plugin.getProposal(pid); + (executed, approvals, parameters, metadataURI, actions, destPlugin) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); assertEq(approvals, 3, "Should be 3"); @@ -2334,12 +2074,11 @@ contract MultisigTest is AragonTest { assertEq(address(destPlugin), address(optimisticPlugin), "Incorrect destPlugin"); // Execute - vm.stopPrank(); - vm.startPrank(alice); - plugin.execute(pid); + switchTo(alice); + multisig.execute(pid); // Check round 4 - (executed, approvals, parameters, metadataURI, actions, destPlugin) = plugin.getProposal(pid); + (executed, approvals, parameters, metadataURI, actions, destPlugin) = multisig.getProposal(pid); assertEq(executed, true, "Should be executed"); assertEq(approvals, 3, "Should be 3"); @@ -2363,27 +2102,12 @@ contract MultisigTest is AragonTest { assertEq(metadataURI, "ipfs://metadata", "Incorrect metadata URI"); assertEq(address(destPlugin), address(optimisticPlugin), "Incorrect destPlugin"); - // New proposal, new settings - vm.stopPrank(); - vm.startPrank(alice); + // New multisig, new settings + switchTo(alice); - { - // Deploy a new multisig instance - Multisig.MultisigSettings memory settings = - Multisig.MultisigSettings({onlyListed: true, minApprovals: 2, destinationProposalDuration: 4 days}); - address[] memory signers = new address[](3); - signers[0] = alice; - signers[1] = bob; - signers[2] = carol; + // Deploy new instances + (dao, optimisticPlugin, multisig,,,) = builder.withMinApprovals(2).build(); - plugin = Multisig( - createProxyAndCall( - address(MULTISIG_BASE), abi.encodeCall(Multisig.initialize, (dao, signers, settings)) - ) - ); - blockForward(1); - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - } createActions = new IDAO.Action[](2); createActions[1].to = alice; createActions[1].value = 1 ether; @@ -2392,13 +2116,13 @@ contract MultisigTest is AragonTest { createActions[0].value = 3 ether; createActions[0].data = hex"223344556677"; - vm.warp(50); // Timestamp = 50 + setTime(50); // Timestamp = 50 - pid = plugin.createProposal("ipfs://different-metadata", createActions, optimisticPlugin, true); + pid = multisig.createProposal("ipfs://different-metadata", createActions, optimisticPlugin, true); assertEq(pid, 0, "PID should be 0"); // Check round 1 - (executed, approvals, parameters, metadataURI, actions, destPlugin) = plugin.getProposal(pid); + (executed, approvals, parameters, metadataURI, actions, destPlugin) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); assertEq(approvals, 1, "Should be 1"); @@ -2420,12 +2144,11 @@ contract MultisigTest is AragonTest { assertEq(address(destPlugin), address(optimisticPlugin), "Incorrect destPlugin"); // Approve - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); // Check round 2 - (executed, approvals, parameters, metadataURI, actions, destPlugin) = plugin.getProposal(pid); + (executed, approvals, parameters, metadataURI, actions, destPlugin) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); assertEq(approvals, 2, "Should be 2"); @@ -2447,12 +2170,11 @@ contract MultisigTest is AragonTest { assertEq(address(destPlugin), address(optimisticPlugin), "Incorrect destPlugin"); // Approve - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); // Check round 3 - (executed, approvals, parameters, metadataURI, actions, destPlugin) = plugin.getProposal(pid); + (executed, approvals, parameters, metadataURI, actions, destPlugin) = multisig.getProposal(pid); assertEq(executed, false, "Should not be executed"); assertEq(approvals, 3, "Should be 3"); @@ -2474,12 +2196,11 @@ contract MultisigTest is AragonTest { assertEq(address(destPlugin), address(optimisticPlugin), "Incorrect destPlugin"); // Execute - vm.stopPrank(); - vm.startPrank(alice); - plugin.execute(pid); + switchTo(alice); + multisig.execute(pid); // Check round 4 - (executed, approvals, parameters, metadataURI, actions, destPlugin) = plugin.getProposal(pid); + (executed, approvals, parameters, metadataURI, actions, destPlugin) = multisig.getProposal(pid); assertEq(executed, true, "Should be executed"); assertEq(approvals, 3, "Should be 3"); @@ -2511,9 +2232,7 @@ contract MultisigTest is AragonTest { IDAO.Action[] memory actions; uint256 allowFailureMap; - blockForward(1); - dao.grant(address(optimisticPlugin), address(plugin), optimisticPlugin.PROPOSER_PERMISSION_ID()); - vm.warp(0); // timestamp = 0 + setTime(1 days); IDAO.Action[] memory createActions = new IDAO.Action[](3); createActions[0].to = alice; @@ -2526,29 +2245,29 @@ contract MultisigTest is AragonTest { createActions[2].value = 3 ether; createActions[2].data = hex"223344556677"; - uint256 pid = plugin.createProposal("ipfs://metadata", createActions, optimisticPlugin, false); + uint256 pid = multisig.createProposal("ipfs://metadata", createActions, optimisticPlugin, false); // Approve - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - - vm.stopPrank(); - vm.startPrank(alice); - plugin.execute(pid); + multisig.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); + + switchTo(alice); + multisig.execute(pid); // Check round - uint256 targetPid = uint256(4 days) << 64; // start=0, end=4d, counter=0 + uint256 targetPid = uint256(1 days) << 128 | uint256(1 days + 10 days) << 64; // start=1d, end=10d, counter=0 (open, executed, parameters, vetoTally, actions, allowFailureMap) = optimisticPlugin.getProposal(targetPid); assertEq(open, true, "Should be open"); assertEq(executed, false, "Should not be executed"); assertEq(vetoTally, 0, "Should be 0"); + assertEq(parameters.metadataUri, "ipfs://metadata", "Incorrect target metadataUri"); + assertEq(parameters.vetoEndDate, 1 days + 10 days, "Incorrect target vetoEndDate"); + assertEq(actions.length, 3, "Should be 3"); assertEq(actions[0].to, alice, "Incorrect to"); @@ -2564,6 +2283,7 @@ contract MultisigTest is AragonTest { assertEq(allowFailureMap, 0, "Should be 0"); // New proposal + setTime(2 days); createActions = new IDAO.Action[](2); createActions[1].to = alice; @@ -2573,29 +2293,29 @@ contract MultisigTest is AragonTest { createActions[0].value = 3 ether; createActions[0].data = hex"223344556677"; - pid = plugin.createProposal("ipfs://more-metadata", createActions, optimisticPlugin, false); + pid = multisig.createProposal("ipfs://more-metadata", createActions, optimisticPlugin, false); // Approve - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(bob); - plugin.approve(pid, false); - vm.stopPrank(); - vm.startPrank(carol); - plugin.approve(pid, false); - - vm.stopPrank(); - vm.startPrank(alice); - plugin.execute(pid); + multisig.approve(pid, false); + switchTo(bob); + multisig.approve(pid, false); + switchTo(carol); + multisig.approve(pid, false); + + switchTo(alice); + multisig.execute(pid); // Check round - targetPid = (uint256(block.timestamp + 1 days) << 128 | uint256(block.timestamp + 6 days) << 64) + 1; + targetPid = (uint256(2 days) << 128 | uint256(2 days + 10 days) << 64) + 1; (open, executed, parameters, vetoTally, actions, allowFailureMap) = optimisticPlugin.getProposal(targetPid); - assertEq(open, false, "Should not be open"); + assertEq(open, true, "Should be open"); assertEq(executed, false, "Should not be executed"); assertEq(vetoTally, 0, "Should be 0"); + assertEq(parameters.metadataUri, "ipfs://more-metadata", "Incorrect target metadataUri"); + assertEq(parameters.vetoEndDate, 2 days + 10 days, "Incorrect target vetoEndDate"); + assertEq(actions.length, 2, "Should be 2"); assertEq(actions[1].to, alice, "Incorrect to"); @@ -2608,24 +2328,30 @@ contract MultisigTest is AragonTest { assertEq(allowFailureMap, 0, "Should be 0"); } - // Upgrade plugin + // Upgrade multisig function test_UpgradeToRevertsWhenCalledFromNonUpgrader() public { + address initialImplementation = multisig.implementation(); address _newImplementation = address(new Multisig()); vm.expectRevert( abi.encodeWithSelector( - DaoUnauthorized.selector, address(dao), address(plugin), alice, plugin.UPGRADE_PLUGIN_PERMISSION_ID() + DaoUnauthorized.selector, + address(dao), + address(multisig), + alice, + multisig.UPGRADE_PLUGIN_PERMISSION_ID() ) ); - plugin.upgradeTo(_newImplementation); + multisig.upgradeTo(_newImplementation); - assertEq(plugin.implementation(), address(MULTISIG_BASE)); + assertEq(multisig.implementation(), initialImplementation); } function test_UpgradeToAndCallRevertsWhenCalledFromNonUpgrader() public { - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + address initialImplementation = multisig.implementation(); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); address _newImplementation = address(new Multisig()); Multisig.MultisigSettings memory settings = @@ -2633,30 +2359,34 @@ contract MultisigTest is AragonTest { vm.expectRevert( abi.encodeWithSelector( - DaoUnauthorized.selector, address(dao), address(plugin), alice, plugin.UPGRADE_PLUGIN_PERMISSION_ID() + DaoUnauthorized.selector, + address(dao), + address(multisig), + alice, + multisig.UPGRADE_PLUGIN_PERMISSION_ID() ) ); - plugin.upgradeToAndCall(_newImplementation, abi.encodeCall(Multisig.updateMultisigSettings, (settings))); + multisig.upgradeToAndCall(_newImplementation, abi.encodeCall(Multisig.updateMultisigSettings, (settings))); - assertEq(plugin.implementation(), address(MULTISIG_BASE)); + assertEq(multisig.implementation(), initialImplementation); } function test_UpgradeToSucceedsWhenCalledFromUpgrader() public { - dao.grant(address(plugin), alice, plugin.UPGRADE_PLUGIN_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPGRADE_PLUGIN_PERMISSION_ID()); address _newImplementation = address(new Multisig()); vm.expectEmit(); emit Upgraded(_newImplementation); - plugin.upgradeTo(_newImplementation); + multisig.upgradeTo(_newImplementation); - assertEq(plugin.implementation(), address(_newImplementation)); + assertEq(multisig.implementation(), address(_newImplementation)); } function test_UpgradeToAndCallSucceedsWhenCalledFromUpgrader() public { - dao.grant(address(plugin), alice, plugin.UPGRADE_PLUGIN_PERMISSION_ID()); - dao.grant(address(plugin), alice, plugin.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPGRADE_PLUGIN_PERMISSION_ID()); + dao.grant(address(multisig), alice, multisig.UPDATE_MULTISIG_SETTINGS_PERMISSION_ID()); address _newImplementation = address(new Multisig()); @@ -2665,8 +2395,8 @@ contract MultisigTest is AragonTest { Multisig.MultisigSettings memory settings = Multisig.MultisigSettings({onlyListed: false, minApprovals: 2, destinationProposalDuration: 14 days}); - plugin.upgradeToAndCall(_newImplementation, abi.encodeCall(Multisig.updateMultisigSettings, (settings))); + multisig.upgradeToAndCall(_newImplementation, abi.encodeCall(Multisig.updateMultisigSettings, (settings))); - assertEq(plugin.implementation(), address(_newImplementation)); + assertEq(multisig.implementation(), address(_newImplementation)); } } diff --git a/test/helpers/DaoBuilder.sol b/test/helpers/DaoBuilder.sol index 8a48729..21a168e 100644 --- a/test/helpers/DaoBuilder.sol +++ b/test/helpers/DaoBuilder.sol @@ -74,6 +74,11 @@ contract DaoBuilder is Test { return this; } + function withDuration(uint32 newDuration) public returns (DaoBuilder) { + stdProposalDuration = newDuration; + return this; + } + function withL2InactivityPeriod(uint64 newL2InactivityPeriod) public returns (DaoBuilder) { l2InactivityPeriod = newL2InactivityPeriod; return this;