fix(FlashMintDexV5): redeem-side WETH accounting must use balance delta - #225
Merged
Merged
Conversation
Adds a regression spec that issues 1 uSOL3x via the working two-component path, then attempts to redeem it back to WETH using `noopSwap` for the USDC dust component. Currently reverts with `SafeERC20: low-level call failed` — DEXAdapterV5 short-circuits an empty-path swap by returning `_amountIn`, and `_sellComponentsForWeth` sums those returns into `totalWethReceived` as if they were WETH. The bridge swap then over-quotes by exactly the dust amount and the SafeERC20 transferFrom inside the UniV3 swap path reverts. Test currently fails (expected). Fix is in the next commit.
ckoopmann
changed the title
Rewrites _sellComponentsForWeth to read WETH delta over the loop instead of
summing per-component swap return values, swap min(componentUnits[i],
balanceOf(component)) instead of trusting the issuance module's view-derived
target, and skip components whose effective amount is 0.
Closes three related redeem-side bugs:
A. Noop-swap inflation. DEXAdapterV5.swapExactTokensForTokens short-
circuits an empty path by returning _amountIn unchanged. Pre-fix
_sellComponentsForWeth summed those into totalWethReceived as if they
were WETH, so any caller passing noopSwap for a non-zero component
inflated the bridge total by the input amount in the input token's
units → STF / SafeERC20: low-level call failed at the WETH→output
bridge. Reproduced in the spec from the previous commit.
B. Pre/post-sync drift on dust components. DebtIssuanceModuleV3.redeem's
internal sync + tokenTransferBuffer clamp can flip the pull direction
by ±1 wei vs the SDK's pre-quote view. Pre-fix the contract called
swap(componentUnits[i], …) with the stale target → ERC20: transfer
amount exceeds balance. Post-fix it caps at actual balance. (Verified
via SDK e2e once the fix ships.)
C. Zero-amount components crash UniV3 / Aerodrome routers. Pre-fix the
SDK substituted noopSwap to dodge this; combined with bug A above
that workaround was unsound for non-zero amounts. Post-fix the
contract skips 0-amount components internally, so the SDK
substitution becomes optional. (Verified via SDK e2e once the fix
ships.)
Issue side is unchanged — the deficit buffer (#223) and
_syncExternalPositions (#224) already cover the analogous drift there, and
the issue path doesn't have a noopSwap step so bug A doesn't apply.
|
🎉 This PR is included in version 0.45.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
1 participant
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.
Summary
Three related redeem-side bugs in
FlashMintDexV5, all rooted in the same assumption: that_sellComponentsForWethcan trust the per-component swap amounts and return values fromDEXAdapterV5.swapExactTokensForTokensrather than the contract's actual token balances. Switching to balance-based accounting (read WETH delta over the loop, swapmin(target, actualBalance)per component, skip 0-amount components) closes all three.The three bugs
A. Noop-swap inflation (tested in this PR).
DEXAdapterV5.swapExactTokensForTokensshort-circuits an empty path by returning_amountInunchanged (DEXAdapterV5.sol:114-116). When the caller passesnoopSwap(e.g. to leave a USDC dust component as residue rather than route a tiny ~11_000 wei swap through Uniswap V3),_sellComponentsForWethsums that USDC_amountInintototalWethReceivedas if it were WETH. The downstream_swapWethForPaymentTokenthen over-quotes the WETH→output bridge by exactly the dust amount and reverts withSTF/SafeERC20: low-level call failed.B. Pre/post-sync component drift on dust component (verified later via SDK e2e). SDK reads pre-sync
componentUnits[i]=11 weifrom the issuance module's view;DebtIssuanceModuleV3.redeem(...)syncs first and the V3tokenTransferBufferclamp can flip the pull direction by ±1 wei. Contract ends up holding 10 wei but the swap call passes 11 →ERC20: transfer amount exceeds balanceat the component swap'stransferFrom. This is the long-standing uSUI3x@1 redeem failure documented in the SDK'stest-scenarios.ts.C. Zero-amount components crash the underlying router (verified later via SDK e2e). When
getRequiredComponentRedemptionUnitsreturns 0 for a dust component,swapExactTokensForTokens(0, ...)is rejected by UniV3 / Aerodrome's quoter. Currently worked around in the SDK byresolveDexV5SwapDataForAmountsubstitutingnoopSwap; with the contract-side fix the SDK substitution becomes optional.What this PR does
noopSwapfor the USDC dust component. CI on this commit should be red withSafeERC20: low-level call failed._sellComponentsForWethto use balance-based accounting, swapmin(componentUnits[i], balanceOf(component)), and skip 0-amount components. CI on this commit should be green for the new spec and all existing FlashMintDexV5 specs.Bugs B and C cannot be cleanly reproduced inside
index-coop-smart-contractsintegration tests at the current pinned fork block (B needs a uSUI3x whale to source SetTokens since uSUI3x@1 issuance can't construct an 11-wei UniV3 swap; C needs a setAmount small enough to trigger thetokenTransferBufferclamp, which on uSOL3x is sub-attomole). They will be verified in the SDK e2e suite once the fix lands and the SDK'sFlashMintDexV5constant is bumped to the new deployment.Test plan
test/integration/base/flashMintDexV5.spec.ts › regression: redeem-side noopSwap on dust component must not inflate WETH accountingreverts on commit 1 (red — expected).FlashMintDexV5address, re-enables uSUI3x@1 intest-scenarios.ts, and confirms via e2e that bugs B and C are gone.