From ad1445d6040e799c333f202fbec33a4b061e475e Mon Sep 17 00:00:00 2001 From: Nick Addison Date: Wed, 25 Sep 2024 22:21:35 +1000 Subject: [PATCH] Ensure swaps don't use withdrawal queue liquidity (#23) * Ensure swaps don't use WETH reserved for the withdrawal queue * Fund the withdrawal queue when WETH is swapped into the ARM * More refactoring of MultiLP deposit and swap not longer add to the withdrawal queue. Its only need on claim * Refactor withdrawal queue storage to optimize swap gas usage * Generated latest contract diagrams * Removed _postClaimHook as it's no longer needed * Added more Natspec * Add more Natspec * Changed the _postWithdrawHook to _postRequestRedeemHook * Generated latest contract diagrams * Minor change to constructor param --- docs/LidoFixedPriceMultiLpARMHierarchy.svg | 32 ++-- docs/LidoFixedPriceMultiLpARMSquashed.svg | 142 +++++++++--------- docs/OEthARMHierarchy.svg | 56 +++---- docs/OEthARMSquashed.svg | 66 ++++---- src/contracts/AbstractARM.sol | 10 ++ src/contracts/FixedPriceARM.sol | 18 +-- src/contracts/LidoFixedPriceMultiLpARM.sol | 52 ++++--- src/contracts/LidoLiquidityManager.sol | 4 - src/contracts/MultiLP.sol | 114 ++++++++------ src/contracts/PerformanceFee.sol | 6 +- .../LidoFixedPriceMultiLpARM/Deposit.t.sol | 2 +- .../SwapExactTokensForTokens.t.sol | 2 +- test/fork/utils/Helpers.sol | 12 +- 13 files changed, 272 insertions(+), 244 deletions(-) diff --git a/docs/LidoFixedPriceMultiLpARMHierarchy.svg b/docs/LidoFixedPriceMultiLpARMHierarchy.svg index ffb8bb9..1c83217 100644 --- a/docs/LidoFixedPriceMultiLpARMHierarchy.svg +++ b/docs/LidoFixedPriceMultiLpARMHierarchy.svg @@ -17,16 +17,16 @@ AbstractARM ../src/contracts/AbstractARM.sol - + -23 +22 OwnableOperable ../src/contracts/OwnableOperable.sol - + -0->23 +0->22 @@ -99,23 +99,23 @@ - + -26 +25 <<Abstract>> PerformanceFee ../src/contracts/PerformanceFee.sol - + -13->26 +13->25 - + -14->23 +14->22 @@ -131,22 +131,22 @@ - + -22 +21 Ownable ../src/contracts/Ownable.sol - + -23->22 +22->21 - + -26->17 +25->17 diff --git a/docs/LidoFixedPriceMultiLpARMSquashed.svg b/docs/LidoFixedPriceMultiLpARMSquashed.svg index eab636e..525a2dc 100644 --- a/docs/LidoFixedPriceMultiLpARMSquashed.svg +++ b/docs/LidoFixedPriceMultiLpARMSquashed.svg @@ -4,78 +4,82 @@ - - + + UmlClassDiagram - + 13 - -LidoFixedPriceMultiLpARM -../src/contracts/LidoFixedPriceMultiLpARM.sol - -Private: -   _gap: uint256[49] <<OwnableOperable>> -   _gap: uint256[50] <<AbstractARM>> -   _gap: uint256[47] <<MultiLP>> -   _gap: uint256[48] <<PerformanceFee>> -   _gap: uint256[49] <<LiquidityProviderControllerARM>> -   _gap: uint256[48] <<FixedPriceARM>> -   _gap: uint256[49] <<LidoLiquidityManager>> -Internal: -   OWNER_SLOT: bytes32 <<Ownable>> -   MIN_TOTAL_SUPPLY: uint256 <<MultiLP>> -   DEAD_ACCOUNT: address <<MultiLP>> -   liquidityAsset: address <<MultiLP>> -Public: -   operator: address <<OwnableOperable>> -   token0: IERC20 <<AbstractARM>> -   token1: IERC20 <<AbstractARM>> -   CLAIM_DELAY: uint256 <<MultiLP>> -   withdrawalQueueMetadata: WithdrawalQueueMetadata <<MultiLP>> -   withdrawalRequests: mapping(uint256=>WithdrawalRequest) <<MultiLP>> -   FEE_SCALE: uint256 <<PerformanceFee>> -   feeCollector: address <<PerformanceFee>> -   fee: uint16 <<PerformanceFee>> -   feesAccrued: uint112 <<PerformanceFee>> -   lastTotalAssets: uint128 <<PerformanceFee>> -   liquidityProviderController: address <<LiquidityProviderControllerARM>> -   traderate0: uint256 <<FixedPriceARM>> -   traderate1: uint256 <<FixedPriceARM>> -   MAX_PRICE_DEVIATION: uint256 <<FixedPriceARM>> -   PRICE_SCALE: uint256 <<FixedPriceARM>> -   steth: IERC20 <<LidoLiquidityManager>> -   weth: IWETH <<LidoLiquidityManager>> -   withdrawalQueue: IStETHWithdrawal <<LidoLiquidityManager>> -   outstandingEther: uint256 <<LidoLiquidityManager>> - -Internal: -    _owner(): (ownerOut: address) <<Ownable>> -    _setOwner(newOwner: address) <<Ownable>> -    _onlyOwner() <<Ownable>> -    _initOwnableOperable(_operator: address) <<OwnableOperable>> -    _setOperator(newOperator: address) <<OwnableOperable>> -    _swapExactTokensForTokens(inToken: IERC20, outToken: IERC20, amountIn: uint256, to: address): (amountOut: uint256) <<FixedPriceARM>> -    _swapTokensForExactTokens(inToken: IERC20, outToken: IERC20, amountOut: uint256, to: address): (amountIn: uint256) <<FixedPriceARM>> -    _inDeadline(deadline: uint256) <<AbstractARM>> -    _initMultiLP(_name: string, _symbol: string) <<MultiLP>> -    _preDepositHook() <<PerformanceFee>> -    _postDepositHook(assets: uint256) <<LidoFixedPriceMultiLpARM>> -    _preWithdrawHook() <<PerformanceFee>> -    _postWithdrawHook(assets: uint256) <<LidoFixedPriceMultiLpARM>> -    _addWithdrawalQueueLiquidity(): (addedClaimable: uint256) <<MultiLP>> -    _externalWithdrawQueue(): uint256 <<LidoFixedPriceMultiLpARM>> -    _initPerformanceFee(_fee: uint256, _feeCollector: address) <<PerformanceFee>> -    _calcFee() <<PerformanceFee>> -    _rawTotalAssets(): uint256 <<PerformanceFee>> -    _setFee(_fee: uint256) <<PerformanceFee>> -    _setFeeCollector(_feeCollector: address) <<PerformanceFee>> -    _initLPControllerARM(_liquidityProviderController: address) <<LiquidityProviderControllerARM>> -    _calcTransferAmount(outToken: address, amount: uint256): (transferAmount: uint256) <<LidoFixedPriceMultiLpARM>> -    _setTraderates(_traderate0: uint256, _traderate1: uint256) <<FixedPriceARM>> -    _postClaimHook(assets: uint256) <<LidoFixedPriceMultiLpARM>> + +LidoFixedPriceMultiLpARM +../src/contracts/LidoFixedPriceMultiLpARM.sol + +Private: +   _gap: uint256[49] <<OwnableOperable>> +   _gap: uint256[50] <<AbstractARM>> +   _gap: uint256[47] <<MultiLP>> +   _gap: uint256[48] <<PerformanceFee>> +   _gap: uint256[49] <<LiquidityProviderControllerARM>> +   _gap: uint256[48] <<FixedPriceARM>> +   _gap: uint256[49] <<LidoLiquidityManager>> +Internal: +   OWNER_SLOT: bytes32 <<Ownable>> +   MIN_TOTAL_SUPPLY: uint256 <<MultiLP>> +   DEAD_ACCOUNT: address <<MultiLP>> +   liquidityAsset: address <<MultiLP>> +Public: +   operator: address <<OwnableOperable>> +   token0: IERC20 <<AbstractARM>> +   token1: IERC20 <<AbstractARM>> +   CLAIM_DELAY: uint256 <<MultiLP>> +   withdrawsQueued: uint128 <<MultiLP>> +   withdrawsClaimed: uint128 <<MultiLP>> +   withdrawsClaimable: uint128 <<MultiLP>> +   nextWithdrawalIndex: uint128 <<MultiLP>> +   withdrawalRequests: mapping(uint256=>WithdrawalRequest) <<MultiLP>> +   FEE_SCALE: uint256 <<PerformanceFee>> +   feeCollector: address <<PerformanceFee>> +   fee: uint16 <<PerformanceFee>> +   feesAccrued: uint112 <<PerformanceFee>> +   lastTotalAssets: uint128 <<PerformanceFee>> +   liquidityProviderController: address <<LiquidityProviderControllerARM>> +   traderate0: uint256 <<FixedPriceARM>> +   traderate1: uint256 <<FixedPriceARM>> +   MAX_PRICE_DEVIATION: uint256 <<FixedPriceARM>> +   PRICE_SCALE: uint256 <<FixedPriceARM>> +   steth: IERC20 <<LidoLiquidityManager>> +   weth: IWETH <<LidoLiquidityManager>> +   withdrawalQueue: IStETHWithdrawal <<LidoLiquidityManager>> +   outstandingEther: uint256 <<LidoLiquidityManager>> + +Internal: +    _owner(): (ownerOut: address) <<Ownable>> +    _setOwner(newOwner: address) <<Ownable>> +    _onlyOwner() <<Ownable>> +    _initOwnableOperable(_operator: address) <<OwnableOperable>> +    _setOperator(newOperator: address) <<OwnableOperable>> +    _swapExactTokensForTokens(inToken: IERC20, outToken: IERC20, amountIn: uint256, to: address): (amountOut: uint256) <<FixedPriceARM>> +    _swapTokensForExactTokens(inToken: IERC20, outToken: IERC20, amountOut: uint256, to: address): (amountIn: uint256) <<FixedPriceARM>> +    _inDeadline(deadline: uint256) <<AbstractARM>> +    _transferAsset(asset: address, to: address, amount: uint256) <<LidoFixedPriceMultiLpARM>> +    _transferAssetFrom(asset: address, from: address, to: address, amount: uint256) <<AbstractARM>> +    _initMultiLP(_name: string, _symbol: string) <<MultiLP>> +    _preDepositHook() <<PerformanceFee>> +    _postDepositHook(assets: uint256) <<LidoFixedPriceMultiLpARM>> +    _preWithdrawHook() <<PerformanceFee>> +    _postRequestRedeemHook() <<LidoFixedPriceMultiLpARM>> +    _updateWithdrawalQueueLiquidity() <<MultiLP>> +    _liquidityAvailable(): uint256 <<MultiLP>> +    _externalWithdrawQueue(): uint256 <<LidoFixedPriceMultiLpARM>> +    _initPerformanceFee(_fee: uint256, _feeCollector: address) <<PerformanceFee>> +    _calcFee() <<PerformanceFee>> +    _rawTotalAssets(): uint256 <<PerformanceFee>> +    _setFee(_fee: uint256) <<PerformanceFee>> +    _setFeeCollector(_feeCollector: address) <<PerformanceFee>> +    _initLPControllerARM(_liquidityProviderController: address) <<LiquidityProviderControllerARM>> +    _setTraderates(_traderate0: uint256, _traderate1: uint256) <<FixedPriceARM>> External:    <<payable>> null() <<LidoLiquidityManager>>    owner(): address <<Ownable>> @@ -118,7 +122,7 @@    totalAssets(): uint256 <<LidoFixedPriceMultiLpARM>>    convertToShares(assets: uint256): (shares: uint256) <<MultiLP>>    convertToAssets(shares: uint256): (assets: uint256) <<MultiLP>> -    constructor(_stEth: address, _weth: address, _lidoWithdrawalQueue: address) <<LidoFixedPriceMultiLpARM>> +    constructor(_steth: address, _weth: address, _lidoWithdrawalQueue: address) <<LidoFixedPriceMultiLpARM>> diff --git a/docs/OEthARMHierarchy.svg b/docs/OEthARMHierarchy.svg index 2f99897..89dbae3 100644 --- a/docs/OEthARMHierarchy.svg +++ b/docs/OEthARMHierarchy.svg @@ -17,95 +17,95 @@ AbstractARM ../src/contracts/AbstractARM.sol - + -24 +22 OwnableOperable ../src/contracts/OwnableOperable.sol - + -0->24 +0->22 - + -21 +19 OethARM ../src/contracts/OethARM.sol - + -22 +20 OethLiquidityManager ../src/contracts/OethLiquidityManager.sol - + -21->22 +19->20 - + -25 +23 <<Abstract>> OwnerLP ../src/contracts/OwnerLP.sol - + -21->25 +19->23 - + -26 +24 <<Abstract>> PeggedARM ../src/contracts/PeggedARM.sol - + -21->26 +19->24 - + -22->24 +20->22 - + -23 +21 Ownable ../src/contracts/Ownable.sol - + -24->23 +22->21 - + -25->23 +23->21 - + -26->0 +24->0 diff --git a/docs/OEthARMSquashed.svg b/docs/OEthARMSquashed.svg index 516cd18..62cc57f 100644 --- a/docs/OEthARMSquashed.svg +++ b/docs/OEthARMSquashed.svg @@ -4,40 +4,42 @@ - - + + UmlClassDiagram - - + + -21 - -OethARM -../src/contracts/OethARM.sol - -Private: -   _gap: uint256[50] <<OwnableOperable>> -   _gap: uint256[50] <<AbstractARM>> -Internal: -   OWNER_SLOT: bytes32 <<Ownable>> -Public: -   operator: address <<OwnableOperable>> -   token0: IERC20 <<AbstractARM>> -   token1: IERC20 <<AbstractARM>> -   bothDirections: bool <<PeggedARM>> -   oeth: address <<OethLiquidityManager>> -   oethVault: address <<OethLiquidityManager>> - -Internal: -    _owner(): (ownerOut: address) <<Ownable>> -    _setOwner(newOwner: address) <<Ownable>> -    _onlyOwner() <<Ownable>> -    _initOwnableOperable(_operator: address) <<OwnableOperable>> -    _setOperator(newOperator: address) <<OwnableOperable>> -    _swapExactTokensForTokens(inToken: IERC20, outToken: IERC20, amountIn: uint256, to: address): (amountOut: uint256) <<PeggedARM>> -    _swapTokensForExactTokens(inToken: IERC20, outToken: IERC20, amountOut: uint256, to: address): (amountIn: uint256) <<PeggedARM>> -    _inDeadline(deadline: uint256) <<AbstractARM>> +19 + +OethARM +../src/contracts/OethARM.sol + +Private: +   _gap: uint256[49] <<OwnableOperable>> +   _gap: uint256[50] <<AbstractARM>> +Internal: +   OWNER_SLOT: bytes32 <<Ownable>> +Public: +   operator: address <<OwnableOperable>> +   token0: IERC20 <<AbstractARM>> +   token1: IERC20 <<AbstractARM>> +   bothDirections: bool <<PeggedARM>> +   oeth: address <<OethLiquidityManager>> +   oethVault: address <<OethLiquidityManager>> + +Internal: +    _owner(): (ownerOut: address) <<Ownable>> +    _setOwner(newOwner: address) <<Ownable>> +    _onlyOwner() <<Ownable>> +    _initOwnableOperable(_operator: address) <<OwnableOperable>> +    _setOperator(newOperator: address) <<OwnableOperable>> +    _swapExactTokensForTokens(inToken: IERC20, outToken: IERC20, amountIn: uint256, to: address): (amountOut: uint256) <<PeggedARM>> +    _swapTokensForExactTokens(inToken: IERC20, outToken: IERC20, amountOut: uint256, to: address): (amountIn: uint256) <<PeggedARM>> +    _inDeadline(deadline: uint256) <<AbstractARM>> +    _transferAsset(asset: address, to: address, amount: uint256) <<AbstractARM>> +    _transferAssetFrom(asset: address, from: address, to: address, amount: uint256) <<AbstractARM>>    _swap(inToken: IERC20, outToken: IERC20, amount: uint256, to: address): uint256 <<PeggedARM>>    _approvals() <<OethLiquidityManager>> External: diff --git a/src/contracts/AbstractARM.sol b/src/contracts/AbstractARM.sol index f56e200..8132261 100644 --- a/src/contracts/AbstractARM.sol +++ b/src/contracts/AbstractARM.sol @@ -154,4 +154,14 @@ abstract contract AbstractARM is OwnableOperable { function _inDeadline(uint256 deadline) internal view { require(deadline >= block.timestamp, "ARM: Deadline expired"); } + + /// @dev Hook to transfer assets out of the ARM contract + function _transferAsset(address asset, address to, uint256 amount) internal virtual { + IERC20(asset).transfer(to, amount); + } + + /// @dev Hook to transfer assets into the ARM contract + function _transferAssetFrom(address asset, address from, address to, uint256 amount) internal virtual { + IERC20(asset).transferFrom(from, to, amount); + } } diff --git a/src/contracts/FixedPriceARM.sol b/src/contracts/FixedPriceARM.sol index 1e9e60f..c6587aa 100644 --- a/src/contracts/FixedPriceARM.sol +++ b/src/contracts/FixedPriceARM.sol @@ -54,11 +54,10 @@ abstract contract FixedPriceARM is AbstractARM { amountOut = amountIn * price / PRICE_SCALE; // Transfer the input tokens from the caller to this ARM contract - inToken.transferFrom(msg.sender, address(this), amountIn); + _transferAssetFrom(address(inToken), msg.sender, address(this), amountIn); // Transfer the output tokens to the recipient - uint256 transferAmountOut = _calcTransferAmount(address(outToken), amountOut); - outToken.transfer(to, transferAmountOut); + _transferAsset(address(outToken), to, amountOut); } function _swapTokensForExactTokens(IERC20 inToken, IERC20 outToken, uint256 amountOut, address to) @@ -79,19 +78,10 @@ abstract contract FixedPriceARM is AbstractARM { amountIn = ((amountOut * PRICE_SCALE) / price) + 1; // +1 to always round in our favor // Transfer the input tokens from the caller to this ARM contract - inToken.transferFrom(msg.sender, address(this), amountIn); + _transferAssetFrom(address(inToken), msg.sender, address(this), amountIn); // Transfer the output tokens to the recipient - uint256 transferAmountOut = _calcTransferAmount(address(outToken), amountOut); - outToken.transfer(to, transferAmountOut); - } - - /** - * @notice Calculate transfer amount for outToken. - * Some tokens like stETH transfer less than the requested amount due to internal mechanics. - */ - function _calcTransferAmount(address, uint256 amount) internal view virtual returns (uint256 transferAmount) { - transferAmount = amount; + _transferAsset(address(outToken), to, amountOut); } /** diff --git a/src/contracts/LidoFixedPriceMultiLpARM.sol b/src/contracts/LidoFixedPriceMultiLpARM.sol index be7fd25..5b2a1a8 100644 --- a/src/contracts/LidoFixedPriceMultiLpARM.sol +++ b/src/contracts/LidoFixedPriceMultiLpARM.sol @@ -14,6 +14,9 @@ import {PerformanceFee} from "./PerformanceFee.sol"; /** * @title Lido (stETH) Application Redemption Manager (ARM) * @dev This implementation supports multiple Liquidity Providers (LPs) with single buy and sell prices. + * It also integrates to a LiquidityProviderController contract that caps the amount of assets a liquidity provider + * can deposit and caps the ARM's total assets. + * A performance fee is also collected on increases in the ARM's total assets. * @author Origin Protocol Inc */ contract LidoFixedPriceMultiLpARM is @@ -24,17 +27,18 @@ contract LidoFixedPriceMultiLpARM is FixedPriceARM, LidoLiquidityManager { - /// @param _stEth The address of the stETH token + /// @param _steth The address of the stETH token /// @param _weth The address of the WETH token /// @param _lidoWithdrawalQueue The address of the Lido's withdrawal queue contract - constructor(address _stEth, address _weth, address _lidoWithdrawalQueue) - AbstractARM(_stEth, _weth) + constructor(address _steth, address _weth, address _lidoWithdrawalQueue) + AbstractARM(_steth, _weth) MultiLP(_weth) FixedPriceARM() - LidoLiquidityManager(_stEth, _weth, _lidoWithdrawalQueue) + LidoLiquidityManager(_steth, _weth, _lidoWithdrawalQueue) {} /// @notice Initialize the contract. + /// The deployer that calls initialize has to approve the this ARM's proxy contract to transfer 1e12 WETH. /// @param _name The name of the liquidity provider (LP) token. /// @param _symbol The symbol of the liquidity provider (LP) token. /// @param _operator The address of the account that can request and claim Lido withdrawals. @@ -58,25 +62,28 @@ contract LidoFixedPriceMultiLpARM is } /** - * @notice Calculate transfer amount for outToken. - * Due to internal stETH mechanics required for rebasing support, - * in most cases stETH transfers are performed for the value of 1 wei less than passed to transfer method. - * Larger transfer amounts can be 2 wei less. + * @dev Due to internal stETH mechanics required for rebasing support, in most cases stETH transfers are performed + * for the value of 1 wei less than passed to transfer method. Larger transfer amounts can be 2 wei less. + * + * The MultiLP implementation ensures any WETH reserved for the withdrawal queue is not used in swaps from stETH to WETH. */ - function _calcTransferAmount(address outToken, uint256 amount) - internal - view - override - returns (uint256 transferAmount) - { + function _transferAsset(address asset, address to, uint256 amount) internal override(AbstractARM, MultiLP) { // Add 2 wei if transferring stETH - transferAmount = outToken == address(token0) ? amount + 2 : amount; + uint256 transferAmount = asset == address(token0) ? amount + 2 : amount; + + MultiLP._transferAsset(asset, to, transferAmount); } + /** + * @dev Calculates the amount of stETH in the Lido Withdrawal Queue. + */ function _externalWithdrawQueue() internal view override(MultiLP, LidoLiquidityManager) returns (uint256) { return LidoLiquidityManager._externalWithdrawQueue(); } + /** + * @dev Is called after assets are transferred to the ARM in the `deposit` method. + */ function _postDepositHook(uint256 assets) internal override(MultiLP, LiquidityProviderControllerARM, PerformanceFee) @@ -88,15 +95,18 @@ contract LidoFixedPriceMultiLpARM is LiquidityProviderControllerARM._postDepositHook(assets); } - function _postWithdrawHook(uint256 assets) internal override(MultiLP, PerformanceFee) { + /** + * @dev Is called after the performance fee is accrued in the `requestRedeem` method. + */ + function _postRequestRedeemHook() internal override(MultiLP, PerformanceFee) { // Store the new total assets after the withdrawal and performance fee accrued - PerformanceFee._postWithdrawHook(assets); - } - - function _postClaimHook(uint256 assets) internal override { - // do nothing + PerformanceFee._postRequestRedeemHook(); } + /** + * @notice The total amount of assets in the ARM and Lido withdrawal queue, + * less the WETH reserved for the ARM's withdrawal queue and accrued fees. + */ function totalAssets() public view override(MultiLP, PerformanceFee) returns (uint256) { return PerformanceFee.totalAssets(); } diff --git a/src/contracts/LidoLiquidityManager.sol b/src/contracts/LidoLiquidityManager.sol index 76235b0..311438b 100644 --- a/src/contracts/LidoLiquidityManager.sol +++ b/src/contracts/LidoLiquidityManager.sol @@ -72,12 +72,8 @@ abstract contract LidoLiquidityManager is OwnableOperable { // Wrap all the received ETH to WETH. (bool success,) = address(weth).call{value: etherAfter}(new bytes(0)); require(success, "ARM: ETH transfer failed"); - - _postClaimHook(etherAfter); } - function _postClaimHook(uint256 assets) internal virtual; - function _externalWithdrawQueue() internal view virtual returns (uint256 assets) { return outstandingEther; } diff --git a/src/contracts/MultiLP.sol b/src/contracts/MultiLP.sol index 69a9fe2..651927e 100644 --- a/src/contracts/MultiLP.sol +++ b/src/contracts/MultiLP.sol @@ -21,24 +21,14 @@ abstract contract MultiLP is AbstractARM, ERC20Upgradeable { /// @notice The address of the asset that is used to add and remove liquidity. eg WETH address internal immutable liquidityAsset; - struct WithdrawalQueueMetadata { - // cumulative total of all withdrawal requests included the ones that have already been claimed - uint128 queued; - // cumulative total of all the requests that can be claimed including the ones that have already been claimed - uint128 claimable; - // total of all the requests that have been claimed - uint128 claimed; - // index of the next withdrawal request starting at 0 - uint128 nextWithdrawalIndex; - } - - /// @notice Global metadata for the withdrawal queue including: - /// queued - cumulative total of all withdrawal requests included the ones that have already been claimed - /// claimable - cumulative total of all the requests that can be claimed including the ones already claimed - /// claimed - total of all the requests that have been claimed - /// nextWithdrawalIndex - index of the next withdrawal request starting at 0 - // slither-disable-next-line uninitialized-state - WithdrawalQueueMetadata public withdrawalQueueMetadata; + /// @notice cumulative total of all withdrawal requests included the ones that have already been claimed + uint128 public withdrawsQueued; + /// @notice total of all the withdrawal requests that have been claimed + uint128 public withdrawsClaimed; + /// @notice cumulative total of all the withdrawal requests that can be claimed including the ones already claimed + uint128 public withdrawsClaimable; + /// @notice index of the next withdrawal request starting at 0 + uint128 public nextWithdrawalIndex; struct WithdrawalRequest { address withdrawer; @@ -101,9 +91,6 @@ abstract contract MultiLP is AbstractARM, ERC20Upgradeable { // mint shares _mint(msg.sender, shares); - // Add WETH to the withdrawal queue if the queue has a shortfall - _addWithdrawalQueueLiquidity(); - _postDepositHook(assets); } @@ -127,55 +114,54 @@ abstract contract MultiLP is AbstractARM, ERC20Upgradeable { // Calculate the amount of assets to transfer to the redeemer assets = convertToAssets(shares); - requestId = withdrawalQueueMetadata.nextWithdrawalIndex; - uint256 queued = withdrawalQueueMetadata.queued + assets; - uint256 claimTimestamp = block.timestamp + CLAIM_DELAY; + requestId = nextWithdrawalIndex; + uint128 queued = SafeCast.toUint128(withdrawsQueued + assets); + uint40 claimTimestamp = uint40(block.timestamp + CLAIM_DELAY); // Store the next withdrawal request - withdrawalQueueMetadata.nextWithdrawalIndex = SafeCast.toUint128(requestId + 1); + nextWithdrawalIndex = SafeCast.toUint128(requestId + 1); // Store the updated queued amount which reserves WETH in the withdrawal queue - withdrawalQueueMetadata.queued = SafeCast.toUint128(queued); + withdrawsQueued = queued; // Store requests withdrawalRequests[requestId] = WithdrawalRequest({ withdrawer: msg.sender, claimed: false, - claimTimestamp: uint40(claimTimestamp), + claimTimestamp: claimTimestamp, assets: SafeCast.toUint128(assets), - queued: SafeCast.toUint128(queued) + queued: queued }); // burn redeemer's shares _burn(msg.sender, shares); - _postWithdrawHook(assets); + _postRequestRedeemHook(); emit RedeemRequested(msg.sender, requestId, assets, queued, claimTimestamp); } function _preWithdrawHook() internal virtual; - function _postWithdrawHook(uint256 assets) internal virtual; + function _postRequestRedeemHook() internal virtual; /// @notice Claim liquidity assets from a previous withdrawal request after the claim delay has passed /// @param requestId The index of the withdrawal request /// @return assets The amount of liquidity assets that were transferred to the redeemer function claimRedeem(uint256 requestId) external returns (uint256 assets) { - // Update the ARM's withdrawal queue details - _addWithdrawalQueueLiquidity(); + // Update the ARM's withdrawal queue's claimable amount + _updateWithdrawalQueueLiquidity(); // Load the structs from storage into memory WithdrawalRequest memory request = withdrawalRequests[requestId]; - WithdrawalQueueMetadata memory queue = withdrawalQueueMetadata; require(request.claimTimestamp <= block.timestamp, "Claim delay not met"); // If there isn't enough reserved liquidity in the queue to claim - require(request.queued <= queue.claimable, "Queue pending liquidity"); + require(request.queued <= withdrawsClaimable, "Queue pending liquidity"); require(request.withdrawer == msg.sender, "Not requester"); require(request.claimed == false, "Already claimed"); // Store the request as claimed withdrawalRequests[requestId].claimed = true; // Store the updated claimed amount - withdrawalQueueMetadata.claimed = queue.claimed + request.assets; + withdrawsClaimed += request.assets; assets = request.assets; @@ -185,37 +171,65 @@ abstract contract MultiLP is AbstractARM, ERC20Upgradeable { IERC20(liquidityAsset).transfer(msg.sender, assets); } - /// @dev Adds liquidity to the withdrawal queue if there is a funding shortfall. - function _addWithdrawalQueueLiquidity() internal returns (uint256 addedClaimable) { - WithdrawalQueueMetadata memory queue = withdrawalQueueMetadata; + /// @dev Updates the claimable amount in the ARM's withdrawal queue. + /// That's the amount that is used to check if a request can be claimed or not. + function _updateWithdrawalQueueLiquidity() internal { + // Load the claimable amount from storage into memory + uint256 withdrawsClaimableMem = withdrawsClaimable; // Check if the claimable amount is less than the queued amount - uint256 queueShortfall = queue.queued - queue.claimable; + uint256 queueShortfall = withdrawsQueued - withdrawsClaimableMem; - // No need to do anything is the withdrawal queue is full funded + // No need to do anything is the withdrawal queue is fully funded if (queueShortfall == 0) { - return 0; + return; } uint256 liquidityBalance = IERC20(liquidityAsset).balanceOf(address(this)); // Of the claimable withdrawal requests, how much is unclaimed? // That is, the amount of the liquidity assets that is currently allocated for the withdrawal queue - uint256 allocatedLiquidity = queue.claimable - queue.claimed; + uint256 allocatedLiquidity = withdrawsClaimableMem - withdrawsClaimed; // If there is no unallocated liquidity assets then there is nothing to add to the queue if (liquidityBalance <= allocatedLiquidity) { - return 0; + return; } uint256 unallocatedLiquidity = liquidityBalance - allocatedLiquidity; // the new claimable amount is the smaller of the queue shortfall or unallocated weth - addedClaimable = queueShortfall < unallocatedLiquidity ? queueShortfall : unallocatedLiquidity; - uint256 newClaimable = queue.claimable + addedClaimable; + uint256 addedClaimable = queueShortfall < unallocatedLiquidity ? queueShortfall : unallocatedLiquidity; // Store the new claimable amount back to storage - withdrawalQueueMetadata.claimable = SafeCast.toUint128(newClaimable); + withdrawsClaimable = SafeCast.toUint128(withdrawsClaimableMem + addedClaimable); + } + + /// @dev Calculate how much of the liquidity asset in the ARM is not reserved for the withdrawal queue. + // That is, it is available to be swapped. + function _liquidityAvailable() internal view returns (uint256) { + // The amount of WETH that is still to be claimed in the withdrawal queue + uint256 outstandingWithdrawals = withdrawsQueued - withdrawsClaimed; + + // The amount of the liquidity asset is in the ARM + uint256 liquidityBalance = IERC20(liquidityAsset).balanceOf(address(this)); + + // If there is not enough liquidity assets in the ARM to cover the outstanding withdrawals + if (liquidityBalance <= outstandingWithdrawals) { + return 0; + } + + return liquidityBalance - outstandingWithdrawals; + } + + /// @dev Ensure any liquidity assets reserved for the withdrawal queue are not used + /// in swaps that send liquidity assets out of the ARM + function _transferAsset(address asset, address to, uint256 amount) internal virtual override { + if (asset == liquidityAsset) { + require(amount <= _liquidityAvailable(), "ARM: Insufficient liquidity"); + } + + IERC20(asset).transfer(to, amount); } /// @notice The total amount of assets in the ARM and external withdrawal queue, @@ -224,16 +238,18 @@ abstract contract MultiLP is AbstractARM, ERC20Upgradeable { // Get the assets in the ARM and external withdrawal queue assets = token0.balanceOf(address(this)) + token1.balanceOf(address(this)) + _externalWithdrawQueue(); - WithdrawalQueueMetadata memory queue = withdrawalQueueMetadata; + // Load the queue metadata from storage into memory + uint256 queuedMem = withdrawsQueued; + uint256 claimedMem = withdrawsClaimed; // If the ARM becomes insolvent enough that the total value in the ARM and external withdrawal queue // is less than the outstanding withdrawals. - if (assets + queue.claimed < queue.queued) { + if (assets + claimedMem < queuedMem) { return 0; } // Need to remove the liquidity assets that have been reserved for the withdrawal queue - return assets + queue.claimed - queue.queued; + return assets + claimedMem - queuedMem; } /// @notice Calculates the amount of shares for a given amount of liquidity assets diff --git a/src/contracts/PerformanceFee.sol b/src/contracts/PerformanceFee.sol index 351ddc1..32abc8c 100644 --- a/src/contracts/PerformanceFee.sol +++ b/src/contracts/PerformanceFee.sol @@ -61,8 +61,8 @@ abstract contract PerformanceFee is MultiLP { lastTotalAssets = SafeCast.toUint128(_rawTotalAssets()); } - /// @dev Save the new total assets after the withdrawal and performance fee accrued - function _postWithdrawHook(uint256) internal virtual override { + /// @dev Save the new total assets after the requestRedeem and performance fee accrued + function _postRequestRedeemHook() internal virtual override { lastTotalAssets = SafeCast.toUint128(_rawTotalAssets()); } @@ -87,6 +87,8 @@ abstract contract PerformanceFee is MultiLP { emit FeeCalculated(newFeesAccrued, assetIncrease); } + /// @notice The total amount of assets in the ARM and external withdrawal queue, + /// less the liquidity assets reserved for the ARM's withdrawal queue and accrued fees. function totalAssets() public view virtual override returns (uint256) { uint256 totalAssetsBeforeFees = _rawTotalAssets(); diff --git a/test/fork/LidoFixedPriceMultiLpARM/Deposit.t.sol b/test/fork/LidoFixedPriceMultiLpARM/Deposit.t.sol index 13d0bbf..3878c93 100644 --- a/test/fork/LidoFixedPriceMultiLpARM/Deposit.t.sol +++ b/test/fork/LidoFixedPriceMultiLpARM/Deposit.t.sol @@ -371,7 +371,7 @@ contract Fork_Concrete_LidoFixedPriceMultiLpARM_Deposit_Test_ is Fork_Shared_Tes assertEq(lidoFixedPriceMulltiLpARM.totalAssets(), MIN_TOTAL_SUPPLY + amount, "total assets after"); assertEq(liquidityProviderController.liquidityProviderCaps(alice), DEFAULT_AMOUNT * 3, "alice cap after"); // All the caps are used // withdrawal request is now claimable - assertEqQueueMetadata(DEFAULT_AMOUNT, DEFAULT_AMOUNT, 0, 1); + assertEqQueueMetadata(DEFAULT_AMOUNT, 0, 0, 1); assertEq(shares, amount); // No perfs, so 1 ether * totalSupply (1e18 + 1e12) / totalAssets (1e18 + 1e12) = 1 ether } } diff --git a/test/fork/LidoFixedPriceMultiLpARM/SwapExactTokensForTokens.t.sol b/test/fork/LidoFixedPriceMultiLpARM/SwapExactTokensForTokens.t.sol index c2fd61e..c920476 100644 --- a/test/fork/LidoFixedPriceMultiLpARM/SwapExactTokensForTokens.t.sol +++ b/test/fork/LidoFixedPriceMultiLpARM/SwapExactTokensForTokens.t.sol @@ -116,7 +116,7 @@ contract Fork_Concrete_LidoFixedPriceMultiLpARM_SwapExactTokensForTokens_Test is initialBalance = weth.balanceOf(address(lidoFixedPriceMulltiLpARM)); deal(address(steth), address(this), initialBalance * 2); - vm.expectRevert(); // Lido error + vm.expectRevert("ARM: Insufficient liquidity"); lidoFixedPriceMulltiLpARM.swapExactTokensForTokens( steth, // inToken weth, // outToken diff --git a/test/fork/utils/Helpers.sol b/test/fork/utils/Helpers.sol index 29c6bb4..448aa2a 100644 --- a/test/fork/utils/Helpers.sol +++ b/test/fork/utils/Helpers.sol @@ -33,19 +33,17 @@ abstract contract Helpers is Base_Test_ { } } - /// @notice Asserts the equality bewteen value of `withdrawalQueueMetadata()` and the expected values. + /// @notice Asserts the equality between value of `withdrawalQueueMetadata()` and the expected values. function assertEqQueueMetadata( uint256 expectedQueued, uint256 expectedClaimable, uint256 expectedClaimed, uint256 expectedNextIndex ) public view { - (uint256 queued, uint256 claimable, uint256 claimed, uint256 nextWithdrawalIndex) = - lidoFixedPriceMulltiLpARM.withdrawalQueueMetadata(); - assertEq(queued, expectedQueued, "metadata queued"); - assertEq(claimable, expectedClaimable, "metadata claimable"); - assertEq(claimed, expectedClaimed, "metadata claimed"); - assertEq(nextWithdrawalIndex, expectedNextIndex, "metadata nextWithdrawalIndex"); + assertEq(lidoFixedPriceMulltiLpARM.withdrawsQueued(), expectedQueued, "metadata queued"); + assertEq(lidoFixedPriceMulltiLpARM.withdrawsClaimable(), expectedClaimable, "metadata claimable"); + assertEq(lidoFixedPriceMulltiLpARM.withdrawsClaimed(), expectedClaimed, "metadata claimed"); + assertEq(lidoFixedPriceMulltiLpARM.nextWithdrawalIndex(), expectedNextIndex, "metadata nextWithdrawalIndex"); } /// @notice Asserts the equality bewteen value of `withdrawalRequests()` and the expected values.