From f696d5a3d45c755219b7392599e82486f892ca65 Mon Sep 17 00:00:00 2001 From: Aleksandr Tarelkin Date: Thu, 5 Dec 2024 14:34:01 +0300 Subject: [PATCH 1/2] increase MIN_TRANSFERRABLE_ST_ETH_AMOUNT --- contracts/Escrow.sol | 4 +- test/mocks/WithdrawalQueueMock.sol | 17 +++- test/unit/DualGovernance.t.sol | 2 +- test/unit/Escrow.t.sol | 155 ++++++++++++++++++++++++----- 4 files changed, 148 insertions(+), 30 deletions(-) diff --git a/contracts/Escrow.sol b/contracts/Escrow.sol index 3ba3a99f..87c61c97 100644 --- a/contracts/Escrow.sol +++ b/contracts/Escrow.sol @@ -48,7 +48,7 @@ contract Escrow is IEscrow { /// @dev The lower limit for stETH transfers when requesting a withdrawal batch /// during the Rage Quit phase. For more details, see https://github.com/lidofinance/lido-dao/issues/442. /// The current value is chosen to ensure functionality over an extended period, spanning several decades. - uint256 private constant _MIN_TRANSFERRABLE_ST_ETH_AMOUNT = 8 wei; + uint256 public constant MIN_TRANSFERRABLE_ST_ETH_AMOUNT = 100 wei; // --- // Sanity Check Parameters & Immutables @@ -303,7 +303,7 @@ contract Escrow is IEscrow { /// Using only `minStETHWithdrawalRequestAmount` is insufficient because it is an external variable /// that could be decreased independently. Introducing `minWithdrawableStETHAmount` provides /// an internal safeguard, enforcing a minimum threshold within the contract. - uint256 minWithdrawableStETHAmount = Math.max(_MIN_TRANSFERRABLE_ST_ETH_AMOUNT, minStETHWithdrawalRequestAmount); + uint256 minWithdrawableStETHAmount = Math.max(MIN_TRANSFERRABLE_ST_ETH_AMOUNT, minStETHWithdrawalRequestAmount); if (stETHRemaining < minWithdrawableStETHAmount) { return _batchesQueue.close(); diff --git a/test/mocks/WithdrawalQueueMock.sol b/test/mocks/WithdrawalQueueMock.sol index 4fde8a18..06cd5253 100644 --- a/test/mocks/WithdrawalQueueMock.sol +++ b/test/mocks/WithdrawalQueueMock.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.26; // import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol"; /*, ERC721("test", "test")*/ import {IWithdrawalQueue} from "contracts/interfaces/IWithdrawalQueue.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; /* solhint-disable no-unused-vars,custom-errors */ contract WithdrawalQueueMock is IWithdrawalQueue { @@ -11,8 +12,11 @@ contract WithdrawalQueueMock is IWithdrawalQueue { uint256 private _minStETHWithdrawalAmount; uint256 private _maxStETHWithdrawalAmount; uint256[] private _requestWithdrawalsResult; + IERC20 private _stETH; - constructor() {} + constructor(IERC20 stETH) { + _stETH = stETH; + } function MIN_STETH_WITHDRAWAL_AMOUNT() external view returns (uint256) { return _minStETHWithdrawalAmount; @@ -70,6 +74,17 @@ contract WithdrawalQueueMock is IWithdrawalQueue { uint256[] calldata _amounts, address _owner ) external returns (uint256[] memory requestIds) { + uint256 totalAmount = 0; + for (uint256 i = 0; i < _amounts.length; i++) { + if (_amounts[i] < _minStETHWithdrawalAmount) { + revert("Amount is less than MIN_STETH_WITHDRAWAL_AMOUNT"); + } + if (_amounts[i] > _maxStETHWithdrawalAmount) { + revert("Amount is more than MAX_STETH_WITHDRAWAL_AMOUNT"); + } + totalAmount += _amounts[i]; + } + IERC20(_stETH).transferFrom(_owner, address(this), totalAmount); return _requestWithdrawalsResult; } diff --git a/test/unit/DualGovernance.t.sol b/test/unit/DualGovernance.t.sol index 8de47811..7e7f3d44 100644 --- a/test/unit/DualGovernance.t.sol +++ b/test/unit/DualGovernance.t.sol @@ -43,7 +43,7 @@ contract DualGovernanceUnitTests is UnitTest { address private proposalsCanceller = makeAddr("proposalsCanceller"); StETHMock private immutable _STETH_MOCK = new StETHMock(); - IWithdrawalQueue private immutable _WITHDRAWAL_QUEUE_MOCK = new WithdrawalQueueMock(); + IWithdrawalQueue private immutable _WITHDRAWAL_QUEUE_MOCK = new WithdrawalQueueMock(_STETH_MOCK); // TODO: Replace with mocks IWstETH private immutable _WSTETH_STUB = IWstETH(makeAddr("WSTETH_STUB")); diff --git a/test/unit/Escrow.t.sol b/test/unit/Escrow.t.sol index 1e49c5d7..50ef9ee9 100644 --- a/test/unit/Escrow.t.sol +++ b/test/unit/Escrow.t.sol @@ -33,7 +33,7 @@ contract EscrowUnitTests is UnitTest { StETHMock private _stETH; IWstETH private _wstETH; - address private _withdrawalQueue; + WithdrawalQueueMock private _withdrawalQueue; Duration private _minLockAssetDuration = Durations.from(1 days); uint256 private stethAmount = 100 ether; @@ -42,11 +42,15 @@ contract EscrowUnitTests is UnitTest { _stETH = new StETHMock(); _stETH.__setShareRate(1); _wstETH = IWstETH(address(new ERC20Mock())); - _withdrawalQueue = address(new WithdrawalQueueMock()); + _withdrawalQueue = new WithdrawalQueueMock(_stETH); + _withdrawalQueue.setMaxStETHWithdrawalAmount(1_000 ether); _masterCopy = new Escrow(_stETH, _wstETH, WithdrawalQueueMock(_withdrawalQueue), IDualGovernance(_dualGovernance), 100); _escrow = Escrow(payable(Clones.clone(address(_masterCopy)))); + vm.prank(address(_escrow)); + _stETH.approve(address(_withdrawalQueue), type(uint256).max); + vm.prank(_dualGovernance); _escrow.initialize(_minLockAssetDuration); @@ -141,6 +145,125 @@ contract EscrowUnitTests is UnitTest { _masterCopy.isWithdrawalsBatchesClosed(); } + // --- + // MIN_TRANSFERRABLE_ST_ETH_AMOUNT + // --- + + function test_MIN_TRANSFERRABLE_ST_ETH_AMOUNT_gt_minWithdrawableStETHAmountWei_HappyPath() external { + uint256 amountToLock = 100; + + uint256 minWithdrawableStETHAmountWei = 99; + assertEq(_escrow.MIN_TRANSFERRABLE_ST_ETH_AMOUNT(), 100); + _withdrawalQueue.setMinStETHWithdrawalAmount(minWithdrawableStETHAmountWei); + + // Lock stETH + _stETH.mint(_vetoer, amountToLock); + vm.prank(_vetoer); + _escrow.lockStETH(amountToLock); + assertEq(_stETH.balanceOf(address(_escrow)), amountToLock); + + // Request withdrawal + vm.prank(_dualGovernance); + _escrow.startRageQuit(Durations.ZERO, Durations.ZERO); + assertEq(_escrow.getNextWithdrawalBatch(100).length, 0); + assertEq(_escrow.isWithdrawalsBatchesClosed(), false); + + uint256[] memory unstEthIds = new uint256[](1); + unstEthIds[0] = 1; + _withdrawalQueue.setRequestWithdrawalsResult(unstEthIds); + _escrow.requestNextWithdrawalsBatch(100); + + assertEq(_stETH.balanceOf(address(_escrow)), 0); + assertEq(_escrow.getNextWithdrawalBatch(100).length, 1); + assertEq(_escrow.isWithdrawalsBatchesClosed(), true); + } + + function test_MIN_TRANSFERRABLE_ST_ETH_AMOUNT_gt_minWithdrawableStETHAmountWei_HappyPath_closes_queue() external { + uint256 amountToLock = 99; + + uint256 minWithdrawableStETHAmountWei = 99; + assertEq(_escrow.MIN_TRANSFERRABLE_ST_ETH_AMOUNT(), 100); + _withdrawalQueue.setMinStETHWithdrawalAmount(minWithdrawableStETHAmountWei); + + // Lock stETH + _stETH.mint(_vetoer, amountToLock); + vm.prank(_vetoer); + _escrow.lockStETH(amountToLock); + assertEq(_stETH.balanceOf(address(_escrow)), amountToLock); + + // Request withdrawal + vm.prank(_dualGovernance); + _escrow.startRageQuit(Durations.ZERO, Durations.ZERO); + assertEq(_escrow.getNextWithdrawalBatch(100).length, 0); + assertEq(_escrow.isWithdrawalsBatchesClosed(), false); + + uint256[] memory unstEthIds = new uint256[](0); + _withdrawalQueue.setRequestWithdrawalsResult(unstEthIds); + _escrow.requestNextWithdrawalsBatch(100); + + assertEq(_stETH.balanceOf(address(_escrow)), 99); + assertEq(_escrow.getNextWithdrawalBatch(100).length, 0); + assertEq(_escrow.isWithdrawalsBatchesClosed(), true); + } + + function test_MIN_TRANSFERRABLE_ST_ETH_AMOUNT_lt_minWithdrawableStETHAmountWei_HappyPath() external { + uint256 amountToLock = 101; + + uint256 minWithdrawableStETHAmountWei = 101; + assertEq(_escrow.MIN_TRANSFERRABLE_ST_ETH_AMOUNT(), 100); + _withdrawalQueue.setMinStETHWithdrawalAmount(minWithdrawableStETHAmountWei); + + // Lock stETH + _stETH.mint(_vetoer, amountToLock); + vm.prank(_vetoer); + _escrow.lockStETH(amountToLock); + assertEq(_stETH.balanceOf(address(_escrow)), amountToLock); + + // Request withdrawal + vm.prank(_dualGovernance); + _escrow.startRageQuit(Durations.ZERO, Durations.ZERO); + assertEq(_escrow.getNextWithdrawalBatch(100).length, 0); + assertEq(_escrow.isWithdrawalsBatchesClosed(), false); + + uint256[] memory unstEthIds = new uint256[](1); + unstEthIds[0] = 1; + _withdrawalQueue.setRequestWithdrawalsResult(unstEthIds); + _escrow.requestNextWithdrawalsBatch(100); + + assertEq(_stETH.balanceOf(address(_escrow)), 0); + assertEq(_escrow.getNextWithdrawalBatch(100).length, 1); + assertEq(_escrow.isWithdrawalsBatchesClosed(), true); + } + + function test_MIN_TRANSFERRABLE_ST_ETH_AMOUNT_lt_minWithdrawableStETHAmountWei_HappyPath_closes_queue() external { + uint256 amountToLock = 100; + + uint256 minWithdrawableStETHAmountWei = 101; + assertEq(_escrow.MIN_TRANSFERRABLE_ST_ETH_AMOUNT(), 100); + _withdrawalQueue.setMinStETHWithdrawalAmount(minWithdrawableStETHAmountWei); + + // Lock stETH + _stETH.mint(_vetoer, amountToLock); + vm.prank(_vetoer); + _escrow.lockStETH(amountToLock); + assertEq(_stETH.balanceOf(address(_escrow)), amountToLock); + + // Request withdrawal + vm.prank(_dualGovernance); + _escrow.startRageQuit(Durations.ZERO, Durations.ZERO); + assertEq(_escrow.getNextWithdrawalBatch(100).length, 0); + assertEq(_escrow.isWithdrawalsBatchesClosed(), false); + + uint256[] memory unstEthIds = new uint256[](1); + unstEthIds[0] = 1; + _withdrawalQueue.setRequestWithdrawalsResult(unstEthIds); + _escrow.requestNextWithdrawalsBatch(100); + + assertEq(_stETH.balanceOf(address(_escrow)), 100); + assertEq(_escrow.getNextWithdrawalBatch(100).length, 0); + assertEq(_escrow.isWithdrawalsBatchesClosed(), true); + } + function vetoerLockedUnstEth(uint256[] memory amounts) internal returns (uint256[] memory unstethIds) { unstethIds = new uint256[](amounts.length); IWithdrawalQueue.WithdrawalRequestStatus[] memory statuses = @@ -153,36 +276,16 @@ contract EscrowUnitTests is UnitTest { } vm.mockCall( - _withdrawalQueue, + address(_withdrawalQueue), abi.encodeWithSelector(IWithdrawalQueue.getWithdrawalStatus.selector, unstethIds), abi.encode(statuses) ); - vm.mockCall(_withdrawalQueue, abi.encodeWithSelector(IWithdrawalQueue.transferFrom.selector), abi.encode(true)); + vm.mockCall( + address(_withdrawalQueue), abi.encodeWithSelector(IWithdrawalQueue.transferFrom.selector), abi.encode(true) + ); vm.startPrank(_vetoer); _escrow.lockUnstETH(unstethIds); vm.stopPrank(); } - - function createEscrow(uint256 size) internal returns (Escrow) { - return - new Escrow(_stETH, _wstETH, WithdrawalQueueMock(_withdrawalQueue), IDualGovernance(_dualGovernance), size); - } - - function createEscrowProxy(uint256 minWithdrawalsBatchSize) internal returns (Escrow) { - Escrow masterCopy = createEscrow(minWithdrawalsBatchSize); - return Escrow(payable(Clones.clone(address(masterCopy)))); - } - - function createInitializedEscrowProxy( - uint256 minWithdrawalsBatchSize, - Duration minAssetsLockDuration - ) internal returns (Escrow) { - Escrow instance = createEscrowProxy(minWithdrawalsBatchSize); - - vm.startPrank(_dualGovernance); - instance.initialize(minAssetsLockDuration); - vm.stopPrank(); - return instance; - } } From 88a133fac3bd776bc525453b53ca7c52c901f551 Mon Sep 17 00:00:00 2001 From: Aleksandr Tarelkin Date: Thu, 5 Dec 2024 14:35:33 +0300 Subject: [PATCH 2/2] test getter --- test/unit/Escrow.t.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/unit/Escrow.t.sol b/test/unit/Escrow.t.sol index 50ef9ee9..0f1a17ce 100644 --- a/test/unit/Escrow.t.sol +++ b/test/unit/Escrow.t.sol @@ -149,6 +149,10 @@ contract EscrowUnitTests is UnitTest { // MIN_TRANSFERRABLE_ST_ETH_AMOUNT // --- + function test_MIN_TRANSFERRABLE_ST_ETH_AMOUNT() external { + assertEq(_escrow.MIN_TRANSFERRABLE_ST_ETH_AMOUNT(), 100); + } + function test_MIN_TRANSFERRABLE_ST_ETH_AMOUNT_gt_minWithdrawableStETHAmountWei_HappyPath() external { uint256 amountToLock = 100;