diff --git a/src/staking/AuctionManager.sol b/src/staking/AuctionManager.sol index 30489860c..9904f0e93 100644 --- a/src/staking/AuctionManager.sol +++ b/src/staking/AuctionManager.sol @@ -48,7 +48,6 @@ contract AuctionManager is IBlacklister public immutable blacklister; INodeOperatorManager public immutable nodeOperatorManager; address public immutable stakingManagerContractAddress; - address public immutable membershipManagerContractAddress; address public immutable treasury; //-------------------------------------------------------------------------------------- @@ -58,6 +57,7 @@ contract AuctionManager is event BidCreated(address indexed bidder, uint256 amountPerBid, uint256[] bidIdArray, uint64[] ipfsIndexArray); event BidCancelled(uint256 indexed bidId); event BidReEnteredAuction(uint256 indexed bidId); + event BidRevenueForwarded(uint256 indexed bidId, address indexed treasury, uint256 amount); event WhitelistDisabled(bool whitelistStatus); event WhitelistEnabled(bool whitelistStatus); @@ -85,11 +85,10 @@ contract AuctionManager is //-------------------------------------------------------------------------------------- /// @custom:oz-upgrades-unsafe-allow constructor - constructor(address _roleRegistry, address _blacklister, address _nodeOperatorManagerContract, address _stakingManagerContractAddress, address _membershipManagerContractAddress, address _treasury) RolesLibrary(_roleRegistry) { + constructor(address _roleRegistry, address _blacklister, address _nodeOperatorManagerContract, address _stakingManagerContractAddress, address _treasury) RolesLibrary(_roleRegistry) { blacklister = IBlacklister(_blacklister); nodeOperatorManager = INodeOperatorManager(_nodeOperatorManagerContract); stakingManagerContractAddress = _stakingManagerContractAddress; - membershipManagerContractAddress = _membershipManagerContractAddress; treasury = _treasury; _disableInitializers(); } @@ -193,7 +192,9 @@ contract AuctionManager is } /// @notice Updates the details of the bid which has been used in a stake match - /// @dev Called by batchDepositWithBidIds() in StakingManager.sol + /// @dev Called by batchDepositWithBidIds() in StakingManager.sol. Forwards the + /// consumed bid's ETH to `treasury` so protocol-side bid revenue is not + /// stranded in this contract. /// @param _bidId the ID of the bid being removed from the auction (since it has been selected) function updateSelectedBidInformation( uint256 _bidId @@ -203,6 +204,13 @@ contract AuctionManager is bid.isActive = false; numberOfActiveBids--; + + uint256 amount = bid.amount; + if (amount > 0) { + (bool sent, ) = treasury.call{value: amount}(""); + if (!sent) revert EtherTransferFailed(); + emit BidRevenueForwarded(_bidId, treasury, amount); + } } /// @notice Lets a bid that was matched to a cancelled stake re-enter the auction diff --git a/src/withdrawals/WithdrawRequestNFT.sol b/src/withdrawals/WithdrawRequestNFT.sol index 9c4bb682e..aeeaaa9eb 100644 --- a/src/withdrawals/WithdrawRequestNFT.sol +++ b/src/withdrawals/WithdrawRequestNFT.sol @@ -141,6 +141,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad error InvalidMinAcceptableShareRate(); error InvalidMinMaxAcceptableShareRate(); error AlreadyInitialized(); + error NotInitialized(); error InvalidLiveRate(); error BurnExceedsShares(); error InvalidEEthShares(); @@ -326,6 +327,12 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad function finalizeRequests(uint256 requestId) external { if (msg.sender != address(etherFiAdmin)) revert IncorrectCaller(); + // Defends against the upgrade-ordering trap: if `initializeShareRateFreezeUpgrade` + // has not yet pushed the sentinel, the first `finalizeRequests` would push a real + // rate as the trace's first entry, causing every legacy tokenId to look up to that + // rate instead of 0 — corrupting the live-rate fallback semantic for pre-upgrade + // requests. Block until the sentinel is in place. + if (_finalizationRates.length() == 0) revert NotInitialized(); if (requestId < lastFinalizedRequestId) revert CannotUndoFinalization(); if (requestId >= nextRequestId) revert CannotFinalizeFutureRequests(); diff --git a/test/AddressProvider.t.sol b/test/AddressProvider.t.sol index 9e3f1690c..0d1e03221 100644 --- a/test/AddressProvider.t.sol +++ b/test/AddressProvider.t.sol @@ -9,9 +9,8 @@ contract AuctionManagerV2Test is AuctionManager { address _blacklister, address _nodeOperatorManagerContract, address _stakingManagerContractAddress, - address _membershipManagerContractAddress, address _treasury - ) AuctionManager(_roleRegistry, _blacklister, _nodeOperatorManagerContract, _stakingManagerContractAddress, _membershipManagerContractAddress, _treasury) {} + ) AuctionManager(_roleRegistry, _blacklister, _nodeOperatorManagerContract, _stakingManagerContractAddress, _treasury) {} function isUpgraded() public pure returns(bool){ return true; } @@ -143,7 +142,6 @@ contract AddressProviderTest is TestSetup { address(blacklisterInstance), address(nodeOperatorManagerInstance), address(stakingManagerInstance), - address(membershipManagerInstance), address(treasuryInstance) ); auctionInstance.upgradeTo(address(auctionManagerV2Implementation)); diff --git a/test/TestSetup.sol b/test/TestSetup.sol index 2bd90cf83..b6b706e53 100644 --- a/test/TestSetup.sol +++ b/test/TestSetup.sol @@ -763,6 +763,12 @@ contract TestSetup is Test, ContractCodeChecker, DepositDataGeneration { liquidityPoolInstance.initializeOnUpgradeV2(); } + // Push the share-rate-freeze sentinel checkpoint that the new WRNFT + // `finalizeRequests` guard requires post-upgrade. Idempotent: skip if + // already pushed (length != 0 reverts AlreadyInitialized). + vm.prank(_roleRegOwner); + try withdrawRequestNFTInstance.initializeShareRateFreezeUpgrade() {} catch {} + // The upgraded WeETH/EETH still have global MINT/BURN circuit-breaker // buckets (plus the new opt-in per-address buckets for transfers). // Bootstrap MINT/BURN at unbounded capacity on the fork so generic @@ -1032,7 +1038,6 @@ contract TestSetup is Test, ContractCodeChecker, DepositDataGeneration { address(blacklisterInstance), address(nodeOperatorManagerProxy), address(stakingManagerProxy), - address(membershipManagerProxy), address(treasuryInstance) ); auctionInstance.upgradeTo(address(auctionImplementation)); @@ -1329,6 +1334,10 @@ contract TestSetup is Test, ContractCodeChecker, DepositDataGeneration { membershipNftInstance.initialize("https://etherfi-cdn/{id}.json", address(membershipManagerInstance)); withdrawRequestNFTInstance.initialize(payable(address(liquidityPoolProxy)), payable(address(eETHProxy)), payable(address(membershipManagerProxy))); + // Push the share-rate-freeze sentinel checkpoint that the new finalizeRequests + // guard requires. We're inside an active `vm.startPrank(owner)` block here and + // `owner` was granted UPGRADE_TIMELOCK_ROLE in Phase 1, so no extra prank needed. + withdrawRequestNFTInstance.initializeShareRateFreezeUpgrade(); // initializeOnUpgrade now reverts (checks immutable roleRegistry == 0). Set the // share-remainder split + paused state it used to bootstrap directly. withdrawRequestNFTInstance.updateShareRemainderSplitToTreasuryInBps(1000); diff --git a/test/WithdrawRequestNFT.t.sol b/test/WithdrawRequestNFT.t.sol index ce1b05e8a..8c20697a0 100644 --- a/test/WithdrawRequestNFT.t.sol +++ b/test/WithdrawRequestNFT.t.sol @@ -20,6 +20,14 @@ contract WithdrawRequestNFTIntrusive is WithdrawRequestNFT { function setLastFinalizedRequestIdForTest(uint32 _id) external { lastFinalizedRequestId = _id; } + + /// @dev Test-only: zero out the `_finalizationRates` length so tests can re-exercise + /// the `initializeShareRateFreezeUpgrade` flow from a "pre-init" state. Slot 311 + /// matches `forge inspect WithdrawRequestNFT storageLayout`; keep in sync if the + /// storage layout shifts. + function clearFinalizationRatesForTest() external { + assembly { sstore(311, 0) } + } } contract WithdrawRequestNFTTest is TestSetup { @@ -2019,9 +2027,25 @@ contract WithdrawRequestNFTTest is TestSetup { withdrawRequestNFTInstance.upgradeTo(cur_impl); } + /// @dev Test-only helper to revert the WRNFT to the "pre-init" state for `_finalizationRates`, + /// so tests can exercise the `initializeShareRateFreezeUpgrade` flow directly even + /// though `setUp` pushes the sentinel by default. + function _clearFinalizationRatesForTest() internal { + address cur_impl = withdrawRequestNFTInstance.getImplementation(); + address new_impl = address(new WithdrawRequestNFTIntrusive(address(roleRegistryInstance))); + vm.prank(withdrawRequestNFTInstance.owner()); + withdrawRequestNFTInstance.upgradeTo(new_impl); + WithdrawRequestNFTIntrusive(payable(address(withdrawRequestNFTInstance))).clearFinalizationRatesForTest(); + vm.prank(withdrawRequestNFTInstance.owner()); + withdrawRequestNFTInstance.upgradeTo(cur_impl); + } + /// @dev Requests that pre-date `initializeShareRateFreezeUpgrade()` see value 0 from /// `lowerLookup` and fall back to the live-rate path. function test_shareRateFreeze_legacySentinel_fallsBackToLiveRate() public { + // setUp pushes the sentinel by default; this test exercises the pre-init flow. + _clearFinalizationRatesForTest(); + startHoax(bob); liquidityPoolInstance.deposit{value: 10 ether}(); vm.stopPrank(); @@ -2074,6 +2098,7 @@ contract WithdrawRequestNFTTest is TestSetup { /// @dev After the upgrade init, the next finalize pushes a real snapshot and all /// requestIds strictly above the sentinel use the frozen rate. function test_shareRateFreeze_postUpgradeRequests_useFrozenRate() public { + _clearFinalizationRatesForTest(); vm.prank(withdrawRequestNFTInstance.owner()); withdrawRequestNFTInstance.initializeShareRateFreezeUpgrade(); @@ -2092,6 +2117,7 @@ contract WithdrawRequestNFTTest is TestSetup { /// @dev `initializeShareRateFreezeUpgrade` is a one-shot. function test_shareRateFreeze_initializeUpgrade_revertsIfAlreadyInitialized() public { + _clearFinalizationRatesForTest(); vm.startPrank(withdrawRequestNFTInstance.owner()); withdrawRequestNFTInstance.initializeShareRateFreezeUpgrade(); vm.expectRevert(WithdrawRequestNFT.AlreadyInitialized.selector); @@ -2228,6 +2254,9 @@ contract WithdrawRequestNFTTest is TestSetup { /// local live-rate fallback path. The claim payout must match what the pre-upgrade /// code (live `amountForShare`-based) would have computed at that moment. function test_legacyFallback_matchesPreUpgradeLiveRate() public { + // setUp pushes the sentinel by default; this test exercises the pre-init flow. + _clearFinalizationRatesForTest(); + // 1. set up a "pre-upgrade finalized request" by stepping lastFinalizedRequestId without // pushing a real snapshot — mirroring on-chain state at the moment of the upgrade. startHoax(bob); diff --git a/test/fork-tests/RoleMigrationStorageIntegrity.t.sol b/test/fork-tests/RoleMigrationStorageIntegrity.t.sol index 38aafe0a2..f592e7150 100644 --- a/test/fork-tests/RoleMigrationStorageIntegrity.t.sol +++ b/test/fork-tests/RoleMigrationStorageIntegrity.t.sol @@ -99,7 +99,7 @@ contract RoleMigrationStorageIntegrityTest is Test, Deployed { bytes32[] memory preAM = _snapshot(AUCTION_MANAGER); bytes32[] memory preER = _snapshot(ETHERFI_RESTAKER); - address newAM = address(new AuctionManager(ROLE_REGISTRY, address(0), NODE_OPERATOR_MANAGER, STAKING_MANAGER, MEMBERSHIP_MANAGER, TREASURY)); + address newAM = address(new AuctionManager(ROLE_REGISTRY, address(0), NODE_OPERATOR_MANAGER, STAKING_MANAGER, TREASURY)); address newER = address(new EtherFiRestaker( LIQUIDITY_POOL, LIQUIFIER, diff --git a/test/fork-tests/validator-key-gen.t.sol b/test/fork-tests/validator-key-gen.t.sol index 443d1b43a..7356bd950 100644 --- a/test/fork-tests/validator-key-gen.t.sol +++ b/test/fork-tests/validator-key-gen.t.sol @@ -86,7 +86,7 @@ contract ValidatorKeyGenTest is Test, ArrayTestHelper { vm.prank(liquidityPool.owner()); liquidityPool.upgradeTo(address(liquidityPoolImpl)); - AuctionManager auctionManagerImpl = new AuctionManager(address(roleRegistry), address(blacklister), address(nodeOperatorManager), address(stakingManager), 0x3d320286E014C3e1ce99Af6d6B00f0C1D63E3000, 0x0c83EAe1FE72c390A02E426572854931EefF93BA); + AuctionManager auctionManagerImpl = new AuctionManager(address(roleRegistry), address(blacklister), address(nodeOperatorManager), address(stakingManager), 0x0c83EAe1FE72c390A02E426572854931EefF93BA); vm.prank(auctionManager.owner()); auctionManager.upgradeTo(address(auctionManagerImpl));