Test + docs: pin withdraw(_amountOfEEth) accounting (follow-up to #454)

[seongyun-ko]

Stacked on #454. Addresses the four review findings on that PR without changing its behavior.

#454 makes LiquidityPool.withdraw debit totalValueOutOfLp by _amountOfEEth (the value credited at fulfill/lock) instead of _amount (the ETH actually paid on claim). That fix is correct — but it shipped without a guard that distinguishes the two, and left a few stale comments. This PR closes those gaps.

Changes

1. Regression test (the main gap). Every existing withdraw test passes the two amounts equal (withdraw(amount, amount, ...)), so none would have failed against the pre-fix code. test_withdraw_debitsAmountOfEEth_notAmountPaid pins the divergent down-rebase case (_amount < _amountOfEEth) at the LP boundary where the bug lived: it asserts the debit equals the fulfill-time credit (_amountOfEEth) and explicitly does not track the smaller ETH paid. Runs offline (no fork).

2. _amountOfEEth natspec. Documents that it must equal the fulfill-time credit (not _amount), how WRN and PWQ each supply it, and that — unlike _amount/_rate/_shareOfEEth — it is trusted caller-supplied state not bounded by Guards 1–3. The checked subtraction is the only LP-local backstop.

3. Negative-rebase underflow note. Records that the fix shifts the documented finalized-withdrawal DoS threshold from _amount to the (larger) _amountOfEEth, and fixes the stale LiquidityPool.sol:297 / _amount reference in the LpRebaseWrnClaimUnderflow finding header. Bounded by the rebase-APR cap in EtherFiAdmin.

4. Stale comment in WithdrawEscrowE2E step4. The comment said totalValueOutOfLp "decrements by claimable (the actual ETH paid)" — that's the pre-fix behavior. Corrected to describe the fulfill-time-credit debit and to point at the new LP-boundary regression test for the divergent case.

Verification

forge build clean. Full withdraw suite (18 tests incl. the new one) passes offline:

Suite result: ok. 18 passed; 0 failed; 0 skipped

The WithdrawEscrowE2E change is comment-only (that file is a mainnet-fork test).

---

Note

Low Risk
No production logic changes—only documentation, comments, and a unit regression test for accounting already fixed in #454.

Overview
Follow-up to #454 (no change to LiquidityPool.withdraw behavior): it documents and tests that totalValueOutOfLp must be reduced by _amountOfEEth (fulfill/lock credit), not _amount (ETH paid on claim).

LiquidityPool.sol natspec clarifies _amount vs _amountOfEEth, that _amountOfEEth is trusted caller state outside Guards 1–3, and that a negative rebase can DoS finalized claims if totalValueOutOfLp falls below that debit (bounded by admin rebase APR).

test_withdraw_debitsAmountOfEEth_notAmountPaid is a new offline regression where _amount < _amountOfEEth (down-rebase-style) and asserts the LP debit matches the fulfill credit.

Comment fixes: LpRebaseWrnClaimUnderflow header now references _amountOfEEth; WithdrawEscrowE2E step4 comment matches fulfill-credit accounting and points at the new LP test for the divergent case.

^{Reviewed by Cursor Bugbot for commit acc4795. 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.