diff --git a/src/core/LiquidityPool.sol b/src/core/LiquidityPool.sol index 3999d37b..eb9dd939 100644 --- a/src/core/LiquidityPool.sol +++ b/src/core/LiquidityPool.sol @@ -73,6 +73,14 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, Re //-------------------------------------------------------------------------------------- uint256 public constant SHARE_UNIT = 1e18; + // Hard cap on how far a single rebase may INCREASE TVL (rewards), in bps of TVL. + // 25 bps ≈ 1 month of reward accrual at 3% APR — there is no legitimate reason for a + // single report to raise the rate by more than this, so it is a fixed invariant (not + // governance-configurable). Bounds a buggy/compromised rebase caller at the share-rate + // chokepoint regardless of the oracle-side checks. + uint256 public constant MAX_POSITIVE_REBASE_BPS = 25; + uint256 private constant REBASE_BPS_DENOMINATOR = 10_000; + //-------------------------------------------------------------------------------------- //------------------------------------- EVENTS --------------------------------------- //-------------------------------------------------------------------------------------- @@ -113,6 +121,7 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, Re error InvalidValidatorSize(); error InvalidAmountForShare(); error InvalidRate(); + error RebaseExceedsPositiveCap(); error AlreadyMigrated(); error MigrationNotComplete(); error AlreadyRegistered(); @@ -438,6 +447,17 @@ contract LiquidityPool is Initializable, OwnableUpgradeable, UUPSUpgradeable, Re /// @notice Rebase by ether.fi function rebase(int128 _accruedRewards) public { if (msg.sender != address(membershipManager)) revert IncorrectCaller(); + + // Positive (reward) upper bound, enforced at the share-rate chokepoint regardless + // of who calls rebase. A single rebase cannot increase TVL by more than + // MAX_POSITIVE_REBASE_BPS of pre-rebase TVL. Defense-in-depth alongside the + // oracle-side negative cap in EtherFiAdmin; the negative side is intentionally not + // re-checked here (the oracle path owns it and bounds it tighter). + if (_accruedRewards > 0) { + uint256 maxIncrease = (getTotalPooledEther() * MAX_POSITIVE_REBASE_BPS) / REBASE_BPS_DENOMINATOR; + if (uint256(uint128(_accruedRewards)) > maxIncrease) revert RebaseExceedsPositiveCap(); + } + totalValueOutOfLp = uint128(int128(totalValueOutOfLp) + _accruedRewards); _checkMinAmountForShare(); diff --git a/src/governance/utils/PausableUntil.sol b/src/governance/utils/PausableUntil.sol index 3ca7d74b..cead3d12 100644 --- a/src/governance/utils/PausableUntil.sol +++ b/src/governance/utils/PausableUntil.sol @@ -57,6 +57,12 @@ abstract contract PausableUntil { _requireNotPausedUntil(); PausableUntilStorage storage $ = _getPausableUntilStorage(); uint256 pauseUntilDuration = $.pauseUntilDuration; + // If the duration was never configured (0 — e.g. a fresh proxy or a just-upgraded + // contract before setPauseUntilDuration is called), fall back to MIN_PAUSE_DURATION + // so the emergency pause is always effective. Without this, `pausedUntil` would be + // `block.timestamp + 0` (expires the same block) — a silent no-op that still burns + // the pauser's cooldown. + if (pauseUntilDuration == 0) pauseUntilDuration = MIN_PAUSE_DURATION; if ($.lastPauseTimestamp[msg.sender] + pauseUntilDuration + PAUSER_UNTIL_COOLDOWN > block.timestamp) revert PauserCooldownStillActive(); $.pausedUntil = block.timestamp + pauseUntilDuration; $.lastPauseTimestamp[msg.sender] = block.timestamp; diff --git a/src/oracle/EtherFiAdmin.sol b/src/oracle/EtherFiAdmin.sol index ea2e8e2a..20aa6311 100644 --- a/src/oracle/EtherFiAdmin.sol +++ b/src/oracle/EtherFiAdmin.sol @@ -60,6 +60,10 @@ contract EtherFiAdmin is Initializable, OwnableUpgradeable, UUPSUpgradeable, Rol uint256 public maxFinalizedWithdrawalAmountPerDay; uint256 public maxNumValidatorsToApprovePerDay; + // Override for the per-report negative (slashing) rebase cap, in bps of TVL. + // 0 = use DEFAULT_MAX_NEGATIVE_REBASE_BPS. Settable behind the operating timelock. + uint256 public maxNegativeRebaseBps; + //-------------------------------------------------------------------------------------- //--------------------------------- IMMUTABLES -------------------------------------- //-------------------------------------------------------------------------------------- @@ -89,6 +93,17 @@ contract EtherFiAdmin is Initializable, OwnableUpgradeable, UUPSUpgradeable, Rol uint256 public constant STALE_REPORT_FINALIZATION_COOLDOWN = 7200; // 1 day uint256 public constant BASIS_POINTS_DENOMINATOR = 10_000; + // Default cap on how far a single report may DECREASE TVL (slashing), in bps of TVL, + // independent of elapsedTime. `acceptableRebaseAprInBps` is annualized over elapsedTime, + // so a long-spanning report could pass it while still dropping an outsized absolute + // amount in one rebase. The max *initial* slashing penalty is maxEffBalance/4096 ≈ + // 2.44 bps of TVL even if every validator is slashed at once, so 3 bps is the tight + // default. `maxNegativeRebaseBps` (settable behind the operating timelock) overrides + // it — raised only if a correlated/mid-term slashing event is detected (visible well + // before a 2-day timelock matters). The positive/reward upper bound lives in + // LiquidityPool.rebase (the share-rate-increasing chokepoint). + uint256 public constant DEFAULT_MAX_NEGATIVE_REBASE_BPS = 3; + struct ConstructorAddresses { address etherFiOracle; address stakingManager; @@ -106,6 +121,7 @@ contract EtherFiAdmin is Initializable, OwnableUpgradeable, UUPSUpgradeable, Rol //-------------------------------------------------------------------------------------- event AdminUpdated(address _address, bool _isAdmin); + event MaxNegativeRebaseBpsUpdated(uint256 bps); event AdminOperationsExecuted(address indexed _address, bytes32 indexed _reportHash); event ValidatorApprovalTaskCreated(bytes32 indexed _taskHash, bytes32 indexed _reportHash, uint256[] _validators); @@ -121,6 +137,7 @@ contract EtherFiAdmin is Initializable, OwnableUpgradeable, UUPSUpgradeable, Rol error InvalidMaxFinalizedWithdrawalAmountPerDay(); error InvalidMaxNumValidatorsToApprovePerDay(); error InvalidAcceptableRebaseApr(); + error InvalidMaxNegativeRebaseBps(); error InvalidValidatorTaskBatchSize(); error InvalidMaxAcceptableRebaseApr(); error InvalidStaleOracleReportBlockWindow(); @@ -410,6 +427,19 @@ contract EtherFiAdmin is Initializable, OwnableUpgradeable, UUPSUpgradeable, Rol } int256 absApr = (apr > 0) ? apr : - apr; if (absApr > acceptableRebaseAprInBps) return (false, "EtherFiAdmin: TVL changed too much"); + + // Negative (slashing) cap, independent of elapsedTime. The APR check above is + // annualized, so a long-spanning report could pass it while still dropping an + // outsized absolute amount in one rebase. A single report can only legitimately + // DECREASE TVL by at most the max initial slashing penalty (~2.44 bps if every + // validator is slashed at once), so bound the drop to effectiveMaxNegativeRebaseBps. + // The positive/reward upper bound is enforced in LiquidityPool.rebase. + if (_report.accruedRewards < 0 && currentTVL > 0) { + int256 drop = -int256(_report.accruedRewards); + if (drop * int256(BASIS_POINTS_DENOMINATOR) > currentTVL * int256(effectiveMaxNegativeRebaseBps())) { + return (false, "EtherFiAdmin: negative rebase exceeds cap"); + } + } return (true, ""); } @@ -447,6 +477,21 @@ contract EtherFiAdmin is Initializable, OwnableUpgradeable, UUPSUpgradeable, Rol return (true, ""); } + /// @notice Override the per-report negative (slashing) rebase cap, in bps of TVL. + /// @dev Operation-Timelock-gated. 0 resets to DEFAULT_MAX_NEGATIVE_REBASE_BPS. Capped + /// at 100% so it can be raised during a real correlated-slashing event but never + /// set to a nonsensical value. + function setMaxNegativeRebaseBps(uint256 _bps) external onlyAdmin { + if (_bps > BASIS_POINTS_DENOMINATOR) revert InvalidMaxNegativeRebaseBps(); + maxNegativeRebaseBps = _bps; + emit MaxNegativeRebaseBpsUpdated(_bps); + } + + function effectiveMaxNegativeRebaseBps() public view returns (uint256) { + uint256 v = maxNegativeRebaseBps; + return v == 0 ? DEFAULT_MAX_NEGATIVE_REBASE_BPS : v; + } + function slotForNextReportToProcess() public view returns (uint32) { return (lastHandledReportRefSlot == 0) ? 0 : lastHandledReportRefSlot + 1; } diff --git a/src/restaking/EtherFiRestaker.sol b/src/restaking/EtherFiRestaker.sol index 5311afac..e5dec4e7 100644 --- a/src/restaking/EtherFiRestaker.sol +++ b/src/restaking/EtherFiRestaker.sol @@ -12,6 +12,7 @@ import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import "@etherfi/deposits/Liquifier.sol"; import "@etherfi/core/LiquidityPool.sol"; import "@etherfi/governance/utils/RolesLibrary.sol"; +import "@etherfi/governance/utils/PausableUntil.sol"; import "@etherfi/eigenlayer-interfaces/IStrategyManager.sol"; import "@etherfi/eigenlayer-interfaces/IDelegationManager.sol"; @@ -21,7 +22,7 @@ import "@etherfi/deposits/interfaces/ILiquifier.sol"; import "@etherfi/core/interfaces/ILiquidityPool.sol"; import "@etherfi/governance/rate-limiting/interfaces/IEtherFiRateLimiter.sol"; -contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, PausableUpgradeable, RolesLibrary { +contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, PausableUpgradeable, PausableUntil, RolesLibrary { using SafeERC20 for IERC20; using EnumerableSet for EnumerableSet.Bytes32Set; @@ -143,7 +144,7 @@ contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, /// @notice Transfer stETH to a recipient for instant withdrawal /// @param recipient The address to receive stETH /// @param amount The amount of stETH to transfer - function transferStETH(address recipient, uint256 amount) external { + function transferStETH(address recipient, uint256 amount) external whenNotPaused { if(msg.sender != etherFiRedemptionManager) revert IncorrectCaller(); if (amount > lido.balanceOf(address(this))) revert InsufficientBalance(); IERC20(address(lido)).safeTransfer(recipient, amount); @@ -158,7 +159,7 @@ contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, /// @notice Request for a specific amount of stETH holdings /// @param _amount the amount of stETH to request - function stEthRequestWithdrawal(uint256 _amount) public onlyExecutorOperations returns (uint256[] memory) { + function stEthRequestWithdrawal(uint256 _amount) public onlyExecutorOperations whenNotPaused returns (uint256[] memory) { rateLimiter.consume(STETH_REQUEST_WITHDRAWAL_LIMIT_ID, _amountToGwei(_amount)); uint256 minAmount = lidoWithdrawalQueue.MIN_STETH_WITHDRAWAL_AMOUNT(); @@ -192,7 +193,7 @@ contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, /// @notice Claim a batch of withdrawal requests if they are finalized sending the ETH to the this contract back /// @param _requestIds array of request ids to claim /// @param _hints checkpoint hint for each id. Can be obtained with `findCheckpointHints()` - function stEthClaimWithdrawals(uint256[] calldata _requestIds, uint256[] calldata _hints) external onlyHousekeepingOperations { + function stEthClaimWithdrawals(uint256[] calldata _requestIds, uint256[] calldata _hints) external onlyHousekeepingOperations whenNotPaused { lidoWithdrawalQueue.claimWithdrawals(_requestIds, _hints); _withdrawEther(); @@ -201,7 +202,7 @@ contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, } // Send the ETH back to the liquidity pool - function withdrawEther() public onlyHousekeepingOperations { + function withdrawEther() public onlyHousekeepingOperations whenNotPaused { _withdrawEther(); } @@ -230,7 +231,7 @@ contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, } // undelegate from the current AVS operator & un-restake all - function undelegate() external onlyOperatingMultisig returns (bytes32[] memory) { + function undelegate() external onlyOperatingMultisig whenNotPaused returns (bytes32[] memory) { bytes32[] memory withdrawalRoots = eigenLayerDelegationManager.undelegate(address(this)); for (uint256 i = 0; i < withdrawalRoots.length; i++) { @@ -245,7 +246,7 @@ contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, } // deposit the token in holding into the restaking strategy - function depositIntoStrategy(address token, uint256 amount) external onlyExecutorOperations returns (uint256) { + function depositIntoStrategy(address token, uint256 amount) external onlyExecutorOperations whenNotPaused returns (uint256) { rateLimiter.consume(DEPOSIT_INTO_STRATEGY_LIMIT_ID, _amountToGwei(amount)); IERC20(token).safeIncreaseAllowance(address(eigenLayerStrategyManager), amount); @@ -260,7 +261,7 @@ contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, /// Made easy for operators /// @param token the token to withdraw /// @param amount the amount of token to withdraw - function queueWithdrawals(address token, uint256 amount) public onlyExecutorOperations returns (bytes32[] memory) { + function queueWithdrawals(address token, uint256 amount) public onlyExecutorOperations whenNotPaused returns (bytes32[] memory) { rateLimiter.consume(QUEUE_WITHDRAWALS_LIMIT_ID, _amountToGwei(amount)); uint256 shares = getEigenLayerRestakingStrategy(token).underlyingToSharesView(amount); @@ -280,7 +281,7 @@ contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, function completeQueuedWithdrawals( IDelegationManager.Withdrawal[] memory _queuedWithdrawals, IERC20[][] memory _tokens - ) external onlyHousekeepingOperations { + ) external onlyHousekeepingOperations whenNotPaused { uint256 num = _queuedWithdrawals.length; bool[] memory receiveAsTokens = new bool[](num); for (uint256 i = 0; i < num; i++) { @@ -397,16 +398,42 @@ contract EtherFiRestaker is Initializable, UUPSUpgradeable, OwnableUpgradeable, return total; } - // Pauses the contract + // Boolean pause (multisig). Indefinite until the multisig unpauses. function pauseContract() external onlyOperatingMultisig { _pause(); } - // Unpauses the contract - function unPauseContract() external onlyOperatingMultisig { + // Unpauses the boolean pause (deliberate, multisig-only) + function unpauseContract() external onlyOperatingMultisig { _unpause(); } + /// @notice Fast, auto-expiring emergency halt callable by the Guardian (Hypernative / + /// EOA keys). Mirrors the protocol-wide halt model (tokens, LP, redemption): + /// the Guardian can stop fund movement immediately without assembling the + /// multisig, the pause auto-expires (8h–30d) with a per-pauser cooldown so a + /// wrong/compromised halt self-heals, and resume stays deliberate (multisig). + function pauseContractUntil() external onlyGuardian { + _pauseUntil(); + } + + function unpauseContractUntil() external onlyOperatingMultisig { + _unpauseUntil(); + } + + function setPauseUntilDuration(uint256 _pauseUntilDuration) external onlyAdmin { + _setPauseUntilDuration(_pauseUntilDuration); + } + + /// @dev Fold the auto-expiring PausableUntil check into OZ's `_requireNotPaused`, so + /// the standard `whenNotPaused` modifier (used on every fund-flow fn) halts on + /// EITHER the boolean pause or the Guardian's auto-expiring pause — consistent + /// with the rest of the protocol, no bespoke modifier needed. + function _requireNotPaused() internal view override { + _requireNotPausedUntil(); + super._requireNotPaused(); + } + // INTERNAL functions /// @dev Convert wei to gwei for rate-limiter buckets, with overflow check. diff --git a/src/restaking/interfaces/IEtherFiRestaker.sol b/src/restaking/interfaces/IEtherFiRestaker.sol index 48ec17bb..3aafb33c 100644 --- a/src/restaking/interfaces/IEtherFiRestaker.sol +++ b/src/restaking/interfaces/IEtherFiRestaker.sol @@ -12,5 +12,5 @@ interface IEtherFiRestaker { function transferStETH(address recipient, uint256 amount) external; function lido() external view returns (ILido); function pauseContract() external; - function unPauseContract() external; + function unpauseContract() external; } \ No newline at end of file diff --git a/test/EETH.t.sol b/test/EETH.t.sol index fd239f39..e6e4d67e 100644 --- a/test/EETH.t.sol +++ b/test/EETH.t.sol @@ -84,72 +84,62 @@ contract EETHTest is TestSetup { function test_EEthRebase() public { assertEq(liquidityPoolInstance.getTotalPooledEther(), 0 ether); - // Total pooled ether = 10 + // Amounts scaled so each rebase stays within the 25 bps per-report cap (0.25% of + // TVL) while preserving clean share math. Total pooled ether = 10000 startHoax(alice); - liquidityPoolInstance.deposit{value: 10 ether}(); + liquidityPoolInstance.deposit{value: 10000 ether}(); vm.stopPrank(); - assertEq(liquidityPoolInstance.getTotalPooledEther(), 10 ether); - assertEq(eETHInstance.totalSupply(), 10 ether); - assertEq(eETHInstance.totalShares(), 10 ether); - assertEq(eETHInstance.shares(alice), 10 ether); + assertEq(liquidityPoolInstance.getTotalPooledEther(), 10000 ether); + assertEq(eETHInstance.totalSupply(), 10000 ether); + assertEq(eETHInstance.totalShares(), 10000 ether); + assertEq(eETHInstance.shares(alice), 10000 ether); - // Total pooled ether = 20 + // +25 ether reward = 0.25% of 10000 (exactly at the cap). Pooled = 10025 vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(10 ether); - _transferTo(address(liquidityPoolInstance), 10 ether); + liquidityPoolInstance.rebase(25 ether); + _transferTo(address(liquidityPoolInstance), 25 ether); - assertEq(liquidityPoolInstance.getTotalPooledEther(), 20 ether); - assertEq(eETHInstance.totalSupply(), 20 ether); - assertEq(eETHInstance.totalShares(), 10 ether); - assertEq(eETHInstance.shares(alice), 10 ether); + assertEq(liquidityPoolInstance.getTotalPooledEther(), 10025 ether); + assertEq(eETHInstance.totalSupply(), 10025 ether); + assertEq(eETHInstance.totalShares(), 10000 ether); + assertEq(eETHInstance.shares(alice), 10000 ether); - // ALice total claimable Ether - /// (20 * 10) / 10 - assertEq(liquidityPoolInstance.getTotalEtherClaimOf(alice), 20 ether); + // Alice total claimable Ether: (10025 * 10000) / 10000 = 10025 + assertEq(liquidityPoolInstance.getTotalEtherClaimOf(alice), 10025 ether); startHoax(bob); - liquidityPoolInstance.deposit{value: 5 ether}(); + liquidityPoolInstance.deposit{value: 10025 ether}(); vm.stopPrank(); - assertEq(liquidityPoolInstance.getTotalPooledEther(), 25 ether); - assertEq(eETHInstance.totalSupply(), 25 ether); + assertEq(liquidityPoolInstance.getTotalPooledEther(), 20050 ether); + assertEq(eETHInstance.totalSupply(), 20050 ether); - // Bob Shares = (5 * 10) / (25 - 5) = 2,5 - assertEq(eETHInstance.shares(bob), 2.5 ether); - assertEq(eETHInstance.totalShares(), 12.5 ether); + // Bob Shares = (10025 * 10000) / (20050 - 10025) = 10000 + assertEq(eETHInstance.shares(bob), 10000 ether); + assertEq(eETHInstance.totalShares(), 20000 ether); - // Bob claimable Ether - /// (25 * 2,5) / 12,5 = 5 ether + // claimable: (20050 * 10000) / 20000 = 10025 each + assertEq(liquidityPoolInstance.getTotalEtherClaimOf(alice), 10025 ether); + assertEq(liquidityPoolInstance.getTotalEtherClaimOf(bob), 10025 ether); - //ALice Claimable Ether - /// (25 * 10) / 12,5 = 20 ether - assertEq(liquidityPoolInstance.getTotalEtherClaimOf(alice), 20 ether); - assertEq(liquidityPoolInstance.getTotalEtherClaimOf(bob), 5 ether); + assertEq(eETHInstance.balanceOf(alice), 10025 ether); + assertEq(eETHInstance.balanceOf(bob), 10025 ether); - assertEq(eETHInstance.balanceOf(alice), 20 ether); - assertEq(eETHInstance.balanceOf(bob), 5 ether); - - // Staking Rewards sent to liquidity pool - /// vm.deal sets the balance of whoever its called on - /// In this case 10 ether is added as reward + // +50 ether reward = ~0.25% of 20050 (within cap). Pooled = 20100 vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(10 ether); - _transferTo(address(liquidityPoolInstance), 10 ether); - - assertEq(liquidityPoolInstance.getTotalPooledEther(), 35 ether); - assertEq(eETHInstance.totalSupply(), 35 ether); + liquidityPoolInstance.rebase(50 ether); + _transferTo(address(liquidityPoolInstance), 50 ether); - // Bob claimable Ether - /// (35 * 2,5) / 12,5 = 7 ether - assertEq(liquidityPoolInstance.getTotalEtherClaimOf(bob), 7 ether); + assertEq(liquidityPoolInstance.getTotalPooledEther(), 20100 ether); + assertEq(eETHInstance.totalSupply(), 20100 ether); - // Alice Claimable Ether - /// (35 * 10) / 12,5 = 20 ether - assertEq(liquidityPoolInstance.getTotalEtherClaimOf(alice), 28 ether); + // claimable: (20100 * 10000) / 20000 = 10050 each + assertEq(liquidityPoolInstance.getTotalEtherClaimOf(bob), 10050 ether); + assertEq(liquidityPoolInstance.getTotalEtherClaimOf(alice), 10050 ether); - assertEq(eETHInstance.balanceOf(alice), 28 ether); - assertEq(eETHInstance.balanceOf(bob), 7 ether); + assertEq(eETHInstance.balanceOf(alice), 10050 ether); + assertEq(eETHInstance.balanceOf(bob), 10050 ether); } function test_TransferWithAmount() public { diff --git a/test/EtherFiRestaker.t.sol b/test/EtherFiRestaker.t.sol index 45a9ae82..33280777 100644 --- a/test/EtherFiRestaker.t.sol +++ b/test/EtherFiRestaker.t.sol @@ -7,6 +7,7 @@ import "forge-std/Test.sol"; import "@openzeppelin-upgradeable/contracts/token/ERC20/extensions/ERC20BurnableUpgradeable.sol"; import "@etherfi/eigenlayer-interfaces/IDelegationManager.sol"; +import "@etherfi/governance/utils/PausableUntil.sol"; import "@etherfi/eigenlayer-interfaces/IStrategyManager.sol"; import "@etherfi/eigenlayer-interfaces/ISignatureUtils.sol"; @@ -292,4 +293,59 @@ contract EtherFiRestakerTest is TestSetup { etherFiRestakerInstance.getTotalPooledEther(); } + // PR #385 security review (H1 + Yash's review): the restaker's pause previously gated + // nothing — pauseContract() flipped a flag but no money-movement function checked it. + // It now uses the protocol-wide PausableUntil model: the Guardian (HN/EOA keys) fires + // an auto-expiring halt, the multisig has a boolean pause, and both stop fund movement. + function test_guardianPauseUntil_halts_fund_movement() public { + vm.startPrank(roleRegistryInstance.owner()); + roleRegistryInstance.grantRole(roleRegistryInstance.GUARDIAN_ROLE(), bob); + vm.stopPrank(); + + // operating timelock (owner) sets the auto-expiry duration + vm.prank(owner); + etherFiRestakerInstance.setPauseUntilDuration(8 hours); + + // Guardian fast-halt (auto-expiring) + vm.prank(bob); + etherFiRestakerInstance.pauseContractUntil(); + uint256 until = etherFiRestakerInstance.pausedUntil(); + assertGt(until, block.timestamp); + + // whenNotHalted is the first gate on transferStETH, so the halt fires before the + // caller check — proves fund movement is actually stopped. + vm.expectRevert(abi.encodeWithSelector(PausableUntil.ContractPausedUntil.selector, until)); + etherFiRestakerInstance.transferStETH(bob, 1); + + // undelegate (owner holds OPERATION_MULTISIG) queues withdrawal of ALL restaked + // assets — same fund-flow category, so it must also be halted. + vm.prank(owner); + vm.expectRevert(abi.encodeWithSelector(PausableUntil.ContractPausedUntil.selector, until)); + etherFiRestakerInstance.undelegate(); + + // a non-guardian cannot fire the auto-expiring halt + vm.prank(alice); + vm.expectRevert(RoleRegistry.OnlyGuardian.selector); + etherFiRestakerInstance.pauseContractUntil(); + + // resume is deliberate / multisig-only + vm.prank(owner); + etherFiRestakerInstance.unpauseContractUntil(); + assertEq(etherFiRestakerInstance.pausedUntil(), 0); + } + + // The boolean multisig pause also halts fund movement (reverts via OZ Pausable). + function test_booleanPause_also_halts_fund_movement() public { + vm.prank(owner); // owner holds OPERATION_MULTISIG + etherFiRestakerInstance.pauseContract(); + assertTrue(etherFiRestakerInstance.paused()); + + vm.expectRevert("Pausable: paused"); + etherFiRestakerInstance.transferStETH(bob, 1); + + vm.prank(owner); + etherFiRestakerInstance.unpauseContract(); + assertFalse(etherFiRestakerInstance.paused()); + } + } diff --git a/test/LiquidityPool.t.sol b/test/LiquidityPool.t.sol index 92bdea45..0b407a51 100644 --- a/test/LiquidityPool.t.sol +++ b/test/LiquidityPool.t.sol @@ -246,42 +246,45 @@ contract LiquidityPoolTest is TestSetup { } function test_WithdrawLiquidityPoolAccrueStakingRewardsWithoutPartialWithdrawal() public { - vm.deal(alice, 3 ether); + // Deposits scaled to 400 each (TVL 800) so the 2 ETH rebase stays within the + // 25 bps cap (0.25% of 800 = 2 ETH). The rebase still adds exactly 2 ETH of + // out-of-LP headroom, so the fallback-funding pattern below is unchanged. + vm.deal(alice, 401 ether); vm.startPrank(alice); - liquidityPoolInstance.deposit{value: 2 ether}(); + liquidityPoolInstance.deposit{value: 400 ether}(); assertEq(alice.balance, 1 ether); - assertEq(eETHInstance.balanceOf(alice), 2 ether); + assertEq(eETHInstance.balanceOf(alice), 400 ether); assertEq(eETHInstance.balanceOf(bob), 0); vm.stopPrank(); - vm.deal(bob, 3 ether); + vm.deal(bob, 401 ether); vm.startPrank(bob); - liquidityPoolInstance.deposit{value: 2 ether}(); + liquidityPoolInstance.deposit{value: 400 ether}(); assertEq(bob.balance, 1 ether); - assertEq(eETHInstance.balanceOf(alice), 2 ether); - assertEq(eETHInstance.balanceOf(bob), 2 ether); + assertEq(eETHInstance.balanceOf(alice), 400 ether); + assertEq(eETHInstance.balanceOf(bob), 400 ether); vm.stopPrank(); vm.deal(owner, 100 ether); vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(2 ether); - assertEq(eETHInstance.balanceOf(alice), 3 ether); - assertEq(eETHInstance.balanceOf(bob), 3 ether); + liquidityPoolInstance.rebase(2 ether); // 0.25% of 800 ETH TVL — exactly at the cap + assertEq(eETHInstance.balanceOf(alice), 401 ether); + assertEq(eETHInstance.balanceOf(bob), 401 ether); (bool sent, ) = address(liquidityPoolInstance).call{value: 1 ether}(""); assertEq(sent, true); - assertEq(eETHInstance.balanceOf(alice), 3 ether); - assertEq(eETHInstance.balanceOf(bob), 3 ether); + assertEq(eETHInstance.balanceOf(alice), 401 ether); + assertEq(eETHInstance.balanceOf(bob), 401 ether); (sent, ) = address(liquidityPoolInstance).call{value: 1 ether}(""); assertEq(sent, true); - assertEq(eETHInstance.balanceOf(alice), 3 ether); - assertEq(eETHInstance.balanceOf(bob), 3 ether); + assertEq(eETHInstance.balanceOf(alice), 401 ether); + assertEq(eETHInstance.balanceOf(bob), 401 ether); (sent, ) = address(liquidityPoolInstance).call{value: 1 ether}(""); assertEq(sent, false); - assertEq(eETHInstance.balanceOf(alice), 3 ether); - assertEq(eETHInstance.balanceOf(bob), 3 ether); + assertEq(eETHInstance.balanceOf(alice), 401 ether); + assertEq(eETHInstance.balanceOf(bob), 401 ether); } @@ -290,23 +293,26 @@ contract LiquidityPoolTest is TestSetup { function test_fallback() public { assertEq(liquidityPoolInstance.getTotalPooledEther(), 0 ether); - vm.deal(bob, 100 ether); + // Deposit 400 ETH so a 1 ETH rebase is within the 25 bps cap (0.25% of 400 = 1 ETH). + // The raw fallback send below is only accepted up to totalValueOutOfLp (= the + // rebased amount), so out-of-LP must be >= 1 ETH for the 1 ETH send to land. + vm.deal(bob, 400 ether); vm.startPrank(bob); - liquidityPoolInstance.deposit{value: 100 ether}(); + liquidityPoolInstance.deposit{value: 400 ether}(); vm.stopPrank(); - assertEq(liquidityPoolInstance.getTotalPooledEther(), 100 ether); + assertEq(liquidityPoolInstance.getTotalPooledEther(), 400 ether); vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(3 ether); - assertEq(liquidityPoolInstance.getTotalPooledEther(), 103 ether); + liquidityPoolInstance.rebase(1 ether); // 0.25% of 400 ETH — exactly at the cap + assertEq(liquidityPoolInstance.getTotalPooledEther(), 401 ether); vm.deal(alice, 3 ether); vm.prank(alice); (bool sent, ) = address(liquidityPoolInstance).call{value: 1 ether}(""); - assertEq(address(liquidityPoolInstance).balance, 100 ether + 1 ether); + assertEq(address(liquidityPoolInstance).balance, 400 ether + 1 ether); assertEq(sent, true); - assertEq(liquidityPoolInstance.getTotalPooledEther(), 103 ether); + assertEq(liquidityPoolInstance.getTotalPooledEther(), 401 ether); } function test_RegisterAsBnftHolder() public { @@ -1137,12 +1143,41 @@ contract LiquidityPoolTest is TestSetup { vm.stopPrank(); uint256 totalPooledBefore = liquidityPoolInstance.getTotalPooledEther(); - + + // rebase within the 25 bps per-report cap (0.25% of 100 ETH TVL = 0.25 ETH) vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(5 ether); - + liquidityPoolInstance.rebase(0.25 ether); + uint256 totalPooledAfter = liquidityPoolInstance.getTotalPooledEther(); - assertEq(totalPooledAfter, totalPooledBefore + 5 ether); + assertEq(totalPooledAfter, totalPooledBefore + 0.25 ether); + } + + // PR #385 (Yash review): the positive (reward) rebase upper bound is a hard 25 bps + // constant enforced inside LiquidityPool.rebase, at the share-rate chokepoint, + // regardless of caller. Not governance-configurable. + function test_positiveRebaseCap_enforced() public { + vm.deal(alice, 100 ether); + vm.prank(alice); + liquidityPoolInstance.deposit{value: 100 ether}(); + + assertEq(liquidityPoolInstance.MAX_POSITIVE_REBASE_BPS(), 25); + + uint256 tvl = liquidityPoolInstance.getTotalPooledEther(); + uint256 cap = (tvl * 25) / 10_000; + + // one wei over the cap reverts + vm.prank(address(membershipManagerInstance)); + vm.expectRevert(LiquidityPool.RebaseExceedsPositiveCap.selector); + liquidityPoolInstance.rebase(int128(uint128(cap + 1))); + + // exactly at the cap succeeds + vm.prank(address(membershipManagerInstance)); + liquidityPoolInstance.rebase(int128(uint128(cap))); + + // a negative rebase (slashing) is not bounded by this LP-side cap — the oracle + // path (EtherFiAdmin negative cap) owns that direction + vm.prank(address(membershipManagerInstance)); + liquidityPoolInstance.rebase(-int128(uint128(cap))); } function test_RebaseFailsIfNotMembershipManager() public { @@ -1323,7 +1358,7 @@ contract LiquidityPoolTest is TestSetup { vm.stopPrank(); vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(10 ether); + liquidityPoolInstance.rebase(0.25 ether); // within 25 bps cap uint256 claim = liquidityPoolInstance.getTotalEtherClaimOf(alice); assertGt(claim, 100 ether); @@ -1508,9 +1543,9 @@ contract LiquidityPoolTest is TestSetup { vm.stopPrank(); vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(10 ether); + liquidityPoolInstance.rebase(0.25 ether); // within 25 bps cap - assertEq(liquidityPoolInstance.getTotalPooledEther(), 110 ether); + assertEq(liquidityPoolInstance.getTotalPooledEther(), 100.25 ether); } //-------------------------------------------------------------------------------------- @@ -2101,22 +2136,23 @@ contract LiquidityPoolTest is TestSetup { } function test_minAmountForShare_subsequent_deposit_succeeds_above_min() public { - // Establish ratio = 1.1 ether per share (positive rebase), then upgrade with MIN below it. + // Establish ratio = 1.0025 ether per share via a within-cap positive rebase + // (0.25% of 100 ETH), then upgrade with MIN below it. vm.deal(alice, 100 ether); vm.prank(alice); liquidityPoolInstance.deposit{value: 100 ether}(); vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(10 ether); - assertEq(liquidityPoolInstance.amountForShare(1 ether), 1.1 ether); + liquidityPoolInstance.rebase(0.25 ether); + assertEq(liquidityPoolInstance.amountForShare(1 ether), 1.0025 ether); - _upgradeLpWithMinAmount(1.05 ether); + _upgradeLpWithMinAmount(1.001 ether); vm.deal(bob, 5 ether); vm.prank(bob); uint256 shares = liquidityPoolInstance.deposit{value: 5 ether}(); assertGt(shares, 0); // Proportional minting preserves ratio. - assertEq(liquidityPoolInstance.amountForShare(1 ether), 1.1 ether); + assertEq(liquidityPoolInstance.amountForShare(1 ether), 1.0025 ether); } // --- rebase flow --- @@ -2128,9 +2164,11 @@ contract LiquidityPoolTest is TestSetup { _upgradeLpWithMinAmount(1 ether); + // rebase within the 25 bps cap (0.25% of 100 ETH = 0.25 ETH) → ratio 1.0025, + // still above the 1 ETH min-amount-per-share floor. vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(20 ether); - assertEq(liquidityPoolInstance.amountForShare(1 ether), 1.2 ether); + liquidityPoolInstance.rebase(0.25 ether); + assertEq(liquidityPoolInstance.amountForShare(1 ether), 1.0025 ether); } function test_minAmountForShare_negative_rebase_above_min_succeeds() public { @@ -2259,8 +2297,10 @@ contract LiquidityPoolTest is TestSetup { _setLpAccounting(50 ether, 50 ether); // Bound rebase to keep totalValueOutOfLp in [0, type(uint128).max] after the update. - // Pre-rebase totalValueOutOfLp == 50e18. - rebaseAmount = int128(bound(int256(rebaseAmount), -50 ether, 50 ether)); + // Pre-rebase totalValueOutOfLp == 50e18. The positive side is bounded to the 25 bps + // per-report cap (0.25% of the 100 ETH TVL = 0.25 ETH); the negative side (the + // minAmountForShare-underflow boundary this test exercises) is not capped. + rebaseAmount = int128(bound(int256(rebaseAmount), -50 ether, 0.25 ether)); uint256 minAmount = 0.5 ether; _upgradeLpWithMinAmount(minAmount); diff --git a/test/LpRebaseWrnClaimUnderflow.t.sol b/test/LpRebaseWrnClaimUnderflow.t.sol index 6daa39e3..d2f2d214 100644 --- a/test/LpRebaseWrnClaimUnderflow.t.sol +++ b/test/LpRebaseWrnClaimUnderflow.t.sol @@ -147,13 +147,13 @@ contract LpRebaseWrnClaimUnderflowTest is TestSetup { function test_small_rebase_after_finalize_claim_succeeds() public { uint256 requestAmount = 50 ether; - // Seed 10 ether of headroom in outOfLp via a positive rebase. - // This simulates legitimate accrued rewards: outOfLp grows - // without ETH actually moving into the LP. Without this, the - // lock transfer at finalize would leave outOfLp == lock exactly, - // and ANY negative rebase would underflow at claim time. + // Seed 0.25 ether of headroom in outOfLp via a positive rebase (within the 25 bps + // cap: 0.25% of the 100 ETH pool). This simulates legitimate accrued rewards: + // outOfLp grows without ETH actually moving into the LP. Without this, the lock + // transfer at finalize would leave outOfLp == lock exactly, and a negative rebase + // would underflow at claim time. vm.prank(address(membershipManagerInstance)); - liquidityPoolInstance.rebase(int128(int256(uint256(10 ether)))); + liquidityPoolInstance.rebase(int128(int256(uint256(0.25 ether)))); vm.prank(claimant); uint256 tokenId = liquidityPoolInstance.requestWithdraw(claimant, requestAmount); @@ -163,10 +163,10 @@ contract LpRebaseWrnClaimUnderflowTest is TestSetup { withdrawRequestNFTInstance.finalizeRequests(tokenId); vm.stopPrank(); - // outOfLp == 60 ether (10 seeded + 50 from lock), lock == 50. - // A small negative rebase (-1 ether) drops outOfLp to 59 — - // still well above the lock. - int128 rebaseDelta = -int128(int256(uint256(1 ether))); + // outOfLp == 50.25 ether (0.25 seeded + 50 from lock), lock == 50. + // A small negative rebase (-0.1 ether) drops outOfLp to 50.15 — + // still above the lock. + int128 rebaseDelta = -int128(int256(uint256(0.1 ether))); vm.prank(address(membershipManagerInstance)); liquidityPoolInstance.rebase(rebaseDelta); diff --git a/test/MembershipManager.t.sol b/test/MembershipManager.t.sol index a45e980b..0fb05578 100644 --- a/test/MembershipManager.t.sol +++ b/test/MembershipManager.t.sol @@ -1438,11 +1438,12 @@ contract MembershipManagerTest is TestSetup { // rebase must succeed (no division-by-zero) AND the underlying LP rebase must apply. uint256 tvolBefore = liquidityPoolInstance.totalValueOutOfLp(); + // rebase within the 25 bps cap (0.25% of the 5 ETH pool = 0.0125 ETH) vm.prank(address(etherFiAdminInstance)); - membershipManagerInstance.rebase(1 ether); + membershipManagerInstance.rebase(0.01 ether); assertEq( liquidityPoolInstance.totalValueOutOfLp(), - tvolBefore + 1 ether, + tvolBefore + 0.01 ether, "LP rebase still applied even on early-return" ); } diff --git a/test/TestSetup.sol b/test/TestSetup.sol index 9937f036..42818d6f 100644 --- a/test/TestSetup.sol +++ b/test/TestSetup.sol @@ -755,6 +755,12 @@ contract TestSetup is Test, ContractCodeChecker, DepositDataGeneration { liquidityPoolInstance.setMinWithdrawAmount(0.001 ether); vm.stopPrank(); + // PR #385: disable the new per-rebase caps for generic fork tests (some rebase a + // few % of TVL, above the 25bps default). Dedicated tests set explicit values. + vm.startPrank(admin); + etherFiAdminInstance.setMaxNegativeRebaseBps(10_000); + vm.stopPrank(); + // Run the one-shot escrow migration so `requestWithdraw` / `withdraw` // (both gated by `escrowMigrationCompleted`) function on the upgraded // LP. Idempotent: skip if already complete. @@ -1439,6 +1445,11 @@ contract TestSetup is Test, ContractCodeChecker, DepositDataGeneration { liquidityPoolInstance.setValidatorSizeWei(32 ether); liquidityPoolInstance.setMaxWithdrawAmount(1000 ether); liquidityPoolInstance.setMinWithdrawAmount(0.001 ether); + // PR #385: the new per-rebase caps (positive in LP, negative in EtherFiAdmin) + // would reject the large artificial rebases many generic tests perform on a small + // fresh-deploy TVL. Disable them here (100% = effectively no bound); dedicated + // tests set explicit values to exercise enforcement. + etherFiAdminInstance.setMaxNegativeRebaseBps(10_000); // Pause WithdrawRequestNFT so existing tests that unPauseContract in their // own setUp continue to find it paused (initializeOnUpgrade used to set this). withdrawRequestNFTInstance.pauseContract();