diff --git a/contracts/0.4.24/Lido.sol b/contracts/0.4.24/Lido.sol index d006e5928..520a9b4ae 100644 --- a/contracts/0.4.24/Lido.sol +++ b/contracts/0.4.24/Lido.sol @@ -122,7 +122,7 @@ interface IWithdrawalQueue { view returns (uint256 ethToLock, uint256 sharesToBurn); - function finalize(uint256[] _batches, uint256 _maxShareRate) external payable; + function finalize(uint256 _lastIdToFinalize, uint256 _maxShareRate) external payable; function isPaused() external view returns (bool); @@ -282,12 +282,12 @@ contract Lido is Versioned, StETHPermit, AragonApp { LIDO_LOCATOR_POSITION.setStorageAddress(_lidoLocator); _initializeEIP712StETH(_eip712StETH); - // set unlimited allowance for burner from withdrawal queue + // set infinite allowance for burner from withdrawal queue // to burn finalized requests' shares _approve( ILidoLocator(_lidoLocator).withdrawalQueue(), ILidoLocator(_lidoLocator).burner(), - ~uint256(0) + INFINITE_ALLOWANCE ); emit LidoLocatorSet(_lidoLocator); @@ -851,7 +851,7 @@ contract Lido is Versioned, StETHPermit, AragonApp { if (_etherToLockOnWithdrawalQueue > 0) { IWithdrawalQueue withdrawalQueue = IWithdrawalQueue(_contracts.withdrawalQueue); withdrawalQueue.finalize.value(_etherToLockOnWithdrawalQueue)( - _withdrawalFinalizationBatches, + _withdrawalFinalizationBatches[_withdrawalFinalizationBatches.length - 1], _simulatedShareRate ); } diff --git a/contracts/0.4.24/StETH.sol b/contracts/0.4.24/StETH.sol index 86ec56d53..8a4b40ff6 100644 --- a/contracts/0.4.24/StETH.sol +++ b/contracts/0.4.24/StETH.sol @@ -51,6 +51,7 @@ contract StETH is IERC20, Pausable { using UnstructuredStorage for bytes32; address constant internal INITIAL_TOKEN_HOLDER = 0xdead; + uint256 constant internal INFINITE_ALLOWANCE = ~uint256(0); /** * @dev StETH balances are dynamic and are calculated based on the accounts' shares @@ -234,11 +235,8 @@ contract StETH is IERC20, Pausable { * @dev The `_amount` argument is the amount of tokens, not shares. */ function transferFrom(address _sender, address _recipient, uint256 _amount) external returns (bool) { - uint256 currentAllowance = allowances[_sender][msg.sender]; - require(currentAllowance >= _amount, "ALLOWANCE_EXCEEDED"); - + _spendAllowance(_sender, msg.sender, _amount); _transfer(_sender, _recipient, _amount); - _approve(_sender, msg.sender, currentAllowance.sub(_amount)); return true; } @@ -247,7 +245,7 @@ contract StETH is IERC20, Pausable { * * This is an alternative to `approve` that can be used as a mitigation for * problems described in: - * https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/IERC20.sol#L42 + * https://github.com/OpenZeppelin/openzeppelin-contracts/blob/b709eae01d1da91902d06ace340df6b324e6f049/contracts/token/ERC20/IERC20.sol#L57 * Emits an `Approval` event indicating the updated allowance. * * Requirements: @@ -264,7 +262,7 @@ contract StETH is IERC20, Pausable { * * This is an alternative to `approve` that can be used as a mitigation for * problems described in: - * https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/IERC20.sol#L42 + * https://github.com/OpenZeppelin/openzeppelin-contracts/blob/b709eae01d1da91902d06ace340df6b324e6f049/contracts/token/ERC20/IERC20.sol#L57 * Emits an `Approval` event indicating the updated allowance. * * Requirements: @@ -355,12 +353,9 @@ contract StETH is IERC20, Pausable { function transferSharesFrom( address _sender, address _recipient, uint256 _sharesAmount ) external returns (uint256) { - uint256 currentAllowance = allowances[_sender][msg.sender]; uint256 tokensAmount = getPooledEthByShares(_sharesAmount); - require(currentAllowance >= tokensAmount, "ALLOWANCE_EXCEEDED"); - + _spendAllowance(_sender, msg.sender, tokensAmount); _transferShares(_sender, _recipient, _sharesAmount); - _approve(_sender, msg.sender, currentAllowance.sub(tokensAmount)); _emitTransferEvents(_sender, _recipient, tokensAmount, _sharesAmount); return tokensAmount; } @@ -403,6 +398,22 @@ contract StETH is IERC20, Pausable { emit Approval(_owner, _spender, _amount); } + /** + * @dev Updates `owner` s allowance for `spender` based on spent `amount`. + * + * Does not update the allowance amount in case of infinite allowance. + * Revert if not enough allowance is available. + * + * Might emit an {Approval} event. + */ + function _spendAllowance(address _owner, address _spender, uint256 _amount) internal { + uint256 currentAllowance = allowances[_owner][_spender]; + if (currentAllowance != INFINITE_ALLOWANCE) { + require(currentAllowance >= _amount, "ALLOWANCE_EXCEEDED"); + _approve(_owner, _spender, currentAllowance - _amount); + } + } + /** * @return the total amount of shares in existence. */ diff --git a/contracts/0.4.24/nos/NodeOperatorsRegistry.sol b/contracts/0.4.24/nos/NodeOperatorsRegistry.sol index 72f078d95..21765d131 100644 --- a/contracts/0.4.24/nos/NodeOperatorsRegistry.sol +++ b/contracts/0.4.24/nos/NodeOperatorsRegistry.sol @@ -85,7 +85,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned { // SigningKeysStats /// @dev Operator's max validator keys count approved for deposit by the DAO uint8 internal constant TOTAL_VETTED_KEYS_COUNT_OFFSET = 0; - /// @dev Number of keys in the EXITED state for this operator for all time + /// @dev Number of keys in the EXITED state of this operator for all time uint8 internal constant TOTAL_EXITED_KEYS_COUNT_OFFSET = 1; /// @dev Total number of keys of this operator for all time uint8 internal constant TOTAL_KEYS_COUNT_OFFSET = 2; @@ -116,11 +116,11 @@ contract NodeOperatorsRegistry is AragonApp, Versioned { // Summary SigningKeysStats uint8 internal constant SUMMARY_MAX_VALIDATORS_COUNT_OFFSET = 0; - /// @dev Number of keys in the EXITED state for this operator for all time + /// @dev Number of keys of all operators which were in the EXITED state for all time uint8 internal constant SUMMARY_EXITED_KEYS_COUNT_OFFSET = 1; - /// @dev Total number of keys of this operator for all time + /// @dev Total number of keys of all operators for all time uint8 internal constant SUMMARY_TOTAL_KEYS_COUNT_OFFSET = 2; - /// @dev Number of keys of this operator which were in DEPOSITED state for all time + /// @dev Number of keys of all operators which were in the DEPOSITED state for all time uint8 internal constant SUMMARY_DEPOSITED_KEYS_COUNT_OFFSET = 3; // @@ -150,7 +150,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned { // bytes32 internal constant TYPE_POSITION = keccak256("lido.NodeOperatorsRegistry.type"); bytes32 internal constant TYPE_POSITION = 0xbacf4236659a602d72c631ba0b0d67ec320aaf523f3ae3590d7faee4f42351d0; - // bytes32 internal constant TYPE_POSITION = keccak256("lido.NodeOperatorsRegistry.stuckPenaltyDelay"); + // bytes32 internal constant STUCK_PENALTY_DELAY_POSITION = keccak256("lido.NodeOperatorsRegistry.stuckPenaltyDelay"); bytes32 internal constant STUCK_PENALTY_DELAY_POSITION = 0x8e3a1f3826a82c1116044b334cae49f3c3d12c3866a1c4b18af461e12e58a18e; // @@ -286,7 +286,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned { /// @return id a unique key of the added operator function addNodeOperator(string _name, address _rewardAddress) external returns (uint256 id) { _onlyValidNodeOperatorName(_name); - _onlyNonZeroAddress(_rewardAddress); + _onlyValidRewardAddress(_rewardAddress); _auth(MANAGE_NODE_OPERATOR_ROLE); id = getNodeOperatorsCount(); @@ -370,7 +370,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned { /// @param _nodeOperatorId Node operator id to set reward address for /// @param _rewardAddress Execution layer Ethereum address to set as reward address function setNodeOperatorRewardAddress(uint256 _nodeOperatorId, address _rewardAddress) external { - _onlyNonZeroAddress(_rewardAddress); + _onlyValidRewardAddress(_rewardAddress); _onlyExistedNodeOperator(_nodeOperatorId); _auth(MANAGE_NODE_OPERATOR_ROLE); @@ -383,7 +383,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned { /// @dev Current implementation preserves invariant: depositedSigningKeysCount <= vettedSigningKeysCount <= totalSigningKeysCount. /// If _vettedSigningKeysCount out of range [depositedSigningKeysCount, totalSigningKeysCount], the new vettedSigningKeysCount /// value will be set to the nearest range border. - /// @param _nodeOperatorId Node operator id to set reward address for + /// @param _nodeOperatorId Node operator id to set staking limit for /// @param _vettedSigningKeysCount New staking limit of the node operator function setNodeOperatorStakingLimit(uint256 _nodeOperatorId, uint64 _vettedSigningKeysCount) external { _onlyExistedNodeOperator(_nodeOperatorId); @@ -1439,6 +1439,13 @@ contract NodeOperatorsRegistry is AragonApp, Versioned { require(bytes(_name).length > 0 && bytes(_name).length <= MAX_NODE_OPERATOR_NAME_LENGTH, "WRONG_NAME_LENGTH"); } + function _onlyValidRewardAddress(address _rewardAddress) internal view { + _onlyNonZeroAddress(_rewardAddress); + // The Lido address is forbidden explicitly because stETH transfers on this contract will revert + // See onExitedAndStuckValidatorsCountsUpdated() and StETH._transferShares() for details + require(_rewardAddress != getLocator().lido(), "LIDO_REWARD_ADDRESS"); + } + function _onlyNonZeroAddress(address _a) internal pure { require(_a != address(0), "ZERO_ADDRESS"); } diff --git a/contracts/0.8.9/Burner.sol b/contracts/0.8.9/Burner.sol index 82b9faf43..696a2eb2d 100644 --- a/contracts/0.8.9/Burner.sol +++ b/contracts/0.8.9/Burner.sol @@ -61,7 +61,6 @@ contract Burner is IBurner, AccessControlEnumerable { error ZeroAddress(string field); bytes32 public constant REQUEST_BURN_MY_STETH_ROLE = keccak256("REQUEST_BURN_MY_STETH_ROLE"); - bytes32 public constant RECOVER_ASSETS_ROLE = keccak256("RECOVER_ASSETS_ROLE"); bytes32 public constant REQUEST_BURN_SHARES_ROLE = keccak256("REQUEST_BURN_SHARES_ROLE"); uint256 private coverSharesBurnRequested; @@ -164,7 +163,7 @@ contract Burner is IBurner, AccessControlEnumerable { * */ function requestBurnMyStETHForCover(uint256 _stETHAmountToBurn) external onlyRole(REQUEST_BURN_MY_STETH_ROLE) { - require(IStETH(STETH).transferFrom(msg.sender, address(this), _stETHAmountToBurn)); + IStETH(STETH).transferFrom(msg.sender, address(this), _stETHAmountToBurn); uint256 sharesAmount = IStETH(STETH).getSharesByPooledEth(_stETHAmountToBurn); _requestBurn(sharesAmount, _stETHAmountToBurn, true /* _isCover */); } @@ -197,7 +196,7 @@ contract Burner is IBurner, AccessControlEnumerable { * */ function requestBurnMyStETH(uint256 _stETHAmountToBurn) external onlyRole(REQUEST_BURN_MY_STETH_ROLE) { - require(IStETH(STETH).transferFrom(msg.sender, address(this), _stETHAmountToBurn)); + IStETH(STETH).transferFrom(msg.sender, address(this), _stETHAmountToBurn); uint256 sharesAmount = IStETH(STETH).getSharesByPooledEth(_stETHAmountToBurn); _requestBurn(sharesAmount, _stETHAmountToBurn, false /* _isCover */); } @@ -223,7 +222,7 @@ contract Burner is IBurner, AccessControlEnumerable { * but not marked for burning) to the Lido treasury address set upon the * contract construction. */ - function recoverExcessStETH() external onlyRole(RECOVER_ASSETS_ROLE) { + function recoverExcessStETH() external { uint256 excessStETH = getExcessStETH(); if (excessStETH > 0) { @@ -231,7 +230,7 @@ contract Burner is IBurner, AccessControlEnumerable { emit ExcessStETHRecovered(msg.sender, excessStETH, excessSharesAmount); - require(IStETH(STETH).transfer(TREASURY, excessStETH)); + IStETH(STETH).transfer(TREASURY, excessStETH); } } @@ -249,7 +248,7 @@ contract Burner is IBurner, AccessControlEnumerable { * @param _token an ERC20-compatible token * @param _amount token amount */ - function recoverERC20(address _token, uint256 _amount) external onlyRole(RECOVER_ASSETS_ROLE) { + function recoverERC20(address _token, uint256 _amount) external { if (_amount == 0) revert ZeroRecoveryAmount(); if (_token == STETH) revert StETHRecoveryWrongFunc(); @@ -265,7 +264,7 @@ contract Burner is IBurner, AccessControlEnumerable { * @param _token an ERC721-compatible token * @param _tokenId minted token id */ - function recoverERC721(address _token, uint256 _tokenId) external onlyRole(RECOVER_ASSETS_ROLE) { + function recoverERC721(address _token, uint256 _tokenId) external { if (_token == STETH) revert StETHRecoveryWrongFunc(); emit ERC721Recovered(msg.sender, _token, _tokenId); diff --git a/contracts/0.8.9/DepositSecurityModule.sol b/contracts/0.8.9/DepositSecurityModule.sol index 4e0059bd4..2cc0a4194 100644 --- a/contracts/0.8.9/DepositSecurityModule.sol +++ b/contracts/0.8.9/DepositSecurityModule.sol @@ -72,6 +72,11 @@ contract DepositSecurityModule { IStakingRouter public immutable STAKING_ROUTER; IDepositContract public immutable DEPOSIT_CONTRACT; + /** + * NB: both `maxDepositsPerBlock` and `minDepositBlockDistance` values + * must be harmonized with `OracleReportSanityChecker.churnValidatorsPerDayLimit` + * (see docs for the `OracleReportSanityChecker.setChurnValidatorsPerDayLimit` function) + */ uint256 internal maxDepositsPerBlock; uint256 internal minDepositBlockDistance; uint256 internal pauseIntentValidityPeriodBlocks; @@ -176,6 +181,9 @@ contract DepositSecurityModule { /** * Sets `maxDepositsPerBlock`. Only callable by the owner. + * + * NB: the value must be harmonized with `OracleReportSanityChecker.churnValidatorsPerDayLimit` + * (see docs for the `OracleReportSanityChecker.setChurnValidatorsPerDayLimit` function) */ function setMaxDeposits(uint256 newValue) external onlyOwner { _setMaxDeposits(newValue); @@ -195,6 +203,9 @@ contract DepositSecurityModule { /** * Sets `minDepositBlockDistance`. Only callable by the owner. + * + * NB: the value must be harmonized with `OracleReportSanityChecker.churnValidatorsPerDayLimit` + * (see docs for the `OracleReportSanityChecker.setChurnValidatorsPerDayLimit` function) */ function setMinDepositBlockDistance(uint256 newValue) external onlyOwner { _setMinDepositBlockDistance(newValue); diff --git a/contracts/0.8.9/OracleDaemonConfig.sol b/contracts/0.8.9/OracleDaemonConfig.sol index da5d4a4cf..028492197 100644 --- a/contracts/0.8.9/OracleDaemonConfig.sol +++ b/contracts/0.8.9/OracleDaemonConfig.sol @@ -38,6 +38,7 @@ contract OracleDaemonConfig is AccessControlEnumerable { function update(string calldata _key, bytes calldata _value) external onlyRole(CONFIG_MANAGER_ROLE) { if (values[_key].length == 0) revert ValueDoesntExist(_key); if (_value.length == 0) revert EmptyValue(_key); + if (keccak256(values[_key]) == keccak256(_value)) revert ValueIsSame(_key, _value); values[_key] = _value; emit ConfigValueUpdated(_key, _value); @@ -76,6 +77,7 @@ contract OracleDaemonConfig is AccessControlEnumerable { error EmptyValue(string key); error ValueDoesntExist(string key); error ZeroAddress(); + error ValueIsSame(string key, bytes value); event ConfigValueSet(string key, bytes value); event ConfigValueUpdated(string key, bytes value); diff --git a/contracts/0.8.9/StakingRouter.sol b/contracts/0.8.9/StakingRouter.sol index 0e2404864..adcb1d6bb 100644 --- a/contracts/0.8.9/StakingRouter.sol +++ b/contracts/0.8.9/StakingRouter.sol @@ -497,7 +497,7 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version uint256 totalExitedValidators, /* uint256 totalDepositedValidators */, /* uint256 depositableValidatorsCount */ - ) = IStakingModule(stakingModule.stakingModuleAddress).getNodeOperatorSummary(_nodeOperatorId); + ) = IStakingModule(moduleAddr).getNodeOperatorSummary(_nodeOperatorId); if (_correction.currentModuleExitedValidatorsCount != stakingModule.exitedValidatorsCount || _correction.currentNodeOperatorExitedValidatorsCount != totalExitedValidators || @@ -1038,8 +1038,8 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version } } - // sanity check - if (totalFee >= precisionPoints) revert ValueOver100Percent("totalFee"); + // Total fee never exceeds 100% + assert(totalFee <= precisionPoints); /// @dev shrink arrays if (rewardedStakingModulesCount < stakingModulesCount) { diff --git a/contracts/0.8.9/WithdrawalQueue.sol b/contracts/0.8.9/WithdrawalQueue.sol index 23caa00f6..733c4a830 100644 --- a/contracts/0.8.9/WithdrawalQueue.sol +++ b/contracts/0.8.9/WithdrawalQueue.sol @@ -48,20 +48,19 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit bytes32 public constant FINALIZE_ROLE = keccak256("FINALIZE_ROLE"); bytes32 public constant ORACLE_ROLE = keccak256("ORACLE_ROLE"); - /// @notice minimal possible sum that is possible to withdraw + /// @notice minimal amount of stETH that is possible to withdraw uint256 public constant MIN_STETH_WITHDRAWAL_AMOUNT = 100; - /// @notice maximum possible sum that is possible to withdraw by a single request + /// @notice maximum amount of stETH that is possible to withdraw by a single request /// Prevents accumulating too much funds per single request fulfillment in the future. /// @dev To withdraw larger amounts, it's recommended to split it to several requests uint256 public constant MAX_STETH_WITHDRAWAL_AMOUNT = 1000 * 1e18; - /// @notice Lido stETH token address to be set upon construction + /// @notice Lido stETH token address IStETH public immutable STETH; - /// @notice Lido wstETH token address to be set upon construction + /// @notice Lido wstETH token address IWstETH public immutable WSTETH; - /// @notice Emitted when the contract initialized event InitializedV1(address _admin); event BunkerModeEnabled(uint256 _sinceTimestamp); event BunkerModeDisabled(); @@ -72,6 +71,7 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit error InvalidReportTimestamp(); error RequestIdsNotSorted(); error ZeroRecipient(); + error ArraysLengthMismatch(uint256 _firstArrayLength, uint256 _secondArrayLength); /// @param _wstETH address of WstETH contract constructor(IWstETH _wstETH) { @@ -85,72 +85,71 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit /// @dev Reverts if `_admin` equals to `address(0)` /// @dev NB! It's initialized in paused state by default and should be resumed explicitly to start /// @dev NB! Bunker mode is disabled by default - function initialize(address _admin) - external - { + function initialize(address _admin) external { if (_admin == address(0)) revert AdminZeroAddress(); _initialize(_admin); } /// @notice Resume withdrawal requests placement and finalization + /// Contract is deployed in paused state and should be resumed explicitly function resume() external { _checkRole(RESUME_ROLE, msg.sender); _resume(); } /// @notice Pause withdrawal requests placement and finalization. Claiming finalized requests will still be available - /// @param _duration pause duration, seconds (use `PAUSE_INFINITELY` for unlimited) - /// @dev Reverts with `ResumedExpected()` if contract is already paused - /// @dev Reverts with `AccessControl:...` reason if sender has no `PAUSE_ROLE` - /// @dev Reverts with `ZeroPauseDuration()` if zero duration is passed + /// @param _duration pause duration in seconds (use `PAUSE_INFINITELY` for unlimited) + /// @dev Reverts if contract is already paused + /// @dev Reverts reason if sender has no `PAUSE_ROLE` + /// @dev Reverts if zero duration is passed function pauseFor(uint256 _duration) external onlyRole(PAUSE_ROLE) { _pauseFor(_duration); } /// @notice Pause withdrawal requests placement and finalization. Claiming finalized requests will still be available /// @param _pauseUntilInclusive the last second to pause until inclusive - /// @dev Reverts with `ResumeSinceInPast()` if the timestamp is in the past - /// @dev Reverts with `AccessControl:...` reason if sender has no `PAUSE_ROLE` - /// @dev Reverts with `ResumedExpected()` if contract is already paused + /// @dev Reverts if the timestamp is in the past + /// @dev Reverts if sender has no `PAUSE_ROLE` + /// @dev Reverts if contract is already paused function pauseUntil(uint256 _pauseUntilInclusive) external onlyRole(PAUSE_ROLE) { _pauseUntil(_pauseUntilInclusive); } - /// @notice Request the sequence of stETH withdrawals according to passed `withdrawalRequestInputs` data - /// @param amounts an array of stETH amount values. The standalone withdrawal request will - /// be created for each item in the passed list. - /// @param _owner address that will be able to transfer or claim the request. - /// If `owner` is set to `address(0)`, `msg.sender` will be used as owner. - /// @return requestIds an array of the created withdrawal requests - function requestWithdrawals(uint256[] calldata amounts, address _owner) + /// @notice Request the batch of stETH for withdrawal. Approvals for the passed amounts should be done before. + /// @param _amounts an array of stETH amount values. + /// The standalone withdrawal request will be created for each item in the passed list. + /// @param _owner address that will be able to manage the created requests. + /// If `address(0)` is passed, `msg.sender` will be used as owner. + /// @return requestIds an array of the created withdrawal request ids + function requestWithdrawals(uint256[] calldata _amounts, address _owner) public returns (uint256[] memory requestIds) { _checkResumed(); if (_owner == address(0)) _owner = msg.sender; - requestIds = new uint256[](amounts.length); - for (uint256 i = 0; i < amounts.length; ++i) { - _checkWithdrawalRequestAmount(amounts[i]); - requestIds[i] = _requestWithdrawal(amounts[i], _owner); + requestIds = new uint256[](_amounts.length); + for (uint256 i = 0; i < _amounts.length; ++i) { + _checkWithdrawalRequestAmount(_amounts[i]); + requestIds[i] = _requestWithdrawal(_amounts[i], _owner); } } - /// @notice Request the sequence of wstETH withdrawals according to passed `withdrawalRequestInputs` data - /// @param amounts an array of stETH amount values. The standalone withdrawal request will - /// be created for each item in the passed list. - /// @param _owner address that will be able to transfer or claim the request. - /// If `owner` is set to `address(0)`, `msg.sender` will be used as owner. - /// @return requestIds an array of the created withdrawal requests - function requestWithdrawalsWstETH(uint256[] calldata amounts, address _owner) + /// @notice Request the batch of wstETH for withdrawal. Approvals for the passed amounts should be done before. + /// @param _amounts an array of wstETH amount values. + /// The standalone withdrawal request will be created for each item in the passed list. + /// @param _owner address that will be able to manage the created requests. + /// If `address(0)` is passed, `msg.sender` will be used as an owner. + /// @return requestIds an array of the created withdrawal request ids + function requestWithdrawalsWstETH(uint256[] calldata _amounts, address _owner) public returns (uint256[] memory requestIds) { _checkResumed(); if (_owner == address(0)) _owner = msg.sender; - requestIds = new uint256[](amounts.length); - for (uint256 i = 0; i < amounts.length; ++i) { - requestIds[i] = _requestWithdrawalWstETH(amounts[i], _owner); + requestIds = new uint256[](_amounts.length); + for (uint256 i = 0; i < _amounts.length; ++i) { + requestIds[i] = _requestWithdrawalWstETH(_amounts[i], _owner); } } @@ -162,14 +161,13 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit bytes32 s; } - /// @notice Request the sequence of stETH withdrawals according to passed `withdrawalRequestInputs` - /// using EIP-2612 Permit - /// @param _amounts an array of stETH amount values. The standalone withdrawal request will - /// be created for each item in the passed list. - /// @param _owner address that will be able to transfer or claim the request. - /// If `owner` is set to `address(0)`, `msg.sender` will be used as owner. + /// @notice Request the batch of stETH for withdrawal using EIP-2612 Permit + /// @param _amounts an array of stETH amount values + /// The standalone withdrawal request will be created for each item in the passed list. + /// @param _owner address that will be able to manage the created requests. + /// If `address(0)` is passed, `msg.sender` will be used as an owner. /// @param _permit data required for the stETH.permit() method to set the allowance - /// @return requestIds an array of the created withdrawal requests + /// @return requestIds an array of the created withdrawal request ids function requestWithdrawalsWithPermit(uint256[] calldata _amounts, address _owner, PermitInput calldata _permit) external returns (uint256[] memory requestIds) @@ -178,14 +176,13 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit return requestWithdrawals(_amounts, _owner); } - /// @notice Request the sequence of wstETH withdrawals according to passed `withdrawalRequestInputs` data - /// using EIP-2612 Permit - /// @param _amounts an array of stETH amount values. The standalone withdrawal request will - /// be created for each item in the passed list. - /// @param _owner address that will be able to transfer or claim the request. - /// If `owner` is set to `address(0)`, `msg.sender` will be used as owner. - /// @param _permit data required for the wstETH.permit() method to set the allowance - /// @return requestIds an array of the created withdrawal requests + /// @notice Request the batch of wstETH for withdrawal using EIP-2612 Permit + /// @param _amounts an array of wstETH amount values + /// The standalone withdrawal request will be created for each item in the passed list. + /// @param _owner address that will be able to manage the created requests. + /// If `address(0)` is passed, `msg.sender` will be used as an owner. + /// @param _permit data required for the wtETH.permit() method to set the allowance + /// @return requestIds an array of the created withdrawal request ids function requestWithdrawalsWstETHWithPermit( uint256[] calldata _amounts, address _owner, @@ -205,7 +202,7 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit return _getRequestsByOwner()[_owner].values(); } - /// @notice Returns statuses for the array of request ids + /// @notice Returns status for requests with provided ids /// @param _requestIds array of withdrawal request ids function getWithdrawalStatus(uint256[] calldata _requestIds) external @@ -218,10 +215,11 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit } } - /// @notice Returns array of claimable eth amounts that is locked for each request - /// @param _requestIds array of request ids to find claimable ether for - /// @param _hints checkpoint hint for each id. - /// Can be retrieved with `findCheckpointHints()` + /// @notice Returns amount of ether available for claim for each provided request id + /// @param _requestIds array of request ids + /// @param _hints checkpoint hints. can be found with `findCheckpointHints(_requestIds, 1, getLastCheckpointIndex())` + /// @return claimableEthValues amount of claimable ether for each request, amount is equal to 0 if request + /// is not finalized or already claimed function getClaimableEther(uint256[] calldata _requestIds, uint256[] calldata _hints) external view @@ -233,13 +231,13 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit } } - /// @notice Claim a batch of withdrawal requests once finalized (claimable) sending ether to `_recipient` + /// @notice Claim a batch of withdrawal requests if they are finalized sending ether to `_recipient` /// @param _requestIds array of request ids to claim - /// @param _hints checkpoint hint for each id. - /// Can be retrieved with `findCheckpointHints()` + /// @param _hints checkpoint hint for each id. Can be obtained with `findCheckpointHints()` /// @param _recipient address where claimed ether will be sent to /// @dev /// Reverts if recipient is equal to zero + /// Reverts if requestIds and hints arrays length differs /// Reverts if any requestId or hint in arguments are not valid /// Reverts if any request is not finalized or already claimed /// Reverts if msg sender is not an owner of the requests @@ -247,6 +245,9 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit external { if (_recipient == address(0)) revert ZeroRecipient(); + if (_requestIds.length != _hints.length) { + revert ArraysLengthMismatch(_requestIds.length, _hints.length); + } for (uint256 i = 0; i < _requestIds.length; ++i) { _claim(_requestIds[i], _hints[i], _recipient); @@ -254,15 +255,19 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit } } - /// @notice Claim a batch of withdrawal requests once finalized (claimable) sending locked ether to the owner + /// @notice Claim a batch of withdrawal requests if they are finalized sending locked ether to the owner /// @param _requestIds array of request ids to claim - /// @param _hints checkpoint hint for each id. - /// Can be retrieved with `findCheckpointHints()` + /// @param _hints checkpoint hint for each id. Can be obtained with `findCheckpointHints()` /// @dev + /// Reverts if requestIds and hints arrays length differs /// Reverts if any requestId or hint in arguments are not valid /// Reverts if any request is not finalized or already claimed /// Reverts if msg sender is not an owner of the requests function claimWithdrawals(uint256[] calldata _requestIds, uint256[] calldata _hints) external { + if (_requestIds.length != _hints.length) { + revert ArraysLengthMismatch(_requestIds.length, _hints.length); + } + for (uint256 i = 0; i < _requestIds.length; ++i) { _claim(_requestIds[i], _hints[i], msg.sender); _emitTransfer(msg.sender, address(0), _requestIds[i]); @@ -282,11 +287,14 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit } /// @notice Finds the list of hints for the given `_requestIds` searching among the checkpoints with indices - /// in the range `[_firstIndex, _lastIndex]`. NB! Array of request ids should be sorted + /// in the range `[_firstIndex, _lastIndex]`. + /// NB! Array of request ids should be sorted + /// NB! `_firstIndex` should be greater than 0, because checkpoint list is 1-based array + /// Usage: findCheckpointHints(_requestIds, 1, getLastCheckpointIndex()) /// @param _requestIds ids of the requests sorted in the ascending order to get hints for - /// @param _firstIndex left boundary of the search range - /// @param _lastIndex right boundary of the search range - /// @return hintIds the hints for `claimWithdrawal` to find the checkpoint for the passed request ids + /// @param _firstIndex left boundary of the search range. Should be greater than 0 + /// @param _lastIndex right boundary of the search range. Should be less than or equal to getLastCheckpointIndex() + /// @return hintIds array of hints used to find required checkpoint for the request function findCheckpointHints(uint256[] calldata _requestIds, uint256 _firstIndex, uint256 _lastIndex) external view @@ -302,25 +310,15 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit } } - /// @notice Finalize requests from last finalized one up to `_lastRequestIdToFinalize` - /// @dev ether to finalize all the requests should be calculated using `finalizationValue()` and sent along - function finalize(uint256[] calldata _batches, uint256 _maxShareRate) - external - payable - { - _checkResumed(); - _checkRole(FINALIZE_ROLE, msg.sender); - - _finalize(_batches, msg.value, _maxShareRate); - } - - /// @notice Update bunker mode state and last report timestamp + /// @notice Update bunker mode state and last report timestamp on oracle report /// @dev should be called by oracle /// /// @param _isBunkerModeNow is bunker mode reported by oracle /// @param _bunkerStartTimestamp timestamp of start of the bunker mode /// @param _currentReportTimestamp timestamp of the current report ref slot - function onOracleReport(bool _isBunkerModeNow, uint256 _bunkerStartTimestamp, uint256 _currentReportTimestamp) external { + function onOracleReport(bool _isBunkerModeNow, uint256 _bunkerStartTimestamp, uint256 _currentReportTimestamp) + external + { _checkRole(ORACLE_ROLE, msg.sender); if (_bunkerStartTimestamp >= block.timestamp) revert InvalidReportTimestamp(); if (_currentReportTimestamp >= block.timestamp) revert InvalidReportTimestamp(); @@ -359,9 +357,7 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit function _emitTransfer(address from, address to, uint256 _requestId) internal virtual; /// @dev internal initialization helper. Doesn't check provided addresses intentionally - function _initialize(address _admin) - internal - { + function _initialize(address _admin) internal { _initializeQueue(); _pauseFor(PAUSE_INFINITELY); @@ -405,7 +401,7 @@ abstract contract WithdrawalQueue is AccessControlEnumerable, PausableUntil, Wit } } - /// @notice returns claimable ether under the request with _requestId. + /// @notice returns claimable ether under the request. Returns 0 if request is not finalized or claimed function _getClaimableEther(uint256 _requestId, uint256 _hint) internal view returns (uint256) { if (_requestId == 0 || _requestId > getLastRequestId()) revert InvalidRequestId(_requestId); diff --git a/contracts/0.8.9/WithdrawalQueueBase.sol b/contracts/0.8.9/WithdrawalQueueBase.sol index 2f3bb499c..bd443abc4 100644 --- a/contracts/0.8.9/WithdrawalQueueBase.sol +++ b/contracts/0.8.9/WithdrawalQueueBase.sol @@ -16,37 +16,37 @@ abstract contract WithdrawalQueueBase { using EnumerableSet for EnumerableSet.UintSet; using UnstructuredStorage for bytes32; - /// @notice precision base for share rate and discounting factor values in the contract - uint256 internal constant E27_PRECISION_BASE = 1e27; - /// @dev maximal length of the batches array that oracle should deliver on finalization + /// @dev maximal length of the batch array provided for prefinalization. See `prefinalize()` uint256 public constant MAX_BATCHES_LENGTH = 36; + + /// @notice precision base for share rate + uint256 internal constant E27_PRECISION_BASE = 1e27; /// @dev return value for the `find...` methods in case of no result uint256 internal constant NOT_FOUND = 0; /// @dev queue for withdrawal requests, indexes (requestId) start from 1 bytes32 internal constant QUEUE_POSITION = keccak256("lido.WithdrawalQueue.queue"); - /// @dev length of the queue + /// @dev last index in request queue bytes32 internal constant LAST_REQUEST_ID_POSITION = keccak256("lido.WithdrawalQueue.lastRequestId"); - /// @dev length of the finalized part of the queue. Always <= `requestCounter` + /// @dev last index of finalized request in the queue bytes32 internal constant LAST_FINALIZED_REQUEST_ID_POSITION = keccak256("lido.WithdrawalQueue.lastFinalizedRequestId"); - /// @dev finalization discount history, indexes start from 1 + /// @dev finalization rate history, indexes start from 1 bytes32 internal constant CHECKPOINTS_POSITION = keccak256("lido.WithdrawalQueue.checkpoints"); - /// @dev length of the checkpoints + /// @dev last index in checkpoints array bytes32 internal constant LAST_CHECKPOINT_INDEX_POSITION = keccak256("lido.WithdrawalQueue.lastCheckpointIndex"); - /// @dev amount of eth locked on contract for withdrawal + /// @dev amount of eth locked on contract for further claiming bytes32 internal constant LOCKED_ETHER_AMOUNT_POSITION = keccak256("lido.WithdrawalQueue.lockedEtherAmount"); /// @dev withdrawal requests mapped to the owners bytes32 internal constant REQUEST_BY_OWNER_POSITION = keccak256("lido.WithdrawalQueue.requestsByOwner"); /// @dev timestamp of the last oracle report bytes32 internal constant LAST_REPORT_TIMESTAMP_POSITION = keccak256("lido.WithdrawalQueue.lastReportTimestamp"); - - /// @notice structure representing a request for withdrawal. + /// @notice structure representing a request for withdrawal struct WithdrawalRequest { - /// @notice sum of the all stETH submitted for withdrawals up to this request + /// @notice sum of the all stETH submitted for withdrawals including this request uint128 cumulativeStETH; - /// @notice sum of the all shares locked for withdrawal up to this request + /// @notice sum of the all shares locked for withdrawal including this request uint128 cumulativeShares; /// @notice address that can claim or transfer the request address owner; @@ -111,12 +111,14 @@ abstract contract WithdrawalQueueBase { error InvalidHint(uint256 _hint); error CantSendValueRecipientMayHaveReverted(); - /// @notice id of the last request, returns 0, if no request in the queue + /// @notice id of the last request + /// NB! requests are indexed from 1, so it returns 0 if there is no requests in the queue function getLastRequestId() public view returns (uint256) { return LAST_REQUEST_ID_POSITION.getStorageUint256(); } - /// @notice id of the last finalized request, returns 0 if no finalized requests in the queue + /// @notice id of the last finalized request + /// NB! requests are indexed from 1, so it returns 0 if there is no finalized requests in the queue function getLastFinalizedRequestId() public view returns (uint256) { return LAST_FINALIZED_REQUEST_ID_POSITION.getStorageUint256(); } @@ -126,7 +128,8 @@ abstract contract WithdrawalQueueBase { return LOCKED_ETHER_AMOUNT_POSITION.getStorageUint256(); } - /// @notice length of the checkpoints. Last possible value for the claim hint + /// @notice length of the checkpoint array. Last possible value for the hint. + /// NB! checkpoints are indexed from 1, so it returns 0 if there is no checkpoints function getLastCheckpointIndex() public view returns (uint256) { return LAST_CHECKPOINT_INDEX_POSITION.getStorageUint256(); } @@ -154,10 +157,10 @@ abstract contract WithdrawalQueueBase { // - current share rate of the protocol // - id of the last request that can be finalized // - the amount of eth that must be locked for these requests - // To calculate the eth amount we'll need to know which requests int the queue will be finalized as nominal + // To calculate the eth amount we'll need to know which requests in the queue will be finalized as nominal // and which as discounted and the exact value of the discount. It's impossible to calculate without the unbounded // loop over the unfinalized part of the queue. So, we need to extract a part of the algorithm off-chain, bring the - // result with oracle report and check it later and check the resukt later. + // result with oracle report and check it later and check the result later. // So, we came to this solution: // Off-chain // 1. Oracle iterates over the queue off-chain and calculate the id of the latest finalizable request @@ -171,26 +174,26 @@ abstract contract WithdrawalQueueBase { // set's the discount checkpoint for these request's if required that will be applied on claim for each request's // individually depending on request's share rate. - /// @notice transient state that is used to pass intemediate results between several `calculateFinalizationBatches` - // invokations + /// @notice transient state that is used to pass intermediate results between several `calculateFinalizationBatches` + // invocations struct BatchesCalculationState { /// @notice amount of ether available in the protocol that can be used to finalize withdrawal requests - /// Will decrease on each invokation and will be equal to the remainder when calculation is finished - /// Should be set before the first invokation + /// Will decrease on each call and will be equal to the remainder when calculation is finished + /// Should be set before the first call uint256 remainingEthBudget; - /// @notice flag that is `true` if returned state is final and `false` if more invokations required + /// @notice flag that is set to `true` if returned state is final and `false` if more calls are required bool finished; - /// @notice static array to store all the batches ending request id + /// @notice static array to store last request id in each batch uint256[MAX_BATCHES_LENGTH] batches; /// @notice length of the filled part of `batches` array uint256 batchesLength; } /// @notice Offchain view for the oracle daemon that calculates how many requests can be finalized within - /// the given budget and timestamp and share rate limits. Returned requests are split into the batches. + /// the given budget, time period and share rate limits. Returned requests are split into batches. /// Each batch consist of the requests that all have the share rate below the `_maxShareRate` or above it. /// Below you can see an example how 14 requests with different share rates will be split into 5 batches by - /// this algorithm + /// this method /// /// ^ share rate /// | @@ -202,26 +205,19 @@ abstract contract WithdrawalQueueBase { /// +-------------------------------> requestId /// | 1st| 2nd |3| 4th | 5th | /// - /// @param _maxShareRate current share rate of the protocol with 1e27 precision + /// @param _maxShareRate current share rate of the protocol (1e27 precision) /// @param _maxTimestamp max timestamp of the request that can be finalized - /// @param _maxRequestsPerCall max request number that can be processed by the call. Better to me max possible - /// number for EL node to handle before hitting `out of gas`. More this number is less calls it will require to - /// calculate the result - /// @param _state structure that accumulates the state across multiple invokations to overcome gas limits. + /// @param _maxRequestsPerCall max request number that can be processed per call. + /// @param _state structure that accumulates the state across multiple invocations to overcome gas limits. /// To start calculation you should pass `state.remainingEthBudget` and `state.finished == false` and then invoke /// the function with returned `state` until it returns a state with `finished` flag set - /// @return state that was changed during this function invokation. - /// If (state.finished) than calculation is finished and returned `state` is ready to be used + /// @return state that is changing on each call and should be passed to the next call until `state.finished` is true function calculateFinalizationBatches( uint256 _maxShareRate, uint256 _maxTimestamp, uint256 _maxRequestsPerCall, BatchesCalculationState memory _state - ) - external - view - returns (BatchesCalculationState memory) - { + ) external view returns (BatchesCalculationState memory) { if (_state.finished || _state.remainingEthBudget == 0) revert InvalidState(); uint256 currentId; @@ -246,7 +242,7 @@ abstract contract WithdrawalQueueBase { while (currentId < queueLength && currentId < nextCallRequestId) { WithdrawalRequest memory request = _getQueue()[currentId]; - if (request.timestamp > _maxTimestamp) break; // max timestamp break + if (request.timestamp > _maxTimestamp) break; // max timestamp break (uint256 requestShareRate, uint256 ethToFinalize, uint256 shares) = _calcBatch(prevRequest, request); @@ -261,17 +257,17 @@ abstract contract WithdrawalQueueBase { if (_state.batchesLength != 0 && ( // share rate of requests in the same batch can differ by 1-2 wei because of the rounding error // (issue: https://github.com/lidofinance/lido-dao/issues/442 ) - // so we're counting requests that are placed during the same report day + // so we're taking requests that are placed during the same report // as equal even if their actual share rate are different prevRequest.reportTimestamp == request.reportTimestamp || - // both requests are below or + // both requests are below the line prevRequestShareRate <= _maxShareRate && requestShareRate <= _maxShareRate || - // both are above the line + // both requests are above the line prevRequestShareRate > _maxShareRate && requestShareRate > _maxShareRate )) { _state.batches[_state.batchesLength - 1] = currentId; // extend the last batch } else { - // to be able to check batches on-chain we need it to have fixed max length + // to be able to check batches on-chain we need array to have limited length if (_state.batchesLength == MAX_BATCHES_LENGTH) break; // create a new batch @@ -289,11 +285,11 @@ abstract contract WithdrawalQueueBase { return _state; } - /// @notice Checks the finalization batches, calculates required ether and the amount of shares to burn and - /// @param _batches finalization batches calculated offchain using `calculateFinalizationBatches` - /// @param _maxShareRate max possible share rate that will be used for request finalization with 1e27 precision - /// @return ethToLock amount of ether that should be sent with `finalize()` method later - /// @return sharesToBurn amount of shares that belongs tho finalizable requests + /// @notice Checks finalization batches, calculates required ether and the amount of shares to burn + /// @param _batches finalization batches calculated offchain using `calculateFinalizationBatches()` + /// @param _maxShareRate max share rate that will be used for request finalization (1e27 precision) + /// @return ethToLock amount of ether that should be sent with `finalize()` method + /// @return sharesToBurn amount of shares that belongs to requests that will be finalized function prefinalize(uint256[] calldata _batches, uint256 _maxShareRate) external view @@ -332,18 +328,14 @@ abstract contract WithdrawalQueueBase { } /// @dev Finalize requests in the queue - /// Emits WithdrawalBatchFinalized event. - /// Checks that: - /// - _amountOfETH is less or equal to the nominal value of all requests to be finalized - function _finalize(uint256[] memory _batches, uint256 _amountOfETH, uint256 _maxShareRate) internal { - if (_batches.length == 0) revert EmptyBatches(); - uint256 lastRequestIdToBeFinalized = _batches[_batches.length - 1]; - if (lastRequestIdToBeFinalized > getLastRequestId()) revert InvalidRequestId(lastRequestIdToBeFinalized); + /// Emits WithdrawalsFinalized event. + function _finalize(uint256 _lastRequestIdToBeFinalized, uint256 _amountOfETH, uint256 _maxShareRate) internal { + if (_lastRequestIdToBeFinalized > getLastRequestId()) revert InvalidRequestId(_lastRequestIdToBeFinalized); uint256 lastFinalizedRequestId = getLastFinalizedRequestId(); - if (lastRequestIdToBeFinalized <= lastFinalizedRequestId) revert InvalidRequestId(lastRequestIdToBeFinalized); + if (_lastRequestIdToBeFinalized <= lastFinalizedRequestId) revert InvalidRequestId(_lastRequestIdToBeFinalized); WithdrawalRequest memory lastFinalizedRequest = _getQueue()[lastFinalizedRequestId]; - WithdrawalRequest memory requestToFinalize = _getQueue()[lastRequestIdToBeFinalized]; + WithdrawalRequest memory requestToFinalize = _getQueue()[_lastRequestIdToBeFinalized]; uint128 stETHToFinalize = requestToFinalize.cumulativeStETH - lastFinalizedRequest.cumulativeStETH; if (_amountOfETH > stETHToFinalize) revert TooMuchEtherToFinalize(_amountOfETH, stETHToFinalize); @@ -356,20 +348,19 @@ abstract contract WithdrawalQueueBase { _setLastCheckpointIndex(lastCheckpointIndex + 1); _setLockedEtherAmount(getLockedEtherAmount() + _amountOfETH); - _setLastFinalizedRequestId(lastRequestIdToBeFinalized); + _setLastFinalizedRequestId(_lastRequestIdToBeFinalized); emit WithdrawalsFinalized( firstRequestIdToFinalize, - lastRequestIdToBeFinalized, + _lastRequestIdToBeFinalized, _amountOfETH, requestToFinalize.cumulativeShares - lastFinalizedRequest.cumulativeShares, block.timestamp - ); + ); } /// @dev creates a new `WithdrawalRequest` in the queue /// Emits WithdrawalRequested event - /// Does not check parameters function _enqueue(uint128 _amountOfStETH, uint128 _amountOfShares, address _owner) internal returns (uint256 requestId) @@ -398,7 +389,7 @@ abstract contract WithdrawalQueueBase { emit WithdrawalRequested(requestId, msg.sender, _owner, _amountOfStETH, _amountOfShares); } - /// @dev Returns status of the withdrawal request with `_requestId` id + /// @dev Returns the status of the withdrawal request with `_requestId` id function _getStatus(uint256 _requestId) internal view returns (WithdrawalRequestStatus memory status) { if (_requestId == 0 || _requestId > getLastRequestId()) revert InvalidRequestId(_requestId); @@ -415,28 +406,22 @@ abstract contract WithdrawalQueueBase { ); } - /// @dev View function to find a checkpoint hint for `claimWithdrawal()` + /// @dev View function to find a checkpoint hint to use in `claimWithdrawal()` and `getClaimableEther()` /// Search will be performed in the range of `[_firstIndex, _lastIndex]` /// - /// NB!: Range search ought to be used to optimize gas cost. - /// You can utilize the following invariant: - /// `if (requestId2 > requestId1) than hint2 >= hint1`, - /// so you can search for `hint2` in the range starting from `hint1` + /// @param _requestId request id to search the checkpoint for + /// @param _start index of the left boundary of the search range, should be greater than 0 + /// @param _end index of the right boundary of the search range, should be less than or equal + /// to `getLastCheckpointIndex()` /// - /// @param _requestId request id we are searching the checkpoint for - /// @param _start index of the left boundary of the search range - /// @param _end index of the right boundary of the search range - /// - /// @return value that hints `claimWithdrawal` to find the discount for the request, - /// or 0 if hint not found in the range + /// @return hint for later use in other methods or 0 if hint not found in the range function _findCheckpointHint(uint256 _requestId, uint256 _start, uint256 _end) internal view returns (uint256) { - if (_requestId == 0) revert InvalidRequestId(_requestId); - if (_start == 0) revert InvalidRequestIdRange(_start, _end); + if (_requestId == 0 || _requestId > getLastRequestId()) revert InvalidRequestId(_requestId); + uint256 lastCheckpointIndex = getLastCheckpointIndex(); - if (_end > lastCheckpointIndex) revert InvalidRequestIdRange(_start, _end); - if (_requestId > getLastFinalizedRequestId()) revert RequestNotFoundOrNotFinalized(_requestId); + if (_start == 0 || _end > lastCheckpointIndex) revert InvalidRequestIdRange(_start, _end); - if (_start > _end) return NOT_FOUND; // we have an empty range to search in, so return NOT_FOUND + if (lastCheckpointIndex == 0 || _requestId > getLastFinalizedRequestId() || _start > _end) return NOT_FOUND; // Right boundary if (_requestId >= _getCheckpoints()[_end].fromRequestId) { @@ -467,9 +452,10 @@ abstract contract WithdrawalQueueBase { return min; } - /// @dev Claim `_requestId` request and transfer locked ether to `_recipient`. Emits WithdrawalClaimed event - /// @param _requestId request id to claim - /// @param _hint hint for discount checkpoint index to avoid extensive search over the checkpoints. + /// @dev Claim the request and transfer locked ether to `_recipient`. + /// Emits WithdrawalClaimed event + /// @param _requestId id of the request to claim + /// @param _hint hint the checkpoint to use. Can be obtained by calling `findCheckpointHint()` /// @param _recipient address to send ether to function _claim(uint256 _requestId, uint256 _hint, address _recipient) internal { if (_requestId == 0) revert InvalidRequestId(_requestId); @@ -488,13 +474,13 @@ abstract contract WithdrawalQueueBase { // (issue: https://github.com/lidofinance/lido-dao/issues/442 ) // some dust (1-2 wei per request) will be accumulated upon claiming _setLockedEtherAmount(getLockedEtherAmount() - ethWithDiscount); - _sendValue(payable(_recipient), ethWithDiscount); + _sendValue(_recipient, ethWithDiscount); emit WithdrawalClaimed(_requestId, msg.sender, _recipient, ethWithDiscount); } - /// @dev Calculates discounted ether value for `_requestId` using a provided `_hint`. Checks if hint is valid - /// @return claimableEther discounted eth for `_requestId`. Returns 0 if request is not claimable + /// @dev Calculates ether value for the request using the provided hint. Checks if hint is valid + /// @return claimableEther discounted eth for `_requestId` function _calculateClaimableEther(WithdrawalRequest storage _request, uint256 _requestId, uint256 _hint) internal view @@ -544,11 +530,12 @@ abstract contract WithdrawalQueueBase { if (!success) revert CantSendValueRecipientMayHaveReverted(); } - /// @dev calculate batch stats (shareRate, stETH and shares) for the batch of `(_preStartRequest, _endRequest]` - function _calcBatch( - WithdrawalRequest memory _preStartRequest, - WithdrawalRequest memory _endRequest - ) internal pure returns (uint256 shareRate, uint256 stETH, uint256 shares) { + /// @dev calculate batch stats (shareRate, stETH and shares) for the range of `(_preStartRequest, _endRequest]` + function _calcBatch(WithdrawalRequest memory _preStartRequest, WithdrawalRequest memory _endRequest) + internal + pure + returns (uint256 shareRate, uint256 stETH, uint256 shares) + { stETH = _endRequest.cumulativeStETH - _preStartRequest.cumulativeStETH; shares = _endRequest.cumulativeShares - _preStartRequest.cumulativeShares; diff --git a/contracts/0.8.9/WithdrawalQueueERC721.sol b/contracts/0.8.9/WithdrawalQueueERC721.sol index 67a2f1c08..ecaeef17d 100644 --- a/contracts/0.8.9/WithdrawalQueueERC721.sol +++ b/contracts/0.8.9/WithdrawalQueueERC721.sol @@ -8,6 +8,7 @@ import {IERC721} from "@openzeppelin/contracts-v4.4/token/ERC721/IERC721.sol"; import {IERC721Receiver} from "@openzeppelin/contracts-v4.4/token/ERC721/IERC721Receiver.sol"; import {IERC721Metadata} from "@openzeppelin/contracts-v4.4/token/ERC721/extensions/IERC721Metadata.sol"; import {IERC165} from "@openzeppelin/contracts-v4.4/utils/introspection/IERC165.sol"; +import {IERC4906} from "./interfaces/IERC4906.sol"; import {EnumerableSet} from "@openzeppelin/contracts-v4.4/utils/structs/EnumerableSet.sol"; import {Address} from "@openzeppelin/contracts-v4.4/utils/Address.sol"; @@ -18,14 +19,10 @@ import {AccessControlEnumerable} from "./utils/access/AccessControlEnumerable.so import {UnstructuredRefStorage} from "./lib/UnstructuredRefStorage.sol"; import {UnstructuredStorage} from "./lib/UnstructuredStorage.sol"; -/** - * @title Interface defining INFTDescriptor to generate ERC721 tokenURI - */ +/// @title Interface defining INFTDescriptor to generate ERC721 tokenURI interface INFTDescriptor { - /** - * @notice Returns ERC721 tokenURI content - * @param _requestId is an id for particular withdrawal request - */ + /// @notice Returns ERC721 tokenURI content + /// @param _requestId is an id for particular withdrawal request function constructTokenURI(uint256 _requestId) external view returns (string memory); } @@ -33,7 +30,7 @@ interface INFTDescriptor { /// NFT is minted on every request and burned on claim /// /// @author psirex, folkyatina -contract WithdrawalQueueERC721 is IERC721Metadata, WithdrawalQueue { +contract WithdrawalQueueERC721 is IERC721Metadata, IERC4906, WithdrawalQueue { using Address for address; using Strings for uint256; using EnumerableSet for EnumerableSet.UintSet; @@ -43,7 +40,8 @@ contract WithdrawalQueueERC721 is IERC721Metadata, WithdrawalQueue { bytes32 internal constant TOKEN_APPROVALS_POSITION = keccak256("lido.WithdrawalQueueERC721.tokenApprovals"); bytes32 internal constant OPERATOR_APPROVALS_POSITION = keccak256("lido.WithdrawalQueueERC721.operatorApprovals"); bytes32 internal constant BASE_URI_POSITION = keccak256("lido.WithdrawalQueueERC721.baseUri"); - bytes32 internal constant NFT_DESCRIPTOR_ADDRESS_POSITION = keccak256("lido.WithdrawalQueueERC721.nftDescriptorAddress"); + bytes32 internal constant NFT_DESCRIPTOR_ADDRESS_POSITION = + keccak256("lido.WithdrawalQueueERC721.nftDescriptorAddress"); bytes32 public constant MANAGE_TOKEN_URI_ROLE = keccak256("MANAGE_TOKEN_URI_ROLE"); @@ -91,15 +89,16 @@ contract WithdrawalQueueERC721 is IERC721Metadata, WithdrawalQueue { returns (bool) { return interfaceId == type(IERC721).interfaceId || interfaceId == type(IERC721Metadata).interfaceId - || super.supportsInterface(interfaceId); + // 0x49064906 is magic number ERC4906 interfaceId as defined in the standard https://eips.ethereum.org/EIPS/eip-4906 + || interfaceId == bytes4(0x49064906) || super.supportsInterface(interfaceId); } - /// @dev Se_toBytes321Metadata-name}. + /// @dev See {IERC721Metadata-name}. function name() external view override returns (string memory) { return _toString(NAME); } - /// @dev Se_toBytes321Metadata-symbol}. + /// @dev See {IERC721Metadata-symbol}. function symbol() external view override returns (string memory) { return _toString(SYMBOL); } @@ -114,8 +113,7 @@ contract WithdrawalQueueERC721 is IERC721Metadata, WithdrawalQueue { if (nftDescriptorAddress != address(0)) { return INFTDescriptor(nftDescriptorAddress).constructTokenURI(_requestId); } else { - string memory baseURI = _getBaseURI().value; - return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, _requestId.toString())) : ""; + return _constructTokenUri(_requestId); } } @@ -125,7 +123,7 @@ contract WithdrawalQueueERC721 is IERC721Metadata, WithdrawalQueue { return _getBaseURI().value; } - /// @notice Sets the Base URI for computing {tokenURI} + /// @notice Sets the Base URI for computing {tokenURI}. It does not expect the ending slash in provided string. /// @dev If NFTDescriptor address isn't set the `baseURI` would be used for generating erc721 tokenURI. In case /// NFTDescriptor address is set it would be used as a first-priority method. function setBaseURI(string calldata _baseURI) external onlyRole(MANAGE_TOKEN_URI_ROLE) { @@ -146,6 +144,21 @@ contract WithdrawalQueueERC721 is IERC721Metadata, WithdrawalQueue { emit NftDescriptorAddressSet(_nftDescriptorAddress); } + /// @notice Finalize requests from last finalized one up to `_lastRequestIdToBeFinalized` + /// @dev ether to finalize all the requests should be calculated using `prefinalize()` and sent along + function finalize(uint256 _lastRequestIdToBeFinalized, uint256 _maxShareRate) external payable { + _checkResumed(); + _checkRole(FINALIZE_ROLE, msg.sender); + + uint256 firstFinalizedRequestId = getLastFinalizedRequestId() + 1; + + _finalize(_lastRequestIdToBeFinalized, msg.value, _maxShareRate); + + // ERC4906 metadata update event + // We are updating all unfinalized to make it look different as they move closer to finalization in the future + emit BatchMetadataUpdate(firstFinalizedRequestId, getLastRequestId()); + } + /// @dev See {IERC721-balanceOf}. function balanceOf(address _owner) external view override returns (uint256) { if (_owner == address(0)) revert InvalidOwnerAddress(_owner); @@ -156,7 +169,7 @@ contract WithdrawalQueueERC721 is IERC721Metadata, WithdrawalQueue { function ownerOf(uint256 _requestId) public view override returns (address) { if (_requestId == 0 || _requestId > getLastRequestId()) revert InvalidRequestId(_requestId); - WithdrawalRequest memory request = _getQueue()[_requestId]; + WithdrawalRequest storage request = _getQueue()[_requestId]; if (request.claimed) revert RequestAlreadyClaimed(_requestId); return request.owner; @@ -227,7 +240,9 @@ contract WithdrawalQueueERC721 is IERC721Metadata, WithdrawalQueue { address msgSender = msg.sender; if ( !(_from == msgSender || isApprovedForAll(_from, msgSender) || _getTokenApprovals()[_requestId] == msgSender) - ) revert NotOwnerOrApproved(msgSender); + ) { + revert NotOwnerOrApproved(msgSender); + } delete _getTokenApprovals()[_requestId]; request.owner = _to; @@ -338,4 +353,42 @@ contract WithdrawalQueueERC721 is IERC721Metadata, WithdrawalQueue { baseURI.slot := position } } + + function _constructTokenUri(uint256 _requestId) internal view returns (string memory) { + string memory baseURI = _getBaseURI().value; + if (bytes(baseURI).length == 0) return ""; + + // ${baseUri}/${_requestId}?requested=${amount}&created_at=${timestamp}[&finalized=${claimableAmount}] + string memory uri = string( + // we have no string.concat in 0.8.9 yet, so we have to do it with bytes.concat + bytes.concat( + bytes(baseURI), + bytes("/"), + bytes(_requestId.toString()), + bytes("?requested="), + bytes( + uint256(_getQueue()[_requestId].cumulativeStETH - _getQueue()[_requestId - 1].cumulativeStETH) + .toString() + ), + bytes("&created_at="), + bytes(uint256(_getQueue()[_requestId].timestamp).toString()) + ) + ); + bool finalized = _requestId <= getLastFinalizedRequestId(); + + if (finalized) { + uri = string( + bytes.concat( + bytes(uri), + bytes("&finalized="), + bytes( + _getClaimableEther(_requestId, _findCheckpointHint(_requestId, 1, getLastCheckpointIndex())) + .toString() + ) + ) + ); + } + + return uri; + } } diff --git a/contracts/0.8.9/interfaces/IERC4906.sol b/contracts/0.8.9/interfaces/IERC4906.sol new file mode 100644 index 000000000..506608ee8 --- /dev/null +++ b/contracts/0.8.9/interfaces/IERC4906.sol @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: 2023 OpenZeppelin, Lido +// SPDX-License-Identifier: MIT + +// Based on https://github.com/OpenZeppelin/openzeppelin-contracts/blob/96a2297e15f1a4bbcf470d2d0d6cb9c579c63893/contracts/interfaces/IERC4906.sol + +pragma solidity 0.8.9; + +import {IERC165} from "@openzeppelin/contracts-v4.4/utils/introspection/IERC165.sol"; +import {IERC721} from "@openzeppelin/contracts-v4.4/token/ERC721/IERC721.sol"; + +/// @title EIP-721 Metadata Update Extension +interface IERC4906 is IERC165, IERC721 { + /// @dev This event emits when the metadata of a token is changed. + /// So that the third-party platforms such as NFT market could + /// timely update the images and related attributes of the NFT. + event MetadataUpdate(uint256 _tokenId); + + /// @dev This event emits when the metadata of a range of tokens is changed. + /// So that the third-party platforms such as NFT market could + /// timely update the images and related attributes of the NFTs. + event BatchMetadataUpdate(uint256 _fromTokenId, uint256 _toTokenId); +} diff --git a/contracts/0.8.9/oracle/BaseOracle.sol b/contracts/0.8.9/oracle/BaseOracle.sol index 0cfbead9c..255c1fec5 100644 --- a/contracts/0.8.9/oracle/BaseOracle.sol +++ b/contracts/0.8.9/oracle/BaseOracle.sol @@ -50,6 +50,7 @@ abstract contract BaseOracle is IReportAsyncProcessor, AccessControlEnumerable, error UnexpectedConsensusVersion(uint256 expectedVersion, uint256 receivedVersion); error HashCannotBeZero(); error UnexpectedDataHash(bytes32 consensusHash, bytes32 receivedHash); + error SecondsPerSlotCannotBeZero(); event ConsensusHashContractSet(address indexed addr, address indexed prevAddr); event ConsensusVersionSet(uint256 indexed version, uint256 indexed prevVersion); @@ -100,6 +101,7 @@ abstract contract BaseOracle is IReportAsyncProcessor, AccessControlEnumerable, /// constructor(uint256 secondsPerSlot, uint256 genesisTime) { + if (secondsPerSlot == 0) revert SecondsPerSlotCannotBeZero(); SECONDS_PER_SLOT = secondsPerSlot; GENESIS_TIME = genesisTime; } diff --git a/contracts/0.8.9/sanity_checks/OracleReportSanityChecker.sol b/contracts/0.8.9/sanity_checks/OracleReportSanityChecker.sol index 8ebccb9f3..b147bc9b7 100644 --- a/contracts/0.8.9/sanity_checks/OracleReportSanityChecker.sol +++ b/contracts/0.8.9/sanity_checks/OracleReportSanityChecker.sol @@ -37,8 +37,10 @@ interface IWithdrawalQueue { /// @notice The set of restrictions used in the sanity checks of the oracle report /// @dev struct is loaded from the storage and stored in memory during the tx running struct LimitsList { - /// @notice The max possible number of validators that might appear or exit on the Consensus - /// Layer during one day + /// @notice The max possible number of validators that might been reported as `appeared` or `exited` + /// during a single day + /// NB: `appeared` means `pending` (maybe not `activated` yet), see further explanations + // in docs for the `setChurnValidatorsPerDayLimit` func below. /// @dev Must fit into uint16 (<= 65_535) uint256 churnValidatorsPerDayLimit; @@ -217,6 +219,15 @@ contract OracleReportSanityChecker is AccessControlEnumerable { } /// @notice Sets the new value for the churnValidatorsPerDayLimit + /// The limit is applicable for `appeared` and `exited` validators + /// + /// NB: AccountingOracle reports validators as `appeared` once them become `pending` + /// (might be not `activated` yet). Thus, this limit should be high enough for such cases + /// because Consensus Layer has no intrinsic churn limit for the amount of `pending` validators + /// (only for `activated` instead). For Lido it's limited by the max daily deposits via DepositSecurityModule + /// + /// In contrast, `exited` are reported according to the Consensus Layer churn limit. + /// /// @param _churnValidatorsPerDayLimit new churnValidatorsPerDayLimit value function setChurnValidatorsPerDayLimit(uint256 _churnValidatorsPerDayLimit) external @@ -426,7 +437,7 @@ contract OracleReportSanityChecker is AccessControlEnumerable { // 6. Appeared validators increase if (_postCLValidators > _preCLValidators) { - _checkValidatorsChurnLimit(limitsList, (_postCLValidators - _preCLValidators), _timeElapsed); + _checkAppearedValidatorsChurnLimit(limitsList, (_postCLValidators - _preCLValidators), _timeElapsed); } } @@ -589,7 +600,7 @@ contract OracleReportSanityChecker is AccessControlEnumerable { } } - function _checkValidatorsChurnLimit( + function _checkAppearedValidatorsChurnLimit( LimitsList memory _limitsList, uint256 _appearedValidators, uint256 _timeElapsed @@ -633,20 +644,37 @@ contract OracleReportSanityChecker is AccessControlEnumerable { revert ActualShareRateIsZero(); } - if (_simulatedShareRate > actualShareRate) { - // the simulated share rate can't be higher than the actual one - // invariant: rounding only can lower the simulated share rate - revert TooHighSimulatedShareRate(_simulatedShareRate, actualShareRate); - } - - uint256 simulatedShareDiff = actualShareRate - _simulatedShareRate; + // the simulated share rate can be either higher or lower than the actual one + // in case of new user-submitted ether & minted `stETH` between the oracle reference slot + // and the actual report delivery slot + // + // it happens because the oracle daemon snapshots rewards or losses at the reference slot, + // and then calculates simulated share rate, but if new ether was submitted together with minting new `stETH` + // after the reference slot passed, the oracle daemon still submits the same amount of rewards or losses, + // which now is applicable to more 'shareholders', lowering the impact per a single share + // (i.e, changing the actual share rate) + // + // simulated share rate ≤ actual share rate can be for a negative token rebase + // simulated share rate ≥ actual share rate can be for a positive token rebase + // + // Given that: + // 1) CL one-off balance decrease ≤ token rebase ≤ max positive token rebase + // 2) user-submitted ether & minted `stETH` don't exceed the current staking rate limit + // (see Lido.getCurrentStakeLimit()) + // + // can conclude that `simulatedShareRateDeviationBPLimit` (L) should be set as follows: + // L = (2 * SRL) * max(CLD, MPR), + // where: + // - CLD is consensus layer one-off balance decrease (as BP), + // - MPR is max positive token rebase (as BP), + // - SRL is staking rate limit normalized by TVL (`maxStakeLimit / totalPooledEther`) + // totalPooledEther should be chosen as a reasonable lower bound of the protocol TVL + // + uint256 simulatedShareDiff = Math256.absDiff(actualShareRate, _simulatedShareRate); uint256 simulatedShareDeviation = (MAX_BASIS_POINTS * simulatedShareDiff) / actualShareRate; if (simulatedShareDeviation > _limitsList.simulatedShareRateDeviationBPLimit) { - // the simulated share rate can be lower than the actual one due to rounding - // e.g., new user-submitted ether & minted `stETH` - // between an oracle reference slot and an actual accounting report delivery - revert TooLowSimulatedShareRate(_simulatedShareRate, actualShareRate); + revert IncorrectSimulatedShareRate(_simulatedShareRate, actualShareRate); } } @@ -724,8 +752,7 @@ contract OracleReportSanityChecker is AccessControlEnumerable { error IncorrectExitedValidators(uint256 churnLimit); error IncorrectRequestFinalization(uint256 requestCreationBlock); error ActualShareRateIsZero(); - error TooHighSimulatedShareRate(uint256 simulatedShareRate, uint256 actualShareRate); - error TooLowSimulatedShareRate(uint256 simulatedShareRate, uint256 actualShareRate); + error IncorrectSimulatedShareRate(uint256 simulatedShareRate, uint256 actualShareRate); error MaxAccountingExtraDataItemsCountExceeded(uint256 maxItemsCount, uint256 receivedItemsCount); error ExitedValidatorsLimitExceeded(uint256 limitPerDay, uint256 exitedPerDay); error TooManyNodeOpsPerExtraDataItem(uint256 itemIndex, uint256 nodeOpsCount); diff --git a/contracts/common/lib/ECDSA.sol b/contracts/common/lib/ECDSA.sol index 3ecac4e37..5e24a39e3 100644 --- a/contracts/common/lib/ECDSA.sol +++ b/contracts/common/lib/ECDSA.sol @@ -35,7 +35,6 @@ library ECDSA { // vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept // these malleable signatures as well. require(uint256(s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0, "ECDSA: invalid signature 's' value"); - require(v == 27 || v == 28, "ECDSA: invalid signature 'v' value"); // If the signature is valid (and not malleable), return the signer address address signer = ecrecover(hash, v, r, s); diff --git a/test/0.4.24/lido-handle-oracle-report.test.js b/test/0.4.24/lido-handle-oracle-report.test.js index 6ff1e8a65..f5cdfdffe 100644 --- a/test/0.4.24/lido-handle-oracle-report.test.js +++ b/test/0.4.24/lido-handle-oracle-report.test.js @@ -11,6 +11,7 @@ const { calcSharesMintedAsFees, calcShareRateDeltaE27, limitRebase, + addSendWithResult, } = require('../helpers/utils') const { deployProtocol } = require('../helpers/protocol') const { @@ -31,7 +32,7 @@ const ORACLE_REPORT_LIMITS_BOILERPLATE = { churnValidatorsPerDayLimit: 255, oneOffCLBalanceDecreaseBPLimit: 100, annualBalanceIncreaseBPLimit: 10000, - simulatedShareRateDeviationBPLimit: 1, + simulatedShareRateDeviationBPLimit: 15, maxValidatorExitRequestsPerReport: 10000, maxAccountingExtraDataListItemsCount: 10000, maxNodeOperatorsPerExtraDataItemCount: 10000, @@ -170,6 +171,8 @@ contract('Lido: handleOracleReport', ([appManager, , , , , , bob, stranger, anot snapshot = new EvmSnapshot(ethers.provider) await snapshot.make() + + addSendWithResult(lido.handleOracleReport) }) beforeEach(async () => { @@ -2023,7 +2026,7 @@ contract('Lido: handleOracleReport', ([appManager, , , , , , bob, stranger, anot }), { from: oracle, gasPrice: 1 } ), - `TooLowSimulatedShareRate(${tooLowSimulatedShareRate.toString()}, ${simulatedShareRate.toString()})` + `IncorrectSimulatedShareRate(${tooLowSimulatedShareRate.toString()}, ${simulatedShareRate.toString()})` ) const tooHighSimulatedShareRate = simulatedShareRate.mul(toBN(3)).div(toBN(2)) @@ -2043,7 +2046,7 @@ contract('Lido: handleOracleReport', ([appManager, , , , , , bob, stranger, anot }), { from: oracle, gasPrice: 1 } ), - `TooHighSimulatedShareRate(${tooHighSimulatedShareRate.toString()}, ${simulatedShareRate.toString()})` + `IncorrectSimulatedShareRate(${tooHighSimulatedShareRate.toString()}, ${simulatedShareRate.toString()})` ) await lido.handleOracleReport( @@ -2192,5 +2195,251 @@ contract('Lido: handleOracleReport', ([appManager, , , , , , bob, stranger, anot assert.equals(coverShares, toBN(0)) assert.equals(nonCoverShares, toBN(0)) }) + + it('simulatedShareRate is higher due to outstanding submits if token rebase is positive', async () => { + // Some EL rewards to report + await setBalance(elRewardsVault, ETH(1)) + + // Check that we haven't finalized anything yet + assert.equals(await withdrawalQueue.getLastFinalizedRequestId(), toBN(0)) + await withdrawalQueue.resume({ from: appManager }) + assert.isFalse(await withdrawalQueue.isPaused()) + + // Stranger decides to withdraw his stETH(10) + await lido.approve(withdrawalQueue.address, StETH(10), { from: stranger }) + await withdrawalQueue.requestWithdrawals([StETH(10)], stranger, { from: stranger }) + assert.equals(await withdrawalQueue.unfinalizedStETH(), StETH(10)) + assert.equals(await withdrawalQueue.unfinalizedRequestNumber(), 1) + + const maxPositiveTokenRebase = 1000000000 // Setting daily positive rebase as 100% + await oracleReportSanityChecker.setOracleReportLimits( + { + ...ORACLE_REPORT_LIMITS_BOILERPLATE, + churnValidatorsPerDayLimit: 100, + maxPositiveTokenRebase, + }, + { from: voting, gasPrice: 1 } + ) + + await advanceChainTime(30) + + // Performing dry-run to estimate simulated share rate + const [postTotalPooledEther, postTotalShares, withdrawals, elRewards] = await lido.handleOracleReport.call( + ...Object.values({ + ...DEFAULT_LIDO_ORACLE_REPORT, + timeElapsed: ONE_DAY, + clValidators: 3, + postCLBalance: ETH(96.1), + elRewardsVaultBalance: ETH(1), + }), + { from: oracle, gasPrice: 1 } + ) + const { elBalanceUpdate } = limitRebase( + toBN(maxPositiveTokenRebase), + ETH(101), + ETH(101), + ETH(0.1), + ETH(1), + StETH(0) + ) + assert.equals(withdrawals.add(elRewards), elBalanceUpdate) + // Ensuring that the EL vault didn't hit the positive rebase limit + assert.equals(await getBalance(elRewardsVault), elRewards) + const simulatedShareRate = postTotalPooledEther.mul(toBN(shareRate(1))).div(postTotalShares) + + await advanceChainTime(30) + + // Bob decides to stake rather massive amount in between reference slot and real report submission + await lido.submit(ZERO_ADDRESS, { from: bob, value: ETH(10) }) + + // Sending the real report with finalization attempts + const [realPostTotalPooledEther, realPostTotalShares] = await lido.handleOracleReport.sendWithResult( + ...Object.values({ + ...DEFAULT_LIDO_ORACLE_REPORT, + reportTimestamp: await getCurrentBlockTimestamp(), + timeElapsed: ONE_DAY, + clValidators: 3, + postCLBalance: ETH(96.1), + elRewardsVaultBalance: ETH(1), + withdrawalFinalizationBatches: [1], + simulatedShareRate: simulatedShareRate.toString(), + }), + { from: oracle, gasPrice: 1 } + ) + const realShareRate = realPostTotalPooledEther.mul(toBN(shareRate(1))).div(realPostTotalShares) + + // simulated share rate is greater than the really reported + assert.isTrue(simulatedShareRate.gt(realShareRate)) + + await checkStat({ depositedValidators: 3, beaconValidators: 3, beaconBalance: ETH(96.1) }) + + // Checking that both vaults are withdrawn + assert.equals(await getBalance(elRewardsVault), toBN(0)) + assert.equals(await getBalance(withdrawalVault), toBN(0)) + // Check total pooled ether + const totalPooledEtherAfterFinalization = await lido.getTotalPooledEther() + // Add Bob's recently staked funds, deduct finalized with 1:1 stranger's StETH(10) + assert.equals(totalPooledEtherAfterFinalization, postTotalPooledEther.add(toBN(ETH(10 - 10)))) + + // Checking that finalization of the previously placed withdrawal request completed + assert.equals(await withdrawalQueue.getLastFinalizedRequestId(), toBN(1)) + const strangerBalanceBeforeClaim = await getBalance(stranger) + await withdrawalQueue.claimWithdrawal(1, { from: stranger, gasPrice: 0 }) + const strangerBalanceAfterClaim = await getBalance(stranger) + // Happy-path: user receive ETH corresponding to the requested StETH amount + assert.equals(strangerBalanceAfterClaim - strangerBalanceBeforeClaim, StETH(10)) + }) + + it('simulatedShareRate is lower due to outstanding submits if token rebase is negative', async () => { + // Check that we haven't finalized anything yet + assert.equals(await withdrawalQueue.getLastFinalizedRequestId(), toBN(0)) + await withdrawalQueue.resume({ from: appManager }) + assert.isFalse(await withdrawalQueue.isPaused()) + + // Stranger decides to withdraw his stETH(10) + await lido.approve(withdrawalQueue.address, StETH(10), { from: stranger }) + await withdrawalQueue.requestWithdrawals([StETH(10)], stranger, { from: stranger }) + assert.equals(await withdrawalQueue.unfinalizedStETH(), StETH(10)) + assert.equals(await withdrawalQueue.unfinalizedRequestNumber(), 1) + + await advanceChainTime(30) + + // Performing dry-run to estimate simulated share rate + const [postTotalPooledEther, postTotalShares] = await lido.handleOracleReport.call( + ...Object.values({ + ...DEFAULT_LIDO_ORACLE_REPORT, + timeElapsed: ONE_DAY, + clValidators: 3, + postCLBalance: ETH(95), // CL rebase is negative (was 96 ETH before the report) + }), + { from: oracle, gasPrice: 1 } + ) + const simulatedShareRate = postTotalPooledEther.mul(toBN(shareRate(1))).div(postTotalShares) + + await advanceChainTime(30) + + // Bob decides to stake rather massive amount in between reference slot and real report submission + await lido.submit(ZERO_ADDRESS, { from: bob, value: ETH(10) }) + + // Sending the real report with finalization attempts + const [realPostTotalPooledEther, realPostTotalShares] = await lido.handleOracleReport.sendWithResult( + ...Object.values({ + ...DEFAULT_LIDO_ORACLE_REPORT, + reportTimestamp: await getCurrentBlockTimestamp(), + timeElapsed: ONE_DAY, + clValidators: 3, + postCLBalance: ETH(95), + withdrawalFinalizationBatches: [1], + simulatedShareRate: simulatedShareRate.toString(), + }), + { from: oracle, gasPrice: 1 } + ) + const realShareRate = realPostTotalPooledEther.mul(toBN(shareRate(1))).div(realPostTotalShares) + + // simulated share rate is lower than the really reported + assert.isTrue(simulatedShareRate.lt(realShareRate)) + + await checkStat({ depositedValidators: 3, beaconValidators: 3, beaconBalance: ETH(95) }) + + // Check total pooled ether + const totalPooledEtherAfterFinalization = await lido.getTotalPooledEther() + // Add Bob's recently staked funds, deduct finalized with 9:10 stranger's StETH(10) + assert.equals(totalPooledEtherAfterFinalization, postTotalPooledEther.add(toBN(ETH(0.1)))) + + // Checking that finalization of the previously placed withdrawal request completed + assert.equals(await withdrawalQueue.getLastFinalizedRequestId(), toBN(1)) + const strangerBalanceBeforeClaim = await getBalance(stranger) + await withdrawalQueue.claimWithdrawal(1, { from: stranger, gasPrice: 0 }) + const strangerBalanceAfterClaim = await getBalance(stranger) + // Losses-path: user receive ETH lower than the requested StETH amount + assert.equals(strangerBalanceAfterClaim - strangerBalanceBeforeClaim, StETH(9.9)) + }) + }) + + describe('100% of rewards receive staking module & treasury', () => { + beforeEach(async () => { + await lido.deposit(3, 1, '0x', { from: depositor }) + await checkStat({ depositedValidators: 3, beaconValidators: 0, beaconBalance: 0 }) + await checkBalanceDeltas({ + totalPooledEtherDiff: 0, + treasuryBalanceDiff: 0, + strangerBalanceDiff: 0, + anotherStrangerBalanceDiff: 0, + curatedModuleBalanceDiff: 0, + initialHolderBalanceDiff: 0, + }) + const curatedModuleId = 1 + await deployed.stakingRouter.updateStakingModule(curatedModuleId, 100_00, 50_00, 50_00, { from: voting }) + const curatedModuleStats = await deployed.stakingRouter.getStakingModule(curatedModuleId) + assert.equals(curatedModuleStats.stakingModuleFee, 50_00) + assert.equals(curatedModuleStats.treasuryFee, 50_00) + }) + + it('oracle report handled correctly', async () => { + // set annualBalanceIncreaseBPLimit = 1% + await oracleReportSanityChecker.setOracleReportLimits( + { + ...ORACLE_REPORT_LIMITS_BOILERPLATE, + annualBalanceIncreaseBPLimit: 100, + }, + { from: voting } + ) + + await lido.handleOracleReport( + ...Object.values({ ...DEFAULT_LIDO_ORACLE_REPORT, clValidators: 3, postCLBalance: ETH(96) }), + { from: oracle } + ) + await checkStat({ depositedValidators: 3, beaconValidators: 3, beaconBalance: ETH(96) }) + await checkBalanceDeltas({ + totalPooledEtherDiff: 0, + treasuryBalanceDiff: 0, + strangerBalanceDiff: 0, + anotherStrangerBalanceDiff: 0, + curatedModuleBalanceDiff: 0, + initialHolderBalanceDiff: 0, + }) + const tx = await lido.handleOracleReport( + ...Object.values({ + ...DEFAULT_LIDO_ORACLE_REPORT, + timeElapsed: ONE_YEAR, + clValidators: 3, + postCLBalance: ETH(96.96), + }), + { from: oracle } + ) + const sharesMintedAsFees = calcSharesMintedAsFees( + ETH(0.96), // rewards + 100, // fee + 100, // feePoints + ETH(100), // prevTotalShares + ETH(100.96) // newTotalEther + ) + await checkEvents({ + tx, + preCLValidators: 3, + postCLValidators: 3, + preCLBalance: ETH(96), + postCLBalance: ETH(96.96), + withdrawalsWithdrawn: 0, + executionLayerRewardsWithdrawn: 0, + postBufferedEther: ETH(4), + timeElapsed: ONE_YEAR, + preTotalShares: ETH(100), + preTotalEther: ETH(100), + postTotalShares: toBN(ETH(100)).add(sharesMintedAsFees).toString(), + postTotalEther: ETH(100.96), + sharesMintedAsFees: sharesMintedAsFees.toString(), + }) + await checkStat({ depositedValidators: 3, beaconValidators: 3, beaconBalance: ETH(96.96) }) + + await checkBalanceDeltas({ + strangerBalanceDiff: ETH(0), + initialHolderBalanceDiff: ETH(0), + anotherStrangerBalanceDiff: ETH(0), + totalPooledEtherDiff: ETH(0.96), + treasuryBalanceDiff: ETH(0.96 * 0.5), + curatedModuleBalanceDiff: ETH(0.96 * 0.5), + }) + }) }) }) diff --git a/test/0.4.24/node-operators-registry.test.js b/test/0.4.24/node-operators-registry.test.js index 639f52eeb..088bccd7a 100644 --- a/test/0.4.24/node-operators-registry.test.js +++ b/test/0.4.24/node-operators-registry.test.js @@ -392,6 +392,16 @@ contract('NodeOperatorsRegistry', (addresses) => { await assert.reverts(app.addNodeOperator(name, ZERO_ADDRESS, { from: nodeOperatorsManager }), 'ZERO_ADDRESS') }) + it('reverts with error "LIDO_REWARD_ADDRESS" when called with lido as reward address', async () => { + const hasPermission = await dao.hasPermission(nodeOperatorsManager, app, 'MANAGE_NODE_OPERATOR_ROLE') + assert.isTrue(hasPermission) + const name = 'Node Operator #1' + await assert.reverts( + app.addNodeOperator(name, steth.address, { from: nodeOperatorsManager }), + 'LIDO_REWARD_ADDRESS' + ) + }) + it('reverts with error "MAX_COUNT_EXCEEDED" when total count of node operators = MAX_COUNT_EXCEEDED', async () => { const hasPermission = await dao.hasPermission(nodeOperatorsManager, app, 'MANAGE_NODE_OPERATOR_ROLE') assert.isTrue(hasPermission) @@ -889,6 +899,13 @@ contract('NodeOperatorsRegistry', (addresses) => { ) }) + it('reverts with error "LIDO_REWARD_ADDRESS" when new reward address is lido', async () => { + await assert.reverts( + app.setNodeOperatorRewardAddress(firstNodeOperatorId, steth.address, { from: nodeOperatorsManager }), + 'LIDO_REWARD_ADDRESS' + ) + }) + it(`reverts with "APP_AUTH_FAILED" error when caller doesn't have MANAGE_NODE_OPERATOR_ROLE`, async () => { const hasPermission = await dao.hasPermission(nobody, app, 'MANAGE_NODE_OPERATOR_ROLE') assert.isFalse(hasPermission) diff --git a/test/0.4.24/steth.test.js b/test/0.4.24/steth.test.js index 4cd0ca01e..23f5482b3 100644 --- a/test/0.4.24/steth.test.js +++ b/test/0.4.24/steth.test.js @@ -2,9 +2,9 @@ const { artifacts, contract, ethers } = require('hardhat') const { assert } = require('../helpers/assert') const { bn } = require('@aragon/contract-helpers-test') -const { tokens, ETH } = require('./../helpers/utils') +const { tokens, ETH, shares } = require('./../helpers/utils') const { EvmSnapshot } = require('../helpers/blockchain') -const { INITIAL_HOLDER, ZERO_ADDRESS } = require('../helpers/constants') +const { INITIAL_HOLDER, ZERO_ADDRESS, MAX_UINT256 } = require('../helpers/constants') const StETHMock = artifacts.require('StETHMock') @@ -187,7 +187,7 @@ contract('StETH', ([_, __, user1, user2, user3, nobody]) => { it('reverts when sender is zero address', async () => { await assert.reverts( stEth.transferFrom(ZERO_ADDRESS, user3, tokens(0), { from: user2 }), - 'TRANSFER_FROM_ZERO_ADDR' + 'APPROVE_FROM_ZERO_ADDR' ) }) @@ -213,7 +213,27 @@ contract('StETH', ([_, __, user1, user2, user3, nobody]) => { to: user3, sharesValue: sharesAmount, }) - assert.equals(await stEth.allowance(user2, user1), bn(0)) + assert.equals(await stEth.allowance(user1, user2), bn(0)) + assert.equals(await stEth.balanceOf(user1), tokens(49)) + assert.equals(await stEth.balanceOf(user3), tokens(50)) + }) + + it("doesn't spent allowance if it was set to MAX_UINT256", async () => { + await stEth.approve(user2, MAX_UINT256, { from: user1 }) + assert.equals(await stEth.allowance(user1, user2), bn(MAX_UINT256)) + const amount = tokens(50) + const receipt = await stEth.transferFrom(user1, user3, amount, { from: user2 }) + assert.emitsNumberOfEvents(receipt, 'Transfer', 1) + assert.emitsNumberOfEvents(receipt, 'TransferShares', 1) + assert.emitsNumberOfEvents(receipt, 'Approval', 0) + assert.emits(receipt, 'Transfer', { from: user1, to: user3, value: amount }) + const sharesAmount = await stEth.getSharesByPooledEth(amount) + assert.emits(receipt, 'TransferShares', { + from: user1, + to: user3, + sharesValue: sharesAmount, + }) + assert.equals(await stEth.allowance(user1, user2), bn(MAX_UINT256)) assert.equals(await stEth.balanceOf(user1), tokens(49)) assert.equals(await stEth.balanceOf(user3), tokens(50)) }) @@ -660,6 +680,26 @@ contract('StETH', ([_, __, user1, user2, user3, nobody]) => { assert.equals(await stEth.balanceOf(user1), tokens(0)) assert.equals(await stEth.balanceOf(nobody), '118800000000000000000') }) + + it("transferSharesFrom doesn't spent allowance if it was set to MAX_UINT256", async () => { + await stEth.approve(user2, MAX_UINT256, { from: user1 }) + assert.equals(await stEth.allowance(user1, user2), bn(MAX_UINT256)) + const sharesAmount = shares(50) + const tokensAmount = await stEth.getPooledEthByShares(sharesAmount) + const receipt = await stEth.transferSharesFrom(user1, user3, sharesAmount, { from: user2 }) + assert.emitsNumberOfEvents(receipt, 'Transfer', 1) + assert.emitsNumberOfEvents(receipt, 'TransferShares', 1) + assert.emitsNumberOfEvents(receipt, 'Approval', 0) + assert.emits(receipt, 'Transfer', { from: user1, to: user3, value: tokensAmount }) + assert.emits(receipt, 'TransferShares', { + from: user1, + to: user3, + sharesValue: sharesAmount, + }) + assert.equals(await stEth.allowance(user1, user2), bn(MAX_UINT256)) + assert.equals(await stEth.balanceOf(user1), tokens(49)) + assert.equals(await stEth.balanceOf(user3), tokens(50)) + }) }) }) }) diff --git a/test/0.8.9/burner.test.js b/test/0.8.9/burner.test.js index 88dcc6f6a..4e3957cc5 100644 --- a/test/0.8.9/burner.test.js +++ b/test/0.8.9/burner.test.js @@ -66,44 +66,6 @@ contract('Burner', ([deployer, _, anotherAccount]) => { await burner.requestBurnMyStETHForCover(StETH(1), { from: anotherAccount }) }) - it(`RECOVER_ASSETS_ROLE works`, async () => { - const nft1 = bn(666) - const totalERC20Supply = bn(1000000) - const mockERC20Token = await ERC20OZMock.new(totalERC20Supply, { from: deployer }) - const mockNFT = await ERC721OZMock.new({ from: deployer }) - await mockNFT.mintToken(nft1, { from: deployer }) - await web3.eth.sendTransaction({ from: anotherAccount, to: lido.address, value: ETH(2) }) - - await mockERC20Token.transfer(burner.address, bn(600000), { from: deployer }) - await mockNFT.transferFrom(deployer, burner.address, nft1, { from: deployer }) - await lido.transfer(burner.address, StETH(1), { from: anotherAccount }) - - assert.isFalse(await burner.hasRole(await burner.RECOVER_ASSETS_ROLE(), anotherAccount)) - - await assert.revertsOZAccessControl( - burner.recoverERC20(mockERC20Token.address, bn(600000), { from: anotherAccount }), - anotherAccount, - `RECOVER_ASSETS_ROLE` - ) - await assert.revertsOZAccessControl( - burner.recoverERC721(mockNFT.address, nft1, { from: anotherAccount }), - anotherAccount, - `RECOVER_ASSETS_ROLE` - ) - await assert.revertsOZAccessControl( - burner.recoverExcessStETH({ from: anotherAccount }), - anotherAccount, - `RECOVER_ASSETS_ROLE` - ) - - await burner.grantRole(await burner.RECOVER_ASSETS_ROLE(), anotherAccount, { from: appManager }) - assert.isTrue(await burner.hasRole(await burner.RECOVER_ASSETS_ROLE(), anotherAccount)) - - await burner.recoverERC20(mockERC20Token.address, bn(600000), { from: anotherAccount }) - await burner.recoverERC721(mockNFT.address, nft1, { from: anotherAccount }) - await burner.recoverExcessStETH({ from: anotherAccount }) - }) - it(`REQUEST_BURN_SHARES_ROLE works`, async () => { await web3.eth.sendTransaction({ from: anotherAccount, to: lido.address, value: ETH(2) }) await lido.approve(burner.address, StETH(2), { from: anotherAccount }) @@ -687,7 +649,7 @@ contract('Burner', ([deployer, _, anotherAccount]) => { assert.equals(await lido.balanceOf(burner.address), StETH(7.1)) // should change nothing - const receipt = await burner.recoverExcessStETH({ from: voting }) + const receipt = await burner.recoverExcessStETH({ from: anotherAccount }) assert.emitsNumberOfEvents(receipt, `ExcessStETHRecovered`, 0) // excess stETH amount didn't changed @@ -707,9 +669,9 @@ contract('Burner', ([deployer, _, anotherAccount]) => { assert.equals(await lido.balanceOf(treasury), StETH(0)) const sharesAmount2_3StETH = await lido.sharesOf(burner.address) - const receipt = await burner.recoverExcessStETH({ from: voting }) + const receipt = await burner.recoverExcessStETH({ from: anotherAccount }) assert.emits(receipt, `ExcessStETHRecovered`, { - requestedBy: voting, + requestedBy: anotherAccount, amountOfStETH: StETH(2.3), amountOfShares: sharesAmount2_3StETH, }) @@ -754,9 +716,9 @@ contract('Burner', ([deployer, _, anotherAccount]) => { // run recovery process, excess stETH amount (5) // should be transferred to the treasury const sharesAmount5stETH = await lido.getSharesByPooledEth(StETH(5)) - const receipt = await burner.recoverExcessStETH({ from: voting }) + const receipt = await burner.recoverExcessStETH({ from: anotherAccount }) assert.emits(receipt, `ExcessStETHRecovered`, { - requestedBy: voting, + requestedBy: anotherAccount, amountOfStETH: StETH(5), amountOfShares: sharesAmount5stETH, }) @@ -807,7 +769,7 @@ contract('Burner', ([deployer, _, anotherAccount]) => { }) it(`can't recover zero-address ERC20`, async () => { - await assert.reverts(burner.recoverERC20(ZERO_ADDRESS, bn(10), { from: voting })) + await assert.reverts(burner.recoverERC20(ZERO_ADDRESS, bn(10), { from: anotherAccount })) }) it(`can't recover stETH by recoverERC20`, async () => { @@ -826,17 +788,9 @@ contract('Burner', ([deployer, _, anotherAccount]) => { // revert from anotherAccount // need to use recoverExcessStETH await assert.revertsWithCustomError( - burner.recoverERC20(lido.address, StETH(1), { from: voting }), + burner.recoverERC20(lido.address, StETH(1), { from: anotherAccount }), `StETHRecoveryWrongFunc()` ) - - // revert from deployer - // acl - await assert.revertsOZAccessControl( - burner.recoverERC20(lido.address, StETH(1), { from: deployer }), - deployer, - `RECOVER_ASSETS_ROLE` - ) }) it(`recover some accidentally sent ERC20`, async () => { @@ -849,9 +803,9 @@ contract('Burner', ([deployer, _, anotherAccount]) => { assert.equals(await mockERC20Token.balanceOf(burner.address), bn(600000)) // recover ERC20 - const firstReceipt = await burner.recoverERC20(mockERC20Token.address, bn(100000), { from: voting }) + const firstReceipt = await burner.recoverERC20(mockERC20Token.address, bn(100000), { from: anotherAccount }) assert.emits(firstReceipt, `ERC20Recovered`, { - requestedBy: voting, + requestedBy: anotherAccount, token: mockERC20Token.address, amount: bn(100000), }) @@ -861,24 +815,17 @@ contract('Burner', ([deployer, _, anotherAccount]) => { assert.equals(await mockERC20Token.balanceOf(treasury), bn(100000)) assert.equals(await mockERC20Token.balanceOf(voting), bn(0)) - // acl error - await assert.revertsOZAccessControl( - burner.recoverERC20(mockERC20Token.address, bn(1), { from: anotherAccount }), - anotherAccount, - `RECOVER_ASSETS_ROLE` - ) - // recover last portion - const lastReceipt = await burner.recoverERC20(mockERC20Token.address, bn(500000), { from: voting }) + const lastReceipt = await burner.recoverERC20(mockERC20Token.address, bn(500000), { from: anotherAccount }) assert.emits(lastReceipt, `ERC20Recovered`, { - requestedBy: voting, + requestedBy: anotherAccount, token: mockERC20Token.address, amount: bn(500000), }) // balance is zero already, have to be reverted await assert.reverts( - burner.recoverERC20(mockERC20Token.address, bn(1), { from: voting }), + burner.recoverERC20(mockERC20Token.address, bn(1), { from: anotherAccount }), `ERC20: transfer amount exceeds balance` ) }) @@ -908,30 +855,20 @@ contract('Burner', ([deployer, _, anotherAccount]) => { assert.equals(await burner.getExcessStETH(), StETH(1)) // can't abuse recoverERC721 API to perform griefing-like attack - await assert.revertsOZAccessControl( - burner.recoverERC721(lido.address, StETH(1), { from: anotherAccount }), - anotherAccount, - `RECOVER_ASSETS_ROLE` - ) - await assert.revertsOZAccessControl( - burner.recoverERC721(lido.address, StETH(1), { from: deployer }), - deployer, - `RECOVER_ASSETS_ROLE` - ) await assert.revertsWithCustomError( - burner.recoverERC721(lido.address, StETH(1), { from: voting }), + burner.recoverERC721(lido.address, StETH(1), { from: anotherAccount }), `StETHRecoveryWrongFunc()` ) - const receipt = await burner.recoverExcessStETH({ from: voting }) - assert.emits(receipt, `ExcessStETHRecovered`, { requestedBy: voting, amountOfStETH: StETH(1) }) + const receipt = await burner.recoverExcessStETH({ from: anotherAccount }) + assert.emits(receipt, `ExcessStETHRecovered`, { requestedBy: anotherAccount, amountOfStETH: StETH(1) }) // ensure that excess amount is zero assert.equals(await burner.getExcessStETH(), StETH(0)) }) it(`can't recover zero-address ERC721(NFT)`, async () => { - await assert.reverts(burner.recoverERC721(ZERO_ADDRESS, 0, { from: voting })) + await assert.reverts(burner.recoverERC721(ZERO_ADDRESS, 0, { from: anotherAccount })) }) it(`recover some accidentally sent NFTs`, async () => { @@ -944,13 +881,6 @@ contract('Burner', ([deployer, _, anotherAccount]) => { assert.equals(await mockNFT.balanceOf(anotherAccount), bn(1)) assert.equals(await mockNFT.balanceOf(burner.address), bn(1)) - // access control revert - await assert.revertsOZAccessControl( - burner.recoverERC721(mockNFT.address, nft2, { from: anotherAccount }), - anotherAccount, - `RECOVER_ASSETS_ROLE` - ) - // recover nft2 should work const receiptNfc2 = await burner.recoverERC721(mockNFT.address, nft2, { from: voting }) assert.emits(receiptNfc2, `ERC721Recovered`, { requestedBy: voting, token: mockNFT.address, tokenId: nft2 }) diff --git a/test/0.8.9/oracle-daemon-config.test.js b/test/0.8.9/oracle-daemon-config.test.js index bae27a78c..5105b29ef 100644 --- a/test/0.8.9/oracle-daemon-config.test.js +++ b/test/0.8.9/oracle-daemon-config.test.js @@ -102,20 +102,28 @@ contract('OracleDaemonConfig', async ([deployer, manager, stranger]) => { ) }) - it('revers when empty value passed to set', async () => { + it('reverts when empty value passed to set', async () => { await assert.revertsWithCustomError( config.set(defaultKey, '0x', { from: manager }), `EmptyValue("${defaultKey}")` ) }) - it('revers when empty value passed to update', async () => { + it('reverts when empty value passed to update', async () => { await config.set(defaultKey, defaultValue, { from: manager }) await assert.revertsWithCustomError( config.update(defaultKey, '0x', { from: manager }), `EmptyValue("${defaultKey}")` ) }) + + it('reverts when set key with the same value', async () => { + await config.set(defaultKey, defaultValue, { from: manager }) + await assert.revertsWithCustomError( + config.update(defaultKey, defaultValue, { from: manager }), + `ValueIsSame("${defaultKey}", "${defaultValue}")` + ) + }) }) describe('access control', async () => { diff --git a/test/0.8.9/oracle-report-sanity-checker.test.js b/test/0.8.9/oracle-report-sanity-checker.test.js index 2be6b557b..27ac1e304 100644 --- a/test/0.8.9/oracle-report-sanity-checker.test.js +++ b/test/0.8.9/oracle-report-sanity-checker.test.js @@ -428,7 +428,7 @@ contract('OracleReportSanityChecker', ([deployer, admin, withdrawalVault, elRewa simulatedShareRate: (BigInt(2) * 10n ** 27n).toString(), } - it('reverts with error TooHighSimulatedShareRate() when reported and onchain share rate differs', async () => { + it('reverts with error IncorrectSimulatedShareRate() when simulated share rate is higher than expected', async () => { const simulatedShareRate = BigInt(ETH(2.1)) * 10n ** 9n const actualShareRate = BigInt(2) * 10n ** 27n await assert.reverts( @@ -438,11 +438,11 @@ contract('OracleReportSanityChecker', ([deployer, admin, withdrawalVault, elRewa simulatedShareRate: simulatedShareRate.toString(), }) ), - `TooHighSimulatedShareRate(${simulatedShareRate.toString()}, ${actualShareRate.toString()})` + `IncorrectSimulatedShareRate(${simulatedShareRate.toString()}, ${actualShareRate.toString()})` ) }) - it('reverts with error TooLowSimulatedShareRate() when reported and onchain share rate differs', async () => { + it('reverts with error IncorrectSimulatedShareRate() when simulated share rate is lower than expected', async () => { const simulatedShareRate = BigInt(ETH(1.9)) * 10n ** 9n const actualShareRate = BigInt(2) * 10n ** 27n await assert.reverts( @@ -452,7 +452,7 @@ contract('OracleReportSanityChecker', ([deployer, admin, withdrawalVault, elRewa simulatedShareRate: simulatedShareRate.toString(), }) ), - `TooLowSimulatedShareRate(${simulatedShareRate.toString()}, ${actualShareRate.toString()})` + `IncorrectSimulatedShareRate(${simulatedShareRate.toString()}, ${actualShareRate.toString()})` ) }) diff --git a/test/0.8.9/oracle/accounting-oracle-deploy.test.js b/test/0.8.9/oracle/accounting-oracle-deploy.test.js index 4e603cf3b..fad53e429 100644 --- a/test/0.8.9/oracle/accounting-oracle-deploy.test.js +++ b/test/0.8.9/oracle/accounting-oracle-deploy.test.js @@ -376,6 +376,10 @@ contract('AccountingOracle', ([admin, member1]) => { await snapshot.rollback() }) + it('reverts when slotsPerSecond is zero', async () => { + await assert.reverts(deployAccountingOracleSetup(admin, { secondsPerSlot: 0 }), 'SecondsPerSlotCannotBeZero()') + }) + it('deployment and init finishes successfully otherwise', async () => { const deployed = await deployAccountingOracleSetup(admin) diff --git a/test/0.8.9/oracle/base-oracle-deploy.test.js b/test/0.8.9/oracle/base-oracle-deploy.test.js index 126bd7bb7..9efb99776 100644 --- a/test/0.8.9/oracle/base-oracle-deploy.test.js +++ b/test/0.8.9/oracle/base-oracle-deploy.test.js @@ -1,5 +1,6 @@ const { contract, artifacts } = require('hardhat') const { bn } = require('@aragon/contract-helpers-test') +const { assert } = require('../../helpers/assert') const BaseOracle = artifacts.require('BaseOracleTimeTravellable') const MockConsensusContract = artifacts.require('MockConsensusContract') @@ -106,6 +107,10 @@ contract('BaseOracle', ([admin]) => { await deployBaseOracle(admin) }) + it('reverts when slotsPerSecond is zero', async () => { + await assert.reverts(deployBaseOracle(admin, { secondsPerSlot: 0 }), 'SecondsPerSlotCannotBeZero()') + }) + // TODO: add more base tests }) }) diff --git a/test/0.8.9/oracle/validators-exit-bus-oracle-deploy.test.js b/test/0.8.9/oracle/validators-exit-bus-oracle-deploy.test.js index b69b28e8f..0ca15d0aa 100644 --- a/test/0.8.9/oracle/validators-exit-bus-oracle-deploy.test.js +++ b/test/0.8.9/oracle/validators-exit-bus-oracle-deploy.test.js @@ -94,11 +94,11 @@ async function deployOracleReportSanityCheckerForExitBus(lidoLocator, admin) { async function deployExitBusOracle( admin, - { dataSubmitter = null, lastProcessingRefSlot = 0, resumeAfterDeploy = false } = {} + { dataSubmitter = null, lastProcessingRefSlot = 0, resumeAfterDeploy = false, secondsPerSlot = SECONDS_PER_SLOT } = {} ) { const locator = (await deployLocatorWithDummyAddressesImplementation(admin)).address - const oracle = await ValidatorsExitBusOracle.new(SECONDS_PER_SLOT, GENESIS_TIME, locator, { from: admin }) + const oracle = await ValidatorsExitBusOracle.new(secondsPerSlot, GENESIS_TIME, locator, { from: admin }) const { consensus } = await deployHashConsensus(admin, { epochsPerFrame: EPOCHS_PER_FRAME, @@ -159,6 +159,10 @@ contract('ValidatorsExitBusOracle', ([admin, member1]) => { oracle = deployed.oracle }) + it('reverts when slotsPerSecond is zero', async () => { + await assert.reverts(deployExitBusOracle(admin, { secondsPerSlot: 0 }), 'SecondsPerSlotCannotBeZero()') + }) + it('mock time-travellable setup is correct', async () => { const time1 = +(await consensus.getTime()) assert.equals(await oracle.getTime(), time1) diff --git a/test/0.8.9/staking-router/rewards-distribution.test.js b/test/0.8.9/staking-router/rewards-distribution.test.js index 230013c53..b54212f56 100644 --- a/test/0.8.9/staking-router/rewards-distribution.test.js +++ b/test/0.8.9/staking-router/rewards-distribution.test.js @@ -103,8 +103,9 @@ contract('StakingRouter', ([deployer, admin, depositor, stranger]) => { ) }) - it('getStakingRewardsDistribution() - reverts if total fee >= 100%', async () => { - await assert.reverts(router.getStakingRewardsDistribution(), 'ValueOver100Percent("totalFee")') + it("getStakingRewardsDistribution() - doesn't reverts if total fee = 100%", async () => { + const { totalFee } = await router.getStakingRewardsDistribution() + await assert.equals(totalFee, await router.FEE_PRECISION_POINTS()) }) it('update module - set fee and treasury fee', async () => { diff --git a/test/0.8.9/withdrawal-nft-gas.test.js b/test/0.8.9/withdrawal-nft-gas.test.js new file mode 100644 index 000000000..972e69869 --- /dev/null +++ b/test/0.8.9/withdrawal-nft-gas.test.js @@ -0,0 +1,64 @@ +const { contract, web3 } = require('hardhat') + +const { ETH, StETH, shareRate, shares } = require('../helpers/utils') +const { assert } = require('../helpers/assert') + +const { deployWithdrawalQueue } = require('./withdrawal-queue-deploy.test') + +contract('WithdrawalQueue', ([owner, daoAgent, user, tokenUriManager]) => { + let withdrawalQueue + + before('deploy', async function () { + if (!process.env.REPORT_GAS) { + this.skip() + } + const deployed = await deployWithdrawalQueue({ + stethOwner: owner, + queueAdmin: daoAgent, + queuePauser: daoAgent, + queueResumer: daoAgent, + queueFinalizer: daoAgent, + }) + + const steth = deployed.steth + withdrawalQueue = deployed.withdrawalQueue + await withdrawalQueue.grantRole(web3.utils.keccak256('MANAGE_TOKEN_URI_ROLE'), tokenUriManager, { from: daoAgent }) + await withdrawalQueue.setBaseURI('http://example.com', { from: tokenUriManager }) + + await steth.setTotalPooledEther(ETH(600)) + await steth.mintShares(user, shares(1)) + await steth.approve(withdrawalQueue.address, StETH(300), { from: user }) + }) + + it('findCheckpointHints gas spendings', async () => { + // checkpoints is created daily, so 2048 is enough for 6 years at least + const maxCheckpontSize = 2048 + + let size = 1 + while (size <= maxCheckpontSize) { + await setUpCheckpointsUpTo(size) + + console.log( + 'findCheckpointHints([1], 1, checkpointsSize): Gas spent:', + await withdrawalQueue.findCheckpointHints.estimateGas([1], 1, size), + 'tokenURI(1): Gas spent:', + await withdrawalQueue.tokenURI.estimateGas(1), + 'checkpoints size: ', + size + ) + size = size * 2 + } + }).timeout(0) + + async function setUpCheckpointsUpTo(n) { + for (let i = await withdrawalQueue.getLastCheckpointIndex(); i < n; i++) { + await withdrawalQueue.requestWithdrawals([StETH(0.00001)], user, { from: user }) + await withdrawalQueue.finalize([await withdrawalQueue.getLastRequestId()], shareRate(300), { + from: daoAgent, + value: ETH(0.00001), + }) + } + + assert.equals(await withdrawalQueue.getLastCheckpointIndex(), n, 'last checkpoint index') + } +}) diff --git a/test/0.8.9/withdrawal-queue-deploy.test.js b/test/0.8.9/withdrawal-queue-deploy.test.js index 075d93ed3..5e4d7b17c 100644 --- a/test/0.8.9/withdrawal-queue-deploy.test.js +++ b/test/0.8.9/withdrawal-queue-deploy.test.js @@ -12,7 +12,7 @@ const NFTDescriptorMock = artifacts.require('NFTDescriptorMock.sol') const QUEUE_NAME = 'Unsteth nft' const QUEUE_SYMBOL = 'UNSTETH' -const NFT_DESCRIPTOR_BASE_URI = 'https://exampleDescriptor.com/' +const NFT_DESCRIPTOR_BASE_URI = 'https://exampleDescriptor.com' async function deployWithdrawalQueue({ stethOwner, diff --git a/test/0.8.9/withdrawal-queue-gas.test.js b/test/0.8.9/withdrawal-queue-gas.test.js index 9700e153d..239de091e 100644 --- a/test/0.8.9/withdrawal-queue-gas.test.js +++ b/test/0.8.9/withdrawal-queue-gas.test.js @@ -126,7 +126,7 @@ contract('WithdrawalQueue', ([owner, user]) => { }) const finalization_args = [ - [batchEnd], + batchEnd, slash ? aboveShareRate : belowShareRate, { from: owner, value: prefinalize_res.ethToLock, gasPrice: gasPrice++ }, ] diff --git a/test/0.8.9/withdrawal-queue-nft.test.js b/test/0.8.9/withdrawal-queue-nft.test.js index c9a2f627a..e1028be23 100644 --- a/test/0.8.9/withdrawal-queue-nft.test.js +++ b/test/0.8.9/withdrawal-queue-nft.test.js @@ -58,14 +58,14 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, assert.equals(await withdrawalQueue.getLockedEtherAmount(), ETH(0)) }) - context('constructor', function () { + context('constructor', () => { it('should set name and symbol', async function () { assert.equals(await withdrawalQueue.name(), QUEUE_NAME) assert.equals(await withdrawalQueue.symbol(), QUEUE_SYMBOL) }) }) - context('supportsInterface', async () => { + context('supportsInterface', () => { it('supports ERC165', async () => { assert.isTrue(await withdrawalQueue.supportsInterface('0x01ffc9a7')) }) @@ -83,29 +83,65 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, }) }) - context('name', async () => { + context('name', () => { it('returns name', async () => { assert.equals(await withdrawalQueue.name(), QUEUE_NAME) }) }) - context('symbol', async () => { + context('symbol', () => { it('returns symbol', async () => { assert.equals(await withdrawalQueue.symbol(), QUEUE_SYMBOL) }) }) - context('tokenURI', async () => { + context('tokenURI', () => { const requestId = 1 - const baseTokenUri = 'https://example.com/' + const baseTokenUri = 'https://example.com' beforeEach(async function () { await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) }) it('returns tokenURI without nftDescriptor', async () => { - await withdrawalQueue.setBaseURI(baseTokenUri, { from: tokenUriManager }) - assert.equals(await withdrawalQueue.tokenURI(1), `${baseTokenUri}${requestId}`) + const tx = await withdrawalQueue.setBaseURI(baseTokenUri, { from: tokenUriManager }) + assert.emits(tx, 'BaseURISet', { baseURI: baseTokenUri }) + + assert.equals( + await withdrawalQueue.tokenURI(1), + `${baseTokenUri}/${requestId}?requested=${ETH(25)}&created_at=${ + (await withdrawalQueue.getWithdrawalStatus([1]))[0].timestamp + }` + ) + }) + + it('correct tokenURI after finalization', async () => { + const tx = await withdrawalQueue.setBaseURI(baseTokenUri, { from: tokenUriManager }) + assert.emits(tx, 'BaseURISet', { baseURI: baseTokenUri }) + + await withdrawalQueue.finalize(1, shareRate(300), { from: daoAgent, value: ETH(25) }) + + assert.equals( + await withdrawalQueue.tokenURI(1), + `${baseTokenUri}/${requestId}?requested=${ETH(25)}&created_at=${ + (await withdrawalQueue.getWithdrawalStatus([1]))[0].timestamp + }&finalized=${(await withdrawalQueue.getClaimableEther([1], [1]))[0]}` + ) + }) + + it('correct tokenURI after finalization with discount', async () => { + const tx = await withdrawalQueue.setBaseURI(baseTokenUri, { from: tokenUriManager }) + assert.emits(tx, 'BaseURISet', { baseURI: baseTokenUri }) + + const batch = await withdrawalQueue.prefinalize([1], shareRate(1)) + await withdrawalQueue.finalize(1, shareRate(1), { from: daoAgent, value: batch.ethToLock }) + + assert.equals( + await withdrawalQueue.tokenURI(1), + `${baseTokenUri}/${requestId}?requested=${ETH(25)}&created_at=${ + (await withdrawalQueue.getWithdrawalStatus([1]))[0].timestamp + }&finalized=${batch.sharesToBurn}` + ) }) it('returns tokenURI without nftDescriptor and baseUri', async () => { @@ -113,8 +149,8 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, }) it('returns tokenURI with nftDescriptor', async () => { - await withdrawalQueue.setNFTDescriptorAddress(nftDescriptor.address, { from: tokenUriManager }) - + const tx = await withdrawalQueue.setNFTDescriptorAddress(nftDescriptor.address, { from: tokenUriManager }) + assert.emits(tx, 'NftDescriptorAddressSet', { nftDescriptorAddress: nftDescriptor.address }) assert.equals(await withdrawalQueue.tokenURI(1), `${NFT_DESCRIPTOR_BASE_URI}${requestId}`) }) @@ -123,12 +159,14 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, }) it('should set baseURI and return', async () => { - await withdrawalQueue.setBaseURI(baseTokenUri, { from: tokenUriManager }) + const tx = await withdrawalQueue.setBaseURI(baseTokenUri, { from: tokenUriManager }) + assert.emits(tx, 'BaseURISet', { baseURI: baseTokenUri }) assert.equals(await withdrawalQueue.getBaseURI(), baseTokenUri) }) it('should set nftDescriptorAddress and return', async () => { - await withdrawalQueue.setNFTDescriptorAddress(nftDescriptor.address, { from: tokenUriManager }) + const tx = await withdrawalQueue.setNFTDescriptorAddress(nftDescriptor.address, { from: tokenUriManager }) + assert.emits(tx, 'NftDescriptorAddressSet', { nftDescriptorAddress: nftDescriptor.address }) assert.equals(await withdrawalQueue.getNFTDescriptorAddress(), nftDescriptor.address) }) }) @@ -153,7 +191,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, assert.equals(await withdrawalQueue.balanceOf(user), 1) const batch = await withdrawalQueue.prefinalize([1], shareRate(1)) - await withdrawalQueue.finalize([1], shareRate(1), { from: daoAgent, value: batch.ethToLock }) + await withdrawalQueue.finalize(1, shareRate(1), { from: daoAgent, value: batch.ethToLock }) await withdrawalQueue.claimWithdrawal(1, { from: user }) assert.equals(await withdrawalQueue.balanceOf(user), 0) @@ -182,14 +220,14 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, assert.equals(await withdrawalQueue.ownerOf(1), user) const batch = await withdrawalQueue.prefinalize([1], shareRate(1)) - await withdrawalQueue.finalize([1], shareRate(1), { from: daoAgent, value: batch.ethToLock }) + await withdrawalQueue.finalize(1, shareRate(1), { from: daoAgent, value: batch.ethToLock }) await withdrawalQueue.claimWithdrawal(1, { from: user }) await assert.reverts(withdrawalQueue.ownerOf(1), 'RequestAlreadyClaimed(1)') }) }) - context('approve()', async () => { + context('approve()', () => { let tokenId1 beforeEach(async () => { await snapshot.rollback() @@ -218,6 +256,24 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, await withdrawalQueue.transferFrom(user, recipient, tokenId1, { from: recipient }) assert.equals(await withdrawalQueue.ownerOf(tokenId1), recipient) }) + + it('approval can be revoked', async () => { + const tx1 = await withdrawalQueue.approve(recipient, tokenId1, { from: user }) + assert.equal(await withdrawalQueue.getApproved(tokenId1), recipient) + + assert.emits(tx1, 'Approval', { owner: user, approved: recipient, tokenId: tokenId1 }) + + const tx2 = await withdrawalQueue.approve(ZERO_ADDRESS, tokenId1, { from: user }) + assert.equal(await withdrawalQueue.getApproved(tokenId1), ZERO_ADDRESS) + + assert.emits(tx2, 'Approval', { owner: user, approved: ZERO_ADDRESS, tokenId: tokenId1 }) + + await assert.reverts( + withdrawalQueue.transferFrom(user, recipient, tokenId1, { from: recipient }), + 'NotOwnerOrApproved', + [`"${recipient}"`] + ) + }) }) context('getApproved', () => { @@ -232,12 +288,13 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, it('should return approved address', async () => { await withdrawalQueue.requestWithdrawals([ETH(25)], user, { from: user }) - await withdrawalQueue.approve(stranger, 1, { from: user }) + const tx = await withdrawalQueue.approve(stranger, 1, { from: user }) + assert.emits(tx, 'Approval', { owner: user, approved: stranger, tokenId: 1 }) assert.equals(await withdrawalQueue.getApproved(1), stranger) }) }) - context('setApprovalForAll()', async () => { + context('setApprovalForAll()', () => { let tokenId1, tokenId2 beforeEach(async () => { await snapshot.rollback() @@ -264,6 +321,22 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, assert.equals(await withdrawalQueue.ownerOf(tokenId1), recipient) assert.equals(await withdrawalQueue.ownerOf(tokenId2), recipient) }) + + it('approvalForAll can be revoked', async () => { + const tx1 = await withdrawalQueue.setApprovalForAll(recipient, true, { from: user }) + assert.emits(tx1, 'ApprovalForAll', { owner: user, operator: recipient, approved: true }) + assert.isTrue(await withdrawalQueue.isApprovedForAll(user, recipient)) + + const tx2 = await withdrawalQueue.setApprovalForAll(recipient, false, { from: user }) + assert.emits(tx2, 'ApprovalForAll', { owner: user, operator: recipient, approved: false }) + assert.isFalse(await withdrawalQueue.isApprovedForAll(user, recipient)) + + await assert.reverts( + withdrawalQueue.transferFrom(user, recipient, tokenId2, { from: recipient }), + 'NotOwnerOrApproved', + [`"${recipient}"`] + ) + }) }) context('isApprovedForAll', () => { @@ -277,13 +350,23 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, }) }) - context('safeTransferFrom(address,address,uint256)', async () => { + context('safeTransferFrom(address,address,uint256)', () => { let requestIds beforeEach(async () => { requestIds = await withdrawalQueue.requestWithdrawals.call([ETH(25), ETH(25)], user, { from: user, }) - await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + const tx = await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + assert.emits(tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: requestIds[0], + }) + assert.emits(tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: requestIds[1], + }) }) it('reverts with message "NotOwnerOrApproved()" when approvalNotSet and not owner', async () => { @@ -297,18 +380,30 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, it('transfers if called by owner', async () => { assert.notEqual(await withdrawalQueue.ownerOf(requestIds[0]), recipient) - await withdrawalQueue.safeTransferFrom(user, recipient, requestIds[0], { + const tx = await withdrawalQueue.safeTransferFrom(user, recipient, requestIds[0], { from: user, }) + assert.emits(tx, 'Transfer', { + from: user, + to: recipient, + tokenId: requestIds[0], + }) assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), recipient) }) it('transfers if token approval set', async () => { - await withdrawalQueue.approve(recipient, requestIds[0], { from: user }) + const approve_tx = await withdrawalQueue.approve(recipient, requestIds[0], { from: user }) + assert.emits(approve_tx, 'Approval', { owner: user, approved: recipient, tokenId: requestIds[0] }) + assert.notEqual(await withdrawalQueue.ownerOf(requestIds[0]), recipient) - await withdrawalQueue.safeTransferFrom(user, recipient, requestIds[0], { + const tx = await withdrawalQueue.safeTransferFrom(user, recipient, requestIds[0], { from: recipient, }) + assert.emits(tx, 'Transfer', { + from: user, + to: recipient, + tokenId: requestIds[0], + }) assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), recipient) }) @@ -316,12 +411,22 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, await withdrawalQueue.setApprovalForAll(recipient, true, { from: user }) assert.notEqual(await withdrawalQueue.ownerOf(requestIds[0]), recipient) assert.notEqual(await withdrawalQueue.ownerOf(requestIds[1]), recipient) - await withdrawalQueue.safeTransferFrom(user, recipient, requestIds[0], { + const tx1 = await withdrawalQueue.safeTransferFrom(user, recipient, requestIds[0], { from: recipient, }) - await withdrawalQueue.safeTransferFrom(user, recipient, requestIds[1], { + assert.emits(tx1, 'Transfer', { + from: user, + to: recipient, + tokenId: requestIds[0], + }) + const tx2 = await withdrawalQueue.safeTransferFrom(user, recipient, requestIds[1], { from: recipient, }) + assert.emits(tx2, 'Transfer', { + from: user, + to: recipient, + tokenId: requestIds[1], + }) assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), recipient) assert.equal(await withdrawalQueue.ownerOf(requestIds[1]), recipient) }) @@ -348,21 +453,37 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, it("doesn't revert when recipient contract implements ERC721Receiver interface and accepts tokens", async () => { await erc721ReceiverMock.setDoesAcceptTokens(true, { from: owner }) assert.notEqual(await withdrawalQueue.ownerOf(requestIds[0]), erc721ReceiverMock.address) - await withdrawalQueue.safeTransferFrom(user, erc721ReceiverMock.address, requestIds[0], { + const tx = await withdrawalQueue.safeTransferFrom(user, erc721ReceiverMock.address, requestIds[0], { from: user, }) + assert.emits(tx, 'Transfer', { + from: user, + to: erc721ReceiverMock.address, + tokenId: requestIds[0], + }) + assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), erc721ReceiverMock.address) }) }) - describe('transferFrom()', async () => { + describe('transferFrom()', () => { let requestIds beforeEach(async () => { requestIds = await withdrawalQueue.requestWithdrawals.call([ETH(25), ETH(25)], user, { from: user, }) - await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + const tx = await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + assert.emits(tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: requestIds[0], + }) + assert.emits(tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: requestIds[1], + }) }) it('reverts with message "NotOwnerOrApproved()" when approvalNotSet and not owner', async () => { @@ -383,7 +504,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, it('reverts with error "RequestAlreadyClaimed()" when called on claimed request', async () => { const batch = await withdrawalQueue.prefinalize([2], shareRate(1)) - await withdrawalQueue.finalize([2], shareRate(1), { from: daoAgent, value: batch.ethToLock }) + await withdrawalQueue.finalize(2, shareRate(1), { from: daoAgent, value: batch.ethToLock }) await withdrawalQueue.methods['claimWithdrawal(uint256)'](requestIds[0], { from: user, @@ -399,18 +520,29 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, it('transfers if called by owner', async () => { assert.notEqual(await withdrawalQueue.ownerOf(requestIds[0]), recipient) - await withdrawalQueue.transferFrom(user, recipient, requestIds[0], { + const tx = await withdrawalQueue.transferFrom(user, recipient, requestIds[0], { + from: user, + }) + assert.emits(tx, 'Transfer', { from: user, + to: recipient, + tokenId: requestIds[0], }) assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), recipient) }) it('transfers if token approval set', async () => { await withdrawalQueue.approve(recipient, requestIds[0], { from: user }) + assert.notEqual(await withdrawalQueue.ownerOf(requestIds[0]), recipient) - await withdrawalQueue.transferFrom(user, recipient, requestIds[0], { + const tx = await withdrawalQueue.transferFrom(user, recipient, requestIds[0], { from: recipient, }) + assert.emits(tx, 'Transfer', { + from: user, + to: recipient, + tokenId: requestIds[0], + }) assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), recipient) }) @@ -418,43 +550,80 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, await withdrawalQueue.setApprovalForAll(recipient, true, { from: user }) assert.notEqual(await withdrawalQueue.ownerOf(requestIds[0]), recipient) assert.notEqual(await withdrawalQueue.ownerOf(requestIds[1]), recipient) - await withdrawalQueue.transferFrom(user, recipient, requestIds[0], { + const tx1 = await withdrawalQueue.transferFrom(user, recipient, requestIds[0], { from: recipient, }) - await withdrawalQueue.transferFrom(user, recipient, requestIds[1], { + assert.emits(tx1, 'Transfer', { + from: user, + to: recipient, + tokenId: requestIds[0], + }) + + const tx2 = await withdrawalQueue.transferFrom(user, recipient, requestIds[1], { from: recipient, }) + + assert.emits(tx2, 'Transfer', { + from: user, + to: recipient, + tokenId: requestIds[1], + }) + assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), recipient) assert.equal(await withdrawalQueue.ownerOf(requestIds[1]), recipient) }) it('can claim request after transfer', async () => { - await withdrawalQueue.transferFrom(user, recipient, requestIds[0], { + const tx = await withdrawalQueue.transferFrom(user, recipient, requestIds[0], { + from: user, + }) + assert.emits(tx, 'Transfer', { from: user, + to: recipient, + tokenId: requestIds[0], }) assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), recipient) const batch = await withdrawalQueue.prefinalize([2], shareRate(1)) - await withdrawalQueue.finalize([2], shareRate(1), { from: daoAgent, value: batch.ethToLock }) + await withdrawalQueue.finalize(2, shareRate(1), { from: daoAgent, value: batch.ethToLock }) - await withdrawalQueue.methods['claimWithdrawal(uint256)'](requestIds[0], { + const claim_tx = await withdrawalQueue.methods['claimWithdrawal(uint256)'](requestIds[0], { from: recipient, }) + assert.emits(claim_tx, 'Transfer', { + from: recipient, + to: ZERO_ADDRESS, + tokenId: requestIds[0], + }) }) it("doesn't reverts when transfer to contract that not implements IERC721Receiver interface", async () => { assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), user) - await withdrawalQueue.transferFrom(user, steth.address, requestIds[0], { + const tx = await withdrawalQueue.transferFrom(user, steth.address, requestIds[0], { from: user, }) + assert.emits(tx, 'Transfer', { + from: user, + to: steth.address, + tokenId: requestIds[0], + }) assert.equal(await withdrawalQueue.ownerOf(requestIds[0]), steth.address) }) }) - context('mint', async () => { + context('mint', () => { it('should mint', async () => { - await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) - + const tx = await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + assert.emits(tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 1, + }) + assert.emits(tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 2, + }) assert.equals(await withdrawalQueue.balanceOf(user), 2) assert.equals(await withdrawalQueue.ownerOf(1), user) assert.equals(await withdrawalQueue.tokenURI(1), '') @@ -462,15 +631,30 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, it('should mint with tokenURI', async () => { await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) - await withdrawalQueue.setBaseURI('https://example.com/', { from: tokenUriManager }) + await withdrawalQueue.setBaseURI('https://example.com', { from: tokenUriManager }) assert.equals(await withdrawalQueue.balanceOf(user), 2) assert.equals(await withdrawalQueue.ownerOf(1), user) - assert.equals(await withdrawalQueue.tokenURI(1), 'https://example.com/1') + assert.equals( + await withdrawalQueue.tokenURI(1), + `https://example.com/1?requested=25000000000000000000&created_at=${ + (await withdrawalQueue.getWithdrawalStatus([1]))[0].timestamp + }` + ) }) it('should mint with nftDescriptor', async () => { - await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + const tx = await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + assert.emits(tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 1, + }) + assert.emits(tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 2, + }) await withdrawalQueue.setNFTDescriptorAddress(nftDescriptor.address, { from: tokenUriManager }) nftDescriptor.setBaseTokenURI('https://nftDescriptor.com/') @@ -480,20 +664,39 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, }) it('should mint more after request', async () => { - await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + const tx1 = await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + assert.emits(tx1, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 1, + }) + assert.emits(tx1, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 2, + }) assert.equals(await withdrawalQueue.balanceOf(user), 2) assert.equals(await withdrawalQueue.ownerOf(1), user) assert.equals(await withdrawalQueue.ownerOf(2), user) - await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) - + const tx2 = await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + assert.emits(tx2, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 3, + }) + assert.emits(tx2, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 4, + }) assert.equals(await withdrawalQueue.balanceOf(user), 4) assert.equals(await withdrawalQueue.ownerOf(3), user) assert.equals(await withdrawalQueue.ownerOf(4), user) }) }) - context('burn', async () => { + context('burn', () => { it('should burn', async () => { await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) @@ -502,8 +705,14 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, assert.equals(await withdrawalQueue.ownerOf(2), user) const batch = await withdrawalQueue.prefinalize.call([1], shareRate(1)) - await withdrawalQueue.finalize([1], shareRate(1), { from: daoAgent, value: batch.ethToLock }) - await withdrawalQueue.claimWithdrawal(1, { from: user }) + await withdrawalQueue.finalize(1, shareRate(1), { from: daoAgent, value: batch.ethToLock }) + const tx = await withdrawalQueue.claimWithdrawal(1, { from: user }) + + assert.emits(tx, 'Transfer', { + from: user, + to: ZERO_ADDRESS, + tokenId: 1, + }) assert.equals(await withdrawalQueue.balanceOf(user), 1) assert.equals(await withdrawalQueue.ownerOf(2), user) @@ -518,7 +727,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, assert.equals(await withdrawalQueue.ownerOf(2), user) const batch = await withdrawalQueue.prefinalize.call([1], shareRate(1)) - await withdrawalQueue.finalize([1], shareRate(1), { from: daoAgent, value: batch.ethToLock }) + await withdrawalQueue.finalize(1, shareRate(1), { from: daoAgent, value: batch.ethToLock }) await assert.reverts(withdrawalQueue.claimWithdrawal(1, { from: stranger }), `NotOwner("${stranger}", "${user}")`) @@ -529,7 +738,6 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, it('revert on claim not existing', async () => { await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) - await assert.reverts(withdrawalQueue.claimWithdrawal(1, { from: user }), 'RequestNotFoundOrNotFinalized(1)') }) @@ -540,33 +748,66 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, assert.equals(await withdrawalQueue.ownerOf(2), user) const batch = await withdrawalQueue.prefinalize.call([2], shareRate(1)) - await withdrawalQueue.finalize([2], shareRate(1), { from: daoAgent, value: batch.ethToLock }) - await withdrawalQueue.claimWithdrawal(1, { from: user }) + await withdrawalQueue.finalize(2, shareRate(1), { from: daoAgent, value: batch.ethToLock }) + const tx1 = await withdrawalQueue.claimWithdrawal(1, { from: user }) + + assert.emits(tx1, 'Transfer', { + from: user, + to: ZERO_ADDRESS, + tokenId: 1, + }) assert.equals(await withdrawalQueue.balanceOf(user), 1) assert.equals(await withdrawalQueue.ownerOf(2), user) await assert.reverts(withdrawalQueue.ownerOf(1), 'RequestAlreadyClaimed(1)') - await withdrawalQueue.claimWithdrawal(2, { from: user }) + const tx2 = await withdrawalQueue.claimWithdrawal(2, { from: user }) + + assert.emits(tx2, 'Transfer', { + from: user, + to: ZERO_ADDRESS, + tokenId: 2, + }) assert.equals(await withdrawalQueue.balanceOf(user), 0) }) it('should burn after transfer', async () => { - await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + const mint_tx = await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25)], user, { from: user }) + assert.emits(mint_tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 1, + }) + assert.emits(mint_tx, 'Transfer', { + from: ZERO_ADDRESS, + to: user, + tokenId: 2, + }) + assert.equals(await withdrawalQueue.balanceOf(user), 2) assert.equals(await withdrawalQueue.ownerOf(1), user) assert.equals(await withdrawalQueue.ownerOf(2), user) - await withdrawalQueue.transferFrom(user, stranger, 1, { from: user }) + const tx1 = await withdrawalQueue.transferFrom(user, stranger, 1, { from: user }) + assert.emits(tx1, 'Transfer', { + from: user, + to: stranger, + tokenId: 1, + }) assert.equals(await withdrawalQueue.balanceOf(user), 1) assert.equals(await withdrawalQueue.ownerOf(2), user) assert.equals(await withdrawalQueue.ownerOf(1), stranger) const batch = await withdrawalQueue.prefinalize.call([2], shareRate(1)) - await withdrawalQueue.finalize([2], shareRate(1), { from: daoAgent, value: batch.ethToLock }) - await withdrawalQueue.claimWithdrawal(2, { from: user }) + await withdrawalQueue.finalize(2, shareRate(1), { from: daoAgent, value: batch.ethToLock }) + const tx2 = await withdrawalQueue.claimWithdrawal(2, { from: user }) + assert.emits(tx2, 'Transfer', { + from: user, + to: ZERO_ADDRESS, + tokenId: 2, + }) assert.equals(await withdrawalQueue.balanceOf(user), 0) }) @@ -593,7 +834,9 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, }) it('should burn after approve and transfer ', async () => { - await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25), ETH(25)], user, { from: user }) + const mint_tx = await withdrawalQueue.requestWithdrawals([ETH(25), ETH(25), ETH(25)], user, { from: user }) + assert.emitsNumberOfEvents(mint_tx, 'Transfer', 3) + assert.equals(await withdrawalQueue.balanceOf(user), 3) assert.equals(await withdrawalQueue.ownerOf(1), user) assert.equals(await withdrawalQueue.ownerOf(2), user) @@ -607,7 +850,8 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, assert.equals(await withdrawalQueue.ownerOf(2), user) assert.equals(await withdrawalQueue.ownerOf(3), user) - await withdrawalQueue.transferFrom(user, stranger, 3, { from: stranger }) + const transfer_tx1 = await withdrawalQueue.transferFrom(user, stranger, 3, { from: stranger }) + assert.emitsNumberOfEvents(transfer_tx1, 'Transfer', 1) assert.equals(await withdrawalQueue.balanceOf(user), 2) assert.equals(await withdrawalQueue.balanceOf(stranger), 1) @@ -616,9 +860,12 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, tokenUriManager, assert.equals(await withdrawalQueue.ownerOf(3), stranger) const batch = await withdrawalQueue.prefinalize.call([3], shareRate(1)) - await withdrawalQueue.finalize([3], shareRate(1), { from: daoAgent, value: batch.ethToLock }) - await withdrawalQueue.claimWithdrawal(1, { from: user }) - await withdrawalQueue.claimWithdrawal(3, { from: stranger }) + await withdrawalQueue.finalize(3, shareRate(1), { from: daoAgent, value: batch.ethToLock }) + const claim_tx1 = await withdrawalQueue.claimWithdrawal(1, { from: user }) + assert.emitsNumberOfEvents(claim_tx1, 'Transfer', 1) + + const claim_tx2 = await withdrawalQueue.claimWithdrawal(3, { from: stranger }) + assert.emitsNumberOfEvents(claim_tx2, 'Transfer', 1) assert.equals(await withdrawalQueue.balanceOf(user), 1) assert.equals(await withdrawalQueue.balanceOf(stranger), 0) diff --git a/test/0.8.9/withdrawal-queue-requests-finalization.test.js b/test/0.8.9/withdrawal-queue-requests-finalization.test.js index 88f523a87..9f6737314 100644 --- a/test/0.8.9/withdrawal-queue-requests-finalization.test.js +++ b/test/0.8.9/withdrawal-queue-requests-finalization.test.js @@ -37,11 +37,23 @@ contract('WithdrawalQueue', ([owner, daoAgent, user, anotherUser]) => { assert.equalsDelta(batch.ethToLock, budget, 2) - await withdrawalQueue.finalize(batches, finalizationShareRate, { + const fromRequest = +(await withdrawalQueue.getLastFinalizedRequestId()) + 1 + + const tx = await withdrawalQueue.finalize(batches[batches.length - 1], finalizationShareRate, { from: daoAgent, value: batch.ethToLock, }) + const timestamp = (await ethers.provider.getBlock(tx.receipt.blockNumber)).timestamp + + assert.emits(tx, 'WithdrawalsFinalized', { + to: batches[batches.length - 1], + from: fromRequest, + amountOfETHLocked: batch.ethToLock, + sharesToBurn: batch.sharesToBurn, + timestamp, + }) + return { batch } } @@ -222,10 +234,10 @@ contract('WithdrawalQueue', ([owner, daoAgent, user, anotherUser]) => { }) context('2 users, 1 batch', () => { - ;[0.7].forEach(async (firstRequestRate) => { - ;[0.4, 0.7, 1].forEach(async (secondRequestRate) => { + ;[0.7].forEach((firstRequestRate) => { + ;[0.4, 0.7, 1].forEach((secondRequestRate) => { ;[firstRequestRate, secondRequestRate, secondRequestRate - 0.1, secondRequestRate + 0.1].forEach( - async (finalizationRate) => { + (finalizationRate) => { ;[ firstRequestRate, firstRequestRate - 0.1, @@ -234,7 +246,7 @@ contract('WithdrawalQueue', ([owner, daoAgent, user, anotherUser]) => { finalizationRate, finalizationRate - 0.1, finalizationRate + 0.1, - ].forEach(async (postFinalizationRate) => { + ].forEach((postFinalizationRate) => { it(`rates: first request = ${firstRequestRate}, second request = ${secondRequestRate}, finalization = ${finalizationRate}, claim = ${postFinalizationRate}`, async () => { await setShareRate(firstRequestRate) const userRequestAmount = e18(1) @@ -326,12 +338,12 @@ contract('WithdrawalQueue', ([owner, daoAgent, user, anotherUser]) => { }) context('2 users, 2 batch', () => { - ;[0.7].forEach(async (firstRequestRate) => { - ;[0.4, 0.7, 1].forEach(async (secondRequestRate) => { + ;[0.7].forEach((firstRequestRate) => { + ;[0.4, 0.7, 1].forEach((secondRequestRate) => { ;[firstRequestRate, secondRequestRate, secondRequestRate - 0.1, secondRequestRate + 0.1].forEach( - async (firstFinalizationRate) => { + (firstFinalizationRate) => { ;[firstRequestRate, secondRequestRate, secondRequestRate - 0.1, secondRequestRate + 0.1].forEach( - async (secondFinalizationRate) => { + (secondFinalizationRate) => { ;[ firstRequestRate, firstRequestRate - 0.1, @@ -341,7 +353,7 @@ contract('WithdrawalQueue', ([owner, daoAgent, user, anotherUser]) => { firstFinalizationRate - 0.1, firstFinalizationRate + 0.1, secondFinalizationRate, - ].forEach(async (postFinalizationRate) => { + ].forEach((postFinalizationRate) => { it(`rates: first request = ${firstRequestRate}, second request = ${secondRequestRate}, secondFinalizationRate = ${secondFinalizationRate}, firstFinalization = ${firstFinalizationRate}, claim = ${postFinalizationRate}`, async () => { await setShareRate(firstRequestRate) const userRequestAmount = e18(1) diff --git a/test/0.8.9/withdrawal-queue-share-rate-changes.test.js b/test/0.8.9/withdrawal-queue-share-rate-changes.test.js index 606c64e07..562433f2c 100644 --- a/test/0.8.9/withdrawal-queue-share-rate-changes.test.js +++ b/test/0.8.9/withdrawal-queue-share-rate-changes.test.js @@ -44,7 +44,7 @@ contract('WithdrawalQueue', ([owner, daoAgent, finalizer, user, oracle]) => { await snapshot() }) - context(`2 requests with diff share rate, maxShareRate == shareRate(1)`, async () => { + context(`2 requests with diff share rate, maxShareRate == shareRate(1)`, () => { /// /// invariant 1: all requests in the same batch should be finalized using the same share rate /// @@ -98,7 +98,7 @@ contract('WithdrawalQueue', ([owner, daoAgent, finalizer, user, oracle]) => { let claimableEther it(`requests get finalized`, async () => { - await queue.finalize(batches, e27(1), { from: finalizer, value: e18(2) }) + await queue.finalize(batches[batches.length - 1], e27(1), { from: finalizer, value: e18(2) }) assert.equals(await queue.getLastFinalizedRequestId(), requestIds[1]) const hints = await queue.findCheckpointHints(requestIds, 1, await queue.getLastCheckpointIndex()) @@ -114,7 +114,7 @@ contract('WithdrawalQueue', ([owner, daoAgent, finalizer, user, oracle]) => { }) }) - context(`2 requests 2 batches, shareRate(2) > maxShareRate > shareRate(1)`, async () => { + context(`2 requests 2 batches, shareRate(2) > maxShareRate > shareRate(1)`, () => { /// /// invariant 1: all requests in the same batch should be finalized using the same share rate /// @@ -169,7 +169,7 @@ contract('WithdrawalQueue', ([owner, daoAgent, finalizer, user, oracle]) => { let claimableEther it(`requests get finalized`, async () => { - await queue.finalize(batches, maxShareRate, { from: finalizer, value: e18(2.5) }) + await queue.finalize(batches[batches.length - 1], maxShareRate, { from: finalizer, value: e18(2.5) }) assert.equals(await queue.getLastFinalizedRequestId(), requestIds[1]) const hints = await queue.findCheckpointHints(requestIds, 1, await queue.getLastCheckpointIndex()) diff --git a/test/0.8.9/withdrawal-queue.test.js b/test/0.8.9/withdrawal-queue.test.js index 80c09532b..72ec19673 100644 --- a/test/0.8.9/withdrawal-queue.test.js +++ b/test/0.8.9/withdrawal-queue.test.js @@ -141,7 +141,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, withdrawalQueue.requestWithdrawalsWithPermit([ETH(1)], owner, stETHPermission, { from: alice.address }), 'ResumedExpected()' ) - await assert.reverts(withdrawalQueue.finalize([1], 0, { from: owner }), 'ResumedExpected()') + await assert.reverts(withdrawalQueue.finalize(1, 0, { from: owner }), 'ResumedExpected()') }) it('cant resume without resume role', async () => { @@ -375,12 +375,8 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, }) it('Finalizer can finalize a request', async () => { - await assert.revertsOZAccessControl( - withdrawalQueue.finalize([1], 0, { from: stranger }), - stranger, - 'FINALIZE_ROLE' - ) - await withdrawalQueue.finalize([1], 1, { from: steth.address, value: amount }) + await assert.revertsOZAccessControl(withdrawalQueue.finalize(1, 0, { from: stranger }), stranger, 'FINALIZE_ROLE') + await withdrawalQueue.finalize(1, 1, { from: steth.address, value: amount }) assert.equals(await withdrawalQueue.getLockedEtherAmount(), amount) assert.equals( @@ -390,7 +386,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, }) it('One can finalize requests with discount', async () => { - await withdrawalQueue.finalize([1], shareRate(150), { from: steth.address, value: ETH(150) }) + await withdrawalQueue.finalize(1, shareRate(150), { from: steth.address, value: ETH(150) }) assert.equals(await withdrawalQueue.getLockedEtherAmount(), ETH(150)) assert.equals( @@ -406,7 +402,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, await withdrawalQueue.requestWithdrawals([amount], owner, { from: user }) const batch = await withdrawalQueue.prefinalize.call([2], defaultShareRate) - await withdrawalQueue.finalize([2], defaultShareRate, { from: steth.address, value: batch.ethToLock }) + await withdrawalQueue.finalize(2, defaultShareRate, { from: steth.address, value: batch.ethToLock }) assert.equals(batch.sharesToBurn, shares(2)) assert.equals(await withdrawalQueue.getLastRequestId(), 2) @@ -425,7 +421,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, await withdrawalQueue.requestWithdrawals([amount], owner, { from: user }) - await withdrawalQueue.finalize([1], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(1, defaultShareRate, { from: steth.address, value: amount }) assert.equals(await withdrawalQueue.getLastRequestId(), 2) assert.equals(await withdrawalQueue.getLastFinalizedRequestId(), 1) @@ -435,7 +431,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, await ethers.provider.getBalance(withdrawalQueue.address) ) - await withdrawalQueue.finalize([2], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(2, defaultShareRate, { from: steth.address, value: amount }) assert.equals(await withdrawalQueue.getLastRequestId(), 2) assert.equals(await withdrawalQueue.getLastFinalizedRequestId(), 2) @@ -459,19 +455,11 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, await assert.reverts(withdrawalQueue.prefinalize([2, 1], shareRate(1)), 'BatchesAreNotSorted()') }) - it('reverts if batches are empty', async () => { - await assert.reverts(withdrawalQueue.prefinalize([], shareRate(1.5)), 'EmptyBatches()') - await assert.reverts( - withdrawalQueue.finalize([], defaultShareRate, { from: steth.address, value: amount }), - 'EmptyBatches()' - ) - }) - it('reverts if request with given id did not even created', async () => { const idAhead = +(await withdrawalQueue.getLastRequestId()) + 1 await assert.reverts( - withdrawalQueue.finalize([idAhead], defaultShareRate, { from: steth.address, value: amount }), + withdrawalQueue.finalize(idAhead, defaultShareRate, { from: steth.address, value: amount }), `InvalidRequestId(${idAhead})` ) @@ -480,10 +468,10 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, it('reverts if request with given id was finalized already', async () => { const id = +(await withdrawalQueue.getLastRequestId()) - await withdrawalQueue.finalize([id], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(id, defaultShareRate, { from: steth.address, value: amount }) await assert.reverts( - withdrawalQueue.finalize([id], defaultShareRate, { from: steth.address, value: amount }), + withdrawalQueue.finalize(id, defaultShareRate, { from: steth.address, value: amount }), `InvalidRequestId(${id})` ) @@ -495,7 +483,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, const amountExceeded = bn(ETH(400)) await assert.reverts( - withdrawalQueue.finalize([id], defaultShareRate, { from: steth.address, value: amountExceeded }), + withdrawalQueue.finalize(id, defaultShareRate, { from: steth.address, value: amountExceeded }), `TooMuchEtherToFinalize(${+amountExceeded}, ${+amount})` ) }) @@ -507,15 +495,15 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, }) it('works', async () => { - await withdrawalQueue.finalize([1], defaultShareRate, { from: steth.address, value: ETH(1) }) + await withdrawalQueue.finalize(1, defaultShareRate, { from: steth.address, value: ETH(1) }) assert.almostEqual(await withdrawalQueue.getClaimableEther([1], [1]), ETH(1), 100) }) it('reverts if last hint checkpoint is ahead of requestId', async () => { - await withdrawalQueue.finalize([1], shareRate(0.5), { from: steth.address, value: ETH(0.5) }) + await withdrawalQueue.finalize(1, shareRate(0.5), { from: steth.address, value: ETH(0.5) }) await withdrawalQueue.requestWithdrawals([ETH(2)], owner, { from: user }) - await withdrawalQueue.finalize([2], shareRate(0.5), { from: steth.address, value: ETH(0.5) }) + await withdrawalQueue.finalize(2, shareRate(0.5), { from: steth.address, value: ETH(0.5) }) await assert.reverts(withdrawalQueue.getClaimableEther([1], [2]), 'InvalidHint(2)') }) @@ -526,9 +514,15 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, }) it('return 0 for claimed request', async () => { - await withdrawalQueue.finalize([1], shareRate(1), { from: steth.address, value: ETH(1) }) - await withdrawalQueue.claimWithdrawals([1], [1], { from: owner }) - + await withdrawalQueue.finalize(1, shareRate(1), { from: steth.address, value: ETH(1) }) + const amountOfETH = (await withdrawalQueue.getClaimableEther([1], [1]))[0] + const tx = await withdrawalQueue.claimWithdrawals([1], [1], { from: owner }) + assert.emits(tx, 'WithdrawalClaimed', { + requestId: 1, + owner, + receiver: owner, + amountOfETH, + }) assert.equals(await withdrawalQueue.getClaimableEther([1], [1]), ETH(0)) assert.equals(await withdrawalQueue.getClaimableEther([1], [51]), ETH(0)) }) @@ -537,7 +531,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, await assert.reverts(withdrawalQueue.getClaimableEther([0], [1]), 'InvalidRequestId(0)') await assert.reverts(withdrawalQueue.getClaimableEther([2], [1]), 'InvalidRequestId(2)') - await withdrawalQueue.finalize([1], shareRate(1), { from: steth.address, value: ETH(1) }) + await withdrawalQueue.finalize(1, shareRate(1), { from: steth.address, value: ETH(1) }) await assert.reverts(withdrawalQueue.getClaimableEther([1], [2]), 'InvalidHint(2)') await assert.reverts(withdrawalQueue.getClaimableEther([1], [0]), 'InvalidHint(0)') @@ -545,8 +539,8 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, await assert.reverts(withdrawalQueue.getClaimableEther([1], [2]), 'InvalidHint(2)') await withdrawalQueue.requestWithdrawals([ETH(1), ETH(1)], owner, { from: user }) - await withdrawalQueue.finalize([2], shareRate(0.99), { from: steth.address, value: ETH(0.99) }) - await withdrawalQueue.finalize([3], shareRate(0.98), { from: steth.address, value: ETH(0.98) }) + await withdrawalQueue.finalize(2, shareRate(0.99), { from: steth.address, value: ETH(0.99) }) + await withdrawalQueue.finalize(3, shareRate(0.98), { from: steth.address, value: ETH(0.98) }) await assert.reverts(withdrawalQueue.getClaimableEther([3], [1]), 'InvalidHint(1)') }) @@ -554,10 +548,10 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, it('works on multiple checkpoints, no discount', async () => { const requestCount = 5 const shareRate = await currentRate() - await withdrawalQueue.finalize([1], shareRate, { from: steth.address, value: ETH(1) }) + await withdrawalQueue.finalize(1, shareRate, { from: steth.address, value: ETH(1) }) for (let index = 0; index < requestCount; index++) { await withdrawalQueue.requestWithdrawals([ETH(1)], owner, { from: user }) - await withdrawalQueue.finalize([index + 2], shareRate, { from: steth.address, value: ETH(1) }) + await withdrawalQueue.finalize(index + 2, shareRate, { from: steth.address, value: ETH(1) }) } const requestIds = Array(requestCount + 1) .fill(0) @@ -580,11 +574,17 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, }) it('Owner can claim a finalized request to recipient address', async () => { - await withdrawalQueue.finalize([1], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(1, defaultShareRate, { from: steth.address, value: amount }) const balanceBefore = bn(await ethers.provider.getBalance(user)) - await withdrawalQueue.claimWithdrawalsTo([1], [1], user, { from: owner }) + const tx = await withdrawalQueue.claimWithdrawalsTo([1], [1], user, { from: owner }) + assert.emits(tx, 'WithdrawalClaimed', { + requestId: 1, + owner, + receiver: user, + amountOfETH: amount, + }) assert.equals(await ethers.provider.getBalance(user), balanceBefore.add(bn(amount))) }) @@ -597,12 +597,19 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, ) }) + it('reverts when requestIds and hints arrays length mismatch', async () => { + await assert.reverts( + withdrawalQueue.claimWithdrawalsTo([0], [1, 2], user, { from: owner }), + 'ArraysLengthMismatch(1, 2)' + ) + }) + it('reverts with zero _requestId', async () => { await assert.reverts(withdrawalQueue.claimWithdrawalsTo([0], [1], user, { from: owner }), 'InvalidRequestId(0)') }) it('reverts if sender is not owner', async () => { - await withdrawalQueue.finalize([1], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(1, defaultShareRate, { from: steth.address, value: amount }) await assert.reverts( withdrawalQueue.claimWithdrawalsTo([1], [1], owner, { from: stranger }), `NotOwner("${stranger}", "${owner}")` @@ -610,7 +617,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, }) it('reverts if there is not enough balance', async () => { - await withdrawalQueue.finalize([1], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(1, defaultShareRate, { from: steth.address, value: amount }) await setBalance(withdrawalQueue.address, ETH(200)) await assert.reverts(withdrawalQueue.claimWithdrawalsTo([1], [1], owner, { from: owner }), 'NotEnoughEther()') }) @@ -618,7 +625,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, it('reverts if receiver declines', async () => { const receiver = await ERC721ReceiverMock.new({ from: owner }) await receiver.setDoesAcceptTokens(false, { from: owner }) - await withdrawalQueue.finalize([1], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(1, defaultShareRate, { from: steth.address, value: amount }) await assert.reverts( withdrawalQueue.claimWithdrawalsTo([1], [1], receiver.address, { from: owner }), 'CantSendValueRecipientMayHaveReverted()' @@ -627,11 +634,17 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, }) it('Owner can claim a finalized request without hint', async () => { - await withdrawalQueue.finalize([1], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(1, defaultShareRate, { from: steth.address, value: amount }) const balanceBefore = bn(await ethers.provider.getBalance(owner)) - await withdrawalQueue.claimWithdrawal(1, { from: owner, gasPrice: 0 }) + const tx = await withdrawalQueue.claimWithdrawal(1, { from: owner, gasPrice: 0 }) + assert.emits(tx, 'WithdrawalClaimed', { + requestId: 1, + owner, + receiver: owner, + amountOfETH: amount, + }) assert.equals(await ethers.provider.getBalance(owner), balanceBefore.add(bn(amount))) }) @@ -654,13 +667,13 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, await withdrawalQueue.requestWithdrawals([amount], owner, { from: user }) - await withdrawalQueue.finalize([2], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(2, defaultShareRate, { from: steth.address, value: amount }) await assert.reverts(withdrawalQueue.claimWithdrawals([1], [0], { from: owner }), 'InvalidHint(0)') await assert.reverts(withdrawalQueue.claimWithdrawals([1], [2], { from: owner }), 'InvalidHint(2)') }) it('Cant withdraw token two times', async () => { - await withdrawalQueue.finalize([1], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(1, defaultShareRate, { from: steth.address, value: amount }) await withdrawalQueue.claimWithdrawal(1, { from: owner }) await assert.reverts(withdrawalQueue.claimWithdrawal(1, { from: owner }), 'RequestAlreadyClaimed(1)') @@ -668,7 +681,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, it('Discounted withdrawals produce less eth', async () => { const batch = await withdrawalQueue.prefinalize([1], shareRate(150)) - await withdrawalQueue.finalize([1], shareRate(150), { from: steth.address, value: batch.ethToLock }) + await withdrawalQueue.finalize(1, shareRate(150), { from: steth.address, value: batch.ethToLock }) const balanceBefore = bn(await ethers.provider.getBalance(owner)) assert.equals(await withdrawalQueue.getLockedEtherAmount(), batch.ethToLock) @@ -686,12 +699,12 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, await steth.approve(withdrawalQueue.address, StETH(21), { from: user }) assert.equals(await withdrawalQueue.getLastCheckpointIndex(), 0) const batch = await withdrawalQueue.prefinalize([1], shareRate(1)) - await withdrawalQueue.finalize([1], shareRate(1), { from: steth.address, value: batch.ethToLock }) + await withdrawalQueue.finalize(1, shareRate(1), { from: steth.address, value: batch.ethToLock }) for (let i = 1; i <= 20; i++) { assert.equals(await withdrawalQueue.getLastCheckpointIndex(), i) await withdrawalQueue.requestWithdrawals([StETH(1)], ZERO_ADDRESS, { from: user }) const batch = await withdrawalQueue.prefinalize([i + 1], shareRate(i + 1)) - await withdrawalQueue.finalize([i + 1], shareRate(i + 1), { + await withdrawalQueue.finalize(i + 1, shareRate(i + 1), { from: steth.address, value: batch.ethToLock, }) @@ -716,6 +729,10 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, await withdrawalQueue.requestWithdrawals([amount], owner, { from: user }) }) + it('reverts when requestIds and hints arrays length mismatch', async () => { + await assert.reverts(withdrawalQueue.claimWithdrawals([1, 2], [1], { from: owner }), 'ArraysLengthMismatch(2, 1)') + }) + it('claims correct requests', async () => { await steth.mintShares(owner, shares(300)) // 1 share to user and 299 shares to owner total = 300 ETH await steth.approve(withdrawalQueue.address, StETH(300), { from: owner }) @@ -723,10 +740,20 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, const secondRequestAmount = ETH(10) await withdrawalQueue.requestWithdrawals([secondRequestAmount], owner, { from: owner }) const secondRequestId = await withdrawalQueue.getLastRequestId() - await withdrawalQueue.finalize([secondRequestId], defaultShareRate, { from: steth.address, value: ETH(30) }) + await withdrawalQueue.finalize(secondRequestId, defaultShareRate, { from: steth.address, value: ETH(30) }) const balanceBefore = bn(await ethers.provider.getBalance(owner)) - await withdrawalQueue.claimWithdrawals([1, 2], [1, 1], { from: owner, gasPrice: 0 }) + const tx = await withdrawalQueue.claimWithdrawals([1, 2], [1, 1], { from: owner, gasPrice: 0 }) + assert.emits(tx, 'WithdrawalClaimed', { + requestId: 1, + owner, + receiver: owner, + }) + assert.emits(tx, 'WithdrawalClaimed', { + requestId: 2, + owner, + receiver: owner, + }) assert.almostEqual(await ethers.provider.getBalance(owner), balanceBefore.add(bn(ETH(30))), ALLOWED_ERROR_WEI * 2) }) }) @@ -748,7 +775,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, const id = await withdrawalQueue.getLastRequestId() const batch = await withdrawalQueue.prefinalize([id], normalizedShareRate) - withdrawalQueue.finalize([id], normalizedShareRate, { from: steth.address, value: batch.ethToLock }) + withdrawalQueue.finalize(id, normalizedShareRate, { from: steth.address, value: batch.ethToLock }) for (let index = 0; index < requestIds.length; index++) { const requestId = requestIds[index] await withdrawalQueue.claimWithdrawal(requestId, { from: user, gasPrice: 0 }) @@ -762,7 +789,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, const balanceBefore = bn(await ethers.provider.getBalance(user)) const id = await withdrawalQueue.getLastRequestId() const batch = await withdrawalQueue.prefinalize([id], normalizedShareRate) - withdrawalQueue.finalize([id], normalizedShareRate, { from: steth.address, value: batch.ethToLock }) + withdrawalQueue.finalize(id, normalizedShareRate, { from: steth.address, value: batch.ethToLock }) for (let index = requestIds.length - 1; index >= 0; index--) { const requestId = requestIds[index] await withdrawalQueue.claimWithdrawal(requestId, { from: user, gasPrice: 0 }) @@ -777,7 +804,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, const balanceBefore = bn(await ethers.provider.getBalance(user)) const id = await withdrawalQueue.getLastRequestId() const batch = await withdrawalQueue.prefinalize([id], normalizedShareRate) - withdrawalQueue.finalize([id], normalizedShareRate, { from: steth.address, value: batch.ethToLock }) + withdrawalQueue.finalize(id, normalizedShareRate, { from: steth.address, value: batch.ethToLock }) for (let index = 0; index < randomIds.length; index++) { const requestId = randomIds[index] await withdrawalQueue.claimWithdrawal(requestId, { from: user, gasPrice: 0 }) @@ -792,7 +819,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, for (let index = 0; index < requestIds.length; index++) { const requestId = requestIds[index] const batch = await withdrawalQueue.prefinalize([requestId], shareRate(300 / (index + 1))) - await withdrawalQueue.finalize([requestId], shareRate(300 / (index + 1)), { + await withdrawalQueue.finalize(requestId, shareRate(300 / (index + 1)), { from: steth.address, value: batch.ethToLock, }) @@ -811,7 +838,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, const id = await withdrawalQueue.getLastRequestId() const batches = await withdrawalQueue.prefinalize([id], 1) assert.equals(batches.ethToLock, 0) - withdrawalQueue.finalize([id], 1, { from: steth.address, value: batches.ethToLock }) + withdrawalQueue.finalize(id, 1, { from: steth.address, value: batches.ethToLock }) for (let index = 0; index < requestIds.length; index++) { const requestId = requestIds[index] const tx = await withdrawalQueue.claimWithdrawal(requestId, { from: user, gasPrice: 0 }) @@ -822,181 +849,193 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, }) }) - context('findCheckpointHints()', () => { - beforeEach(async () => { - const numOfRequests = 10 - const requests = Array(numOfRequests).fill(ETH(20)) - const discountedPrices = Array(numOfRequests) - .fill() - .map((_, i) => ETH(i)) - const sharesPerRequest = await steth.getSharesByPooledEth(ETH(20)) - const discountShareRates = discountedPrices.map((p) => shareRate(+p / +sharesPerRequest)) - - await withdrawalQueue.requestWithdrawals(requests, owner, { from: user }) - for (let i = 1; i <= numOfRequests; i++) { - await withdrawalQueue.finalize([i], discountShareRates[i - 1], { - from: steth.address, - value: discountedPrices[i - 1], - }) - } - assert.equals(await withdrawalQueue.getLastCheckpointIndex(), numOfRequests) - }) - - it('reverts if request is not finalized', async () => { - await withdrawalQueue.requestWithdrawals([ETH(1)], owner, { from: user }) - await assert.reverts(withdrawalQueue.findCheckpointHints([11], 1, 10), 'RequestNotFoundOrNotFinalized(11)') - }) + context('findCheckpointHints', () => { + const NOT_FOUND = 0 + context('unit tests', () => { + let requestId + const amount = ETH(20) - it('reverts if there is no such a request', async () => { - await assert.reverts(withdrawalQueue.findCheckpointHints([12], 1, 10), 'RequestNotFoundOrNotFinalized(12)') - }) + beforeEach('Enqueue a request', async () => { + await withdrawalQueue.requestWithdrawals([amount], owner, { from: user }) + requestId = await withdrawalQueue.getLastRequestId() + }) - it('range search (found)', async () => { - assert.equals(await withdrawalQueue.findCheckpointHints([5], 1, 9), 5) - assert.equals(await withdrawalQueue.findCheckpointHints([1], 1, 9), 1) - assert.equals(await withdrawalQueue.findCheckpointHints([9], 1, 9), 9) - assert.equals(await withdrawalQueue.findCheckpointHints([5], 5, 5), 5) - }) + it('correctly works before first finalization', async () => { + const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() + assert.equals(lastCheckpointIndex, 0) + const result = await withdrawalQueue.findCheckpointHints([requestId], 1, lastCheckpointIndex) + assert.isTrue(result.length === 1) + assert.equals(result[0], NOT_FOUND) - it('range search (not found)', async () => { - assert.equals(await withdrawalQueue.findCheckpointHints([10], 1, 5), 0) - assert.equals(await withdrawalQueue.findCheckpointHints([6], 1, 5), 0) - assert.equals(await withdrawalQueue.findCheckpointHints([1], 5, 5), 0) - assert.equals(await withdrawalQueue.findCheckpointHints([4], 5, 9), 0) - }) + const claimableEthResult = await withdrawalQueue.getClaimableEther([requestId], result) + assert.isTrue(claimableEthResult.length === 1) + assert.equals(claimableEthResult[0], NOT_FOUND) + }) - it('sequential search', async () => { - for (const [idToFind, searchLength] of [ - [1, 3], - [1, 10], - [10, 2], - [10, 3], - [8, 2], - [9, 3], - ]) { - assert.equals(await sequentialSearch(idToFind, searchLength), idToFind) - } - }) + it('reverts if first index is zero', async () => { + const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() + await assert.reverts( + withdrawalQueue.findCheckpointHints([1], 0, lastCheckpointIndex), + `InvalidRequestIdRange(0, ${+lastCheckpointIndex})` + ) + }) - const sequentialSearch = async (requestId, searchLength) => { - const lastIndex = await withdrawalQueue.getLastCheckpointIndex() + it('reverts if last index is larger than in store', async () => { + const lastCheckpointWrong = (await withdrawalQueue.getLastCheckpointIndex()) + 1 + await assert.reverts( + withdrawalQueue.findCheckpointHints([1], 1, lastCheckpointWrong), + `InvalidRequestIdRange(1, ${+lastCheckpointWrong})` + ) + }) - for (let i = 1; i <= lastIndex; i += searchLength) { - let end = i + searchLength - 1 - if (end > lastIndex) end = lastIndex - const foundIndex = await withdrawalQueue.findCheckpointHints([requestId], i, end) - if (+foundIndex !== 0) return foundIndex - } - } - }) + it('returns empty list when passed empty request ids list', async () => { + const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() + const hints = await withdrawalQueue.findCheckpointHints([], 1, lastCheckpointIndex) + assert.equal(hints.length, 0) + }) - context('findCheckpointHints() 2', () => { - let requestId - const amount = ETH(20) + it('returns not found when indexes have negative overlap', async () => { + const batch = await withdrawalQueue.prefinalize.call([requestId], defaultShareRate) + await withdrawalQueue.finalize(requestId, defaultShareRate, { from: steth.address, value: batch.ethToLock }) + const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() + const hints = await withdrawalQueue.findCheckpointHints( + [requestId], + +lastCheckpointIndex + 1, + lastCheckpointIndex + ) + assert.equal(hints.length, 1) + assert.equals(hints[0], NOT_FOUND) + }) - beforeEach('Enqueue a request', async () => { - await withdrawalQueue.requestWithdrawals([amount], owner, { from: user }) - requestId = await withdrawalQueue.getLastRequestId() - }) + it('returns hints array with one item for list from single request id', async () => { + const batch = await withdrawalQueue.prefinalize.call([requestId], defaultShareRate) + await withdrawalQueue.finalize(requestId, defaultShareRate, { from: steth.address, value: batch.ethToLock }) + const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() + const hints = await withdrawalQueue.findCheckpointHints([requestId], 1, lastCheckpointIndex) + assert.equal(hints.length, 1) + assert.equals(hints[0], 1) + }) - it('reverts if requestId is zero', async () => { - const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() - await assert.reverts(withdrawalQueue.findCheckpointHints([0], 1, lastCheckpointIndex), 'InvalidRequestId(0)') - }) + it('returns correct hints array for given request ids', async () => { + await withdrawalQueue.finalize(requestId, shareRate(20), { from: steth.address, value: ETH(20) }) - it('reverts if first index is zero', async () => { - const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() - await assert.reverts( - withdrawalQueue.findCheckpointHints([1], 0, lastCheckpointIndex), - `InvalidRequestIdRange(0, ${+lastCheckpointIndex})` - ) - }) + await steth.mintShares(owner, shares(1)) + await steth.approve(withdrawalQueue.address, StETH(300), { from: owner }) - it('reverts if last index is larger than in store', async () => { - const lastCheckpointWrong = (await withdrawalQueue.getLastCheckpointIndex()) + 1 - await assert.reverts( - withdrawalQueue.findCheckpointHints([1], 1, lastCheckpointWrong), - `InvalidRequestIdRange(1, ${+lastCheckpointWrong})` - ) - }) + const secondRequestAmount = ETH(10) + await withdrawalQueue.requestWithdrawals([secondRequestAmount], owner, { from: owner }) + const secondRequestId = await withdrawalQueue.getLastRequestId() - it('returns empty list when passed empty request ids list', async () => { - const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() - const hints = await withdrawalQueue.findCheckpointHints([], 1, lastCheckpointIndex) - assert.equal(hints.length, 0) - }) + const thirdRequestAmount = ETH(30) + await withdrawalQueue.requestWithdrawals([thirdRequestAmount], user, { from: user }) + const thirdRequestId = await withdrawalQueue.getLastRequestId() - it('returns not found when indexes have negative overlap', async () => { - const batch = await withdrawalQueue.prefinalize.call([requestId], defaultShareRate) - await withdrawalQueue.finalize([requestId], defaultShareRate, { from: steth.address, value: batch.ethToLock }) - const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() - const hints = await withdrawalQueue.findCheckpointHints( - [requestId], - +lastCheckpointIndex + 1, - lastCheckpointIndex - ) - assert.equal(hints.length, 1) - assert.equals(hints[0], 0) - }) + await withdrawalQueue.finalize(thirdRequestId, shareRate(20), { from: steth.address, value: ETH(40) }) - it('returns hints array with one item for list from single request id', async () => { - const batch = await withdrawalQueue.prefinalize.call([requestId], defaultShareRate) - await withdrawalQueue.finalize([requestId], defaultShareRate, { from: steth.address, value: batch.ethToLock }) - const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() - const hints = await withdrawalQueue.findCheckpointHints([requestId], 1, lastCheckpointIndex) - assert.equal(hints.length, 1) - assert.equals(hints[0], 1) - }) + const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() + const hints = await withdrawalQueue.findCheckpointHints( + [requestId, secondRequestId, thirdRequestId], + 1, + lastCheckpointIndex + ) + assert.equal(hints.length, 3) + assert.equals(hints[0], 1) + assert.equals(hints[1], 2) + assert.equals(hints[2], 2) + }) - it('returns correct hints array for given request ids', async () => { - await withdrawalQueue.finalize([requestId], shareRate(20), { from: steth.address, value: ETH(20) }) + it('reverts with RequestIdsNotSorted error when request ids not in ascending order', async () => { + await withdrawalQueue.finalize(requestId, shareRate(20), { from: steth.address, value: ETH(20) }) - await steth.mintShares(owner, shares(1)) - await steth.approve(withdrawalQueue.address, StETH(300), { from: owner }) + await steth.mintShares(owner, shares(1)) + await steth.approve(withdrawalQueue.address, StETH(300), { from: owner }) - const secondRequestAmount = ETH(10) - await withdrawalQueue.requestWithdrawals([secondRequestAmount], owner, { from: owner }) - const secondRequestId = await withdrawalQueue.getLastRequestId() + const secondRequestAmount = ETH(10) + await withdrawalQueue.requestWithdrawals([secondRequestAmount], owner, { from: owner }) + const secondRequestId = await withdrawalQueue.getLastRequestId() - const thirdRequestAmount = ETH(30) - await withdrawalQueue.requestWithdrawals([thirdRequestAmount], user, { from: user }) - const thirdRequestId = await withdrawalQueue.getLastRequestId() + const thirdRequestAmount = ETH(30) + await withdrawalQueue.requestWithdrawals([thirdRequestAmount], user, { from: user }) + const thirdRequestId = await withdrawalQueue.getLastRequestId() - await withdrawalQueue.finalize([thirdRequestId], shareRate(20), { from: steth.address, value: ETH(40) }) + await withdrawalQueue.finalize(thirdRequestId, shareRate(20), { from: steth.address, value: ETH(40) }) - const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() - const hints = await withdrawalQueue.findCheckpointHints( - [requestId, secondRequestId, thirdRequestId], - 1, - lastCheckpointIndex - ) - assert.equal(hints.length, 3) - assert.equals(hints[0], 1) - assert.equals(hints[1], 2) - assert.equals(hints[2], 2) + const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() + await assert.reverts( + withdrawalQueue.findCheckpointHints([requestId, thirdRequestId, secondRequestId], 1, lastCheckpointIndex), + 'RequestIdsNotSorted()' + ) + }) }) - it('reverts with RequestIdsNotSorted error when request ids not in ascending order', async () => { - await withdrawalQueue.finalize([requestId], shareRate(20), { from: steth.address, value: ETH(20) }) + context('range tests', () => { + beforeEach(async () => { + const numOfRequests = 10 + const requests = Array(numOfRequests).fill(ETH(20)) + const discountedPrices = Array(numOfRequests) + .fill() + .map((_, i) => ETH(i)) + const sharesPerRequest = await steth.getSharesByPooledEth(ETH(20)) + const discountShareRates = discountedPrices.map((p) => shareRate(+p / +sharesPerRequest)) + + await withdrawalQueue.requestWithdrawals(requests, owner, { from: user }) + for (let i = 1; i <= numOfRequests; i++) { + await withdrawalQueue.finalize([i], discountShareRates[i - 1], { + from: steth.address, + value: discountedPrices[i - 1], + }) + } + assert.equals(await withdrawalQueue.getLastCheckpointIndex(), numOfRequests) + }) - await steth.mintShares(owner, shares(1)) - await steth.approve(withdrawalQueue.address, StETH(300), { from: owner }) + it('return NOT_FOUND if request is not finalized', async () => { + await withdrawalQueue.requestWithdrawals([ETH(1)], owner, { from: user }) + const hints = await withdrawalQueue.findCheckpointHints([11], 1, 10) + assert.equals(hints.length, 1) + assert.equals(hints[0], NOT_FOUND) + }) - const secondRequestAmount = ETH(10) - await withdrawalQueue.requestWithdrawals([secondRequestAmount], owner, { from: owner }) - const secondRequestId = await withdrawalQueue.getLastRequestId() + it('reverts if there is no such a request', async () => { + await assert.reverts(withdrawalQueue.findCheckpointHints([12], 1, 10), 'InvalidRequestId(12)') + }) + + it('range search (found)', async () => { + assert.equals(await withdrawalQueue.findCheckpointHints([5], 1, 9), 5) + assert.equals(await withdrawalQueue.findCheckpointHints([1], 1, 9), 1) + assert.equals(await withdrawalQueue.findCheckpointHints([9], 1, 9), 9) + assert.equals(await withdrawalQueue.findCheckpointHints([5], 5, 5), 5) + }) - const thirdRequestAmount = ETH(30) - await withdrawalQueue.requestWithdrawals([thirdRequestAmount], user, { from: user }) - const thirdRequestId = await withdrawalQueue.getLastRequestId() + it('range search (not found)', async () => { + assert.equals(await withdrawalQueue.findCheckpointHints([10], 1, 5), 0) + assert.equals(await withdrawalQueue.findCheckpointHints([6], 1, 5), 0) + assert.equals(await withdrawalQueue.findCheckpointHints([1], 5, 5), 0) + assert.equals(await withdrawalQueue.findCheckpointHints([4], 5, 9), 0) + }) - await withdrawalQueue.finalize([thirdRequestId], shareRate(20), { from: steth.address, value: ETH(40) }) + it('sequential search', async () => { + for (const [idToFind, searchLength] of [ + [1, 3], + [1, 10], + [10, 2], + [10, 3], + [8, 2], + [9, 3], + ]) { + assert.equals(await sequentialSearch(idToFind, searchLength), idToFind) + } + }) - const lastCheckpointIndex = await withdrawalQueue.getLastCheckpointIndex() - await assert.reverts( - withdrawalQueue.findCheckpointHints([requestId, thirdRequestId, secondRequestId], 1, lastCheckpointIndex), - 'RequestIdsNotSorted()' - ) + const sequentialSearch = async (requestId, searchLength) => { + const lastIndex = await withdrawalQueue.getLastCheckpointIndex() + + for (let i = 1; i <= lastIndex; i += searchLength) { + let end = i + searchLength - 1 + if (end > lastIndex) end = lastIndex + const foundIndex = await withdrawalQueue.findCheckpointHints([requestId], i, end) + if (+foundIndex !== 0) return foundIndex + } + } }) }) @@ -1181,7 +1220,7 @@ contract('WithdrawalQueue', ([owner, stranger, daoAgent, user, pauser, resumer, }) it("One can't change claimed request", async () => { - await withdrawalQueue.finalize([requestId], defaultShareRate, { from: steth.address, value: amount }) + await withdrawalQueue.finalize(requestId, defaultShareRate, { from: steth.address, value: amount }) await withdrawalQueue.claimWithdrawal(requestId, { from: user }) await assert.reverts( diff --git a/test/common/lib/signature-utils.test.js b/test/common/lib/signature-utils.test.js index 6ff92d10c..d49a9c51c 100644 --- a/test/common/lib/signature-utils.test.js +++ b/test/common/lib/signature-utils.test.js @@ -63,7 +63,7 @@ function testWithConsumer(SignatureUtilsConsumer, desc) { await assert.reverts( sigUtils.isValidSignature(alice.address, msgHash, INVALID_V, sig.r, sig.s), - `ECDSA: invalid signature 'v' value` + 'ECDSA: invalid signature' ) }) }) diff --git a/test/helpers/factories.js b/test/helpers/factories.js index 374073f43..87ea41d6c 100644 --- a/test/helpers/factories.js +++ b/test/helpers/factories.js @@ -297,14 +297,12 @@ async function guardiansFactory({ deployParams }) { async function burnerFactory({ appManager, treasury, pool, voting }) { const burner = await Burner.new(appManager.address, treasury.address, pool.address, 0, 0) - const [REQUEST_BURN_MY_STETH_ROLE, REQUEST_BURN_SHARES_ROLE, RECOVER_ASSETS_ROLE] = await Promise.all([ + const [REQUEST_BURN_MY_STETH_ROLE, REQUEST_BURN_SHARES_ROLE] = await Promise.all([ burner.REQUEST_BURN_MY_STETH_ROLE(), burner.REQUEST_BURN_SHARES_ROLE(), - burner.RECOVER_ASSETS_ROLE(), ]) await burner.grantRole(REQUEST_BURN_MY_STETH_ROLE, voting.address, { from: appManager.address }) - await burner.grantRole(RECOVER_ASSETS_ROLE, voting.address, { from: appManager.address }) await burner.grantRole(REQUEST_BURN_SHARES_ROLE, voting.address, { from: appManager.address }) return burner