From 30015d36e08c6388b297a12b864abc54c6b1d1f6 Mon Sep 17 00:00:00 2001 From: Roman Kolpakov Date: Fri, 31 Jan 2025 15:21:31 +0300 Subject: [PATCH 1/2] feat: use setMinAssetsLockDuration on escrow initialize --- contracts/Escrow.sol | 3 ++- contracts/libraries/EscrowState.sol | 22 ++++++++-------- docs/specification.md | 2 ++ test/unit/Escrow.t.sol | 23 ++++++++++++++--- test/unit/libraries/EscrowState.t.sol | 37 ++++++++++++++++++++++++--- test/utils/SetupDeployment.sol | 1 + 6 files changed, 67 insertions(+), 21 deletions(-) diff --git a/contracts/Escrow.sol b/contracts/Escrow.sol index 495474b0..03cf5a58 100644 --- a/contracts/Escrow.sol +++ b/contracts/Escrow.sol @@ -140,7 +140,7 @@ contract Escrow is ISignallingEscrow, IRageQuitEscrow { } _checkCallerIsDualGovernance(); - _escrowState.initialize(minAssetsLockDuration); + _escrowState.initialize(minAssetsLockDuration, MAX_MIN_ASSETS_LOCK_DURATION); ST_ETH.approve(address(WST_ETH), type(uint256).max); ST_ETH.approve(address(WITHDRAWAL_QUEUE), type(uint256).max); @@ -310,6 +310,7 @@ contract Escrow is ISignallingEscrow, IRageQuitEscrow { /// @param newMinAssetsLockDuration The new minimum lock duration to be set. function setMinAssetsLockDuration(Duration newMinAssetsLockDuration) external { _checkCallerIsDualGovernance(); + _escrowState.checkSignallingEscrow(); _escrowState.setMinAssetsLockDuration(newMinAssetsLockDuration, MAX_MIN_ASSETS_LOCK_DURATION); } diff --git a/contracts/libraries/EscrowState.sol b/contracts/libraries/EscrowState.sol index c3b4707e..482d0a78 100644 --- a/contracts/libraries/EscrowState.sol +++ b/contracts/libraries/EscrowState.sol @@ -71,11 +71,16 @@ library EscrowState { /// @notice Initializes the Escrow state to SignallingEscrow. /// @param self The context of the Escrow State library. - /// @param minAssetsLockDuration The minimum assets lock duration. - function initialize(Context storage self, Duration minAssetsLockDuration) internal { + /// @param minAssetsLockDuration The initial minimum assets lock duration. + /// @param maxMinAssetsLockDuration Sanity check upper bound for min assets lock duration. + function initialize( + Context storage self, + Duration minAssetsLockDuration, + Duration maxMinAssetsLockDuration + ) internal { _checkState(self, State.NotInitialized); _setState(self, State.SignallingEscrow); - _setMinAssetsLockDuration(self, minAssetsLockDuration); + setMinAssetsLockDuration(self, minAssetsLockDuration, maxMinAssetsLockDuration); } /// @notice Starts the rage quit process. @@ -116,7 +121,8 @@ library EscrowState { ) { revert InvalidMinAssetsLockDuration(newMinAssetsLockDuration); } - _setMinAssetsLockDuration(self, newMinAssetsLockDuration); + self.minAssetsLockDuration = newMinAssetsLockDuration; + emit MinAssetsLockDurationSet(newMinAssetsLockDuration); } // --- @@ -205,12 +211,4 @@ library EscrowState { self.state = newState; emit EscrowStateChanged(prevState, newState); } - - /// @notice Sets the minimum assets lock duration. - /// @param self The context of the Escrow State library. - /// @param newMinAssetsLockDuration The new minimum assets lock duration. - function _setMinAssetsLockDuration(Context storage self, Duration newMinAssetsLockDuration) private { - self.minAssetsLockDuration = newMinAssetsLockDuration; - emit MinAssetsLockDurationSet(newMinAssetsLockDuration); - } } diff --git a/docs/specification.md b/docs/specification.md index 610ecc38..8eb7d2d2 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -849,6 +849,8 @@ The `Escrow` instance is intended to be used behind [minimal proxy contracts](ht - MUST be called using the proxy contract. - MUST be called by the `DualGovernance` contract. - MUST NOT have been initialized previously. +- `minAssetsLockDuration` MUST NOT exceed `Escrow.MAX_MIN_ASSETS_LOCK_DURATION`. +- `minAssetsLockDuration` MUST NOT be equal to previous value (by default `0`). --- diff --git a/test/unit/Escrow.t.sol b/test/unit/Escrow.t.sol index 4a16ea36..084b0d34 100644 --- a/test/unit/Escrow.t.sol +++ b/test/unit/Escrow.t.sol @@ -135,20 +135,23 @@ contract EscrowUnitTests is UnitTest { // initialize() // --- - function test_initialize_HappyPath() external { + function testFuzz_initialize_HappyPath(Duration minAssetLockDuration) external { + vm.assume(minAssetLockDuration > Durations.ZERO); + vm.assume(minAssetLockDuration <= _maxMinAssetsLockDuration); + vm.expectEmit(); emit EscrowStateLib.EscrowStateChanged(EscrowState.NotInitialized, EscrowState.SignallingEscrow); vm.expectEmit(); - emit EscrowStateLib.MinAssetsLockDurationSet(Durations.ZERO); + emit EscrowStateLib.MinAssetsLockDurationSet(minAssetLockDuration); vm.expectCall(address(_stETH), abi.encodeCall(IERC20.approve, (address(_wstETH), type(uint256).max))); vm.expectCall(address(_stETH), abi.encodeCall(IERC20.approve, (address(_withdrawalQueue), type(uint256).max))); Escrow escrowInstance = - _createInitializedEscrowProxy({minWithdrawalsBatchSize: 100, minAssetsLockDuration: Durations.ZERO}); + _createInitializedEscrowProxy({minWithdrawalsBatchSize: 100, minAssetsLockDuration: minAssetLockDuration}); assertEq(escrowInstance.MIN_WITHDRAWALS_BATCH_SIZE(), 100); - assertEq(escrowInstance.getMinAssetsLockDuration(), Durations.ZERO); + assertEq(escrowInstance.getMinAssetsLockDuration(), minAssetLockDuration); assertTrue(escrowInstance.getEscrowState() == EscrowState.SignallingEscrow); IEscrow.SignallingEscrowDetails memory signallingEscrowDetails = escrowInstance.getSignallingEscrowDetails(); @@ -1190,6 +1193,18 @@ contract EscrowUnitTests is UnitTest { _escrow.setMinAssetsLockDuration(newMinAssetsLockDuration); } + function test_setMinAssetsLockDuration_RevertOn_RageQuitState() external { + Duration newMinAssetsLockDuration = Durations.from(1); + + _transitToRageQuit(); + + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) + ); + vm.prank(_dualGovernance); + _escrow.setMinAssetsLockDuration(newMinAssetsLockDuration); + } + // --- // withdrawETH() // --- diff --git a/test/unit/libraries/EscrowState.t.sol b/test/unit/libraries/EscrowState.t.sol index be0bb1f4..6578b9ae 100644 --- a/test/unit/libraries/EscrowState.t.sol +++ b/test/unit/libraries/EscrowState.t.sol @@ -17,14 +17,20 @@ contract EscrowStateUnitTests is UnitTest { // initialize() // --- - function testFuzz_initialize_happyPath(Duration minAssetsLockDuration) external { + function testFuzz_initialize_happyPath( + Duration minAssetsLockDuration, + Duration maxMinAssetsLockDuration + ) external { + vm.assume(minAssetsLockDuration > Durations.ZERO); + vm.assume(minAssetsLockDuration <= maxMinAssetsLockDuration); + _context.state = State.NotInitialized; vm.expectEmit(); emit EscrowState.EscrowStateChanged(State.NotInitialized, State.SignallingEscrow); emit EscrowState.MinAssetsLockDurationSet(minAssetsLockDuration); - EscrowState.initialize(_context, minAssetsLockDuration); + EscrowState.initialize(_context, minAssetsLockDuration, maxMinAssetsLockDuration); checkContext({ state: State.SignallingEscrow, @@ -35,12 +41,35 @@ contract EscrowStateUnitTests is UnitTest { }); } - function testFuzz_initialize_RevertOn_InvalidState(Duration minAssetsLockDuration) external { + function testFuzz_initialize_RevertOn_InvalidState( + Duration minAssetsLockDuration, + Duration maxMinAssetsLockDuration + ) external { + vm.assume(minAssetsLockDuration <= maxMinAssetsLockDuration); _context.state = State.SignallingEscrow; vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.SignallingEscrow)); - EscrowState.initialize(_context, minAssetsLockDuration); + EscrowState.initialize(_context, minAssetsLockDuration, maxMinAssetsLockDuration); + } + + function testFuzz_initalize_RevertOn_InvalidMinAssetLockDuration_ZeroDuration(Duration maxMinAssetsLockDuration) + external + { + vm.expectRevert(abi.encodeWithSelector(EscrowState.InvalidMinAssetsLockDuration.selector, 0)); + EscrowState.initialize(_context, Durations.ZERO, maxMinAssetsLockDuration); + } + + function testFuzz_initalize_RevertOn_InvalidMinAssetLockDuration_ExceedMaxDuration( + Duration maxMinAssetsLockDuration + ) external { + vm.assume( + maxMinAssetsLockDuration > Durations.ZERO + && maxMinAssetsLockDuration < Durations.from(MAX_DURATION_VALUE - 1) + ); + Duration duration = maxMinAssetsLockDuration + Durations.from(1); + vm.expectRevert(abi.encodeWithSelector(EscrowState.InvalidMinAssetsLockDuration.selector, duration)); + EscrowState.initialize(_context, duration, maxMinAssetsLockDuration); } // --- diff --git a/test/utils/SetupDeployment.sol b/test/utils/SetupDeployment.sol index 9e1d1201..eb6bfb54 100644 --- a/test/utils/SetupDeployment.sol +++ b/test/utils/SetupDeployment.sol @@ -178,6 +178,7 @@ abstract contract SetupDeployment is Test { dgDeployConfig.FIRST_SEAL_RAGE_QUIT_SUPPORT = PercentsD16.fromBasisPoints(3_00); // 3% dgDeployConfig.SECOND_SEAL_RAGE_QUIT_SUPPORT = PercentsD16.fromBasisPoints(15_00); // 15% dgDeployConfig.MIN_ASSETS_LOCK_DURATION = Durations.from(5 hours); + dgDeployConfig.MAX_MIN_ASSETS_LOCK_DURATION = Durations.from(365 days); dgDeployConfig.VETO_SIGNALLING_MIN_DURATION = Durations.from(3 days); dgDeployConfig.VETO_SIGNALLING_MAX_DURATION = Durations.from(30 days); dgDeployConfig.VETO_SIGNALLING_MIN_ACTIVE_DURATION = Durations.from(5 hours); From 005955db7695422a2fae796c639c952080b2e1a3 Mon Sep 17 00:00:00 2001 From: Roman Kolpakov Date: Tue, 4 Feb 2025 15:25:45 +0300 Subject: [PATCH 2/2] fix: escrowState test fix --- test/unit/libraries/EscrowState.t.sol | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/unit/libraries/EscrowState.t.sol b/test/unit/libraries/EscrowState.t.sol index 6578b9ae..09b1d6dd 100644 --- a/test/unit/libraries/EscrowState.t.sol +++ b/test/unit/libraries/EscrowState.t.sol @@ -61,15 +61,14 @@ contract EscrowStateUnitTests is UnitTest { } function testFuzz_initalize_RevertOn_InvalidMinAssetLockDuration_ExceedMaxDuration( + Duration minAssetsLockDuration, Duration maxMinAssetsLockDuration ) external { - vm.assume( - maxMinAssetsLockDuration > Durations.ZERO - && maxMinAssetsLockDuration < Durations.from(MAX_DURATION_VALUE - 1) + vm.assume(maxMinAssetsLockDuration < minAssetsLockDuration); + vm.expectRevert( + abi.encodeWithSelector(EscrowState.InvalidMinAssetsLockDuration.selector, minAssetsLockDuration) ); - Duration duration = maxMinAssetsLockDuration + Durations.from(1); - vm.expectRevert(abi.encodeWithSelector(EscrowState.InvalidMinAssetsLockDuration.selector, duration)); - EscrowState.initialize(_context, duration, maxMinAssetsLockDuration); + EscrowState.initialize(_context, minAssetsLockDuration, maxMinAssetsLockDuration); } // ---