feat: support non-EigenPod withdrawal credentials - #485
Open
pankajjagtapp wants to merge 60 commits into
Open
Conversation
Validators can now be spun up against withdrawal credentials that point at their EtherFiNode instead of at an EigenPod. instantiateEtherFiNode(false) already produced a pod-less node; the three StakingManager creation paths hardcoded the pod as the credential target. Add EtherFiNodesManager.withdrawalCredentialTarget(node), which returns the pod when the node has one and the node itself otherwise, and route createBeaconValidators, registerBeaconValidators and confirmAndFundBeaconValidators through it. One resolver means all three agree bit-for-bit; a disagreement would revert IncorrectBeaconRoot and strand the 1 ETH deposit. The target is derived rather than stored. A node's pod is fixed at instantiation: createEigenPod is callable only by StakingManager, whose only call site is inside instantiateEtherFiNode, and createPod reverts if a pod already exists. Storage layouts are byte-identical to master for all three contracts. EtherFiNode calls the EIP-7002 and EIP-7251 predeploys directly when there is no pod, since the node is then the validators' withdrawal address, and reads request fees from the predeploys instead of the pod. Both request paths now reject batches mixing validators from different nodes: with a pod EigenLayer enforced this, but the predeploys accept any pubkey and the consensus layer silently drops requests whose source withdrawal address is not the caller. Also adds disablePod and withdrawDisabledPodETH for EigenLayer v1.14.0 pod retirement. That version is not on mainnet yet, so the call reverts rather than silently succeeding. Existing pod-backed nodes are unaffected and need no migration.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
📊 Forge Coverage ReportGenerated by workflow run #820 |
sweepFunds(address node) sweeps a node straight to the liquidity pool without resolving a validator id, matching the address/id overload pattern the rest of the contract already uses. Validators in the new credential regime pay out to the node itself, so the node address is the natural handle. The id overload is untouched: the address path adds _validateNode so the overload cannot be used to call sweepFunds() on an arbitrary contract, while leaving the id path's behaviour exactly as it was for legacy nodes that may not be backfilled. Add an end-to-end lifecycle suite covering spawner registration, operator whitelisting, bidding, pod-less node creation, credential derivation, register, 1 ETH create, 31 ETH top-up and the sweep, plus the gate at each step. Review fixes: - cast to address before comparing contract-type variables (solc 9170) - declare disablePod / withdrawDisabledPodETH on IEtherFiNodesManager, which already carried the PodDisabled event and MixedNodeRequest error
…g tests Four suites that pass on master were broken by this branch. Root causes: 1. Fee reads were routed through the EtherFiNode, a function that only exists on the new implementation. That made the EtherFiNodesManager upgrade depend on the EtherFiNode beacon being upgraded first, and EtherFiNode's `fallback() payable` swallows unknown selectors, so it surfaced as an ABI decode failure rather than a clear revert. Pod-backed nodes now read the fee off the pod exactly as before; only the pod-less path uses the node. The two upgrades are independently deployable again. 2. The MixedNodeRequest batch check applied to pod-backed nodes, where EigenLayer already enforces pod membership against the pod's own validator set. Ours checked the pubkey mapping, which does not have every legacy validator linked. Scoped to pod-less nodes, which is the only place the check is needed. 3. The credential resolver imposed _validateNode on the request paths, which never required deployedEtherFiNodes and would have broken legacy nodes that were never backfilled. The validated resolver still gates the creation paths, where the target is baked into a deposit; the request paths use an unvalidated derivation for the emitted event only. validator-key-gen.t.sol now upgrades EtherFiNodesManager alongside StakingManager, which production does anyway. Forwarding: forwardEigenPodCall and forwardExternalCall accept either eigenpod or housekeeping operations, so the withdrawal-completion cron can batch across nodes. The per-caller selector whitelist still applies. Tests: EIP-7002 partial and batch withdrawals, EIP-7251 switch-to-compounding, in-node consolidation and consolidation to an external target, fee and batch rejection cases, and the full disabled-pod sweep to the LiquidityPool against a v1.14.0 stub.
…Manager Removes five overloads that were one-line delegations to their address twins: queueETHWithdrawal, queueWithdrawals, completeQueuedETHWithdrawals, completeQueuedWithdrawals and sweepFunds. Nothing in src/ called them, and the operations tooling already encodes the address form: queueETHWithdrawal(address,uint256) in run_consolidation_python.py and unrestake_validators.py, completeQueuedETHWithdrawals(node, bool) in CompleteEigenLayerWithdrawals.s.sol. sweepFunds(address) also drops its _validateNode check. Reclaims 845 bytes of EtherFiNodesManager, which was at 541 bytes of margin under the 24576 limit. Now 23190, margin 1386. Callers in prelude.t.sol and EtherFiNodesManager.t.sol resolve the node address first. Those resolutions are hoisted above any preceding cheatcode: vm.prank and vm.expectRevert are each consumed by the next call, and the resolver is itself a call, so evaluating it inside a pranked argument list silently moves the assertion onto the wrong call. The four *_byId_blockedByPauseContractUntil tests are deleted rather than converted; each has a _byAddress_ twin covering the same pause behaviour.
disablePod() returns void, so an un-upgraded EtherFiNode beacon swallows the call via its empty fallback and returns success without disabling the pod. The manager then emitted a false PodDisabled, after which consolidating validators out would cut the pod's beacon-chain slashing factor. Assert pod.restakingDisabled() after the call so a stale-beacon deployment reverts PodNotDisabled instead. Adds a regression test; updates the existing mock-based retirement test to also report restakingDisabled(). Finding 1 (HIGH) from the PR #485 pre-audit review; reproduced on a Tenderly fork carrying real EigenLayer v1.14.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The batch-membership check ran only when target == node (pod-less). EigenLayer v1.14 stops enforcing pod membership once a pod is disabled, so a mixed-node batch routed through a disabled pod (target == pod) was validated by nobody: the consensus layer silently drops the mismatched requests while the fee is burned and phantom exit/consolidation events are emitted. Extend the guard to fire when the target pod reports restakingDisabled(). A try/catch leaves pre-v1.14 pods (no such selector) on the original pod-backed path, so existing withdrawals/consolidations are unaffected. Finding 3 (MED) from the PR #485 pre-audit review; reproduced on a Tenderly fork carrying real EigenLayer v1.14.0 (mixed batch on a disabled pod accepted with no MixedNodeRequest revert). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
disablePod does not clear ownerToPod, so the resolver keeps returning a retired pod as the credential target. All three StakingManager creation paths would then bake 0x02 credentials pointing at a dead pod, and the resulting validator could never verify credentials or checkpoint - stranding its 32 ETH. The PR also removed the getEigenPod()==0 guard from registerBeaconValidators, so nothing else blocked this. Make the validated withdrawalCredentialTarget resolver revert PodRetired when the pod reports restakingDisabled(). The check lives in the resolver used only by creation paths (the request paths call _credentialTarget directly), so validators already live on a since-retired pod are unaffected. try/catch keeps pre-v1.14 pods working. Finding 4 (MED) from the PR #485 pre-audit review; the retired-pod resolver output was reproduced on a Tenderly fork with real EigenLayer v1.14.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sweepFunds(address) was the only node-taking entrypoint without _validateNode, so a housekeeping-role caller could pass an arbitrary contract, make the manager call into it, and emit a forged FundsTransferred event that poisons off-chain accounting. Add _validateNode(node) to match every sibling function. Finding 6 (LOW) from the PR #485 pre-audit review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
test_disablePod_revertsUntilEigenLayerV1_14_0 logged a message and returned with zero assertions once EigenLayer v1.14.0 is live, so the moment mainnet upgrades the only disablePod fork test silently stops testing anything. Replace the early return with a real assertion: on a v1.14 fork, drive retirement through the manager and assert the pod reports restakingDisabled(). The pre-v1.14 revert branch is unchanged, so both branches now assert. Finding 12 (test quality) from the PR #485 pre-audit review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Yash Saraswat <107622640+0xpanicError@users.noreply.github.com>
…nt-noop fix(enm): revert disablePod on stale-beacon silent no-op [audit finding 1, HIGH]
…date-node fix(enm): validate node in sweepFunds(address) [audit finding 6, LOW]
…vacuous-pass test(enm): remove vacuous-pass escape hatch in disablePod fork test [audit finding 12]
…-disabled-pods fix(enm): apply MixedNodeRequest guard to disabled pods [audit finding 3, MED]
…wal-credentials' into fix/enm-block-creation-against-retired-pod # Conflicts: # src/staking/interfaces/IEtherFiNodesManager.sol
…against-retired-pod fix(enm): block validator creation against a retired pod [audit finding 4, MED]
Two follow-ups from Cursor Bugbot on the merged fixes: Revert finding-3 change (#487): extending the MixedNodeRequest guard to disabled pods enforced membership via etherFiNodeFromPubkeyHash, which does not contain all legacy validators. A legitimate same-pod consolidation batch of legacy validators - the actual migration case - could revert MixedNodeRequest on the very retirement path it targets. Restore the pod-less-only guard and drop the _requiresBatchMembershipCheck helper. Scope finding-4 check (#488): the PodRetired revert lived in withdrawalCredentialTarget, which confirmAndFundBeaconValidators also calls, so disabling a pod between the 1 ETH create and the 31 ETH top-up stranded the validator at 1 ETH. Resolve the target directly in the top-up path (same pod/node address regardless of retirement); the checked resolver still guards initial creation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
EtherFiAdmin._approveValidators (the production 31 ETH top-up builder, reached via executeValidatorApprovalTask) hardcoded getEigenPod(). For a pod-less node that is address(0), so it baked 0x02+address(0) credentials whose deposit root never matches the node credentials fixed at creation -> confirmAndFundBeacon- Validators reverts IncorrectBeaconRoot and the validator is stranded at 1 ETH. Resolve the target as pod-or-node (raw, no PodRetired gate) so the oracle-built deposit data matches what StakingManager.confirmAndFundBeaconValidators re- derives, for both pod-backed and pod-less nodes. Finding 1 of the independent re-review at head 0101757. The production builder was outside the original 15-file diff, so the existing suite (which hand-builds DepositData and pranks the admin) never exercised it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…valTask Regression for the oracle top-up blocker: drives the REAL production path (EtherFiAdmin.executeValidatorApprovalTask -> _approveValidators -> LiquidityPool.confirmAndFundBeaconValidators) for a pod-less validator, instead of hand-building the top-up DepositData and pranking the admin as the existing pod-less tests do. That fixture skipped _approveValidators, which is why the blocker survived 133 green tests. Creates a pod-less validator (phase 1, 1 ETH), lets _approveValidators build the top-up data, and asserts it funds to full validator size. Fails IncorrectBeaconRoot against the pre-fix builder; passes after it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dators-resolver fix(oracle): resolve top-up credentials through pod-or-node + regression test [re-review finding 1]
…ator set Prior finding 3 (re-review ask 5). EigenLayer v1.14 stops enforcing pod membership once a pod is disabled, so a mixed-node batch routed through a disabled pod (target == pod) was validated by nobody: the consensus layer silently drops the foreign requests while the fee is burned and phantom exit/consolidation events are emitted - a real hazard during the 25k-validator migration, where those events would desync oracle/DOSE monitoring. Add a disabled-pod branch to both request paths that checks each source pubkey against the pod's OWN validator set (IEigenPod.validatorStatus != INACTIVE), not etherFiNodeFromPubkeyHash. This is the correct fix: the earlier map-based attempt (reverted) wrongly rejected legitimate same-pod batches whose legacy sources were never linked into the map. try/catch on restakingDisabled() keeps pre-v1.14 pods on the live-pod path where EigenLayer enforces membership itself. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
[LOW / hardening]
But the same pod function is also reachable through the forwarding path: if the operating timelock ever whitelists the Suggestion: deny-list it in if (bytes4(data[:4]) == IEigenPod.withdrawDisabledPodETH.selector) revert ForwardedCallNotAllowed();The safe path ( |
Contributor
Tenderly e2e rehearsal (re-run, 2026-08-21)Re-ran the full migration rehearsal on a fresh Tenderly vnet forked from mainnet at block 25,806,007, with this PR's head ( What was tested — all passing (9/9):
Confirm it yourself — public vnet RPC (chain id 1): RPC=https://virtual.mainnet.eu.rpc.tenderly.co/d3fd2f6f-1777-4caf-9421-b2d35a58df72
# EigenLayer is v1.14.0
cast call 0x39053D51B77DC0d36036Fc1fCc8Cb819df8Ef37A 'version()(string)' --rpc-url $RPC
# EtherFiNodesManager runs this PR (new resolver exists; pod-backed node resolves to its pod)
cast call 0x8B71140AD2e5d1E7018d2a7f8a288BD3CD38916F 'withdrawalCredentialTarget(address)(address)' 0x1Cd7c7061DE2c6546D61Cf46e26C548d6e2BD7E5 --rpc-url $RPCOne operational note: a real on-vnet |
A node without an EigenPod is itself the validators' withdrawal-credential target and calls the EIP-7002/7251 predeploys directly, so nothing logged the request at the credential target address. Emit the same four events an EigenPod writes, with identical names and signatures so topic0 matches and existing pod decoders work unchanged: ExitRequested, WithdrawalRequested, SwitchToCompoundingRequested and ConsolidationRequested. Only the pod-less branch emits; a pod-backed node stays silent because its pod already logs the request.
Assert the node-level log on the five existing pod-less request tests. Add a selector-equality check against IEigenPodEvents so a rename on either side breaks the build, and a recorded-log test proving a pod-backed node does not duplicate its pod's event.
P2 - validate pubkey length before hashing it for an event. _pubkeyHash relied on the predeploy to reject wrong-length pubkeys. That holds for EIP-7002, which takes exactly 56 bytes (48 + 8), but not for EIP-7251, which takes exactly 96: a 47-byte source paired with a 49-byte target sums to 96, so the predeploy accepts it and the node would hash and log two pubkeys belonging to no validator. EtherFiNodesManager rejects those lengths today, but only as a side effect of hashing both keys for rate limiting, and that loop now looks redundant next to the node's own events. Keep the invariant local instead: revert InvalidPubKeyLength in _pubkeyHash. Verified against the EIP text rather than assumed, and the new test confirms the predeploy does accept the malformed 96-byte payload. P3 - condense the comments added by the previous two commits. Also dismissed, with reasoning in the review summary: the leftover-fee question (both the pod and pod-less paths leave the remainder in the node for sweepFunds, so they are symmetric) and duplicating IEigenPodEvents rather than inheriting it (a selector-equality test guards the parity). forge build clean; 320/320 behaviour tests pass on a mainnet fork. Storage layout unchanged: the change adds a constant and events, neither of which occupies a slot.
This comment was marked as low quality.
This comment was marked as low quality.
P2 - StakingManager.confirmAndFundBeaconValidators resolved the credential target through etherFiNodesManager.getEigenPod(address), which runs _validateNode and reverts UnknownNode. EtherFiAdmin._approveValidators resolves the same target raw, with a comment saying why: withdrawalCredentialTarget "would reject a legacy node missing from deployedEtherFiNodes". The two must agree or the deposit roots disagree, so a legacy node absent from deployedEtherFiNodes reverted the 31 ETH top-up and stranded the 1 ETH leg. Read the node directly, matching EtherFiAdmin. Dropping _validateNode weakens nothing: the node comes from etherFiNodeFromPubkeyHash, and the resolved target only feeds a deposit-root recomputation that reverts IncorrectBeaconRoot on any mismatch. Also pins the premise behind skipping the mixed-batch guard on a live pod. IEigenPod's docstring lists "pubkey MUST belong to a validator whose withdrawal credentials are this pod" under requirements NOT checked by the pod, which would make the guard's justification wrong. The deployed pod disagrees: test_livePod_rejectsAForeignSourceInABatch shows mainnet's EigenPod reverting ValidatorNotActiveInPod on a foreign source, so no fee burns and no phantom event survives. Asserting deployed behaviour rather than the stale comment. Full suite: 1648 passed, 33 failed, all pre-existing and reproduced with this change reverted (Liquifier/Restaker/Redemption Capped() from mainnet state drift, plus a missing OP_RPC_URL). Behaviour tests 321/321.
This comment was marked as low quality.
This comment was marked as low quality.
Drops two that only restated the code, and trims the rest to the reason a reader cannot get from the code itself. Comment-only; build clean and 321/321 behaviour tests pass.
Documents the control this PR removes for pod-less nodes. A live pod reverts ValidatorNotActiveInPod on a consolidation target it has not proven, so a pod-backed node cannot consolidate into an outside validator. The pod-less path applies no target check. 322/322 behaviour tests pass. Not pushed: PR is in audit.
The target decides where a source's balance lands, and nothing below us
checks it. EIP-7251's process_consolidation_request only requires the target
to hold 0x02 credentials; there is no source/target same-address rule. A pod
vets the target only when the pod is the caller, so a pod-less node calling
the predeploy directly had no target constraint at all. One compromised
EXECUTOR_OPERATIONS_ROLE key could have moved every pod-less validator's
stake to a validator it owned.
Accept a target on three grounds, deny otherwise:
- src == target: a switch to compounding, moves no value.
- linked in etherFiNodeFromPubkeyHash: one of ours.
- ACTIVE in the source's own pod: EigenLayer merkle-verified its
credentials via verifyWithdrawalCredentials.
Cross-credential consolidation still works, which is the point of the
feature: a target on another node passes the second branch. Only targets we
have no record of are rejected.
ACTIVE rather than != INACTIVE: WITHDRAWN passes that looser check, and the
consensus layer drops a withdrawn target, burning the fee and logging a
consolidation that never happens.
Note the registry branch is only as strong as the registry, and
linkLegacyValidatorIds writes it under this same role. Moving that function
to a stricter role is required for this guard to hold against a compromised
key, and is tracked separately.
324/324 behaviour tests pass on a mainnet fork.
Completes the separation the consolidation target guard depends on. That guard accepts a target when it is linked in etherFiNodeFromPubkeyHash, and linkLegacyValidatorIds writes that map while verifying nothing about the pair: it reads DEPRECATED_etherfiNodeAddress[id] and stores whatever pubkey the caller passed. Sharing EXECUTOR_OPERATIONS_ROLE with requestConsolidation left one key able to link a validator it owned and then consolidate into it, so the guard was a speed bump rather than a control. onlyOperatingMultisig (OPERATION_MULTISIG_ROLE, 4-of-7), matching setProofSubmitter. The power to grow the trusted set is now separate from the power to use it: a compromised executor key can still shuffle value between validators we already own, which is recoverable griefing, but cannot introduce a new destination. Timelock was the alternative. Multisig is enough here because the Safe is already the trust root, and a 2-day delay on linking a newly discovered legacy key buys nothing. test_linkLegacyValidatorIds now asserts the separation directly: an address holding only EXECUTOR_OPERATIONS_ROLE is rejected. It uses a fresh address because the shared fixture actors hold several roles at once, and startPrank because a nested role-id read consumes a single-call prank. Fixtures that link as setup were granted the multisig role. Deployment order matters: the ~23k legacy backfill must run BEFORE this ships, or every batch needs a 4-of-7 signature. The ops runbook records that. Full suite: 1679/1684. The 5 failures are pre-existing and environmental (a NotRegistered setUp and 6 suites needing OP_RPC_URL), reproduced with these changes reverted.
Merged
Comment-only; build clean and 324/324 behaviour tests pass.
backfillExistingEtherFiNodes took the caller's word: any address it was handed became a trusted node in deployedEtherFiNodes, which _validateNode and the consolidation target guard both read. Both now prove the address by round-tripping its immutable manager: IEtherFiNode(node).etherFiNodesManager() must equal the expected manager. Reading the candidate's ERC-1967 beacon slot was the other option suggested, but it is not implementable: a contract cannot SLOAD another contract's storage. The round-trip is the on-chain equivalent, and it is equally strong here because the manager address is an immutable in the beacon's implementation, so every beacon proxy answers identically and nothing else answers correctly. try/catch on the round-trip so an EOA or a contract without the selector fails as InvalidEtherFiNode / UnknownNode rather than an opaque empty revert. Documents I-01 in the design doc: on the pod-less path a pubkey is authorized by its link alone, which exists from the 1 ETH deposit, so a request sent before activation succeeds here and is discarded by consensus after paying the fee and consuming rate-limit capacity. Accepted rather than fixed - gating it needs either activation proofs, which this design leaves out of scope, or a delay after linking, which would block legitimate emergency exits. Mitigation is on the caller. Existing backfill tests passed literal addresses, so they now use real beacon proxies; the check rejecting them was the point. Full suite 1680/1686. The 6 failures are environmental: a NotRegistered setUp, suites needing OP_RPC_URL, and one RPC connection reset.
sweepFunds took a raw address with an explicit "unvalidated" comment, unlike every other node-taking function. A housekeeping key could point it at any contract returning an arbitrary balance and emit FundsTransferred(node, balance) for a transfer that never happened, poisoning off-chain accounting. The gate was originally dropped because legacy nodes were not all backfilled into deployedEtherFiNodes, which would have made stray ETH on them unreachable. The legacy backfill removes that reason, so the check is restored.
The manager forwarded the entire msg.value while the pod-less path spends only fee * requests.length at the predeploy. The surplus stayed on the node and was later swept into the LiquidityPool as ETH never counted as out-of-LP, so totalValueOutOfLp drifted below actual holdings while TVL stayed flat. At scale that mis-states the figure the oracle rebases against and can make later legitimate sweeps revert on underflow. Anything above the totalValueOutOfLp cap was simply stranded. Now only the required fee is forwarded and the remainder returns to msg.sender. The fee is exact: EIP-7002/7251 fees update at block end, so the manager's read and the node's re-read inside the same transaction always agree. Fee totals are computed into a single local because a second one pushed requestExecutionLayerTriggeredWithdrawal over the stack limit.
Both new insertions landed between an existing docblock and the function it described, so addressToWithdrawalCredentials's block became the first NatSpec preceding withdrawalCredentialTarget, and _sweepToLiquidityPool's preceded disablePod. forge doc and ABI metadata would have described the credential resolver with a non-existent addr param and left both real functions undocumented.
A pod-less validator is created with 0x02 credentials, and EIP-7251's is_valid_switch_to_compounding_request requires 0x01, so such a request can only be dropped by the consensus layer. The on-chain path reported full success anyway: it burned the per-request fee at the predeploy and emitted SwitchToCompoundingRequested plus ValidatorSwitchToCompoundingRequested. It also escaped the throttle. _getTotalConsolidationGwei counts src == target as zero gwei, so CONSOLIDATION_REQUEST_LIMIT_ID was never debited and the pod-less MixedNodeRequest guard only checks sources. An executor key could submit unbounded such requests, inflating the global EIP-7251 excess counter and raising consolidation fees for everyone with no protocol-side limit. Reverts SwitchNotNeeded before the predeploy call, so no fee is spent. The manager-level target guard still exempts src == target, which stays correct for the pod-backed path where a validator may genuinely hold 0x01.
Both bodies were rewritten to cover three credential regimes but only the @dev Access lines were updated, leaving the rest wrong for the operator who hand-builds these calls: - No grouping is performed by either implementation. The node comes from requests[0] and the whole batch goes to it; mixed batches are rejected, not split. - "Caller should ensure all provided validators share the same eigenpod" is meaningless for a pod-less node, which has none. The requirement is that all sources share one EtherFiNode. - "EigenLayer validates that validators belong to the pod automatically" was the most safety-relevant line and the most wrong: it holds only for a live pod, not for a retired one (EL v1.14 stops enforcing) nor pod-less.
EXIT_REQUEST_LIMIT_ID is debited from the request, not from what the beacon chain actually withdraws: consensus caps a partial payout at min(balance - 32 ETH - pending, amount), and a full exit always costs FULL_EXIT_GWEI regardless of validator size. A request for 100 ETH can withdraw 5 ETH and still burn 100 ETH of limit. Accepted rather than fixed. The withdrawn amount is decided by consensus later and the execution layer never learns it, so it cannot be charged at request time. This is griefing by an already-privileged role, bounded by a ~61,440 ETH bucket that refills in about a day.
Those four fixes together pushed the EtherFiNodesManager runtime bytecode to 24,675 bytes, 99 over the 24,576 EIP-170 limit, so the implementation could not be deployed at all. Reverting them puts it back to 24,478, a margin of 98. Nothing caught this earlier: foundry does not enforce the runtime limit in tests without code_size_limit set, and no CI workflow runs forge build --sizes, so all 1,679 tests passed against bytecode that CREATE would reject. Reverted rather than trimmed because the size budget is the binding constraint and it needs a decision that is not one of these four fixes: lower optimizer_runs, via_ir, dropping the legacy uint256 id overloads, or moving pure helpers behind an external library. Re-landing any of I-03, I-04, I-06 or I-08 depends on which of those we take, and how much headroom it frees. Kept: I-05 and I-07, which are NatSpec only and cost zero bytecode. Reverts, in reverse order: 20810cc I-08: document that the exit rate limit debits requested gwei 8a3c5f1 I-06: reject src == target consolidations on the pod-less path 2c2c461 I-04: refund surplus EIP-7002/7251 fees to the caller 2232d0d I-03: validate the node in sweepFunds(address) Their SHAs are unchanged and remain reachable, since the audit report references them. This is an append-only revert, not a history rewrite.
Deploys the four implementations changed by PR #485 (EtherFiNodesManager, EtherFiNode, StakingManager, EtherFiAdmin) via CREATE2, and builds the UPGRADE_TIMELOCK batch that repoints them and drains the retired Treasury into the LiquidityPool. Constructor args are sourced from the live deployments' immutables. The timelock salt is pinned rather than block-derived so the Safe tx hashes signers reproduce are stable.
Moves TREASURY_LEGACY into the script instead of Deployed.s.sol, trims the comments, and adds: - immutable snapshots for ENM/StakingManager/EtherFiAdmin taken before the batch and re-verified after, plus verifyNotReinitializable on each - fork tests exercising the upgraded contracts: a pod-less validator spin-up, an EL-triggered exit on that pod-less node through the EIP-7002 predeploy, and the classic EigenLayer queue/complete withdrawal path Roles for the fork tests are granted by pranking the UPGRADE_TIMELOCK (RoleRegistry owner) outside the batch, so the proposal calldata and the Safe tx hashes in 3CP-secure#662 are unchanged.
Verifying the impl pointer and immutables does not prove the oracle still works, and EtherFiAdmin is the oracle entry point. This applies the upgrade and runs a real cycle: the three live committee members submit at the real quorum of 3, consensus is reached, then executeTasks runs after the postReportWaitTimeInSlots delay. Asserts the rebase applies exactly (TVL +50 ETH), the eETH exchange rate rises, and both report cursors advance. Kept separate from transactions.s.sol to avoid colliding with concurrent work on that file. The oracle config is left untouched: quorum cannot be lowered to 1 because _checkQuorum requires numActiveCommitteeMembers < 2 * quorumSize, so the real members are pranked instead.
Three genuine failures on master, plus three that are local config. - Withdraw.t.sol, Validator-Flows.t.sol: _syncOracleReportState removed and re-added AVS_OPERATOR_1/2 as committee members, but both were rotated off mainnet, so removeCommitteeMember reverts NotRegistered. Write the committee state and quorum directly instead, so setup no longer depends on who is registered today. setQuorumSize cannot be used because _checkQuorum enforces strict majority against the live member count. - EtherFiRedemptionManager.t.sol: test_end_to_end_redeem_stETH assumed a 20% watermark exceeds the restaker's stETH. It no longer does (456,305 stETH against 2,141,454 TVL = 21.3%), so the redeem succeeded where the test expected a revert. Derive the watermark from live state and assert it stays under maxLowWatermarkInBpsOfTvl so future drift fails loudly. - .example.env: document OP_RPC_URL and SCROLL_RPC_URL, which several suites need and CI supplies as secrets. forge test --no-match-contract 'Invariant|Handler': master 1305 passed / 6 failed -> branch 1594 passed / 3 failed. The 3 remaining are OP_RPC_URL not being set locally; they pass in CI.
…ript-avs-dereg-treasury chore: deploy + upgrade scripts for non-EigenPod withdrawal credentials
Manual review of non-EigenPod validator creation and EL deprecation paths, 8/24-8/25 2026, latest commit 692d3f7. 1 Low, 8 Informational.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Spin up validators whose 0x02 withdrawal credentials point at their
EtherFiNodeinstead of an EigenPod.instantiateEtherFiNode(false)already produced a pod-less node. The blocker was that all threeStakingManagercreation paths hardcoded the pod as the credential target.Change
EtherFiNodesManagerMixedNodeRequestbatch guard;disablePod/withdrawDisabledPodETH;sweepFunds(address); forwarding open to housekeepingStakingManagerEtherFiNodecreatePod/stakeblockedEtherFiAdmin_approveValidatorsuses the resolver, so oracle top-ups no longer strand pod-less keys at 1 ETHOne resolver keeps the three creation paths bit-identical. Disagreement reverts
IncorrectBeaconRootand strands the 1 ETH.Why the node is the target: it is already a
BeaconProxybehindetherFiNodeBeacon, so oneupgradeTocovers every instance, and it already hasfallback() payableplus_sweepToLiquidityPool(). Decisively, the oracle and DOSE already mappubkeyHash -> EtherFiNode, so sweep and monitoring jobs need no registry, no factory-event scan, and no new set to walk (~15,600 entries at 500k ETH).The pod path is byte-identical
EtherFiNodebeacon being upgraded first. The two upgrades stay independently deployable._validateNodegates the creation paths, where the target is baked into a deposit. Request paths derive the target unvalidated for the emitted event only.No stored credential regime
The target is derived, not stored, because a node's pod is already immutable:
createEigenPodrevertsInvalidCallerunlessmsg.sender == stakingManager(EtherFiNodesManager.sol:95)StakingManager's only call site is insideinstantiateEtherFiNode(StakingManager.sol:111)createPod()revertsEigenPodAlreadyExists;disablePodnever clearsownerToPodStorage layouts are byte-identical to master for all three contracts (
forge inspect <c> storageLayout). These are UUPS proxies, so that removes a class of upgrade risk a stored flag would carry.createEigenPodmust stay reachable only frominstantiateEtherFiNode. A second call site would let a target change after validators are funded. A comment on the resolver records this.Security fix
Pod-less request batches reject sources from different nodes (
MixedNodeRequest). The predeploys accept any pubkey from any caller, and the consensus layer silently drops requests whose source withdrawal address is not the caller. Unchecked, a batch would burn the fee, consume the rate limiter, and emit exit events for exits that never happened.requestConsolidationleaves the target unconstrained; it may live outside the node.EigenLayer v1.14.0 readiness
disablePod()podOwnerwithdrawDisabledPodETH_sweepToLiquidityPoolrequestConsolidation(owner-only once disabled)requestWithdrawalcompleteQueuedETHWithdrawalsNoCompleteableWithdrawalsdisablePodis timelock-gated because it is irreversible.Audit
audits/2026.08.25 - Certora - Non-EigenPod Validator Creation, EL Deprecation.pdf— Certora manual review at692d3f75. 0 Critical / High / Medium, 1 Low, 8 Informational.deployedEtherFiNodesis the sole trust anchor;backfillExistingEtherFiNodesdoes not verifysweepFunds(address)makes an unvalidated external call, can emit forgedFundsTransferredsrcPubkey == targetPubkeyconsolidations against pod-less nodes are CL no-ops, exempt from the rate limiter, still emit successFixes for I-03, I-04, I-06 and I-08 were written and then reverted: together they pushed
EtherFiNodesManagerpast the EIP-170 24,576-byte limit. They are operational or cosmetic, so they wait for a size reduction.Tests
Both new suites inherit
PreludeTest, so the 42 existing pod-backed tests run alongside as the old-regime regression signal.non-eigenpod-credentials.t.solnon-eigenpod-validator-lifecycle.t.solPodLess-Validator-Flows.t.sol/oracle-podless-funding.t.sol/disabled-pod-batch-guard.t.sol/forwarding-createpod-deny.t.solprelude/validator-key-genEL-withdrawals/Request-consolidation/Consolidation-through-EOACovers EIP-7002 full, partial and batch withdrawals; EIP-7251 switch-to-compounding, in-node consolidation, and consolidation to an external target; fee and batch rejection cases; both roles on both forwarding entrypoints; and the full lifecycle from spawner registration through oracle approval to a funded 32 ETH validator.
Verified by mutation: dropping the pod-less branch from the resolver fails 4 tests, including the full-flow test with
IncorrectBeaconRoot. That test builds expected credentials literally instead of reading them back from the resolver, so it can catch a resolver bug.Three repo gotchas found along the way:
forge test --force. Test artifacts embed implementation creation-bytecode vianew EtherFiNodesManager(...), and incremental compilation does not invalidate them, so a mutated implementation compiles but never reaches the fork.vm.expectRevertis consumed by a helper call in the argument list. Build deposit data into a local first.EtherFiNode.fallback()accepts any unknown selector silently. A missing function on the node surfaces as an ABI decode failure several frames away.Notes for review
EtherFiNodesManagersits close to the EIP-170 limit with no CI size gate. Reduction options are measured in the thread. Worth deciding before the next feature lands here.disablePodcannot be fork-tested against real code: v1.14.0 is not on mainnet, so the liveEigenPodManagerhas no such selector and no fallback, and the call reverts rather than silently succeeding. A no-op would be dangerous, letting us believe a pod was retired and consolidate out of a live pod. Signatures, predeploy addresses and calldata encoding were read from EigenLayer PR #1758's head.ValidatorWithdrawalRequestSent/ValidatorConsolidationRequestedkeep their signatures but now carry the credential target, which is the node for pod-less validators. Indexers assuming that field is an EigenPod need updating.sweepFunds(uint256 id)is unchanged. The address overload adds_validateNode; the id path does not, so legacy nodes never backfilled keep working.EigenPodrefunds to its caller.b4a09680:Validator-Flows.t.solandWithdraw.t.solfail insetUpwithNotRegistered();LiquidityPool.t.solfails 16/119. All mainnet drift from forking at latest block.EtherFiViewertosrc/archive/.Design doc:
docs/superpowers/specs/2026-08-07-non-eigenpod-withdrawal-credentials-design.md