Skip to content

Commit

Permalink
documentation and optimizations
Browse files Browse the repository at this point in the history
  • Loading branch information
@gershido
gershido committed Aug 23, 2024
1 parent 72c09ec commit 6479a0d
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 20 deletions.
45 changes: 25 additions & 20 deletions src/JokeraceEligibility.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/*//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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];
}
}
Expand All @@ -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.
Expand All @@ -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();
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
45 changes: 45 additions & 0 deletions test/JokeraceEligibility.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) =
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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);
}
}

Expand Down

0 comments on commit 6479a0d

Please sign in to comment.