refactor: clarify auhtorizeUpgrade logic

Author: heueristik

src/XanV1.sol

@@ -64,9 +64,9 @@ contract XanV1 is
     error ImplementationNotDelayed(address expected, address actual);
 
     error UpgradeNotScheduled(address impl);
+    error UpgradeScheduledTwice(address impl);
     error UpgradeAlreadyScheduled(address impl, uint48 endTime);
     error UpgradeCancellationInvalid(address impl, uint48 endTime);
-    error UpgradeIsNotAllowedToBeScheduledTwice(address impl);
 
     error QuorumOrMinLockedSupplyNotReached(address impl);
     error QuorumAndMinLockedSupplyReached(address impl);
@@ -311,6 +311,11 @@ contract XanV1 is
         }
     }
 
+    /// @notice @inheritdoc IXanV1
+    function proposedImplementationsCount() external view returns (uint48 count) {
+        count = _getVotingData().implementationsCount();
+    }
+
     /// @notice @inheritdoc IXanV1
     function lockedSupply() public view override returns (uint256 locked) {
         locked = _getLockingData().lockedSupply;
@@ -415,13 +420,12 @@ contract XanV1 is
         bool isScheduledByVoterBody = (newImpl == votingData.scheduledImpl);
         bool isScheduledByCouncil = (newImpl == councilData.scheduledImpl);
 
-        // NOTE: councilUpgrade.impl and voterBodyUpgrade.impl can still be address(0);
-
-        if (isScheduledByCouncil && isScheduledByVoterBody) {
-            // This should never happen.
-            revert UpgradeIsNotAllowedToBeScheduledTwice(newImpl);
+        if (isScheduledByVoterBody && isScheduledByCouncil) {
+            // This state should never be reached.
+            revert UpgradeScheduledTwice(newImpl);
         }
 
+        // Cache the best ranked implementation proposed by the voter body.
         address bestRankedVoterBodyImpl = votingData.implementationByRank(0);
 
         if (isScheduledByVoterBody) {
@@ -438,12 +442,14 @@ contract XanV1 is
         }
 
         if (isScheduledByCouncil) {
-            // Revert if the quorum and minimum locked supply is reached for best ranked implementation proposed by
-            // the voter body.
-            if (_isQuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl)) {
-                revert QuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl);
+            // Check if the best ranked implementation exists.
+            if (bestRankedVoterBodyImpl != address(0)) {
+                // Revert if the quorum and minimum locked supply is reached for best ranked implementation proposed by
+                // the voter body and it could therefore could be scheduled.
+                if (_isQuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl)) {
+                    revert QuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl);
+                }
             }
-
             _checkDelayCriterion({endTime: councilData.scheduledEndTime});
 
             return;

src/interfaces/IXanV1.sol

@@ -129,6 +129,10 @@ interface IXanV1 {
     /// @return impl The proposed implementation with the respective rank.
     function proposedImplementationByRank(uint48 rank) external view returns (address impl);
 
+    /// @notice Returns the number of implementations proposed by the voter body.
+    /// @return count The proposed implementation count.
+    function proposedImplementationsCount() external view returns (uint48 count);
+
     /// @notice Returns the address of the governance council.
     /// @return council The governance council address.
     function governanceCouncil() external view returns (address council);

src/libs/Voting.sol

@@ -126,4 +126,10 @@ library Voting {
 
         impl = data.ranking[rank];
     }
+
+    /// @notice Returns the number of implementations.
+    /// @return count implementation count.
+    function implementationsCount(Data storage data) internal view returns (uint48 count) {
+        count = data.implCount;
+    }
 }

test/XanV1.upgrade.t.sol

@@ -46,7 +46,19 @@ contract XanV1UpgradeTest is Test {
         _xanProxy.upgradeToAndCall({newImplementation: address(0), data: ""});
     }
 
-    function test_authorizeUpgrade_reverts_voter_body_upgrade_if_implementation_has_not_been_voted_on() public {
+    function test_authorizeUpgrade_reverts_voter_body_upgrade_if_no_implementation_has_been_scheduled() public {
+        vm.expectRevert(
+            abi.encodeWithSelector(XanV1.UpgradeNotScheduled.selector, _voterProposedImpl), address(_xanProxy)
+        );
+        _xanProxy.upgradeToAndCall({newImplementation: _voterProposedImpl, data: ""});
+    }
+
+    function test_authorizeUpgrade_reverts_voter_body_upgrade_if_implementation_has_not_been_scheduled() public {
+        vm.startPrank(_defaultSender);
+        _xanProxy.lock(Parameters.MIN_LOCKED_SUPPLY);
+        _xanProxy.castVote(_voterProposedImpl2);
+        vm.stopPrank();
+
         vm.expectRevert(
             abi.encodeWithSelector(XanV1.UpgradeNotScheduled.selector, _voterProposedImpl), address(_xanProxy)
         );

test/XanV1.voting.t.sol

@@ -55,9 +55,11 @@ contract XanV1VotingTest is Test {
 
     function test_castVote_ranks_an_implementation_on_first_vote() public {
         // Check that no implementation has rank 0.
+        uint48 count = _xanProxy.proposedImplementationsCount();
         uint48 rank = 0;
+
         vm.expectRevert(
-            abi.encodeWithSelector(XanV1.ImplementationRankNonExistent.selector, 0, rank), address(_xanProxy)
+            abi.encodeWithSelector(XanV1.ImplementationRankNonExistent.selector, count, rank), address(_xanProxy)
         );
         _xanProxy.proposedImplementationByRank(rank);
 
@@ -66,12 +68,16 @@ contract XanV1VotingTest is Test {
         _xanProxy.lock(Parameters.MIN_LOCKED_SUPPLY);
         _xanProxy.castVote(_NEW_IMPL);
         vm.stopPrank();
+
+        count = _xanProxy.proposedImplementationsCount();
+        assertEq(count, 1);
+
         assertEq(_NEW_IMPL, _xanProxy.proposedImplementationByRank(rank));
 
         // Check that no implementation has rank 1.
         rank = 1;
         vm.expectRevert(
-            abi.encodeWithSelector(XanV1.ImplementationRankNonExistent.selector, 1, rank), address(_xanProxy)
+            abi.encodeWithSelector(XanV1.ImplementationRankNonExistent.selector, count, rank), address(_xanProxy)
         );
         _xanProxy.proposedImplementationByRank(rank);
     }