feat(deposits): mint weETH directly on the ETH and WETH paths - #499
Open
seongyun-ko wants to merge 1 commit into
Open
feat(deposits): mint weETH directly on the ETH and WETH paths#499seongyun-ko wants to merge 1 commit into
seongyun-ko wants to merge 1 commit into
Conversation
The DepositAdapter took the eETH shares itself, converted them to an eETH amount, and wrapped that back into weETH. Both conversions floor, so the depositor received 1-2 wei less weETH than the shares minted for their deposit, and the difference accrued on the adapter as stranded eETH. It now credits the shares straight to the weETH contract via the new LiquidityPool.depositETHToRecipient, then claims them with WeETH.mintFor. Pooled ETH and total eETH shares move exactly as before, so the exchange rate and every existing holder's balance are untouched. Total shares already grew by the full share amount; the dust never left the ledger, it just sat on the adapter. The depositor now receives it. WEETH_MINTER_ROLE does not permit inflation: mintFor runs through the same _afterTokenTransfer backing check as every other mint, so a role holder cannot mint weETH that no eETH share backs. stETH and wstETH deposits are unchanged. They route through Liquifier.depositWithERC20, which hardcodes its caller as the eETH recipient, so redirecting those shares needs a Liquifier change too. Neither contract's constructor changed. Operations must grant the adapter LIQUIDITY_POOL_DEPOSIT_ADAPTER_ROLE and WEETH_MINTER_ROLE. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the ETH and WETH deposit flow through DepositAdapter to mint weETH directly from newly-minted eETH shares (credited to the weETH contract), eliminating the prior double-flooring round trip through wrap that stranded small eETH dust on the adapter.
Changes:
- Add role-gated direct-mint entrypoints:
LiquidityPool.depositETHToRecipient(credits eETH shares to a chosen recipient) andWeETH.mintFor(mints weETH against shares already held by weETH). - Update
DepositAdapterETH/WETH paths to usedepositETHToRecipient+mintForinstead of_wrapAndReturn. - Add a new mainnet-fork test suite covering rate invariance/backing/access control and update
TestSetupto grant the new roles to the adapter.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/TestSetup.sol | Grants LIQUIDITY_POOL_DEPOSIT_ADAPTER_ROLE and WEETH_MINTER_ROLE to DepositAdapter in fork setup. |
| test/DepositAdapterDirectMint.t.sol | New differential + invariance tests for direct minting on ETH/WETH paths (fork-based). |
| src/deposits/DepositAdapter.sol | Switch ETH/WETH deposit flows to credit shares directly to weETH and call mintFor. |
| src/core/WeETH.sol | Introduces WEETH_MINTER_ROLE and mintFor (still bounded by existing backing invariant). |
| src/core/LiquidityPool.sol | Introduces LIQUIDITY_POOL_DEPOSIT_ADAPTER_ROLE and payable depositETHToRecipient. |
| src/core/interfaces/IWeETH.sol | Adds mintFor(address,uint256) to interface. |
| src/core/interfaces/ILiquidityPool.sol | Adds depositETHToRecipient(address,address) to interface. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📊 Forge Coverage ReportGenerated by workflow run #814 |
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
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.
What this changes
ETH and WETH deposits now mint weETH directly instead of routing through
wrap.The adapter used to take the eETH shares itself, convert them to an eETH amount, and wrap that back into weETH. Both conversions floor, so the depositor received 1–2 wei less weETH than the shares that were minted for their deposit, and the difference piled up on the adapter as stranded eETH.
It now credits the shares straight to the weETH contract and mints against them:
stETH and wstETH deposits are unchanged. They go through
Liquifier.depositWithERC20, which hardcodes its own caller as the eETH recipient, so redirecting those shares would mean changing the Liquifier too. Out of scope here.What is provably unchanged
Every rate function in the protocol reads only
getTotalPooledEther()andeETH.totalShares(). Both move exactly as they did before — same_depositbody, same_sharesForDepositAmount, samenonDecreasingRateguard. The only difference is which address the shares land on.Total eETH shares already grew by the full
shareamount before this change; the 1–2 wei never left the ledger, it just sat on the adapter. So:getTotalPooledEther()eETH.totalShares()sharesForAmount/amountForShare/amountPerShareCeilweETH.getRate()testFuzz_directMintLeavesRateAndOtherHoldersUntouchedasserts all of this withassertEq, not tolerances, across deposits from 1 gwei to 100k ETH.What does change
The depositor receives 1–2 wei more weETH. This is the dust that previously stranded on the adapter, where it was recoverable by operations via
sweepDust. It is not diluted from any other holder — no other balance moves.testFuzz_directMintMatchesWrapPathWithinTwoWeiruns both paths from the same snapshot and bounds the difference at[0, 2].This wants an explicit decision rather than a silent merge. If the preference is byte-identical behaviour including the depositor's balance, the alternative is to reproduce both floors in
depositETHToRecipientand mint the remainder to the adapter as before. That is achievable but adds an ordering dependency, sincemintSharesitself callsamountForShare. Say the word and it can go in that direction.The event trail is shorter. The
LiquidityPool.Depositevent is byte-for-byte what it was — it deliberately emits the caller rather than the recipient so indexers keyed on the DepositAdapter keep working. The tokenTransferevents do change:Transfer(0 -> adapter), thenTransfer(adapter -> weETH)Transfer(0 -> weETH)Transfer(0 -> adapter), thenTransfer(adapter -> user)Transfer(0 -> user)Anything off-chain that identifies a user deposit by
weETH.Transfer(from = DepositAdapter)will stop seeing it — subgraphs, points attribution, tax tooling, accounting jobs. Foundry cannot catch this. The indexers need an audit before this ships.sweepDustbecomes vestigial for ETH and WETH. Keep it: the stETH and wstETH paths still strand dust, and the balance accrued to date still needs recovering.Why
mintForcannot inflate supplyWEETH_MINTER_ROLEis not a licence to mint.mintForroutes through the same_afterTokenTransferbacking check as every other mint, which reverts unlesstotalSupply <= eETH.shares(address(this)). A role holder that callsmintForwithout having credited the shares first reverts withWeETHUnderbacked— covered bytest_mintForRevertsWhenNotBacked.The role governs who may claim newly credited backing, not how much weETH may exist. Even a fully compromised DepositAdapter cannot mint weETH that no eETH share backs.
Operations
Two roles must be granted to the DepositAdapter, or every ETH and WETH deposit reverts:
LIQUIDITY_POOL_DEPOSIT_ADAPTER_ROLEon LiquidityPoolWEETH_MINTER_ROLEon WeETHNeither contract's constructor changed, so no deploy-script signature churn and no test-setup rewrites.
Forward note for the rate-limiter branch
This targets
master, where WeETH has no rate limiter. When the rate-limiting work lands,mintForneeds a deliberate decision on the globalWEETH_MINT_LIMIT_IDbucket.wrapsets_WRAP_CTX_SLOTto skip it, and the comment on that slot names "a new mint authority" as exactly the case the bucket is meant to catch. Either choice is defensible; it should not be made by accident:Testing
test/DepositAdapterDirectMint.t.sol, 11 tests on a mainnet fork:mintForrejects unbacked mints, unauthorised callers, and zero argumentsdepositETHToRecipientrejects unauthorised callersExisting
DepositAdapterTest,WeETHTest,LiquidityPoolTestandEETHTestall pass unchanged — 229 tests across the five suites.Full suite: 1417 passed. The failures are environmental or pre-existing:
liquid-testsOP suites needOP_RPC_URL, which was not set locallyWithdrawalSolvencyInvariantTestintermittently trips its own non-vacuity coverage gate ("no finalized request was ever claimed"), where the fuzzer does not drive a full request/finalize/claim lifecycle within its run budget. Measured over five random-seed runs per side: 5/5 pass with this change, 4/5 on cleanmaster. Three fixed-seed runs pass on both sides. The suite seeds actors throughliquidityPool.depositdirectly and never touches the DepositAdapter, so this change is not in its code path. Worth fixing separately — the gate is flaky onmastertoday.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Medium Risk
New role-gated mint/deposit entry points on core LiquidityPool and WeETH change the main ETH/WETH deposit path and token event shape; supply inflation is mitigated by the existing backing invariant but mis-granted roles or off-chain indexers are operational risks.
Overview
ETH and WETH deposits through DepositAdapter no longer take eETH on the adapter and wrap (double floor rounding). They call
LiquidityPool.depositETHToRecipientto mint backing eETH shares to weETH, thenWeETH.mintForto credit weETH to the depositor—typically 1–2 wei more than the old path, with no change to pool rate or other holders.LiquidityPool adds
LIQUIDITY_POOL_DEPOSIT_ADAPTER_ROLEand payabledepositETHToRecipient(same_depositaccounting asdeposit, recipient chosen by the role holder). WeETH addsWEETH_MINTER_ROLEandmintFor, still bounded by the existing_afterTokenTransferbacking check. stETH/wstETH paths still use_wrapAndReturn.Interfaces, TestSetup role grants for the adapter, and
test/DepositAdapterDirectMint.t.solcover rate invariance, backing, and access control. weETH/eETHTransfertopology changes (mint goes adapter → user directly); indexers keyed on adapter-as-intermediate may need review. Production must grant both new roles to DepositAdapter.Reviewed by Cursor Bugbot for commit b7af8a7. Configure here.