From e3c2aa855cb271189758d6c1c36a981c43fa9964 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Wed, 29 May 2024 19:00:27 +1000 Subject: [PATCH 1/9] WIP LidoWithdrawalStrategy --- .../strategies/LidoWithdrawalStrategy.sol | 297 ++++++++++++++++++ 1 file changed, 297 insertions(+) create mode 100644 contracts/contracts/strategies/LidoWithdrawalStrategy.sol diff --git a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol new file mode 100644 index 0000000000..cdc503f94e --- /dev/null +++ b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol @@ -0,0 +1,297 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import { IERC20, InitializableAbstractStrategy } from "../utils/InitializableAbstractStrategy.sol"; +import { IWETH9 } from "../interfaces/IWETH9.sol"; +import { IVault } from "../interfaces/IVault.sol"; + +interface IStETHWithdrawal { + event WithdrawalRequested( + uint256 indexed requestId, + address indexed requestor, + address indexed owner, + uint256 amountOfStETH, + uint256 amountOfShares + ); + event WithdrawalsFinalized( + uint256 indexed from, + uint256 indexed to, + uint256 amountOfETHLocked, + uint256 sharesToBurn, + uint256 timestamp + ); + event WithdrawalClaimed( + uint256 indexed requestId, + address indexed owner, + address indexed receiver, + uint256 amountOfETH + ); + + struct WithdrawalRequestStatus { + /// @notice stETH token amount that was locked on withdrawal queue for this request + uint256 amountOfStETH; + /// @notice amount of stETH shares locked on withdrawal queue for this request + uint256 amountOfShares; + /// @notice address that can claim or transfer this request + address owner; + /// @notice timestamp of when the request was created, in seconds + uint256 timestamp; + /// @notice true, if request is finalized + bool isFinalized; + /// @notice true, if request is claimed. Request is claimable if (isFinalized && !isClaimed) + bool isClaimed; + } + + function requestWithdrawals(uint256[] calldata _amounts, address _owner) + external + returns (uint256[] memory requestIds); + + function getLastCheckpointIndex() external view returns (uint256); + + function findCheckpointHints( + uint256[] calldata _requestIds, + uint256 _firstIndex, + uint256 _lastIndex + ) external view returns (uint256[] memory hintIds); + + function claimWithdrawals( + uint256[] calldata _requestIds, + uint256[] calldata _hints + ) external; + + function getWithdrawalStatus(uint256[] calldata _requestIds) + external + view + returns (WithdrawalRequestStatus[] memory statuses); + + function getWithdrawalRequests(address _owner) + external + view + returns (uint256[] memory requestsIds); +} + +/** + * @title Lido Withdrawal Strategy + * @notice This strategy withdraws ETH from stETH via the Lido Withdrawal Queue contract + * @author Origin Protocol Inc + */ +contract LidoWithdrawalStrategy is InitializableAbstractStrategy { + IWETH9 private constant weth = + IWETH9(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2); + IERC20 private constant stETH = + IERC20(0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84); + IStETHWithdrawal private constant withdrawalQueue = + IStETHWithdrawal(0x889edC2eDab5f40e902b864aD4d7AdE8E412F9B1); + uint256 public constant MaxWithdrawalAmount = 1000 ether; + uint256 public outstandingWithdrawals; + + event WithdrawalRequests(uint256[] requestIds, uint256[] amounts); + event WithdrawalClaims(uint256[] requestIds, uint256 amount); + + constructor(BaseStrategyConfig memory _stratConfig) + InitializableAbstractStrategy(_stratConfig) + { + require(MaxWithdrawalAmount < type(uint120).max); + } + + /** + * @notice initialize function, to set up initial internal state + * @param _rewardTokenAddresses Address of reward token for platform + * @param _assets Addresses of initial supported assets + * @param _pTokens Platform Token corresponding addresses + */ + function initialize( + address[] memory _rewardTokenAddresses, + address[] memory _assets, + address[] memory _pTokens + ) external onlyGovernor initializer { + InitializableAbstractStrategy._initialize( + _rewardTokenAddresses, + _assets, + _pTokens + ); + safeApproveAllTokens(); + } + + /** + * @notice deposit() function not used for this strategy. Use depositAll() instead. + */ + function deposit(address, uint256) public override onlyVault nonReentrant { + // This method no longer used by the VaultAdmin, and we don't want it + // to be used by VaultCore. + require(false, "use depositAll() instead"); + } + + /** + * @notice Takes all given stETH and creates Lido withdrawal request + */ + function depositAll() external override onlyVault nonReentrant { + uint256 stETHStart = stETH.balanceOf(address(this)); + require(stETHStart > 0, "No stETH to withdraw"); + + uint256 numWithdrawals = stETHStart / MaxWithdrawalAmount; + uint256[] memory amounts = new uint256[](numWithdrawals); + + for (uint256 i = 0; i < numWithdrawals; i++) { + amounts[i] = i < numWithdrawals - 1 + ? MaxWithdrawalAmount + : stETHStart - (numWithdrawals - 1) * MaxWithdrawalAmount; + } + + uint256[] memory requestIds = withdrawalQueue.requestWithdrawals( + amounts, + address(this) + ); + + emit WithdrawalRequests(requestIds, amounts); + + require( + stETH.balanceOf(address(this)) == 0, + "Not all stEth in withdraw queue" + ); + outstandingWithdrawals += stETHStart; // Single set for gas reasons + + // This strategy claims to support WETH, so it is possible for + // the vault to transfer WETH to it. This returns any deposited WETH + // to the vault so that it is not lost for balance tracking purposes. + uint256 wethBalance = weth.balanceOf(address(this)); + if (wethBalance > 0) { + // slither-disable-next-line unchecked-transfer + weth.transfer(vaultAddress, wethBalance); + } + + emit Deposit(address(stETH), address(withdrawalQueue), stETHStart); + } + + /** + * @notice Withdraw an asset from the underlying platform + * @param _recipient Address to receive withdrawn assets + * @param _asset Address of the asset to withdraw + * @param _amount Amount of assets to withdraw + */ + function withdraw( + // solhint-disable-next-line no-unused-vars + address _recipient, + // solhint-disable-next-line no-unused-vars + address _asset, + // solhint-disable-next-line no-unused-vars + uint256 _amount + ) external override onlyVault nonReentrant { + // Does nothing - all withdrawals need to be called manually using the + // Strategist calling claimWithdrawals + revert("use claimWithdrawals()"); + } + + /** + * @notice Claim previously requested withdrawals that have now finalized. + * Called by the Strategist. + * @param _requestIds Array of withdrawal request identifiers + * @param expectedAmount Total amount of ETH expect to be withdrawn + */ + function claimWithdrawals( + uint256[] memory _requestIds, + uint256 expectedAmount + ) external nonReentrant { + require( + msg.sender == IVault(vaultAddress).strategistAddr(), + "Caller is not the Strategist" + ); + uint256 startingBalance = payable(address(this)).balance; + uint256 lastIndex = withdrawalQueue.getLastCheckpointIndex(); + uint256[] memory hintIds = withdrawalQueue.findCheckpointHints( + _requestIds, + 1, + lastIndex + ); + withdrawalQueue.claimWithdrawals(_requestIds, hintIds); + + uint256 currentBalance = payable(address(this)).balance; + uint256 withdrawalAmount = currentBalance - startingBalance; + require( + expectedAmount == withdrawalAmount, + "Withdrawal amount not expected" + ); + + emit WithdrawalClaims(_requestIds, withdrawalAmount); + + outstandingWithdrawals -= withdrawalAmount; + weth.deposit{ value: currentBalance }(); + // slither-disable-next-line unchecked-transfer + weth.transfer(vaultAddress, currentBalance); + emit Withdrawal( + address(weth), + address(withdrawalQueue), + currentBalance + ); + } + + function _abstractSetPToken(address, address) internal override { + revert("No pTokens are used"); + } + + /** + * @notice Withdraw all assets from this strategy, and transfer to the Vault. + * In correct operation, this strategy should never hold any assets. + */ + function withdrawAll() external override onlyVaultOrGovernor nonReentrant { + if (payable(address(this)).balance > 0) { + weth.deposit{ value: payable(address(this)).balance }(); + } + uint256 wethBalance = weth.balanceOf(address(this)); + if (wethBalance > 0) { + // slither-disable-next-line unchecked-transfer + weth.transfer(vaultAddress, wethBalance); + emit Withdrawal(address(weth), address(0), wethBalance); + } + uint256 fraxEthBalance = stETH.balanceOf(address(this)); + if (fraxEthBalance > 0) { + // slither-disable-next-line unchecked-transfer + stETH.transfer(vaultAddress, fraxEthBalance); + emit Withdrawal(address(stETH), address(0), fraxEthBalance); + } + } + + /** + * @notice Returns the amount of queued stETH that will be returned as WETH. + * We return this as a WETH asset, since that is what it will eventually be returned as. + * We only return the outstandingWithdrawals, because the contract itself should never hold any funds. + * @param _asset Address of the asset + * @return balance Total value of the asset in the platform + */ + function checkBalance(address _asset) + external + view + override + returns (uint256 balance) + { + if (_asset == address(weth)) { + return outstandingWithdrawals; + } else if (_asset == address(stETH)) { + return 0; + } else { + revert("Unexpected asset address"); + } + } + + /** + * @notice Approve the spending of all assets by their corresponding cToken, + * if for some reason is it necessary. + */ + function safeApproveAllTokens() public override { + // slither-disable-next-line unused-return + stETH.approve(address(withdrawalQueue), type(uint256).max); + } + + /** + * @notice Check if an asset is supported. + * @param _asset Address of the asset + * @return bool Whether asset is supported + */ + function supportsAsset(address _asset) public pure override returns (bool) { + // stETH can be deposited by the vault and balances are reported in WETH + return _asset == address(stETH) || _asset == address(weth); + } + + // Needed to receive ETH when withdrawal requests are claimed + receive() external payable {} +} From 961074ec32dc11a137a48805642e743fa7dee81c Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Wed, 29 May 2024 21:45:43 +1000 Subject: [PATCH 2/9] Added deployment of Lido withdrawal strategy --- contracts/contracts/proxies/Proxies.sol | 7 + .../deploy/mainnet/097_native_ssv_staking.js | 153 +++++++++++++----- contracts/test/_fixture.js | 2 +- 3 files changed, 119 insertions(+), 43 deletions(-) diff --git a/contracts/contracts/proxies/Proxies.sol b/contracts/contracts/proxies/Proxies.sol index 68763d5c8e..7feeec94da 100644 --- a/contracts/contracts/proxies/Proxies.sol +++ b/contracts/contracts/proxies/Proxies.sol @@ -227,3 +227,10 @@ contract NativeStakingFeeAccumulatorProxy is { } + +/** + * @notice LidoWithdrawalStrategyProxy delegates calls to a LidoWithdrawalStrategy implementation + */ +contract LidoWithdrawalStrategyProxy is InitializeGovernedUpgradeabilityProxy { + +} diff --git a/contracts/deploy/mainnet/097_native_ssv_staking.js b/contracts/deploy/mainnet/097_native_ssv_staking.js index 64fa655a42..6bcfb9eb0e 100644 --- a/contracts/deploy/mainnet/097_native_ssv_staking.js +++ b/contracts/deploy/mainnet/097_native_ssv_staking.js @@ -33,11 +33,11 @@ module.exports = deploymentWithGovernanceProposal( // ---------------- // 1. Fetch the strategy proxy deployed by relayer - const cStrategyProxy = await ethers.getContract( + const cNativeStakingStrategyProxy = await ethers.getContract( "NativeStakingSSVStrategyProxy" ); - // 2. Deploy the new fee accumulator proxy + // 2. Deploy the new FeeAccumulator proxy const dFeeAccumulatorProxy = await deployWithConfirmation( "NativeStakingFeeAccumulatorProxy" ); @@ -46,8 +46,28 @@ module.exports = deploymentWithGovernanceProposal( dFeeAccumulatorProxy.address ); - // 3. Deploy the new strategy implementation - const dStrategyImpl = await deployWithConfirmation( + // 3. Deploy the new FeeAccumulator implementation + const dFeeAccumulator = await deployWithConfirmation("FeeAccumulator", [ + cNativeStakingStrategyProxy.address, // _collector + ]); + const cFeeAccumulator = await ethers.getContractAt( + "FeeAccumulator", + dFeeAccumulator.address + ); + + // 4. Init the FeeAccumulator proxy to point at the implementation, set the governor + const proxyInitFunction = "initialize(address,address,bytes)"; + await withConfirmation( + cFeeAccumulatorProxy.connect(sDeployer)[proxyInitFunction]( + cFeeAccumulator.address, // implementation address + addresses.mainnet.Timelock, // governance + "0x", // do not call any initialize functions + await getTxOpts() + ) + ); + + // 5. Deploy the new Native Staking Strategy implementation + const dNativeStakingStrategyImpl = await deployWithConfirmation( "NativeStakingSSVStrategy", [ [addresses.zero, cVaultProxy.address], //_baseConfig @@ -58,18 +78,18 @@ module.exports = deploymentWithGovernanceProposal( addresses.mainnet.beaconChainDepositContract, // beacon chain deposit contract ] ); - const cStrategyImpl = await ethers.getContractAt( + const cNativeStakingStrategyImpl = await ethers.getContractAt( "NativeStakingSSVStrategy", - dStrategyImpl.address + dNativeStakingStrategyImpl.address ); - const cStrategy = await ethers.getContractAt( + const cNativeStakingStrategy = await ethers.getContractAt( "NativeStakingSSVStrategy", - cStrategyProxy.address + cNativeStakingStrategyProxy.address ); - // 3. Initialize Proxy with new implementation and strategy initialization - const initData = cStrategyImpl.interface.encodeFunctionData( + // 6. Initialize Native Staking Proxy with new implementation and strategy initialization + const initData = cNativeStakingStrategyImpl.interface.encodeFunctionData( "initialize(address[],address[],address[])", [ [addresses.mainnet.WETH], // reward token addresses @@ -87,7 +107,7 @@ module.exports = deploymentWithGovernanceProposal( "100" ); await withConfirmation( - cStrategyProxy + cNativeStakingStrategyProxy .connect(relayerSigner) .transferGovernance(deployerAddr, await getTxOpts()) ); @@ -99,47 +119,73 @@ module.exports = deploymentWithGovernanceProposal( * Run the following to make it happen, and comment this error block out: * yarn run hardhat transferGovernanceNativeStakingProxy --address 0xdeployerAddress --network mainnet */ - new Error("Transfer governance not yet ran"); + const proxyGovernor = await cNativeStakingStrategyProxy.governor(); + if (proxyGovernor != sDeployer.address) { + throw new Error( + `Native Staking Strategy proxy's governor: ${proxyGovernor} does not match current deployer ${sDeployer.address}` + ); + } } + // 7. Transfer governance of the Native Staking Strategy proxy to the deployer await withConfirmation( - cStrategyProxy.connect(sDeployer).claimGovernance(await getTxOpts()) + cNativeStakingStrategyProxy + .connect(sDeployer) + .claimGovernance(await getTxOpts()) ); - // 4. Init the proxy to point at the implementation, set the governor, and call initialize - const proxyInitFunction = "initialize(address,address,bytes)"; + // 9. Init the proxy to point at the implementation, set the governor, and call initialize await withConfirmation( - cStrategyProxy.connect(sDeployer)[proxyInitFunction]( - cStrategyImpl.address, // implementation address + cNativeStakingStrategyProxy.connect(sDeployer)[proxyInitFunction]( + cNativeStakingStrategyImpl.address, // implementation address addresses.mainnet.Timelock, // governance initData, // data for call to the initialize function on the strategy await getTxOpts() ) ); - // 5. Deploy the new fee accumulator implementation - const dFeeAccumulator = await deployWithConfirmation("FeeAccumulator", [ - cStrategyProxy.address, // _collector - ]); - const cFeeAccumulator = await ethers.getContractAt( - "FeeAccumulator", - dFeeAccumulator.address + // 10. Safe approve SSV token spending + await cNativeStakingStrategy.connect(sDeployer).safeApproveAllTokens(); + + // 7. Deploy the Lido Withdrawal Strategy + const dWithdrawalStrategyStrategyProxy = await deployWithConfirmation( + "LidoWithdrawalStrategyProxy" + ); + const cWithdrawalStrategyStrategyProxy = await ethers.getContractAt( + "LidoWithdrawalStrategyProxy", + dWithdrawalStrategyStrategyProxy.address + ); + const dWithdrawalStrategyImpl = await deployWithConfirmation( + "LidoWithdrawalStrategy", + [ + [addresses.zero, cVaultProxy.address], //_baseConfig + ] + ); + const cWithdrawalStrategyImpl = await ethers.getContractAt( + "LidoWithdrawalStrategy", + dWithdrawalStrategyImpl.address ); - // 6. Init the fee accumulator proxy to point at the implementation, set the governor + // 8. Init the Lido Withdrawal strategy proxy to point at the implementation, set the governor, and call initialize + const withdrawalInitData = + cWithdrawalStrategyImpl.interface.encodeFunctionData( + "initialize(address[],address[],address[])", + [ + [], // reward token addresses + [], // asset token addresses + [], // platform tokens addresses + ] + ); await withConfirmation( - cFeeAccumulatorProxy.connect(sDeployer)[proxyInitFunction]( - cFeeAccumulator.address, // implementation address + cWithdrawalStrategyStrategyProxy.connect(sDeployer)[proxyInitFunction]( + cWithdrawalStrategyImpl.address, addresses.mainnet.Timelock, // governance - "0x", // do not call any initialize functions + withdrawalInitData, // data for call to the initialize function on the strategy await getTxOpts() ) ); - // 7. Safe approve SSV token spending - await cStrategy.connect(sDeployer).safeApproveAllTokens(); - - // 8. Deploy Harvester + // 11. Deploy Harvester const cOETHHarvesterProxy = await ethers.getContract("OETHHarvesterProxy"); await deployWithConfirmation("OETHHarvester", [ cVaultProxy.address, @@ -147,13 +193,24 @@ module.exports = deploymentWithGovernanceProposal( ]); const dOETHHarvesterImpl = await ethers.getContract("OETHHarvester"); - console.log("Native Staking SSV Strategy proxy: ", cStrategyProxy.address); + console.log( + "Native Staking SSV Strategy proxy: ", + cNativeStakingStrategyProxy.address + ); console.log( "Native Staking SSV Strategy implementation: ", - dStrategyImpl.address + cNativeStakingStrategyImpl.address ); console.log("Fee accumulator proxy: ", cFeeAccumulatorProxy.address); console.log("Fee accumulator implementation: ", cFeeAccumulator.address); + console.log( + "Lido withdrawal strategy proxy: ", + cWithdrawalStrategyStrategyProxy.address + ); + console.log( + "Lido withdrawal strategy implementation: ", + cWithdrawalStrategyImpl.address + ); console.log( "New OETHHarvester implementation: ", dOETHHarvesterImpl.address @@ -162,29 +219,35 @@ module.exports = deploymentWithGovernanceProposal( // Governance Actions // ---------------- return { - name: "Deploy new OETH Native Staking Strategy\n\nThis is going to become the main strategy to power the reward accrual of OETH by staking ETH into SSV validators.", + name: `Deploy new OETH Native Staking Strategy. + +This is going to become the main strategy to power the reward accrual of OETH by staking ETH in SSV validators. + +Deployed a new strategy to convert stETH to WETH at 1:1 using the Lido withdrawal queue. + +Upgraded the Harvester so ETH rewards can be sent straight to the Dripper as WETH.`, actions: [ // 1. Add new strategy to vault { contract: cVaultAdmin, signature: "approveStrategy(address)", - args: [cStrategyProxy.address], + args: [cNativeStakingStrategyProxy.address], }, // 2. configure Harvester to support the strategy { contract: cHarvester, signature: "setSupportedStrategy(address,bool)", - args: [cStrategyProxy.address, true], + args: [cNativeStakingStrategyProxy.address, true], }, // 3. set harvester to the strategy { - contract: cStrategy, + contract: cNativeStakingStrategy, signature: "setHarvesterAddress(address)", args: [cHarvesterProxy.address], }, // 4. configure the fuse interval { - contract: cStrategy, + contract: cNativeStakingStrategy, signature: "setFuseInterval(uint256,uint256)", args: [ ethers.utils.parseEther("21.6"), @@ -193,20 +256,20 @@ module.exports = deploymentWithGovernanceProposal( }, // 5. set validator registrator { - contract: cStrategy, + contract: cNativeStakingStrategy, signature: "setRegistrator(address)", args: [addresses.mainnet.validatorRegistrator], }, // 6. set staking threshold { - contract: cStrategy, + contract: cNativeStakingStrategy, signature: "setStakeETHThreshold(uint256)", // TODO: confirm this number makes sense args: [ethers.utils.parseEther("1024")], // 32ETH * 32 }, // 7. set staking monitor { - contract: cStrategy, + contract: cNativeStakingStrategy, signature: "setStakingMonitor(address)", // The 5/8 multisig args: [addresses.mainnet.Guardian], @@ -217,6 +280,12 @@ module.exports = deploymentWithGovernanceProposal( signature: "upgradeTo(address)", args: [dOETHHarvesterImpl.address], }, + // 9. Add new Lido Withdrawal Strategy to vault + { + contract: cVaultAdmin, + signature: "approveStrategy(address)", + args: [cWithdrawalStrategyStrategyProxy.address], + }, ], }; } diff --git a/contracts/test/_fixture.js b/contracts/test/_fixture.js index ab50a047b0..3320e128b3 100644 --- a/contracts/test/_fixture.js +++ b/contracts/test/_fixture.js @@ -1975,7 +1975,7 @@ async function aaveVaultFixture() { } /** - * Configure a Vault hold frxEth to be redeeemed by the frxEthRedeemStrategy + * Configure a Vault hold frxEth to be redeemed by the frxEthRedeemStrategy */ async function frxEthRedeemStrategyFixture() { const fixture = await oethDefaultFixture(); From 5fb268679dfa64fa16757da07d58de85754e0f06 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Thu, 30 May 2024 17:04:03 +1000 Subject: [PATCH 3/9] Completed Lido Withdrawal Strategy fork tests --- .../strategies/LidoWithdrawalStrategy.sol | 24 +- contracts/test/_fixture.js | 34 +++ .../lido_withdrawal_strategy.fork-test.js | 227 ++++++++++++++++++ contracts/utils/addresses.js | 4 + 4 files changed, 282 insertions(+), 7 deletions(-) create mode 100644 contracts/test/strategies/lido_withdrawal_strategy.fork-test.js diff --git a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol index cdc503f94e..bd4e438c2e 100644 --- a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol +++ b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol @@ -68,6 +68,11 @@ interface IStETHWithdrawal { external view returns (uint256[] memory requestsIds); + + function finalize( + uint256 _lastRequestIdToBeFinalized, + uint256 _maxShareRate + ) external payable; } /** @@ -129,14 +134,16 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { uint256 stETHStart = stETH.balanceOf(address(this)); require(stETHStart > 0, "No stETH to withdraw"); - uint256 numWithdrawals = stETHStart / MaxWithdrawalAmount; + uint256 numWithdrawals = (stETHStart / MaxWithdrawalAmount) + 1; uint256[] memory amounts = new uint256[](numWithdrawals); - for (uint256 i = 0; i < numWithdrawals; i++) { - amounts[i] = i < numWithdrawals - 1 - ? MaxWithdrawalAmount - : stETHStart - (numWithdrawals - 1) * MaxWithdrawalAmount; + uint256 stETHRemaining = stETHStart; + uint256 i = 0; + while (stETHRemaining > MaxWithdrawalAmount) { + amounts[i++] = MaxWithdrawalAmount; + stETHRemaining -= MaxWithdrawalAmount; } + amounts[i] = stETHRemaining; uint256[] memory requestIds = withdrawalQueue.requestWithdrawals( amounts, @@ -145,8 +152,9 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { emit WithdrawalRequests(requestIds, amounts); + // Is there any stETH left except 1 wei from each request? require( - stETH.balanceOf(address(this)) == 0, + stETH.balanceOf(address(this)) <= numWithdrawals, "Not all stEth in withdraw queue" ); outstandingWithdrawals += stETHStart; // Single set for gas reasons @@ -207,8 +215,10 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { uint256 currentBalance = payable(address(this)).balance; uint256 withdrawalAmount = currentBalance - startingBalance; + // Withdrawal amount should be within 2 wei of expected amount require( - expectedAmount == withdrawalAmount, + withdrawalAmount + 2 >= expectedAmount && + withdrawalAmount <= expectedAmount, "Withdrawal amount not expected" ); diff --git a/contracts/test/_fixture.js b/contracts/test/_fixture.js index 3320e128b3..628a17bacc 100644 --- a/contracts/test/_fixture.js +++ b/contracts/test/_fixture.js @@ -337,6 +337,7 @@ const defaultFixture = deployments.createFixture(async () => { morphoCompoundStrategy, fraxEthStrategy, frxEthRedeemStrategy, + lidoWithdrawalStrategy, balancerREthStrategy, makerDsrStrategy, morphoAaveStrategy, @@ -467,6 +468,14 @@ const defaultFixture = deployments.createFixture(async () => { frxEthRedeemStrategyProxy.address ); + const lidoWithdrawalStrategyProxy = await ethers.getContract( + "LidoWithdrawalStrategyProxy" + ); + lidoWithdrawalStrategy = await ethers.getContractAt( + "LidoWithdrawalStrategy", + lidoWithdrawalStrategyProxy.address + ); + const balancerRethStrategyProxy = await ethers.getContract( "OETHBalancerMetaPoolrEthStrategyProxy" ); @@ -773,6 +782,7 @@ const defaultFixture = deployments.createFixture(async () => { nativeStakingSSVStrategy, nativeStakingFeeAccumulator, frxEthRedeemStrategy, + lidoWithdrawalStrategy, balancerREthStrategy, oethMorphoAaveStrategy, woeth, @@ -1992,6 +2002,29 @@ async function frxEthRedeemStrategyFixture() { return fixture; } +/** + * Configure a Vault hold stEth to be withdrawn by the LidoWithdrawalStrategy + */ +async function lidoWithdrawalStrategyFixture() { + const fixture = await oethDefaultFixture(); + + // Give weth and stETH to the vault + + await fixture.stETH + .connect(fixture.daniel) + .transfer(fixture.oethVault.address, parseUnits("2002")); + await fixture.weth + .connect(fixture.daniel) + .transfer(fixture.oethVault.address, parseUnits("2003")); + + fixture.lidoWithdrawalQueue = await ethers.getContractAt( + "IStETHWithdrawal", + addresses.mainnet.LidoWithdrawalQueue + ); + + return fixture; +} + /** * Configure a compound fixture with a false vault for testing */ @@ -2459,6 +2492,7 @@ module.exports = { untiltBalancerMetaStableWETHPool, fraxETHStrategyFixture, frxEthRedeemStrategyFixture, + lidoWithdrawalStrategyFixture, nativeStakingSSVStrategyFixture, oethMorphoAaveFixture, oeth1InchSwapperFixture, diff --git a/contracts/test/strategies/lido_withdrawal_strategy.fork-test.js b/contracts/test/strategies/lido_withdrawal_strategy.fork-test.js new file mode 100644 index 0000000000..366e8dc76e --- /dev/null +++ b/contracts/test/strategies/lido_withdrawal_strategy.fork-test.js @@ -0,0 +1,227 @@ +const { expect } = require("chai"); +const { + createFixtureLoader, + lidoWithdrawalStrategyFixture, +} = require("./../_fixture"); +const { ousdUnits, isCI } = require("../helpers"); +const { impersonateAccount } = require("../../utils/signers"); +const { parseUnits } = require("ethers/lib/utils"); + +describe("ForkTest: Lido Withdrawal Strategy", function () { + this.timeout(360 * 1000); + + // Retry up to 3 times on CI + this.retries(isCI ? 3 : 0); + + let fixture; + const loadFixture = createFixtureLoader(lidoWithdrawalStrategyFixture); + beforeEach(async () => { + fixture = await loadFixture(); + }); + + describe("Post-deployment", function () { + it("Should support WETH and stETH", async () => { + const { lidoWithdrawalStrategy, weth, stETH } = fixture; + expect(await lidoWithdrawalStrategy.supportsAsset(weth.address)).to.be + .true; + expect(await lidoWithdrawalStrategy.supportsAsset(stETH.address)).to.be + .true; + }); + }); + + describe("Redeem Lifecyle", function () { + it("Should redeem stETH for WETH (multiple requests)", async function () { + await _testWithdrawalCycle(ousdUnits("17003.45"), 18); + }); + it("Should redeem stETH for WETH (1 request)", async function () { + await _testWithdrawalCycle(ousdUnits("250"), 1); + }); + it("Should redeem stETH for WETH (2 request)", async function () { + await _testWithdrawalCycle(ousdUnits("1999.99"), 2); + }); + it("Should redeem stETH for WETH (1 small request)", async function () { + await _testWithdrawalCycle(ousdUnits("0.03"), 1); + }); + it("Should revert on zero amount", async function () { + const { lidoWithdrawalStrategy, stETH, oethVault, strategist } = fixture; + const txPromise = oethVault + .connect(strategist) + .depositToStrategy( + lidoWithdrawalStrategy.address, + [stETH.address], + [0] + ); + await expect(txPromise).to.be.revertedWith("No stETH to withdraw"); + }); + }); + + describe("Strategist sent WETH", function () { + it("Should return WETH to the vault", async function () { + const { lidoWithdrawalStrategy, weth, stETH, oethVault, strategist } = + fixture; + const initialWeth = await weth.balanceOf(oethVault.address); + await oethVault + .connect(strategist) + .depositToStrategy( + lidoWithdrawalStrategy.address, + [weth.address, stETH.address], + [ousdUnits("1000"), ousdUnits("1000")] + ); + const afterWeth = await weth.balanceOf(oethVault.address); + expect(afterWeth).to.equal(initialWeth); + }); + }); + + describe("withdrawAll()", function () { + it("Should withdraw all WETH, stETH, and native ETH to the vault", async function () { + const { + lidoWithdrawalStrategy, + weth, + stETH, + oethVault, + strategist, + daniel, + } = fixture; + + // Deposit some WETH, stETH, and native ETH to the strategy contract + const wethAmount = ousdUnits("10.34"); + const stETHAmount = ousdUnits("20.123"); + const nativeEthAmount = ousdUnits("5.2345"); + await weth + .connect(daniel) + .transfer(lidoWithdrawalStrategy.address, wethAmount); + await stETH + .connect(daniel) + .transfer(lidoWithdrawalStrategy.address, stETHAmount); + await strategist.sendTransaction({ + to: lidoWithdrawalStrategy.address, + value: nativeEthAmount, + }); + + // Get initial balances + const initialWethBalanceVault = await weth.balanceOf(oethVault.address); + const initialStEthBalanceVault = await stETH.balanceOf(oethVault.address); + const initialNativeEthBalanceVault = await oethVault.provider.getBalance( + oethVault.address + ); + + // Call withdrawAll() + await oethVault + .connect(strategist) + .withdrawAllFromStrategy(lidoWithdrawalStrategy.address); + + // Check final balances + const finalWethBalanceVault = await weth.balanceOf(oethVault.address); + const finalstEthBalanceVault = await stETH.balanceOf(oethVault.address); + const finalNativeEthBalanceVault = await oethVault.provider.getBalance( + oethVault.address + ); + expect(finalWethBalanceVault.sub(initialWethBalanceVault)) + .to.gte(wethAmount.add(nativeEthAmount).sub(2)) + .lte(wethAmount.add(nativeEthAmount)); + expect(finalstEthBalanceVault.sub(initialStEthBalanceVault)) + .to.gte( + // stETH transfers can leave 1-2 wei in the contract + stETHAmount.sub(2) + ) + .lte(stETHAmount); + expect(finalNativeEthBalanceVault).to.equal(initialNativeEthBalanceVault); + }); + + it("Should handle the case when the strategy has no assets", async function () { + const { lidoWithdrawalStrategy, oethVault, strategist } = fixture; + await oethVault + .connect(strategist) + .withdrawAllFromStrategy(lidoWithdrawalStrategy.address); + }); + }); + + async function parseRequestIds(tx, lidoWithdrawalStrategy) { + const WithdrawalRequestedTopic = + "0x4a54e868001801e435d72d0f5a4ead23b6be3f49544fcfde1b83dd6d779a50f4"; + const receipt = await tx.wait(); + const log = receipt.events.find( + (x) => + x.address == lidoWithdrawalStrategy.address && + x.topics[0] == WithdrawalRequestedTopic + ); + const event = lidoWithdrawalStrategy.interface.parseLog(log); + return event.args.requestIds; + } + + async function finalizeRequests(requestIds, stETH, withdrawalQueue) { + const stETHSigner = await impersonateAccount(stETH.address); + const lastRequest = requestIds.slice(-1)[0]; + + const maxShareRate = parseUnits("1200000000", 18); + await withdrawalQueue + .connect(stETHSigner) + .finalize(lastRequest, maxShareRate); + + // check the first request is finalized + const requestStatuses = await withdrawalQueue.getWithdrawalStatus( + requestIds + ); + expect(requestStatuses[0].isFinalized).to.be.true; + } + + async function _testWithdrawalCycle(amount, expectedRequests) { + const { + lidoWithdrawalStrategy, + lidoWithdrawalQueue, + weth, + stETH, + oethVault, + strategist, + } = fixture; + const initialEth = await weth.balanceOf(oethVault.address); + const initialOutstanding = + await lidoWithdrawalStrategy.outstandingWithdrawals(); + expect(await lidoWithdrawalStrategy.checkBalance(weth.address)).to.equal( + await lidoWithdrawalStrategy.outstandingWithdrawals() + ); + expect(await lidoWithdrawalStrategy.checkBalance(stETH.address)).to.equal( + 0 + ); + + const tx = await oethVault + .connect(strategist) + .depositToStrategy( + lidoWithdrawalStrategy.address, + [stETH.address], + [amount] + ); + expect(await lidoWithdrawalStrategy.outstandingWithdrawals()) + .to.gte(initialOutstanding.add(amount).sub(2)) + .lte(initialOutstanding.add(amount)); + expect(await lidoWithdrawalStrategy.checkBalance(weth.address)).to.equal( + await lidoWithdrawalStrategy.outstandingWithdrawals() + ); + expect(await lidoWithdrawalStrategy.checkBalance(stETH.address)).to.equal( + 0 + ); + + const requestIds = await parseRequestIds(tx, lidoWithdrawalStrategy); + + // finalize the requests so they can be claimed + await finalizeRequests(requestIds, stETH, lidoWithdrawalQueue); + + // Claim finalized requests + await lidoWithdrawalStrategy + .connect(strategist) + .claimWithdrawals(requestIds, amount); + + const afterEth = await weth.balanceOf(oethVault.address); + expect(requestIds.length).to.equal(expectedRequests); + expect(afterEth.sub(initialEth)).to.gte(amount.sub(2)).lte(amount); + expect(await lidoWithdrawalStrategy.outstandingWithdrawals()).to.equal( + initialOutstanding + ); + expect(await lidoWithdrawalStrategy.checkBalance(weth.address)).to.equal( + await lidoWithdrawalStrategy.outstandingWithdrawals() + ); + expect(await lidoWithdrawalStrategy.checkBalance(stETH.address)).to.equal( + 0 + ); + } +}); diff --git a/contracts/utils/addresses.js b/contracts/utils/addresses.js index c1d5e42395..6367ebedd9 100644 --- a/contracts/utils/addresses.js +++ b/contracts/utils/addresses.js @@ -261,6 +261,10 @@ addresses.mainnet.NativeStakingSSVStrategyProxy = addresses.mainnet.validatorRegistrator = "0x4b91827516f79d6F6a1F292eD99671663b09169a"; +// Lido Withdrawal Queue +addresses.mainnet.LidoWithdrawalQueue = + "0x889edC2eDab5f40e902b864aD4d7AdE8E412F9B1"; + // Arbitrum One addresses.arbitrumOne = {}; addresses.arbitrumOne.WOETHProxy = "0xD8724322f44E5c58D7A815F542036fb17DbbF839"; From a24726a72e11f60302344161a3029beb216c1b8b Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Tue, 4 Jun 2024 09:21:41 +1000 Subject: [PATCH 4/9] Updated Natspec --- contracts/contracts/strategies/LidoWithdrawalStrategy.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol index bd4e438c2e..599206f698 100644 --- a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol +++ b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol @@ -81,13 +81,18 @@ interface IStETHWithdrawal { * @author Origin Protocol Inc */ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { + /// @notice Address of the WETH token IWETH9 private constant weth = IWETH9(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2); + /// @notice Address of the stETH token IERC20 private constant stETH = IERC20(0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84); + /// @notice Address of the Lido Withdrawal Queue contract IStETHWithdrawal private constant withdrawalQueue = IStETHWithdrawal(0x889edC2eDab5f40e902b864aD4d7AdE8E412F9B1); + /// @notice Maximum amount of stETH that can be withdrawn in a single request uint256 public constant MaxWithdrawalAmount = 1000 ether; + /// @notice Total amount of stETH that has been requested to be withdrawn for ETH uint256 public outstandingWithdrawals; event WithdrawalRequests(uint256[] requestIds, uint256[] amounts); @@ -302,6 +307,6 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { return _asset == address(stETH) || _asset == address(weth); } - // Needed to receive ETH when withdrawal requests are claimed + /// @notice Needed to receive ETH when withdrawal requests are claimed receive() external payable {} } From b6913ccc2bc32c1bec329fffe44218fcc82b1ba1 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Tue, 4 Jun 2024 09:25:41 +1000 Subject: [PATCH 5/9] Renamed fraxEth to stEth in LidoWithdrawalStrategy --- contracts/contracts/strategies/LidoWithdrawalStrategy.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol index 599206f698..8ba5179b00 100644 --- a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol +++ b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol @@ -258,11 +258,11 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { weth.transfer(vaultAddress, wethBalance); emit Withdrawal(address(weth), address(0), wethBalance); } - uint256 fraxEthBalance = stETH.balanceOf(address(this)); - if (fraxEthBalance > 0) { + uint256 stEthBalance = stETH.balanceOf(address(this)); + if (stEthBalance > 0) { // slither-disable-next-line unchecked-transfer - stETH.transfer(vaultAddress, fraxEthBalance); - emit Withdrawal(address(stETH), address(0), fraxEthBalance); + stETH.transfer(vaultAddress, stEthBalance); + emit Withdrawal(address(stETH), address(0), stEthBalance); } } From 3129889faad1a55ba3d8f7def7d1404c2e35addb Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Tue, 4 Jun 2024 09:28:35 +1000 Subject: [PATCH 6/9] Moved _abstractSetPToken to bottom of the LidoWithdrawalStrategy made _abstractSetPToken pure --- contracts/contracts/strategies/LidoWithdrawalStrategy.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol index 8ba5179b00..5cb8021391 100644 --- a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol +++ b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol @@ -240,10 +240,6 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { ); } - function _abstractSetPToken(address, address) internal override { - revert("No pTokens are used"); - } - /** * @notice Withdraw all assets from this strategy, and transfer to the Vault. * In correct operation, this strategy should never hold any assets. @@ -309,4 +305,8 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { /// @notice Needed to receive ETH when withdrawal requests are claimed receive() external payable {} + + function _abstractSetPToken(address, address) internal pure override { + revert("No pTokens are used"); + } } From 77bf1e0d5b1faab9d6f8dca365d1b102c5250f27 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Tue, 4 Jun 2024 09:31:50 +1000 Subject: [PATCH 7/9] renamed numWithdrawals to withdrawalLength --- contracts/contracts/strategies/LidoWithdrawalStrategy.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol index 5cb8021391..a8fd75b7ff 100644 --- a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol +++ b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol @@ -139,8 +139,8 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { uint256 stETHStart = stETH.balanceOf(address(this)); require(stETHStart > 0, "No stETH to withdraw"); - uint256 numWithdrawals = (stETHStart / MaxWithdrawalAmount) + 1; - uint256[] memory amounts = new uint256[](numWithdrawals); + uint256 withdrawalLength = (stETHStart / MaxWithdrawalAmount) + 1; + uint256[] memory amounts = new uint256[](withdrawalLength); uint256 stETHRemaining = stETHStart; uint256 i = 0; @@ -159,7 +159,7 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { // Is there any stETH left except 1 wei from each request? require( - stETH.balanceOf(address(this)) <= numWithdrawals, + stETH.balanceOf(address(this)) <= withdrawalLength, "Not all stEth in withdraw queue" ); outstandingWithdrawals += stETHStart; // Single set for gas reasons From 0c320f6f11733668434e0c96f6509178b1d9e695 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Tue, 4 Jun 2024 09:51:22 +1000 Subject: [PATCH 8/9] outstandingWithdrawals now accounts for stETH dust left behind --- contracts/contracts/strategies/LidoWithdrawalStrategy.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol index a8fd75b7ff..a36a8968ac 100644 --- a/contracts/contracts/strategies/LidoWithdrawalStrategy.sol +++ b/contracts/contracts/strategies/LidoWithdrawalStrategy.sol @@ -158,11 +158,13 @@ contract LidoWithdrawalStrategy is InitializableAbstractStrategy { emit WithdrawalRequests(requestIds, amounts); // Is there any stETH left except 1 wei from each request? + // This is because stETH does not transfer all the transfer amount. + uint256 stEthDust = stETH.balanceOf(address(this)); require( - stETH.balanceOf(address(this)) <= withdrawalLength, + stEthDust <= withdrawalLength, "Not all stEth in withdraw queue" ); - outstandingWithdrawals += stETHStart; // Single set for gas reasons + outstandingWithdrawals += stETHStart - stEthDust; // This strategy claims to support WETH, so it is possible for // the vault to transfer WETH to it. This returns any deposited WETH From 84250d688f45901d2410281d411840680651b3a0 Mon Sep 17 00:00:00 2001 From: Nicholas Addison Date: Tue, 4 Jun 2024 09:52:06 +1000 Subject: [PATCH 9/9] don't format Defender Action code in dist folder --- contracts/.prettierignore | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/.prettierignore b/contracts/.prettierignore index 69d1888e21..79cdd3d840 100644 --- a/contracts/.prettierignore +++ b/contracts/.prettierignore @@ -1,2 +1,3 @@ scripts/compensation/forkDeployment.js coverage +dist \ No newline at end of file