Skip to content

Commit

Permalink
👷 add todos to GovenorModule and add stubs to AutomatedVoting
Browse files Browse the repository at this point in the history
  • Loading branch information
cmontecoding committed Sep 29, 2023
1 parent c198723 commit b669caf
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 11 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down
22 changes: 22 additions & 0 deletions src/AutomatedVoting.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -331,19 +335,26 @@ 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++) {
if (council[i] != address(0)) {
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);
}
Expand Down Expand Up @@ -508,25 +519,33 @@ 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];
winners[2] = elections[_election].candidateAddresses[2];
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];
winners[2] = elections[_election].candidateAddresses[2];
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
Expand All @@ -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)
}
}

Expand Down
22 changes: 12 additions & 10 deletions src/GovernorModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down Expand Up @@ -72,26 +70,30 @@ 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
/// @dev threshold is adjusted according to the number of owners
/// 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
}
}
5 changes: 5 additions & 0 deletions src/interfaces/IAutomatedVoting.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
5 changes: 4 additions & 1 deletion test/AutomatedVoting.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ contract AutomatedVotingTest is DefaultStakingV2Setup {
automatedVoting.stepDown();
}

function testEveryoneCanStepDown() public {
function testLastMemberCantStepDown() public {
vm.prank(user1);
automatedVoting.stepDown();
vm.prank(user3);
Expand All @@ -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();
}
Expand Down
4 changes: 4 additions & 0 deletions test/AutomatedVotingInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,8 @@ contract AutomatedVotingInternals is AutomatedVoting {
removeOwner(prevOwner, owner, threshold);
}

function putInFullElectionInternal(address[5] memory owners) public {
putInFullElection(owners);
}

}
21 changes: 21 additions & 0 deletions test/GovernorModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit b669caf

Please sign in to comment.