Merge pull request #94 from lidofinance/fix/executed-proposals-canceling

Fix: Prevent executed proposals from changing state after cancelAll is called
lidofinance · Aug 28, 2024 · 72a994a · 72a994a
2 parents 10c2c2c + e8a7a9f
commit 72a994a
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 1 deletion.
diff --git a/contracts/libraries/ExecutableProposals.sol b/contracts/libraries/ExecutableProposals.sol
@@ -217,6 +217,6 @@ library ExecutableProposals {
         uint256 proposalId,
         ProposalData memory proposalData
     ) private view returns (bool) {
-        return proposalId <= self.lastCancelledProposalId || proposalData.status == Status.Cancelled;
+        return proposalId <= self.lastCancelledProposalId && proposalData.status != Status.Executed;
     }
 }
diff --git a/test/unit/libraries/ExecutableProposals.t.sol b/test/unit/libraries/ExecutableProposals.t.sol
@@ -401,6 +401,54 @@ contract ExecutableProposalsUnitTests is UnitTest {
         assert(!_proposals.canExecute(proposalId, Durations.ZERO));
     }
 
+    function test_cancelAll_DoesNotModifyStateOfExecutedProposals() external {
+        _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)));
+        assertEq(_proposals.getProposalsCount(), 1);
+        uint256 executedProposalId = 1;
+        _proposals.schedule(executedProposalId, Durations.ZERO);
+        _proposals.execute(executedProposalId, Durations.ZERO);
+
+        _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)));
+        assertEq(_proposals.getProposalsCount(), 2);
+        uint256 scheduledProposalId = 2;
+        _proposals.schedule(scheduledProposalId, Durations.ZERO);
+
+        _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)));
+        assertEq(_proposals.getProposalsCount(), 3);
+        uint256 submittedProposalId = 3;
+
+        // Validate the state of the proposals is correct before proceeding with cancellation.
+
+        (ProposalStatus executedProposalStatus, /* executor */, /* submittedAt */, /* scheduledAt */ ) =
+            _proposals.getProposalInfo(executedProposalId);
+        assertEq(executedProposalStatus, ProposalStatus.Executed);
+
+        (ProposalStatus scheduledProposalStatus, /* executor */, /* submittedAt */, /* scheduledAt */ ) =
+            _proposals.getProposalInfo(scheduledProposalId);
+        assertEq(scheduledProposalStatus, ProposalStatus.Scheduled);
+
+        (ProposalStatus submittedProposalStatus, /* executor */, /* submittedAt */, /* scheduledAt */ ) =
+            _proposals.getProposalInfo(submittedProposalId);
+        assertEq(submittedProposalStatus, ProposalStatus.Submitted);
+
+        // After canceling the proposals, both submitted and scheduled proposals should transition to the Cancelled state.
+        // However, executed proposals should remain in the Executed state.
+
+        _proposals.cancelAll();
+
+        (executedProposalStatus, /* executor */, /* submittedAt */, /* scheduledAt */ ) =
+            _proposals.getProposalInfo(executedProposalId);
+        assertEq(executedProposalStatus, ProposalStatus.Executed);
+
+        (scheduledProposalStatus, /* executor */, /* submittedAt */, /* scheduledAt */ ) =
+            _proposals.getProposalInfo(scheduledProposalId);
+        assertEq(scheduledProposalStatus, ProposalStatus.Cancelled);
+
+        (submittedProposalStatus, /* executor */, /* submittedAt */, /* scheduledAt */ ) =
+            _proposals.getProposalInfo(submittedProposalId);
+        assertEq(submittedProposalStatus, ProposalStatus.Cancelled);
+    }
+
     function test_can_schedule_proposal() external {
         Duration delay = Durations.from(100 seconds);
         _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)));