diff --git a/README.md b/README.md index b97d038..81576ba 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,8 @@ ## Council Member Steps Down - Single Election that replaces 1 council member - Council member can step down at any time (except during a scheduled election), triggering a single election to replace the member +- Cannot step down if last member + - due to a Safe requirement, there must always be at least 1 owner # Notes for Continued Implementation: @@ -41,6 +43,8 @@ - todo: community re-elections should not be able to start if the last one !isElectionFinalized() - currently its if the last one was started within the last 3 weeks. this is innacurate though because overlapping could happen like specified above +- council array in AutomatedVoting was chosen to be a fixed array of 5 rather than a dynamic array + - multiple elections can be ongoing, except for scheduled elections - meaning the other 3 elections can be ongoing at the same time - multiple different single elections can be ongoing at the same time diff --git a/src/AutomatedVoting.sol b/src/AutomatedVoting.sol index f4ee622..3c818a3 100644 --- a/src/AutomatedVoting.sol +++ b/src/AutomatedVoting.sol @@ -308,6 +308,10 @@ contract AutomatedVoting is IAutomatedVoting, GovernorModule { delete council[i]; } } + // remove owner from safe + //todo: removeSingleOwner(msg.sender); + //todo: commented out because tests need to be fixed (AutomatedVoting.t.sol needs safe setup) + // start election state _startElection(Enums.electionType.replacement); } @@ -331,6 +335,7 @@ contract AutomatedVoting is IAutomatedVoting, GovernorModule { /// @notice function for council member to step down /// @dev cannot step down if there is only one council member + /// this is a built in Safe precaution function stepDown() public override onlyCouncil notDuringScheduledElection { uint councilMemberCount = 0; for (uint i = 0; i < council.length; i++) { @@ -338,12 +343,18 @@ contract AutomatedVoting is IAutomatedVoting, GovernorModule { councilMemberCount++; } } + if (councilMemberCount == 1) { + revert CannotStepDown(); + } // burn msg.sender rights for (uint i = 0; i < council.length; i++) { if (council[i] == msg.sender) { delete council[i]; } } + // remove owner from safe + //todo: removeSingleOwner(msg.sender); + //todo: commented out because tests need to be fixed (AutomatedVoting.t.sol needs safe setup) // start election state _startElection(Enums.electionType.replacement); } @@ -508,6 +519,7 @@ contract AutomatedVoting is IAutomatedVoting, GovernorModule { elections[_election].theElectionType == Enums.electionType.scheduled ) { /// @dev this is for a full election + //todo: consider what happens if 5 or more people are never nominated address[5] memory winners; winners[0] = elections[_election].candidateAddresses[0]; winners[1] = elections[_election].candidateAddresses[1]; @@ -515,11 +527,15 @@ contract AutomatedVoting is IAutomatedVoting, GovernorModule { winners[3] = elections[_election].candidateAddresses[3]; winners[4] = elections[_election].candidateAddresses[4]; council = winners; + /// @dev put the new council into the safe + //todo: putInFullElection(winners); + //todo: commented out because tests need to be fixed (AutomatedVoting.t.sol needs safe setup) } else if ( elections[_election].theElectionType == Enums.electionType.community ) { if (_checkIfQuorumReached(_election)) { /// @dev this is for a full election + //todo: consider what happens if 5 or more people are never nominated address[5] memory winners; winners[0] = elections[_election].candidateAddresses[0]; winners[1] = elections[_election].candidateAddresses[1]; @@ -527,6 +543,9 @@ contract AutomatedVoting is IAutomatedVoting, GovernorModule { winners[3] = elections[_election].candidateAddresses[3]; winners[4] = elections[_election].candidateAddresses[4]; council = winners; + /// @dev put the new council into the safe + //todo: putInFullElection(winners); + //todo: commented out because tests need to be fixed (AutomatedVoting.t.sol needs safe setup) } else { /// @dev do nothing because quorum not reached //todo: maybe emit an event that quorum was not reached @@ -543,6 +562,9 @@ contract AutomatedVoting is IAutomatedVoting, GovernorModule { break; } } + /// @dev put the new member into the safe + //todo: addSingleOwner(winner); + //todo: commented out because tests need to be fixed (AutomatedVoting.t.sol needs safe setup) } } diff --git a/src/GovernorModule.sol b/src/GovernorModule.sol index 65c35dd..6258995 100644 --- a/src/GovernorModule.sol +++ b/src/GovernorModule.sol @@ -13,8 +13,6 @@ contract GovernorModule { /// @notice this is to call replaceOwner() on the safe /// @dev done with execTransactionFromModule() - //todo: check if this function is still needed - // we might only need addOwnerWithThreshold(), removeOwner(), and setupOwners() function replaceOwner(address prevOwner, address oldOwner, address newOwner) internal { bytes memory swapOwner = abi.encodeWithSignature( "swapOwner(address,address,address)", @@ -72,15 +70,15 @@ contract GovernorModule { function removeSingleOwner(address owner) internal { /// @dev this is to get the previous owner param - for (int i = 0; i < safeProxy.getOwners().length; i++) { + for (uint256 i = 0; i < safeProxy.getOwners().length; i++) { //todo: get prevOwner - if (safeProxy.getOwner(prevOwner) == owner) { - break; - } + // if (safeProxy.getOwner(prevOwner) == owner) { + // break; + // } } //todo: removeOwner and adjust threshold accordingly - removeOwner(address(0), owner, 1); + // removeOwner(address(0), owner, 1); } /// @notice this is to add one owner to the safe when a replacement election ends @@ -88,10 +86,14 @@ contract GovernorModule { /// should always be majority /// exact thresholds are listed in README.md function addSingleOwner(address owner) internal { - addOwnerWithThreshold(owner, 1); + //todo: add owner and adjust threshold accordingly + // addOwnerWithThreshold(owner, 1); } - function putInFullElection() internal { - + /// @notice this is to add in the whole council to the safe when a full election ends + function putInFullElection(address[5] memory owners) internal { + //todo: if current owners are less than 5, add in the rest + //todo: replace all current owners + // make threshold 3 } } diff --git a/src/interfaces/IAutomatedVoting.sol b/src/interfaces/IAutomatedVoting.sol index 9dae595..6db2f86 100644 --- a/src/interfaces/IAutomatedVoting.sol +++ b/src/interfaces/IAutomatedVoting.sol @@ -55,6 +55,11 @@ interface IAutomatedVoting { /// @notice emitted when the caller votes for too many candidates error TooManyCandidates(); + /// @notice emitted when the member is the last council member + /// and tries to step down + /// @dev this is because the Safe requires at least one owner + error CannotStepDown(); + /// @notice emitted when candidate is not nominated error CandidateNotNominated(); diff --git a/test/AutomatedVoting.t.sol b/test/AutomatedVoting.t.sol index 5523ff0..e55e15a 100644 --- a/test/AutomatedVoting.t.sol +++ b/test/AutomatedVoting.t.sol @@ -596,7 +596,7 @@ contract AutomatedVotingTest is DefaultStakingV2Setup { automatedVoting.stepDown(); } - function testEveryoneCanStepDown() public { + function testLastMemberCantStepDown() public { vm.prank(user1); automatedVoting.stepDown(); vm.prank(user3); @@ -605,6 +605,9 @@ contract AutomatedVotingTest is DefaultStakingV2Setup { automatedVoting.stepDown(); vm.prank(user5); automatedVoting.stepDown(); + vm.expectRevert( + abi.encodeWithSelector(IAutomatedVoting.CannotStepDown.selector) + ); vm.prank(user2); automatedVoting.stepDown(); } diff --git a/test/AutomatedVotingInternals.sol b/test/AutomatedVotingInternals.sol index 37c3c96..389945e 100644 --- a/test/AutomatedVotingInternals.sol +++ b/test/AutomatedVotingInternals.sol @@ -55,4 +55,8 @@ contract AutomatedVotingInternals is AutomatedVoting { removeOwner(prevOwner, owner, threshold); } + function putInFullElectionInternal(address[5] memory owners) public { + putInFullElection(owners); + } + } diff --git a/test/GovernorModule.t.sol b/test/GovernorModule.t.sol index 15642c4..1681923 100644 --- a/test/GovernorModule.t.sol +++ b/test/GovernorModule.t.sol @@ -172,6 +172,27 @@ contract GovernorModuleTest is DefaultStakingV2Setup { execTransactionTransfer(Owner1, owner1PrivateKey); } + function testPutInFullElection() public { + address[5] memory owners; + owners[0] = Owner1; + owners[1] = Owner2; + owners[2] = address(0x3); + owners[3] = address(0x4); + owners[4] = address(0x5); + + // put in full election + automatedVotingInternals.putInFullElectionInternal(owners); + // make sure owners 1-5 are owners + assertTrue(safeProxy.isOwner(Owner1)); + assertTrue(safeProxy.isOwner(Owner2)); + assertTrue(safeProxy.isOwner(address(0x3))); + assertTrue(safeProxy.isOwner(address(0x4))); + assertTrue(safeProxy.isOwner(address(0x5))); + + } + + //todo: do end to end tests of safe functionality with the starting and ending of elections + //util functions function execTransactionTransfer(