From 322563d7f67227fc35ff9de25e808620b58ec137 Mon Sep 17 00:00:00 2001 From: Ido Date: Mon, 12 Aug 2024 16:15:12 +0200 Subject: [PATCH 01/10] draft --- src/JokeraceEligibility.sol | 39 ++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/src/JokeraceEligibility.sol b/src/JokeraceEligibility.sol index c805005..f7c6f4e 100644 --- a/src/JokeraceEligibility.sol +++ b/src/JokeraceEligibility.sol @@ -69,9 +69,16 @@ contract JokeraceEligibility is HatsEligibilityModule { //////////////////////////////////////////////////////////////*/ /// @notice Current Jokerace contest (election) - address public underlyingContest; + address public currentContest; + /// @notice Next Jokerace contest (election) + address public nextContest; /// @notice First second after the current term (a unix timestamp) uint256 public termEnd; + /** + * @notice Period of time after the term end when reelection is allowed. During this period, previous elected members + * are still considered eligible until a new term is set. + */ + uint256 public transitionPeriod; /// @notice First K winners of the contest will be eligible uint256 public topK; /// @notice Eligible wearers according to each contest @@ -86,19 +93,20 @@ contract JokeraceEligibility is HatsEligibilityModule { * @dev Only callable by the hats-module factory. Since the factory only calls this function during a new deployment, * this ensures it can only be called once per instance, and that the implementation contract is never initialized. * @param _initData Packed initialization data with the following parameters: - * _underlyingContest - Jokerace contest. The contest must have down-voting disabled and sorting enabled. + * _contest - Jokerace contest. The contest must have down-voting disabled and sorting enabled. * _termEnd - Final second of the current term (a unix timestamp) * _topK - First K winners of the contest will be eligible */ function _setUp(bytes calldata _initData) internal override { - (address payable _underlyingContest, uint256 _termEnd, uint256 _topK) = - abi.decode(_initData, (address, uint256, uint256)); + (address payable _contest, uint256 _termEnd, uint256 _transitionPeriod, uint256 _topK) = + abi.decode(_initData, (address, uint256, uint256, uint256)); - _checkContestSupportsSorting(GovernorCountingSimple(_underlyingContest)); + _checkContestSupportsSorting(GovernorCountingSimple(_contest)); // initialize the mutable state vars - underlyingContest = _underlyingContest; + nextContest = _contest; termEnd = _termEnd; + transitionPeriod = _transitionPeriod; topK = _topK; } @@ -125,8 +133,8 @@ contract JokeraceEligibility is HatsEligibilityModule { returns (bool eligible, bool standing) { standing = true; - if (block.timestamp < termEnd) { - eligible = eligibleWearersPerContest[_wearer][underlyingContest]; + if (block.timestamp < termEnd + transitionPeriod) { + eligible = eligibleWearersPerContest[_wearer][currentContest]; } } @@ -141,7 +149,7 @@ contract JokeraceEligibility is HatsEligibilityModule { * rejected. */ function pullElectionResults() public returns (bool success) { - GovernorCountingSimple currentContest = GovernorCountingSimple(payable(underlyingContest)); + GovernorCountingSimple currentContest = GovernorCountingSimple(payable(currentContest)); if (currentContest.state() != Governor.ContestState.Completed) { revert JokeraceEligibility_ContestNotCompleted(); @@ -161,6 +169,7 @@ contract JokeraceEligibility is HatsEligibilityModule { // if there's a tie if (winningProposalsCount > k) { termEnd = block.timestamp; // update the term end so that reelection will be immediately possible + nextContest = address(0); return false; } @@ -187,6 +196,8 @@ contract JokeraceEligibility is HatsEligibilityModule { } } + currentContest = nextContest; + nextContest = address(0); return true; } @@ -195,12 +206,12 @@ contract JokeraceEligibility is HatsEligibilityModule { * @dev Only the module's admin/s have the permission to set a reelection. If an admin is not set at the module * creation, then any admin of hatId is considered an admin by the module. */ - function reelection(address newUnderlyingContest, uint256 newTermEnd, uint256 newTopK) public { + function reelection(address newContest, uint256 newTermEnd, uint256 newTopK) public { if (!reelectionAllowed()) { revert JokeraceEligibility_TermNotCompleted(); } - _checkContestSupportsSorting(GovernorCountingSimple(payable(newUnderlyingContest))); + _checkContestSupportsSorting(GovernorCountingSimple(payable(newContest))); uint256 admin = ADMIN_HAT(); // if an admin hat is not set, then the Hats admins of hatId are granted the permission to set a reelection @@ -214,11 +225,11 @@ contract JokeraceEligibility is HatsEligibilityModule { } } - underlyingContest = newUnderlyingContest; + nextContest = newContest; termEnd = newTermEnd; topK = newTopK; - emit NewTerm(newUnderlyingContest, newTopK, newTermEnd); + emit NewTerm(newContest, newTopK, newTermEnd); } /*////////////////////////////////////////////////////////////// @@ -228,7 +239,7 @@ contract JokeraceEligibility is HatsEligibilityModule { /// @notice Check if setting a new election is allowed. function reelectionAllowed() public view returns (bool allowed) { allowed = block.timestamp >= termEnd - || GovernorCountingSimple(payable(underlyingContest)).state() == Governor.ContestState.Canceled; + || GovernorCountingSimple(payable(currentContest)).state() == Governor.ContestState.Canceled; } /*////////////////////////////////////////////////////////////// From 3868915fafc5453d538d0a6bc3d6f7a2f203a9f5 Mon Sep 17 00:00:00 2001 From: Ido Date: Tue, 13 Aug 2024 16:25:10 +0200 Subject: [PATCH 02/10] implementation and tests --- src/JokeraceEligibility.sol | 30 ++++---- test/JokeraceEligibility.t.sol | 122 ++++++++++++++++++++++++++------- 2 files changed, 115 insertions(+), 37 deletions(-) diff --git a/src/JokeraceEligibility.sol b/src/JokeraceEligibility.sol index f7c6f4e..b926aee 100644 --- a/src/JokeraceEligibility.sol +++ b/src/JokeraceEligibility.sol @@ -30,7 +30,7 @@ contract JokeraceEligibility is HatsEligibilityModule { //////////////////////////////////////////////////////////////*/ /// @notice Emitted when a reelection is set - event NewTerm(address NewContest, uint256 newTopK, uint256 newTermEnd); + event NewTerm(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); /*////////////////////////////////////////////////////////////// PUBLIC CONSTANTS @@ -95,6 +95,8 @@ contract JokeraceEligibility is HatsEligibilityModule { * @param _initData Packed initialization data with the following parameters: * _contest - Jokerace contest. The contest must have down-voting disabled and sorting enabled. * _termEnd - Final second of the current term (a unix timestamp) + * _transitionPeriod - Period of time after the term end when calling reelection is allowed and previous elected are + * still eligibile as long as reelection was not called * _topK - First K winners of the contest will be eligible */ function _setUp(bytes calldata _initData) internal override { @@ -149,20 +151,20 @@ contract JokeraceEligibility is HatsEligibilityModule { * rejected. */ function pullElectionResults() public returns (bool success) { - GovernorCountingSimple currentContest = GovernorCountingSimple(payable(currentContest)); + GovernorCountingSimple contest = GovernorCountingSimple(payable(nextContest)); - if (currentContest.state() != Governor.ContestState.Completed) { + if (contest.state() != Governor.ContestState.Completed) { revert JokeraceEligibility_ContestNotCompleted(); } uint256 k = topK; uint256 winningProposalsCount; for (uint256 currentRank = 1; currentRank <= k;) { - try currentContest.getRankIndex(currentRank) returns (uint256 rankIndex) { + try contest.getRankIndex(currentRank) returns (uint256 rankIndex) { // get the score of the curent rank (amount of 'for' votes) - uint256 forVotesOfCurrentRank = currentContest.sortedRanks(rankIndex); + uint256 forVotesOfCurrentRank = contest.sortedRanks(rankIndex); // get the proposal IDs with the current score - uint256[] memory proposalsOfCurrentRank = currentContest.getProposalsWithThisManyForVotes(forVotesOfCurrentRank); + uint256[] memory proposalsOfCurrentRank = contest.getProposalsWithThisManyForVotes(forVotesOfCurrentRank); uint256 numProposalsOfCurrentRank = proposalsOfCurrentRank.length; winningProposalsCount += numProposalsOfCurrentRank; @@ -175,8 +177,8 @@ contract JokeraceEligibility is HatsEligibilityModule { // get the authors of the proposals and update their eligibility for (uint256 proposalIndex; proposalIndex < numProposalsOfCurrentRank;) { - address candidate = _getCandidate(currentContest, proposalsOfCurrentRank[proposalIndex]); - eligibleWearersPerContest[candidate][address(currentContest)] = true; + address candidate = _getCandidate(contest, proposalsOfCurrentRank[proposalIndex]); + eligibleWearersPerContest[candidate][address(contest)] = true; unchecked { ++proposalIndex; @@ -196,7 +198,7 @@ contract JokeraceEligibility is HatsEligibilityModule { } } - currentContest = nextContest; + currentContest = address(contest); nextContest = address(0); return true; } @@ -206,7 +208,7 @@ contract JokeraceEligibility is HatsEligibilityModule { * @dev Only the module's admin/s have the permission to set a reelection. If an admin is not set at the module * creation, then any admin of hatId is considered an admin by the module. */ - function reelection(address newContest, uint256 newTermEnd, uint256 newTopK) public { + function reelection(address newContest, uint256 newTermEnd, uint256 newTransitionPeriod, uint256 newTopK) public { if (!reelectionAllowed()) { revert JokeraceEligibility_TermNotCompleted(); } @@ -227,9 +229,10 @@ contract JokeraceEligibility is HatsEligibilityModule { nextContest = newContest; termEnd = newTermEnd; + transitionPeriod = newTransitionPeriod; topK = newTopK; - emit NewTerm(newContest, newTopK, newTermEnd); + emit NewTerm(newContest, newTopK, newTermEnd, newTransitionPeriod); } /*////////////////////////////////////////////////////////////// @@ -239,7 +242,10 @@ contract JokeraceEligibility is HatsEligibilityModule { /// @notice Check if setting a new election is allowed. function reelectionAllowed() public view returns (bool allowed) { allowed = block.timestamp >= termEnd - || GovernorCountingSimple(payable(currentContest)).state() == Governor.ContestState.Canceled; + || ( + currentContest != address(0) + && GovernorCountingSimple(payable(currentContest)).state() == Governor.ContestState.Canceled + ); } /*////////////////////////////////////////////////////////////// diff --git a/test/JokeraceEligibility.t.sol b/test/JokeraceEligibility.t.sol index 1d2f1d0..bb4eaa9 100644 --- a/test/JokeraceEligibility.t.sol +++ b/test/JokeraceEligibility.t.sol @@ -20,7 +20,7 @@ contract DeployImplementationTest is DeployImplementation, Test { // bytes32 public SALT; uint256 public fork; - uint256 public BLOCK_NUMBER = 9_713_194; // the block number where hats module factory was deployed on Goerli; + uint256 public BLOCK_NUMBER = 6_488_268; // the block number where hats module factory was deployed on Goerli; IHats public constant HATS = IHats(0x3bc1A0Ad72417f2d411118085256fC53CBdDd137); // v1.hatsprotocol.eth string public FACTORY_VERSION = "factory test version"; @@ -28,7 +28,7 @@ contract DeployImplementationTest is DeployImplementation, Test { function setUp() public virtual { // create and activate a fork, at BLOCK_NUMBER - fork = vm.createSelectFork(vm.rpcUrl("goerli"), BLOCK_NUMBER); + fork = vm.createSelectFork(vm.rpcUrl("sepolia"), BLOCK_NUMBER); // deploy via the script DeployImplementation.prepare(JOKERACE_ELIGIBILITY_VERSION, false); // set last arg to true to log deployment DeployImplementation.run(); @@ -79,6 +79,8 @@ contract TestSetup is DeployImplementationTest { uint256 constant voteDelay = 3600; uint256 constant votePeriod = 3600; uint256 constant termPeriod = 86_400; + uint256 constant transitionPeriod = 604_800; + uint256 constant transitionPeriod2 = 1_209_600; enum ContestState { NotStarted, @@ -88,14 +90,18 @@ contract TestSetup is DeployImplementationTest { Completed } - function deployInstance(uint256 _winnersHat, uint256 _adminHat, address _contest, uint256 _termEnd, uint256 _topK) - public - returns (JokeraceEligibility) - { + function deployInstance( + uint256 _winnersHat, + uint256 _adminHat, + address _contest, + uint256 _termEnd, + uint256 _topK, + uint256 _transitionPeriod + ) public returns (JokeraceEligibility) { // encode the other immutable args as packed bytes otherImmutableArgs = abi.encodePacked(_adminHat); // encoded the initData as unpacked bytes - initData = abi.encode(_contest, _termEnd, _topK); + initData = abi.encode(_contest, _termEnd, _transitionPeriod, _topK); // deploy the instance return JokeraceEligibility( deployModuleInstance(FACTORY, address(implementation), _winnersHat, otherImmutableArgs, initData) @@ -156,12 +162,18 @@ contract TestSetup is DeployImplementationTest { HATS.mintHat(optionalAdminHat, optionalAdmin); vm.stopPrank(); // deploy the eligibility instance with a default admin - instanceDefaultAdmin = - deployInstance(winnersHat, uint256(0), address(contest), contestStart + voteDelay + votePeriod + termPeriod, 2); + instanceDefaultAdmin = deployInstance( + winnersHat, uint256(0), address(contest), contestStart + voteDelay + votePeriod + termPeriod, 2, transitionPeriod + ); // deploy the eligibility instance with a specific hat admin. This instance is used only to check correct admin // rights instanceHatAdmin = deployInstance( - winnersHat, optionalAdminHat, address(contest), contestStart + voteDelay + votePeriod + termPeriod, 2 + winnersHat, + optionalAdminHat, + address(contest), + contestStart + voteDelay + votePeriod + termPeriod, + 2, + transitionPeriod ); // update winners hat eligibilty to instance @@ -183,14 +195,22 @@ contract TestDeployment is TestSetup { assertEq(instanceHatAdmin.ADMIN_HAT(), optionalAdminHat); } - function test_instanceContest() public { - assertEq(address(instanceDefaultAdmin.underlyingContest()), address(contest)); + function test_instanceNextContest() public { + assertEq(address(instanceDefaultAdmin.nextContest()), address(contest)); + } + + function test_instanceCurrentContest() public { + assertEq(address(instanceDefaultAdmin.currentContest()), address(0)); } function test_instanceTermEnd() public { assertEq(instanceDefaultAdmin.termEnd(), contest.contestDeadline() + 86_400); } + function test_instanceTransitionPeriod() public { + assertEq(instanceDefaultAdmin.transitionPeriod(), transitionPeriod); + } + function test_instanceTopK() public { assertEq(instanceDefaultAdmin.topK(), 2); } @@ -284,7 +304,9 @@ contract TestProposing1Scenario is Proposing1Scenario { function test_setReelection_reverts() public { vm.expectRevert(JokeraceEligibility_TermNotCompleted.selector); - instanceDefaultAdmin.reelection(address(contest), contestStart + voteDelay + votePeriod + termPeriod, 2); + instanceDefaultAdmin.reelection( + address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 + ); } } @@ -337,7 +359,9 @@ contract TestVoting1Proposing1Scenario is Voting1Proposing1Scenario { function test_setReelection_reverts() public { vm.expectRevert(JokeraceEligibility_TermNotCompleted.selector); - instanceDefaultAdmin.reelection(address(contest), contestStart + voteDelay + votePeriod + termPeriod, 2); + instanceDefaultAdmin.reelection( + address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 + ); } } @@ -394,12 +418,16 @@ contract TestContestCompletedVoting2Proposing1Scenario is ContestCompletedVoting bool success = instanceDefaultAdmin.pullElectionResults(); assertEq(success, false); assertEq(instanceDefaultAdmin.termEnd(), block.timestamp); + assertEq(instanceDefaultAdmin.currentContest(), address(0)); + assertEq(instanceDefaultAdmin.nextContest(), address(0)); } } contract TestContestCompletedVoting1Proposing1Scenario is ContestCompletedVoting1Proposing1Scenario { function test_pullElectionResult() public { assertEq(pullElectionResultsSuccees, true); + assertEq(instanceDefaultAdmin.currentContest(), address(contest)); + assertEq(instanceDefaultAdmin.nextContest(), address(0)); } function test_eligibilityInstance() public { @@ -419,7 +447,9 @@ contract TestContestCompletedVoting1Proposing1Scenario is ContestCompletedVoting function test_setReelection_reverts() public { vm.expectRevert(JokeraceEligibility_TermNotCompleted.selector); - instanceDefaultAdmin.reelection(address(contest), contestStart + voteDelay + votePeriod + termPeriod, 2); + instanceDefaultAdmin.reelection( + address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 + ); } } @@ -436,10 +466,38 @@ contract TestTermEndedVoting1Proposing1Scenario is TermEndedVoting1Proposing1Sce function test_setReelectionNotAdmin_reverts() public { vm.startPrank(candidate1); vm.expectRevert(JokeraceEligibility_NotAdmin.selector); - instanceDefaultAdmin.reelection(address(contest), contestStart + voteDelay + votePeriod + termPeriod, 2); + instanceDefaultAdmin.reelection( + address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 + ); vm.stopPrank(); } + function test_eligibilityInstance() public { + (bool eligible1,) = instanceDefaultAdmin.getWearerStatus(candidate1, winnersHat); + assertEq(eligible1, true, "candidate 1 eligibility"); + (bool eligible2,) = instanceDefaultAdmin.getWearerStatus(candidate2, winnersHat); + assertEq(eligible2, false, "candidate 2 eligibility"); + (bool eligible3,) = instanceDefaultAdmin.getWearerStatus(candidate3, winnersHat); + assertEq(eligible3, true, "candidate 3 eligibility"); + } + + function test_eligibilityHats() public { + assertEq(HATS.isEligible(candidate1, winnersHat), true, "candidate 1 eligibility"); + assertEq(HATS.isEligible(candidate2, winnersHat), false, "candidate 2 eligibility"); + assertEq(HATS.isEligible(candidate3, winnersHat), true, "candidate 3 eligibility"); + } +} + +// Transition period ended, no reelection, previous elected should not be eligible anymore +contract TransitionPeriodEndedVoting1Proposing1Scenario is TermEndedVoting1Proposing1Scenario { + function setUp() public virtual override { + super.setUp(); + // set time to contest completion + vm.warp(contestStart + voteDelay + votePeriod + termPeriod + transitionPeriod + 1); + } +} + +contract TestTransitionPeriodEndedVoting1Proposing1Scenario is TransitionPeriodEndedVoting1Proposing1Scenario { function test_eligibilityInstance() public { (bool eligible1,) = instanceDefaultAdmin.getWearerStatus(candidate1, winnersHat); assertEq(eligible1, false, "candidate 1 eligibility"); @@ -483,10 +541,12 @@ contract TestReelectionVoting1Proposing1Scenario is TermEndedVoting1Proposing1Sc vm.prank(dao); uint256 newTermEnd = block.timestamp + voteDelay + votePeriod; uint256 newTopK = 5; - instanceDefaultAdmin.reelection(newContest, newTermEnd, newTopK); - assertEq(address(instanceDefaultAdmin.underlyingContest()), newContest); + instanceDefaultAdmin.reelection(newContest, newTermEnd, transitionPeriod2, newTopK); + assertEq(address(instanceDefaultAdmin.nextContest()), address(newContest)); + assertEq(address(instanceDefaultAdmin.currentContest()), address(contest)); assertEq(instanceDefaultAdmin.topK(), newTopK); assertEq(instanceDefaultAdmin.termEnd(), newTermEnd); + assertEq(instanceDefaultAdmin.transitionPeriod(), transitionPeriod2); } } @@ -500,7 +560,9 @@ contract TestReelectionHatAdmin is TestSetup { function test_reelectionByTopHat_reverts() public { vm.startPrank(dao); vm.expectRevert(JokeraceEligibility_NotAdmin.selector); - instanceHatAdmin.reelection(address(contest), contestStart + voteDelay + votePeriod + termPeriod, 2); + instanceHatAdmin.reelection( + address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 + ); vm.stopPrank(); } } @@ -518,11 +580,11 @@ contract TestReelectionDefaultAdmin is TestSetup { newTermEnd = contestStart + voteDelay + votePeriod + termPeriod + 86_000; newTopK = 5; vm.prank(optionalAdmin); - instanceHatAdmin.reelection(newContest, newTermEnd, newTopK); + instanceHatAdmin.reelection(newContest, newTermEnd, transitionPeriod, newTopK); } function test_reelectionDefaultAdmin() public { - assertEq(address(instanceHatAdmin.underlyingContest()), newContest); + assertEq(address(instanceHatAdmin.nextContest()), newContest); assertEq(instanceHatAdmin.topK(), newTopK); assertEq(instanceHatAdmin.termEnd(), newTermEnd); } @@ -538,7 +600,12 @@ contract TestSetupContestWithDownVoting is TestSetup { function test_setUp_reverts() public { vm.expectRevert(JokeraceEligibility_MustHaveDownvotingDisabled.selector); deployInstance( - winnersHat, uint256(1), address(contestWithDownVoting), contestStart + voteDelay + votePeriod + termPeriod, 2 + winnersHat, + uint256(1), + address(contestWithDownVoting), + contestStart + voteDelay + votePeriod + termPeriod, + 2, + transitionPeriod ); } } @@ -553,7 +620,12 @@ contract TestSetupContestWithSortingDisabled is TestSetup { function test_setUp_reverts() public { vm.expectRevert(JokeraceEligibility_MustHaveSortingEnabled.selector); deployInstance( - winnersHat, uint256(1), address(contestWithSortingDisabled), contestStart + voteDelay + votePeriod + termPeriod, 2 + winnersHat, + uint256(1), + address(contestWithSortingDisabled), + contestStart + voteDelay + votePeriod + termPeriod, + 2, + transitionPeriod ); } } @@ -568,7 +640,7 @@ contract TestReelectionContestWithDownVoting is TestSetup { function test_reelection_reverts() public { vm.startPrank(dao); vm.expectRevert(JokeraceEligibility_MustHaveDownvotingDisabled.selector); - instanceDefaultAdmin.reelection(address(contestWithDownVoting), block.timestamp + 86_400, 5); + instanceDefaultAdmin.reelection(address(contestWithDownVoting), block.timestamp + 86_400, transitionPeriod, 5); vm.stopPrank(); } } @@ -583,7 +655,7 @@ contract TestReelectionContestWithSortingDisabled is TestSetup { function test_reelection_reverts() public { vm.startPrank(dao); vm.expectRevert(JokeraceEligibility_MustHaveSortingEnabled.selector); - instanceDefaultAdmin.reelection(address(contestWithSortingDisabled), block.timestamp + 86_400, 5); + instanceDefaultAdmin.reelection(address(contestWithSortingDisabled), block.timestamp + 86_400, transitionPeriod, 5); vm.stopPrank(); } } From eb8adaa9872dcc9335c544d66e3c77ba918ec0bd Mon Sep 17 00:00:00 2001 From: Ido Date: Tue, 13 Aug 2024 16:32:06 +0200 Subject: [PATCH 03/10] format --- script/JokeraceEligibility.s.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/JokeraceEligibility.s.sol b/script/JokeraceEligibility.s.sol index 4104247..7f29f4e 100644 --- a/script/JokeraceEligibility.s.sol +++ b/script/JokeraceEligibility.s.sol @@ -26,7 +26,7 @@ contract DeployImplementation is Script { address deployer = vm.rememberKey(privKey); vm.startBroadcast(deployer); - implementation = new JokeraceEligibility{ salt: SALT}(version); + implementation = new JokeraceEligibility{ salt: SALT }(version); vm.stopBroadcast(); From 7de54f82b6a7d7c39f21f8b9bb2eeadf67a56f22 Mon Sep 17 00:00:00 2001 From: Ido Date: Wed, 14 Aug 2024 16:18:20 +0200 Subject: [PATCH 04/10] emit event on election pulling and update logic of allowed reelection time --- src/JokeraceEligibility.sol | 10 +++++++++- test/JokeraceEligibility.t.sol | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/JokeraceEligibility.sol b/src/JokeraceEligibility.sol index b926aee..566dfaa 100644 --- a/src/JokeraceEligibility.sol +++ b/src/JokeraceEligibility.sol @@ -31,6 +31,8 @@ contract JokeraceEligibility is HatsEligibilityModule { /// @notice Emitted when a reelection is set event NewTerm(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); + /// @notice Emitted when election's results are pulled + event ElectionResultsPulled(address NewContest); /*////////////////////////////////////////////////////////////// PUBLIC CONSTANTS @@ -200,6 +202,7 @@ contract JokeraceEligibility is HatsEligibilityModule { currentContest = address(contest); nextContest = address(0); + emit ElectionResultsPulled(address(contest)); return true; } @@ -241,10 +244,15 @@ contract JokeraceEligibility is HatsEligibilityModule { /// @notice Check if setting a new election is allowed. function reelectionAllowed() public view returns (bool allowed) { - allowed = block.timestamp >= termEnd + // if the current term has ended + allowed = block.timestamp >= termEnd // or if the current contest was canceled || ( currentContest != address(0) && GovernorCountingSimple(payable(currentContest)).state() == Governor.ContestState.Canceled + ) // or if we're in a transition period, and the next contest was canceled + || ( + nextContest != address(0) + && GovernorCountingSimple(payable(nextContest)).state() == Governor.ContestState.Canceled ); } diff --git a/test/JokeraceEligibility.t.sol b/test/JokeraceEligibility.t.sol index bb4eaa9..3f31f92 100644 --- a/test/JokeraceEligibility.t.sol +++ b/test/JokeraceEligibility.t.sol @@ -20,7 +20,7 @@ contract DeployImplementationTest is DeployImplementation, Test { // bytes32 public SALT; uint256 public fork; - uint256 public BLOCK_NUMBER = 6_488_268; // the block number where hats module factory was deployed on Goerli; + uint256 public BLOCK_NUMBER = 6_488_268; // the block number where hats module factory was deployed on Sepolia; IHats public constant HATS = IHats(0x3bc1A0Ad72417f2d411118085256fC53CBdDd137); // v1.hatsprotocol.eth string public FACTORY_VERSION = "factory test version"; From 6afd3dd72eade0674eea99553904b3728c49ff93 Mon Sep 17 00:00:00 2001 From: Ido Date: Wed, 14 Aug 2024 20:37:06 +0200 Subject: [PATCH 05/10] add tie indication in event and test events --- src/JokeraceEligibility.sol | 5 ++-- test/JokeraceEligibility.t.sol | 47 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/JokeraceEligibility.sol b/src/JokeraceEligibility.sol index 566dfaa..e04af95 100644 --- a/src/JokeraceEligibility.sol +++ b/src/JokeraceEligibility.sol @@ -32,7 +32,7 @@ contract JokeraceEligibility is HatsEligibilityModule { /// @notice Emitted when a reelection is set event NewTerm(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); /// @notice Emitted when election's results are pulled - event ElectionResultsPulled(address NewContest); + event ElectionResultsPulled(address NewContest, bool isTie); /*////////////////////////////////////////////////////////////// PUBLIC CONSTANTS @@ -174,6 +174,7 @@ contract JokeraceEligibility is HatsEligibilityModule { if (winningProposalsCount > k) { termEnd = block.timestamp; // update the term end so that reelection will be immediately possible nextContest = address(0); + emit ElectionResultsPulled(address(contest), true); return false; } @@ -202,7 +203,7 @@ contract JokeraceEligibility is HatsEligibilityModule { currentContest = address(contest); nextContest = address(0); - emit ElectionResultsPulled(address(contest)); + emit ElectionResultsPulled(address(contest), false); return true; } diff --git a/test/JokeraceEligibility.t.sol b/test/JokeraceEligibility.t.sol index 3f31f92..b39cee9 100644 --- a/test/JokeraceEligibility.t.sol +++ b/test/JokeraceEligibility.t.sol @@ -42,6 +42,9 @@ contract TestSetup is DeployImplementationTest { error JokeraceEligibility_MustHaveDownvotingDisabled(); error JokeraceEligibility_MustHaveSortingEnabled(); + event NewTerm(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); + event ElectionResultsPulled(address NewContest, bool isTie); + HatsModuleFactory constant FACTORY = HatsModuleFactory(0xfE661c01891172046feE16D3a57c3Cf456729efA); JokeraceEligibility public instanceDefaultAdmin; JokeraceEligibility public instanceHatAdmin; @@ -73,6 +76,7 @@ contract TestSetup is DeployImplementationTest { Contest contest; Contest contestWithDownVoting; Contest contestWithSortingDisabled; + Contest contestCanceled; //GenericVotesTimestampToken token; uint256[] args; uint256 contestStart; @@ -144,6 +148,10 @@ contract TestSetup is DeployImplementationTest { args.push(250); contest = new Contest("test contest", "contest", bytes32(0), votingMerkleRoot, args); + // set up a contest and cancel it + contestCanceled = new Contest("test contest", "contest", bytes32(0), votingMerkleRoot, args); + contestCanceled.cancel(); + // set up a contest with sorting enabled and with down voting args[5] = 1; contestWithDownVoting = new Contest("test contest", "contest", bytes32(0), votingMerkleRoot, args); @@ -373,6 +381,8 @@ contract ContestCompletedVoting1Proposing1Scenario is Voting1Proposing1Scenario super.setUp(); // set time to contest completion vm.warp(contestStart + voteDelay + votePeriod + 1); + vm.expectEmit(); + emit ElectionResultsPulled(address(contest), false); pullElectionResultsSuccees = instanceDefaultAdmin.pullElectionResults(); } } @@ -392,6 +402,8 @@ contract ContestCompletedProposing2Scenario is Proposing2Scenario { super.setUp(); // set time to contest completion vm.warp(contestStart + voteDelay + votePeriod + 1); + vm.expectEmit(); + emit ElectionResultsPulled(address(contest), false); instanceDefaultAdmin.pullElectionResults(); } } @@ -415,6 +427,8 @@ contract TestContestCompletedProposing2Scenario is ContestCompletedProposing2Sce contract TestContestCompletedVoting2Proposing1Scenario is ContestCompletedVoting2Proposing1Scenario { function test_pullElectionResults() public { + vm.expectEmit(); + emit ElectionResultsPulled(address(contest), true); bool success = instanceDefaultAdmin.pullElectionResults(); assertEq(success, false); assertEq(instanceDefaultAdmin.termEnd(), block.timestamp); @@ -514,6 +528,35 @@ contract TestTransitionPeriodEndedVoting1Proposing1Scenario is TransitionPeriodE } } +// Transition period ended, no reelection, previous elected should not be eligible anymore +contract NextContestCanceledVoting1Proposing1Scenario is TermEndedVoting1Proposing1Scenario { + function setUp() public virtual override { + super.setUp(); + uint256 newTermEnd = block.timestamp + voteDelay + votePeriod; + uint256 newTopK = 5; + vm.prank(dao); + vm.expectEmit(); + emit NewTerm(address(contestCanceled), newTopK, newTermEnd, transitionPeriod); + instanceDefaultAdmin.reelection(address(contestCanceled), newTermEnd, transitionPeriod, newTopK); + } +} + +contract TestNextContestCanceledVoting1Proposing1Scenario is NextContestCanceledVoting1Proposing1Scenario { + function test_canSetNewElection() public { + vm.prank(dao); + uint256 newTermEnd = block.timestamp + voteDelay + votePeriod; + uint256 newTopK = 5; + vm.expectEmit(); + emit NewTerm(address(contest), newTopK, newTermEnd, transitionPeriod2); + instanceDefaultAdmin.reelection(address(contest), newTermEnd, transitionPeriod2, newTopK); + assertEq(address(instanceDefaultAdmin.nextContest()), address(contest)); + assertEq(address(instanceDefaultAdmin.currentContest()), address(contest)); + assertEq(instanceDefaultAdmin.topK(), newTopK); + assertEq(instanceDefaultAdmin.termEnd(), newTermEnd); + assertEq(instanceDefaultAdmin.transitionPeriod(), transitionPeriod2); + } +} + contract TestReelectionVoting1Proposing1Scenario is TermEndedVoting1Proposing1Scenario { address public newContest; @@ -541,6 +584,8 @@ contract TestReelectionVoting1Proposing1Scenario is TermEndedVoting1Proposing1Sc vm.prank(dao); uint256 newTermEnd = block.timestamp + voteDelay + votePeriod; uint256 newTopK = 5; + vm.expectEmit(); + emit NewTerm(address(newContest), newTopK, newTermEnd, transitionPeriod2); instanceDefaultAdmin.reelection(newContest, newTermEnd, transitionPeriod2, newTopK); assertEq(address(instanceDefaultAdmin.nextContest()), address(newContest)); assertEq(address(instanceDefaultAdmin.currentContest()), address(contest)); @@ -580,6 +625,8 @@ contract TestReelectionDefaultAdmin is TestSetup { newTermEnd = contestStart + voteDelay + votePeriod + termPeriod + 86_000; newTopK = 5; vm.prank(optionalAdmin); + vm.expectEmit(); + emit NewTerm(address(newContest), newTopK, newTermEnd, transitionPeriod); instanceHatAdmin.reelection(newContest, newTermEnd, transitionPeriod, newTopK); } From 72c09ec05feeb48d0202a9172f6360cded875af6 Mon Sep 17 00:00:00 2001 From: Ido Date: Fri, 23 Aug 2024 16:32:08 +0200 Subject: [PATCH 06/10] implementation updates --- src/JokeraceEligibility.sol | 114 +++++++------- test/JokeraceEligibility.t.sol | 263 +++++++++++++++------------------ 2 files changed, 178 insertions(+), 199 deletions(-) diff --git a/src/JokeraceEligibility.sol b/src/JokeraceEligibility.sol index e04af95..b131127 100644 --- a/src/JokeraceEligibility.sol +++ b/src/JokeraceEligibility.sol @@ -16,6 +16,8 @@ contract JokeraceEligibility is HatsEligibilityModule { /// @notice Indicates that the underlying contest has not completed yet error JokeraceEligibility_ContestNotCompleted(); + /// @notice Indicates that the next contest's results are a tie + error JokeraceEligibility_ContestTie(); /// @notice Indicates that the current term is still on-going error JokeraceEligibility_TermNotCompleted(); /// @notice Indicates that the caller doesn't have admin permsissions @@ -30,9 +32,9 @@ contract JokeraceEligibility is HatsEligibilityModule { //////////////////////////////////////////////////////////////*/ /// @notice Emitted when a reelection is set - event NewTerm(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); + event NextTermSet(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); /// @notice Emitted when election's results are pulled - event ElectionResultsPulled(address NewContest, bool isTie); + event TermStarted(address contest, uint256 topK, uint256 termEnd, uint256 transitionPeriod); /*////////////////////////////////////////////////////////////// PUBLIC CONSTANTS @@ -66,25 +68,31 @@ contract JokeraceEligibility is HatsEligibilityModule { return _getArgUint256(72); } + /*////////////////////////////////////////////////////////////// + Data Structures + //////////////////////////////////////////////////////////////*/ + + struct TermDetails { + /// @notice Current Jokerace contest (election) + address contest; + /// @notice First K winners of the contest will be eligible + uint96 topK; + /// @notice First second after the term (a unix timestamp) + uint256 termEnd; + /// @notice Period of time after the term end when previous elected members are still considered eligible until a + /// new term is set. + uint256 transitionPeriod; + } + /*////////////////////////////////////////////////////////////// MUTABLE STATE //////////////////////////////////////////////////////////////*/ - /// @notice Current Jokerace contest (election) - address public currentContest; - /// @notice Next Jokerace contest (election) - address public nextContest; - /// @notice First second after the current term (a unix timestamp) - uint256 public termEnd; - /** - * @notice Period of time after the term end when reelection is allowed. During this period, previous elected members - * are still considered eligible until a new term is set. - */ - uint256 public transitionPeriod; - /// @notice First K winners of the contest will be eligible - uint256 public topK; - /// @notice Eligible wearers according to each contest - mapping(address wearer => mapping(address contest => bool eligible)) public eligibleWearersPerContest; + mapping(uint256 termIndex => TermDetails termDetails) public terms; + + uint256 public currentTermIndex; + + mapping(address wearer => mapping(uint256 termIndex => bool eligible)) public eligibleWearersPerTerm; /*////////////////////////////////////////////////////////////// INITIALIZER @@ -102,16 +110,12 @@ contract JokeraceEligibility is HatsEligibilityModule { * _topK - First K winners of the contest will be eligible */ function _setUp(bytes calldata _initData) internal override { - (address payable _contest, uint256 _termEnd, uint256 _transitionPeriod, uint256 _topK) = - abi.decode(_initData, (address, uint256, uint256, uint256)); + (address payable _contest, uint256 _termEnd, uint256 _transitionPeriod, uint96 _topK) = + abi.decode(_initData, (address, uint256, uint256, uint96)); _checkContestSupportsSorting(GovernorCountingSimple(_contest)); - // initialize the mutable state vars - nextContest = _contest; - termEnd = _termEnd; - transitionPeriod = _transitionPeriod; - topK = _topK; + terms[1] = TermDetails({ contest: _contest, topK: _topK, termEnd: _termEnd, transitionPeriod: _transitionPeriod }); } /*////////////////////////////////////////////////////////////// @@ -137,8 +141,9 @@ contract JokeraceEligibility is HatsEligibilityModule { returns (bool eligible, bool standing) { standing = true; - if (block.timestamp < termEnd + transitionPeriod) { - eligible = eligibleWearersPerContest[_wearer][currentContest]; + TermDetails memory currentTerm = terms[currentTermIndex]; + if (block.timestamp < currentTerm.termEnd + currentTerm.transitionPeriod) { + eligible = eligibleWearersPerTerm[_wearer][currentTermIndex]; } } @@ -152,14 +157,19 @@ contract JokeraceEligibility is HatsEligibilityModule { * tie, meaning that candidates in places K and K+1 have the same score, then the results of this contest are * rejected. */ - function pullElectionResults() public returns (bool success) { - GovernorCountingSimple contest = GovernorCountingSimple(payable(nextContest)); + function startNextTerm() public { + TermDetails memory nextTerm = terms[currentTermIndex + 1]; + GovernorCountingSimple contest = GovernorCountingSimple(payable(nextTerm.contest)); + uint96 k = nextTerm.topK; + + if (!_canStartNextTerm(terms[currentTermIndex].termEnd)) { + revert JokeraceEligibility_TermNotCompleted(); + } if (contest.state() != Governor.ContestState.Completed) { revert JokeraceEligibility_ContestNotCompleted(); } - uint256 k = topK; uint256 winningProposalsCount; for (uint256 currentRank = 1; currentRank <= k;) { try contest.getRankIndex(currentRank) returns (uint256 rankIndex) { @@ -172,16 +182,13 @@ contract JokeraceEligibility is HatsEligibilityModule { // if there's a tie if (winningProposalsCount > k) { - termEnd = block.timestamp; // update the term end so that reelection will be immediately possible - nextContest = address(0); - emit ElectionResultsPulled(address(contest), true); - return false; + revert JokeraceEligibility_ContestTie(); } // get the authors of the proposals and update their eligibility for (uint256 proposalIndex; proposalIndex < numProposalsOfCurrentRank;) { address candidate = _getCandidate(contest, proposalsOfCurrentRank[proposalIndex]); - eligibleWearersPerContest[candidate][address(contest)] = true; + eligibleWearersPerTerm[candidate][currentTermIndex + 1] = true; unchecked { ++proposalIndex; @@ -201,10 +208,8 @@ contract JokeraceEligibility is HatsEligibilityModule { } } - currentContest = address(contest); - nextContest = address(0); - emit ElectionResultsPulled(address(contest), false); - return true; + currentTermIndex += 1; + emit TermStarted(nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod); } /** @@ -212,11 +217,7 @@ contract JokeraceEligibility is HatsEligibilityModule { * @dev Only the module's admin/s have the permission to set a reelection. If an admin is not set at the module * creation, then any admin of hatId is considered an admin by the module. */ - function reelection(address newContest, uint256 newTermEnd, uint256 newTransitionPeriod, uint256 newTopK) public { - if (!reelectionAllowed()) { - revert JokeraceEligibility_TermNotCompleted(); - } - + function setNextTerm(address newContest, uint256 newTermEnd, uint256 newTransitionPeriod, uint96 newTopK) public { _checkContestSupportsSorting(GovernorCountingSimple(payable(newContest))); uint256 admin = ADMIN_HAT(); @@ -231,12 +232,10 @@ contract JokeraceEligibility is HatsEligibilityModule { } } - nextContest = newContest; - termEnd = newTermEnd; - transitionPeriod = newTransitionPeriod; - topK = newTopK; + terms[currentTermIndex + 1] = + TermDetails({ contest: newContest, topK: newTopK, termEnd: newTermEnd, transitionPeriod: newTransitionPeriod }); - emit NewTerm(newContest, newTopK, newTermEnd, newTransitionPeriod); + emit NextTermSet(newContest, newTopK, newTermEnd, newTransitionPeriod); } /*////////////////////////////////////////////////////////////// @@ -244,23 +243,20 @@ contract JokeraceEligibility is HatsEligibilityModule { //////////////////////////////////////////////////////////////*/ /// @notice Check if setting a new election is allowed. - function reelectionAllowed() public view returns (bool allowed) { - // if the current term has ended - allowed = block.timestamp >= termEnd // or if the current contest was canceled - || ( - currentContest != address(0) - && GovernorCountingSimple(payable(currentContest)).state() == Governor.ContestState.Canceled - ) // or if we're in a transition period, and the next contest was canceled - || ( - nextContest != address(0) - && GovernorCountingSimple(payable(nextContest)).state() == Governor.ContestState.Canceled - ); + function canStartNextTerm() public view returns (bool allowed) { + // If the current term has ended + return _canStartNextTerm(terms[currentTermIndex].termEnd); } /*////////////////////////////////////////////////////////////// INTERNAL FUNCTIONS //////////////////////////////////////////////////////////////*/ + /// @notice Check if setting a new election is allowed. + function _canStartNextTerm(uint256 currentTermEnd) internal view returns (bool allowed) { + allowed = block.timestamp > currentTermEnd; + } + function _getCandidate(GovernorCountingSimple contest, uint256 proposalId) internal view returns (address candidate) { candidate = contest.getProposal(proposalId).author; } diff --git a/test/JokeraceEligibility.t.sol b/test/JokeraceEligibility.t.sol index b39cee9..516a87c 100644 --- a/test/JokeraceEligibility.t.sol +++ b/test/JokeraceEligibility.t.sol @@ -37,13 +37,14 @@ contract DeployImplementationTest is DeployImplementation, Test { contract TestSetup is DeployImplementationTest { error JokeraceEligibility_ContestNotCompleted(); + error JokeraceEligibility_ContestTie(); error JokeraceEligibility_TermNotCompleted(); error JokeraceEligibility_NotAdmin(); error JokeraceEligibility_MustHaveDownvotingDisabled(); error JokeraceEligibility_MustHaveSortingEnabled(); - event NewTerm(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); - event ElectionResultsPulled(address NewContest, bool isTie); + event NextTermSet(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); + event TermStarted(address contest, uint256 topK, uint256 termEnd, uint256 transitionPeriod); HatsModuleFactory constant FACTORY = HatsModuleFactory(0xfE661c01891172046feE16D3a57c3Cf456729efA); JokeraceEligibility public instanceDefaultAdmin; @@ -85,6 +86,7 @@ contract TestSetup is DeployImplementationTest { uint256 constant termPeriod = 86_400; uint256 constant transitionPeriod = 604_800; uint256 constant transitionPeriod2 = 1_209_600; + uint256 termEnd1; enum ContestState { NotStarted, @@ -94,6 +96,13 @@ contract TestSetup is DeployImplementationTest { Completed } + struct TermDetails { + address contest; + uint96 topK; + uint256 termEnd; + uint256 transitionPeriod; + } + function deployInstance( uint256 _winnersHat, uint256 _adminHat, @@ -128,6 +137,7 @@ contract TestSetup is DeployImplementationTest { function setUp() public virtual override { super.setUp(); contestStart = block.timestamp; + termEnd1 = contestStart + voteDelay + votePeriod + termPeriod; // set up a contest with sorting enabled and without down voting leaf1 = keccak256(abi.encodePacked(candidate1, uint256(100))); leaf2 = keccak256(abi.encodePacked(candidate2, uint256(100))); @@ -170,19 +180,10 @@ contract TestSetup is DeployImplementationTest { HATS.mintHat(optionalAdminHat, optionalAdmin); vm.stopPrank(); // deploy the eligibility instance with a default admin - instanceDefaultAdmin = deployInstance( - winnersHat, uint256(0), address(contest), contestStart + voteDelay + votePeriod + termPeriod, 2, transitionPeriod - ); + instanceDefaultAdmin = deployInstance(winnersHat, uint256(0), address(contest), termEnd1, 2, transitionPeriod); // deploy the eligibility instance with a specific hat admin. This instance is used only to check correct admin // rights - instanceHatAdmin = deployInstance( - winnersHat, - optionalAdminHat, - address(contest), - contestStart + voteDelay + votePeriod + termPeriod, - 2, - transitionPeriod - ); + instanceHatAdmin = deployInstance(winnersHat, optionalAdminHat, address(contest), termEnd1, 2, transitionPeriod); // update winners hat eligibilty to instance vm.prank(dao); @@ -203,24 +204,26 @@ contract TestDeployment is TestSetup { assertEq(instanceHatAdmin.ADMIN_HAT(), optionalAdminHat); } - function test_instanceNextContest() public { - assertEq(address(instanceDefaultAdmin.nextContest()), address(contest)); + function test_instanceNextTerm() public { + uint256 currentTermIndex = instanceDefaultAdmin.currentTermIndex(); + TermDetails memory nextTerm; + (nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod) = + instanceDefaultAdmin.terms(currentTermIndex + 1); + assertEq(address(nextTerm.contest), address(contest)); + assertEq(nextTerm.termEnd, termEnd1); + assertEq(nextTerm.topK, 2); + assertEq(nextTerm.transitionPeriod, transitionPeriod); } - function test_instanceCurrentContest() public { - assertEq(address(instanceDefaultAdmin.currentContest()), address(0)); - } - - function test_instanceTermEnd() public { - assertEq(instanceDefaultAdmin.termEnd(), contest.contestDeadline() + 86_400); - } - - function test_instanceTransitionPeriod() public { - assertEq(instanceDefaultAdmin.transitionPeriod(), transitionPeriod); - } - - function test_instanceTopK() public { - assertEq(instanceDefaultAdmin.topK(), 2); + function test_instanceCurrentTerm() public { + uint256 currentTermIndex = instanceDefaultAdmin.currentTermIndex(); + TermDetails memory currentTerm; + (currentTerm.contest, currentTerm.topK, currentTerm.termEnd, currentTerm.transitionPeriod) = + instanceDefaultAdmin.terms(currentTermIndex); + assertEq(currentTerm.contest, address(0)); + assertEq(currentTerm.termEnd, 0); + assertEq(currentTerm.topK, 0); + assertEq(currentTerm.transitionPeriod, 0); } function test_hatEligibility() public { @@ -305,16 +308,9 @@ contract TestProposing1Scenario is Proposing1Scenario { assertEq(proposalIds.length, 3, "number of proposals"); } - function test_pullContestResults_reverts() public { + function test_startNextTerm_reverts() public { vm.expectRevert(JokeraceEligibility_ContestNotCompleted.selector); - instanceDefaultAdmin.pullElectionResults(); - } - - function test_setReelection_reverts() public { - vm.expectRevert(JokeraceEligibility_TermNotCompleted.selector); - instanceDefaultAdmin.reelection( - address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 - ); + instanceDefaultAdmin.startNextTerm(); } } @@ -360,30 +356,25 @@ contract TestVoting1Proposing1Scenario is Voting1Proposing1Scenario { assertEq(int256(forVotes3) - int256(againstVotes3), 100, "candidate 3 votes"); } - function test_pullContestResults_reverts() public { + function test_startNextTerm_reverts() public { vm.expectRevert(JokeraceEligibility_ContestNotCompleted.selector); - instanceDefaultAdmin.pullElectionResults(); - } - - function test_setReelection_reverts() public { - vm.expectRevert(JokeraceEligibility_TermNotCompleted.selector); - instanceDefaultAdmin.reelection( - address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 - ); + instanceDefaultAdmin.startNextTerm(); } } // Contest completed with candidates 1 & 2 as winners contract ContestCompletedVoting1Proposing1Scenario is Voting1Proposing1Scenario { - bool pullElectionResultsSuccees; - function setUp() public virtual override { super.setUp(); // set time to contest completion vm.warp(contestStart + voteDelay + votePeriod + 1); + uint256 currentTermIndex = instanceDefaultAdmin.currentTermIndex(); + TermDetails memory nextTerm; + (nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod) = + instanceDefaultAdmin.terms(currentTermIndex + 1); vm.expectEmit(); - emit ElectionResultsPulled(address(contest), false); - pullElectionResultsSuccees = instanceDefaultAdmin.pullElectionResults(); + emit TermStarted(nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod); + instanceDefaultAdmin.startNextTerm(); } } @@ -402,9 +393,13 @@ contract ContestCompletedProposing2Scenario is Proposing2Scenario { super.setUp(); // set time to contest completion vm.warp(contestStart + voteDelay + votePeriod + 1); + uint256 currentTermIndex = instanceDefaultAdmin.currentTermIndex(); + TermDetails memory nextTerm; + (nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod) = + instanceDefaultAdmin.terms(currentTermIndex + 1); vm.expectEmit(); - emit ElectionResultsPulled(address(contest), false); - instanceDefaultAdmin.pullElectionResults(); + emit TermStarted(address(nextTerm.contest), nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod); + instanceDefaultAdmin.startNextTerm(); } } @@ -426,22 +421,46 @@ contract TestContestCompletedProposing2Scenario is ContestCompletedProposing2Sce } contract TestContestCompletedVoting2Proposing1Scenario is ContestCompletedVoting2Proposing1Scenario { - function test_pullElectionResults() public { - vm.expectEmit(); - emit ElectionResultsPulled(address(contest), true); - bool success = instanceDefaultAdmin.pullElectionResults(); - assertEq(success, false); - assertEq(instanceDefaultAdmin.termEnd(), block.timestamp); - assertEq(instanceDefaultAdmin.currentContest(), address(0)); - assertEq(instanceDefaultAdmin.nextContest(), address(0)); + function test_startNextTerm() public { + vm.expectRevert(JokeraceEligibility_ContestTie.selector); + instanceDefaultAdmin.startNextTerm(); + uint256 currentTermIndex = instanceDefaultAdmin.currentTermIndex(); + assertEq(currentTermIndex, 0); + TermDetails memory currentTerm; + (currentTerm.contest, currentTerm.topK, currentTerm.termEnd, currentTerm.transitionPeriod) = + instanceDefaultAdmin.terms(currentTermIndex); + assertEq(currentTerm.contest, address(0)); + assertEq(currentTerm.topK, 0); + assertEq(currentTerm.termEnd, 0); + assertEq(currentTerm.transitionPeriod, 0); + TermDetails memory nextTerm; + (nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod) = + instanceDefaultAdmin.terms(currentTermIndex + 1); + assertEq(nextTerm.contest, address(contest)); + assertEq(nextTerm.topK, 2); + assertEq(nextTerm.termEnd, termEnd1); + assertEq(nextTerm.transitionPeriod, transitionPeriod); } } contract TestContestCompletedVoting1Proposing1Scenario is ContestCompletedVoting1Proposing1Scenario { - function test_pullElectionResult() public { - assertEq(pullElectionResultsSuccees, true); - assertEq(instanceDefaultAdmin.currentContest(), address(contest)); - assertEq(instanceDefaultAdmin.nextContest(), address(0)); + function test_startNextTermResult() public { + uint256 currentTermIndex = instanceDefaultAdmin.currentTermIndex(); + assertEq(currentTermIndex, 1); + TermDetails memory currentTerm; + (currentTerm.contest, currentTerm.topK, currentTerm.termEnd, currentTerm.transitionPeriod) = + instanceDefaultAdmin.terms(currentTermIndex); + assertEq(currentTerm.contest, address(contest)); + assertEq(currentTerm.topK, 2); + assertEq(currentTerm.termEnd, termEnd1); + assertEq(currentTerm.transitionPeriod, transitionPeriod); + TermDetails memory nextTerm; + (nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod) = + instanceDefaultAdmin.terms(currentTermIndex + 1); + assertEq(nextTerm.contest, address(0)); + assertEq(nextTerm.topK, 0); + assertEq(nextTerm.termEnd, 0); + assertEq(nextTerm.transitionPeriod, 0); } function test_eligibilityInstance() public { @@ -458,13 +477,6 @@ contract TestContestCompletedVoting1Proposing1Scenario is ContestCompletedVoting assertEq(HATS.isEligible(candidate2, winnersHat), false, "candidate 2 eligibility"); assertEq(HATS.isEligible(candidate3, winnersHat), true, "candidate 3 eligibility"); } - - function test_setReelection_reverts() public { - vm.expectRevert(JokeraceEligibility_TermNotCompleted.selector); - instanceDefaultAdmin.reelection( - address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 - ); - } } // Current term ended, ready for reelection @@ -477,12 +489,10 @@ contract TermEndedVoting1Proposing1Scenario is ContestCompletedVoting1Proposing1 } contract TestTermEndedVoting1Proposing1Scenario is TermEndedVoting1Proposing1Scenario { - function test_setReelectionNotAdmin_reverts() public { + function test_setNextTermnNotAdmin_reverts() public { vm.startPrank(candidate1); vm.expectRevert(JokeraceEligibility_NotAdmin.selector); - instanceDefaultAdmin.reelection( - address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 - ); + instanceDefaultAdmin.setNextTerm(address(contest), termEnd1, transitionPeriod, 2); vm.stopPrank(); } @@ -507,7 +517,7 @@ contract TransitionPeriodEndedVoting1Proposing1Scenario is TermEndedVoting1Propo function setUp() public virtual override { super.setUp(); // set time to contest completion - vm.warp(contestStart + voteDelay + votePeriod + termPeriod + transitionPeriod + 1); + vm.warp(termEnd1 + transitionPeriod + 1); } } @@ -533,31 +543,31 @@ contract NextContestCanceledVoting1Proposing1Scenario is TermEndedVoting1Proposi function setUp() public virtual override { super.setUp(); uint256 newTermEnd = block.timestamp + voteDelay + votePeriod; - uint256 newTopK = 5; + uint96 newTopK = 5; vm.prank(dao); vm.expectEmit(); - emit NewTerm(address(contestCanceled), newTopK, newTermEnd, transitionPeriod); - instanceDefaultAdmin.reelection(address(contestCanceled), newTermEnd, transitionPeriod, newTopK); + emit NextTermSet(address(contestCanceled), newTopK, newTermEnd, transitionPeriod); + instanceDefaultAdmin.setNextTerm(address(contestCanceled), newTermEnd, transitionPeriod, newTopK); } } contract TestNextContestCanceledVoting1Proposing1Scenario is NextContestCanceledVoting1Proposing1Scenario { + function test_startNextTerm_reverts() public { + vm.expectRevert(JokeraceEligibility_ContestNotCompleted.selector); + instanceDefaultAdmin.startNextTerm(); + } + function test_canSetNewElection() public { vm.prank(dao); uint256 newTermEnd = block.timestamp + voteDelay + votePeriod; - uint256 newTopK = 5; + uint96 newTopK = 5; vm.expectEmit(); - emit NewTerm(address(contest), newTopK, newTermEnd, transitionPeriod2); - instanceDefaultAdmin.reelection(address(contest), newTermEnd, transitionPeriod2, newTopK); - assertEq(address(instanceDefaultAdmin.nextContest()), address(contest)); - assertEq(address(instanceDefaultAdmin.currentContest()), address(contest)); - assertEq(instanceDefaultAdmin.topK(), newTopK); - assertEq(instanceDefaultAdmin.termEnd(), newTermEnd); - assertEq(instanceDefaultAdmin.transitionPeriod(), transitionPeriod2); + emit NextTermSet(address(contest), newTopK, newTermEnd, transitionPeriod2); + instanceDefaultAdmin.setNextTerm(address(contest), newTermEnd, transitionPeriod2, newTopK); } } -contract TestReelectionVoting1Proposing1Scenario is TermEndedVoting1Proposing1Scenario { +contract TestSetNextTermVoting1Proposing1Scenario is TermEndedVoting1Proposing1Scenario { address public newContest; function setUp() public virtual override { @@ -583,67 +593,58 @@ contract TestReelectionVoting1Proposing1Scenario is TermEndedVoting1Proposing1Sc function test_reelection() public { vm.prank(dao); uint256 newTermEnd = block.timestamp + voteDelay + votePeriod; - uint256 newTopK = 5; + uint96 newTopK = 5; vm.expectEmit(); - emit NewTerm(address(newContest), newTopK, newTermEnd, transitionPeriod2); - instanceDefaultAdmin.reelection(newContest, newTermEnd, transitionPeriod2, newTopK); - assertEq(address(instanceDefaultAdmin.nextContest()), address(newContest)); - assertEq(address(instanceDefaultAdmin.currentContest()), address(contest)); - assertEq(instanceDefaultAdmin.topK(), newTopK); - assertEq(instanceDefaultAdmin.termEnd(), newTermEnd); - assertEq(instanceDefaultAdmin.transitionPeriod(), transitionPeriod2); + emit NextTermSet(address(newContest), newTopK, newTermEnd, transitionPeriod2); + instanceDefaultAdmin.setNextTerm(newContest, newTermEnd, transitionPeriod2, newTopK); } } -contract TestReelectionHatAdmin is TestSetup { +contract TestSetNextTermHatAdmin is TestSetup { function setUp() public virtual override { super.setUp(); // set time to contest completion - vm.warp(contestStart + voteDelay + votePeriod + termPeriod + 1); + vm.warp(termEnd1 + 1); } - function test_reelectionByTopHat_reverts() public { + function test_setNextTermByTopHat_reverts() public { vm.startPrank(dao); vm.expectRevert(JokeraceEligibility_NotAdmin.selector); - instanceHatAdmin.reelection( + instanceHatAdmin.setNextTerm( address(contest), contestStart + voteDelay + votePeriod + termPeriod, transitionPeriod, 2 ); vm.stopPrank(); } } -contract TestReelectionDefaultAdmin is TestSetup { +contract TestSetNextTermDefaultAdmin is TestSetup { address newContest; uint256 newTermEnd; - uint256 newTopK; + uint96 newTopK; function setUp() public virtual override { super.setUp(); - // set time to contest completion - vm.warp(contestStart + voteDelay + votePeriod + termPeriod + 1); newContest = address(contest); - newTermEnd = contestStart + voteDelay + votePeriod + termPeriod + 86_000; + newTermEnd = termEnd1 + 86_000; newTopK = 5; vm.prank(optionalAdmin); vm.expectEmit(); - emit NewTerm(address(newContest), newTopK, newTermEnd, transitionPeriod); - instanceHatAdmin.reelection(newContest, newTermEnd, transitionPeriod, newTopK); + emit NextTermSet(address(newContest), newTopK, newTermEnd, transitionPeriod); + instanceHatAdmin.setNextTerm(newContest, newTermEnd, transitionPeriod, newTopK); } - function test_reelectionDefaultAdmin() public { - assertEq(address(instanceHatAdmin.nextContest()), newContest); - assertEq(instanceHatAdmin.topK(), newTopK); - assertEq(instanceHatAdmin.termEnd(), newTermEnd); + function test_setNextTermDefaultAdmin() public { + assertEq(instanceHatAdmin.currentTermIndex(), 0); + TermDetails memory nextTerm; + (nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod) = instanceHatAdmin.terms(1); + assertEq(address(nextTerm.contest), address(newContest)); + assertEq(nextTerm.termEnd, newTermEnd); + assertEq(nextTerm.topK, newTopK); + assertEq(nextTerm.transitionPeriod, transitionPeriod); } } contract TestSetupContestWithDownVoting is TestSetup { - function setUp() public virtual override { - super.setUp(); - // set time to contest completion - vm.warp(contestStart + voteDelay + votePeriod + termPeriod + 1); - } - function test_setUp_reverts() public { vm.expectRevert(JokeraceEligibility_MustHaveDownvotingDisabled.selector); deployInstance( @@ -658,12 +659,6 @@ contract TestSetupContestWithDownVoting is TestSetup { } contract TestSetupContestWithSortingDisabled is TestSetup { - function setUp() public virtual override { - super.setUp(); - // set time to contest completion - vm.warp(contestStart + voteDelay + votePeriod + termPeriod + 1); - } - function test_setUp_reverts() public { vm.expectRevert(JokeraceEligibility_MustHaveSortingEnabled.selector); deployInstance( @@ -677,32 +672,20 @@ contract TestSetupContestWithSortingDisabled is TestSetup { } } -contract TestReelectionContestWithDownVoting is TestSetup { - function setUp() public virtual override { - super.setUp(); - // set time to contest completion - vm.warp(contestStart + voteDelay + votePeriod + termPeriod + 1); - } - +contract TestSetNextTermContestWithDownVoting is TestSetup { function test_reelection_reverts() public { vm.startPrank(dao); vm.expectRevert(JokeraceEligibility_MustHaveDownvotingDisabled.selector); - instanceDefaultAdmin.reelection(address(contestWithDownVoting), block.timestamp + 86_400, transitionPeriod, 5); + instanceDefaultAdmin.setNextTerm(address(contestWithDownVoting), block.timestamp + 86_400, transitionPeriod, 5); vm.stopPrank(); } } -contract TestReelectionContestWithSortingDisabled is TestSetup { - function setUp() public virtual override { - super.setUp(); - // set time to contest completion - vm.warp(contestStart + voteDelay + votePeriod + termPeriod + 1); - } - +contract TestSetNextTermContestWithSortingDisabled is TestSetup { function test_reelection_reverts() public { vm.startPrank(dao); vm.expectRevert(JokeraceEligibility_MustHaveSortingEnabled.selector); - instanceDefaultAdmin.reelection(address(contestWithSortingDisabled), block.timestamp + 86_400, transitionPeriod, 5); + instanceDefaultAdmin.setNextTerm(address(contestWithSortingDisabled), block.timestamp + 86_400, transitionPeriod, 5); vm.stopPrank(); } } From 6479a0d0523434b22361dbac015cf335775edf43 Mon Sep 17 00:00:00 2001 From: Ido Date: Fri, 23 Aug 2024 19:00:33 +0200 Subject: [PATCH 07/10] documentation and optimizations --- src/JokeraceEligibility.sol | 45 +++++++++++++++++++--------------- test/JokeraceEligibility.t.sol | 45 ++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 20 deletions(-) diff --git a/src/JokeraceEligibility.sol b/src/JokeraceEligibility.sol index b131127..cf428e3 100644 --- a/src/JokeraceEligibility.sol +++ b/src/JokeraceEligibility.sol @@ -31,9 +31,9 @@ contract JokeraceEligibility is HatsEligibilityModule { EVENTS //////////////////////////////////////////////////////////////*/ - /// @notice Emitted when a reelection is set + /// @notice Emitted when the next term is set event NextTermSet(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); - /// @notice Emitted when election's results are pulled + /// @notice Emitted when the next term is started event TermStarted(address contest, uint256 topK, uint256 termEnd, uint256 transitionPeriod); /*////////////////////////////////////////////////////////////// @@ -73,14 +73,14 @@ contract JokeraceEligibility is HatsEligibilityModule { //////////////////////////////////////////////////////////////*/ struct TermDetails { - /// @notice Current Jokerace contest (election) + /// @notice Jokerace contest (election) address contest; /// @notice First K winners of the contest will be eligible uint96 topK; - /// @notice First second after the term (a unix timestamp) + /// @notice Term's ending time (a unix timestamp) uint256 termEnd; /// @notice Period of time after the term end when previous elected members are still considered eligible until a - /// new term is set. + /// new term begins. uint256 transitionPeriod; } @@ -104,9 +104,9 @@ contract JokeraceEligibility is HatsEligibilityModule { * this ensures it can only be called once per instance, and that the implementation contract is never initialized. * @param _initData Packed initialization data with the following parameters: * _contest - Jokerace contest. The contest must have down-voting disabled and sorting enabled. - * _termEnd - Final second of the current term (a unix timestamp) - * _transitionPeriod - Period of time after the term end when calling reelection is allowed and previous elected are - * still eligibile as long as reelection was not called + * _termEnd - term's ending time (a unix timestamp) + * _transitionPeriod - Period of time after the term end when previous elected members are still considered eligible + * until a new term begins. * _topK - First K winners of the contest will be eligible */ function _setUp(bytes calldata _initData) internal override { @@ -141,8 +141,9 @@ contract JokeraceEligibility is HatsEligibilityModule { returns (bool eligible, bool standing) { standing = true; - TermDetails memory currentTerm = terms[currentTermIndex]; - if (block.timestamp < currentTerm.termEnd + currentTerm.transitionPeriod) { + uint256 currentTermEnd = terms[currentTermIndex].termEnd; + uint256 currentTransitionPeriod = terms[currentTermIndex].transitionPeriod; + if (block.timestamp < currentTermEnd + currentTransitionPeriod) { eligible = eligibleWearersPerTerm[_wearer][currentTermIndex]; } } @@ -152,7 +153,7 @@ contract JokeraceEligibility is HatsEligibilityModule { //////////////////////////////////////////////////////////////*/ /** - * @notice Pulls the contest results from the jokerace contest contract. + * @notice Pulls the contest results from the next term's Jokerace contest contract and activates the next term. * @dev The eligible wearers for a given completed contest are the top K winners of the contest. In case there is a * tie, meaning that candidates in places K and K+1 have the same score, then the results of this contest are * rejected. @@ -162,11 +163,11 @@ contract JokeraceEligibility is HatsEligibilityModule { GovernorCountingSimple contest = GovernorCountingSimple(payable(nextTerm.contest)); uint96 k = nextTerm.topK; - if (!_canStartNextTerm(terms[currentTermIndex].termEnd)) { + if (!_currentTermEnded(terms[currentTermIndex].termEnd)) { revert JokeraceEligibility_TermNotCompleted(); } - if (contest.state() != Governor.ContestState.Completed) { + if (!_nextContestCompleted(contest)) { revert JokeraceEligibility_ContestNotCompleted(); } @@ -213,8 +214,8 @@ contract JokeraceEligibility is HatsEligibilityModule { } /** - * @notice Sets a reelection, i.e. updates the module with a new term. - * @dev Only the module's admin/s have the permission to set a reelection. If an admin is not set at the module + * @notice Sets the next term. + * @dev Only the module's admin/s have the permission to set the next term. If an admin is not set at the module * creation, then any admin of hatId is considered an admin by the module. */ function setNextTerm(address newContest, uint256 newTermEnd, uint256 newTransitionPeriod, uint96 newTopK) public { @@ -242,21 +243,25 @@ contract JokeraceEligibility is HatsEligibilityModule { VIEW FUNCTIONS //////////////////////////////////////////////////////////////*/ - /// @notice Check if setting a new election is allowed. + /// @notice Check if starting the next term is allowed, meaning that the current term has ended and the next contest + /// is completed function canStartNextTerm() public view returns (bool allowed) { - // If the current term has ended - return _canStartNextTerm(terms[currentTermIndex].termEnd); + return _currentTermEnded(terms[currentTermIndex].termEnd) + && _nextContestCompleted(GovernorCountingSimple(payable(terms[currentTermIndex + 1].contest))); } /*////////////////////////////////////////////////////////////// INTERNAL FUNCTIONS //////////////////////////////////////////////////////////////*/ - /// @notice Check if setting a new election is allowed. - function _canStartNextTerm(uint256 currentTermEnd) internal view returns (bool allowed) { + function _currentTermEnded(uint256 currentTermEnd) internal view returns (bool allowed) { allowed = block.timestamp > currentTermEnd; } + function _nextContestCompleted(GovernorCountingSimple nextContest) internal view returns (bool completed) { + completed = nextContest.state() == Governor.ContestState.Completed; + } + function _getCandidate(GovernorCountingSimple contest, uint256 proposalId) internal view returns (address candidate) { candidate = contest.getProposal(proposalId).author; } diff --git a/test/JokeraceEligibility.t.sol b/test/JokeraceEligibility.t.sol index 516a87c..9ea2e89 100644 --- a/test/JokeraceEligibility.t.sol +++ b/test/JokeraceEligibility.t.sol @@ -231,6 +231,11 @@ contract TestDeployment is TestSetup { HATS.getHatEligibilityModule(winnersHat), address(instanceDefaultAdmin), "eligibility module of winners hat" ); } + + function test_canStartNextTerm() public { + bool canStart = instanceDefaultAdmin.canStartNextTerm(); + assertEq(canStart, false); + } } // Three candidates propose @@ -393,6 +398,8 @@ contract ContestCompletedProposing2Scenario is Proposing2Scenario { super.setUp(); // set time to contest completion vm.warp(contestStart + voteDelay + votePeriod + 1); + bool canStart = instanceDefaultAdmin.canStartNextTerm(); + assertEq(canStart, true); uint256 currentTermIndex = instanceDefaultAdmin.currentTermIndex(); TermDetails memory nextTerm; (nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod) = @@ -418,6 +425,11 @@ contract TestContestCompletedProposing2Scenario is ContestCompletedProposing2Sce assertEq(HATS.isEligible(candidate2, winnersHat), false, "candidate 2 eligibility"); assertEq(HATS.isEligible(candidate3, winnersHat), false, "candidate 3 eligibility"); } + + function test_canStartNextTerm() public { + bool canStart = instanceDefaultAdmin.canStartNextTerm(); + assertEq(canStart, false); + } } contract TestContestCompletedVoting2Proposing1Scenario is ContestCompletedVoting2Proposing1Scenario { @@ -479,6 +491,20 @@ contract TestContestCompletedVoting1Proposing1Scenario is ContestCompletedVoting } } +contract TestTermNotCompleted is ContestCompletedVoting1Proposing1Scenario { + function test_canStartNextTerm() public { + bool canStart = instanceDefaultAdmin.canStartNextTerm(); + assertEq(canStart, false); + } + + function test_startNextTerm_reverts() public { + vm.prank(dao); + instanceDefaultAdmin.setNextTerm(address(contest), termEnd1 + 86_400, transitionPeriod, 3); + vm.expectRevert(JokeraceEligibility_TermNotCompleted.selector); + instanceDefaultAdmin.startNextTerm(); + } +} + // Current term ended, ready for reelection contract TermEndedVoting1Proposing1Scenario is ContestCompletedVoting1Proposing1Scenario { function setUp() public virtual override { @@ -488,6 +514,18 @@ contract TermEndedVoting1Proposing1Scenario is ContestCompletedVoting1Proposing1 } } +contract TestStartEmptyTerm is TermEndedVoting1Proposing1Scenario { + function test_canStartNextTerm() public { + vm.expectRevert(); + instanceDefaultAdmin.canStartNextTerm(); + } + + function test_startNextTerm_reverts() public { + vm.expectRevert(); + instanceDefaultAdmin.startNextTerm(); + } +} + contract TestTermEndedVoting1Proposing1Scenario is TermEndedVoting1Proposing1Scenario { function test_setNextTermnNotAdmin_reverts() public { vm.startPrank(candidate1); @@ -552,6 +590,11 @@ contract NextContestCanceledVoting1Proposing1Scenario is TermEndedVoting1Proposi } contract TestNextContestCanceledVoting1Proposing1Scenario is NextContestCanceledVoting1Proposing1Scenario { + function test_canStartNextTerm() public { + bool canStart = instanceDefaultAdmin.canStartNextTerm(); + assertEq(canStart, false); + } + function test_startNextTerm_reverts() public { vm.expectRevert(JokeraceEligibility_ContestNotCompleted.selector); instanceDefaultAdmin.startNextTerm(); @@ -564,6 +607,8 @@ contract TestNextContestCanceledVoting1Proposing1Scenario is NextContestCanceled vm.expectEmit(); emit NextTermSet(address(contest), newTopK, newTermEnd, transitionPeriod2); instanceDefaultAdmin.setNextTerm(address(contest), newTermEnd, transitionPeriod2, newTopK); + bool canStart = instanceDefaultAdmin.canStartNextTerm(); + assertEq(canStart, true); } } From db7f0b445f2194cd807dc957cbd8e9260f0896a5 Mon Sep 17 00:00:00 2001 From: Ido Date: Fri, 23 Aug 2024 20:46:10 +0200 Subject: [PATCH 08/10] documentation --- src/JokeraceEligibility.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/JokeraceEligibility.sol b/src/JokeraceEligibility.sol index cf428e3..0e45e53 100644 --- a/src/JokeraceEligibility.sol +++ b/src/JokeraceEligibility.sol @@ -88,10 +88,13 @@ contract JokeraceEligibility is HatsEligibilityModule { MUTABLE STATE //////////////////////////////////////////////////////////////*/ + /// @notice Indexed terms mapping(uint256 termIndex => TermDetails termDetails) public terms; + /// @notice Current term index uint256 public currentTermIndex; + /// @notice Eligible wearers per term mapping(address wearer => mapping(uint256 termIndex => bool eligible)) public eligibleWearersPerTerm; /*////////////////////////////////////////////////////////////// From 7547ce9f7a13f6088dc3c1a32ad65e188e899cee Mon Sep 17 00:00:00 2001 From: Ido Date: Tue, 27 Aug 2024 15:14:01 +0200 Subject: [PATCH 09/10] optimizations --- src/JokeraceEligibility.sol | 22 ++++++++++++++++------ test/JokeraceEligibility.t.sol | 20 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/JokeraceEligibility.sol b/src/JokeraceEligibility.sol index 0e45e53..a8a84fd 100644 --- a/src/JokeraceEligibility.sol +++ b/src/JokeraceEligibility.sol @@ -32,7 +32,7 @@ contract JokeraceEligibility is HatsEligibilityModule { //////////////////////////////////////////////////////////////*/ /// @notice Emitted when the next term is set - event NextTermSet(address NewContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); + event NextTermSet(address newContest, uint256 newTopK, uint256 newTermEnd, uint256 newTransitionPeriod); /// @notice Emitted when the next term is started event TermStarted(address contest, uint256 topK, uint256 termEnd, uint256 transitionPeriod); @@ -162,11 +162,12 @@ contract JokeraceEligibility is HatsEligibilityModule { * rejected. */ function startNextTerm() public { - TermDetails memory nextTerm = terms[currentTermIndex + 1]; + uint256 currentTermIndexMem = currentTermIndex; + TermDetails memory nextTerm = terms[currentTermIndexMem + 1]; GovernorCountingSimple contest = GovernorCountingSimple(payable(nextTerm.contest)); uint96 k = nextTerm.topK; - if (!_currentTermEnded(terms[currentTermIndex].termEnd)) { + if (!_currentTermEnded(terms[currentTermIndexMem].termEnd)) { revert JokeraceEligibility_TermNotCompleted(); } @@ -192,7 +193,7 @@ contract JokeraceEligibility is HatsEligibilityModule { // get the authors of the proposals and update their eligibility for (uint256 proposalIndex; proposalIndex < numProposalsOfCurrentRank;) { address candidate = _getCandidate(contest, proposalsOfCurrentRank[proposalIndex]); - eligibleWearersPerTerm[candidate][currentTermIndex + 1] = true; + eligibleWearersPerTerm[candidate][currentTermIndexMem + 1] = true; unchecked { ++proposalIndex; @@ -253,12 +254,21 @@ contract JokeraceEligibility is HatsEligibilityModule { && _nextContestCompleted(GovernorCountingSimple(payable(terms[currentTermIndex + 1].contest))); } + function currentTermEnded() public view returns (bool ended) { + ended = block.timestamp > terms[currentTermIndex].termEnd; + } + + function nextContestCompleted() public view returns (bool completed) { + completed = + GovernorCountingSimple(payable(terms[currentTermIndex + 1].contest)).state() == Governor.ContestState.Completed; + } + /*////////////////////////////////////////////////////////////// INTERNAL FUNCTIONS //////////////////////////////////////////////////////////////*/ - function _currentTermEnded(uint256 currentTermEnd) internal view returns (bool allowed) { - allowed = block.timestamp > currentTermEnd; + function _currentTermEnded(uint256 currentTermEnd) internal view returns (bool ended) { + ended = block.timestamp > currentTermEnd; } function _nextContestCompleted(GovernorCountingSimple nextContest) internal view returns (bool completed) { diff --git a/test/JokeraceEligibility.t.sol b/test/JokeraceEligibility.t.sol index 9ea2e89..de98846 100644 --- a/test/JokeraceEligibility.t.sol +++ b/test/JokeraceEligibility.t.sol @@ -400,6 +400,10 @@ contract ContestCompletedProposing2Scenario is Proposing2Scenario { vm.warp(contestStart + voteDelay + votePeriod + 1); bool canStart = instanceDefaultAdmin.canStartNextTerm(); assertEq(canStart, true); + bool termEnded = instanceDefaultAdmin.currentTermEnded(); + assertEq(termEnded, true); + bool nextContestCompleted = instanceDefaultAdmin.nextContestCompleted(); + assertEq(nextContestCompleted, true); uint256 currentTermIndex = instanceDefaultAdmin.currentTermIndex(); TermDetails memory nextTerm; (nextTerm.contest, nextTerm.topK, nextTerm.termEnd, nextTerm.transitionPeriod) = @@ -429,6 +433,10 @@ contract TestContestCompletedProposing2Scenario is ContestCompletedProposing2Sce function test_canStartNextTerm() public { bool canStart = instanceDefaultAdmin.canStartNextTerm(); assertEq(canStart, false); + bool currentTermEnded = instanceDefaultAdmin.currentTermEnded(); + assertEq(currentTermEnded, false); + vm.expectRevert(); + instanceDefaultAdmin.nextContestCompleted(); } } @@ -495,6 +503,8 @@ contract TestTermNotCompleted is ContestCompletedVoting1Proposing1Scenario { function test_canStartNextTerm() public { bool canStart = instanceDefaultAdmin.canStartNextTerm(); assertEq(canStart, false); + bool termEnded = instanceDefaultAdmin.currentTermEnded(); + assertEq(termEnded, false); } function test_startNextTerm_reverts() public { @@ -595,6 +605,16 @@ contract TestNextContestCanceledVoting1Proposing1Scenario is NextContestCanceled assertEq(canStart, false); } + function test_currentTermEnded() public { + bool ended = instanceDefaultAdmin.currentTermEnded(); + assertEq(ended, true); + } + + function test_nextContestCompleted() public { + bool completed = instanceDefaultAdmin.nextContestCompleted(); + assertEq(completed, false); + } + function test_startNextTerm_reverts() public { vm.expectRevert(JokeraceEligibility_ContestNotCompleted.selector); instanceDefaultAdmin.startNextTerm(); From 28c9614e18e250a4a8bd3fdc01edba576f2b3a15 Mon Sep 17 00:00:00 2001 From: Ido Date: Tue, 27 Aug 2024 16:39:37 +0200 Subject: [PATCH 10/10] wrap internal functions --- src/JokeraceEligibility.sol | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/JokeraceEligibility.sol b/src/JokeraceEligibility.sol index a8a84fd..141b1df 100644 --- a/src/JokeraceEligibility.sol +++ b/src/JokeraceEligibility.sol @@ -255,12 +255,11 @@ contract JokeraceEligibility is HatsEligibilityModule { } function currentTermEnded() public view returns (bool ended) { - ended = block.timestamp > terms[currentTermIndex].termEnd; + ended = _currentTermEnded(terms[currentTermIndex].termEnd); } function nextContestCompleted() public view returns (bool completed) { - completed = - GovernorCountingSimple(payable(terms[currentTermIndex + 1].contest)).state() == Governor.ContestState.Completed; + completed = _nextContestCompleted(GovernorCountingSimple(payable(terms[currentTermIndex + 1].contest))); } /*//////////////////////////////////////////////////////////////