Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use setMinAssetsLockDuration on escrow initialize #256

Merged
merged 2 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion contracts/Escrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down
22 changes: 10 additions & 12 deletions contracts/libraries/EscrowState.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
rkolpakov marked this conversation as resolved.
Show resolved Hide resolved
}

/// @notice Starts the rage quit process.
Expand Down Expand Up @@ -116,7 +121,8 @@ library EscrowState {
) {
revert InvalidMinAssetsLockDuration(newMinAssetsLockDuration);
}
_setMinAssetsLockDuration(self, newMinAssetsLockDuration);
self.minAssetsLockDuration = newMinAssetsLockDuration;
emit MinAssetsLockDurationSet(newMinAssetsLockDuration);
}

// ---
Expand Down Expand Up @@ -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);
}
}
2 changes: 2 additions & 0 deletions docs/specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`).

---

Expand Down
23 changes: 19 additions & 4 deletions test/unit/Escrow.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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()
// ---
Expand Down
36 changes: 32 additions & 4 deletions test/unit/libraries/EscrowState.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -35,12 +41,34 @@ 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 minAssetsLockDuration,
Duration maxMinAssetsLockDuration
) external {
vm.assume(maxMinAssetsLockDuration < minAssetsLockDuration);
vm.expectRevert(
abi.encodeWithSelector(EscrowState.InvalidMinAssetsLockDuration.selector, minAssetsLockDuration)
);
EscrowState.initialize(_context, minAssetsLockDuration, maxMinAssetsLockDuration);
rkolpakov marked this conversation as resolved.
Show resolved Hide resolved
}

// ---
Expand Down
1 change: 1 addition & 0 deletions test/utils/SetupDeployment.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down