Merge pull request #107 from lidofinance/fix/rage-quit-round-overflow

Fix the possible overflow of the rage quit round
lidofinance · Aug 28, 2024 · 64ea7a9 · 64ea7a9
2 parents a251e6e + 51ad0ef
commit 64ea7a9
Show file tree

Hide file tree

Showing 3 changed files with 136 additions and 3 deletions.
diff --git a/contracts/libraries/DualGovernanceStateMachine.sol b/contracts/libraries/DualGovernanceStateMachine.sol
@@ -64,6 +64,8 @@ library DualGovernanceStateMachine {
     event NewSignallingEscrowDeployed(IEscrow indexed escrow);
     event DualGovernanceStateChanged(State from, State to, Context state);
 
+    uint256 internal constant MAX_RAGE_QUIT_ROUND = type(uint8).max;
+
     function initialize(
         Context storage self,
         DualGovernanceConfig.Context memory config,
@@ -108,10 +110,16 @@ library DualGovernanceStateMachine {
             }
         } else if (newState == State.RageQuit) {
             IEscrow signallingEscrow = self.signallingEscrow;
-            uint256 rageQuitRound = Math.min(self.rageQuitRound + 1, type(uint8).max);
-            self.rageQuitRound = uint8(rageQuitRound);
+
+            uint256 currentRageQuitRound = self.rageQuitRound;
+
+            /// @dev Limits the maximum value of the rage quit round to prevent failures due to arithmetic overflow
+            /// if the number of consecutive rage quits reaches MAX_RAGE_QUIT_ROUND.
+            uint256 newRageQuitRound = Math.min(currentRageQuitRound + 1, MAX_RAGE_QUIT_ROUND);
+            self.rageQuitRound = uint8(newRageQuitRound);
+
             signallingEscrow.startRageQuit(
-                config.rageQuitExtensionDelay, config.calcRageQuitWithdrawalsTimelock(rageQuitRound)
+                config.rageQuitExtensionDelay, config.calcRageQuitWithdrawalsTimelock(newRageQuitRound)
             );
             self.rageQuitEscrow = signallingEscrow;
             _deployNewSignallingEscrow(self, escrowMasterCopy, config.minAssetsLockDuration);

diff --git a/test/mocks/EscrowMock.sol b/test/mocks/EscrowMock.sol
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.26;
+
+import {Duration} from "contracts/types/Duration.sol";
+import {PercentD16} from "contracts/types/PercentD16.sol";
+
+import {IEscrow} from "contracts/interfaces/IEscrow.sol";
+
+contract EscrowMock is IEscrow {
+    event __RageQuitStarted(Duration rageQuitExtraTimelock, Duration rageQuitWithdrawalsTimelock);
+
+    Duration public __minAssetsLockDuration;
+    PercentD16 public __rageQuitSupport;
+    bool public __isRageQuitFinalized;
+
+    function __setRageQuitSupport(PercentD16 newRageQuitSupport) external {
+        __rageQuitSupport = newRageQuitSupport;
+    }
+
+    function __setIsRageQuitFinalized(bool newIsRageQuitFinalized) external {
+        __isRageQuitFinalized = newIsRageQuitFinalized;
+    }
+
+    function initialize(Duration minAssetsLockDuration) external {
+        __minAssetsLockDuration = minAssetsLockDuration;
+    }
+
+    function startRageQuit(Duration rageQuitExtraTimelock, Duration rageQuitWithdrawalsTimelock) external {
+        emit __RageQuitStarted(rageQuitExtraTimelock, rageQuitWithdrawalsTimelock);
+    }
+
+    function isRageQuitFinalized() external view returns (bool) {
+        return __isRageQuitFinalized;
+    }
+
+    function getRageQuitSupport() external view returns (PercentD16 rageQuitSupport) {
+        return __rageQuitSupport;
+    }
+
+    function setMinAssetsLockDuration(Duration newMinAssetsLockDuration) external {
+        __minAssetsLockDuration = newMinAssetsLockDuration;
+    }
+}
diff --git a/test/unit/libraries/DualGovernanceStateMachine.t.sol b/test/unit/libraries/DualGovernanceStateMachine.t.sol
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.26;
+
+import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
+
+import {Durations} from "contracts/types/Duration.sol";
+import {PercentsD16} from "contracts/types/PercentD16.sol";
+
+import {DualGovernanceStateMachine, State} from "contracts/libraries/DualGovernanceStateMachine.sol";
+import {DualGovernanceConfig, ImmutableDualGovernanceConfigProvider} from "contracts/DualGovernanceConfigProvider.sol";
+
+import {UnitTest} from "test/utils/unit-test.sol";
+import {EscrowMock} from "test/mocks/EscrowMock.sol";
+
+contract DualGovernanceStateMachineUnitTests is UnitTest {
+    using DualGovernanceStateMachine for DualGovernanceStateMachine.Context;
+
+    address private immutable _ESCROW_MASTER_COPY = address(new EscrowMock());
+    ImmutableDualGovernanceConfigProvider internal immutable _CONFIG_PROVIDER = new ImmutableDualGovernanceConfigProvider(
+        DualGovernanceConfig.Context({
+            firstSealRageQuitSupport: PercentsD16.fromBasisPoints(3_00), // 3%
+            secondSealRageQuitSupport: PercentsD16.fromBasisPoints(15_00), // 15%
+            //
+            minAssetsLockDuration: Durations.from(5 hours),
+            dynamicTimelockMinDuration: Durations.from(3 days),
+            dynamicTimelockMaxDuration: Durations.from(30 days),
+            //
+            vetoSignallingMinActiveDuration: Durations.from(5 hours),
+            vetoSignallingDeactivationMaxDuration: Durations.from(5 days),
+            vetoCooldownDuration: Durations.from(4 days),
+            //
+            rageQuitExtensionDelay: Durations.from(7 days),
+            rageQuitEthWithdrawalsMinTimelock: Durations.from(60 days),
+            rageQuitEthWithdrawalsTimelockGrowthStartSeqNumber: 2,
+            rageQuitEthWithdrawalsTimelockGrowthCoeffs: [uint256(0), 0, 0]
+        })
+    );
+
+    DualGovernanceStateMachine.Context private _stateMachine;
+
+    function setUp() external {
+        _stateMachine.initialize(_CONFIG_PROVIDER.getDualGovernanceConfig(), _ESCROW_MASTER_COPY);
+    }
+
+    function test_activateNextState_HappyPath_MaxRageQuitsRound() external {
+        assertEq(_stateMachine.state, State.Normal);
+
+        for (uint256 i = 0; i < 2 * DualGovernanceStateMachine.MAX_RAGE_QUIT_ROUND; ++i) {
+            address signallingEscrow = address(_stateMachine.signallingEscrow);
+            EscrowMock(signallingEscrow).__setRageQuitSupport(
+                _CONFIG_PROVIDER.SECOND_SEAL_RAGE_QUIT_SUPPORT() + PercentsD16.fromBasisPoints(1_00)
+            );
+            assertTrue(
+                _stateMachine.signallingEscrow.getRageQuitSupport() > _CONFIG_PROVIDER.SECOND_SEAL_RAGE_QUIT_SUPPORT()
+            );
+            assertEq(_stateMachine.rageQuitRound, Math.min(i, DualGovernanceStateMachine.MAX_RAGE_QUIT_ROUND));
+
+            // wait here the full duration of the veto cooldown to make sure it's over from the previous iteration
+            _wait(_CONFIG_PROVIDER.VETO_COOLDOWN_DURATION().plusSeconds(1));
+
+            _stateMachine.activateNextState(_CONFIG_PROVIDER.getDualGovernanceConfig(), _ESCROW_MASTER_COPY);
+            assertEq(_stateMachine.state, State.VetoSignalling);
+
+            _wait(_CONFIG_PROVIDER.DYNAMIC_TIMELOCK_MAX_DURATION().plusSeconds(1));
+            _stateMachine.activateNextState(_CONFIG_PROVIDER.getDualGovernanceConfig(), _ESCROW_MASTER_COPY);
+
+            assertEq(_stateMachine.state, State.RageQuit);
+            assertEq(_stateMachine.rageQuitRound, Math.min(i + 1, DualGovernanceStateMachine.MAX_RAGE_QUIT_ROUND));
+
+            EscrowMock(signallingEscrow).__setIsRageQuitFinalized(true);
+            _stateMachine.activateNextState(_CONFIG_PROVIDER.getDualGovernanceConfig(), _ESCROW_MASTER_COPY);
+            assertEq(_stateMachine.state, State.VetoCooldown);
+        }
+
+        // after the sequential rage quits chain is broken, the rage quit resets to 0
+        _wait(_CONFIG_PROVIDER.VETO_COOLDOWN_DURATION().plusSeconds(1));
+        _stateMachine.activateNextState(_CONFIG_PROVIDER.getDualGovernanceConfig(), _ESCROW_MASTER_COPY);
+
+        assertEq(_stateMachine.rageQuitRound, 0);
+        assertEq(_stateMachine.state, State.Normal);
+    }
+}