feat: add tests

Author: heueristik

src/XanV1.sol

@@ -115,11 +115,11 @@ contract XanV1 is
     function castVote(address proposedImpl) external override {
         address voter = msg.sender;
 
-        Voting.Data storage $ = _getVotingData();
-        Voting.Ballot storage ballot = $.ballots[proposedImpl];
+        Voting.Data storage data = _getVotingData();
+        Voting.Ballot storage ballot = data.ballots[proposedImpl];
 
         if (!ballot.exists) {
-            $.assignWorstRank(proposedImpl);
+            data.assignWorstRank(proposedImpl);
         }
 
         // Cache the old votum of the voter.
@@ -147,7 +147,7 @@ contract XanV1 is
         ballot.totalVotes += delta;
 
         // Bubble the proposed implementation up in the ranking.
-        $.bubbleUp(proposedImpl);
+        data.bubbleUp(proposedImpl);
 
         emit VoteCast({voter: voter, implementation: proposedImpl, value: delta});
     }
@@ -156,8 +156,8 @@ contract XanV1 is
     function revokeVote(address proposedImpl) external override {
         address voter = msg.sender;
 
-        Voting.Data storage $ = _getVotingData();
-        Voting.Ballot storage ballot = $.ballots[proposedImpl];
+        Voting.Data storage data = _getVotingData();
+        Voting.Ballot storage ballot = data.ballots[proposedImpl];
 
         // Cache the old votum of the voter.
         uint256 oldVotum = ballot.vota[voter];
@@ -174,7 +174,7 @@ contract XanV1 is
         ballot.totalVotes -= oldVotum;
 
         // Bubble the proposed implementation down in the ranking.
-        $.bubbleDown(proposedImpl);
+        data.bubbleDown(proposedImpl);
 
         emit VoteRevoked({voter: voter, implementation: proposedImpl, value: oldVotum});
     }
@@ -184,24 +184,24 @@ contract XanV1 is
         // Check that all upgrade criteria are met before starting the delay.
         _checkVoterBodyUpgradeCriteria(proposedImpl);
 
-        Voting.Data storage $ = _getVotingData();
+        Voting.Data storage data = _getVotingData();
 
         uint48 currentTime = Time.timestamp();
 
         // Check that the delay period hasn't been started yet by
         // ensuring that no end time and implementation has been set.
-        if ($.delayEndTime != 0 && $.delayedUpgradeImpl != address(0)) {
-            revert DelayPeriodAlreadyStarted($.delayedUpgradeImpl);
+        if (data.delayEndTime != 0 && data.delayedUpgradeImpl != address(0)) {
+            revert DelayPeriodAlreadyStarted(data.delayedUpgradeImpl);
         }
 
         // Set the end time and emit the associated event.
-        $.delayEndTime = currentTime + Parameters.DELAY_DURATION;
-        $.delayedUpgradeImpl = proposedImpl;
+        data.delayEndTime = currentTime + Parameters.DELAY_DURATION;
+        data.delayedUpgradeImpl = proposedImpl;
 
         emit VoterBodyUpgradeDelayStarted({
             implementation: proposedImpl,
             startTime: currentTime,
-            endTime: $.delayEndTime
+            endTime: data.delayEndTime
         });
     }
 
@@ -383,18 +383,29 @@ contract XanV1 is
             revert ImplementationZero();
         }
 
-        // TODO optimize fetching? Data is also used in Criteria checks // No, happens only once.
-        address councilProposedImpl = _getCouncilData().proposedImpl;
+        Council.Data storage council = _getCouncilData();
+        Voting.Data storage voting = _getVotingData();
 
-        // TODO! What if the voter body votes for the same impl as the council?
+        // TODO REFACTOR
+        if (newImpl != council.proposedImpl) {
+            _checkVoterBodyDelayCriterion(newImpl);
+            _checkVoterBodyUpgradeCriteria(newImpl);
+            return;
+        }
 
-        if (newImpl == councilProposedImpl) {
-            _checkCouncilDelayCriterion( /*TODO! //councilProposedImpl*/ );
+        // TODO! rename delayedUpgradeImpl to scheduled impl!!!
+        if (newImpl != voting.delayedUpgradeImpl) {
+            _checkCouncilDelayCriterion();
+            _checkCouncilUpgradeCriteria(newImpl);
+            return;
+        }
 
-            _checkCouncilUpgradeCriteria(councilProposedImpl);
+        // The same implementation has been proposed by both
+        if (council.delayEndTime < voting.delayEndTime) {
+            _checkCouncilDelayCriterion();
+            _checkCouncilUpgradeCriteria(newImpl);
         } else {
             _checkVoterBodyDelayCriterion(newImpl);
-
             _checkVoterBodyUpgradeCriteria(newImpl);
         }
     }

test/XanV1.council.t.sol

@@ -149,7 +149,19 @@ contract XanV1CouncilTest is Test {
         assertEq(_xanProxy.councilProposedImplementation(), _NEW_IMPL);
     }
 
-    function test_councilProposedImplementation_returns_address_0_if_no_upgrade_delay_has_been_started() public view {
+    function test_councilDelayEndTime_returns_the_end_time_if_an_upgrade_delay_has_been_started() public {
+        uint256 expectedEndTime = block.timestamp + Parameters.DELAY_DURATION;
+        vm.prank(_COUNCIL);
+        _xanProxy.proposeCouncilUpgrade(_NEW_IMPL);
+
+        assertEq(_xanProxy.councilDelayEndTime(), expectedEndTime);
+    }
+
+    function test_councilProposedImplementation_returns_0_if_no_upgrade_delay_has_been_started() public view {
         assertEq(_xanProxy.councilProposedImplementation(), address(0));
     }
+
+    function test_councilDelayEndTime_returns_0_if_no_upgrade_delay_has_been_started() public view {
+        assertEq(_xanProxy.councilDelayEndTime(), 0);
+    }
 }

test/XanV1.upgrade.t.sol

@@ -176,8 +176,10 @@ contract XanV1UpgradeTest is Test {
         vm.prank(_COUNCIL);
         _xanProxy.proposeCouncilUpgrade(_voterProposedImpl);
 
+        assertLt(_xanProxy.voterBodyDelayEndTime(), _xanProxy.councilDelayEndTime());
+
         // Advance time after the delay end of the implementation proposed by the voter body.
-        skip(_xanProxy.voterBodyDelayEndTime());
+        skip(_xanProxy.voterBodyDelayEndTime() - block.timestamp);
 
         _xanProxy.upgradeToAndCall({newImplementation: _voterProposedImpl, data: ""});
     }

test/XanV1.voting.t.sol

@@ -369,16 +369,30 @@ contract XanV1VotingTest is Test {
         vm.startPrank(_defaultSender);
         _xanProxy.lock(Parameters.MIN_LOCKED_SUPPLY);
         _xanProxy.castVote(_NEW_IMPL);
-        _xanProxy.startVoterBodyUpgradeDelay(_NEW_IMPL);
         vm.stopPrank();
 
+        _xanProxy.startVoterBodyUpgradeDelay(_NEW_IMPL);
+
         assertEq(_xanProxy.voterBodyProposedImplementation(), _NEW_IMPL);
     }
 
-    function test_voterBodyProposedImplementation_returns_address_0_if_no_upgrade_delay_has_been_started()
-        public
-        view
-    {
+    function test_councilDelayEndTime_returns_the_end_time_if_an_upgrade_delay_has_been_started() public {
+        vm.startPrank(_defaultSender);
+        _xanProxy.lock(Parameters.MIN_LOCKED_SUPPLY);
+        _xanProxy.castVote(_NEW_IMPL);
+        vm.stopPrank();
+
+        uint256 expectedEndTime = block.timestamp + Parameters.DELAY_DURATION;
+        _xanProxy.startVoterBodyUpgradeDelay(_NEW_IMPL);
+
+        assertEq(_xanProxy.voterBodyDelayEndTime(), expectedEndTime);
+    }
+
+    function test_voterBodyProposedImplementation_returns_0_if_no_upgrade_delay_has_been_started() public view {
         assertEq(_xanProxy.voterBodyProposedImplementation(), address(0));
     }
+
+    function test_voterBodyDelayEndTime_returns_0_if_no_upgrade_delay_has_been_started() public view {
+        assertEq(_xanProxy.voterBodyDelayEndTime(), 0);
+    }
 }