P0 security hardening for #385: oracle price-move cap + restaker pause

[seongyun-ko]

P0 security hardening for #385

Two high-priority fixes from the security review. Each is small, isolated, and has a test. Base: pankaj/feat/security-upgrades.

---

1. Cap how much a single oracle report can move the weETH price (EtherFiAdmin)

The problem, in plain terms: the oracle periodically reports staking rewards, which moves the weETH↔eETH exchange rate. There's a safety check on this — but it's expressed as an annual rate (APR). The catch: the longer the gap a report covers, the bigger the one-time jump it's allowed to make while still looking fine on an annual basis.

Example: a normal daily report nudges the rate by ~0.01%. But a report that covers 60 days (say the oracle was paused and is catching up) is allowed to move the rate ~60× more in a single transaction — and still pass the APR check. A buggy or compromised oracle could exploit exactly this to inflate or crash the price in one shot. No other guard catches it, because a price change doesn't mint or burn any tokens.

The fix: add a hard ceiling — no single report may move total value by more than 2% of TVL, no matter how long a window it covers. Normal reports are nowhere near this; anything bigger now has to be done deliberately by governance instead of sailing through automatically.

• 200 bps is a constant for now. If you'd rather make it adjustable, say so and I'll wire it as a governance setting with a fixed upper bound.
• A full report-level test belongs in the oracle test suite (it has the report builders). The arithmetic is simple; happy to add it if you point me at the harness.

---

2. Make the restaker's "pause" button actually do something (EtherFiRestaker)

The problem: pauseContract() flips a paused flag — but not one function actually checks that flag. So hitting pause did nothing: stETH transfers, ETH withdrawals, EigenLayer deposits/withdrawals all kept working. An operator who thought they'd stopped the contract during an incident hadn't.

Example: during an incident someone calls pauseContract() to freeze the restaker. An attacker (or a compromised ops key) calls withdrawEther() one block later — it still goes through, because nothing was checking "are we paused?".

The fix: every money-moving function now respects the pause. Also added guardianPause() so the Guardian (our Hypernative automation / emergency keys) can hit the brake instantly without gathering multisig signatures. Unpausing still requires the multisig (deliberate to turn off, easy to turn on — the intended asymmetry).

• Test test_guardianPause_halts_fund_movement confirms the pause now blocks transfers, that only the Guardian can fire it, and that resume is multisig-only. Passes on a mainnet fork.

---

Test status

• New test passes on a mainnet fork.
• The pre-existing EtherFiOracleTest::test_finalizeWithdrawalsWhenStale_* failures (latest-block fork drift) reproduce identically on the untouched base branch — unrelated to this change.

---

Note

High Risk
Changes oracle report validation and the LiquidityPool share-rate rebase chokepoint, which directly affect eETH/weETH pricing; restaker pause wiring controls stETH/EigenLayer/ETH flows during incidents.

Overview
This PR adds defense-in-depth limits on how much TVL can move in a single rebase/report, and makes EtherFiRestaker pauses actually block fund flows, aligned with the rest of the protocol.

Rebase / oracle bounds: LiquidityPool.rebase now rejects positive reward accrual above a fixed 25 bps of pre-rebase TVL (RebaseExceedsPositiveCap). EtherFiAdmin adds a separate per-report negative (slashing) cap (default 3 bps, overridable via setMaxNegativeRebaseBps behind admin) so long-gap reports cannot pass the annualized APR check while still dropping TVL by a large absolute amount. Positive reward limits stay on the LP chokepoint; negative limits stay on report validation.

Restaker pause: Money-moving paths on EtherFiRestaker use whenNotPaused. The contract inherits PausableUntil: Guardian pauseContractUntil, multisig boolean pause/unpause, and _requireNotPaused also enforces the timed halt. PausableUntil._pauseUntil falls back to MIN_PAUSE_DURATION when duration was never set, avoiding a same-block no-op pause.

Tests / harness: Rebase-related tests use smaller accruals within the new cap; new tests cover the positive cap and restaker halts. TestSetup relaxes negative cap for generic suites via setMaxNegativeRebaseBps(10_000).

^{Reviewed by Cursor Bugbot for commit 019e84f. Bugbot is set up for automated code reviews on this repo. Configure here.}

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[0xpanicError]

Let's set a min value per report to 3 bps which represents the max initial slashing penalty

[0xpanicError]

Rationale: initial slashing penalty is maxEffBalance / 4096, which is around 2.44 bps of tvl in case all validators inccur it. We should make this value configurable behing operatingTimelock so it can later be increased if we detect 18 days mid term slashing penalty event (can be detected way before 2 day timelock).

[seongyun-ko]

Done in dfac525. Implemented exactly this on the negative side: a slashing cap on accruedRewards defaulting to 3 bps (DEFAULT_MAX_NEGATIVE_REBASE_BPS), overridable via setMaxNegativeRebaseBps behind the operating timelock (0 = use default). So it ships tight at 3 bps and can be raised if a correlated/mid-term slashing event is detected. The old symmetric APR-only abs check is gone.

[0xpanicError]

Let's only compare accruedRewards per report with max_neg_rebase value (which we can set to 3 bps). to put invariant on upper-bound, better to put inside LP on functions expected to increase share rate and can set max upper-bound to something like 25 bps (that's 1 month of reward accrual at 3% APR).

[seongyun-ko]

Done in dfac525. The negative side now only compares accruedRewards against the max-negative cap in EtherFiAdmin (default 3 bps). The positive/upper bound moved into LiquidityPool.rebase — the share-rate-increasing chokepoint — defaulting to 25 bps (DEFAULT_MAX_POSITIVE_REBASE_BPS ≈ 1 month at 3% APR), settable behind the operating timelock via setMaxPositiveRebaseBps. Enforced there it holds regardless of which contract calls rebase. Test: test_positiveRebaseCap_enforced.

[0xpanicError]

why not use pausable until here?

[0xpanicError]

having the same function being called by 2 tiers of roles break the purpose of those roles.

[seongyun-ko]

Good call — switched to PausableUntil in dfac525. The restaker now mirrors the protocol-wide model: pauseContractUntil() (onlyGuardian, auto-expiring 8h–30d with the per-pauser cooldown) + the boolean multisig pauseContract(), and a whenNotHalted modifier gates every fund-flow fn (incl. undelegate) on both. Replaced the OZ-only guardianPause I had. Tests updated (test_guardianPauseUntil_halts_fund_movement, test_booleanPause_also_halts_fund_movement).

[seongyun-ko]

Resolved by the PausableUntil switch: pausing is now two distinct functions on two tiers — pauseContract() (multisig, boolean) and pauseContractUntil() (Guardian, auto-expiring). No single function is callable by two role tiers anymore.

[0xpanicError]

I don't think we need this value to be configurable. There's no way a single report should increase rate by more than 25 bps.

[seongyun-ko]

Agreed — made it a hard constant in 019e84f. MAX_POSITIVE_REBASE_BPS = 25, no setter/storage. (The negative/slashing cap in EtherFiAdmin stays timelock-configurable, since a real correlated-slashing event can legitimately exceed the initial-penalty default.) Rewrote the ~11 accounting tests that were doing 5%–100% rebases on tiny test TVL to use within-cap rebases.

[0xpanicError]

can you use the whenNotPaused modifier used everywhere else for consistency? For contracts like this where PausableUpgradeable is inherited, override this internal function:

function _requireNotPaused() internal override view {
        _requireNotPausedUntil();
        super._requireNotPaused();
    }

[seongyun-ko]

Done in 019e84f — exactly your pattern: overrode _requireNotPaused() to call _requireNotPausedUntil() + super._requireNotPaused(), dropped the bespoke whenNotHalted, and all fund-flow fns use the standard whenNotPaused again.

[0xpanicError]

can you rename this: unpauseContract()

[seongyun-ko]

Renamed to unpauseContract() in 019e84f (+ IEtherFiRestaker).

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 019e84f. Configure here.}

[cursor]

Override blocks boolean pause while PausableUntil is active

Medium Severity

The _requireNotPaused() override checks _requireNotPausedUntil() first, which reverts when the PausableUntil pause is active. Since OZ's _pause() uses the whenNotPaused modifier internally, calling pauseContract() while the Guardian's auto-expiring halt is active will revert. This means the multisig cannot "promote" the temporary halt to a permanent boolean pause without first unpausing PausableUntil — creating a potential gap window between the two operations where the contract is momentarily unpaused and fund-moving transactions could execute.

 

^{Reviewed by Cursor Bugbot for commit 019e84f. Configure here.}