Harden Lido withdrawal claims#242
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR tightens access control around claiming Lido withdrawals and prevents wrapping unrelated pre-existing ETH balances when converting claimed ETH to WETH.
Changes:
- Restrict
claimLidoWithdrawalsto operator/owner and add a regression test for unauthorized callers. - Track pre-claim ETH balance and wrap only the ETH received from the claim (not pre-existing/donated ETH).
- Adjust a test utility (
ETHSender.sendETH) to preserve a target’s existing ETH balance when “sending” viavm.deal.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/fork/utils/MockCall.sol | Updates ETH test helper to add to (rather than overwrite) a target’s ETH balance. |
| test/fork/LidoARM/ClaimStETHWithdrawalForWETH.t.sol | Adds tests for unauthorized access and for not wrapping pre-existing ETH donations. |
| src/contracts/LidoARM.sol | Adds onlyOperatorOrOwner gating and wraps only newly received ETH from Lido claims. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| function sendETH(address target) external { | ||
| vm.deal(target, address(this).balance); | ||
| vm.deal(target, target.balance + address(this).balance); |
| external | ||
| onlyOperatorOrOwner | ||
| { | ||
| uint256 ethBalanceBefore = address(this).balance; |
| // Wrap the ETH received from this claim. This can be less than the requested amount | ||
| // in the event of slashing, and ignores ETH donated before the claim. |
TUPM96
force-pushed
the
codex/lido-claim-access-control-241
branch
from
49e7ac3 to
76b5c9a
Compare
|
Follow-up pushed in Revalidated locally:
|
TUPM96
force-pushed
the
codex/lido-claim-access-control-241
branch
from
76b5c9a to
cf6d3ce
Compare
|
It was a design decision to allow anyone to claim Lido withdrawals. That way, if the Operator or Owner stopped, the ARM LPs could still get the liquidity back into the ARM so they could claim redeem requests. We also want any donated ETH to be included in the ARM's assets. |
Summary
onlyOperatorOrOwnertoLidoARM.claimLidoWithdrawals()so claim timing stays under the same operator/owner control asrequestLidoWithdrawals().address(this).balance, so pre-existing donated ETH is not swept into WETH during a claim.Fixes #241.
Validation
MAINNET_URL=https://ethereum-rpc.publicnode.com C:\Users\tupm96\.foundry\bin\forge.exe test --match-path test/fork/LidoARM/ClaimStETHWithdrawalForWETH.t.sol --summary -vvv-> 5 passedC:\Users\tupm96\.foundry\bin\forge.exe build --skip test script --force-> success, existing warnings onlyC:\Users\tupm96\.foundry\bin\forge.exe fmt --check src/contracts/LidoARM.sol test/fork/LidoARM/ClaimStETHWithdrawalForWETH.t.sol test/fork/utils/MockCall.sol-> successgit diff --check-> cleanNo private keys, wallet secrets, payout details, private vulnerability details, or mainnet transactions are included.