Exp external functions fsm

src/Governance.sol

@@ -444,7 +444,8 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance
         bold.safeTransferFrom(msg.sender, address(this), REGISTRATION_FEE);
 
         require(_initiative != address(0), "Governance: zero-address");
-        require(registeredInitiatives[_initiative] == 0, "Governance: initiative-already-registered");
+        (InitiativeStatus status, ,) = getInitiativeState(_initiative);
+        require(status == InitiativeStatus.NONEXISTENT, "Governance: initiative-already-registered");
 
         address userProxyAddress = deriveUserProxyAddress(msg.sender);
         (VoteSnapshot memory snapshot,) = _snapshotVotes();
@@ -479,7 +480,7 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance
             "Governance: array-length-mismatch"
         );
 
-        (, GlobalState memory state) = _snapshotVotes();
+        (VoteSnapshot memory votesSnapshot_ , GlobalState memory state) = _snapshotVotes();
 
         uint16 currentEpoch = epoch();
 
@@ -496,20 +497,27 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance
                 "Governance: epoch-voting-cutoff"
             );
 
+            // Check FSM
+            // Can vote positively in SKIP, CLAIMABLE, CLAIMED and UNREGISTERABLE states
+            // Force to remove votes if disabled
+            // Can remove votes and vetos in every stage
+            (InitiativeVoteSnapshot memory votesForInitiativeSnapshot_, InitiativeState memory initiativeState) =
+                _snapshotVotesForInitiative(initiative);
+
             {
-                uint16 registeredAtEpoch = registeredInitiatives[initiative];
+                (InitiativeStatus status, , ) = getInitiativeState(initiative, votesSnapshot_, votesForInitiativeSnapshot_, initiativeState);
+
                 if(deltaLQTYVotes > 0 || deltaLQTYVetos > 0) {
-                    require(currentEpoch > registeredAtEpoch && registeredAtEpoch != 0, "Governance: initiative-not-active");
+                    /// @audit FSM CHECK, note that the original version allowed voting on `Unregisterable` Initiatives - Prob should fix
+                    require(status == InitiativeStatus.SKIP || status == InitiativeStatus.CLAIMABLE || status == InitiativeStatus.CLAIMED  || status == InitiativeStatus.UNREGISTERABLE, "Governance: active-vote-fsm");
                 }
 
-                if(registeredAtEpoch == UNREGISTERED_INITIATIVE) {
+                if(status == InitiativeStatus.DISABLED) {
                     require(deltaLQTYVotes <= 0 && deltaLQTYVetos <= 0, "Must be a withdrawal");
                 }
             }
 
 
-            (, InitiativeState memory initiativeState) = _snapshotVotesForInitiative(initiative);
-
             // deep copy of the initiative's state before the allocation
             InitiativeState memory prevInitiativeState = InitiativeState(
                 initiativeState.voteLQTY,
@@ -585,24 +593,25 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance
 
     /// @inheritdoc IGovernance
     function unregisterInitiative(address _initiative) external nonReentrant {
-        uint16 registrationEpoch = registeredInitiatives[_initiative];
-        require(registrationEpoch != 0, "Governance: initiative-not-registered");
-        uint16 currentEpoch = epoch();
-        require(registrationEpoch + REGISTRATION_WARM_UP_PERIOD < currentEpoch, "Governance: initiative-in-warm-up");
-
-        (, GlobalState memory state) = _snapshotVotes();
+        /// Enforce FSM
+        (VoteSnapshot memory votesSnapshot_ , GlobalState memory state) = _snapshotVotes();
         (InitiativeVoteSnapshot memory votesForInitiativeSnapshot_, InitiativeState memory initiativeState) =
             _snapshotVotesForInitiative(_initiative);
 
-        /// Invariant: Must only claim once or unregister
-        require(initiativeState.lastEpochClaim < epoch() - 1);
-
-        (InitiativeStatus status, , )= getInitiativeState(_initiative);
+        (InitiativeStatus status, , ) = getInitiativeState(_initiative, votesSnapshot_, votesForInitiativeSnapshot_, initiativeState);
+        require(status != InitiativeStatus.NONEXISTENT, "Governance: initiative-not-registered");
+        require(status != InitiativeStatus.COOLDOWN, "Governance: initiative-in-warm-up");
         require(status == InitiativeStatus.UNREGISTERABLE, "Governance: cannot-unregister-initiative");
 
-        /// @audit TODO: Verify that the FSM here is correct
+        // Remove weight from current state
+        uint16 currentEpoch = epoch();
+
+        /// @audit Invariant: Must only claim once or unregister
+        assert(initiativeState.lastEpochClaim < currentEpoch - 1);
 
         // recalculate the average staking timestamp for all counted voting LQTY if the initiative was counted in
+        /// @audit CRIT HERE | The math on removing messes stuff up
+        /// Prob need to remove this
         state.countedVoteLQTYAverageTimestamp = _calculateAverageTimestamp(
             state.countedVoteLQTYAverageTimestamp,
             initiativeState.averageStakingTimestampVoteLQTY,
@@ -623,10 +632,11 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance
 
     /// @inheritdoc IGovernance
     function claimForInitiative(address _initiative) external nonReentrant returns (uint256) {
-        (VoteSnapshot memory votesSnapshot_,) = _snapshotVotes();
-        (InitiativeVoteSnapshot memory votesForInitiativeSnapshot_, InitiativeState memory initiativeState_) = _snapshotVotesForInitiative(_initiative);
+        (VoteSnapshot memory votesSnapshot_ , GlobalState memory state) = _snapshotVotes();
+        (InitiativeVoteSnapshot memory votesForInitiativeSnapshot_, InitiativeState memory initiativeState) =
+            _snapshotVotesForInitiative(_initiative);
 
-        (InitiativeStatus status, , uint256 claimableAmount ) = getInitiativeState(_initiative);
+        (InitiativeStatus status, , uint256 claimableAmount) = getInitiativeState(_initiative, votesSnapshot_, votesForInitiativeSnapshot_, initiativeState);
 
         /// INVARIANT:
         /// We cannot claim only for 2 reasons:
@@ -643,7 +653,6 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance
         /// Invariant `lastEpochClaim` is < epoch() - 1; | 
         /// If `lastEpochClaim` is older than epoch() - 1 it means the initiative couldn't claim any rewards this epoch
         initiativeStates[_initiative].lastEpochClaim = epoch() - 1;
-        votesForInitiativeSnapshot[_initiative] = votesForInitiativeSnapshot_; // implicitly prevents double claiming
 
         bold.safeTransfer(_initiative, claimableAmount);
 

test/Governance.t.sol

@@ -795,7 +795,7 @@ contract GovernanceTest is Test {
         int88[] memory reAddDeltaLQTYVetos = new int88[](1);
 
         /// @audit This MUST revert, an initiative should not be re-votable once disabled
-        vm.expectRevert("Governance: initiative-not-active");
+        vm.expectRevert("Governance: active-vote-fsm");
         governance.allocateLQTY(reAddInitiatives, reAddDeltaLQTYVotes, reAddDeltaLQTYVetos);
     }
 
@@ -877,7 +877,7 @@ contract GovernanceTest is Test {
         int88[] memory deltaLQTYVetos = new int88[](1);
 
         // should revert if the initiative has been registered in the current epoch
-        vm.expectRevert("Governance: initiative-not-active");
+        vm.expectRevert("Governance: active-vote-fsm");
         governance.allocateLQTY(initiatives, deltaLQTYVotes, deltaLQTYVetos);
 
         vm.warp(block.timestamp + 365 days);

test/recon/properties/GovernanceProperties.sol

@@ -40,7 +40,6 @@ abstract contract GovernanceProperties is BeforeAfter {
                     }
                 }
 
-
                 eq(uint256(_before.initiativeStatus[initiative]), uint256(_after.initiativeStatus[initiative]), "GV-01: Initiative state should only return one state per epoch");
             }
         }