From d356cc9e2e999bca4a66461717a44c5698fa52e2 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Fri, 6 Dec 2024 13:03:10 +0900 Subject: [PATCH 01/42] init. Instant Withdrawal via Buffer --- lib/BucketLimiter.sol | 5 + src/EtherFiWithdrawalBuffer.sol | 224 ++++++++++++++++++++++++++++++++ src/LiquidityPool.sol | 11 +- 3 files changed, 239 insertions(+), 1 deletion(-) create mode 100644 src/EtherFiWithdrawalBuffer.sol diff --git a/lib/BucketLimiter.sol b/lib/BucketLimiter.sol index eb410f78e..83f749156 100644 --- a/lib/BucketLimiter.sol +++ b/lib/BucketLimiter.sol @@ -71,6 +71,11 @@ library BucketLimiter { return limit.remaining >= amount; } + function consumable(Limit memory limit) external view returns (uint64) { + _refill(limit); + return limit.remaining; + } + /* * Consumes the given amount from the bucket, if there is sufficient capacity, and returns * whether the bucket had enough remaining capacity to consume the amount. diff --git a/src/EtherFiWithdrawalBuffer.sol b/src/EtherFiWithdrawalBuffer.sol new file mode 100644 index 000000000..95bf930ce --- /dev/null +++ b/src/EtherFiWithdrawalBuffer.sol @@ -0,0 +1,224 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.13; + +import "@openzeppelin/contracts/utils/math/SafeCast.sol"; +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/extensions/draft-IERC20Permit.sol"; +import "@openzeppelin-upgradeable/contracts/token/ERC20/IERC20Upgradeable.sol"; +import "@openzeppelin-upgradeable/contracts/proxy/utils/Initializable.sol"; +import "@openzeppelin-upgradeable/contracts/proxy/utils/UUPSUpgradeable.sol"; +import "@openzeppelin-upgradeable/contracts/security/ReentrancyGuardUpgradeable.sol"; +import "@openzeppelin-upgradeable/contracts/access/OwnableUpgradeable.sol"; +import "@openzeppelin-upgradeable/contracts/security/PausableUpgradeable.sol"; + +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import "@openzeppelin/contracts/utils/math/Math.sol"; + +import "./interfaces/ILiquidityPool.sol"; +import "./interfaces/IeETH.sol"; +import "./interfaces/IWeETH.sol"; + +import "lib/BucketLimiter.sol"; + + +/* + The contract allows instant redemption of eETH and weETH tokens to ETH with an exit fee. + - It has the exit fee as a percentage of the total amount redeemed. + - It has a rate limiter to limit the total amount that can be redeemed in a given time period. +*/ +contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable, UUPSUpgradeable { + using SafeERC20 for IERC20; + using Math for uint256; + + uint256 private constant BUCKEt_UNIT_SCALE = 1e12; + uint256 private constant BASIS_POINT_SCALE = 1e4; + address public immutable feeReceiver; + IeETH public immutable eEth; + IWeETH public immutable weEth; + ILiquidityPool public immutable liquidityPool; + + BucketLimiter.Limit public limit; + uint256 public exitFeeBasisPoints; + uint256 public lowWatermarkInBpsOfTvl; // bps of TVL + + receive() external payable {} + + /// @custom:oz-upgrades-unsafe-allow constructor + constructor(address _liquidityPool, address _eEth, address _weEth, address _feeReceiver) { + require(address(liquidityPool) == address(0) && address(eEth) == address(0) && address(feeReceiver) == address(0), "EtherFiWithdrawalBuffer: Cannot initialize twice"); + + feeReceiver = _feeReceiver; + liquidityPool = ILiquidityPool(payable(_liquidityPool)); + eEth = IeETH(_eEth); + weEth = IWeETH(_weEth); + + _disableInitializers(); + } + + function initialize(uint256 _exitFeeBasisPoints, uint256 _lowWatermarkInBpsOfTvl) external initializer { + __Ownable_init(); + __UUPSUpgradeable_init(); + __Pausable_init(); + __ReentrancyGuard_init(); + + limit = BucketLimiter.create(0, 0); + exitFeeBasisPoints = _exitFeeBasisPoints; + lowWatermarkInBpsOfTvl = _lowWatermarkInBpsOfTvl; + } + + /** + * @notice Redeems eETH for ETH. + * @param eEthAmount The amount of eETH to redeem after the exit fee. + * @param receiver The address to receive the redeemed ETH. + * @param owner The address of the owner of the eETH. + * @return The amount of ETH sent to the receiver and the exit fee amount. + */ + function redeemEEth(uint256 eEthAmount, address receiver, address owner) public whenNotPaused nonReentrant returns (uint256, uint256) { + uint256 eEthShares = liquidityPool.sharesForWithdrawalAmount(eEthAmount); + require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); + require(eEthShares <= eEth.shares(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); + + uint256 beforeEEthAmount = eEth.balanceOf(address(this)); + IERC20(address(eEth)).safeTransferFrom(owner, address(this), eEthAmount); + uint256 afterEEthAmount = eEth.balanceOf(address(this)); + + uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; + return _redeem(transferredEEthAmount, receiver); + } + + /** + * @notice Redeems weETH for ETH. + * @param weEthAmount The amount of weETH to redeem after the exit fee. + * @param receiver The address to receive the redeemed ETH. + * @param owner The address of the owner of the weETH. + * @return The amount of ETH sent to the receiver and the exit fee amount. + */ + function redeemWeEth(uint256 weEthAmount, address receiver, address owner) public whenNotPaused nonReentrant returns (uint256, uint256) { + uint256 eEthShares = weEthAmount; + uint256 eEthAmount = liquidityPool.amountForShare(eEthShares); + require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); + require(weEthAmount <= weEth.balanceOf(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); + + uint256 beforeEEthAmount = eEth.balanceOf(address(this)); + IERC20(address(weEth)).safeTransferFrom(owner, address(this), weEthAmount); + weEth.unwrap(weEthAmount); + uint256 afterEEthAmount = eEth.balanceOf(address(this)); + + uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; + return _redeem(transferredEEthAmount, receiver); + } + + + /** + * @notice Redeems ETH. + * @param ethAmount The amount of ETH to redeem after the exit fee. + * @param receiver The address to receive the redeemed ETH. + * @return The amount of ETH sent to the receiver and the exit fee amount. + */ + function _redeem(uint256 ethAmount, address receiver) internal returns (uint256, uint256) { + _updateRateLimit(ethAmount); + + uint256 ethFeeAmount = _feeOnTotal(ethAmount, exitFeeBasisPoints); + uint256 ethToReceiver = ethAmount - ethFeeAmount; + + liquidityPool.withdraw(msg.sender, ethAmount); + + uint256 prevBalance = address(this).balance; + payable(feeReceiver).transfer(ethFeeAmount); + payable(receiver).transfer(ethToReceiver); + uint256 ethSent = address(this).balance - prevBalance; + require(ethSent == ethAmount, "EtherFiWithdrawalBuffer: Transfer failed"); + + return (ethToReceiver, ethFeeAmount); + } + + /** + * @dev if the contract has less than the low watermark, it will not allow any instant redemption. + */ + function lowWatermarkInETH() public view returns (uint256) { + return liquidityPool.getTotalPooledEther().mulDiv(lowWatermarkInBpsOfTvl, BASIS_POINT_SCALE); + } + + /** + * @dev Returns the total amount that can be redeemed. + */ + function totalRedeemableAmount() external view returns (uint256) { + uint64 consumableBucketUnits = BucketLimiter.consumable(limit); + uint256 consumableAmount = _convertBucketUnitToAmount(consumableBucketUnits); + return consumableAmount; + } + + /** + * @dev Returns whether the given amount can be redeemed. + * @param amount The ETH or eETH amount to check. + */ + function canRedeem(uint256 amount) public view returns (bool) { + if (address(liquidityPool).balance < lowWatermarkInETH()) { + return false; + } + uint64 bucketUnit = _convertSharesToBucketUnit(amount, Math.Rounding.Up); + bool consumable = BucketLimiter.canConsume(limit, bucketUnit); + return consumable; + } + + /** + * @dev Sets the maximum size of the bucket that can be consumed in a given time period. + * @param capacity The capacity of the bucket. + */ + function setCapacity(uint256 capacity) external onlyOwner { + // max capacity = max(uint64) * 1e12 ~= 16 * 1e18 * 1e12 = 16 * 1e12 ether, which is practically enough + uint64 bucketUnit = _convertSharesToBucketUnit(capacity, Math.Rounding.Down); + BucketLimiter.setCapacity(limit, bucketUnit); + } + + /** + * @dev Sets the rate at which the bucket is refilled per second. + * @param refillRate The rate at which the bucket is refilled per second. + */ + function setRefillRatePerSecond(uint256 refillRate) external onlyOwner { + // max refillRate = max(uint64) * 1e12 ~= 16 * 1e18 * 1e12 = 16 * 1e12 ether per second, which is practically enough + uint64 bucketUnit = _convertSharesToBucketUnit(refillRate, Math.Rounding.Down); + BucketLimiter.setRefillRate(limit, bucketUnit); + } + + /** + * @dev Sets the exit fee. + * @param _exitFeeBasisPoints The exit fee. + */ + function setExitFeeBasisPoints(uint256 _exitFeeBasisPoints) external onlyOwner { + exitFeeBasisPoints = _exitFeeBasisPoints; + } + + function _updateRateLimit(uint256 shares) internal { + uint64 bucketUnit = _convertSharesToBucketUnit(shares, Math.Rounding.Up); + require(BucketLimiter.consume(limit, bucketUnit), "BucketRateLimiter: rate limit exceeded"); + } + + function _convertSharesToBucketUnit(uint256 shares, Math.Rounding rounding) internal pure returns (uint64) { + return (rounding == Math.Rounding.Up) ? SafeCast.toUint64((shares + BUCKEt_UNIT_SCALE - 1) / BUCKEt_UNIT_SCALE) : SafeCast.toUint64(shares / BUCKEt_UNIT_SCALE); + } + + function _convertBucketUnitToAmount(uint64 bucketUnit) internal pure returns (uint256) { + return bucketUnit * BUCKEt_UNIT_SCALE; + } + + /** + * @dev Preview taking an exit fee on redeem. See {IERC4626-previewRedeem}. + */ + // redeemable amount after exit fee + function previewRedeem(uint256 shares) public view returns (uint256) { + uint256 amountInEth = liquidityPool.amountForShare(shares); + return amountInEth - _feeOnTotal(amountInEth, exitFeeBasisPoints); + } + + /** + * @dev Calculates the fee part of an amount `assets` that already includes fees. + * Used in {IERC4626-redeem}. + */ + function _feeOnTotal(uint256 assets, uint256 feeBasisPoints) internal pure virtual returns (uint256) { + return assets.mulDiv(feeBasisPoints, feeBasisPoints + BASIS_POINT_SCALE, Math.Rounding.Up); + } + + function _authorizeUpgrade(address newImplementation) internal override onlyOwner {} + +} \ No newline at end of file diff --git a/src/LiquidityPool.sol b/src/LiquidityPool.sol index 93e032abf..28ee11fdd 100644 --- a/src/LiquidityPool.sol +++ b/src/LiquidityPool.sol @@ -23,6 +23,9 @@ import "./interfaces/IEtherFiAdmin.sol"; import "./interfaces/IAuctionManager.sol"; import "./interfaces/ILiquifier.sol"; +import "./EtherFiWithdrawalBuffer.sol"; + + contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, ILiquidityPool { //-------------------------------------------------------------------------------------- //--------------------------------- STATE-VARIABLES ---------------------------------- @@ -69,6 +72,8 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL bool private isLpBnftHolder; + EtherFiWithdrawalBuffer public etherFiWithdrawalBuffer; + //-------------------------------------------------------------------------------------- //------------------------------------- EVENTS --------------------------------------- //-------------------------------------------------------------------------------------- @@ -139,6 +144,10 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL liquifier = ILiquifier(_liquifier); } + function initializeOnUpgradeWithWithdrawalBuffer(address _withdrawalBuffer) external onlyOwner { + etherFiWithdrawalBuffer = EtherFiWithdrawalBuffer(payable(_withdrawalBuffer)); + } + // Used by eETH staking flow function deposit() external payable returns (uint256) { return deposit(address(0)); @@ -179,7 +188,7 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL /// it returns the amount of shares burned function withdraw(address _recipient, uint256 _amount) external whenNotPaused returns (uint256) { uint256 share = sharesForWithdrawalAmount(_amount); - require(msg.sender == address(withdrawRequestNFT) || msg.sender == address(membershipManager), "Incorrect Caller"); + require(msg.sender == address(withdrawRequestNFT) || msg.sender == address(membershipManager) || msg.sender == address(etherFiWithdrawalBuffer), "Incorrect Caller"); if (totalValueInLp < _amount || (msg.sender == address(withdrawRequestNFT) && ethAmountLockedForWithdrawal < _amount) || eETH.balanceOf(msg.sender) < _amount) revert InsufficientLiquidity(); if (_amount > type(uint128).max || _amount == 0 || share == 0) revert InvalidAmount(); From 82fab2eabebce87f7f53058e4d040b069d7dc0a2 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Mon, 9 Dec 2024 17:36:21 +0900 Subject: [PATCH 02/42] implemented instant fee mechanism, handling of the implicit fee --- src/EtherFiWithdrawalBuffer.sol | 89 +++++++++++------- src/WithdrawRequestNFT.sol | 79 +++++++++++++--- test/EtherFiWithdrawalBuffer.t.sol | 145 +++++++++++++++++++++++++++++ test/TestSetup.sol | 12 ++- test/WithdrawRequestNFT.t.sol | 18 ++-- 5 files changed, 288 insertions(+), 55 deletions(-) create mode 100644 test/EtherFiWithdrawalBuffer.t.sol diff --git a/src/EtherFiWithdrawalBuffer.sol b/src/EtherFiWithdrawalBuffer.sol index 95bf930ce..811d8d149 100644 --- a/src/EtherFiWithdrawalBuffer.sol +++ b/src/EtherFiWithdrawalBuffer.sol @@ -30,24 +30,25 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU using SafeERC20 for IERC20; using Math for uint256; - uint256 private constant BUCKEt_UNIT_SCALE = 1e12; + uint256 private constant BUCKET_UNIT_SCALE = 1e12; uint256 private constant BASIS_POINT_SCALE = 1e4; - address public immutable feeReceiver; + address public immutable treasury; IeETH public immutable eEth; IWeETH public immutable weEth; ILiquidityPool public immutable liquidityPool; BucketLimiter.Limit public limit; - uint256 public exitFeeBasisPoints; - uint256 public lowWatermarkInBpsOfTvl; // bps of TVL + uint16 public exitFeeSplitToTreasuryInBps; + uint16 public exitFeeInBps; + uint16 public lowWatermarkInBpsOfTvl; // bps of TVL receive() external payable {} /// @custom:oz-upgrades-unsafe-allow constructor - constructor(address _liquidityPool, address _eEth, address _weEth, address _feeReceiver) { - require(address(liquidityPool) == address(0) && address(eEth) == address(0) && address(feeReceiver) == address(0), "EtherFiWithdrawalBuffer: Cannot initialize twice"); + constructor(address _liquidityPool, address _eEth, address _weEth, address _treasury) { + require(address(liquidityPool) == address(0) && address(eEth) == address(0) && address(treasury) == address(0), "EtherFiWithdrawalBuffer: Cannot initialize twice"); - feeReceiver = _feeReceiver; + treasury = _treasury; liquidityPool = ILiquidityPool(payable(_liquidityPool)); eEth = IeETH(_eEth); weEth = IWeETH(_weEth); @@ -55,14 +56,15 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU _disableInitializers(); } - function initialize(uint256 _exitFeeBasisPoints, uint256 _lowWatermarkInBpsOfTvl) external initializer { + function initialize(uint16 _exitFeeSplitToTreasuryInBps, uint16 _exitFeeInBps, uint16 _lowWatermarkInBpsOfTvl) external initializer { __Ownable_init(); __UUPSUpgradeable_init(); __Pausable_init(); __ReentrancyGuard_init(); limit = BucketLimiter.create(0, 0); - exitFeeBasisPoints = _exitFeeBasisPoints; + exitFeeSplitToTreasuryInBps = _exitFeeSplitToTreasuryInBps; + exitFeeInBps = _exitFeeInBps; lowWatermarkInBpsOfTvl = _lowWatermarkInBpsOfTvl; } @@ -74,9 +76,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @return The amount of ETH sent to the receiver and the exit fee amount. */ function redeemEEth(uint256 eEthAmount, address receiver, address owner) public whenNotPaused nonReentrant returns (uint256, uint256) { - uint256 eEthShares = liquidityPool.sharesForWithdrawalAmount(eEthAmount); require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); - require(eEthShares <= eEth.shares(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); + require(eEthAmount <= eEth.balanceOf(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); uint256 beforeEEthAmount = eEth.balanceOf(address(this)); IERC20(address(eEth)).safeTransferFrom(owner, address(this), eEthAmount); @@ -118,18 +119,32 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU function _redeem(uint256 ethAmount, address receiver) internal returns (uint256, uint256) { _updateRateLimit(ethAmount); - uint256 ethFeeAmount = _feeOnTotal(ethAmount, exitFeeBasisPoints); - uint256 ethToReceiver = ethAmount - ethFeeAmount; - - liquidityPool.withdraw(msg.sender, ethAmount); + uint256 ethShares = liquidityPool.sharesForAmount(ethAmount); + uint256 ethShareToReceiver = ethShares.mulDiv(BASIS_POINT_SCALE - exitFeeInBps, BASIS_POINT_SCALE); + uint256 eEthAmountToReceiver = liquidityPool.amountForShare(ethShareToReceiver); + uint256 prevLpBalance = address(liquidityPool).balance; uint256 prevBalance = address(this).balance; - payable(feeReceiver).transfer(ethFeeAmount); - payable(receiver).transfer(ethToReceiver); - uint256 ethSent = address(this).balance - prevBalance; - require(ethSent == ethAmount, "EtherFiWithdrawalBuffer: Transfer failed"); + uint256 burnedShares = (eEthAmountToReceiver > 0) ? liquidityPool.withdraw(address(this), eEthAmountToReceiver) : 0; + uint256 ethReceived = address(this).balance - prevBalance; + + uint256 ethShareFee = ethShares - burnedShares; + uint256 eEthAmountFee = liquidityPool.amountForShare(ethShareFee); + uint256 feeShareToTreasury = ethShareFee.mulDiv(exitFeeSplitToTreasuryInBps, BASIS_POINT_SCALE); + uint256 eEthFeeAmountToTreasury = liquidityPool.amountForShare(feeShareToTreasury); + uint256 feeShareToStakers = ethShareFee - feeShareToTreasury; + + // To Stakers by burning shares + eEth.burnShares(address(this), liquidityPool.sharesForAmount(feeShareToStakers)); + + // To Treasury by transferring eETH + IERC20(address(eEth)).safeTransfer(treasury, eEthFeeAmountToTreasury); + + // To Receiver by transferring ETH + payable(receiver).transfer(ethReceived); + require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiWithdrawalBuffer: Transfer failed"); - return (ethToReceiver, ethFeeAmount); + return (ethReceived, eEthAmountFee); } /** @@ -143,6 +158,9 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @dev Returns the total amount that can be redeemed. */ function totalRedeemableAmount() external view returns (uint256) { + if (address(liquidityPool).balance < lowWatermarkInETH()) { + return 0; + } uint64 consumableBucketUnits = BucketLimiter.consumable(limit); uint256 consumableAmount = _convertBucketUnitToAmount(consumableBucketUnits); return consumableAmount; @@ -183,10 +201,21 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU /** * @dev Sets the exit fee. - * @param _exitFeeBasisPoints The exit fee. + * @param _exitFeeInBps The exit fee. */ - function setExitFeeBasisPoints(uint256 _exitFeeBasisPoints) external onlyOwner { - exitFeeBasisPoints = _exitFeeBasisPoints; + function setExitFeeBasisPoints(uint16 _exitFeeInBps) external onlyOwner { + require(_exitFeeInBps <= BASIS_POINT_SCALE, "INVALID"); + exitFeeInBps = _exitFeeInBps; + } + + function setLowWatermarkInBpsOfTvl(uint16 _lowWatermarkInBpsOfTvl) external onlyOwner { + require(_lowWatermarkInBpsOfTvl <= BASIS_POINT_SCALE, "INVALID"); + lowWatermarkInBpsOfTvl = _lowWatermarkInBpsOfTvl; + } + + function setExitFeeSplitToTreasuryInBps(uint16 _exitFeeSplitToTreasuryInBps) external onlyOwner { + require(_exitFeeSplitToTreasuryInBps <= BASIS_POINT_SCALE, "INVALID"); + exitFeeSplitToTreasuryInBps = _exitFeeSplitToTreasuryInBps; } function _updateRateLimit(uint256 shares) internal { @@ -195,11 +224,11 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU } function _convertSharesToBucketUnit(uint256 shares, Math.Rounding rounding) internal pure returns (uint64) { - return (rounding == Math.Rounding.Up) ? SafeCast.toUint64((shares + BUCKEt_UNIT_SCALE - 1) / BUCKEt_UNIT_SCALE) : SafeCast.toUint64(shares / BUCKEt_UNIT_SCALE); + return (rounding == Math.Rounding.Up) ? SafeCast.toUint64((shares + BUCKET_UNIT_SCALE - 1) / BUCKET_UNIT_SCALE) : SafeCast.toUint64(shares / BUCKET_UNIT_SCALE); } function _convertBucketUnitToAmount(uint64 bucketUnit) internal pure returns (uint256) { - return bucketUnit * BUCKEt_UNIT_SCALE; + return bucketUnit * BUCKET_UNIT_SCALE; } /** @@ -208,15 +237,11 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU // redeemable amount after exit fee function previewRedeem(uint256 shares) public view returns (uint256) { uint256 amountInEth = liquidityPool.amountForShare(shares); - return amountInEth - _feeOnTotal(amountInEth, exitFeeBasisPoints); + return amountInEth - _fee(amountInEth, exitFeeInBps); } - /** - * @dev Calculates the fee part of an amount `assets` that already includes fees. - * Used in {IERC4626-redeem}. - */ - function _feeOnTotal(uint256 assets, uint256 feeBasisPoints) internal pure virtual returns (uint256) { - return assets.mulDiv(feeBasisPoints, feeBasisPoints + BASIS_POINT_SCALE, Math.Rounding.Up); + function _fee(uint256 assets, uint256 feeBasisPoints) internal pure virtual returns (uint256) { + return assets.mulDiv(feeBasisPoints, BASIS_POINT_SCALE, Math.Rounding.Up); } function _authorizeUpgrade(address newImplementation) internal override onlyOwner {} diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 64d30ae4f..439f0fa24 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -10,9 +10,15 @@ import "./interfaces/ILiquidityPool.sol"; import "./interfaces/IWithdrawRequestNFT.sol"; import "./interfaces/IMembershipManager.sol"; +import "@openzeppelin/contracts/utils/math/Math.sol"; + contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgradeable, IWithdrawRequestNFT { + using Math for uint256; + uint256 private constant BASIS_POINT_SCALE = 1e4; + address public immutable treasury; + ILiquidityPool public liquidityPool; IeETH public eETH; IMembershipManager public membershipManager; @@ -22,16 +28,20 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint32 public nextRequestId; uint32 public lastFinalizedRequestId; - uint96 public accumulatedDustEEthShares; // to be burned or used to cover the validator churn cost + uint16 public shareRemainderSplitToTreasuryInBps; event WithdrawRequestCreated(uint32 indexed requestId, uint256 amountOfEEth, uint256 shareOfEEth, address owner, uint256 fee); event WithdrawRequestClaimed(uint32 indexed requestId, uint256 amountOfEEth, uint256 burntShareOfEEth, address owner, uint256 fee); event WithdrawRequestInvalidated(uint32 indexed requestId); event WithdrawRequestValidated(uint32 indexed requestId); event WithdrawRequestSeized(uint32 indexed requestId); + event HandledRemainderOfClaimedWithdrawRequests(uint256 eEthAmountToTreasury, uint256 eEthAmountBurnt); /// @custom:oz-upgrades-unsafe-allow constructor - constructor() { + constructor(address _treasury, uint16 _shareRemainderSplitToTreasuryInBps) { + treasury = _treasury; + shareRemainderSplitToTreasuryInBps = _shareRemainderSplitToTreasuryInBps; + _disableInitializers(); } @@ -100,16 +110,13 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad _burn(tokenId); delete _requests[tokenId]; + uint256 amountBurnedShare = 0; if (fee > 0) { - // send fee to membership manager - liquidityPool.withdraw(address(membershipManager), fee); + amountBurnedShare += liquidityPool.withdraw(address(membershipManager), fee); } - - uint256 amountBurnedShare = liquidityPool.withdraw(recipient, amountToWithdraw); + amountBurnedShare += liquidityPool.withdraw(recipient, amountToWithdraw); uint256 amountUnBurnedShare = request.shareOfEEth - amountBurnedShare; - if (amountUnBurnedShare > 0) { - accumulatedDustEEthShares += uint96(amountUnBurnedShare); - } + handleRemainder(amountUnBurnedShare); emit WithdrawRequestClaimed(uint32(tokenId), amountToWithdraw + fee, amountBurnedShare, recipient, fee); } @@ -120,13 +127,33 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } } - // a function to transfer accumulated shares to admin - function burnAccumulatedDustEEthShares() external onlyAdmin { - require(eETH.totalShares() > accumulatedDustEEthShares, "Inappropriate burn"); - uint256 amount = accumulatedDustEEthShares; - accumulatedDustEEthShares = 0; + // There have been errors tracking `accumulatedDustEEthShares` in the past. + // - https://github.com/etherfi-protocol/smart-contracts/issues/24 + // This is a one-time function to handle the remainder of the eEth shares after the claim of the withdraw requests + // It must be called only once with ALL the requests that have not been claimed yet. + // there are <3000 such requests and the total gas spending is expected to be ~9.0 M gas. + function handleAccumulatedShareRemainder(uint256[] memory _reqIds) external onlyOwner { + bytes32 slot = keccak256("handleAccumulatedShareRemainder"); + uint256 executed; + assembly { + executed := sload(slot) + } + require(executed == 0, "ALREADY_EXECUTED"); + + uint256 eEthSharesUnclaimedYet = 0; + for (uint256 i = 0; i < _reqIds.length; i++) { + assert (_requests[_reqIds[i]].isValid); + eEthSharesUnclaimedYet += _requests[_reqIds[i]].shareOfEEth; + } + uint256 eEthSharesRemainder = eETH.shares(address(this)) - eEthSharesUnclaimedYet; + + handleRemainder(eEthSharesRemainder); - eETH.burnShares(address(this), amount); + assembly { + sstore(slot, 1) + executed := sload(slot) + } + assert (executed == 1); } // Given an invalidated withdrawal request NFT of ID `requestId`:, @@ -196,6 +223,28 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad admins[_address] = _isAdmin; } + function updateShareRemainderSplitToTreasuryInBps(uint16 _shareRemainderSplitToTreasuryInBps) external onlyOwner { + shareRemainderSplitToTreasuryInBps = _shareRemainderSplitToTreasuryInBps; + } + + /// @dev Handles the remainder of the eEth shares after the claim of the withdraw request + /// the remainder eETH share for a request = request.shareOfEEth - request.amountOfEEth / (eETH amount to eETH shares rate) + /// - Splits the remainder into two parts: + /// - Treasury: treasury gets a split of the remainder + /// - Burn: the rest of the remainder is burned + /// @param _eEthShares: the remainder of the eEth shares + function handleRemainder(uint256 _eEthShares) internal { + uint256 eEthSharesToTreasury = _eEthShares.mulDiv(shareRemainderSplitToTreasuryInBps, BASIS_POINT_SCALE); + + uint256 eEthAmountToTreasury = liquidityPool.amountForShare(eEthSharesToTreasury); + eETH.transfer(treasury, eEthAmountToTreasury); + + uint256 eEthSharesToBurn = _eEthShares - eEthSharesToTreasury; + eETH.burnShares(address(this), eEthSharesToBurn); + + emit HandledRemainderOfClaimedWithdrawRequests(eEthAmountToTreasury, liquidityPool.amountForShare(eEthSharesToBurn)); + } + // invalid NFTs is non-transferable except for the case they are being burnt by the owner via `seizeInvalidRequest` function _beforeTokenTransfer(address from, address to, uint256 firstTokenId, uint256 batchSize) internal override { for (uint256 i = 0; i < batchSize; i++) { diff --git a/test/EtherFiWithdrawalBuffer.t.sol b/test/EtherFiWithdrawalBuffer.t.sol new file mode 100644 index 000000000..ce7f99548 --- /dev/null +++ b/test/EtherFiWithdrawalBuffer.t.sol @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.13; + +import "forge-std/console2.sol"; +import "./TestSetup.sol"; + +contract EtherFiWithdrawalBufferTest is TestSetup { + + address user = vm.addr(999); + + function setUp() public { + setUpTests(); + + vm.startPrank(owner); + etherFiWithdrawalBufferInstance.setCapacity(10 ether); + etherFiWithdrawalBufferInstance.setRefillRatePerSecond(0.001 ether); + etherFiWithdrawalBufferInstance.setExitFeeSplitToTreasuryInBps(1e4); + vm.stopPrank(); + + vm.warp(block.timestamp + 5 * 1000); // 0.001 ether * 5000 = 5 ether refilled + + + } + + function test_rate_limit() public { + assertEq(etherFiWithdrawalBufferInstance.canRedeem(1 ether), true); + assertEq(etherFiWithdrawalBufferInstance.canRedeem(5 ether - 1), true); + assertEq(etherFiWithdrawalBufferInstance.canRedeem(5 ether + 1), false); + assertEq(etherFiWithdrawalBufferInstance.canRedeem(10 ether), false); + assertEq(etherFiWithdrawalBufferInstance.totalRedeemableAmount(), 5 ether); + } + + function test_lowwatermark_guardrail() public { + vm.deal(user, 100 ether); + + assertEq(etherFiWithdrawalBufferInstance.lowWatermarkInETH(), 0 ether); + + vm.prank(user); + liquidityPoolInstance.deposit{value: 100 ether}(); + + vm.startPrank(etherFiWithdrawalBufferInstance.owner()); + etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(1_00); // 1% + assertEq(etherFiWithdrawalBufferInstance.lowWatermarkInETH(), 1 ether); + + etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(50_00); // 50% + assertEq(etherFiWithdrawalBufferInstance.lowWatermarkInETH(), 50 ether); + + etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(100_00); // 100% + assertEq(etherFiWithdrawalBufferInstance.lowWatermarkInETH(), 100 ether); + } + + function test_redeem_eEth() public { + vm.deal(user, 100 ether); + vm.startPrank(user); + + assertEq(etherFiWithdrawalBufferInstance.canRedeem(1 ether), true); + assertEq(etherFiWithdrawalBufferInstance.canRedeem(10 ether), false); + + liquidityPoolInstance.deposit{value: 1 ether}(); + + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 0.5 ether); + vm.expectRevert("TRANSFER_AMOUNT_EXCEEDS_ALLOWANCE"); + etherFiWithdrawalBufferInstance.redeemEEth(1 ether, user, user); + + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 2 ether); + vm.expectRevert("EtherFiWithdrawalBuffer: Insufficient balance"); + etherFiWithdrawalBufferInstance.redeemEEth(2 ether, user, user); + + liquidityPoolInstance.deposit{value: 10 ether}(); + + uint256 totalRedeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); + uint256 userBalance = address(user).balance; + uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); + etherFiWithdrawalBufferInstance.redeemEEth(1 ether, user, user); + assertEq(eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalance + 0.01 ether); + assertEq(address(user).balance, userBalance + 0.99 ether); + assertEq(etherFiWithdrawalBufferInstance.totalRedeemableAmount(), totalRedeemableAmount - 1 ether); + + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 10 ether); + vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); + etherFiWithdrawalBufferInstance.redeemEEth(10 ether, user, user); + + vm.stopPrank(); + } + + function test_redeem_weEth() public { + vm.deal(user, 100 ether); + vm.startPrank(user); + + assertEq(etherFiWithdrawalBufferInstance.canRedeem(1 ether), true); + assertEq(etherFiWithdrawalBufferInstance.canRedeem(10 ether), false); + + liquidityPoolInstance.deposit{value: 1 ether}(); + eETHInstance.approve(address(weEthInstance), 1 ether); + weEthInstance.wrap(1 ether); + + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 0.5 ether); + vm.expectRevert("ERC20: insufficient allowance"); + etherFiWithdrawalBufferInstance.redeemWeEth(1 ether, user, user); + + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 2 ether); + vm.expectRevert("EtherFiWithdrawalBuffer: Insufficient balance"); + etherFiWithdrawalBufferInstance.redeemWeEth(2 ether, user, user); + + liquidityPoolInstance.deposit{value: 10 ether}(); + + uint256 totalRedeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); + uint256 userBalance = address(user).balance; + uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); + etherFiWithdrawalBufferInstance.redeemWeEth(1 ether, user, user); + assertEq(eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalance + 0.01 ether); + assertEq(address(user).balance, userBalance + 0.99 ether); + assertEq(etherFiWithdrawalBufferInstance.totalRedeemableAmount(), totalRedeemableAmount - 1 ether); + + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 10 ether); + vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); + etherFiWithdrawalBufferInstance.redeemWeEth(10 ether, user, user); + + vm.stopPrank(); + } + + function test_redeem_weEth_with_varying_exchange_rate() public { + vm.deal(user, 100 ether); + + vm.startPrank(user); + liquidityPoolInstance.deposit{value: 10 ether}(); + eETHInstance.approve(address(weEthInstance), 1 ether); + weEthInstance.wrap(1 ether); + vm.stopPrank(); + + vm.prank(address(membershipManagerInstance)); + liquidityPoolInstance.rebase(1 ether); // 10 eETH earned 1 ETH + + vm.startPrank(user); + uint256 userBalance = address(user).balance; + uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); + etherFiWithdrawalBufferInstance.redeemWeEth(1 ether, user, user); + assertEq(eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalance + 0.011 ether); + assertEq(address(user).balance, userBalance + (1.1 ether - 0.011 ether)); + vm.stopPrank(); + } +} diff --git a/test/TestSetup.sol b/test/TestSetup.sol index 0f13eb4b9..423a3b612 100644 --- a/test/TestSetup.sol +++ b/test/TestSetup.sol @@ -49,6 +49,7 @@ import "../src/EtherFiAdmin.sol"; import "../src/EtherFiTimelock.sol"; import "../src/BucketRateLimiter.sol"; +import "../src/EtherFiWithdrawalBuffer.sol"; contract TestSetup is Test { @@ -102,6 +103,7 @@ contract TestSetup is Test { UUPSProxy public membershipNftProxy; UUPSProxy public nftExchangeProxy; UUPSProxy public withdrawRequestNFTProxy; + UUPSProxy public etherFiWithdrawalBufferProxy; UUPSProxy public etherFiOracleProxy; UUPSProxy public etherFiAdminProxy; @@ -161,6 +163,8 @@ contract TestSetup is Test { WithdrawRequestNFT public withdrawRequestNFTImplementation; WithdrawRequestNFT public withdrawRequestNFTInstance; + EtherFiWithdrawalBuffer public etherFiWithdrawalBufferInstance; + NFTExchange public nftExchangeImplementation; NFTExchange public nftExchangeInstance; @@ -551,7 +555,7 @@ contract TestSetup is Test { membershipNftProxy = new UUPSProxy(address(membershipNftImplementation), ""); membershipNftInstance = MembershipNFT(payable(membershipNftProxy)); - withdrawRequestNFTImplementation = new WithdrawRequestNFT(); + withdrawRequestNFTImplementation = new WithdrawRequestNFT(address(0), 0); withdrawRequestNFTProxy = new UUPSProxy(address(withdrawRequestNFTImplementation), ""); withdrawRequestNFTInstance = WithdrawRequestNFT(payable(withdrawRequestNFTProxy)); @@ -572,7 +576,13 @@ contract TestSetup is Test { etherFiRestakerProxy = new UUPSProxy(address(etherFiRestakerImplementation), ""); etherFiRestakerInstance = EtherFiRestaker(payable(etherFiRestakerProxy)); + etherFiWithdrawalBufferProxy = new UUPSProxy(address(new EtherFiWithdrawalBuffer(address(liquidityPoolInstance), address(eETHInstance), address(weEthInstance), address(treasuryInstance))), ""); + etherFiWithdrawalBufferInstance = EtherFiWithdrawalBuffer(payable(etherFiWithdrawalBufferProxy)); + etherFiWithdrawalBufferInstance.initialize(0, 1_00, 10_00); + liquidityPoolInstance.initialize(address(eETHInstance), address(stakingManagerInstance), address(etherFiNodeManagerProxy), address(membershipManagerInstance), address(TNFTInstance), address(etherFiAdminProxy), address(withdrawRequestNFTInstance)); + liquidityPoolInstance.initializeOnUpgradeWithWithdrawalBuffer(address(etherFiWithdrawalBufferInstance)); + membershipNftInstance.initialize("https://etherfi-cdn/{id}.json", address(membershipManagerInstance)); withdrawRequestNFTInstance.initialize(payable(address(liquidityPoolInstance)), payable(address(eETHInstance)), payable(address(membershipManagerInstance))); membershipManagerInstance.initialize( diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 98accc85b..924545421 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -8,6 +8,8 @@ import "./TestSetup.sol"; contract WithdrawRequestNFTTest is TestSetup { + uint256[] public reqIds =[ 20, 388, 478, 714, 726, 729, 735, 815, 861, 916, 941, 1014, 1067, 1154, 1194, 1253]; + function setUp() public { setUpTests(); } @@ -179,7 +181,6 @@ contract WithdrawRequestNFTTest is TestSetup { vm.prank(bob); uint256 requestId = liquidityPoolInstance.requestWithdraw(bob, 1 ether); - assertEq(withdrawRequestNFTInstance.accumulatedDustEEthShares(), 0, "Accumulated dust should be 0"); assertEq(eETHInstance.balanceOf(bob), 9 ether); assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 1 ether, "eETH balance should be 1 ether"); @@ -202,12 +203,6 @@ contract WithdrawRequestNFTTest is TestSetup { assertEq(bobsEndingBalance, bobsStartingBalance + 1 ether, "Bobs balance should be 1 ether higher"); assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 1 ether, "eETH balance should be 1 ether"); - assertEq(liquidityPoolInstance.amountForShare(withdrawRequestNFTInstance.accumulatedDustEEthShares()), 1 ether); - - vm.prank(alice); - withdrawRequestNFTInstance.burnAccumulatedDustEEthShares(); - assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 0 ether, "eETH balance should be 0 ether"); - assertEq(eETHInstance.balanceOf(bob), 18 ether + 1 ether); // 1 ether eETH in `withdrawRequestNFT` contract is re-distributed to the eETH holders } function test_ValidClaimWithdrawWithNegativeRebase() public { @@ -418,4 +413,13 @@ contract WithdrawRequestNFTTest is TestSetup { assertEq(liquidityPoolInstance.ethAmountLockedForWithdrawal(), 0, "Must be withdrawn"); assertEq(address(chad).balance, chadBalance + claimableAmount, "Chad should receive the claimable amount"); } + + function test_distributeImplicitFee() public { + initializeRealisticFork(MAINNET_FORK); + + vm.startPrank(withdrawRequestNFTInstance.owner()); + withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(address(owner), 50_00))); + + withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIds); + } } From 0f13953a94b65a17b61c4491544a80274c14af03 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 12 Dec 2024 09:33:34 +0900 Subject: [PATCH 03/42] fix scripts --- script/deploys/DeployPhaseTwo.s.sol | 2 +- script/upgrades/WithdrawRequestNFTUpgradeScript.s.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/script/deploys/DeployPhaseTwo.s.sol b/script/deploys/DeployPhaseTwo.s.sol index a144e0a35..2417e191b 100644 --- a/script/deploys/DeployPhaseTwo.s.sol +++ b/script/deploys/DeployPhaseTwo.s.sol @@ -81,7 +81,7 @@ contract DeployPhaseTwoScript is Script { } retrieve_contract_addresses(); - withdrawRequestNftImplementation = new WithdrawRequestNFT(); + withdrawRequestNftImplementation = new WithdrawRequestNFT(address(0), 0); withdrawRequestNftProxy = new UUPSProxy(address(withdrawRequestNftImplementation), ""); withdrawRequestNftInstance = WithdrawRequestNFT(payable(withdrawRequestNftProxy)); diff --git a/script/upgrades/WithdrawRequestNFTUpgradeScript.s.sol b/script/upgrades/WithdrawRequestNFTUpgradeScript.s.sol index 823cf90a5..5862fb36e 100644 --- a/script/upgrades/WithdrawRequestNFTUpgradeScript.s.sol +++ b/script/upgrades/WithdrawRequestNFTUpgradeScript.s.sol @@ -20,7 +20,7 @@ contract WithdrawRequestNFTUpgrade is Script { vm.startBroadcast(deployerPrivateKey); WithdrawRequestNFT oracleInstance = WithdrawRequestNFT(proxyAddress); - WithdrawRequestNFT v2Implementation = new WithdrawRequestNFT(); + WithdrawRequestNFT v2Implementation = new WithdrawRequestNFT(address(0), 0); oracleInstance.upgradeTo(address(v2Implementation)); From 0b68310623d61f3c8ebc481e5b3f90743e237f52 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Tue, 17 Dec 2024 14:43:25 +0900 Subject: [PATCH 04/42] add role registry, consider eth amount locked for withdrawal in liquidity pool, add tests --- src/EtherFiWithdrawalBuffer.sol | 85 ++++++--- src/interfaces/ILiquidityPool.sol | 1 + test/EtherFiWithdrawalBuffer.t.sol | 271 ++++++++++++++++++++++++++++- test/TestSetup.sol | 14 +- 4 files changed, 336 insertions(+), 35 deletions(-) diff --git a/src/EtherFiWithdrawalBuffer.sol b/src/EtherFiWithdrawalBuffer.sol index 811d8d149..a79b9e948 100644 --- a/src/EtherFiWithdrawalBuffer.sol +++ b/src/EtherFiWithdrawalBuffer.sol @@ -20,6 +20,7 @@ import "./interfaces/IWeETH.sol"; import "lib/BucketLimiter.sol"; +import "./RoleRegistry.sol"; /* The contract allows instant redemption of eETH and weETH tokens to ETH with an exit fee. @@ -32,6 +33,12 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU uint256 private constant BUCKET_UNIT_SCALE = 1e12; uint256 private constant BASIS_POINT_SCALE = 1e4; + + bytes32 public constant PROTOCOL_PAUSER = keccak256("PROTOCOL_PAUSER"); + bytes32 public constant PROTOCOL_UNPAUSER = keccak256("PROTOCOL_UNPAUSER"); + bytes32 public constant PROTOCOL_ADMIN = keccak256("PROTOCOL_ADMIN"); + + RoleRegistry public immutable roleRegistry; address public immutable treasury; IeETH public immutable eEth; IWeETH public immutable weEth; @@ -45,9 +52,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU receive() external payable {} /// @custom:oz-upgrades-unsafe-allow constructor - constructor(address _liquidityPool, address _eEth, address _weEth, address _treasury) { - require(address(liquidityPool) == address(0) && address(eEth) == address(0) && address(treasury) == address(0), "EtherFiWithdrawalBuffer: Cannot initialize twice"); - + constructor(address _liquidityPool, address _eEth, address _weEth, address _treasury, address _roleRegistry) { + roleRegistry = RoleRegistry(_roleRegistry); treasury = _treasury; liquidityPool = ILiquidityPool(payable(_liquidityPool)); eEth = IeETH(_eEth); @@ -56,13 +62,13 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU _disableInitializers(); } - function initialize(uint16 _exitFeeSplitToTreasuryInBps, uint16 _exitFeeInBps, uint16 _lowWatermarkInBpsOfTvl) external initializer { + function initialize(uint16 _exitFeeSplitToTreasuryInBps, uint16 _exitFeeInBps, uint16 _lowWatermarkInBpsOfTvl, uint256 _bucketCapacity, uint256 _bucketRefillRate) external initializer { __Ownable_init(); __UUPSUpgradeable_init(); __Pausable_init(); __ReentrancyGuard_init(); - limit = BucketLimiter.create(0, 0); + limit = BucketLimiter.create(_convertToBucketUnit(_bucketCapacity, Math.Rounding.Down), _convertToBucketUnit(_bucketRefillRate, Math.Rounding.Down)); exitFeeSplitToTreasuryInBps = _exitFeeSplitToTreasuryInBps; exitFeeInBps = _exitFeeInBps; lowWatermarkInBpsOfTvl = _lowWatermarkInBpsOfTvl; @@ -76,8 +82,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @return The amount of ETH sent to the receiver and the exit fee amount. */ function redeemEEth(uint256 eEthAmount, address receiver, address owner) public whenNotPaused nonReentrant returns (uint256, uint256) { - require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); require(eEthAmount <= eEth.balanceOf(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); + require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); uint256 beforeEEthAmount = eEth.balanceOf(address(this)); IERC20(address(eEth)).safeTransferFrom(owner, address(this), eEthAmount); @@ -97,8 +103,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU function redeemWeEth(uint256 weEthAmount, address receiver, address owner) public whenNotPaused nonReentrant returns (uint256, uint256) { uint256 eEthShares = weEthAmount; uint256 eEthAmount = liquidityPool.amountForShare(eEthShares); - require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); require(weEthAmount <= weEth.balanceOf(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); + require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); uint256 beforeEEthAmount = eEth.balanceOf(address(this)); IERC20(address(weEth)).safeTransferFrom(owner, address(this), weEthAmount); @@ -135,14 +141,14 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU uint256 feeShareToStakers = ethShareFee - feeShareToTreasury; // To Stakers by burning shares - eEth.burnShares(address(this), liquidityPool.sharesForAmount(feeShareToStakers)); + eEth.burnShares(address(this), feeShareToStakers); // To Treasury by transferring eETH IERC20(address(eEth)).safeTransfer(treasury, eEthFeeAmountToTreasury); // To Receiver by transferring ETH - payable(receiver).transfer(ethReceived); - require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiWithdrawalBuffer: Transfer failed"); + (bool success, ) = receiver.call{value: ethReceived, gas: 100_000}(""); + require(success && address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiWithdrawalBuffer: Transfer failed"); return (ethReceived, eEthAmountFee); } @@ -158,12 +164,13 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @dev Returns the total amount that can be redeemed. */ function totalRedeemableAmount() external view returns (uint256) { - if (address(liquidityPool).balance < lowWatermarkInETH()) { + uint256 liquidEthAmount = address(liquidityPool).balance - liquidityPool.ethAmountLockedForWithdrawal(); + if (liquidEthAmount < lowWatermarkInETH()) { return 0; } uint64 consumableBucketUnits = BucketLimiter.consumable(limit); - uint256 consumableAmount = _convertBucketUnitToAmount(consumableBucketUnits); - return consumableAmount; + uint256 consumableAmount = _convertFromBucketUnit(consumableBucketUnits); + return Math.min(consumableAmount, liquidEthAmount); } /** @@ -171,21 +178,22 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @param amount The ETH or eETH amount to check. */ function canRedeem(uint256 amount) public view returns (bool) { - if (address(liquidityPool).balance < lowWatermarkInETH()) { + uint256 liquidEthAmount = address(liquidityPool).balance - liquidityPool.ethAmountLockedForWithdrawal(); + if (liquidEthAmount < lowWatermarkInETH()) { return false; } - uint64 bucketUnit = _convertSharesToBucketUnit(amount, Math.Rounding.Up); + uint64 bucketUnit = _convertToBucketUnit(amount, Math.Rounding.Up); bool consumable = BucketLimiter.canConsume(limit, bucketUnit); - return consumable; + return consumable && amount <= liquidEthAmount; } /** * @dev Sets the maximum size of the bucket that can be consumed in a given time period. * @param capacity The capacity of the bucket. */ - function setCapacity(uint256 capacity) external onlyOwner { + function setCapacity(uint256 capacity) external hasRole(PROTOCOL_ADMIN) { // max capacity = max(uint64) * 1e12 ~= 16 * 1e18 * 1e12 = 16 * 1e12 ether, which is practically enough - uint64 bucketUnit = _convertSharesToBucketUnit(capacity, Math.Rounding.Down); + uint64 bucketUnit = _convertToBucketUnit(capacity, Math.Rounding.Down); BucketLimiter.setCapacity(limit, bucketUnit); } @@ -193,9 +201,9 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @dev Sets the rate at which the bucket is refilled per second. * @param refillRate The rate at which the bucket is refilled per second. */ - function setRefillRatePerSecond(uint256 refillRate) external onlyOwner { + function setRefillRatePerSecond(uint256 refillRate) external hasRole(PROTOCOL_ADMIN) { // max refillRate = max(uint64) * 1e12 ~= 16 * 1e18 * 1e12 = 16 * 1e12 ether per second, which is practically enough - uint64 bucketUnit = _convertSharesToBucketUnit(refillRate, Math.Rounding.Down); + uint64 bucketUnit = _convertToBucketUnit(refillRate, Math.Rounding.Down); BucketLimiter.setRefillRate(limit, bucketUnit); } @@ -203,31 +211,39 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @dev Sets the exit fee. * @param _exitFeeInBps The exit fee. */ - function setExitFeeBasisPoints(uint16 _exitFeeInBps) external onlyOwner { + function setExitFeeBasisPoints(uint16 _exitFeeInBps) external hasRole(PROTOCOL_ADMIN) { require(_exitFeeInBps <= BASIS_POINT_SCALE, "INVALID"); exitFeeInBps = _exitFeeInBps; } - function setLowWatermarkInBpsOfTvl(uint16 _lowWatermarkInBpsOfTvl) external onlyOwner { + function setLowWatermarkInBpsOfTvl(uint16 _lowWatermarkInBpsOfTvl) external hasRole(PROTOCOL_ADMIN) { require(_lowWatermarkInBpsOfTvl <= BASIS_POINT_SCALE, "INVALID"); lowWatermarkInBpsOfTvl = _lowWatermarkInBpsOfTvl; } - function setExitFeeSplitToTreasuryInBps(uint16 _exitFeeSplitToTreasuryInBps) external onlyOwner { + function setExitFeeSplitToTreasuryInBps(uint16 _exitFeeSplitToTreasuryInBps) external hasRole(PROTOCOL_ADMIN) { require(_exitFeeSplitToTreasuryInBps <= BASIS_POINT_SCALE, "INVALID"); exitFeeSplitToTreasuryInBps = _exitFeeSplitToTreasuryInBps; } - function _updateRateLimit(uint256 shares) internal { - uint64 bucketUnit = _convertSharesToBucketUnit(shares, Math.Rounding.Up); + function pauseContract() external hasRole(PROTOCOL_PAUSER) { + _pause(); + } + + function unPauseContract() external hasRole(PROTOCOL_UNPAUSER) { + _unpause(); + } + + function _updateRateLimit(uint256 amount) internal { + uint64 bucketUnit = _convertToBucketUnit(amount, Math.Rounding.Up); require(BucketLimiter.consume(limit, bucketUnit), "BucketRateLimiter: rate limit exceeded"); } - function _convertSharesToBucketUnit(uint256 shares, Math.Rounding rounding) internal pure returns (uint64) { - return (rounding == Math.Rounding.Up) ? SafeCast.toUint64((shares + BUCKET_UNIT_SCALE - 1) / BUCKET_UNIT_SCALE) : SafeCast.toUint64(shares / BUCKET_UNIT_SCALE); + function _convertToBucketUnit(uint256 amount, Math.Rounding rounding) internal pure returns (uint64) { + return (rounding == Math.Rounding.Up) ? SafeCast.toUint64((amount + BUCKET_UNIT_SCALE - 1) / BUCKET_UNIT_SCALE) : SafeCast.toUint64(amount / BUCKET_UNIT_SCALE); } - function _convertBucketUnitToAmount(uint64 bucketUnit) internal pure returns (uint256) { + function _convertFromBucketUnit(uint64 bucketUnit) internal pure returns (uint256) { return bucketUnit * BUCKET_UNIT_SCALE; } @@ -246,4 +262,17 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU function _authorizeUpgrade(address newImplementation) internal override onlyOwner {} + function getImplementation() external view returns (address) { + return _getImplementation(); + } + + function _hasRole(bytes32 role, address account) internal view returns (bool) { + require(roleRegistry.hasRole(role, account), "EtherFiWithdrawalBuffer: Unauthorized"); + } + + modifier hasRole(bytes32 role) { + _hasRole(role, msg.sender); + _; + } + } \ No newline at end of file diff --git a/src/interfaces/ILiquidityPool.sol b/src/interfaces/ILiquidityPool.sol index 2f12fd76d..a71b2d6c7 100644 --- a/src/interfaces/ILiquidityPool.sol +++ b/src/interfaces/ILiquidityPool.sol @@ -50,6 +50,7 @@ interface ILiquidityPool { function sharesForAmount(uint256 _amount) external view returns (uint256); function sharesForWithdrawalAmount(uint256 _amount) external view returns (uint256); function amountForShare(uint256 _share) external view returns (uint256); + function ethAmountLockedForWithdrawal() external view returns (uint128); function deposit() external payable returns (uint256); function deposit(address _referral) external payable returns (uint256); diff --git a/test/EtherFiWithdrawalBuffer.t.sol b/test/EtherFiWithdrawalBuffer.t.sol index ce7f99548..d73268fd0 100644 --- a/test/EtherFiWithdrawalBuffer.t.sol +++ b/test/EtherFiWithdrawalBuffer.t.sol @@ -7,6 +7,7 @@ import "./TestSetup.sol"; contract EtherFiWithdrawalBufferTest is TestSetup { address user = vm.addr(999); + address op_admin = vm.addr(1000); function setUp() public { setUpTests(); @@ -18,8 +19,18 @@ contract EtherFiWithdrawalBufferTest is TestSetup { vm.stopPrank(); vm.warp(block.timestamp + 5 * 1000); // 0.001 ether * 5000 = 5 ether refilled + } + + function setUp_Fork() public { + initializeRealisticFork(MAINNET_FORK); + vm.startPrank(owner); + roleRegistry.grantRole(keccak256("PROTOCOL_ADMIN"), op_admin); + vm.stopPrank(); + etherFiWithdrawalBufferProxy = new UUPSProxy(address(new EtherFiWithdrawalBuffer(address(liquidityPoolInstance), address(eETHInstance), address(weEthInstance), address(treasuryInstance), address(roleRegistry))), ""); + etherFiWithdrawalBufferInstance = EtherFiWithdrawalBuffer(payable(etherFiWithdrawalBufferProxy)); + etherFiWithdrawalBufferInstance.initialize(1e4, 1_00, 10_00); // 10% fee split to treasury, 1% exit fee, 10% low watermark } function test_rate_limit() public { @@ -77,14 +88,38 @@ contract EtherFiWithdrawalBufferTest is TestSetup { assertEq(address(user).balance, userBalance + 0.99 ether); assertEq(etherFiWithdrawalBufferInstance.totalRedeemableAmount(), totalRedeemableAmount - 1 ether); - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 10 ether); + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 5 ether); vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); - etherFiWithdrawalBufferInstance.redeemEEth(10 ether, user, user); + etherFiWithdrawalBufferInstance.redeemEEth(5 ether, user, user); + + vm.stopPrank(); + } + + function test_mainnet_redeem_weEth_with_rebase() public { + vm.deal(user, 100 ether); + + vm.startPrank(user); + liquidityPoolInstance.deposit{value: 10 ether}(); + eETHInstance.approve(address(weEthInstance), 10 ether); + weEthInstance.wrap(10 ether); + vm.stopPrank(); + + uint256 one_percent_of_tvl = liquidityPoolInstance.getTotalPooledEther() / 100; + vm.prank(address(membershipManagerInstance)); + liquidityPoolInstance.rebase(int128(uint128(one_percent_of_tvl))); // 10 eETH earned 1 ETH + + vm.startPrank(user); + uint256 userBalance = address(user).balance; + uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); + etherFiWithdrawalBufferInstance.redeemWeEth(1 ether, user, user); + assertEq(eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalance + 0.0101 ether); + assertEq(address(user).balance, userBalance + 0.9999 ether); vm.stopPrank(); } - function test_redeem_weEth() public { + function test_redeem_weEth_1() public { vm.deal(user, 100 ether); vm.startPrank(user); @@ -114,9 +149,11 @@ contract EtherFiWithdrawalBufferTest is TestSetup { assertEq(address(user).balance, userBalance + 0.99 ether); assertEq(etherFiWithdrawalBufferInstance.totalRedeemableAmount(), totalRedeemableAmount - 1 ether); - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 10 ether); + eETHInstance.approve(address(weEthInstance), 6 ether); + weEthInstance.wrap(6 ether); + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 5 ether); vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); - etherFiWithdrawalBufferInstance.redeemWeEth(10 ether, user, user); + etherFiWithdrawalBufferInstance.redeemWeEth(5 ether, user, user); vm.stopPrank(); } @@ -142,4 +179,228 @@ contract EtherFiWithdrawalBufferTest is TestSetup { assertEq(address(user).balance, userBalance + (1.1 ether - 0.011 ether)); vm.stopPrank(); } + + // The test ensures that: + // - Redemption works correctly within allowed limits. + // - Fees are applied accurately. + // - The function properly reverts when redemption conditions aren't met. + function testFuzz_redeemEEth( + uint256 depositAmount, + uint256 redeemAmount, + uint256 exitFeeSplitBps, + uint16 exitFeeBps, + uint16 lowWatermarkBps + ) public { + depositAmount = bound(depositAmount, 1 ether, 1000 ether); + redeemAmount = bound(redeemAmount, 0.1 ether, depositAmount); + exitFeeSplitBps = bound(exitFeeSplitBps, 0, 10000); + exitFeeBps = uint16(bound(uint256(exitFeeBps), 0, 10000)); + lowWatermarkBps = uint16(bound(uint256(lowWatermarkBps), 0, 10000)); + + vm.deal(user, depositAmount); + vm.startPrank(user); + liquidityPoolInstance.deposit{value: depositAmount}(); + vm.stopPrank(); + + // Set exitFeeSplitToTreasuryInBps + vm.prank(owner); + etherFiWithdrawalBufferInstance.setExitFeeSplitToTreasuryInBps(uint16(exitFeeSplitBps)); + + // Set exitFeeBasisPoints and lowWatermarkInBpsOfTvl + vm.prank(owner); + etherFiWithdrawalBufferInstance.setExitFeeBasisPoints(exitFeeBps); + + vm.prank(owner); + etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(lowWatermarkBps); + + vm.startPrank(user); + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), redeemAmount); + uint256 totalRedeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); + + if (redeemAmount <= totalRedeemableAmount && etherFiWithdrawalBufferInstance.canRedeem(redeemAmount)) { + uint256 userBalanceBefore = address(user).balance; + uint256 treasuryBalanceBefore = eETHInstance.balanceOf(address(treasuryInstance)); + + etherFiWithdrawalBufferInstance.redeemEEth(redeemAmount, user, user); + + uint256 totalFee = (redeemAmount * exitFeeBps) / 10000; + uint256 treasuryFee = (totalFee * exitFeeSplitBps) / 10000; + uint256 userReceives = redeemAmount - totalFee; + + assertApproxEqAbs( + eETHInstance.balanceOf(address(treasuryInstance)), + treasuryBalanceBefore + treasuryFee, + 1e1 + ); + assertApproxEqAbs( + address(user).balance, + userBalanceBefore + userReceives, + 1e1 + ); + + } else { + vm.expectRevert(); + etherFiWithdrawalBufferInstance.redeemEEth(redeemAmount, user, user); + } + vm.stopPrank(); + } + + function testFuzz_redeemWeEth( + uint256 depositAmount, + uint256 redeemAmount, + uint256 exitFeeSplitBps, + uint16 exitFeeBps, + uint16 lowWatermarkBps + ) public { + // Bound the parameters + depositAmount = bound(depositAmount, 1 ether, 1000 ether); + redeemAmount = bound(redeemAmount, 0.1 ether, depositAmount); + exitFeeSplitBps = bound(exitFeeSplitBps, 0, 10000); + exitFeeBps = uint16(bound(uint256(exitFeeBps), 0, 10000)); + lowWatermarkBps = uint16(bound(uint256(lowWatermarkBps), 0, 10000)); + + // Deal Ether to user and perform deposit + vm.deal(user, depositAmount); + vm.startPrank(user); + liquidityPoolInstance.deposit{value: depositAmount}(); + vm.stopPrank(); + + // Set fee and watermark configurations + vm.prank(owner); + etherFiWithdrawalBufferInstance.setExitFeeSplitToTreasuryInBps(uint16(exitFeeSplitBps)); + + vm.prank(owner); + etherFiWithdrawalBufferInstance.setExitFeeBasisPoints(exitFeeBps); + + vm.prank(owner); + etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(lowWatermarkBps); + + // User approves weETH and attempts redemption + vm.startPrank(user); + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), redeemAmount); + uint256 totalRedeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); + + if (redeemAmount <= totalRedeemableAmount && etherFiWithdrawalBufferInstance.canRedeem(redeemAmount)) { + uint256 userBalanceBefore = address(user).balance; + uint256 treasuryBalanceBefore = eETHInstance.balanceOf(address(treasuryInstance)); + + etherFiWithdrawalBufferInstance.redeemWeEth(redeemAmount, user, user); + + uint256 totalFee = (redeemAmount * exitFeeBps) / 10000; + uint256 treasuryFee = (totalFee * exitFeeSplitBps) / 10000; + uint256 userReceives = redeemAmount - totalFee; + + assertApproxEqAbs( + eETHInstance.balanceOf(address(treasuryInstance)), + treasuryBalanceBefore + treasuryFee, + 1e1 + ); + assertApproxEqAbs( + address(user).balance, + userBalanceBefore + userReceives, + 1e1 + ); + + } else { + vm.expectRevert(); + etherFiWithdrawalBufferInstance.redeemWeEth(redeemAmount, user, user); + } + vm.stopPrank(); + } + + function testFuzz_role_management(address admin, address pauser, address unpauser, address user) public { + address owner = roleRegistry.owner(); + bytes32 PROTOCOL_ADMIN = keccak256("PROTOCOL_ADMIN"); + bytes32 PROTOCOL_PAUSER = keccak256("PROTOCOL_PAUSER"); + bytes32 PROTOCOL_UNPAUSER = keccak256("PROTOCOL_UNPAUSER"); + + vm.assume(admin != address(0) && admin != owner); + vm.assume(pauser != address(0) && pauser != owner && pauser != admin); + vm.assume(unpauser != address(0) && unpauser != owner && unpauser != admin && unpauser != pauser); + vm.assume(user != address(0) && user != owner && user != admin && user != pauser && user != unpauser); + + // Grant roles to respective addresses + vm.prank(owner); + roleRegistry.grantRole(PROTOCOL_ADMIN, admin); + vm.prank(owner); + roleRegistry.grantRole(PROTOCOL_PAUSER, pauser); + vm.prank(owner); + roleRegistry.grantRole(PROTOCOL_UNPAUSER, unpauser); + + // Admin performs admin-only actions + vm.startPrank(admin); + etherFiWithdrawalBufferInstance.setCapacity(10 ether); + etherFiWithdrawalBufferInstance.setRefillRatePerSecond(0.001 ether); + etherFiWithdrawalBufferInstance.setExitFeeSplitToTreasuryInBps(1e4); + etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(1e2); + etherFiWithdrawalBufferInstance.setExitFeeBasisPoints(1e2); + vm.stopPrank(); + + // Pauser pauses the contract + vm.startPrank(pauser); + etherFiWithdrawalBufferInstance.pauseContract(); + assertTrue(etherFiWithdrawalBufferInstance.paused()); + vm.stopPrank(); + + // Unpauser unpauses the contract + vm.startPrank(unpauser); + etherFiWithdrawalBufferInstance.unPauseContract(); + assertFalse(etherFiWithdrawalBufferInstance.paused()); + vm.stopPrank(); + + // Revoke PROTOCOL_ADMIN role from admin + vm.prank(owner); + roleRegistry.revokeRole(PROTOCOL_ADMIN, admin); + + // Admin attempts admin-only actions after role revocation + vm.startPrank(admin); + vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); + etherFiWithdrawalBufferInstance.setCapacity(10 ether); + vm.stopPrank(); + + // Pauser attempts to unpause (should fail) + vm.startPrank(pauser); + vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); + etherFiWithdrawalBufferInstance.unPauseContract(); + vm.stopPrank(); + + // Unpauser attempts to pause (should fail) + vm.startPrank(unpauser); + vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); + etherFiWithdrawalBufferInstance.pauseContract(); + vm.stopPrank(); + + // User without role attempts admin-only actions + vm.startPrank(user); + vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); + etherFiWithdrawalBufferInstance.pauseContract(); + vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); + etherFiWithdrawalBufferInstance.unPauseContract(); + vm.stopPrank(); + } + + function test_mainnet_redeem_eEth() public { + vm.deal(user, 100 ether); + vm.startPrank(user); + + assertEq(etherFiWithdrawalBufferInstance.canRedeem(1 ether), true); + assertEq(etherFiWithdrawalBufferInstance.canRedeem(10 ether), false); + + liquidityPoolInstance.deposit{value: 1 ether}(); + + uint256 userBalance = address(user).balance; + uint256 treasuryBalance = eETHInstance.balanceOf(address(etherFiWithdrawalBufferInstance.treasury())); + + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); + etherFiWithdrawalBufferInstance.redeemEEth(1 ether, user, user); + + assertEq(eETHInstance.balanceOf(address(etherFiWithdrawalBufferInstance.treasury())), treasuryBalance + 0.01 ether); + assertEq(address(user).balance, userBalance + 0.99 ether); + + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 5 ether); + vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); + etherFiWithdrawalBufferInstance.redeemEEth(5 ether, user, user); + + vm.stopPrank(); + } } diff --git a/test/TestSetup.sol b/test/TestSetup.sol index 423a3b612..9c9fe030c 100644 --- a/test/TestSetup.sol +++ b/test/TestSetup.sol @@ -106,6 +106,7 @@ contract TestSetup is Test { UUPSProxy public etherFiWithdrawalBufferProxy; UUPSProxy public etherFiOracleProxy; UUPSProxy public etherFiAdminProxy; + UUPSProxy public roleRegistryProxy; DepositDataGeneration public depGen; IDepositContract public depositContractEth2; @@ -190,6 +191,8 @@ contract TestSetup is Test { EtherFiTimelock public etherFiTimelockInstance; BucketRateLimiter public bucketRateLimiter; + RoleRegistry public roleRegistry; + bytes32 root; bytes32 rootMigration; bytes32 rootMigration2; @@ -392,6 +395,7 @@ contract TestSetup is Test { etherFiTimelockInstance = EtherFiTimelock(payable(addressProviderInstance.getContractAddress("EtherFiTimelock"))); etherFiAdminInstance = EtherFiAdmin(payable(addressProviderInstance.getContractAddress("EtherFiAdmin"))); etherFiOracleInstance = EtherFiOracle(payable(addressProviderInstance.getContractAddress("EtherFiOracle"))); + roleRegistry = RoleRegistry(0x1d3Af47C1607A2EF33033693A9989D1d1013BB50); } function setUpLiquifier(uint8 forkEnum) internal { @@ -576,9 +580,15 @@ contract TestSetup is Test { etherFiRestakerProxy = new UUPSProxy(address(etherFiRestakerImplementation), ""); etherFiRestakerInstance = EtherFiRestaker(payable(etherFiRestakerProxy)); - etherFiWithdrawalBufferProxy = new UUPSProxy(address(new EtherFiWithdrawalBuffer(address(liquidityPoolInstance), address(eETHInstance), address(weEthInstance), address(treasuryInstance))), ""); + roleRegistryProxy = new UUPSProxy(address(new RoleRegistry()), ""); + roleRegistry = RoleRegistry(address(roleRegistryProxy)); + roleRegistry.initialize(owner); + + etherFiWithdrawalBufferProxy = new UUPSProxy(address(new EtherFiWithdrawalBuffer(address(liquidityPoolInstance), address(eETHInstance), address(weEthInstance), address(treasuryInstance), address(roleRegistry))), ""); etherFiWithdrawalBufferInstance = EtherFiWithdrawalBuffer(payable(etherFiWithdrawalBufferProxy)); - etherFiWithdrawalBufferInstance.initialize(0, 1_00, 10_00); + etherFiWithdrawalBufferInstance.initialize(10_00, 1_00, 1_00, 5 ether, 0.001 ether); + + roleRegistry.grantRole(keccak256("PROTOCOL_ADMIN"), owner); liquidityPoolInstance.initialize(address(eETHInstance), address(stakingManagerInstance), address(etherFiNodeManagerProxy), address(membershipManagerInstance), address(TNFTInstance), address(etherFiAdminProxy), address(withdrawRequestNFTInstance)); liquidityPoolInstance.initializeOnUpgradeWithWithdrawalBuffer(address(etherFiWithdrawalBufferInstance)); From c9fd604d8b0017e7b9faa5cdbf017ea374a827cc Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 19 Dec 2024 16:38:40 +0900 Subject: [PATCH 05/42] use setter for 'shareRemainderSplitToTreasuryInBps', add more fuzz tests, add deploy script --- .../DeployEtherFiWithdrawalBuffer.s.sol | 40 +++ script/deploys/DeployPhaseTwo.s.sol | 2 +- .../WithdrawRequestNFTUpgradeScript.s.sol | 2 +- src/WithdrawRequestNFT.sol | 3 +- test/EtherFiWithdrawalBuffer.t.sol | 270 +++++++----------- test/TestSetup.sol | 2 +- test/WithdrawRequestNFT.t.sol | 254 ++++++++++++---- 7 files changed, 353 insertions(+), 220 deletions(-) create mode 100644 script/deploys/DeployEtherFiWithdrawalBuffer.s.sol diff --git a/script/deploys/DeployEtherFiWithdrawalBuffer.s.sol b/script/deploys/DeployEtherFiWithdrawalBuffer.s.sol new file mode 100644 index 000000000..34f132e0c --- /dev/null +++ b/script/deploys/DeployEtherFiWithdrawalBuffer.s.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "forge-std/Script.sol"; + +import "@openzeppelin/contracts/utils/Strings.sol"; + +import "../../src/Liquifier.sol"; +import "../../src/EtherFiRestaker.sol"; +import "../../src/helpers/AddressProvider.sol"; +import "../../src/UUPSProxy.sol"; +import "../../src/EtherFiWithdrawalBuffer.sol"; + + +contract Deploy is Script { + using Strings for string; + AddressProvider public addressProvider; + + function run() external { + uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY"); + address addressProviderAddress = vm.envAddress("CONTRACT_REGISTRY"); + addressProvider = AddressProvider(addressProviderAddress); + + vm.startBroadcast(deployerPrivateKey); + + EtherFiWithdrawalBuffer impl = new EtherFiWithdrawalBuffer( + addressProvider.getContractAddress("LiquidityPool"), + addressProvider.getContractAddress("EETH"), + addressProvider.getContractAddress("WeETH"), + 0x0c83EAe1FE72c390A02E426572854931EefF93BA, // protocol safe + 0x1d3Af47C1607A2EF33033693A9989D1d1013BB50 // role registry + ); + UUPSProxy proxy = new UUPSProxy(payable(impl), ""); + + EtherFiWithdrawalBuffer instance = EtherFiWithdrawalBuffer(payable(proxy)); + instance.initialize(10_00, 1_00, 1_00, 5 ether, 0.001 ether); + + vm.stopBroadcast(); + } +} diff --git a/script/deploys/DeployPhaseTwo.s.sol b/script/deploys/DeployPhaseTwo.s.sol index 2417e191b..c61d13feb 100644 --- a/script/deploys/DeployPhaseTwo.s.sol +++ b/script/deploys/DeployPhaseTwo.s.sol @@ -81,7 +81,7 @@ contract DeployPhaseTwoScript is Script { } retrieve_contract_addresses(); - withdrawRequestNftImplementation = new WithdrawRequestNFT(address(0), 0); + withdrawRequestNftImplementation = new WithdrawRequestNFT(address(0)); withdrawRequestNftProxy = new UUPSProxy(address(withdrawRequestNftImplementation), ""); withdrawRequestNftInstance = WithdrawRequestNFT(payable(withdrawRequestNftProxy)); diff --git a/script/upgrades/WithdrawRequestNFTUpgradeScript.s.sol b/script/upgrades/WithdrawRequestNFTUpgradeScript.s.sol index 5862fb36e..115251475 100644 --- a/script/upgrades/WithdrawRequestNFTUpgradeScript.s.sol +++ b/script/upgrades/WithdrawRequestNFTUpgradeScript.s.sol @@ -20,7 +20,7 @@ contract WithdrawRequestNFTUpgrade is Script { vm.startBroadcast(deployerPrivateKey); WithdrawRequestNFT oracleInstance = WithdrawRequestNFT(proxyAddress); - WithdrawRequestNFT v2Implementation = new WithdrawRequestNFT(address(0), 0); + WithdrawRequestNFT v2Implementation = new WithdrawRequestNFT(address(0)); oracleInstance.upgradeTo(address(v2Implementation)); diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 439f0fa24..d3f6f29fe 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -38,9 +38,8 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad event HandledRemainderOfClaimedWithdrawRequests(uint256 eEthAmountToTreasury, uint256 eEthAmountBurnt); /// @custom:oz-upgrades-unsafe-allow constructor - constructor(address _treasury, uint16 _shareRemainderSplitToTreasuryInBps) { + constructor(address _treasury) { treasury = _treasury; - shareRemainderSplitToTreasuryInBps = _shareRemainderSplitToTreasuryInBps; _disableInitializers(); } diff --git a/test/EtherFiWithdrawalBuffer.t.sol b/test/EtherFiWithdrawalBuffer.t.sol index d73268fd0..5073ed989 100644 --- a/test/EtherFiWithdrawalBuffer.t.sol +++ b/test/EtherFiWithdrawalBuffer.t.sol @@ -11,29 +11,30 @@ contract EtherFiWithdrawalBufferTest is TestSetup { function setUp() public { setUpTests(); - - vm.startPrank(owner); - etherFiWithdrawalBufferInstance.setCapacity(10 ether); - etherFiWithdrawalBufferInstance.setRefillRatePerSecond(0.001 ether); - etherFiWithdrawalBufferInstance.setExitFeeSplitToTreasuryInBps(1e4); - vm.stopPrank(); - - vm.warp(block.timestamp + 5 * 1000); // 0.001 ether * 5000 = 5 ether refilled } function setUp_Fork() public { initializeRealisticFork(MAINNET_FORK); - vm.startPrank(owner); + vm.startPrank(roleRegistry.owner()); roleRegistry.grantRole(keccak256("PROTOCOL_ADMIN"), op_admin); vm.stopPrank(); etherFiWithdrawalBufferProxy = new UUPSProxy(address(new EtherFiWithdrawalBuffer(address(liquidityPoolInstance), address(eETHInstance), address(weEthInstance), address(treasuryInstance), address(roleRegistry))), ""); etherFiWithdrawalBufferInstance = EtherFiWithdrawalBuffer(payable(etherFiWithdrawalBufferProxy)); - etherFiWithdrawalBufferInstance.initialize(1e4, 1_00, 10_00); // 10% fee split to treasury, 1% exit fee, 10% low watermark + etherFiWithdrawalBufferInstance.initialize(10_00, 1_00, 1_00, 5 ether, 0.001 ether); // 10% fee split to treasury, 1% exit fee, 1% low watermark + + _upgrade_liquidity_pool_contract(); + + vm.prank(liquidityPoolInstance.owner()); + liquidityPoolInstance.initializeOnUpgradeWithWithdrawalBuffer(address(etherFiWithdrawalBufferInstance)); } function test_rate_limit() public { + vm.deal(user, 1000 ether); + vm.prank(user); + liquidityPoolInstance.deposit{value: 1000 ether}(); + assertEq(etherFiWithdrawalBufferInstance.canRedeem(1 ether), true); assertEq(etherFiWithdrawalBufferInstance.canRedeem(5 ether - 1), true); assertEq(etherFiWithdrawalBufferInstance.canRedeem(5 ether + 1), false); @@ -58,132 +59,11 @@ contract EtherFiWithdrawalBufferTest is TestSetup { etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(100_00); // 100% assertEq(etherFiWithdrawalBufferInstance.lowWatermarkInETH(), 100 ether); - } - - function test_redeem_eEth() public { - vm.deal(user, 100 ether); - vm.startPrank(user); - - assertEq(etherFiWithdrawalBufferInstance.canRedeem(1 ether), true); - assertEq(etherFiWithdrawalBufferInstance.canRedeem(10 ether), false); - - liquidityPoolInstance.deposit{value: 1 ether}(); - - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 0.5 ether); - vm.expectRevert("TRANSFER_AMOUNT_EXCEEDS_ALLOWANCE"); - etherFiWithdrawalBufferInstance.redeemEEth(1 ether, user, user); - - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 2 ether); - vm.expectRevert("EtherFiWithdrawalBuffer: Insufficient balance"); - etherFiWithdrawalBufferInstance.redeemEEth(2 ether, user, user); - - liquidityPoolInstance.deposit{value: 10 ether}(); - - uint256 totalRedeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); - uint256 userBalance = address(user).balance; - uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); - etherFiWithdrawalBufferInstance.redeemEEth(1 ether, user, user); - assertEq(eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalance + 0.01 ether); - assertEq(address(user).balance, userBalance + 0.99 ether); - assertEq(etherFiWithdrawalBufferInstance.totalRedeemableAmount(), totalRedeemableAmount - 1 ether); - - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 5 ether); - vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); - etherFiWithdrawalBufferInstance.redeemEEth(5 ether, user, user); - - vm.stopPrank(); - } - - function test_mainnet_redeem_weEth_with_rebase() public { - vm.deal(user, 100 ether); - - vm.startPrank(user); - liquidityPoolInstance.deposit{value: 10 ether}(); - eETHInstance.approve(address(weEthInstance), 10 ether); - weEthInstance.wrap(10 ether); - vm.stopPrank(); - - uint256 one_percent_of_tvl = liquidityPoolInstance.getTotalPooledEther() / 100; - - vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(int128(uint128(one_percent_of_tvl))); // 10 eETH earned 1 ETH - - vm.startPrank(user); - uint256 userBalance = address(user).balance; - uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); - etherFiWithdrawalBufferInstance.redeemWeEth(1 ether, user, user); - assertEq(eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalance + 0.0101 ether); - assertEq(address(user).balance, userBalance + 0.9999 ether); - vm.stopPrank(); - } - - function test_redeem_weEth_1() public { - vm.deal(user, 100 ether); - vm.startPrank(user); - - assertEq(etherFiWithdrawalBufferInstance.canRedeem(1 ether), true); - assertEq(etherFiWithdrawalBufferInstance.canRedeem(10 ether), false); - - liquidityPoolInstance.deposit{value: 1 ether}(); - eETHInstance.approve(address(weEthInstance), 1 ether); - weEthInstance.wrap(1 ether); - - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 0.5 ether); - vm.expectRevert("ERC20: insufficient allowance"); - etherFiWithdrawalBufferInstance.redeemWeEth(1 ether, user, user); - - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 2 ether); - vm.expectRevert("EtherFiWithdrawalBuffer: Insufficient balance"); - etherFiWithdrawalBufferInstance.redeemWeEth(2 ether, user, user); - - liquidityPoolInstance.deposit{value: 10 ether}(); - - uint256 totalRedeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); - uint256 userBalance = address(user).balance; - uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); - etherFiWithdrawalBufferInstance.redeemWeEth(1 ether, user, user); - assertEq(eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalance + 0.01 ether); - assertEq(address(user).balance, userBalance + 0.99 ether); - assertEq(etherFiWithdrawalBufferInstance.totalRedeemableAmount(), totalRedeemableAmount - 1 ether); - - eETHInstance.approve(address(weEthInstance), 6 ether); - weEthInstance.wrap(6 ether); - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 5 ether); - vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); - etherFiWithdrawalBufferInstance.redeemWeEth(5 ether, user, user); - - vm.stopPrank(); - } - - function test_redeem_weEth_with_varying_exchange_rate() public { - vm.deal(user, 100 ether); - - vm.startPrank(user); - liquidityPoolInstance.deposit{value: 10 ether}(); - eETHInstance.approve(address(weEthInstance), 1 ether); - weEthInstance.wrap(1 ether); - vm.stopPrank(); - vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(1 ether); // 10 eETH earned 1 ETH - - vm.startPrank(user); - uint256 userBalance = address(user).balance; - uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); - etherFiWithdrawalBufferInstance.redeemWeEth(1 ether, user, user); - assertEq(eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalance + 0.011 ether); - assertEq(address(user).balance, userBalance + (1.1 ether - 0.011 ether)); - vm.stopPrank(); + vm.expectRevert("INVALID"); + etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(100_01); // 100.01% } - // The test ensures that: - // - Redemption works correctly within allowed limits. - // - Fees are applied accurately. - // - The function properly reverts when redemption conditions aren't met. function testFuzz_redeemEEth( uint256 depositAmount, uint256 redeemAmount, @@ -214,13 +94,11 @@ contract EtherFiWithdrawalBufferTest is TestSetup { etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(lowWatermarkBps); vm.startPrank(user); - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), redeemAmount); - uint256 totalRedeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); - - if (redeemAmount <= totalRedeemableAmount && etherFiWithdrawalBufferInstance.canRedeem(redeemAmount)) { + if (etherFiWithdrawalBufferInstance.canRedeem(redeemAmount)) { uint256 userBalanceBefore = address(user).balance; uint256 treasuryBalanceBefore = eETHInstance.balanceOf(address(treasuryInstance)); + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), redeemAmount); etherFiWithdrawalBufferInstance.redeemEEth(redeemAmount, user, user); uint256 totalFee = (redeemAmount * exitFeeBps) / 10000; @@ -230,12 +108,12 @@ contract EtherFiWithdrawalBufferTest is TestSetup { assertApproxEqAbs( eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalanceBefore + treasuryFee, - 1e1 + 1e2 ); assertApproxEqAbs( address(user).balance, userBalanceBefore + userReceives, - 1e1 + 1e2 ); } else { @@ -248,16 +126,18 @@ contract EtherFiWithdrawalBufferTest is TestSetup { function testFuzz_redeemWeEth( uint256 depositAmount, uint256 redeemAmount, - uint256 exitFeeSplitBps, + uint16 exitFeeSplitBps, + int256 rebase, uint16 exitFeeBps, uint16 lowWatermarkBps ) public { // Bound the parameters depositAmount = bound(depositAmount, 1 ether, 1000 ether); redeemAmount = bound(redeemAmount, 0.1 ether, depositAmount); - exitFeeSplitBps = bound(exitFeeSplitBps, 0, 10000); - exitFeeBps = uint16(bound(uint256(exitFeeBps), 0, 10000)); - lowWatermarkBps = uint16(bound(uint256(lowWatermarkBps), 0, 10000)); + exitFeeSplitBps = uint16(bound(exitFeeSplitBps, 0, 10000)); + exitFeeBps = uint16(bound(exitFeeBps, 0, 10000)); + lowWatermarkBps = uint16(bound(lowWatermarkBps, 0, 10000)); + rebase = bound(rebase, 0, int128(uint128(depositAmount) / 10)); // Deal Ether to user and perform deposit vm.deal(user, depositAmount); @@ -265,6 +145,10 @@ contract EtherFiWithdrawalBufferTest is TestSetup { liquidityPoolInstance.deposit{value: depositAmount}(); vm.stopPrank(); + // Apply rebase + vm.prank(address(membershipManagerInstance)); + liquidityPoolInstance.rebase(int128(rebase)); + // Set fee and watermark configurations vm.prank(owner); etherFiWithdrawalBufferInstance.setExitFeeSplitToTreasuryInBps(uint16(exitFeeSplitBps)); @@ -275,35 +159,39 @@ contract EtherFiWithdrawalBufferTest is TestSetup { vm.prank(owner); etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(lowWatermarkBps); - // User approves weETH and attempts redemption + // Convert redeemAmount from ETH to weETH vm.startPrank(user); - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), redeemAmount); - uint256 totalRedeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); + eETHInstance.approve(address(weEthInstance), redeemAmount); + weEthInstance.wrap(redeemAmount); + uint256 weEthAmount = weEthInstance.balanceOf(user); - if (redeemAmount <= totalRedeemableAmount && etherFiWithdrawalBufferInstance.canRedeem(redeemAmount)) { + if (etherFiWithdrawalBufferInstance.canRedeem(redeemAmount)) { uint256 userBalanceBefore = address(user).balance; uint256 treasuryBalanceBefore = eETHInstance.balanceOf(address(treasuryInstance)); - etherFiWithdrawalBufferInstance.redeemWeEth(redeemAmount, user, user); + uint256 eEthAmount = liquidityPoolInstance.amountForShare(weEthAmount); - uint256 totalFee = (redeemAmount * exitFeeBps) / 10000; + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), weEthAmount); + etherFiWithdrawalBufferInstance.redeemWeEth(weEthAmount, user, user); + + uint256 totalFee = (eEthAmount * exitFeeBps) / 10000; uint256 treasuryFee = (totalFee * exitFeeSplitBps) / 10000; - uint256 userReceives = redeemAmount - totalFee; + uint256 userReceives = eEthAmount - totalFee; assertApproxEqAbs( eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalanceBefore + treasuryFee, - 1e1 + 1e2 ); assertApproxEqAbs( address(user).balance, userBalanceBefore + userReceives, - 1e1 + 1e2 ); } else { vm.expectRevert(); - etherFiWithdrawalBufferInstance.redeemWeEth(redeemAmount, user, user); + etherFiWithdrawalBufferInstance.redeemWeEth(weEthAmount, user, user); } vm.stopPrank(); } @@ -380,22 +268,26 @@ contract EtherFiWithdrawalBufferTest is TestSetup { } function test_mainnet_redeem_eEth() public { + setUp_Fork(); + vm.deal(user, 100 ether); vm.startPrank(user); - assertEq(etherFiWithdrawalBufferInstance.canRedeem(1 ether), true); - assertEq(etherFiWithdrawalBufferInstance.canRedeem(10 ether), false); - - liquidityPoolInstance.deposit{value: 1 ether}(); + liquidityPoolInstance.deposit{value: 10 ether}(); + uint256 redeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); uint256 userBalance = address(user).balance; uint256 treasuryBalance = eETHInstance.balanceOf(address(etherFiWithdrawalBufferInstance.treasury())); eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); etherFiWithdrawalBufferInstance.redeemEEth(1 ether, user, user); - assertEq(eETHInstance.balanceOf(address(etherFiWithdrawalBufferInstance.treasury())), treasuryBalance + 0.01 ether); - assertEq(address(user).balance, userBalance + 0.99 ether); + uint256 totalFee = (1 ether * 1e2) / 1e4; + uint256 treasuryFee = (totalFee * 1e3) / 1e4; + uint256 userReceives = 1 ether - totalFee; + + assertApproxEqAbs(eETHInstance.balanceOf(address(etherFiWithdrawalBufferInstance.treasury())), treasuryBalance + treasuryFee, 1e1); + assertApproxEqAbs(address(user).balance, userBalance + userReceives, 1e1); eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 5 ether); vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); @@ -403,4 +295,64 @@ contract EtherFiWithdrawalBufferTest is TestSetup { vm.stopPrank(); } + + function test_mainnet_redeem_weEth_with_rebase() public { + setUp_Fork(); + + vm.deal(user, 100 ether); + + vm.startPrank(user); + liquidityPoolInstance.deposit{value: 10 ether}(); + eETHInstance.approve(address(weEthInstance), 10 ether); + weEthInstance.wrap(1 ether); + vm.stopPrank(); + + uint256 one_percent_of_tvl = liquidityPoolInstance.getTotalPooledEther() / 100; + + vm.prank(address(membershipManagerV1Instance)); + liquidityPoolInstance.rebase(int128(uint128(one_percent_of_tvl))); // 10 eETH earned 1 ETH + + vm.startPrank(user); + uint256 weEthAmount = weEthInstance.balanceOf(user); + uint256 eEthAmount = liquidityPoolInstance.amountForShare(weEthAmount); + uint256 userBalance = address(user).balance; + uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); + weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); + etherFiWithdrawalBufferInstance.redeemWeEth(weEthAmount, user, user); + + uint256 totalFee = (eEthAmount * 1e2) / 1e4; + uint256 treasuryFee = (totalFee * 1e3) / 1e4; + uint256 userReceives = eEthAmount - totalFee; + + assertApproxEqAbs(eETHInstance.balanceOf(address(treasuryInstance)), treasuryBalance + treasuryFee, 1e1); + assertApproxEqAbs(address(user).balance, userBalance + userReceives, 1e1); + + vm.stopPrank(); + } + + function test_mainnet_redeem_beyond_liquidity_fails() public { + setUp_Fork(); + + uint256 redeemAmount = liquidityPoolInstance.getTotalPooledEther() / 2; + vm.prank(address(liquidityPoolInstance)); + eETHInstance.mintShares(user, 2 * redeemAmount); + + vm.startPrank(op_admin); + etherFiWithdrawalBufferInstance.setCapacity(2 * redeemAmount); + etherFiWithdrawalBufferInstance.setRefillRatePerSecond(2 * redeemAmount); + vm.stopPrank(); + + vm.warp(block.timestamp + 1); + + vm.startPrank(user); + + uint256 userBalance = address(user).balance; + uint256 treasuryBalance = eETHInstance.balanceOf(address(etherFiWithdrawalBufferInstance.treasury())); + + eETHInstance.approve(address(etherFiWithdrawalBufferInstance), redeemAmount); + vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); + etherFiWithdrawalBufferInstance.redeemEEth(redeemAmount, user, user); + + vm.stopPrank(); + } } diff --git a/test/TestSetup.sol b/test/TestSetup.sol index 9c9fe030c..af6a4ca82 100644 --- a/test/TestSetup.sol +++ b/test/TestSetup.sol @@ -559,7 +559,7 @@ contract TestSetup is Test { membershipNftProxy = new UUPSProxy(address(membershipNftImplementation), ""); membershipNftInstance = MembershipNFT(payable(membershipNftProxy)); - withdrawRequestNFTImplementation = new WithdrawRequestNFT(address(0), 0); + withdrawRequestNFTImplementation = new WithdrawRequestNFT(address(treasuryInstance)); withdrawRequestNFTProxy = new UUPSProxy(address(withdrawRequestNFTImplementation), ""); withdrawRequestNFTInstance = WithdrawRequestNFT(payable(withdrawRequestNFTProxy)); diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 924545421..816d65424 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -14,60 +14,6 @@ contract WithdrawRequestNFTTest is TestSetup { setUpTests(); } - function test_WithdrawRequestNftInitializedCorrectly() public { - assertEq(address(withdrawRequestNFTInstance.liquidityPool()), address(liquidityPoolInstance)); - assertEq(address(withdrawRequestNFTInstance.eETH()), address(eETHInstance)); - } - - function test_RequestWithdraw() public { - startHoax(bob); - liquidityPoolInstance.deposit{value: 10 ether}(); - vm.stopPrank(); - - assertEq(liquidityPoolInstance.getTotalPooledEther(), 10 ether); - assertEq(eETHInstance.balanceOf(address(bob)), 10 ether); - - uint96 amountOfEEth = 1 ether; - - vm.prank(bob); - eETHInstance.approve(address(liquidityPoolInstance), amountOfEEth); - - vm.prank(bob); - uint256 requestId = liquidityPoolInstance.requestWithdraw(bob, amountOfEEth); - - WithdrawRequestNFT.WithdrawRequest memory request = withdrawRequestNFTInstance.getRequest(requestId); - - assertEq(request.amountOfEEth, 1 ether, "Amount of eEth should match"); - assertEq(request.shareOfEEth, 1 ether, "Share of eEth should match"); - assertTrue(request.isValid, "Request should be valid"); - } - - function test_RequestIdIncrements() public { - startHoax(bob); - liquidityPoolInstance.deposit{value: 10 ether}(); - vm.stopPrank(); - - assertEq(liquidityPoolInstance.getTotalPooledEther(), 10 ether); - - uint96 amountOfEEth = 1 ether; - - vm.prank(bob); - eETHInstance.approve(address(liquidityPoolInstance), amountOfEEth); - - vm.prank(bob); - uint256 requestId1 = liquidityPoolInstance.requestWithdraw(bob, amountOfEEth); - - assertEq(requestId1, 1, "Request id should be 1"); - - vm.prank(bob); - eETHInstance.approve(address(liquidityPoolInstance), amountOfEEth); - - vm.prank(bob); - uint256 requestId2 = liquidityPoolInstance.requestWithdraw(bob, amountOfEEth); - - assertEq(requestId2, 2, "Request id should be 2"); - } - function test_finalizeRequests() public { startHoax(bob); liquidityPoolInstance.deposit{value: 10 ether}(); @@ -183,6 +129,7 @@ contract WithdrawRequestNFTTest is TestSetup { assertEq(eETHInstance.balanceOf(bob), 9 ether); assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 1 ether, "eETH balance should be 1 ether"); + assertEq(eETHInstance.balanceOf(address(treasuryInstance)), 0 ether, "Treasury balance should be 0 ether"); // Rebase with accrued_rewards = 10 ether for the deposited 10 ether // -> 1 ether eETH shares = 2 ether ETH @@ -202,7 +149,7 @@ contract WithdrawRequestNFTTest is TestSetup { uint256 bobsEndingBalance = address(bob).balance; assertEq(bobsEndingBalance, bobsStartingBalance + 1 ether, "Bobs balance should be 1 ether higher"); - assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 1 ether, "eETH balance should be 1 ether"); + assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 0 ether, "eETH balance should be 0 ether"); } function test_ValidClaimWithdrawWithNegativeRebase() public { @@ -418,8 +365,203 @@ contract WithdrawRequestNFTTest is TestSetup { initializeRealisticFork(MAINNET_FORK); vm.startPrank(withdrawRequestNFTInstance.owner()); - withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(address(owner), 50_00))); + withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(address(owner)))); + + withdrawRequestNFTInstance.updateShareRemainderSplitToTreasuryInBps(50_00); withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIds); } + + function testFuzz_RequestWithdraw(uint96 depositAmount, uint96 withdrawAmount, address recipient) public { + // Assume valid conditions + vm.assume(depositAmount >= 1 ether && depositAmount <= 1000 ether); + vm.assume(withdrawAmount > 0 && withdrawAmount <= depositAmount); + vm.assume(recipient != address(0) && recipient != address(liquidityPoolInstance)); + + // Setup initial balance for bob + vm.deal(bob, depositAmount); + + // Deposit ETH and get eETH + vm.startPrank(bob); + liquidityPoolInstance.deposit{value: depositAmount}(); + + // Approve and request withdraw + eETHInstance.approve(address(liquidityPoolInstance), withdrawAmount); + uint256 requestId = liquidityPoolInstance.requestWithdraw(recipient, withdrawAmount); + vm.stopPrank(); + + // Verify the request was created correctly + WithdrawRequestNFT.WithdrawRequest memory request = withdrawRequestNFTInstance.getRequest(requestId); + + assertEq(request.amountOfEEth, withdrawAmount, "Incorrect withdrawal amount"); + assertEq(request.shareOfEEth, liquidityPoolInstance.sharesForAmount(withdrawAmount), "Incorrect share amount"); + assertTrue(request.isValid, "Request should be valid"); + assertEq(withdrawRequestNFTInstance.ownerOf(requestId), recipient, "Incorrect NFT owner"); + + // Verify eETH balances + assertEq(eETHInstance.balanceOf(bob), depositAmount - withdrawAmount, "Incorrect remaining eETH balance"); + assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), withdrawAmount, "Incorrect contract eETH balance"); + assertEq(withdrawRequestNFTInstance.nextRequestId(), requestId + 1, "Incorrect next request ID"); + + if (eETHInstance.balanceOf(bob) > 0) { + uint256 reqAmount = eETHInstance.balanceOf(bob); + vm.startPrank(bob); + eETHInstance.approve(address(liquidityPoolInstance), reqAmount); + uint256 requestId2 = liquidityPoolInstance.requestWithdraw(recipient, reqAmount); + vm.stopPrank(); + assertEq(requestId2, requestId + 1, "Incorrect next request ID"); + } + } + + function testFuzz_ClaimWithdraw( + uint96 depositAmount, + uint96 withdrawAmount, + uint96 rebaseAmount, + uint16 remainderSplitBps, + address recipient + ) public { + // Assume valid conditions + vm.assume(depositAmount >= 1 ether && depositAmount <= 1e6 ether); + vm.assume(withdrawAmount > 0 && withdrawAmount <= depositAmount); + vm.assume(rebaseAmount >= 0 && rebaseAmount <= depositAmount); + vm.assume(remainderSplitBps <= 10000); + vm.assume(recipient != address(0) && recipient != address(liquidityPoolInstance)); + + // Setup initial balance for recipient + vm.deal(recipient, depositAmount); + + // Configure remainder split + vm.prank(withdrawRequestNFTInstance.owner()); + withdrawRequestNFTInstance.updateShareRemainderSplitToTreasuryInBps(remainderSplitBps); + + // First deposit ETH to get eETH + vm.startPrank(recipient); + liquidityPoolInstance.deposit{value: depositAmount}(); + + // Record initial balances + uint256 treasuryEEthBefore = eETHInstance.balanceOf(address(treasuryInstance)); + uint256 recipientBalanceBefore = address(recipient).balance; + + // Request withdraw + eETHInstance.approve(address(liquidityPoolInstance), withdrawAmount); + uint256 requestId = liquidityPoolInstance.requestWithdraw(recipient, withdrawAmount); + vm.stopPrank(); + + // Get initial request state + WithdrawRequestNFT.WithdrawRequest memory request = withdrawRequestNFTInstance.getRequest(requestId); + + // Simulate rebase after request but before claim + vm.prank(address(membershipManagerInstance)); + liquidityPoolInstance.rebase(int128(uint128(rebaseAmount))); + + // Calculate expected withdrawal amounts after rebase + uint256 sharesValue = liquidityPoolInstance.amountForShare(request.shareOfEEth); + uint256 expectedWithdrawAmount = withdrawAmount < sharesValue ? withdrawAmount : sharesValue; + uint256 unusedShares = request.shareOfEEth - liquidityPoolInstance.sharesForWithdrawalAmount(expectedWithdrawAmount); + uint256 expectedTreasuryShares = (unusedShares * remainderSplitBps) / 10000; + uint256 expectedBurnedShares = request.shareOfEEth - expectedTreasuryShares; + assertGe(unusedShares, 0, "Unused shares should be non-negative because there was positive rebase"); + + // Track initial shares and total supply + uint256 initialTotalShares = eETHInstance.totalShares(); + + _finalizeWithdrawalRequest(requestId); + + vm.prank(recipient); + withdrawRequestNFTInstance.claimWithdraw(requestId); + + // Calculate expected burnt shares + uint256 burnedShares = initialTotalShares - eETHInstance.totalShares(); + + // Verify share burning + assertApproxEqAbs( + burnedShares, + expectedBurnedShares, + 1e1, + "Incorrect amount of shares burnt" + ); + assertLe(burnedShares, request.shareOfEEth, "Burned shares should be less than or equal to requested shares"); + + // Verify total supply reduction + assertApproxEqAbs( + eETHInstance.totalShares(), + initialTotalShares - burnedShares, + 1, + "Total shares not reduced correctly" + ); + assertGe( + eETHInstance.totalShares(), + initialTotalShares - burnedShares, + "Total shares should be greater than or equal to initial shares minus burned shares" + ); + + // Verify the withdrawal results + WithdrawRequestNFT.WithdrawRequest memory requestAfter = withdrawRequestNFTInstance.getRequest(requestId); + + // Request should be cleared + assertEq(requestAfter.amountOfEEth, 0, "Request should be cleared after claim"); + + // NFT should be burned + vm.expectRevert("ERC721: invalid token ID"); + withdrawRequestNFTInstance.ownerOf(requestId); + + // Calculate and verify remainder splitting + if (unusedShares > 0) { + assertApproxEqAbs( + eETHInstance.balanceOf(address(treasuryInstance)) - treasuryEEthBefore, + liquidityPoolInstance.amountForShare(expectedTreasuryShares), + 1e1, + "Incorrect treasury eETH balance" + ); + } + + // Verify recipient received correct ETH amount + assertEq( + address(recipient).balance, + recipientBalanceBefore + expectedWithdrawAmount, + "Recipient should receive correct ETH amount" + ); + } + + function testFuzz_InvalidateRequest(uint96 depositAmount, uint96 withdrawAmount, address recipient) public { + // Assume valid conditions + vm.assume(depositAmount >= 1 ether && depositAmount <= 1000 ether); + vm.assume(withdrawAmount > 0 && withdrawAmount <= depositAmount); + vm.assume(recipient != address(0) && recipient != address(liquidityPoolInstance) && !withdrawRequestNFTInstance.admins(recipient)); + + // Setup initial balance and deposit + vm.deal(recipient, depositAmount); + + vm.startPrank(recipient); + liquidityPoolInstance.deposit{value: depositAmount}(); + + // Request withdraw + eETHInstance.approve(address(liquidityPoolInstance), withdrawAmount); + uint256 requestId = liquidityPoolInstance.requestWithdraw(recipient, withdrawAmount); + vm.stopPrank(); + + // Verify request is initially valid + assertTrue(withdrawRequestNFTInstance.isValid(requestId), "Request should start valid"); + assertEq(withdrawRequestNFTInstance.ownerOf(requestId), recipient, "Recipient should own NFT"); + + // Non-admin cannot invalidate + vm.prank(recipient); + vm.expectRevert("Caller is not the admin"); + withdrawRequestNFTInstance.invalidateRequest(requestId); + + // Admin invalidates request + vm.prank(withdrawRequestNFTInstance.owner()); + withdrawRequestNFTInstance.updateAdmin(recipient, true); + vm.prank(recipient); + withdrawRequestNFTInstance.invalidateRequest(requestId); + + // Verify request state after invalidation + assertFalse(withdrawRequestNFTInstance.isValid(requestId), "Request should be invalid"); + assertEq(withdrawRequestNFTInstance.ownerOf(requestId), recipient, "NFT ownership should remain unchanged"); + + // Verify cannot transfer invalid request + vm.prank(recipient); + vm.expectRevert("INVALID_REQUEST"); + withdrawRequestNFTInstance.transferFrom(recipient, address(0xdead), requestId); + } } From c377e8e13d13661b35d63148598aa56ffb73b863 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Fri, 20 Dec 2024 10:24:20 +0900 Subject: [PATCH 06/42] handle issues in calculating the dust shares --- src/WithdrawRequestNFT.sol | 10 ++++++++-- test/WithdrawRequestNFT.t.sol | 27 +++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index d3f6f29fe..03e9b8b28 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -131,7 +131,9 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad // This is a one-time function to handle the remainder of the eEth shares after the claim of the withdraw requests // It must be called only once with ALL the requests that have not been claimed yet. // there are <3000 such requests and the total gas spending is expected to be ~9.0 M gas. - function handleAccumulatedShareRemainder(uint256[] memory _reqIds) external onlyOwner { + function handleAccumulatedShareRemainder(uint256[] memory _reqIds, uint256 _scanBegin) external onlyOwner { + assert (_scanBegin < nextRequestId); + bytes32 slot = keccak256("handleAccumulatedShareRemainder"); uint256 executed; assembly { @@ -141,9 +143,13 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint256 eEthSharesUnclaimedYet = 0; for (uint256 i = 0; i < _reqIds.length; i++) { - assert (_requests[_reqIds[i]].isValid); + if (!_requests[_reqIds[i]].isValid) continue; eEthSharesUnclaimedYet += _requests[_reqIds[i]].shareOfEEth; } + for (uint256 i = _scanBegin + 1; i < nextRequestId; i++) { + if (!_requests[i].isValid) continue; + eEthSharesUnclaimedYet += _requests[i].shareOfEEth; + } uint256 eEthSharesRemainder = eETH.shares(address(this)) - eEthSharesUnclaimedYet; handleRemainder(eEthSharesRemainder); diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 816d65424..ed2c584d7 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -368,8 +368,31 @@ contract WithdrawRequestNFTTest is TestSetup { withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(address(owner)))); withdrawRequestNFTInstance.updateShareRemainderSplitToTreasuryInBps(50_00); - - withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIds); + vm.stopPrank(); + + // The goal is to count ALL dust shares that could be burnt in the past if we had the feature. + // Option 1 is to perform the off-chain calculation and input it as a parameter to the function, which is less transparent and not ideal + // Option 2 is to perform the calculation on-chain, which is more transparent but would require a lot of gas iterating for all CLAIMED requests + // -> The idea is to calculate the total eETH shares of ALL UNCLAIMED requests. + // Then, we can calculate the dust shares as the difference between the total eETH shares and the total eETH shares of all CLAIMED requests. + // -> eETH.share(withdrawRequsetNFT) - Sum(request.shareOfEEth) for ALL unclaimed + + // Now the question is how to calculate the total eETH shares of all unclaimed requests on-chain: + // 1. When we queue up the txn, we will take a snapshot of ALL unclaimed requests and put their IDs as a parameter. + // 2. (issue) during the timelock period, there will be new requests that can't be included in the snapshot. + // the idea is to input last finalized request ID and scan from there to the latest request ID on-chain + uint256 scanBegin = withdrawRequestNFTInstance.lastFinalizedRequestId(); + + // If the request gets claimed during the timelock period, it will get skipped in the calculation. + vm.prank(withdrawRequestNFTInstance.ownerOf(reqIds[0])); + withdrawRequestNFTInstance.claimWithdraw(reqIds[0]); + + vm.startPrank(withdrawRequestNFTInstance.owner()); + withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIds, scanBegin); + vm.stopPrank(); + + vm.prank(withdrawRequestNFTInstance.ownerOf(reqIds[1])); + withdrawRequestNFTInstance.claimWithdraw(reqIds[1]); } function testFuzz_RequestWithdraw(uint96 depositAmount, uint96 withdrawAmount, address recipient) public { From a3aeac94b565830ddb0a0bc1f450d4d7662768a9 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Fri, 20 Dec 2024 10:26:11 +0900 Subject: [PATCH 07/42] improve comments --- test/WithdrawRequestNFT.t.sol | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index ed2c584d7..939d4a705 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -374,10 +374,14 @@ contract WithdrawRequestNFTTest is TestSetup { // Option 1 is to perform the off-chain calculation and input it as a parameter to the function, which is less transparent and not ideal // Option 2 is to perform the calculation on-chain, which is more transparent but would require a lot of gas iterating for all CLAIMED requests // -> The idea is to calculate the total eETH shares of ALL UNCLAIMED requests. - // Then, we can calculate the dust shares as the difference between the total eETH shares and the total eETH shares of all CLAIMED requests. + // Then, we can calculate the dust shares as the difference between the total eETH shares and the total eETH shares of all UNCLAIMED requests. // -> eETH.share(withdrawRequsetNFT) - Sum(request.shareOfEEth) for ALL unclaimed - // Now the question is how to calculate the total eETH shares of all unclaimed requests on-chain: + // Now the question is how to calculate the total eETH shares of all unclaimed requests on-chain. + // One way is to iterate through all requests and sum up the shareOfEEth for all unclaimed requests. + // However, this would require a lot of gas and is not ideal. + // + // The idea is: // 1. When we queue up the txn, we will take a snapshot of ALL unclaimed requests and put their IDs as a parameter. // 2. (issue) during the timelock period, there will be new requests that can't be included in the snapshot. // the idea is to input last finalized request ID and scan from there to the latest request ID on-chain From 57bd0fc7d379183a2ee82001debcea5c733afb66 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Tue, 24 Dec 2024 09:27:38 +0900 Subject: [PATCH 08/42] add sorted & unique constraints + type change to reduce gas --- src/WithdrawRequestNFT.sol | 7 ++++++- test/WithdrawRequestNFT.t.sol | 20 +++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 03e9b8b28..2a1da3b57 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -131,7 +131,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad // This is a one-time function to handle the remainder of the eEth shares after the claim of the withdraw requests // It must be called only once with ALL the requests that have not been claimed yet. // there are <3000 such requests and the total gas spending is expected to be ~9.0 M gas. - function handleAccumulatedShareRemainder(uint256[] memory _reqIds, uint256 _scanBegin) external onlyOwner { + function handleAccumulatedShareRemainder(uint32[] memory _reqIds, uint256 _scanBegin) external onlyOwner { assert (_scanBegin < nextRequestId); bytes32 slot = keccak256("handleAccumulatedShareRemainder"); @@ -141,6 +141,11 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } require(executed == 0, "ALREADY_EXECUTED"); + // Check that _reqIds are sorted in ascending order and there is no duplication + for (uint256 i = 1; i < _reqIds.length; i++) { + require(_reqIds[i] > _reqIds[i - 1], "Entries must be sorted and unique"); + } + uint256 eEthSharesUnclaimedYet = 0; for (uint256 i = 0; i < _reqIds.length; i++) { if (!_requests[_reqIds[i]].isValid) continue; diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 939d4a705..f7d7ea8c6 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -8,7 +8,7 @@ import "./TestSetup.sol"; contract WithdrawRequestNFTTest is TestSetup { - uint256[] public reqIds =[ 20, 388, 478, 714, 726, 729, 735, 815, 861, 916, 941, 1014, 1067, 1154, 1194, 1253]; + uint32[] public reqIds =[ 20, 388, 478, 714, 726, 729, 735, 815, 861, 916, 941, 1014, 1067, 1154, 1194, 1253]; function setUp() public { setUpTests(); @@ -392,6 +392,24 @@ contract WithdrawRequestNFTTest is TestSetup { withdrawRequestNFTInstance.claimWithdraw(reqIds[0]); vm.startPrank(withdrawRequestNFTInstance.owner()); + uint32[] memory reqIdsWithIssues = new uint32[](4); + reqIdsWithIssues[0] = reqIds[0]; + reqIdsWithIssues[1] = reqIds[1]; + reqIdsWithIssues[2] = reqIds[3]; + reqIdsWithIssues[3] = reqIds[2]; + vm.expectRevert(); + withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIdsWithIssues, scanBegin); + + reqIdsWithIssues[0] = reqIds[0]; + reqIdsWithIssues[1] = reqIds[1]; + reqIdsWithIssues[2] = reqIds[2]; + reqIdsWithIssues[3] = reqIds[2]; + vm.expectRevert(); + withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIdsWithIssues, scanBegin); + vm.stopPrank(); + + + vm.startPrank(withdrawRequestNFTInstance.owner()); withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIds, scanBegin); vm.stopPrank(); From b6100b64935a8dcbb1a430fb1b213a17cf06f717 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Tue, 24 Dec 2024 10:05:15 +0900 Subject: [PATCH 09/42] update 'handleAccumulatedShareRemainder' to be callable by admin --- src/WithdrawRequestNFT.sol | 2 +- test/WithdrawRequestNFT.t.sol | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 2a1da3b57..58b851be8 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -131,7 +131,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad // This is a one-time function to handle the remainder of the eEth shares after the claim of the withdraw requests // It must be called only once with ALL the requests that have not been claimed yet. // there are <3000 such requests and the total gas spending is expected to be ~9.0 M gas. - function handleAccumulatedShareRemainder(uint32[] memory _reqIds, uint256 _scanBegin) external onlyOwner { + function handleAccumulatedShareRemainder(uint32[] memory _reqIds, uint256 _scanBegin) external onlyAdmin { assert (_scanBegin < nextRequestId); bytes32 slot = keccak256("handleAccumulatedShareRemainder"); diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index f7d7ea8c6..4bd8bc627 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -364,6 +364,8 @@ contract WithdrawRequestNFTTest is TestSetup { function test_distributeImplicitFee() public { initializeRealisticFork(MAINNET_FORK); + address etherfi_admin_wallet = 0x2aCA71020De61bb532008049e1Bd41E451aE8AdC; + vm.startPrank(withdrawRequestNFTInstance.owner()); withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(address(owner)))); @@ -391,7 +393,7 @@ contract WithdrawRequestNFTTest is TestSetup { vm.prank(withdrawRequestNFTInstance.ownerOf(reqIds[0])); withdrawRequestNFTInstance.claimWithdraw(reqIds[0]); - vm.startPrank(withdrawRequestNFTInstance.owner()); + vm.startPrank(etherfi_admin_wallet); uint32[] memory reqIdsWithIssues = new uint32[](4); reqIdsWithIssues[0] = reqIds[0]; reqIdsWithIssues[1] = reqIds[1]; @@ -406,10 +408,7 @@ contract WithdrawRequestNFTTest is TestSetup { reqIdsWithIssues[3] = reqIds[2]; vm.expectRevert(); withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIdsWithIssues, scanBegin); - vm.stopPrank(); - - vm.startPrank(withdrawRequestNFTInstance.owner()); withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIds, scanBegin); vm.stopPrank(); From e8734d6e6ed5faea0a41980128c47283c7970d3e Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 26 Dec 2024 15:57:58 +0900 Subject: [PATCH 10/42] Certora audit: (1) add {aggregateSumEEthShareAmount}, (2) fix {_claimWithdraw} to account with 'totalLockedEEthShares', (3) simplify 'seizeRequest', (4) handle some issues --- src/EtherFiWithdrawalBuffer.sol | 32 +++--- src/WithdrawRequestNFT.sol | 165 +++++++++++++++-------------- test/EtherFiWithdrawalBuffer.t.sol | 9 ++ test/WithdrawRequestNFT.t.sol | 95 ++++++----------- 4 files changed, 143 insertions(+), 158 deletions(-) diff --git a/src/EtherFiWithdrawalBuffer.sol b/src/EtherFiWithdrawalBuffer.sol index a79b9e948..b51ed45b4 100644 --- a/src/EtherFiWithdrawalBuffer.sol +++ b/src/EtherFiWithdrawalBuffer.sol @@ -49,6 +49,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU uint16 public exitFeeInBps; uint16 public lowWatermarkInBpsOfTvl; // bps of TVL + event Redeemed(address indexed receiver, uint256 redemptionAmount, uint256 feeAmountToTreasury, uint256 feeAmountToStakers); + receive() external payable {} /// @custom:oz-upgrades-unsafe-allow constructor @@ -79,9 +81,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @param eEthAmount The amount of eETH to redeem after the exit fee. * @param receiver The address to receive the redeemed ETH. * @param owner The address of the owner of the eETH. - * @return The amount of ETH sent to the receiver and the exit fee amount. */ - function redeemEEth(uint256 eEthAmount, address receiver, address owner) public whenNotPaused nonReentrant returns (uint256, uint256) { + function redeemEEth(uint256 eEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { require(eEthAmount <= eEth.balanceOf(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); @@ -90,7 +91,7 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU uint256 afterEEthAmount = eEth.balanceOf(address(this)); uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; - return _redeem(transferredEEthAmount, receiver); + _redeem(transferredEEthAmount, receiver); } /** @@ -98,9 +99,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @param weEthAmount The amount of weETH to redeem after the exit fee. * @param receiver The address to receive the redeemed ETH. * @param owner The address of the owner of the weETH. - * @return The amount of ETH sent to the receiver and the exit fee amount. */ - function redeemWeEth(uint256 weEthAmount, address receiver, address owner) public whenNotPaused nonReentrant returns (uint256, uint256) { + function redeemWeEth(uint256 weEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { uint256 eEthShares = weEthAmount; uint256 eEthAmount = liquidityPool.amountForShare(eEthShares); require(weEthAmount <= weEth.balanceOf(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); @@ -112,7 +112,7 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU uint256 afterEEthAmount = eEth.balanceOf(address(this)); uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; - return _redeem(transferredEEthAmount, receiver); + _redeem(transferredEEthAmount, receiver); } @@ -120,9 +120,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @notice Redeems ETH. * @param ethAmount The amount of ETH to redeem after the exit fee. * @param receiver The address to receive the redeemed ETH. - * @return The amount of ETH sent to the receiver and the exit fee amount. */ - function _redeem(uint256 ethAmount, address receiver) internal returns (uint256, uint256) { + function _redeem(uint256 ethAmount, address receiver) internal { _updateRateLimit(ethAmount); uint256 ethShares = liquidityPool.sharesForAmount(ethAmount); @@ -130,16 +129,18 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU uint256 eEthAmountToReceiver = liquidityPool.amountForShare(ethShareToReceiver); uint256 prevLpBalance = address(liquidityPool).balance; - uint256 prevBalance = address(this).balance; - uint256 burnedShares = (eEthAmountToReceiver > 0) ? liquidityPool.withdraw(address(this), eEthAmountToReceiver) : 0; - uint256 ethReceived = address(this).balance - prevBalance; + uint256 sharesToBurn = liquidityPool.sharesForWithdrawalAmount(eEthAmountToReceiver); - uint256 ethShareFee = ethShares - burnedShares; - uint256 eEthAmountFee = liquidityPool.amountForShare(ethShareFee); + uint256 ethShareFee = ethShares - sharesToBurn; uint256 feeShareToTreasury = ethShareFee.mulDiv(exitFeeSplitToTreasuryInBps, BASIS_POINT_SCALE); uint256 eEthFeeAmountToTreasury = liquidityPool.amountForShare(feeShareToTreasury); uint256 feeShareToStakers = ethShareFee - feeShareToTreasury; + // Withdraw ETH from the liquidity pool + uint256 prevBalance = address(this).balance; + assert (liquidityPool.withdraw(address(this), eEthAmountToReceiver) == sharesToBurn); + uint256 ethReceived = address(this).balance - prevBalance; + // To Stakers by burning shares eEth.burnShares(address(this), feeShareToStakers); @@ -148,9 +149,10 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU // To Receiver by transferring ETH (bool success, ) = receiver.call{value: ethReceived, gas: 100_000}(""); - require(success && address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiWithdrawalBuffer: Transfer failed"); + require(success, "EtherFiWithdrawalBuffer: Transfer failed"); + require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiWithdrawalBuffer: Invalid liquidity pool balance"); - return (ethReceived, eEthAmountFee); + emit Redeemed(receiver, ethAmount, eEthFeeAmountToTreasury, eEthAmountToReceiver); } /** diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 58b851be8..91fa86325 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -30,13 +30,25 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint32 public lastFinalizedRequestId; uint16 public shareRemainderSplitToTreasuryInBps; + // inclusive + uint32 private _currentRequestIdToScanFromForShareRemainder; + uint32 private _lastRequestIdToScanUntilForShareRemainder; + + uint256 public totalLockedEEthShares; + + bool public paused; + address public pauser; + event WithdrawRequestCreated(uint32 indexed requestId, uint256 amountOfEEth, uint256 shareOfEEth, address owner, uint256 fee); event WithdrawRequestClaimed(uint32 indexed requestId, uint256 amountOfEEth, uint256 burntShareOfEEth, address owner, uint256 fee); event WithdrawRequestInvalidated(uint32 indexed requestId); - event WithdrawRequestValidated(uint32 indexed requestId); event WithdrawRequestSeized(uint32 indexed requestId); event HandledRemainderOfClaimedWithdrawRequests(uint256 eEthAmountToTreasury, uint256 eEthAmountBurnt); + event Paused(address account); + event Unpaused(address account); + + /// @custom:oz-upgrades-unsafe-allow constructor constructor(address _treasury) { treasury = _treasury; @@ -57,6 +69,14 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad nextRequestId = 1; } + function initializeOnUpgrade(address _pauser) external onlyOwner { + paused = false; + pauser = _pauser; + + _currentRequestIdToScanFromForShareRemainder = 1; + _lastRequestIdToScanUntilForShareRemainder = nextRequestId - 1; + } + /// @notice creates a withdraw request and issues an associated NFT to the recipient /// @dev liquidity pool contract will call this function when a user requests withdraw /// @param amountOfEEth amount of eETH requested for withdrawal @@ -64,13 +84,15 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad /// @param recipient address to recieve with WithdrawRequestNFT /// @param fee fee to be subtracted from amount when recipient calls claimWithdraw /// @return uint256 id of the withdraw request - function requestWithdraw(uint96 amountOfEEth, uint96 shareOfEEth, address recipient, uint256 fee) external payable onlyLiquidtyPool returns (uint256) { + function requestWithdraw(uint96 amountOfEEth, uint96 shareOfEEth, address recipient, uint256 fee) external payable onlyLiquidtyPool whenNotPaused returns (uint256) { uint256 requestId = nextRequestId++; uint32 feeGwei = uint32(fee / 1 gwei); _requests[requestId] = IWithdrawRequestNFT.WithdrawRequest(amountOfEEth, shareOfEEth, true, feeGwei); _safeMint(recipient, requestId); + totalLockedEEthShares += shareOfEEth; + emit WithdrawRequestCreated(uint32(requestId), amountOfEEth, shareOfEEth, recipient, fee); return requestId; } @@ -93,7 +115,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad /// @notice called by the NFT owner to claim their ETH /// @dev burns the NFT and transfers ETH from the liquidity pool to the owner minus any fee, withdraw request must be valid and finalized /// @param tokenId the id of the withdraw request and associated NFT - function claimWithdraw(uint256 tokenId) external { + function claimWithdraw(uint256 tokenId) external whenNotPaused { return _claimWithdraw(tokenId, ownerOf(tokenId)); } @@ -102,91 +124,48 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad IWithdrawRequestNFT.WithdrawRequest memory request = _requests[tokenId]; require(request.isValid, "Request is not valid"); - uint256 fee = uint256(request.feeGwei) * 1 gwei; uint256 amountToWithdraw = getClaimableAmount(tokenId); // transfer eth to recipient _burn(tokenId); delete _requests[tokenId]; + + uint256 shareAmountToBurnForWithdrawal = liquidityPool.sharesForWithdrawalAmount(amountToWithdraw); + totalLockedEEthShares -= shareAmountToBurnForWithdrawal; - uint256 amountBurnedShare = 0; - if (fee > 0) { - amountBurnedShare += liquidityPool.withdraw(address(membershipManager), fee); - } - amountBurnedShare += liquidityPool.withdraw(recipient, amountToWithdraw); - uint256 amountUnBurnedShare = request.shareOfEEth - amountBurnedShare; - handleRemainder(amountUnBurnedShare); + uint256 amountBurnedShare = liquidityPool.withdraw(recipient, amountToWithdraw); + assert (amountBurnedShare == shareAmountToBurnForWithdrawal); - emit WithdrawRequestClaimed(uint32(tokenId), amountToWithdraw + fee, amountBurnedShare, recipient, fee); + emit WithdrawRequestClaimed(uint32(tokenId), amountToWithdraw, amountBurnedShare, recipient, 0); } - function batchClaimWithdraw(uint256[] calldata tokenIds) external { + function batchClaimWithdraw(uint256[] calldata tokenIds) external whenNotPaused { for (uint256 i = 0; i < tokenIds.length; i++) { _claimWithdraw(tokenIds[i], ownerOf(tokenIds[i])); } } - // There have been errors tracking `accumulatedDustEEthShares` in the past. - // - https://github.com/etherfi-protocol/smart-contracts/issues/24 - // This is a one-time function to handle the remainder of the eEth shares after the claim of the withdraw requests - // It must be called only once with ALL the requests that have not been claimed yet. - // there are <3000 such requests and the total gas spending is expected to be ~9.0 M gas. - function handleAccumulatedShareRemainder(uint32[] memory _reqIds, uint256 _scanBegin) external onlyAdmin { - assert (_scanBegin < nextRequestId); - - bytes32 slot = keccak256("handleAccumulatedShareRemainder"); - uint256 executed; - assembly { - executed := sload(slot) - } - require(executed == 0, "ALREADY_EXECUTED"); + // This function is used to aggregate the sum of the eEth shares of the requests that have not been claimed yet. + // To be triggered during the upgrade to the new version of the contract. + function aggregateSumEEthShareAmount(uint256 _numReqsToScan) external { + // [scanFrom, scanUntil] + uint256 scanFrom = _currentRequestIdToScanFromForShareRemainder; + uint256 scanUntil = Math.min(_lastRequestIdToScanUntilForShareRemainder, scanFrom + _numReqsToScan - 1); - // Check that _reqIds are sorted in ascending order and there is no duplication - for (uint256 i = 1; i < _reqIds.length; i++) { - require(_reqIds[i] > _reqIds[i - 1], "Entries must be sorted and unique"); - } - - uint256 eEthSharesUnclaimedYet = 0; - for (uint256 i = 0; i < _reqIds.length; i++) { - if (!_requests[_reqIds[i]].isValid) continue; - eEthSharesUnclaimedYet += _requests[_reqIds[i]].shareOfEEth; - } - for (uint256 i = _scanBegin + 1; i < nextRequestId; i++) { + for (uint256 i = scanFrom; i <= scanUntil; i++) { if (!_requests[i].isValid) continue; - eEthSharesUnclaimedYet += _requests[i].shareOfEEth; + totalLockedEEthShares += _requests[i].shareOfEEth; } - uint256 eEthSharesRemainder = eETH.shares(address(this)) - eEthSharesUnclaimedYet; - handleRemainder(eEthSharesRemainder); - - assembly { - sstore(slot, 1) - executed := sload(slot) - } - assert (executed == 1); + _currentRequestIdToScanFromForShareRemainder = uint32(scanUntil + 1); } - // Given an invalidated withdrawal request NFT of ID `requestId`:, - // - burn the NFT - // - withdraw its ETH to the `recipient` - function seizeInvalidRequest(uint256 requestId, address recipient) external onlyOwner { + // Seize the request simply by transferring it to another recipient + function seizeRequest(uint256 requestId, address recipient) external onlyOwner { require(!_requests[requestId].isValid, "Request is valid"); require(ownerOf(requestId) != address(0), "Already Claimed"); - // Bring the NFT to the `msg.sender` == contract owner - _transfer(ownerOf(requestId), owner(), requestId); - - // Undo its invalidation to claim - _requests[requestId].isValid = true; - - // its ETH amount is not locked - // - if it was finalized when being invalidated, we revoked it via `reduceEthAmountLockedForWithdrawal` - // - if it was not finalized when being invalidated, it was not locked - uint256 ethAmount = getClaimableAmount(requestId); - liquidityPool.addEthAmountLockedForWithdrawal(uint128(ethAmount)); - - // withdraw the ETH to the recipient - _claimWithdraw(requestId, recipient); + _transfer(ownerOf(requestId), recipient, requestId); emit WithdrawRequestSeized(uint32(requestId)); } @@ -221,13 +200,6 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad emit WithdrawRequestInvalidated(uint32(requestId)); } - function validateRequest(uint256 requestId) external onlyAdmin { - require(!_requests[requestId].isValid, "Request is valid"); - _requests[requestId].isValid = true; - - emit WithdrawRequestValidated(uint32(requestId)); - } - function updateAdmin(address _address, bool _isAdmin) external onlyOwner { require(_address != address(0), "Cannot be address zero"); admins[_address] = _isAdmin; @@ -237,30 +209,45 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad shareRemainderSplitToTreasuryInBps = _shareRemainderSplitToTreasuryInBps; } + function pauseContract() external onlyPauser { + paused = true; + emit Paused(msg.sender); + } + + function unPauseContract() external onlyAdmin { + paused = false; + emit Unpaused(msg.sender); + } + /// @dev Handles the remainder of the eEth shares after the claim of the withdraw request /// the remainder eETH share for a request = request.shareOfEEth - request.amountOfEEth / (eETH amount to eETH shares rate) /// - Splits the remainder into two parts: /// - Treasury: treasury gets a split of the remainder /// - Burn: the rest of the remainder is burned - /// @param _eEthShares: the remainder of the eEth shares - function handleRemainder(uint256 _eEthShares) internal { - uint256 eEthSharesToTreasury = _eEthShares.mulDiv(shareRemainderSplitToTreasuryInBps, BASIS_POINT_SCALE); + /// @param _eEthAmount: the remainder of the eEth amount + function handleRemainder(uint256 _eEthAmount) external onlyAdmin { + require (getEEthRemainderAmount() >= _eEthAmount, "Not enough eETH remainder"); + + uint256 beforeEEthShares = eETH.shares(address(this)); + + uint256 eEthShares = liquidityPool.sharesForWithdrawalAmount(_eEthAmount); + uint256 eEthSharesToTreasury = eEthShares.mulDiv(shareRemainderSplitToTreasuryInBps, BASIS_POINT_SCALE); uint256 eEthAmountToTreasury = liquidityPool.amountForShare(eEthSharesToTreasury); eETH.transfer(treasury, eEthAmountToTreasury); - uint256 eEthSharesToBurn = _eEthShares - eEthSharesToTreasury; + uint256 eEthSharesToBurn = eEthShares - eEthSharesToTreasury; eETH.burnShares(address(this), eEthSharesToBurn); + uint256 reducedEEthShares = beforeEEthShares - eETH.shares(address(this)); + totalLockedEEthShares -= reducedEEthShares; + emit HandledRemainderOfClaimedWithdrawRequests(eEthAmountToTreasury, liquidityPool.amountForShare(eEthSharesToBurn)); } - // invalid NFTs is non-transferable except for the case they are being burnt by the owner via `seizeInvalidRequest` - function _beforeTokenTransfer(address from, address to, uint256 firstTokenId, uint256 batchSize) internal override { - for (uint256 i = 0; i < batchSize; i++) { - uint256 tokenId = firstTokenId + i; - require(_requests[tokenId].isValid || msg.sender == owner(), "INVALID_REQUEST"); - } + function getEEthRemainderAmount() public view returns (uint256) { + uint256 eEthRemainderShare = eETH.shares(address(this)) - totalLockedEEthShares; + return liquidityPool.amountForShare(eEthRemainderShare); } function _authorizeUpgrade(address newImplementation) internal override onlyOwner {} @@ -269,13 +256,27 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad return _getImplementation(); } + function _requireNotPaused() internal view virtual { + require(!paused, "Pausable: paused"); + } + modifier onlyAdmin() { require(admins[msg.sender], "Caller is not the admin"); _; } + modifier onlyPauser() { + require(msg.sender == pauser || admins[msg.sender] || msg.sender == owner(), "Caller is not the pauser"); + _; + } + modifier onlyLiquidtyPool() { require(msg.sender == address(liquidityPool), "Caller is not the liquidity pool"); _; } + + modifier whenNotPaused() { + _requireNotPaused(); + _; + } } diff --git a/test/EtherFiWithdrawalBuffer.t.sol b/test/EtherFiWithdrawalBuffer.t.sol index 5073ed989..7aede9fcf 100644 --- a/test/EtherFiWithdrawalBuffer.t.sol +++ b/test/EtherFiWithdrawalBuffer.t.sol @@ -270,6 +270,11 @@ contract EtherFiWithdrawalBufferTest is TestSetup { function test_mainnet_redeem_eEth() public { setUp_Fork(); + vm.deal(alice, 50000 ether); + vm.prank(alice); + liquidityPoolInstance.deposit{value: 50000 ether}(); + + vm.deal(user, 100 ether); vm.startPrank(user); @@ -299,6 +304,10 @@ contract EtherFiWithdrawalBufferTest is TestSetup { function test_mainnet_redeem_weEth_with_rebase() public { setUp_Fork(); + vm.deal(alice, 50000 ether); + vm.prank(alice); + liquidityPoolInstance.deposit{value: 50000 ether}(); + vm.deal(user, 100 ether); vm.startPrank(user); diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 4bd8bc627..07908ddf8 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -327,95 +327,68 @@ contract WithdrawRequestNFTTest is TestSetup { withdrawRequestNFTInstance.transferFrom(alice, bob, requestId); } - function test_seizeInvalidAndMintNew_revert_if_not_owner() public { + function test_seizeRequest() public { uint256 requestId = test_InvalidatedRequestNft_after_finalization(); uint256 claimableAmount = withdrawRequestNFTInstance.getRequest(requestId).amountOfEEth; // REVERT if not owner vm.prank(alice); vm.expectRevert("Ownable: caller is not the owner"); - withdrawRequestNFTInstance.seizeInvalidRequest(requestId, chad); - } - - function test_InvalidatedRequestNft_seizeInvalidAndMintNew_1() public { - uint256 requestId = test_InvalidatedRequestNft_after_finalization(); - uint256 claimableAmount = withdrawRequestNFTInstance.getRequest(requestId).amountOfEEth; - uint256 chadBalance = address(chad).balance; + withdrawRequestNFTInstance.seizeRequest(requestId, chad); vm.prank(owner); - withdrawRequestNFTInstance.seizeInvalidRequest(requestId, chad); + withdrawRequestNFTInstance.seizeRequest(requestId, chad); assertEq(liquidityPoolInstance.ethAmountLockedForWithdrawal(), 0, "Must be withdrawn"); - assertEq(address(chad).balance, chadBalance + claimableAmount, "Chad should receive the claimable amount"); + assertEq(withdrawRequestNFTInstance.ownerOf(requestId), chad, "Chad should own the NFT"); } - function test_InvalidatedRequestNft_seizeInvalidAndMintNew_2() public { - uint256 requestId = test_InvalidatedRequestNft_before_finalization(); - uint256 claimableAmount = withdrawRequestNFTInstance.getRequest(requestId).amountOfEEth; - uint256 chadBalance = address(chad).balance; - - vm.prank(owner); - withdrawRequestNFTInstance.seizeInvalidRequest(requestId, chad); - - assertEq(liquidityPoolInstance.ethAmountLockedForWithdrawal(), 0, "Must be withdrawn"); - assertEq(address(chad).balance, chadBalance + claimableAmount, "Chad should receive the claimable amount"); - } - function test_distributeImplicitFee() public { + function test_aggregateSumEEthShareAmount() public { initializeRealisticFork(MAINNET_FORK); address etherfi_admin_wallet = 0x2aCA71020De61bb532008049e1Bd41E451aE8AdC; vm.startPrank(withdrawRequestNFTInstance.owner()); + // 1. Upgrade withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(address(owner)))); - + withdrawRequestNFTInstance.initializeOnUpgrade(etherfi_admin_wallet); withdrawRequestNFTInstance.updateShareRemainderSplitToTreasuryInBps(50_00); + withdrawRequestNFTInstance.updateAdmin(etherfi_admin_wallet, true); + + // 2. PAUSE + withdrawRequestNFTInstance.pauseContract(); vm.stopPrank(); - // The goal is to count ALL dust shares that could be burnt in the past if we had the feature. - // Option 1 is to perform the off-chain calculation and input it as a parameter to the function, which is less transparent and not ideal - // Option 2 is to perform the calculation on-chain, which is more transparent but would require a lot of gas iterating for all CLAIMED requests - // -> The idea is to calculate the total eETH shares of ALL UNCLAIMED requests. - // Then, we can calculate the dust shares as the difference between the total eETH shares and the total eETH shares of all UNCLAIMED requests. - // -> eETH.share(withdrawRequsetNFT) - Sum(request.shareOfEEth) for ALL unclaimed - - // Now the question is how to calculate the total eETH shares of all unclaimed requests on-chain. - // One way is to iterate through all requests and sum up the shareOfEEth for all unclaimed requests. - // However, this would require a lot of gas and is not ideal. - // - // The idea is: - // 1. When we queue up the txn, we will take a snapshot of ALL unclaimed requests and put their IDs as a parameter. - // 2. (issue) during the timelock period, there will be new requests that can't be included in the snapshot. - // the idea is to input last finalized request ID and scan from there to the latest request ID on-chain - uint256 scanBegin = withdrawRequestNFTInstance.lastFinalizedRequestId(); - - // If the request gets claimed during the timelock period, it will get skipped in the calculation. - vm.prank(withdrawRequestNFTInstance.ownerOf(reqIds[0])); - withdrawRequestNFTInstance.claimWithdraw(reqIds[0]); - - vm.startPrank(etherfi_admin_wallet); - uint32[] memory reqIdsWithIssues = new uint32[](4); - reqIdsWithIssues[0] = reqIds[0]; - reqIdsWithIssues[1] = reqIds[1]; - reqIdsWithIssues[2] = reqIds[3]; - reqIdsWithIssues[3] = reqIds[2]; - vm.expectRevert(); - withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIdsWithIssues, scanBegin); - - reqIdsWithIssues[0] = reqIds[0]; - reqIdsWithIssues[1] = reqIds[1]; - reqIdsWithIssues[2] = reqIds[2]; - reqIdsWithIssues[3] = reqIds[2]; - vm.expectRevert(); - withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIdsWithIssues, scanBegin); - - withdrawRequestNFTInstance.handleAccumulatedShareRemainder(reqIds, scanBegin); + vm.startPrank(etherfi_admin_wallet); + + // 3. AggSum + withdrawRequestNFTInstance.aggregateSumEEthShareAmount(1000); + withdrawRequestNFTInstance.aggregateSumEEthShareAmount(1000); + // ... + + vm.stopPrank(); + + // 4. Unpause + vm.startPrank(withdrawRequestNFTInstance.owner()); + withdrawRequestNFTInstance.unPauseContract(); vm.stopPrank(); + // Back to normal vm.prank(withdrawRequestNFTInstance.ownerOf(reqIds[1])); withdrawRequestNFTInstance.claimWithdraw(reqIds[1]); } + function test_handleRemainder() public { + test_aggregateSumEEthShareAmount(); + + vm.startPrank(withdrawRequestNFTInstance.owner()); + + withdrawRequestNFTInstance.handleRemainder(1 ether); + + vm.stopPrank(); + } + function testFuzz_RequestWithdraw(uint96 depositAmount, uint96 withdrawAmount, address recipient) public { // Assume valid conditions vm.assume(depositAmount >= 1 ether && depositAmount <= 1000 ether); From 71ffa8d25b4f7efe455decf005c1d67ba2d13561 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Mon, 30 Dec 2024 09:32:11 +0900 Subject: [PATCH 11/42] wip: to be amended --- src/WithdrawRequestNFT.sol | 8 ++++++-- test/WithdrawRequestNFT.t.sol | 28 ++++++++++++++-------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 91fa86325..d858e827e 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -70,6 +70,8 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function initializeOnUpgrade(address _pauser) external onlyOwner { + require(pauser == address(0), "Already initialized"); + paused = false; pauser = _pauser; @@ -89,10 +91,10 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint32 feeGwei = uint32(fee / 1 gwei); _requests[requestId] = IWithdrawRequestNFT.WithdrawRequest(amountOfEEth, shareOfEEth, true, feeGwei); - _safeMint(recipient, requestId); - totalLockedEEthShares += shareOfEEth; + _safeMint(recipient, requestId); + emit WithdrawRequestCreated(uint32(requestId), amountOfEEth, shareOfEEth, recipient, fee); return requestId; } @@ -206,6 +208,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function updateShareRemainderSplitToTreasuryInBps(uint16 _shareRemainderSplitToTreasuryInBps) external onlyOwner { + require(_shareRemainderSplitToTreasuryInBps <= BASIS_POINT_SCALE, "INVALID"); shareRemainderSplitToTreasuryInBps = _shareRemainderSplitToTreasuryInBps; } @@ -227,6 +230,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad /// @param _eEthAmount: the remainder of the eEth amount function handleRemainder(uint256 _eEthAmount) external onlyAdmin { require (getEEthRemainderAmount() >= _eEthAmount, "Not enough eETH remainder"); + require(_currentRequestIdToScanFromForShareRemainder == nextRequestId, "Not all requests have been scanned"); uint256 beforeEEthShares = eETH.shares(address(this)); diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 07908ddf8..2f5059976 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -458,6 +458,7 @@ contract WithdrawRequestNFTTest is TestSetup { // Record initial balances uint256 treasuryEEthBefore = eETHInstance.balanceOf(address(treasuryInstance)); uint256 recipientBalanceBefore = address(recipient).balance; + uint256 initialTotalLockedEEthShares = withdrawRequestNFTInstance.totalLockedEEthShares(); // Request withdraw eETHInstance.approve(address(liquidityPoolInstance), withdrawAmount); @@ -467,6 +468,8 @@ contract WithdrawRequestNFTTest is TestSetup { // Get initial request state WithdrawRequestNFT.WithdrawRequest memory request = withdrawRequestNFTInstance.getRequest(requestId); + assertEq(withdrawRequestNFTInstance.totalLockedEEthShares(), initialTotalLockedEEthShares + request.shareOfEEth, "Incorrect total locked shares"); + // Simulate rebase after request but before claim vm.prank(address(membershipManagerInstance)); liquidityPoolInstance.rebase(int128(uint128(rebaseAmount))); @@ -474,10 +477,8 @@ contract WithdrawRequestNFTTest is TestSetup { // Calculate expected withdrawal amounts after rebase uint256 sharesValue = liquidityPoolInstance.amountForShare(request.shareOfEEth); uint256 expectedWithdrawAmount = withdrawAmount < sharesValue ? withdrawAmount : sharesValue; - uint256 unusedShares = request.shareOfEEth - liquidityPoolInstance.sharesForWithdrawalAmount(expectedWithdrawAmount); - uint256 expectedTreasuryShares = (unusedShares * remainderSplitBps) / 10000; - uint256 expectedBurnedShares = request.shareOfEEth - expectedTreasuryShares; - assertGe(unusedShares, 0, "Unused shares should be non-negative because there was positive rebase"); + uint256 expectedBurnedShares = liquidityPoolInstance.sharesForWithdrawalAmount(expectedWithdrawAmount); + uint256 expectedLockedShares = request.shareOfEEth - expectedBurnedShares; // Track initial shares and total supply uint256 initialTotalShares = eETHInstance.totalShares(); @@ -491,13 +492,14 @@ contract WithdrawRequestNFTTest is TestSetup { uint256 burnedShares = initialTotalShares - eETHInstance.totalShares(); // Verify share burning + assertLe(burnedShares, request.shareOfEEth, "Burned shares should be less than or equal to requested shares"); assertApproxEqAbs( burnedShares, expectedBurnedShares, - 1e1, + 1e3, "Incorrect amount of shares burnt" ); - assertLe(burnedShares, request.shareOfEEth, "Burned shares should be less than or equal to requested shares"); + // Verify total supply reduction assertApproxEqAbs( @@ -523,14 +525,12 @@ contract WithdrawRequestNFTTest is TestSetup { withdrawRequestNFTInstance.ownerOf(requestId); // Calculate and verify remainder splitting - if (unusedShares > 0) { - assertApproxEqAbs( - eETHInstance.balanceOf(address(treasuryInstance)) - treasuryEEthBefore, - liquidityPoolInstance.amountForShare(expectedTreasuryShares), - 1e1, - "Incorrect treasury eETH balance" - ); - } + assertApproxEqAbs( + expectedLockedShares, + withdrawRequestNFTInstance.totalLockedEEthShares(), + 1e1, + "Incorrect locked eETH share" + ); // Verify recipient received correct ETH amount assertEq( From 9e0fb99c906c8921051e0518569372862073f754 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Mon, 30 Dec 2024 15:13:01 +0900 Subject: [PATCH 12/42] add simplified {invalidate, validate} request, fix unit tests --- src/WithdrawRequestNFT.sol | 33 ++++++++++++++++++-------- test/WithdrawRequestNFT.t.sol | 44 +++++++++-------------------------- 2 files changed, 34 insertions(+), 43 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index d858e827e..90b939b72 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -42,6 +42,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad event WithdrawRequestCreated(uint32 indexed requestId, uint256 amountOfEEth, uint256 shareOfEEth, address owner, uint256 fee); event WithdrawRequestClaimed(uint32 indexed requestId, uint256 amountOfEEth, uint256 burntShareOfEEth, address owner, uint256 fee); event WithdrawRequestInvalidated(uint32 indexed requestId); + event WithdrawRequestValidated(uint32 indexed requestId); event WithdrawRequestSeized(uint32 indexed requestId); event HandledRemainderOfClaimedWithdrawRequests(uint256 eEthAmountToTreasury, uint256 eEthAmountBurnt); @@ -155,7 +156,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint256 scanUntil = Math.min(_lastRequestIdToScanUntilForShareRemainder, scanFrom + _numReqsToScan - 1); for (uint256 i = scanFrom; i <= scanUntil; i++) { - if (!_requests[i].isValid) continue; + if (!_exists(i)) continue; totalLockedEEthShares += _requests[i].shareOfEEth; } @@ -165,7 +166,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad // Seize the request simply by transferring it to another recipient function seizeRequest(uint256 requestId, address recipient) external onlyOwner { require(!_requests[requestId].isValid, "Request is valid"); - require(ownerOf(requestId) != address(0), "Already Claimed"); + require(_exists(requestId), "Request does not exist"); _transfer(ownerOf(requestId), recipient, requestId); @@ -181,7 +182,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function isValid(uint256 requestId) public view returns (bool) { - require(_exists(requestId), "Request does not exist"); + require(_exists(requestId), "Request does not exist11"); return _requests[requestId].isValid; } @@ -191,17 +192,19 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad function invalidateRequest(uint256 requestId) external onlyAdmin { require(isValid(requestId), "Request is not valid"); - - if (isFinalized(requestId)) { - uint256 ethAmount = getClaimableAmount(requestId); - liquidityPool.reduceEthAmountLockedForWithdrawal(uint128(ethAmount)); - } - _requests[requestId].isValid = false; emit WithdrawRequestInvalidated(uint32(requestId)); } + function validateRequest(uint256 requestId) external onlyAdmin { + require(_exists(requestId), "Request does not exist22"); + require(!_requests[requestId].isValid, "Request is valid"); + _requests[requestId].isValid = true; + + emit WithdrawRequestValidated(uint32(requestId)); + } + function updateAdmin(address _address, bool _isAdmin) external onlyOwner { require(_address != address(0), "Cannot be address zero"); admins[_address] = _isAdmin; @@ -229,7 +232,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad /// - Burn: the rest of the remainder is burned /// @param _eEthAmount: the remainder of the eEth amount function handleRemainder(uint256 _eEthAmount) external onlyAdmin { - require (getEEthRemainderAmount() >= _eEthAmount, "Not enough eETH remainder"); + require(getEEthRemainderAmount() >= _eEthAmount, "Not enough eETH remainder"); require(_currentRequestIdToScanFromForShareRemainder == nextRequestId, "Not all requests have been scanned"); uint256 beforeEEthShares = eETH.shares(address(this)); @@ -254,6 +257,16 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad return liquidityPool.amountForShare(eEthRemainderShare); } + // the withdraw request NFT is transferrable + // - if the request is valid, it can be transferred by the owner of the NFT + // - if the request is invalid, it can be transferred only by the owner of the WithdarwRequestNFT contract + function _beforeTokenTransfer(address from, address to, uint256 firstTokenId, uint256 batchSize) internal override { + for (uint256 i = 0; i < batchSize; i++) { + uint256 tokenId = firstTokenId + i; + require(_requests[tokenId].isValid || msg.sender == owner(), "INVALID_REQUEST"); + } + } + function _authorizeUpgrade(address newImplementation) internal override onlyOwner {} function getImplementation() external view returns (address) { diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 2f5059976..7e3e62944 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -65,7 +65,7 @@ contract WithdrawRequestNFTTest is TestSetup { assertTrue(request.isValid, "Request should be valid"); } - function testInvalidClaimWithdraw() public { + function test_InvalidClaimWithdraw() public { startHoax(bob); liquidityPoolInstance.deposit{value: 10 ether}(); vm.stopPrank(); @@ -117,7 +117,7 @@ contract WithdrawRequestNFTTest is TestSetup { assertEq(liquidityPoolInstance.getTotalPooledEther(), 10 ether); assertEq(eETHInstance.balanceOf(bob), 10 ether); - assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 0 ether, "eETH balance should be 0 ether"); + assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 0 ether, "eETH balance should start from 0 ether"); // Case 1. // Even after the rebase, the withdrawal amount should remain the same; 1 eth @@ -149,7 +149,7 @@ contract WithdrawRequestNFTTest is TestSetup { uint256 bobsEndingBalance = address(bob).balance; assertEq(bobsEndingBalance, bobsStartingBalance + 1 ether, "Bobs balance should be 1 ether higher"); - assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 0 ether, "eETH balance should be 0 ether"); + assertEq(eETHInstance.balanceOf(address(withdrawRequestNFTInstance)), 1 ether, "eETH balance should be 1 ether"); } function test_ValidClaimWithdrawWithNegativeRebase() public { @@ -319,31 +319,6 @@ contract WithdrawRequestNFTTest is TestSetup { _finalizeWithdrawalRequest(requestId); } - function test_InvalidatedRequestNft_NonTransferrable() public { - uint256 requestId = test_InvalidatedRequestNft_after_finalization(); - - vm.prank(alice); - vm.expectRevert("INVALID_REQUEST"); - withdrawRequestNFTInstance.transferFrom(alice, bob, requestId); - } - - function test_seizeRequest() public { - uint256 requestId = test_InvalidatedRequestNft_after_finalization(); - uint256 claimableAmount = withdrawRequestNFTInstance.getRequest(requestId).amountOfEEth; - - // REVERT if not owner - vm.prank(alice); - vm.expectRevert("Ownable: caller is not the owner"); - withdrawRequestNFTInstance.seizeRequest(requestId, chad); - - vm.prank(owner); - withdrawRequestNFTInstance.seizeRequest(requestId, chad); - - assertEq(liquidityPoolInstance.ethAmountLockedForWithdrawal(), 0, "Must be withdrawn"); - assertEq(withdrawRequestNFTInstance.ownerOf(requestId), chad, "Chad should own the NFT"); - } - - function test_aggregateSumEEthShareAmount() public { initializeRealisticFork(MAINNET_FORK); @@ -363,8 +338,7 @@ contract WithdrawRequestNFTTest is TestSetup { vm.startPrank(etherfi_admin_wallet); // 3. AggSum - withdrawRequestNFTInstance.aggregateSumEEthShareAmount(1000); - withdrawRequestNFTInstance.aggregateSumEEthShareAmount(1000); + withdrawRequestNFTInstance.aggregateSumEEthShareAmount(128); // ... vm.stopPrank(); @@ -383,7 +357,7 @@ contract WithdrawRequestNFTTest is TestSetup { test_aggregateSumEEthShareAmount(); vm.startPrank(withdrawRequestNFTInstance.owner()); - + vm.expectRevert("Not all requests have been scanned"); withdrawRequestNFTInstance.handleRemainder(1 ether); vm.stopPrank(); @@ -568,8 +542,8 @@ contract WithdrawRequestNFTTest is TestSetup { // Admin invalidates request vm.prank(withdrawRequestNFTInstance.owner()); - withdrawRequestNFTInstance.updateAdmin(recipient, true); - vm.prank(recipient); + withdrawRequestNFTInstance.updateAdmin(admin, true); + vm.prank(admin); withdrawRequestNFTInstance.invalidateRequest(requestId); // Verify request state after invalidation @@ -580,5 +554,9 @@ contract WithdrawRequestNFTTest is TestSetup { vm.prank(recipient); vm.expectRevert("INVALID_REQUEST"); withdrawRequestNFTInstance.transferFrom(recipient, address(0xdead), requestId); + + // Owner can seize the invalidated request NFT + vm.prank(withdrawRequestNFTInstance.owner()); + withdrawRequestNFTInstance.seizeRequest(requestId, admin); } } From 98f483a2f968fb7f8d14b701af32b645f1bc885a Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Mon, 30 Dec 2024 18:09:49 +0900 Subject: [PATCH 13/42] rename EtherFiWithdrawBuffer -> EtherFiRedemptionManager --- script/deploys/DeployEtherFiRestaker.s.sol | 42 +++ .../DeployEtherFiWithdrawalBuffer.s.sol | 6 +- src/EtherFiRedemptionManager.sol | 280 ++++++++++++++++++ src/EtherFiWithdrawalBuffer.sol | 16 +- src/LiquidityPool.sol | 11 +- ...r.t.sol => EtherFiRedemptionManager.t.sol} | 136 ++++----- test/TestSetup.sol | 14 +- 7 files changed, 414 insertions(+), 91 deletions(-) create mode 100644 script/deploys/DeployEtherFiRestaker.s.sol create mode 100644 src/EtherFiRedemptionManager.sol rename test/{EtherFiWithdrawalBuffer.t.sol => EtherFiRedemptionManager.t.sol} (63%) diff --git a/script/deploys/DeployEtherFiRestaker.s.sol b/script/deploys/DeployEtherFiRestaker.s.sol new file mode 100644 index 000000000..0f4ec2225 --- /dev/null +++ b/script/deploys/DeployEtherFiRestaker.s.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "forge-std/Script.sol"; + +import "../../src/Liquifier.sol"; +import "../../src/EtherFiRestaker.sol"; +import "../../src/helpers/AddressProvider.sol"; +import "../../src/UUPSProxy.sol"; +import "@openzeppelin/contracts/utils/Strings.sol"; + +contract Deploy is Script { + using Strings for string; + + UUPSProxy public liquifierProxy; + + Liquifier public liquifierInstance; + + AddressProvider public addressProvider; + + address admin; + + function run() external { + uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY"); + address addressProviderAddress = vm.envAddress("CONTRACT_REGISTRY"); + addressProvider = AddressProvider(addressProviderAddress); + + vm.startBroadcast(deployerPrivateKey); + + EtherFiRestaker restaker = EtherFiRestaker(payable(new UUPSProxy(payable(new EtherFiRestaker()), ""))); + restaker.initialize( + addressProvider.getContractAddress("LiquidityPool"), + addressProvider.getContractAddress("Liquifier") + ); + + new Liquifier(); + + // addressProvider.addContract(address(liquifierInstance), "Liquifier"); + + vm.stopBroadcast(); + } +} diff --git a/script/deploys/DeployEtherFiWithdrawalBuffer.s.sol b/script/deploys/DeployEtherFiWithdrawalBuffer.s.sol index 34f132e0c..ebc4050cf 100644 --- a/script/deploys/DeployEtherFiWithdrawalBuffer.s.sol +++ b/script/deploys/DeployEtherFiWithdrawalBuffer.s.sol @@ -9,7 +9,7 @@ import "../../src/Liquifier.sol"; import "../../src/EtherFiRestaker.sol"; import "../../src/helpers/AddressProvider.sol"; import "../../src/UUPSProxy.sol"; -import "../../src/EtherFiWithdrawalBuffer.sol"; +import "../../src/EtherFiRedemptionManager.sol"; contract Deploy is Script { @@ -23,7 +23,7 @@ contract Deploy is Script { vm.startBroadcast(deployerPrivateKey); - EtherFiWithdrawalBuffer impl = new EtherFiWithdrawalBuffer( + EtherFiRedemptionManager impl = new EtherFiRedemptionManager( addressProvider.getContractAddress("LiquidityPool"), addressProvider.getContractAddress("EETH"), addressProvider.getContractAddress("WeETH"), @@ -32,7 +32,7 @@ contract Deploy is Script { ); UUPSProxy proxy = new UUPSProxy(payable(impl), ""); - EtherFiWithdrawalBuffer instance = EtherFiWithdrawalBuffer(payable(proxy)); + EtherFiRedemptionManager instance = EtherFiRedemptionManager(payable(proxy)); instance.initialize(10_00, 1_00, 1_00, 5 ether, 0.001 ether); vm.stopBroadcast(); diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol new file mode 100644 index 000000000..c6c445c53 --- /dev/null +++ b/src/EtherFiRedemptionManager.sol @@ -0,0 +1,280 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.13; + +import "@openzeppelin/contracts/utils/math/SafeCast.sol"; +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/extensions/draft-IERC20Permit.sol"; +import "@openzeppelin-upgradeable/contracts/token/ERC20/IERC20Upgradeable.sol"; +import "@openzeppelin-upgradeable/contracts/proxy/utils/Initializable.sol"; +import "@openzeppelin-upgradeable/contracts/proxy/utils/UUPSUpgradeable.sol"; +import "@openzeppelin-upgradeable/contracts/security/ReentrancyGuardUpgradeable.sol"; +import "@openzeppelin-upgradeable/contracts/access/OwnableUpgradeable.sol"; +import "@openzeppelin-upgradeable/contracts/security/PausableUpgradeable.sol"; + +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import "@openzeppelin/contracts/utils/math/Math.sol"; + +import "./interfaces/ILiquidityPool.sol"; +import "./interfaces/IeETH.sol"; +import "./interfaces/IWeETH.sol"; + +import "lib/BucketLimiter.sol"; + +import "./RoleRegistry.sol"; + +/* + The contract allows instant redemption of eETH and weETH tokens to ETH with an exit fee. + - It has the exit fee as a percentage of the total amount redeemed. + - It has a rate limiter to limit the total amount that can be redeemed in a given time period. +*/ +contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable, UUPSUpgradeable { + using SafeERC20 for IERC20; + using Math for uint256; + + uint256 private constant BUCKET_UNIT_SCALE = 1e12; + uint256 private constant BASIS_POINT_SCALE = 1e4; + + bytes32 public constant PROTOCOL_PAUSER = keccak256("PROTOCOL_PAUSER"); + bytes32 public constant PROTOCOL_UNPAUSER = keccak256("PROTOCOL_UNPAUSER"); + bytes32 public constant PROTOCOL_ADMIN = keccak256("PROTOCOL_ADMIN"); + + RoleRegistry public immutable roleRegistry; + address public immutable treasury; + IeETH public immutable eEth; + IWeETH public immutable weEth; + ILiquidityPool public immutable liquidityPool; + + BucketLimiter.Limit public limit; + uint16 public exitFeeSplitToTreasuryInBps; + uint16 public exitFeeInBps; + uint16 public lowWatermarkInBpsOfTvl; // bps of TVL + + event Redeemed(address indexed receiver, uint256 redemptionAmount, uint256 feeAmountToTreasury, uint256 feeAmountToStakers); + + receive() external payable {} + + /// @custom:oz-upgrades-unsafe-allow constructor + constructor(address _liquidityPool, address _eEth, address _weEth, address _treasury, address _roleRegistry) { + roleRegistry = RoleRegistry(_roleRegistry); + treasury = _treasury; + liquidityPool = ILiquidityPool(payable(_liquidityPool)); + eEth = IeETH(_eEth); + weEth = IWeETH(_weEth); + + _disableInitializers(); + } + + function initialize(uint16 _exitFeeSplitToTreasuryInBps, uint16 _exitFeeInBps, uint16 _lowWatermarkInBpsOfTvl, uint256 _bucketCapacity, uint256 _bucketRefillRate) external initializer { + __Ownable_init(); + __UUPSUpgradeable_init(); + __Pausable_init(); + __ReentrancyGuard_init(); + + limit = BucketLimiter.create(_convertToBucketUnit(_bucketCapacity, Math.Rounding.Down), _convertToBucketUnit(_bucketRefillRate, Math.Rounding.Down)); + exitFeeSplitToTreasuryInBps = _exitFeeSplitToTreasuryInBps; + exitFeeInBps = _exitFeeInBps; + lowWatermarkInBpsOfTvl = _lowWatermarkInBpsOfTvl; + } + + /** + * @notice Redeems eETH for ETH. + * @param eEthAmount The amount of eETH to redeem after the exit fee. + * @param receiver The address to receive the redeemed ETH. + * @param owner The address of the owner of the eETH. + */ + function redeemEEth(uint256 eEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { + require(eEthAmount <= eEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); + require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); + + uint256 beforeEEthAmount = eEth.balanceOf(address(this)); + IERC20(address(eEth)).safeTransferFrom(owner, address(this), eEthAmount); + uint256 afterEEthAmount = eEth.balanceOf(address(this)); + + uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; + _redeem(transferredEEthAmount, receiver); + } + + /** + * @notice Redeems weETH for ETH. + * @param weEthAmount The amount of weETH to redeem after the exit fee. + * @param receiver The address to receive the redeemed ETH. + * @param owner The address of the owner of the weETH. + */ + function redeemWeEth(uint256 weEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { + uint256 eEthShares = weEthAmount; + uint256 eEthAmount = liquidityPool.amountForShare(eEthShares); + require(weEthAmount <= weEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); + require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); + + uint256 beforeEEthAmount = eEth.balanceOf(address(this)); + IERC20(address(weEth)).safeTransferFrom(owner, address(this), weEthAmount); + weEth.unwrap(weEthAmount); + uint256 afterEEthAmount = eEth.balanceOf(address(this)); + + uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; + _redeem(transferredEEthAmount, receiver); + } + + + /** + * @notice Redeems ETH. + * @param ethAmount The amount of ETH to redeem after the exit fee. + * @param receiver The address to receive the redeemed ETH. + */ + function _redeem(uint256 ethAmount, address receiver) internal { + _updateRateLimit(ethAmount); + + uint256 ethShares = liquidityPool.sharesForAmount(ethAmount); + uint256 ethShareToReceiver = ethShares.mulDiv(BASIS_POINT_SCALE - exitFeeInBps, BASIS_POINT_SCALE); + uint256 eEthAmountToReceiver = liquidityPool.amountForShare(ethShareToReceiver); + + uint256 prevLpBalance = address(liquidityPool).balance; + uint256 sharesToBurn = liquidityPool.sharesForWithdrawalAmount(eEthAmountToReceiver); + + uint256 ethShareFee = ethShares - sharesToBurn; + uint256 feeShareToTreasury = ethShareFee.mulDiv(exitFeeSplitToTreasuryInBps, BASIS_POINT_SCALE); + uint256 eEthFeeAmountToTreasury = liquidityPool.amountForShare(feeShareToTreasury); + uint256 feeShareToStakers = ethShareFee - feeShareToTreasury; + + // Withdraw ETH from the liquidity pool + uint256 prevBalance = address(this).balance; + assert (liquidityPool.withdraw(address(this), eEthAmountToReceiver) == sharesToBurn); + uint256 ethReceived = address(this).balance - prevBalance; + + // To Stakers by burning shares + eEth.burnShares(address(this), feeShareToStakers); + + // To Treasury by transferring eETH + IERC20(address(eEth)).safeTransfer(treasury, eEthFeeAmountToTreasury); + + // To Receiver by transferring ETH + (bool success, ) = receiver.call{value: ethReceived, gas: 100_000}(""); + require(success, "EtherFiRedemptionManager: Transfer failed"); + require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiRedemptionManager: Invalid liquidity pool balance"); + + emit Redeemed(receiver, ethAmount, eEthFeeAmountToTreasury, eEthAmountToReceiver); + } + + /** + * @dev if the contract has less than the low watermark, it will not allow any instant redemption. + */ + function lowWatermarkInETH() public view returns (uint256) { + return liquidityPool.getTotalPooledEther().mulDiv(lowWatermarkInBpsOfTvl, BASIS_POINT_SCALE); + } + + /** + * @dev Returns the total amount that can be redeemed. + */ + function totalRedeemableAmount() external view returns (uint256) { + uint256 liquidEthAmount = address(liquidityPool).balance - liquidityPool.ethAmountLockedForWithdrawal(); + if (liquidEthAmount < lowWatermarkInETH()) { + return 0; + } + uint64 consumableBucketUnits = BucketLimiter.consumable(limit); + uint256 consumableAmount = _convertFromBucketUnit(consumableBucketUnits); + return Math.min(consumableAmount, liquidEthAmount); + } + + /** + * @dev Returns whether the given amount can be redeemed. + * @param amount The ETH or eETH amount to check. + */ + function canRedeem(uint256 amount) public view returns (bool) { + uint256 liquidEthAmount = address(liquidityPool).balance - liquidityPool.ethAmountLockedForWithdrawal(); + if (liquidEthAmount < lowWatermarkInETH()) { + return false; + } + uint64 bucketUnit = _convertToBucketUnit(amount, Math.Rounding.Up); + bool consumable = BucketLimiter.canConsume(limit, bucketUnit); + return consumable && amount <= liquidEthAmount; + } + + /** + * @dev Sets the maximum size of the bucket that can be consumed in a given time period. + * @param capacity The capacity of the bucket. + */ + function setCapacity(uint256 capacity) external hasRole(PROTOCOL_ADMIN) { + // max capacity = max(uint64) * 1e12 ~= 16 * 1e18 * 1e12 = 16 * 1e12 ether, which is practically enough + uint64 bucketUnit = _convertToBucketUnit(capacity, Math.Rounding.Down); + BucketLimiter.setCapacity(limit, bucketUnit); + } + + /** + * @dev Sets the rate at which the bucket is refilled per second. + * @param refillRate The rate at which the bucket is refilled per second. + */ + function setRefillRatePerSecond(uint256 refillRate) external hasRole(PROTOCOL_ADMIN) { + // max refillRate = max(uint64) * 1e12 ~= 16 * 1e18 * 1e12 = 16 * 1e12 ether per second, which is practically enough + uint64 bucketUnit = _convertToBucketUnit(refillRate, Math.Rounding.Down); + BucketLimiter.setRefillRate(limit, bucketUnit); + } + + /** + * @dev Sets the exit fee. + * @param _exitFeeInBps The exit fee. + */ + function setExitFeeBasisPoints(uint16 _exitFeeInBps) external hasRole(PROTOCOL_ADMIN) { + require(_exitFeeInBps <= BASIS_POINT_SCALE, "INVALID"); + exitFeeInBps = _exitFeeInBps; + } + + function setLowWatermarkInBpsOfTvl(uint16 _lowWatermarkInBpsOfTvl) external hasRole(PROTOCOL_ADMIN) { + require(_lowWatermarkInBpsOfTvl <= BASIS_POINT_SCALE, "INVALID"); + lowWatermarkInBpsOfTvl = _lowWatermarkInBpsOfTvl; + } + + function setExitFeeSplitToTreasuryInBps(uint16 _exitFeeSplitToTreasuryInBps) external hasRole(PROTOCOL_ADMIN) { + require(_exitFeeSplitToTreasuryInBps <= BASIS_POINT_SCALE, "INVALID"); + exitFeeSplitToTreasuryInBps = _exitFeeSplitToTreasuryInBps; + } + + function pauseContract() external hasRole(PROTOCOL_PAUSER) { + _pause(); + } + + function unPauseContract() external hasRole(PROTOCOL_UNPAUSER) { + _unpause(); + } + + function _updateRateLimit(uint256 amount) internal { + uint64 bucketUnit = _convertToBucketUnit(amount, Math.Rounding.Up); + require(BucketLimiter.consume(limit, bucketUnit), "BucketRateLimiter: rate limit exceeded"); + } + + function _convertToBucketUnit(uint256 amount, Math.Rounding rounding) internal pure returns (uint64) { + return (rounding == Math.Rounding.Up) ? SafeCast.toUint64((amount + BUCKET_UNIT_SCALE - 1) / BUCKET_UNIT_SCALE) : SafeCast.toUint64(amount / BUCKET_UNIT_SCALE); + } + + function _convertFromBucketUnit(uint64 bucketUnit) internal pure returns (uint256) { + return bucketUnit * BUCKET_UNIT_SCALE; + } + + /** + * @dev Preview taking an exit fee on redeem. See {IERC4626-previewRedeem}. + */ + // redeemable amount after exit fee + function previewRedeem(uint256 shares) public view returns (uint256) { + uint256 amountInEth = liquidityPool.amountForShare(shares); + return amountInEth - _fee(amountInEth, exitFeeInBps); + } + + function _fee(uint256 assets, uint256 feeBasisPoints) internal pure virtual returns (uint256) { + return assets.mulDiv(feeBasisPoints, BASIS_POINT_SCALE, Math.Rounding.Up); + } + + function _authorizeUpgrade(address newImplementation) internal override onlyOwner {} + + function getImplementation() external view returns (address) { + return _getImplementation(); + } + + function _hasRole(bytes32 role, address account) internal view returns (bool) { + require(roleRegistry.hasRole(role, account), "EtherFiRedemptionManager: Unauthorized"); + } + + modifier hasRole(bytes32 role) { + _hasRole(role, msg.sender); + _; + } + +} \ No newline at end of file diff --git a/src/EtherFiWithdrawalBuffer.sol b/src/EtherFiWithdrawalBuffer.sol index b51ed45b4..c6c445c53 100644 --- a/src/EtherFiWithdrawalBuffer.sol +++ b/src/EtherFiWithdrawalBuffer.sol @@ -27,7 +27,7 @@ import "./RoleRegistry.sol"; - It has the exit fee as a percentage of the total amount redeemed. - It has a rate limiter to limit the total amount that can be redeemed in a given time period. */ -contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable, UUPSUpgradeable { +contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable, UUPSUpgradeable { using SafeERC20 for IERC20; using Math for uint256; @@ -83,8 +83,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU * @param owner The address of the owner of the eETH. */ function redeemEEth(uint256 eEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { - require(eEthAmount <= eEth.balanceOf(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); - require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); + require(eEthAmount <= eEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); + require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); uint256 beforeEEthAmount = eEth.balanceOf(address(this)); IERC20(address(eEth)).safeTransferFrom(owner, address(this), eEthAmount); @@ -103,8 +103,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU function redeemWeEth(uint256 weEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { uint256 eEthShares = weEthAmount; uint256 eEthAmount = liquidityPool.amountForShare(eEthShares); - require(weEthAmount <= weEth.balanceOf(owner), "EtherFiWithdrawalBuffer: Insufficient balance"); - require(canRedeem(eEthAmount), "EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); + require(weEthAmount <= weEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); + require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); uint256 beforeEEthAmount = eEth.balanceOf(address(this)); IERC20(address(weEth)).safeTransferFrom(owner, address(this), weEthAmount); @@ -149,8 +149,8 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU // To Receiver by transferring ETH (bool success, ) = receiver.call{value: ethReceived, gas: 100_000}(""); - require(success, "EtherFiWithdrawalBuffer: Transfer failed"); - require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiWithdrawalBuffer: Invalid liquidity pool balance"); + require(success, "EtherFiRedemptionManager: Transfer failed"); + require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiRedemptionManager: Invalid liquidity pool balance"); emit Redeemed(receiver, ethAmount, eEthFeeAmountToTreasury, eEthAmountToReceiver); } @@ -269,7 +269,7 @@ contract EtherFiWithdrawalBuffer is Initializable, OwnableUpgradeable, PausableU } function _hasRole(bytes32 role, address account) internal view returns (bool) { - require(roleRegistry.hasRole(role, account), "EtherFiWithdrawalBuffer: Unauthorized"); + require(roleRegistry.hasRole(role, account), "EtherFiRedemptionManager: Unauthorized"); } modifier hasRole(bytes32 role) { diff --git a/src/LiquidityPool.sol b/src/LiquidityPool.sol index 28ee11fdd..910dd05fe 100644 --- a/src/LiquidityPool.sol +++ b/src/LiquidityPool.sol @@ -23,7 +23,7 @@ import "./interfaces/IEtherFiAdmin.sol"; import "./interfaces/IAuctionManager.sol"; import "./interfaces/ILiquifier.sol"; -import "./EtherFiWithdrawalBuffer.sol"; +import "./EtherFiRedemptionManager.sol"; contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, ILiquidityPool { @@ -72,7 +72,7 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL bool private isLpBnftHolder; - EtherFiWithdrawalBuffer public etherFiWithdrawalBuffer; + EtherFiRedemptionManager public etherFiRedemptionManager; //-------------------------------------------------------------------------------------- //------------------------------------- EVENTS --------------------------------------- @@ -144,8 +144,9 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL liquifier = ILiquifier(_liquifier); } - function initializeOnUpgradeWithWithdrawalBuffer(address _withdrawalBuffer) external onlyOwner { - etherFiWithdrawalBuffer = EtherFiWithdrawalBuffer(payable(_withdrawalBuffer)); + function initializeOnUpgradeWithRedemptionManager(address _etherFiRedemptionManager) external onlyOwner { + require(address(etherFiRedemptionManager) == address(0), "Already initialized"); + etherFiRedemptionManager = EtherFiRedemptionManager(payable(_etherFiRedemptionManager)); } // Used by eETH staking flow @@ -188,7 +189,7 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL /// it returns the amount of shares burned function withdraw(address _recipient, uint256 _amount) external whenNotPaused returns (uint256) { uint256 share = sharesForWithdrawalAmount(_amount); - require(msg.sender == address(withdrawRequestNFT) || msg.sender == address(membershipManager) || msg.sender == address(etherFiWithdrawalBuffer), "Incorrect Caller"); + require(msg.sender == address(withdrawRequestNFT) || msg.sender == address(membershipManager) || msg.sender == address(etherFiRedemptionManager), "Incorrect Caller"); if (totalValueInLp < _amount || (msg.sender == address(withdrawRequestNFT) && ethAmountLockedForWithdrawal < _amount) || eETH.balanceOf(msg.sender) < _amount) revert InsufficientLiquidity(); if (_amount > type(uint128).max || _amount == 0 || share == 0) revert InvalidAmount(); diff --git a/test/EtherFiWithdrawalBuffer.t.sol b/test/EtherFiRedemptionManager.t.sol similarity index 63% rename from test/EtherFiWithdrawalBuffer.t.sol rename to test/EtherFiRedemptionManager.t.sol index 7aede9fcf..f9f8da8e6 100644 --- a/test/EtherFiWithdrawalBuffer.t.sol +++ b/test/EtherFiRedemptionManager.t.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.13; import "forge-std/console2.sol"; import "./TestSetup.sol"; -contract EtherFiWithdrawalBufferTest is TestSetup { +contract EtherFiRedemptionManagerTest is TestSetup { address user = vm.addr(999); address op_admin = vm.addr(1000); @@ -20,14 +20,14 @@ contract EtherFiWithdrawalBufferTest is TestSetup { roleRegistry.grantRole(keccak256("PROTOCOL_ADMIN"), op_admin); vm.stopPrank(); - etherFiWithdrawalBufferProxy = new UUPSProxy(address(new EtherFiWithdrawalBuffer(address(liquidityPoolInstance), address(eETHInstance), address(weEthInstance), address(treasuryInstance), address(roleRegistry))), ""); - etherFiWithdrawalBufferInstance = EtherFiWithdrawalBuffer(payable(etherFiWithdrawalBufferProxy)); - etherFiWithdrawalBufferInstance.initialize(10_00, 1_00, 1_00, 5 ether, 0.001 ether); // 10% fee split to treasury, 1% exit fee, 1% low watermark + etherFiRedemptionManagerProxy = new UUPSProxy(address(new EtherFiRedemptionManager(address(liquidityPoolInstance), address(eETHInstance), address(weEthInstance), address(treasuryInstance), address(roleRegistry))), ""); + etherFiRedemptionManagerInstance = EtherFiRedemptionManager(payable(etherFiRedemptionManagerProxy)); + etherFiRedemptionManagerInstance.initialize(10_00, 1_00, 1_00, 5 ether, 0.001 ether); // 10% fee split to treasury, 1% exit fee, 1% low watermark _upgrade_liquidity_pool_contract(); vm.prank(liquidityPoolInstance.owner()); - liquidityPoolInstance.initializeOnUpgradeWithWithdrawalBuffer(address(etherFiWithdrawalBufferInstance)); + liquidityPoolInstance.initializeOnUpgradeWithRedemptionManager(address(etherFiRedemptionManagerInstance)); } function test_rate_limit() public { @@ -35,33 +35,33 @@ contract EtherFiWithdrawalBufferTest is TestSetup { vm.prank(user); liquidityPoolInstance.deposit{value: 1000 ether}(); - assertEq(etherFiWithdrawalBufferInstance.canRedeem(1 ether), true); - assertEq(etherFiWithdrawalBufferInstance.canRedeem(5 ether - 1), true); - assertEq(etherFiWithdrawalBufferInstance.canRedeem(5 ether + 1), false); - assertEq(etherFiWithdrawalBufferInstance.canRedeem(10 ether), false); - assertEq(etherFiWithdrawalBufferInstance.totalRedeemableAmount(), 5 ether); + assertEq(etherFiRedemptionManagerInstance.canRedeem(1 ether), true); + assertEq(etherFiRedemptionManagerInstance.canRedeem(5 ether - 1), true); + assertEq(etherFiRedemptionManagerInstance.canRedeem(5 ether + 1), false); + assertEq(etherFiRedemptionManagerInstance.canRedeem(10 ether), false); + assertEq(etherFiRedemptionManagerInstance.totalRedeemableAmount(), 5 ether); } function test_lowwatermark_guardrail() public { vm.deal(user, 100 ether); - assertEq(etherFiWithdrawalBufferInstance.lowWatermarkInETH(), 0 ether); + assertEq(etherFiRedemptionManagerInstance.lowWatermarkInETH(), 0 ether); vm.prank(user); liquidityPoolInstance.deposit{value: 100 ether}(); - vm.startPrank(etherFiWithdrawalBufferInstance.owner()); - etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(1_00); // 1% - assertEq(etherFiWithdrawalBufferInstance.lowWatermarkInETH(), 1 ether); + vm.startPrank(etherFiRedemptionManagerInstance.owner()); + etherFiRedemptionManagerInstance.setLowWatermarkInBpsOfTvl(1_00); // 1% + assertEq(etherFiRedemptionManagerInstance.lowWatermarkInETH(), 1 ether); - etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(50_00); // 50% - assertEq(etherFiWithdrawalBufferInstance.lowWatermarkInETH(), 50 ether); + etherFiRedemptionManagerInstance.setLowWatermarkInBpsOfTvl(50_00); // 50% + assertEq(etherFiRedemptionManagerInstance.lowWatermarkInETH(), 50 ether); - etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(100_00); // 100% - assertEq(etherFiWithdrawalBufferInstance.lowWatermarkInETH(), 100 ether); + etherFiRedemptionManagerInstance.setLowWatermarkInBpsOfTvl(100_00); // 100% + assertEq(etherFiRedemptionManagerInstance.lowWatermarkInETH(), 100 ether); vm.expectRevert("INVALID"); - etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(100_01); // 100.01% + etherFiRedemptionManagerInstance.setLowWatermarkInBpsOfTvl(100_01); // 100.01% } function testFuzz_redeemEEth( @@ -84,22 +84,22 @@ contract EtherFiWithdrawalBufferTest is TestSetup { // Set exitFeeSplitToTreasuryInBps vm.prank(owner); - etherFiWithdrawalBufferInstance.setExitFeeSplitToTreasuryInBps(uint16(exitFeeSplitBps)); + etherFiRedemptionManagerInstance.setExitFeeSplitToTreasuryInBps(uint16(exitFeeSplitBps)); // Set exitFeeBasisPoints and lowWatermarkInBpsOfTvl vm.prank(owner); - etherFiWithdrawalBufferInstance.setExitFeeBasisPoints(exitFeeBps); + etherFiRedemptionManagerInstance.setExitFeeBasisPoints(exitFeeBps); vm.prank(owner); - etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(lowWatermarkBps); + etherFiRedemptionManagerInstance.setLowWatermarkInBpsOfTvl(lowWatermarkBps); vm.startPrank(user); - if (etherFiWithdrawalBufferInstance.canRedeem(redeemAmount)) { + if (etherFiRedemptionManagerInstance.canRedeem(redeemAmount)) { uint256 userBalanceBefore = address(user).balance; uint256 treasuryBalanceBefore = eETHInstance.balanceOf(address(treasuryInstance)); - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), redeemAmount); - etherFiWithdrawalBufferInstance.redeemEEth(redeemAmount, user, user); + eETHInstance.approve(address(etherFiRedemptionManagerInstance), redeemAmount); + etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user, user); uint256 totalFee = (redeemAmount * exitFeeBps) / 10000; uint256 treasuryFee = (totalFee * exitFeeSplitBps) / 10000; @@ -118,7 +118,7 @@ contract EtherFiWithdrawalBufferTest is TestSetup { } else { vm.expectRevert(); - etherFiWithdrawalBufferInstance.redeemEEth(redeemAmount, user, user); + etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user, user); } vm.stopPrank(); } @@ -151,13 +151,13 @@ contract EtherFiWithdrawalBufferTest is TestSetup { // Set fee and watermark configurations vm.prank(owner); - etherFiWithdrawalBufferInstance.setExitFeeSplitToTreasuryInBps(uint16(exitFeeSplitBps)); + etherFiRedemptionManagerInstance.setExitFeeSplitToTreasuryInBps(uint16(exitFeeSplitBps)); vm.prank(owner); - etherFiWithdrawalBufferInstance.setExitFeeBasisPoints(exitFeeBps); + etherFiRedemptionManagerInstance.setExitFeeBasisPoints(exitFeeBps); vm.prank(owner); - etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(lowWatermarkBps); + etherFiRedemptionManagerInstance.setLowWatermarkInBpsOfTvl(lowWatermarkBps); // Convert redeemAmount from ETH to weETH vm.startPrank(user); @@ -165,14 +165,14 @@ contract EtherFiWithdrawalBufferTest is TestSetup { weEthInstance.wrap(redeemAmount); uint256 weEthAmount = weEthInstance.balanceOf(user); - if (etherFiWithdrawalBufferInstance.canRedeem(redeemAmount)) { + if (etherFiRedemptionManagerInstance.canRedeem(redeemAmount)) { uint256 userBalanceBefore = address(user).balance; uint256 treasuryBalanceBefore = eETHInstance.balanceOf(address(treasuryInstance)); uint256 eEthAmount = liquidityPoolInstance.amountForShare(weEthAmount); - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), weEthAmount); - etherFiWithdrawalBufferInstance.redeemWeEth(weEthAmount, user, user); + weEthInstance.approve(address(etherFiRedemptionManagerInstance), weEthAmount); + etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user, user); uint256 totalFee = (eEthAmount * exitFeeBps) / 10000; uint256 treasuryFee = (totalFee * exitFeeSplitBps) / 10000; @@ -191,7 +191,7 @@ contract EtherFiWithdrawalBufferTest is TestSetup { } else { vm.expectRevert(); - etherFiWithdrawalBufferInstance.redeemWeEth(weEthAmount, user, user); + etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user, user); } vm.stopPrank(); } @@ -217,23 +217,23 @@ contract EtherFiWithdrawalBufferTest is TestSetup { // Admin performs admin-only actions vm.startPrank(admin); - etherFiWithdrawalBufferInstance.setCapacity(10 ether); - etherFiWithdrawalBufferInstance.setRefillRatePerSecond(0.001 ether); - etherFiWithdrawalBufferInstance.setExitFeeSplitToTreasuryInBps(1e4); - etherFiWithdrawalBufferInstance.setLowWatermarkInBpsOfTvl(1e2); - etherFiWithdrawalBufferInstance.setExitFeeBasisPoints(1e2); + etherFiRedemptionManagerInstance.setCapacity(10 ether); + etherFiRedemptionManagerInstance.setRefillRatePerSecond(0.001 ether); + etherFiRedemptionManagerInstance.setExitFeeSplitToTreasuryInBps(1e4); + etherFiRedemptionManagerInstance.setLowWatermarkInBpsOfTvl(1e2); + etherFiRedemptionManagerInstance.setExitFeeBasisPoints(1e2); vm.stopPrank(); // Pauser pauses the contract vm.startPrank(pauser); - etherFiWithdrawalBufferInstance.pauseContract(); - assertTrue(etherFiWithdrawalBufferInstance.paused()); + etherFiRedemptionManagerInstance.pauseContract(); + assertTrue(etherFiRedemptionManagerInstance.paused()); vm.stopPrank(); // Unpauser unpauses the contract vm.startPrank(unpauser); - etherFiWithdrawalBufferInstance.unPauseContract(); - assertFalse(etherFiWithdrawalBufferInstance.paused()); + etherFiRedemptionManagerInstance.unPauseContract(); + assertFalse(etherFiRedemptionManagerInstance.paused()); vm.stopPrank(); // Revoke PROTOCOL_ADMIN role from admin @@ -242,28 +242,28 @@ contract EtherFiWithdrawalBufferTest is TestSetup { // Admin attempts admin-only actions after role revocation vm.startPrank(admin); - vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); - etherFiWithdrawalBufferInstance.setCapacity(10 ether); + vm.expectRevert("EtherFiRedemptionManager: Unauthorized"); + etherFiRedemptionManagerInstance.setCapacity(10 ether); vm.stopPrank(); // Pauser attempts to unpause (should fail) vm.startPrank(pauser); - vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); - etherFiWithdrawalBufferInstance.unPauseContract(); + vm.expectRevert("EtherFiRedemptionManager: Unauthorized"); + etherFiRedemptionManagerInstance.unPauseContract(); vm.stopPrank(); // Unpauser attempts to pause (should fail) vm.startPrank(unpauser); - vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); - etherFiWithdrawalBufferInstance.pauseContract(); + vm.expectRevert("EtherFiRedemptionManager: Unauthorized"); + etherFiRedemptionManagerInstance.pauseContract(); vm.stopPrank(); // User without role attempts admin-only actions vm.startPrank(user); - vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); - etherFiWithdrawalBufferInstance.pauseContract(); - vm.expectRevert("EtherFiWithdrawalBuffer: Unauthorized"); - etherFiWithdrawalBufferInstance.unPauseContract(); + vm.expectRevert("EtherFiRedemptionManager: Unauthorized"); + etherFiRedemptionManagerInstance.pauseContract(); + vm.expectRevert("EtherFiRedemptionManager: Unauthorized"); + etherFiRedemptionManagerInstance.unPauseContract(); vm.stopPrank(); } @@ -280,23 +280,23 @@ contract EtherFiWithdrawalBufferTest is TestSetup { liquidityPoolInstance.deposit{value: 10 ether}(); - uint256 redeemableAmount = etherFiWithdrawalBufferInstance.totalRedeemableAmount(); + uint256 redeemableAmount = etherFiRedemptionManagerInstance.totalRedeemableAmount(); uint256 userBalance = address(user).balance; - uint256 treasuryBalance = eETHInstance.balanceOf(address(etherFiWithdrawalBufferInstance.treasury())); + uint256 treasuryBalance = eETHInstance.balanceOf(address(etherFiRedemptionManagerInstance.treasury())); - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); - etherFiWithdrawalBufferInstance.redeemEEth(1 ether, user, user); + eETHInstance.approve(address(etherFiRedemptionManagerInstance), 1 ether); + etherFiRedemptionManagerInstance.redeemEEth(1 ether, user, user); uint256 totalFee = (1 ether * 1e2) / 1e4; uint256 treasuryFee = (totalFee * 1e3) / 1e4; uint256 userReceives = 1 ether - totalFee; - assertApproxEqAbs(eETHInstance.balanceOf(address(etherFiWithdrawalBufferInstance.treasury())), treasuryBalance + treasuryFee, 1e1); + assertApproxEqAbs(eETHInstance.balanceOf(address(etherFiRedemptionManagerInstance.treasury())), treasuryBalance + treasuryFee, 1e1); assertApproxEqAbs(address(user).balance, userBalance + userReceives, 1e1); - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), 5 ether); - vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); - etherFiWithdrawalBufferInstance.redeemEEth(5 ether, user, user); + eETHInstance.approve(address(etherFiRedemptionManagerInstance), 5 ether); + vm.expectRevert("EtherFiRedemptionManager: Exceeded total redeemable amount"); + etherFiRedemptionManagerInstance.redeemEEth(5 ether, user, user); vm.stopPrank(); } @@ -326,8 +326,8 @@ contract EtherFiWithdrawalBufferTest is TestSetup { uint256 eEthAmount = liquidityPoolInstance.amountForShare(weEthAmount); uint256 userBalance = address(user).balance; uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); - weEthInstance.approve(address(etherFiWithdrawalBufferInstance), 1 ether); - etherFiWithdrawalBufferInstance.redeemWeEth(weEthAmount, user, user); + weEthInstance.approve(address(etherFiRedemptionManagerInstance), 1 ether); + etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user, user); uint256 totalFee = (eEthAmount * 1e2) / 1e4; uint256 treasuryFee = (totalFee * 1e3) / 1e4; @@ -347,8 +347,8 @@ contract EtherFiWithdrawalBufferTest is TestSetup { eETHInstance.mintShares(user, 2 * redeemAmount); vm.startPrank(op_admin); - etherFiWithdrawalBufferInstance.setCapacity(2 * redeemAmount); - etherFiWithdrawalBufferInstance.setRefillRatePerSecond(2 * redeemAmount); + etherFiRedemptionManagerInstance.setCapacity(2 * redeemAmount); + etherFiRedemptionManagerInstance.setRefillRatePerSecond(2 * redeemAmount); vm.stopPrank(); vm.warp(block.timestamp + 1); @@ -356,11 +356,11 @@ contract EtherFiWithdrawalBufferTest is TestSetup { vm.startPrank(user); uint256 userBalance = address(user).balance; - uint256 treasuryBalance = eETHInstance.balanceOf(address(etherFiWithdrawalBufferInstance.treasury())); + uint256 treasuryBalance = eETHInstance.balanceOf(address(etherFiRedemptionManagerInstance.treasury())); - eETHInstance.approve(address(etherFiWithdrawalBufferInstance), redeemAmount); - vm.expectRevert("EtherFiWithdrawalBuffer: Exceeded total redeemable amount"); - etherFiWithdrawalBufferInstance.redeemEEth(redeemAmount, user, user); + eETHInstance.approve(address(etherFiRedemptionManagerInstance), redeemAmount); + vm.expectRevert("EtherFiRedemptionManager: Exceeded total redeemable amount"); + etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user, user); vm.stopPrank(); } diff --git a/test/TestSetup.sol b/test/TestSetup.sol index af6a4ca82..f27c906b0 100644 --- a/test/TestSetup.sol +++ b/test/TestSetup.sol @@ -49,7 +49,7 @@ import "../src/EtherFiAdmin.sol"; import "../src/EtherFiTimelock.sol"; import "../src/BucketRateLimiter.sol"; -import "../src/EtherFiWithdrawalBuffer.sol"; +import "../src/EtherFiRedemptionManager.sol"; contract TestSetup is Test { @@ -103,7 +103,7 @@ contract TestSetup is Test { UUPSProxy public membershipNftProxy; UUPSProxy public nftExchangeProxy; UUPSProxy public withdrawRequestNFTProxy; - UUPSProxy public etherFiWithdrawalBufferProxy; + UUPSProxy public etherFiRedemptionManagerProxy; UUPSProxy public etherFiOracleProxy; UUPSProxy public etherFiAdminProxy; UUPSProxy public roleRegistryProxy; @@ -164,7 +164,7 @@ contract TestSetup is Test { WithdrawRequestNFT public withdrawRequestNFTImplementation; WithdrawRequestNFT public withdrawRequestNFTInstance; - EtherFiWithdrawalBuffer public etherFiWithdrawalBufferInstance; + EtherFiRedemptionManager public etherFiRedemptionManagerInstance; NFTExchange public nftExchangeImplementation; NFTExchange public nftExchangeInstance; @@ -584,14 +584,14 @@ contract TestSetup is Test { roleRegistry = RoleRegistry(address(roleRegistryProxy)); roleRegistry.initialize(owner); - etherFiWithdrawalBufferProxy = new UUPSProxy(address(new EtherFiWithdrawalBuffer(address(liquidityPoolInstance), address(eETHInstance), address(weEthInstance), address(treasuryInstance), address(roleRegistry))), ""); - etherFiWithdrawalBufferInstance = EtherFiWithdrawalBuffer(payable(etherFiWithdrawalBufferProxy)); - etherFiWithdrawalBufferInstance.initialize(10_00, 1_00, 1_00, 5 ether, 0.001 ether); + etherFiRedemptionManagerProxy = new UUPSProxy(address(new EtherFiRedemptionManager(address(liquidityPoolInstance), address(eETHInstance), address(weEthInstance), address(treasuryInstance), address(roleRegistry))), ""); + etherFiRedemptionManagerInstance = EtherFiRedemptionManager(payable(etherFiRedemptionManagerProxy)); + etherFiRedemptionManagerInstance.initialize(10_00, 1_00, 1_00, 5 ether, 0.001 ether); roleRegistry.grantRole(keccak256("PROTOCOL_ADMIN"), owner); liquidityPoolInstance.initialize(address(eETHInstance), address(stakingManagerInstance), address(etherFiNodeManagerProxy), address(membershipManagerInstance), address(TNFTInstance), address(etherFiAdminProxy), address(withdrawRequestNFTInstance)); - liquidityPoolInstance.initializeOnUpgradeWithWithdrawalBuffer(address(etherFiWithdrawalBufferInstance)); + liquidityPoolInstance.initializeOnUpgradeWithRedemptionManager(address(etherFiRedemptionManagerInstance)); membershipNftInstance.initialize("https://etherfi-cdn/{id}.json", address(membershipManagerInstance)); withdrawRequestNFTInstance.initialize(payable(address(liquidityPoolInstance)), payable(address(eETHInstance)), payable(address(membershipManagerInstance))); From bcc1184daa0f7c28a90d56e5fdd0aea43b474ccc Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Tue, 31 Dec 2024 00:29:44 +0900 Subject: [PATCH 14/42] fix the logic to check the aggr calls --- src/WithdrawRequestNFT.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 90b939b72..6a47c3c3b 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -233,7 +233,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad /// @param _eEthAmount: the remainder of the eEth amount function handleRemainder(uint256 _eEthAmount) external onlyAdmin { require(getEEthRemainderAmount() >= _eEthAmount, "Not enough eETH remainder"); - require(_currentRequestIdToScanFromForShareRemainder == nextRequestId, "Not all requests have been scanned"); + require(_currentRequestIdToScanFromForShareRemainder == _lastRequestIdToScanUntilForShareRemainder + 1, "Not all prev requests have been scanned"); uint256 beforeEEthShares = eETH.shares(address(this)); From 1f85fb6911e403879a714ca77c8843a5b43d70a5 Mon Sep 17 00:00:00 2001 From: Jacob T Firek <106350168+jtfirek@users.noreply.github.com> Date: Mon, 30 Dec 2024 17:14:49 -0500 Subject: [PATCH 15/42] Update test/WithdrawRequestNFT.t.sol Signed-off-by: Jacob T Firek <106350168+jtfirek@users.noreply.github.com> --- test/WithdrawRequestNFT.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 7e3e62944..172362a37 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -357,7 +357,7 @@ contract WithdrawRequestNFTTest is TestSetup { test_aggregateSumEEthShareAmount(); vm.startPrank(withdrawRequestNFTInstance.owner()); - vm.expectRevert("Not all requests have been scanned"); + vm.expectRevert("Not all prev requests have been scanned"); withdrawRequestNFTInstance.handleRemainder(1 ether); vm.stopPrank(); From bdee463a963731570728e7df1a0955bd12c6c7ec Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Tue, 31 Dec 2024 07:48:47 +0900 Subject: [PATCH 16/42] reduce gas spending for 'call', update the upgrade init function, remove unused file --- src/EtherFiRedemptionManager.sol | 2 +- src/EtherFiWithdrawalBuffer.sol | 280 ------------------------------- src/WithdrawRequestNFT.sol | 4 +- test/WithdrawRequestNFT.t.sol | 3 +- 4 files changed, 5 insertions(+), 284 deletions(-) delete mode 100644 src/EtherFiWithdrawalBuffer.sol diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index c6c445c53..434e46f25 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -148,7 +148,7 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable IERC20(address(eEth)).safeTransfer(treasury, eEthFeeAmountToTreasury); // To Receiver by transferring ETH - (bool success, ) = receiver.call{value: ethReceived, gas: 100_000}(""); + (bool success, ) = receiver.call{value: ethReceived, gas: 10_000}(""); require(success, "EtherFiRedemptionManager: Transfer failed"); require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiRedemptionManager: Invalid liquidity pool balance"); diff --git a/src/EtherFiWithdrawalBuffer.sol b/src/EtherFiWithdrawalBuffer.sol deleted file mode 100644 index c6c445c53..000000000 --- a/src/EtherFiWithdrawalBuffer.sol +++ /dev/null @@ -1,280 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.13; - -import "@openzeppelin/contracts/utils/math/SafeCast.sol"; -import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; -import "@openzeppelin/contracts/token/ERC20/extensions/draft-IERC20Permit.sol"; -import "@openzeppelin-upgradeable/contracts/token/ERC20/IERC20Upgradeable.sol"; -import "@openzeppelin-upgradeable/contracts/proxy/utils/Initializable.sol"; -import "@openzeppelin-upgradeable/contracts/proxy/utils/UUPSUpgradeable.sol"; -import "@openzeppelin-upgradeable/contracts/security/ReentrancyGuardUpgradeable.sol"; -import "@openzeppelin-upgradeable/contracts/access/OwnableUpgradeable.sol"; -import "@openzeppelin-upgradeable/contracts/security/PausableUpgradeable.sol"; - -import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import "@openzeppelin/contracts/utils/math/Math.sol"; - -import "./interfaces/ILiquidityPool.sol"; -import "./interfaces/IeETH.sol"; -import "./interfaces/IWeETH.sol"; - -import "lib/BucketLimiter.sol"; - -import "./RoleRegistry.sol"; - -/* - The contract allows instant redemption of eETH and weETH tokens to ETH with an exit fee. - - It has the exit fee as a percentage of the total amount redeemed. - - It has a rate limiter to limit the total amount that can be redeemed in a given time period. -*/ -contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable, UUPSUpgradeable { - using SafeERC20 for IERC20; - using Math for uint256; - - uint256 private constant BUCKET_UNIT_SCALE = 1e12; - uint256 private constant BASIS_POINT_SCALE = 1e4; - - bytes32 public constant PROTOCOL_PAUSER = keccak256("PROTOCOL_PAUSER"); - bytes32 public constant PROTOCOL_UNPAUSER = keccak256("PROTOCOL_UNPAUSER"); - bytes32 public constant PROTOCOL_ADMIN = keccak256("PROTOCOL_ADMIN"); - - RoleRegistry public immutable roleRegistry; - address public immutable treasury; - IeETH public immutable eEth; - IWeETH public immutable weEth; - ILiquidityPool public immutable liquidityPool; - - BucketLimiter.Limit public limit; - uint16 public exitFeeSplitToTreasuryInBps; - uint16 public exitFeeInBps; - uint16 public lowWatermarkInBpsOfTvl; // bps of TVL - - event Redeemed(address indexed receiver, uint256 redemptionAmount, uint256 feeAmountToTreasury, uint256 feeAmountToStakers); - - receive() external payable {} - - /// @custom:oz-upgrades-unsafe-allow constructor - constructor(address _liquidityPool, address _eEth, address _weEth, address _treasury, address _roleRegistry) { - roleRegistry = RoleRegistry(_roleRegistry); - treasury = _treasury; - liquidityPool = ILiquidityPool(payable(_liquidityPool)); - eEth = IeETH(_eEth); - weEth = IWeETH(_weEth); - - _disableInitializers(); - } - - function initialize(uint16 _exitFeeSplitToTreasuryInBps, uint16 _exitFeeInBps, uint16 _lowWatermarkInBpsOfTvl, uint256 _bucketCapacity, uint256 _bucketRefillRate) external initializer { - __Ownable_init(); - __UUPSUpgradeable_init(); - __Pausable_init(); - __ReentrancyGuard_init(); - - limit = BucketLimiter.create(_convertToBucketUnit(_bucketCapacity, Math.Rounding.Down), _convertToBucketUnit(_bucketRefillRate, Math.Rounding.Down)); - exitFeeSplitToTreasuryInBps = _exitFeeSplitToTreasuryInBps; - exitFeeInBps = _exitFeeInBps; - lowWatermarkInBpsOfTvl = _lowWatermarkInBpsOfTvl; - } - - /** - * @notice Redeems eETH for ETH. - * @param eEthAmount The amount of eETH to redeem after the exit fee. - * @param receiver The address to receive the redeemed ETH. - * @param owner The address of the owner of the eETH. - */ - function redeemEEth(uint256 eEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { - require(eEthAmount <= eEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); - require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); - - uint256 beforeEEthAmount = eEth.balanceOf(address(this)); - IERC20(address(eEth)).safeTransferFrom(owner, address(this), eEthAmount); - uint256 afterEEthAmount = eEth.balanceOf(address(this)); - - uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; - _redeem(transferredEEthAmount, receiver); - } - - /** - * @notice Redeems weETH for ETH. - * @param weEthAmount The amount of weETH to redeem after the exit fee. - * @param receiver The address to receive the redeemed ETH. - * @param owner The address of the owner of the weETH. - */ - function redeemWeEth(uint256 weEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { - uint256 eEthShares = weEthAmount; - uint256 eEthAmount = liquidityPool.amountForShare(eEthShares); - require(weEthAmount <= weEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); - require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); - - uint256 beforeEEthAmount = eEth.balanceOf(address(this)); - IERC20(address(weEth)).safeTransferFrom(owner, address(this), weEthAmount); - weEth.unwrap(weEthAmount); - uint256 afterEEthAmount = eEth.balanceOf(address(this)); - - uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; - _redeem(transferredEEthAmount, receiver); - } - - - /** - * @notice Redeems ETH. - * @param ethAmount The amount of ETH to redeem after the exit fee. - * @param receiver The address to receive the redeemed ETH. - */ - function _redeem(uint256 ethAmount, address receiver) internal { - _updateRateLimit(ethAmount); - - uint256 ethShares = liquidityPool.sharesForAmount(ethAmount); - uint256 ethShareToReceiver = ethShares.mulDiv(BASIS_POINT_SCALE - exitFeeInBps, BASIS_POINT_SCALE); - uint256 eEthAmountToReceiver = liquidityPool.amountForShare(ethShareToReceiver); - - uint256 prevLpBalance = address(liquidityPool).balance; - uint256 sharesToBurn = liquidityPool.sharesForWithdrawalAmount(eEthAmountToReceiver); - - uint256 ethShareFee = ethShares - sharesToBurn; - uint256 feeShareToTreasury = ethShareFee.mulDiv(exitFeeSplitToTreasuryInBps, BASIS_POINT_SCALE); - uint256 eEthFeeAmountToTreasury = liquidityPool.amountForShare(feeShareToTreasury); - uint256 feeShareToStakers = ethShareFee - feeShareToTreasury; - - // Withdraw ETH from the liquidity pool - uint256 prevBalance = address(this).balance; - assert (liquidityPool.withdraw(address(this), eEthAmountToReceiver) == sharesToBurn); - uint256 ethReceived = address(this).balance - prevBalance; - - // To Stakers by burning shares - eEth.burnShares(address(this), feeShareToStakers); - - // To Treasury by transferring eETH - IERC20(address(eEth)).safeTransfer(treasury, eEthFeeAmountToTreasury); - - // To Receiver by transferring ETH - (bool success, ) = receiver.call{value: ethReceived, gas: 100_000}(""); - require(success, "EtherFiRedemptionManager: Transfer failed"); - require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiRedemptionManager: Invalid liquidity pool balance"); - - emit Redeemed(receiver, ethAmount, eEthFeeAmountToTreasury, eEthAmountToReceiver); - } - - /** - * @dev if the contract has less than the low watermark, it will not allow any instant redemption. - */ - function lowWatermarkInETH() public view returns (uint256) { - return liquidityPool.getTotalPooledEther().mulDiv(lowWatermarkInBpsOfTvl, BASIS_POINT_SCALE); - } - - /** - * @dev Returns the total amount that can be redeemed. - */ - function totalRedeemableAmount() external view returns (uint256) { - uint256 liquidEthAmount = address(liquidityPool).balance - liquidityPool.ethAmountLockedForWithdrawal(); - if (liquidEthAmount < lowWatermarkInETH()) { - return 0; - } - uint64 consumableBucketUnits = BucketLimiter.consumable(limit); - uint256 consumableAmount = _convertFromBucketUnit(consumableBucketUnits); - return Math.min(consumableAmount, liquidEthAmount); - } - - /** - * @dev Returns whether the given amount can be redeemed. - * @param amount The ETH or eETH amount to check. - */ - function canRedeem(uint256 amount) public view returns (bool) { - uint256 liquidEthAmount = address(liquidityPool).balance - liquidityPool.ethAmountLockedForWithdrawal(); - if (liquidEthAmount < lowWatermarkInETH()) { - return false; - } - uint64 bucketUnit = _convertToBucketUnit(amount, Math.Rounding.Up); - bool consumable = BucketLimiter.canConsume(limit, bucketUnit); - return consumable && amount <= liquidEthAmount; - } - - /** - * @dev Sets the maximum size of the bucket that can be consumed in a given time period. - * @param capacity The capacity of the bucket. - */ - function setCapacity(uint256 capacity) external hasRole(PROTOCOL_ADMIN) { - // max capacity = max(uint64) * 1e12 ~= 16 * 1e18 * 1e12 = 16 * 1e12 ether, which is practically enough - uint64 bucketUnit = _convertToBucketUnit(capacity, Math.Rounding.Down); - BucketLimiter.setCapacity(limit, bucketUnit); - } - - /** - * @dev Sets the rate at which the bucket is refilled per second. - * @param refillRate The rate at which the bucket is refilled per second. - */ - function setRefillRatePerSecond(uint256 refillRate) external hasRole(PROTOCOL_ADMIN) { - // max refillRate = max(uint64) * 1e12 ~= 16 * 1e18 * 1e12 = 16 * 1e12 ether per second, which is practically enough - uint64 bucketUnit = _convertToBucketUnit(refillRate, Math.Rounding.Down); - BucketLimiter.setRefillRate(limit, bucketUnit); - } - - /** - * @dev Sets the exit fee. - * @param _exitFeeInBps The exit fee. - */ - function setExitFeeBasisPoints(uint16 _exitFeeInBps) external hasRole(PROTOCOL_ADMIN) { - require(_exitFeeInBps <= BASIS_POINT_SCALE, "INVALID"); - exitFeeInBps = _exitFeeInBps; - } - - function setLowWatermarkInBpsOfTvl(uint16 _lowWatermarkInBpsOfTvl) external hasRole(PROTOCOL_ADMIN) { - require(_lowWatermarkInBpsOfTvl <= BASIS_POINT_SCALE, "INVALID"); - lowWatermarkInBpsOfTvl = _lowWatermarkInBpsOfTvl; - } - - function setExitFeeSplitToTreasuryInBps(uint16 _exitFeeSplitToTreasuryInBps) external hasRole(PROTOCOL_ADMIN) { - require(_exitFeeSplitToTreasuryInBps <= BASIS_POINT_SCALE, "INVALID"); - exitFeeSplitToTreasuryInBps = _exitFeeSplitToTreasuryInBps; - } - - function pauseContract() external hasRole(PROTOCOL_PAUSER) { - _pause(); - } - - function unPauseContract() external hasRole(PROTOCOL_UNPAUSER) { - _unpause(); - } - - function _updateRateLimit(uint256 amount) internal { - uint64 bucketUnit = _convertToBucketUnit(amount, Math.Rounding.Up); - require(BucketLimiter.consume(limit, bucketUnit), "BucketRateLimiter: rate limit exceeded"); - } - - function _convertToBucketUnit(uint256 amount, Math.Rounding rounding) internal pure returns (uint64) { - return (rounding == Math.Rounding.Up) ? SafeCast.toUint64((amount + BUCKET_UNIT_SCALE - 1) / BUCKET_UNIT_SCALE) : SafeCast.toUint64(amount / BUCKET_UNIT_SCALE); - } - - function _convertFromBucketUnit(uint64 bucketUnit) internal pure returns (uint256) { - return bucketUnit * BUCKET_UNIT_SCALE; - } - - /** - * @dev Preview taking an exit fee on redeem. See {IERC4626-previewRedeem}. - */ - // redeemable amount after exit fee - function previewRedeem(uint256 shares) public view returns (uint256) { - uint256 amountInEth = liquidityPool.amountForShare(shares); - return amountInEth - _fee(amountInEth, exitFeeInBps); - } - - function _fee(uint256 assets, uint256 feeBasisPoints) internal pure virtual returns (uint256) { - return assets.mulDiv(feeBasisPoints, BASIS_POINT_SCALE, Math.Rounding.Up); - } - - function _authorizeUpgrade(address newImplementation) internal override onlyOwner {} - - function getImplementation() external view returns (address) { - return _getImplementation(); - } - - function _hasRole(bytes32 role, address account) internal view returns (bool) { - require(roleRegistry.hasRole(role, account), "EtherFiRedemptionManager: Unauthorized"); - } - - modifier hasRole(bytes32 role) { - _hasRole(role, msg.sender); - _; - } - -} \ No newline at end of file diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 6a47c3c3b..2ad797057 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -70,12 +70,14 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad nextRequestId = 1; } - function initializeOnUpgrade(address _pauser) external onlyOwner { + function initializeOnUpgrade(address _pauser, uint16 _shareRemainderSplitToTreasuryInBps) external onlyOwner { require(pauser == address(0), "Already initialized"); paused = false; pauser = _pauser; + shareRemainderSplitToTreasuryInBps = _shareRemainderSplitToTreasuryInBps; + _currentRequestIdToScanFromForShareRemainder = 1; _lastRequestIdToScanUntilForShareRemainder = nextRequestId - 1; } diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 172362a37..1c8b326c6 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -327,8 +327,7 @@ contract WithdrawRequestNFTTest is TestSetup { vm.startPrank(withdrawRequestNFTInstance.owner()); // 1. Upgrade withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(address(owner)))); - withdrawRequestNFTInstance.initializeOnUpgrade(etherfi_admin_wallet); - withdrawRequestNFTInstance.updateShareRemainderSplitToTreasuryInBps(50_00); + withdrawRequestNFTInstance.initializeOnUpgrade(etherfi_admin_wallet, 50_00); withdrawRequestNFTInstance.updateAdmin(etherfi_admin_wallet, true); // 2. PAUSE From d40a117159983f852ffd40978876048315b1121d Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Tue, 31 Dec 2024 08:10:15 +0900 Subject: [PATCH 17/42] apply gas opt for BucketLimiter --- lib/BucketLimiter.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/BucketLimiter.sol b/lib/BucketLimiter.sol index 83f749156..36dc9a6ed 100644 --- a/lib/BucketLimiter.sol +++ b/lib/BucketLimiter.sol @@ -111,6 +111,11 @@ library BucketLimiter { function _refill(Limit memory limit) internal view { // We allow for overflow here, as the delta is resilient against it. uint64 now_ = uint64(block.timestamp); + + if (now_ == limit.lastRefill) { + return; + } + uint64 delta; unchecked { delta = now_ - limit.lastRefill; From f4f2ffdf7340128abcb1e2e43e3a9558d9de2456 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Tue, 31 Dec 2024 08:32:15 +0900 Subject: [PATCH 18/42] improve assetion tsets, apply design pattern, function rename --- src/EtherFiRedemptionManager.sol | 7 +++++-- src/WithdrawRequestNFT.sol | 11 ++++++----- test/WithdrawRequestNFT.t.sol | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index 434e46f25..a44a2eaa3 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -125,8 +125,7 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable _updateRateLimit(ethAmount); uint256 ethShares = liquidityPool.sharesForAmount(ethAmount); - uint256 ethShareToReceiver = ethShares.mulDiv(BASIS_POINT_SCALE - exitFeeInBps, BASIS_POINT_SCALE); - uint256 eEthAmountToReceiver = liquidityPool.amountForShare(ethShareToReceiver); + uint256 eEthAmountToReceiver = liquidityPool.amountForShare(ethShares.mulDiv(BASIS_POINT_SCALE - exitFeeInBps, BASIS_POINT_SCALE)); // ethShareToReceiver uint256 prevLpBalance = address(liquidityPool).balance; uint256 sharesToBurn = liquidityPool.sharesForWithdrawalAmount(eEthAmountToReceiver); @@ -137,6 +136,7 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable uint256 feeShareToStakers = ethShareFee - feeShareToTreasury; // Withdraw ETH from the liquidity pool + uint256 totalEEthShare = eEth.totalShares(); uint256 prevBalance = address(this).balance; assert (liquidityPool.withdraw(address(this), eEthAmountToReceiver) == sharesToBurn); uint256 ethReceived = address(this).balance - prevBalance; @@ -150,7 +150,10 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable // To Receiver by transferring ETH (bool success, ) = receiver.call{value: ethReceived, gas: 10_000}(""); require(success, "EtherFiRedemptionManager: Transfer failed"); + + // Make sure the liquidity pool balance is correct && total shares are correct require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiRedemptionManager: Invalid liquidity pool balance"); + require(eEth.totalShares() == totalEEthShare - (sharesToBurn + feeShareToStakers), "EtherFiRedemptionManager: Invalid total shares"); emit Redeemed(receiver, ethAmount, eEthFeeAmountToTreasury, eEthAmountToReceiver); } diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 2ad797057..27207a294 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -130,12 +130,13 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad require(request.isValid, "Request is not valid"); uint256 amountToWithdraw = getClaimableAmount(tokenId); + uint256 shareAmountToBurnForWithdrawal = liquidityPool.sharesForWithdrawalAmount(amountToWithdraw); // transfer eth to recipient _burn(tokenId); delete _requests[tokenId]; - - uint256 shareAmountToBurnForWithdrawal = liquidityPool.sharesForWithdrawalAmount(amountToWithdraw); + + // update accounting totalLockedEEthShares -= shareAmountToBurnForWithdrawal; uint256 amountBurnedShare = liquidityPool.withdraw(recipient, amountToWithdraw); @@ -166,7 +167,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } // Seize the request simply by transferring it to another recipient - function seizeRequest(uint256 requestId, address recipient) external onlyOwner { + function seizeInvalidRequest(uint256 requestId, address recipient) external onlyOwner { require(!_requests[requestId].isValid, "Request is valid"); require(_exists(requestId), "Request does not exist"); @@ -184,7 +185,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function isValid(uint256 requestId) public view returns (bool) { - require(_exists(requestId), "Request does not exist11"); + require(_exists(requestId), "Request does not exist"); return _requests[requestId].isValid; } @@ -200,7 +201,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function validateRequest(uint256 requestId) external onlyAdmin { - require(_exists(requestId), "Request does not exist22"); + require(_exists(requestId), "Request does not exist"); require(!_requests[requestId].isValid, "Request is valid"); _requests[requestId].isValid = true; diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 1c8b326c6..a13d3f406 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -556,6 +556,6 @@ contract WithdrawRequestNFTTest is TestSetup { // Owner can seize the invalidated request NFT vm.prank(withdrawRequestNFTInstance.owner()); - withdrawRequestNFTInstance.seizeRequest(requestId, admin); + withdrawRequestNFTInstance.seizeInvalidRequest(requestId, admin); } } From 5069c14ed7a59cae801e7121a89d364945b3ff67 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Tue, 31 Dec 2024 08:39:26 +0900 Subject: [PATCH 19/42] apply CEI pattern to 'handleRemainder' --- src/WithdrawRequestNFT.sol | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 27207a294..0ac30749b 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -242,15 +242,16 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint256 eEthShares = liquidityPool.sharesForWithdrawalAmount(_eEthAmount); uint256 eEthSharesToTreasury = eEthShares.mulDiv(shareRemainderSplitToTreasuryInBps, BASIS_POINT_SCALE); - uint256 eEthAmountToTreasury = liquidityPool.amountForShare(eEthSharesToTreasury); - eETH.transfer(treasury, eEthAmountToTreasury); - uint256 eEthSharesToBurn = eEthShares - eEthSharesToTreasury; + uint256 eEthSharesToMoved = eEthSharesToTreasury + eEthSharesToBurn; + + totalLockedEEthShares -= eEthSharesToMoved; + + eETH.transfer(treasury, eEthAmountToTreasury); eETH.burnShares(address(this), eEthSharesToBurn); - uint256 reducedEEthShares = beforeEEthShares - eETH.shares(address(this)); - totalLockedEEthShares -= reducedEEthShares; + require (beforeEEthShares - eEthSharesToMoved == eETH.shares(address(this)), "Invalid eETH shares after remainder handling"); emit HandledRemainderOfClaimedWithdrawRequests(eEthAmountToTreasury, liquidityPool.amountForShare(eEthSharesToBurn)); } From 2e132028be4e8aa3b31fe1c12b277457f1c02b79 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Tue, 31 Dec 2024 10:04:58 +0900 Subject: [PATCH 20/42] apply CEI pattern to 'redeem' --- src/EtherFiRedemptionManager.sol | 46 +++++++++++++++++--------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index a44a2eaa3..6497c15a2 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -86,12 +86,11 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable require(eEthAmount <= eEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); - uint256 beforeEEthAmount = eEth.balanceOf(address(this)); + (uint256 eEthShares, uint256 eEthAmountToReceiver, uint256 eEthFeeAmountToTreasury, uint256 sharesToBurn, uint256 feeShareToTreasury) = _calcRedemption(eEthAmount); + IERC20(address(eEth)).safeTransferFrom(owner, address(this), eEthAmount); - uint256 afterEEthAmount = eEth.balanceOf(address(this)); - uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; - _redeem(transferredEEthAmount, receiver); + _redeem(eEthAmount, eEthShares, receiver, eEthAmountToReceiver, eEthFeeAmountToTreasury, sharesToBurn, feeShareToTreasury); } /** @@ -101,18 +100,16 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable * @param owner The address of the owner of the weETH. */ function redeemWeEth(uint256 weEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { - uint256 eEthShares = weEthAmount; - uint256 eEthAmount = liquidityPool.amountForShare(eEthShares); + uint256 eEthAmount = weEth.getEETHByWeETH(weEthAmount); require(weEthAmount <= weEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); - uint256 beforeEEthAmount = eEth.balanceOf(address(this)); + (uint256 eEthShares, uint256 eEthAmountToReceiver, uint256 eEthFeeAmountToTreasury, uint256 sharesToBurn, uint256 feeShareToTreasury) = _calcRedemption(eEthAmount); + IERC20(address(weEth)).safeTransferFrom(owner, address(this), weEthAmount); weEth.unwrap(weEthAmount); - uint256 afterEEthAmount = eEth.balanceOf(address(this)); - uint256 transferredEEthAmount = afterEEthAmount - beforeEEthAmount; - _redeem(transferredEEthAmount, receiver); + _redeem(eEthAmount, eEthShares, receiver, eEthAmountToReceiver, eEthFeeAmountToTreasury, sharesToBurn, feeShareToTreasury); } @@ -121,23 +118,19 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable * @param ethAmount The amount of ETH to redeem after the exit fee. * @param receiver The address to receive the redeemed ETH. */ - function _redeem(uint256 ethAmount, address receiver) internal { + function _redeem(uint256 ethAmount, uint256 eEthShares, address receiver, uint256 eEthAmountToReceiver, uint256 eEthFeeAmountToTreasury, uint256 sharesToBurn, uint256 feeShareToTreasury) internal { _updateRateLimit(ethAmount); - uint256 ethShares = liquidityPool.sharesForAmount(ethAmount); - uint256 eEthAmountToReceiver = liquidityPool.amountForShare(ethShares.mulDiv(BASIS_POINT_SCALE - exitFeeInBps, BASIS_POINT_SCALE)); // ethShareToReceiver + // Derive additionals + uint256 eEthShareFee = eEthShares - sharesToBurn; + uint256 feeShareToStakers = eEthShareFee - feeShareToTreasury; + // Snapshot balances & shares for sanity check at the end + uint256 prevBalance = address(this).balance; uint256 prevLpBalance = address(liquidityPool).balance; - uint256 sharesToBurn = liquidityPool.sharesForWithdrawalAmount(eEthAmountToReceiver); - - uint256 ethShareFee = ethShares - sharesToBurn; - uint256 feeShareToTreasury = ethShareFee.mulDiv(exitFeeSplitToTreasuryInBps, BASIS_POINT_SCALE); - uint256 eEthFeeAmountToTreasury = liquidityPool.amountForShare(feeShareToTreasury); - uint256 feeShareToStakers = ethShareFee - feeShareToTreasury; + uint256 totalEEthShare = eEth.totalShares(); // Withdraw ETH from the liquidity pool - uint256 totalEEthShare = eEth.totalShares(); - uint256 prevBalance = address(this).balance; assert (liquidityPool.withdraw(address(this), eEthAmountToReceiver) == sharesToBurn); uint256 ethReceived = address(this).balance - prevBalance; @@ -252,6 +245,17 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable return bucketUnit * BUCKET_UNIT_SCALE; } + + function _calcRedemption(uint256 ethAmount) internal view returns (uint256 eEthShares, uint256 eEthAmountToReceiver, uint256 eEthFeeAmountToTreasury, uint256 sharesToBurn, uint256 feeShareToTreasury) { + eEthShares = liquidityPool.sharesForAmount(ethAmount); + eEthAmountToReceiver = liquidityPool.amountForShare(eEthShares.mulDiv(BASIS_POINT_SCALE - exitFeeInBps, BASIS_POINT_SCALE)); // ethShareToReceiver + + sharesToBurn = liquidityPool.sharesForWithdrawalAmount(eEthAmountToReceiver); + uint256 eEthShareFee = eEthShares - sharesToBurn; + feeShareToTreasury = eEthShareFee.mulDiv(exitFeeSplitToTreasuryInBps, BASIS_POINT_SCALE); + eEthFeeAmountToTreasury = liquidityPool.amountForShare(feeShareToTreasury); + } + /** * @dev Preview taking an exit fee on redeem. See {IERC4626-previewRedeem}. */ From b18bd18b77dbab102d6d49ad6d92c3a6121c2a64 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Wed, 1 Jan 2025 07:29:04 +0900 Subject: [PATCH 21/42] use 'totalRemainderEEthShares' instead of locked share --- src/WithdrawRequestNFT.sol | 33 +++++++++++++++++++-------------- test/WithdrawRequestNFT.t.sol | 18 +++++++----------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 0ac30749b..540d2151e 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -33,8 +33,9 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad // inclusive uint32 private _currentRequestIdToScanFromForShareRemainder; uint32 private _lastRequestIdToScanUntilForShareRemainder; + uint256 public _aggregateSumOfEEthShare; - uint256 public totalLockedEEthShares; + uint256 public totalRemainderEEthShares; bool public paused; address public pauser; @@ -94,7 +95,6 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint32 feeGwei = uint32(fee / 1 gwei); _requests[requestId] = IWithdrawRequestNFT.WithdrawRequest(amountOfEEth, shareOfEEth, true, feeGwei); - totalLockedEEthShares += shareOfEEth; _safeMint(recipient, requestId); @@ -137,7 +137,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad delete _requests[tokenId]; // update accounting - totalLockedEEthShares -= shareAmountToBurnForWithdrawal; + totalRemainderEEthShares += request.shareOfEEth - shareAmountToBurnForWithdrawal; uint256 amountBurnedShare = liquidityPool.withdraw(recipient, amountToWithdraw); assert (amountBurnedShare == shareAmountToBurnForWithdrawal); @@ -160,10 +160,17 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad for (uint256 i = scanFrom; i <= scanUntil; i++) { if (!_exists(i)) continue; - totalLockedEEthShares += _requests[i].shareOfEEth; + _aggregateSumOfEEthShare += _requests[i].shareOfEEth; } _currentRequestIdToScanFromForShareRemainder = uint32(scanUntil + 1); + + // When the scan is completed, update the `totalRemainderEEthShares` and reset the `_aggregateSumOfEEthShare` + if (_currentRequestIdToScanFromForShareRemainder == _lastRequestIdToScanUntilForShareRemainder + 1) { + require(_currentRequestIdToScanFromForShareRemainder == nextRequestId, "new req has been created"); + totalRemainderEEthShares = eETH.shares(address(this)) - _aggregateSumOfEEthShare; + _aggregateSumOfEEthShare = 0; // gone + } } // Seize the request simply by transferring it to another recipient @@ -235,18 +242,17 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad /// - Burn: the rest of the remainder is burned /// @param _eEthAmount: the remainder of the eEth amount function handleRemainder(uint256 _eEthAmount) external onlyAdmin { - require(getEEthRemainderAmount() >= _eEthAmount, "Not enough eETH remainder"); require(_currentRequestIdToScanFromForShareRemainder == _lastRequestIdToScanUntilForShareRemainder + 1, "Not all prev requests have been scanned"); + require(getEEthRemainderAmount() >= _eEthAmount, "Not enough eETH remainder"); uint256 beforeEEthShares = eETH.shares(address(this)); - - uint256 eEthShares = liquidityPool.sharesForWithdrawalAmount(_eEthAmount); - uint256 eEthSharesToTreasury = eEthShares.mulDiv(shareRemainderSplitToTreasuryInBps, BASIS_POINT_SCALE); - uint256 eEthAmountToTreasury = liquidityPool.amountForShare(eEthSharesToTreasury); - uint256 eEthSharesToBurn = eEthShares - eEthSharesToTreasury; - uint256 eEthSharesToMoved = eEthSharesToTreasury + eEthSharesToBurn; - totalLockedEEthShares -= eEthSharesToMoved; + uint256 eEthAmountToTreasury = _eEthAmount.mulDiv(shareRemainderSplitToTreasuryInBps, BASIS_POINT_SCALE); + uint256 eEthAmountToBurn = _eEthAmount - eEthAmountToTreasury; + uint256 eEthSharesToBurn = liquidityPool.sharesForAmount(eEthAmountToBurn); + uint256 eEthSharesToMoved = eEthSharesToBurn + liquidityPool.sharesForAmount(eEthAmountToTreasury); + + totalRemainderEEthShares -= eEthSharesToMoved; eETH.transfer(treasury, eEthAmountToTreasury); eETH.burnShares(address(this), eEthSharesToBurn); @@ -257,8 +263,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function getEEthRemainderAmount() public view returns (uint256) { - uint256 eEthRemainderShare = eETH.shares(address(this)) - totalLockedEEthShares; - return liquidityPool.amountForShare(eEthRemainderShare); + return liquidityPool.amountForShare(totalRemainderEEthShares); } // the withdraw request NFT is transferrable diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index a13d3f406..da8874b71 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -417,6 +417,8 @@ contract WithdrawRequestNFTTest is TestSetup { vm.assume(remainderSplitBps <= 10000); vm.assume(recipient != address(0) && recipient != address(liquidityPoolInstance)); + withdrawRequestNFTInstance.aggregateSumEEthShareAmount(10); + // Setup initial balance for recipient vm.deal(recipient, depositAmount); @@ -431,7 +433,6 @@ contract WithdrawRequestNFTTest is TestSetup { // Record initial balances uint256 treasuryEEthBefore = eETHInstance.balanceOf(address(treasuryInstance)); uint256 recipientBalanceBefore = address(recipient).balance; - uint256 initialTotalLockedEEthShares = withdrawRequestNFTInstance.totalLockedEEthShares(); // Request withdraw eETHInstance.approve(address(liquidityPoolInstance), withdrawAmount); @@ -441,8 +442,6 @@ contract WithdrawRequestNFTTest is TestSetup { // Get initial request state WithdrawRequestNFT.WithdrawRequest memory request = withdrawRequestNFTInstance.getRequest(requestId); - assertEq(withdrawRequestNFTInstance.totalLockedEEthShares(), initialTotalLockedEEthShares + request.shareOfEEth, "Incorrect total locked shares"); - // Simulate rebase after request but before claim vm.prank(address(membershipManagerInstance)); liquidityPoolInstance.rebase(int128(uint128(rebaseAmount))); @@ -497,20 +496,17 @@ contract WithdrawRequestNFTTest is TestSetup { vm.expectRevert("ERC721: invalid token ID"); withdrawRequestNFTInstance.ownerOf(requestId); - // Calculate and verify remainder splitting - assertApproxEqAbs( - expectedLockedShares, - withdrawRequestNFTInstance.totalLockedEEthShares(), - 1e1, - "Incorrect locked eETH share" - ); - // Verify recipient received correct ETH amount assertEq( address(recipient).balance, recipientBalanceBefore + expectedWithdrawAmount, "Recipient should receive correct ETH amount" ); + + uint256 expectedLockedEEthAmount = liquidityPoolInstance.amountForShare(expectedLockedShares); + withdrawRequestNFTInstance.getEEthRemainderAmount(); + vm.prank(admin); + withdrawRequestNFTInstance.handleRemainder(expectedLockedEEthAmount); } function testFuzz_InvalidateRequest(uint96 depositAmount, uint96 withdrawAmount, address recipient) public { From 1e126738b703d96535abe67da7c221ca6c85757e Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Wed, 1 Jan 2025 07:30:36 +0900 Subject: [PATCH 22/42] initializeOnUpgrade cant be called twice --- src/WithdrawRequestNFT.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 540d2151e..3ee26dce1 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -72,7 +72,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function initializeOnUpgrade(address _pauser, uint16 _shareRemainderSplitToTreasuryInBps) external onlyOwner { - require(pauser == address(0), "Already initialized"); + require(_currentRequestIdToScanFromForShareRemainder == 0, "Already initialized"); paused = false; pauser = _pauser; From c14c348fb7d5e271b3c5aad5b9a01fc10ff6ef55 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Wed, 1 Jan 2025 07:44:45 +0900 Subject: [PATCH 23/42] initializeOnUpgrade onlyOnce --- src/WithdrawRequestNFT.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 3ee26dce1..f97f65c13 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -72,7 +72,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function initializeOnUpgrade(address _pauser, uint16 _shareRemainderSplitToTreasuryInBps) external onlyOwner { - require(_currentRequestIdToScanFromForShareRemainder == 0, "Already initialized"); + require(pauser == address(0) && _pauser != address(0), "Already initialized"); paused = false; pauser = _pauser; From 35fba66bdd28ea08a351c44963b41818a960edb6 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Wed, 1 Jan 2025 07:48:15 +0900 Subject: [PATCH 24/42] use uint256 instead of uint32 --- src/WithdrawRequestNFT.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index f97f65c13..3f46010bb 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -31,8 +31,8 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint16 public shareRemainderSplitToTreasuryInBps; // inclusive - uint32 private _currentRequestIdToScanFromForShareRemainder; - uint32 private _lastRequestIdToScanUntilForShareRemainder; + uint256 private _currentRequestIdToScanFromForShareRemainder; + uint256 private _lastRequestIdToScanUntilForShareRemainder; uint256 public _aggregateSumOfEEthShare; uint256 public totalRemainderEEthShares; @@ -163,7 +163,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad _aggregateSumOfEEthShare += _requests[i].shareOfEEth; } - _currentRequestIdToScanFromForShareRemainder = uint32(scanUntil + 1); + _currentRequestIdToScanFromForShareRemainder = scanUntil + 1; // When the scan is completed, update the `totalRemainderEEthShares` and reset the `_aggregateSumOfEEthShare` if (_currentRequestIdToScanFromForShareRemainder == _lastRequestIdToScanUntilForShareRemainder + 1) { From e95cfc023c6589c205a3ca489b75f285eb610a8d Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Wed, 1 Jan 2025 07:50:14 +0900 Subject: [PATCH 25/42] revert --- src/WithdrawRequestNFT.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 3f46010bb..f97f65c13 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -31,8 +31,8 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint16 public shareRemainderSplitToTreasuryInBps; // inclusive - uint256 private _currentRequestIdToScanFromForShareRemainder; - uint256 private _lastRequestIdToScanUntilForShareRemainder; + uint32 private _currentRequestIdToScanFromForShareRemainder; + uint32 private _lastRequestIdToScanUntilForShareRemainder; uint256 public _aggregateSumOfEEthShare; uint256 public totalRemainderEEthShares; @@ -163,7 +163,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad _aggregateSumOfEEthShare += _requests[i].shareOfEEth; } - _currentRequestIdToScanFromForShareRemainder = scanUntil + 1; + _currentRequestIdToScanFromForShareRemainder = uint32(scanUntil + 1); // When the scan is completed, update the `totalRemainderEEthShares` and reset the `_aggregateSumOfEEthShare` if (_currentRequestIdToScanFromForShareRemainder == _lastRequestIdToScanUntilForShareRemainder + 1) { From bbf2d83a568832f97b332220f898a4899ba28400 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Wed, 1 Jan 2025 08:10:18 +0900 Subject: [PATCH 26/42] improve the fuzz test --- test/WithdrawRequestNFT.t.sol | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index da8874b71..5d354b7da 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -449,8 +449,8 @@ contract WithdrawRequestNFTTest is TestSetup { // Calculate expected withdrawal amounts after rebase uint256 sharesValue = liquidityPoolInstance.amountForShare(request.shareOfEEth); uint256 expectedWithdrawAmount = withdrawAmount < sharesValue ? withdrawAmount : sharesValue; - uint256 expectedBurnedShares = liquidityPoolInstance.sharesForWithdrawalAmount(expectedWithdrawAmount); - uint256 expectedLockedShares = request.shareOfEEth - expectedBurnedShares; + uint256 expectedBurnedShares = liquidityPoolInstance.sharesForAmount(expectedWithdrawAmount); + uint256 expectedDustShares = request.shareOfEEth - expectedBurnedShares; // Track initial shares and total supply uint256 initialTotalShares = eETHInstance.totalShares(); @@ -503,10 +503,17 @@ contract WithdrawRequestNFTTest is TestSetup { "Recipient should receive correct ETH amount" ); - uint256 expectedLockedEEthAmount = liquidityPoolInstance.amountForShare(expectedLockedShares); - withdrawRequestNFTInstance.getEEthRemainderAmount(); - vm.prank(admin); - withdrawRequestNFTInstance.handleRemainder(expectedLockedEEthAmount); + assertApproxEqAbs( + withdrawRequestNFTInstance.totalRemainderEEthShares(), + expectedDustShares, + 1, + "Incorrect remainder shares" + ); + + uint256 dustEEthAmount = withdrawRequestNFTInstance.getEEthRemainderAmount(); + vm.startPrank(admin); + withdrawRequestNFTInstance.handleRemainder(dustEEthAmount / 2); + withdrawRequestNFTInstance.handleRemainder(dustEEthAmount / 2); } function testFuzz_InvalidateRequest(uint96 depositAmount, uint96 withdrawAmount, address recipient) public { From 2cbbc047354a0280d0719a1d76b82bf60a52f967 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Wed, 1 Jan 2025 19:51:10 +0900 Subject: [PATCH 27/42] (1) pause the contract on upgrade, (2) prevent from calling 'aggregateSumEEthShareAmount' again after the scan is completed, (3) prevent from undoing the finalization 'finalizeRequests' --- src/WithdrawRequestNFT.sol | 7 +++++-- test/WithdrawRequestNFT.t.sol | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index f97f65c13..759183ac4 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -73,8 +73,9 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad function initializeOnUpgrade(address _pauser, uint16 _shareRemainderSplitToTreasuryInBps) external onlyOwner { require(pauser == address(0) && _pauser != address(0), "Already initialized"); + require(_shareRemainderSplitToTreasuryInBps <= BASIS_POINT_SCALE, "INVALID"); - paused = false; + paused = true; // make sure the contract is paused after the upgrade pauser = _pauser; shareRemainderSplitToTreasuryInBps = _shareRemainderSplitToTreasuryInBps; @@ -154,6 +155,8 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad // This function is used to aggregate the sum of the eEth shares of the requests that have not been claimed yet. // To be triggered during the upgrade to the new version of the contract. function aggregateSumEEthShareAmount(uint256 _numReqsToScan) external { + require(_currentRequestIdToScanFromForShareRemainder != _lastRequestIdToScanUntilForShareRemainder + 1, "scan is completed"); + // [scanFrom, scanUntil] uint256 scanFrom = _currentRequestIdToScanFromForShareRemainder; uint256 scanUntil = Math.min(_lastRequestIdToScanUntilForShareRemainder, scanFrom + _numReqsToScan - 1); @@ -167,7 +170,6 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad // When the scan is completed, update the `totalRemainderEEthShares` and reset the `_aggregateSumOfEEthShare` if (_currentRequestIdToScanFromForShareRemainder == _lastRequestIdToScanUntilForShareRemainder + 1) { - require(_currentRequestIdToScanFromForShareRemainder == nextRequestId, "new req has been created"); totalRemainderEEthShares = eETH.shares(address(this)) - _aggregateSumOfEEthShare; _aggregateSumOfEEthShare = 0; // gone } @@ -197,6 +199,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function finalizeRequests(uint256 requestId) external onlyAdmin { + require(requestId > lastFinalizedRequestId, "Cannot undo finalization"); lastFinalizedRequestId = uint32(requestId); } diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 5d354b7da..21f5f31d0 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -419,6 +419,9 @@ contract WithdrawRequestNFTTest is TestSetup { withdrawRequestNFTInstance.aggregateSumEEthShareAmount(10); + vm.expectRevert("scan is completed"); + withdrawRequestNFTInstance.aggregateSumEEthShareAmount(10); + // Setup initial balance for recipient vm.deal(recipient, depositAmount); From 1482cb0a599661a6d7de68f861db801c0ec54799 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 09:29:30 +0900 Subject: [PATCH 28/42] only owner of the funds can call {redeemEEth, redeemWeEth} --- src/EtherFiRedemptionManager.sol | 14 ++++++-------- test/EtherFiRedemptionManager.t.sol | 16 ++++++++-------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index 6497c15a2..c5449096b 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -80,15 +80,14 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable * @notice Redeems eETH for ETH. * @param eEthAmount The amount of eETH to redeem after the exit fee. * @param receiver The address to receive the redeemed ETH. - * @param owner The address of the owner of the eETH. */ - function redeemEEth(uint256 eEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { - require(eEthAmount <= eEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); + function redeemEEth(uint256 eEthAmount, address receiver) public whenNotPaused nonReentrant { + require(eEthAmount <= eEth.balanceOf(msg.sender), "EtherFiRedemptionManager: Insufficient balance"); require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); (uint256 eEthShares, uint256 eEthAmountToReceiver, uint256 eEthFeeAmountToTreasury, uint256 sharesToBurn, uint256 feeShareToTreasury) = _calcRedemption(eEthAmount); - IERC20(address(eEth)).safeTransferFrom(owner, address(this), eEthAmount); + IERC20(address(eEth)).safeTransferFrom(msg.sender, address(this), eEthAmount); _redeem(eEthAmount, eEthShares, receiver, eEthAmountToReceiver, eEthFeeAmountToTreasury, sharesToBurn, feeShareToTreasury); } @@ -97,16 +96,15 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable * @notice Redeems weETH for ETH. * @param weEthAmount The amount of weETH to redeem after the exit fee. * @param receiver The address to receive the redeemed ETH. - * @param owner The address of the owner of the weETH. */ - function redeemWeEth(uint256 weEthAmount, address receiver, address owner) public whenNotPaused nonReentrant { + function redeemWeEth(uint256 weEthAmount, address receiver) public whenNotPaused nonReentrant { uint256 eEthAmount = weEth.getEETHByWeETH(weEthAmount); - require(weEthAmount <= weEth.balanceOf(owner), "EtherFiRedemptionManager: Insufficient balance"); + require(weEthAmount <= weEth.balanceOf(msg.sender), "EtherFiRedemptionManager: Insufficient balance"); require(canRedeem(eEthAmount), "EtherFiRedemptionManager: Exceeded total redeemable amount"); (uint256 eEthShares, uint256 eEthAmountToReceiver, uint256 eEthFeeAmountToTreasury, uint256 sharesToBurn, uint256 feeShareToTreasury) = _calcRedemption(eEthAmount); - IERC20(address(weEth)).safeTransferFrom(owner, address(this), weEthAmount); + IERC20(address(weEth)).safeTransferFrom(msg.sender, address(this), weEthAmount); weEth.unwrap(weEthAmount); _redeem(eEthAmount, eEthShares, receiver, eEthAmountToReceiver, eEthFeeAmountToTreasury, sharesToBurn, feeShareToTreasury); diff --git a/test/EtherFiRedemptionManager.t.sol b/test/EtherFiRedemptionManager.t.sol index f9f8da8e6..6979fdf9c 100644 --- a/test/EtherFiRedemptionManager.t.sol +++ b/test/EtherFiRedemptionManager.t.sol @@ -99,7 +99,7 @@ contract EtherFiRedemptionManagerTest is TestSetup { uint256 treasuryBalanceBefore = eETHInstance.balanceOf(address(treasuryInstance)); eETHInstance.approve(address(etherFiRedemptionManagerInstance), redeemAmount); - etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user, user); + etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user); uint256 totalFee = (redeemAmount * exitFeeBps) / 10000; uint256 treasuryFee = (totalFee * exitFeeSplitBps) / 10000; @@ -118,7 +118,7 @@ contract EtherFiRedemptionManagerTest is TestSetup { } else { vm.expectRevert(); - etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user, user); + etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user); } vm.stopPrank(); } @@ -172,7 +172,7 @@ contract EtherFiRedemptionManagerTest is TestSetup { uint256 eEthAmount = liquidityPoolInstance.amountForShare(weEthAmount); weEthInstance.approve(address(etherFiRedemptionManagerInstance), weEthAmount); - etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user, user); + etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user); uint256 totalFee = (eEthAmount * exitFeeBps) / 10000; uint256 treasuryFee = (totalFee * exitFeeSplitBps) / 10000; @@ -191,7 +191,7 @@ contract EtherFiRedemptionManagerTest is TestSetup { } else { vm.expectRevert(); - etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user, user); + etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user); } vm.stopPrank(); } @@ -285,7 +285,7 @@ contract EtherFiRedemptionManagerTest is TestSetup { uint256 treasuryBalance = eETHInstance.balanceOf(address(etherFiRedemptionManagerInstance.treasury())); eETHInstance.approve(address(etherFiRedemptionManagerInstance), 1 ether); - etherFiRedemptionManagerInstance.redeemEEth(1 ether, user, user); + etherFiRedemptionManagerInstance.redeemEEth(1 ether, user); uint256 totalFee = (1 ether * 1e2) / 1e4; uint256 treasuryFee = (totalFee * 1e3) / 1e4; @@ -296,7 +296,7 @@ contract EtherFiRedemptionManagerTest is TestSetup { eETHInstance.approve(address(etherFiRedemptionManagerInstance), 5 ether); vm.expectRevert("EtherFiRedemptionManager: Exceeded total redeemable amount"); - etherFiRedemptionManagerInstance.redeemEEth(5 ether, user, user); + etherFiRedemptionManagerInstance.redeemEEth(5 ether, user); vm.stopPrank(); } @@ -327,7 +327,7 @@ contract EtherFiRedemptionManagerTest is TestSetup { uint256 userBalance = address(user).balance; uint256 treasuryBalance = eETHInstance.balanceOf(address(treasuryInstance)); weEthInstance.approve(address(etherFiRedemptionManagerInstance), 1 ether); - etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user, user); + etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user); uint256 totalFee = (eEthAmount * 1e2) / 1e4; uint256 treasuryFee = (totalFee * 1e3) / 1e4; @@ -360,7 +360,7 @@ contract EtherFiRedemptionManagerTest is TestSetup { eETHInstance.approve(address(etherFiRedemptionManagerInstance), redeemAmount); vm.expectRevert("EtherFiRedemptionManager: Exceeded total redeemable amount"); - etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user, user); + etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user); vm.stopPrank(); } From 8ced81ecd1d4ff1a0ace4ac3cf18c7a882f5398d Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 10:13:32 +0900 Subject: [PATCH 29/42] disable unpause until the scan is completed --- src/WithdrawRequestNFT.sol | 36 ++++++++++-------- test/EtherFiRedemptionManager.t.sol | 12 ++++++ test/WithdrawRequestNFT.t.sol | 59 +++++++++++++++++++++-------- 3 files changed, 77 insertions(+), 30 deletions(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 759183ac4..fd7fbb6a2 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -31,9 +31,9 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint16 public shareRemainderSplitToTreasuryInBps; // inclusive - uint32 private _currentRequestIdToScanFromForShareRemainder; - uint32 private _lastRequestIdToScanUntilForShareRemainder; - uint256 public _aggregateSumOfEEthShare; + uint32 public currentRequestIdToScanFromForShareRemainder; + uint32 public lastRequestIdToScanUntilForShareRemainder; + uint256 public aggregateSumOfEEthShare; uint256 public totalRemainderEEthShares; @@ -80,8 +80,8 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad shareRemainderSplitToTreasuryInBps = _shareRemainderSplitToTreasuryInBps; - _currentRequestIdToScanFromForShareRemainder = 1; - _lastRequestIdToScanUntilForShareRemainder = nextRequestId - 1; + currentRequestIdToScanFromForShareRemainder = 1; + lastRequestIdToScanUntilForShareRemainder = nextRequestId - 1; } /// @notice creates a withdraw request and issues an associated NFT to the recipient @@ -155,23 +155,23 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad // This function is used to aggregate the sum of the eEth shares of the requests that have not been claimed yet. // To be triggered during the upgrade to the new version of the contract. function aggregateSumEEthShareAmount(uint256 _numReqsToScan) external { - require(_currentRequestIdToScanFromForShareRemainder != _lastRequestIdToScanUntilForShareRemainder + 1, "scan is completed"); + require(!isScanOfShareRemainderCompleted(), "scan is completed"); // [scanFrom, scanUntil] - uint256 scanFrom = _currentRequestIdToScanFromForShareRemainder; - uint256 scanUntil = Math.min(_lastRequestIdToScanUntilForShareRemainder, scanFrom + _numReqsToScan - 1); + uint256 scanFrom = currentRequestIdToScanFromForShareRemainder; + uint256 scanUntil = Math.min(lastRequestIdToScanUntilForShareRemainder, scanFrom + _numReqsToScan - 1); for (uint256 i = scanFrom; i <= scanUntil; i++) { if (!_exists(i)) continue; - _aggregateSumOfEEthShare += _requests[i].shareOfEEth; + aggregateSumOfEEthShare += _requests[i].shareOfEEth; } - _currentRequestIdToScanFromForShareRemainder = uint32(scanUntil + 1); + currentRequestIdToScanFromForShareRemainder = uint32(scanUntil + 1); - // When the scan is completed, update the `totalRemainderEEthShares` and reset the `_aggregateSumOfEEthShare` - if (_currentRequestIdToScanFromForShareRemainder == _lastRequestIdToScanUntilForShareRemainder + 1) { - totalRemainderEEthShares = eETH.shares(address(this)) - _aggregateSumOfEEthShare; - _aggregateSumOfEEthShare = 0; // gone + // When the scan is completed, update the `totalRemainderEEthShares` and reset the `aggregateSumOfEEthShare` + if (isScanOfShareRemainderCompleted()) { + totalRemainderEEthShares = eETH.shares(address(this)) - aggregateSumOfEEthShare; + aggregateSumOfEEthShare = 0; // gone } } @@ -234,6 +234,8 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function unPauseContract() external onlyAdmin { + require(isScanOfShareRemainderCompleted(), "scan is not completed"); + paused = false; emit Unpaused(msg.sender); } @@ -245,7 +247,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad /// - Burn: the rest of the remainder is burned /// @param _eEthAmount: the remainder of the eEth amount function handleRemainder(uint256 _eEthAmount) external onlyAdmin { - require(_currentRequestIdToScanFromForShareRemainder == _lastRequestIdToScanUntilForShareRemainder + 1, "Not all prev requests have been scanned"); + require(currentRequestIdToScanFromForShareRemainder == lastRequestIdToScanUntilForShareRemainder + 1, "Not all prev requests have been scanned"); require(getEEthRemainderAmount() >= _eEthAmount, "Not enough eETH remainder"); uint256 beforeEEthShares = eETH.shares(address(this)); @@ -269,6 +271,10 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad return liquidityPool.amountForShare(totalRemainderEEthShares); } + function isScanOfShareRemainderCompleted() public view returns (bool) { + return currentRequestIdToScanFromForShareRemainder == (lastRequestIdToScanUntilForShareRemainder + 1); + } + // the withdraw request NFT is transferrable // - if the request is valid, it can be transferred by the owner of the NFT // - if the request is invalid, it can be transferred only by the owner of the WithdarwRequestNFT contract diff --git a/test/EtherFiRedemptionManager.t.sol b/test/EtherFiRedemptionManager.t.sol index 6979fdf9c..cd3f3cc10 100644 --- a/test/EtherFiRedemptionManager.t.sol +++ b/test/EtherFiRedemptionManager.t.sol @@ -30,6 +30,18 @@ contract EtherFiRedemptionManagerTest is TestSetup { liquidityPoolInstance.initializeOnUpgradeWithRedemptionManager(address(etherFiRedemptionManagerInstance)); } + function test_upgrade_only_by_owner() public { + setUp_Fork(); + + address impl = etherFiRedemptionManagerInstance.getImplementation(); + vm.prank(admin); + vm.expectRevert(); + etherFiRedemptionManagerInstance.upgradeTo(impl); + + vm.prank(etherFiRedemptionManagerInstance.owner()); + etherFiRedemptionManagerInstance.upgradeTo(impl); + } + function test_rate_limit() public { vm.deal(user, 1000 ether); vm.prank(user); diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 21f5f31d0..465dc4215 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -6,14 +6,35 @@ pragma solidity ^0.8.13; import "forge-std/console2.sol"; import "./TestSetup.sol"; + +contract WithdrawRequestNFTIntrusive is WithdrawRequestNFT { + + constructor() WithdrawRequestNFT(address(0)) {} + + function updateParam(uint32 _currentRequestIdToScanFromForShareRemainder, uint32 _lastRequestIdToScanUntilForShareRemainder) external { + currentRequestIdToScanFromForShareRemainder = _currentRequestIdToScanFromForShareRemainder; + lastRequestIdToScanUntilForShareRemainder = _lastRequestIdToScanUntilForShareRemainder; + } + +} + contract WithdrawRequestNFTTest is TestSetup { uint32[] public reqIds =[ 20, 388, 478, 714, 726, 729, 735, 815, 861, 916, 941, 1014, 1067, 1154, 1194, 1253]; + address etherfi_admin_wallet = 0x2aCA71020De61bb532008049e1Bd41E451aE8AdC; function setUp() public { setUpTests(); } + function updateParam(uint32 _currentRequestIdToScanFromForShareRemainder, uint32 _lastRequestIdToScanUntilForShareRemainder) internal { + address cur_impl = withdrawRequestNFTInstance.getImplementation(); + address new_impl = address(new WithdrawRequestNFTIntrusive()); + withdrawRequestNFTInstance.upgradeTo(new_impl); + WithdrawRequestNFTIntrusive(address(withdrawRequestNFTInstance)).updateParam(_currentRequestIdToScanFromForShareRemainder, _lastRequestIdToScanUntilForShareRemainder); + withdrawRequestNFTInstance.upgradeTo(cur_impl); + } + function test_finalizeRequests() public { startHoax(bob); liquidityPoolInstance.deposit{value: 10 ether}(); @@ -322,42 +343,50 @@ contract WithdrawRequestNFTTest is TestSetup { function test_aggregateSumEEthShareAmount() public { initializeRealisticFork(MAINNET_FORK); - address etherfi_admin_wallet = 0x2aCA71020De61bb532008049e1Bd41E451aE8AdC; - vm.startPrank(withdrawRequestNFTInstance.owner()); // 1. Upgrade withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(address(owner)))); withdrawRequestNFTInstance.initializeOnUpgrade(etherfi_admin_wallet, 50_00); withdrawRequestNFTInstance.updateAdmin(etherfi_admin_wallet, true); - // 2. PAUSE - withdrawRequestNFTInstance.pauseContract(); - vm.stopPrank(); + // 2. (For test) update the scan range + updateParam(1, 200); - vm.startPrank(etherfi_admin_wallet); + // 2. Confirm Paused & Can't Unpause + assertTrue(withdrawRequestNFTInstance.paused(), "Contract should be paused"); + vm.expectRevert("scan is not completed"); + withdrawRequestNFTInstance.unPauseContract(); + vm.stopPrank(); // 3. AggSum + // - Can't Unpause untill the scan is not completed + // - Can't aggregateSumEEthShareAmount after the scan is completed withdrawRequestNFTInstance.aggregateSumEEthShareAmount(128); - // ... + assertFalse(withdrawRequestNFTInstance.isScanOfShareRemainderCompleted(), "Scan should be completed"); - vm.stopPrank(); + vm.prank(withdrawRequestNFTInstance.owner()); + vm.expectRevert("scan is not completed"); + withdrawRequestNFTInstance.unPauseContract(); + + withdrawRequestNFTInstance.aggregateSumEEthShareAmount(128); + assertTrue(withdrawRequestNFTInstance.isScanOfShareRemainderCompleted(), "Scan should be completed"); - // 4. Unpause + vm.expectRevert("scan is completed"); + withdrawRequestNFTInstance.aggregateSumEEthShareAmount(128); + + // 4. Can Unpause vm.startPrank(withdrawRequestNFTInstance.owner()); withdrawRequestNFTInstance.unPauseContract(); vm.stopPrank(); - // Back to normal - vm.prank(withdrawRequestNFTInstance.ownerOf(reqIds[1])); - withdrawRequestNFTInstance.claimWithdraw(reqIds[1]); + // we will run the test on the forked mainnet to perform the full scan and confirm we can unpause } function test_handleRemainder() public { test_aggregateSumEEthShareAmount(); - vm.startPrank(withdrawRequestNFTInstance.owner()); - vm.expectRevert("Not all prev requests have been scanned"); - withdrawRequestNFTInstance.handleRemainder(1 ether); + vm.prank(etherfi_admin_wallet); + withdrawRequestNFTInstance.handleRemainder(0.01 ether); vm.stopPrank(); } From 86ac4a0d8d2dc34f5f3b79cb9c285177262133f8 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 10:13:57 +0900 Subject: [PATCH 30/42] check the basis points params are below 1e4 --- src/EtherFiRedemptionManager.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index c5449096b..404f66e74 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -65,6 +65,10 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable } function initialize(uint16 _exitFeeSplitToTreasuryInBps, uint16 _exitFeeInBps, uint16 _lowWatermarkInBpsOfTvl, uint256 _bucketCapacity, uint256 _bucketRefillRate) external initializer { + require(_exitFeeInBps <= BASIS_POINT_SCALE, "INVALID"); + require(_exitFeeSplitToTreasuryInBps <= BASIS_POINT_SCALE, "INVALID"); + require(_lowWatermarkInBpsOfTvl <= BASIS_POINT_SCALE, "INVALID"); + __Ownable_init(); __UUPSUpgradeable_init(); __Pausable_init(); From 1e8cdeab1cc35a9f3a60ada3dea4f33d692214c8 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 10:16:39 +0900 Subject: [PATCH 31/42] disallow calling 'initializeOnUpgradeWithRedemptionManager' with invalid param & calling it twice --- src/LiquidityPool.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LiquidityPool.sol b/src/LiquidityPool.sol index 910dd05fe..622f80441 100644 --- a/src/LiquidityPool.sol +++ b/src/LiquidityPool.sol @@ -145,7 +145,7 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL } function initializeOnUpgradeWithRedemptionManager(address _etherFiRedemptionManager) external onlyOwner { - require(address(etherFiRedemptionManager) == address(0), "Already initialized"); + require(address(etherFiRedemptionManager) == address(0) && _etherFiRedemptionManager != address(0), "Invalid"); etherFiRedemptionManager = EtherFiRedemptionManager(payable(_etherFiRedemptionManager)); } From 6fffba6d6ba161a7129e136cf58152036ad5e656 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 10:23:15 +0900 Subject: [PATCH 32/42] (1) withdrawRequestNFT cannot call LiquidityPool.addEthAmountLockedForWithdrawal, (2) remove unused function reduceEthAmountLockedForWithdrawal --- src/LiquidityPool.sol | 8 +------- src/interfaces/ILiquidityPool.sol | 1 - 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/LiquidityPool.sol b/src/LiquidityPool.sol index 622f80441..6fc71c1bc 100644 --- a/src/LiquidityPool.sol +++ b/src/LiquidityPool.sol @@ -532,17 +532,11 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL } function addEthAmountLockedForWithdrawal(uint128 _amount) external { - if (!(msg.sender == address(etherFiAdminContract) || msg.sender == address(withdrawRequestNFT))) revert IncorrectCaller(); + if (!(msg.sender == address(etherFiAdminContract))) revert IncorrectCaller(); ethAmountLockedForWithdrawal += _amount; } - function reduceEthAmountLockedForWithdrawal(uint128 _amount) external { - if (msg.sender != address(withdrawRequestNFT)) revert IncorrectCaller(); - - ethAmountLockedForWithdrawal -= _amount; - } - //-------------------------------------------------------------------------------------- //------------------------------ INTERNAL FUNCTIONS ---------------------------------- //-------------------------------------------------------------------------------------- diff --git a/src/interfaces/ILiquidityPool.sol b/src/interfaces/ILiquidityPool.sol index a71b2d6c7..9400955db 100644 --- a/src/interfaces/ILiquidityPool.sol +++ b/src/interfaces/ILiquidityPool.sol @@ -71,7 +71,6 @@ interface ILiquidityPool { function rebase(int128 _accruedRewards) external; function payProtocolFees(uint128 _protocolFees) external; function addEthAmountLockedForWithdrawal(uint128 _amount) external; - function reduceEthAmountLockedForWithdrawal(uint128 _amount) external; function setStakingTargetWeights(uint32 _eEthWeight, uint32 _etherFanWeight) external; function updateAdmin(address _newAdmin, bool _isAdmin) external; From 89db9d5739a12e343e57a10e61f63c3a596b1a25 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 13:59:25 +0900 Subject: [PATCH 33/42] prevent finalizing future requests --- script/specialized/weEth_withdrawal_v2.s.sol | 69 ++++++++++++++++++++ src/WithdrawRequestNFT.sol | 1 + test/WithdrawRequestNFT.t.sol | 4 +- 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 script/specialized/weEth_withdrawal_v2.s.sol diff --git a/script/specialized/weEth_withdrawal_v2.s.sol b/script/specialized/weEth_withdrawal_v2.s.sol new file mode 100644 index 000000000..680028cde --- /dev/null +++ b/script/specialized/weEth_withdrawal_v2.s.sol @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "forge-std/Script.sol"; +import "test/TestSetup.sol"; + +import "src/helpers/AddressProvider.sol"; + +contract Upgrade is Script { + + AddressProvider public addressProvider; + address public addressProviderAddress = 0x8487c5F8550E3C3e7734Fe7DCF77DB2B72E4A848; + address public roleRegistry = 0x1d3Af47C1607A2EF33033693A9989D1d1013BB50; + address public treasury = 0x0c83EAe1FE72c390A02E426572854931EefF93BA; + address public pauser = 0x9AF1298993DC1f397973C62A5D47a284CF76844D; + + WithdrawRequestNFT withdrawRequestNFTInstance; + LiquidityPool liquidityPoolInstance; + + function run() external { + uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY"); + AddressProvider addressProvider = AddressProvider(addressProviderAddress); + + withdrawRequestNFTInstance = WithdrawRequestNFT(payable(addressProvider.getContractAddress("WithdrawRequestNFT"))); + liquidityPoolInstance = LiquidityPool(payable(addressProvider.getContractAddress("LiquidityPool"))); + + vm.startBroadcast(deployerPrivateKey); + + // deploy_upgrade(); + // agg(); + // handle_remainder(); + + vm.stopBroadcast(); + } + + function deploy_upgrade() internal { + UUPSProxy etherFiRedemptionManagerProxy = new UUPSProxy(address(new EtherFiRedemptionManager( + addressProvider.getContractAddress("LiquidityPool"), + addressProvider.getContractAddress("EETH"), + addressProvider.getContractAddress("WeETH"), + treasury, + roleRegistry)), ""); + EtherFiRedemptionManager etherFiRedemptionManagerInstance = EtherFiRedemptionManager(payable(etherFiRedemptionManagerProxy)); + etherFiRedemptionManagerInstance.initialize(10_00, 1_00, 1_00, 5 ether, 0.001 ether); // 10% fee split to treasury, 1% exit fee, 1% low watermark + + withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(treasury))); + withdrawRequestNFTInstance.initializeOnUpgrade(pauser, 50_00); // 50% fee split to treasury + + liquidityPoolInstance.upgradeTo(address(new LiquidityPool())); + liquidityPoolInstance.initializeOnUpgradeWithRedemptionManager(address(etherFiRedemptionManagerInstance)); + } + + function agg() internal { + uint256 numToScanPerTx = 1024; + uint256 cnt = (withdrawRequestNFTInstance.nextRequestId() / numToScanPerTx) + 1; + console.log(cnt); + for (uint256 i = 0; i < cnt; i++) { + withdrawRequestNFTInstance.aggregateSumEEthShareAmount(numToScanPerTx); + } + } + + function handle_remainder() internal { + withdrawRequestNFTInstance.updateAdmin(msg.sender, true); + withdrawRequestNFTInstance.unPauseContract(); + uint256 remainder = withdrawRequestNFTInstance.getEEthRemainderAmount(); + console.log(remainder); + withdrawRequestNFTInstance.handleRemainder(remainder); + } +} \ No newline at end of file diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index fd7fbb6a2..cd03bcc1e 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -200,6 +200,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad function finalizeRequests(uint256 requestId) external onlyAdmin { require(requestId > lastFinalizedRequestId, "Cannot undo finalization"); + require(requestId < nextRequestId, "Cannot finalize future requests"); lastFinalizedRequestId = uint32(requestId); } diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 465dc4215..02954b842 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -343,10 +343,12 @@ contract WithdrawRequestNFTTest is TestSetup { function test_aggregateSumEEthShareAmount() public { initializeRealisticFork(MAINNET_FORK); + address pauser = 0x9af1298993dc1f397973c62a5d47a284cf76844d; + vm.startPrank(withdrawRequestNFTInstance.owner()); // 1. Upgrade withdrawRequestNFTInstance.upgradeTo(address(new WithdrawRequestNFT(address(owner)))); - withdrawRequestNFTInstance.initializeOnUpgrade(etherfi_admin_wallet, 50_00); + withdrawRequestNFTInstance.initializeOnUpgrade(pauser, 50_00); withdrawRequestNFTInstance.updateAdmin(etherfi_admin_wallet, true); // 2. (For test) update the scan range From c5a2dd1a39cff016106f8cf5481576009d3e4992 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 14:19:23 +0900 Subject: [PATCH 34/42] add {redeemEEthWithPermit, redeemWeEthWithPermit}, use try-catch for --- src/EtherFiRedemptionManager.sol | 9 +++++++++ src/LiquidityPool.sol | 2 +- src/interfaces/IWeETH.sol | 9 +++++++++ src/interfaces/IeETH.sol | 9 +++++++++ test/EtherFiRedemptionManager.t.sol | 10 +++++----- test/TestSetup.sol | 28 ++++++++++++++++++++++++++++ test/WithdrawRequestNFT.t.sol | 2 +- 7 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index 404f66e74..5fc5fd8ed 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -114,6 +114,15 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable _redeem(eEthAmount, eEthShares, receiver, eEthAmountToReceiver, eEthFeeAmountToTreasury, sharesToBurn, feeShareToTreasury); } + function redeemEEthWithPermit(uint256 eEthAmount, address receiver, IeETH.PermitInput calldata permit) external { + try eEth.permit(msg.sender, address(this), permit.value, permit.deadline, permit.v, permit.r, permit.s) {} catch {} + redeemEEth(eEthAmount, receiver); + } + + function redeemWeEthWithPermit(uint256 weEthAmount, address receiver, IWeETH.PermitInput calldata permit) external { + try weEth.permit(msg.sender, address(this), permit.value, permit.deadline, permit.v, permit.r, permit.s) {} catch {} + redeemWeEth(weEthAmount, receiver); + } /** * @notice Redeems ETH. diff --git a/src/LiquidityPool.sol b/src/LiquidityPool.sol index 6fc71c1bc..8a95400fa 100644 --- a/src/LiquidityPool.sol +++ b/src/LiquidityPool.sol @@ -235,7 +235,7 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL whenNotPaused returns (uint256) { - eETH.permit(msg.sender, address(this), _permit.value, _permit.deadline, _permit.v, _permit.r, _permit.s); + try eETH.permit(msg.sender, address(this), _permit.value, _permit.deadline, _permit.v, _permit.r, _permit.s) {} catch {} return requestWithdraw(_owner, _amount); } diff --git a/src/interfaces/IWeETH.sol b/src/interfaces/IWeETH.sol index b64d76ca3..4741da079 100644 --- a/src/interfaces/IWeETH.sol +++ b/src/interfaces/IWeETH.sol @@ -6,6 +6,15 @@ import "./ILiquidityPool.sol"; import "./IeETH.sol"; interface IWeETH is IERC20Upgradeable { + + struct PermitInput { + uint256 value; + uint256 deadline; + uint8 v; + bytes32 r; + bytes32 s; + } + // STATE VARIABLES function eETH() external view returns (IeETH); function liquidityPool() external view returns (ILiquidityPool); diff --git a/src/interfaces/IeETH.sol b/src/interfaces/IeETH.sol index 74fc6affa..f8ee974b9 100644 --- a/src/interfaces/IeETH.sol +++ b/src/interfaces/IeETH.sol @@ -2,6 +2,15 @@ pragma solidity ^0.8.13; interface IeETH { + + struct PermitInput { + uint256 value; + uint256 deadline; + uint8 v; + bytes32 r; + bytes32 s; + } + function name() external pure returns (string memory); function symbol() external pure returns (string memory); function decimals() external pure returns (uint8); diff --git a/test/EtherFiRedemptionManager.t.sol b/test/EtherFiRedemptionManager.t.sol index cd3f3cc10..7aa5374d4 100644 --- a/test/EtherFiRedemptionManager.t.sol +++ b/test/EtherFiRedemptionManager.t.sol @@ -109,9 +109,9 @@ contract EtherFiRedemptionManagerTest is TestSetup { if (etherFiRedemptionManagerInstance.canRedeem(redeemAmount)) { uint256 userBalanceBefore = address(user).balance; uint256 treasuryBalanceBefore = eETHInstance.balanceOf(address(treasuryInstance)); - - eETHInstance.approve(address(etherFiRedemptionManagerInstance), redeemAmount); - etherFiRedemptionManagerInstance.redeemEEth(redeemAmount, user); + + IeETH.PermitInput memory permit = eEth_createPermitInput(999, address(etherFiRedemptionManagerInstance), redeemAmount, eETHInstance.nonces(user), 2**256 - 1, eETHInstance.DOMAIN_SEPARATOR()); + etherFiRedemptionManagerInstance.redeemEEthWithPermit(redeemAmount, user, permit); uint256 totalFee = (redeemAmount * exitFeeBps) / 10000; uint256 treasuryFee = (totalFee * exitFeeSplitBps) / 10000; @@ -183,8 +183,8 @@ contract EtherFiRedemptionManagerTest is TestSetup { uint256 eEthAmount = liquidityPoolInstance.amountForShare(weEthAmount); - weEthInstance.approve(address(etherFiRedemptionManagerInstance), weEthAmount); - etherFiRedemptionManagerInstance.redeemWeEth(weEthAmount, user); + IWeETH.PermitInput memory permit = weEth_createPermitInput(999, address(etherFiRedemptionManagerInstance), weEthAmount, weEthInstance.nonces(user), 2**256 - 1, weEthInstance.DOMAIN_SEPARATOR()); + etherFiRedemptionManagerInstance.redeemWeEthWithPermit(weEthAmount, user, permit); uint256 totalFee = (eEthAmount * exitFeeBps) / 10000; uint256 treasuryFee = (totalFee * exitFeeSplitBps) / 10000; diff --git a/test/TestSetup.sol b/test/TestSetup.sol index f27c906b0..4eca2441f 100644 --- a/test/TestSetup.sol +++ b/test/TestSetup.sol @@ -1033,6 +1033,34 @@ contract TestSetup is Test { return permitInput; } + function eEth_createPermitInput(uint256 privKey, address spender, uint256 value, uint256 nonce, uint256 deadline, bytes32 domianSeparator) public returns (IeETH.PermitInput memory) { + address _owner = vm.addr(privKey); + bytes32 digest = calculatePermitDigest(_owner, spender, value, nonce, deadline, domianSeparator); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(privKey, digest); + IeETH.PermitInput memory permitInput = IeETH.PermitInput({ + value: value, + deadline: deadline, + v: v, + r: r, + s: s + }); + return permitInput; + } + + function weEth_createPermitInput(uint256 privKey, address spender, uint256 value, uint256 nonce, uint256 deadline, bytes32 domianSeparator) public returns (IWeETH.PermitInput memory) { + address _owner = vm.addr(privKey); + bytes32 digest = calculatePermitDigest(_owner, spender, value, nonce, deadline, domianSeparator); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(privKey, digest); + IWeETH.PermitInput memory permitInput = IWeETH.PermitInput({ + value: value, + deadline: deadline, + v: v, + r: r, + s: s + }); + return permitInput; + } + function registerAsBnftHolder(address _user) internal { (bool registered, uint32 index) = liquidityPoolInstance.bnftHoldersIndexes(_user); if (!registered) liquidityPoolInstance.registerAsBnftHolder(_user); diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index 02954b842..aeebdcb35 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -343,7 +343,7 @@ contract WithdrawRequestNFTTest is TestSetup { function test_aggregateSumEEthShareAmount() public { initializeRealisticFork(MAINNET_FORK); - address pauser = 0x9af1298993dc1f397973c62a5d47a284cf76844d; + address pauser = 0x9AF1298993DC1f397973C62A5D47a284CF76844D; vm.startPrank(withdrawRequestNFTInstance.owner()); // 1. Upgrade From c85e92028d4eda92273ef38fa40840c1c5b21ff9 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 15:28:16 +0900 Subject: [PATCH 35/42] remove a redundant check --- src/WithdrawRequestNFT.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index cd03bcc1e..96a5213ad 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -104,7 +104,6 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad } function getClaimableAmount(uint256 tokenId) public view returns (uint256) { - require(tokenId < nextRequestId, "Request does not exist"); require(tokenId <= lastFinalizedRequestId, "Request is not finalized"); require(ownerOf(tokenId) != address(0), "Already Claimed"); From cca361ec30d5fbf76cfdc31aac2993b94ece0c75 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 18:10:42 +0900 Subject: [PATCH 36/42] Prevent 'initializeOnUpgrade' of LiquidityPool from being called twice, add the unused gap var and add initialization for added variables --- src/LiquidityPool.sol | 2 +- src/WithdrawRequestNFT.sol | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/LiquidityPool.sol b/src/LiquidityPool.sol index 8a95400fa..13f2d6e0f 100644 --- a/src/LiquidityPool.sol +++ b/src/LiquidityPool.sol @@ -138,7 +138,7 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, IL } function initializeOnUpgrade(address _auctionManager, address _liquifier) external onlyOwner { - require(_auctionManager != address(0) && _liquifier != address(0), "Invalid params"); + require(_auctionManager != address(0) && _liquifier != address(0) && address(auctionManager) == address(0) && address(liquifier) == address(0), "Invalid"); auctionManager = IAuctionManager(_auctionManager); liquifier = ILiquifier(_liquifier); diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 96a5213ad..0c3088cd5 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -29,6 +29,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad uint32 public nextRequestId; uint32 public lastFinalizedRequestId; uint16 public shareRemainderSplitToTreasuryInBps; + uint16 public _unused_gap; // inclusive uint32 public currentRequestIdToScanFromForShareRemainder; @@ -78,10 +79,15 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad paused = true; // make sure the contract is paused after the upgrade pauser = _pauser; + _unused_gap = 0; + shareRemainderSplitToTreasuryInBps = _shareRemainderSplitToTreasuryInBps; currentRequestIdToScanFromForShareRemainder = 1; lastRequestIdToScanUntilForShareRemainder = nextRequestId - 1; + + aggregateSumOfEEthShare = 0; + totalRemainderEEthShares = 0; } /// @notice creates a withdraw request and issues an associated NFT to the recipient From 268efe5b9085d281a5dd2676b3fb4ccd3055af74 Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 18:13:46 +0900 Subject: [PATCH 37/42] use 'isScanOfShareRemainderCompleted' --- src/WithdrawRequestNFT.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WithdrawRequestNFT.sol b/src/WithdrawRequestNFT.sol index 0c3088cd5..a5fbeb413 100644 --- a/src/WithdrawRequestNFT.sol +++ b/src/WithdrawRequestNFT.sol @@ -253,7 +253,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad /// - Burn: the rest of the remainder is burned /// @param _eEthAmount: the remainder of the eEth amount function handleRemainder(uint256 _eEthAmount) external onlyAdmin { - require(currentRequestIdToScanFromForShareRemainder == lastRequestIdToScanUntilForShareRemainder + 1, "Not all prev requests have been scanned"); + require(isScanOfShareRemainderCompleted(), "Not all prev requests have been scanned"); require(getEEthRemainderAmount() >= _eEthAmount, "Not enough eETH remainder"); uint256 beforeEEthShares = eETH.shares(address(this)); From a13919db2ef14ce3ec2c869264f42bb4f2d7300c Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 20:40:37 +0900 Subject: [PATCH 38/42] add the max value constraint on rate limit --- src/EtherFiRedemptionManager.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index 5fc5fd8ed..9e9b97cd4 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -249,6 +249,7 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable } function _convertToBucketUnit(uint256 amount, Math.Rounding rounding) internal pure returns (uint64) { + require(amount <= type(uint64).max * BUCKET_UNIT_SCALE, "EtherFiRedemptionManager: Amount too large"); return (rounding == Math.Rounding.Up) ? SafeCast.toUint64((amount + BUCKET_UNIT_SCALE - 1) / BUCKET_UNIT_SCALE) : SafeCast.toUint64(amount / BUCKET_UNIT_SCALE); } From 58fe3fbcfc345f797be222619a1efe0b4a287bce Mon Sep 17 00:00:00 2001 From: ReposCollector Date: Thu, 2 Jan 2025 20:42:21 +0900 Subject: [PATCH 39/42] remove equality condition --- src/EtherFiRedemptionManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index 9e9b97cd4..630f959c4 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -249,7 +249,7 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable } function _convertToBucketUnit(uint256 amount, Math.Rounding rounding) internal pure returns (uint64) { - require(amount <= type(uint64).max * BUCKET_UNIT_SCALE, "EtherFiRedemptionManager: Amount too large"); + require(amount < type(uint64).max * BUCKET_UNIT_SCALE, "EtherFiRedemptionManager: Amount too large"); return (rounding == Math.Rounding.Up) ? SafeCast.toUint64((amount + BUCKET_UNIT_SCALE - 1) / BUCKET_UNIT_SCALE) : SafeCast.toUint64(amount / BUCKET_UNIT_SCALE); } From 898896a34b893a1809a49620f610ab68798407f2 Mon Sep 17 00:00:00 2001 From: syko Date: Thu, 2 Jan 2025 23:41:16 +0900 Subject: [PATCH 40/42] fix the overflow issue in '_refill' Signed-off-by: syko --- lib/BucketLimiter.sol | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/BucketLimiter.sol b/lib/BucketLimiter.sol index 36dc9a6ed..87a70a2f1 100644 --- a/lib/BucketLimiter.sol +++ b/lib/BucketLimiter.sol @@ -109,23 +109,22 @@ library BucketLimiter { } function _refill(Limit memory limit) internal view { - // We allow for overflow here, as the delta is resilient against it. uint64 now_ = uint64(block.timestamp); if (now_ == limit.lastRefill) { return; } - uint64 delta; + uint256 delta; unchecked { delta = now_ - limit.lastRefill; } - uint64 tokens = delta * limit.refillRate; - uint64 newRemaining = limit.remaining + tokens; + uint256 tokens = delta * uint256(limit.refillRate); + uint256 newRemaining = uint256(limit.remaining) + tokens; if (newRemaining > limit.capacity) { limit.remaining = limit.capacity; } else { - limit.remaining = newRemaining; + limit.remaining = uint64(newRemaining); } limit.lastRefill = now_; } @@ -167,4 +166,4 @@ library BucketLimiter { refill(limit); limit.remaining = remaining; } -} \ No newline at end of file +} From 25ac914b3b48ec57cf232e82424b3e0903a1b011 Mon Sep 17 00:00:00 2001 From: syko Date: Thu, 2 Jan 2025 23:49:18 +0900 Subject: [PATCH 41/42] Update EtherFiRedemptionManager.sol Signed-off-by: syko --- src/EtherFiRedemptionManager.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index 630f959c4..3969e624c 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -151,6 +151,8 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable // To Treasury by transferring eETH IERC20(address(eEth)).safeTransfer(treasury, eEthFeeAmountToTreasury); + require(eEth.totalShares() == totalEEthShare - (sharesToBurn + feeShareToStakers), "EtherFiRedemptionManager: Invalid total shares"); + // To Receiver by transferring ETH (bool success, ) = receiver.call{value: ethReceived, gas: 10_000}(""); require(success, "EtherFiRedemptionManager: Transfer failed"); @@ -296,4 +298,4 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable _; } -} \ No newline at end of file +} From fd52a523caf8f34a1e79a6d9ca7eb95203b490cc Mon Sep 17 00:00:00 2001 From: syko Date: Fri, 3 Jan 2025 00:08:28 +0900 Subject: [PATCH 42/42] prevent the total shares from going below 1 gwei after redemption Signed-off-by: syko --- src/EtherFiRedemptionManager.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EtherFiRedemptionManager.sol b/src/EtherFiRedemptionManager.sol index 3969e624c..5ff6b5cf7 100644 --- a/src/EtherFiRedemptionManager.sol +++ b/src/EtherFiRedemptionManager.sol @@ -151,7 +151,7 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable // To Treasury by transferring eETH IERC20(address(eEth)).safeTransfer(treasury, eEthFeeAmountToTreasury); - require(eEth.totalShares() == totalEEthShare - (sharesToBurn + feeShareToStakers), "EtherFiRedemptionManager: Invalid total shares"); + require(eEth.totalShares() >= 1 gwei && eEth.totalShares() == totalEEthShare - (sharesToBurn + feeShareToStakers), "EtherFiRedemptionManager: Invalid total shares"); // To Receiver by transferring ETH (bool success, ) = receiver.call{value: ethReceived, gas: 10_000}(""); @@ -159,7 +159,7 @@ contract EtherFiRedemptionManager is Initializable, OwnableUpgradeable, Pausable // Make sure the liquidity pool balance is correct && total shares are correct require(address(liquidityPool).balance == prevLpBalance - ethReceived, "EtherFiRedemptionManager: Invalid liquidity pool balance"); - require(eEth.totalShares() == totalEEthShare - (sharesToBurn + feeShareToStakers), "EtherFiRedemptionManager: Invalid total shares"); + require(eEth.totalShares() >= 1 gwei && eEth.totalShares() == totalEEthShare - (sharesToBurn + feeShareToStakers), "EtherFiRedemptionManager: Invalid total shares"); emit Redeemed(receiver, ethAmount, eEthFeeAmountToTreasury, eEthAmountToReceiver); }