refactor: leverage deleverage hooks#441
Merged
antoncoding merged 5 commits intomasterfrom Mar 8, 2026
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds new swap and ERC4626 leverage/deleverage flows, shared transaction utilities (Velora/Bundler3 helpers, transaction-shared modules), refactors leverage/deleverage hooks to delegate to those flows, updates slippage/math and quote-preview shapes, and includes small UI/markup cleanups and agent rules. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Hook as Leverage/Deleverage Hook
participant Velora as Velora/ParaSwap
participant Bundler as Bundler (Morpho)
participant Protocol as ERC4626/Adapters
rect rgba(100,150,200,0.5)
Note over User,Protocol: Leverage (swap or ERC4626)
User->>Hook: start flow (params, route, auth)
Hook->>Velora: build & verify price route/payload
Hook->>Hook: ensure bundler authorization / Permit2
Hook->>Bundler: send multicall (flash loan + callback)
Bundler->>Protocol: execute callback (swap/deposit/morpho ops)
Bundler->>Hook: sweep residuals to user
end
rect rgba(200,150,100,0.5)
Note over User,Protocol: Deleverage (swap or ERC4626)
User->>Hook: start deleverage (repay params)
Hook->>Hook: compute repay bounds & validate quotes
Hook->>Hook: ensure bundler authorization (sig/tx)
Hook->>Bundler: send multicall (repay, withdraw, swap/redeem)
Bundler->>Protocol: execute redeem/swap callbacks
Bundler->>Hook: sweep leftover assets to user
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
the
feature request
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hooks/useDeleverageTransaction.ts (1)
64-88:⚠️ Potential issue | 🟠 MajorFail closed before calling the authorization hook.
bundlerAddresscan now beundefined, butuseBundlerAuthorizationStep()still receivesauthorizationTarget as Address. That hides a missing config behind a type assertion, and the later toast guard is too late because the hook has already been invoked.As per coding guidelines: Route spender/permit/authorization logic through shared chokepoints; fail closed when readiness or config is missing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDeleverageTransaction.ts` around lines 64 - 88, The code calls useBundlerAuthorizationStep({ chainId: market.morphoBlue.chain.id, bundlerAddress: authorizationTarget as Address }) even when bundlerAddress (and thus authorizationTarget) can be undefined; remove the type assertion and fail closed by only invoking useBundlerAuthorizationStep when a valid bundlerAddress exists (or when route.kind === 'swap'), e.g., compute authorizationTarget safely and conditionally call useBundlerAuthorizationStep so that bundlerAddress/authorizationTarget is validated before passing into the hook (refer to bundlerAddress, authorizationTarget, and useBundlerAuthorizationStep to locate the code), and ensure downstream logic handles the undefined/absent-authorization case via early returns or disabled flows.src/hooks/useLeverageTransaction.ts (1)
66-72:⚠️ Potential issue | 🟠 MajorDon't default
route === nullinto the ERC4626 branch.A missing route still falls through to
resolveErc4626RouteBundler(...). That wires the hook up with a bundler/approval target before a route exists, and any resolver miss turns an unsupported market into a render-time failure instead of a safe no-op.As per coding guidelines: Route spender/permit/authorization logic through shared chokepoints; fail closed when readiness or config is missing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLeverageTransaction.ts` around lines 66 - 72, The bundlerAddress memo currently falls back to resolveErc4626RouteBundler when route is null, which wires the hook before a route exists; update useLeverageTransaction's bundlerAddress logic to fail closed by returning undefined (or null) when route is missing: inside the useMemo for bundlerAddress, first check if (!route) return undefined; then if (route.kind === 'swap') return route.bundler3Address; otherwise call resolveErc4626RouteBundler(market.morphoBlue.chain.id, market.uniqueKey). Keep the same deps ([route, market.uniqueKey, market.morphoBlue.chain.id]) and reference bundlerAddress and resolveErc4626RouteBundler in your change.
🧹 Nitpick comments (1)
src/hooks/useSupplyMarket.ts (1)
223-238: Minor: redundant variable assignment.
minSharesSlippageAmountjust copies the constant. Could useMIN_SHARES_SLIPPAGE_AMOUNTdirectly:- const minSharesSlippageAmount = MIN_SHARES_SLIPPAGE_AMOUNT; ... - minSharesSlippageAmount, + MIN_SHARES_SLIPPAGE_AMOUNT,Not blocking - just slightly cleaner.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useSupplyMarket.ts` around lines 223 - 238, The local variable minSharesSlippageAmount is redundant because it simply duplicates the constant MIN_SHARES_SLIPPAGE_AMOUNT; remove the local declaration and replace uses of minSharesSlippageAmount with MIN_SHARES_SLIPPAGE_AMOUNT where constructing the morphoSupply call (see encodeFunctionData and the morphoSupply args) to keep the code cleaner and avoid an unnecessary assignment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/deleverage/deleverageWithErc4626Redeem.ts`:
- Around line 84-107: The current loanAssetsOutSlippageFloor computed via
withSlippageFloor(flashLoanAmount, slippageBps) can be below flashLoanAmount in
close mode causing erc4626Redeem to return too little to cover the flash loan;
change the logic so loanAssetsOutSlippageFloor = max(flashLoanAmount,
withSlippageFloor(flashLoanAmount, slippageBps)) (or if useCloseRoute is true,
set it to flashLoanAmount) and use that value in the erc4626Redeem args; also
ensure the callbackTxs bundle includes a final explicit sweep transfer from the
bundler/adapter balances to the intended recipient (bundlerAddress/account) so
execute-time asset routing matches quote-time slippage assumptions.
In `@src/hooks/leverage/leverageWithErc4626Deposit.ts`:
- Around line 17-40: The helper leverageWithErc4626Deposit is not receiving the
caller's slippageBps so erc4626Deposit() calls that wrap amounts with
withSlippageFloor(...) use a default tolerance; update the helper signature to
accept slippageBps and thread it through to every call site that constructs
slippage bounds (notably the erc4626Deposit(...) invocations that call
withSlippageFloor(...)), and ensure the trailing asset sweep logic also uses the
same slippageBps; apply the same change for the other occurrences referenced
around the 135-152 region so quote-time slippageBps is preserved through
execute-time bounds.
In `@src/hooks/useLeverageTransaction.ts`:
- Around line 381-410: runLeverageFlow currently calls tracking.start() before
performing preflight checks in executeLeverage(), so early-return branches in
executeLeverage() can bypass tracking.complete()/tracking.fail() and leave the
UI stuck; move all synchronous preflight validation/toast checks out of
executeLeverage() and perform them inside runLeverageFlow before calling
tracking.start(), then call tracking.start(getStepsForFlow(...), ...) only when
preflight passes; also ensure the remainder of executeLeverage() returns a clear
success/failure result (or throws) so runLeverageFlow can call
tracking.update(...) only when advancing to a strictly later step and finally
call tracking.complete() on success or tracking.fail() on any failure (catch),
referencing runLeverageFlow, executeLeverage, tracking.start, tracking.update,
tracking.complete, tracking.fail, and getStepsForFlow.
---
Outside diff comments:
In `@src/hooks/useDeleverageTransaction.ts`:
- Around line 64-88: The code calls useBundlerAuthorizationStep({ chainId:
market.morphoBlue.chain.id, bundlerAddress: authorizationTarget as Address })
even when bundlerAddress (and thus authorizationTarget) can be undefined; remove
the type assertion and fail closed by only invoking useBundlerAuthorizationStep
when a valid bundlerAddress exists (or when route.kind === 'swap'), e.g.,
compute authorizationTarget safely and conditionally call
useBundlerAuthorizationStep so that bundlerAddress/authorizationTarget is
validated before passing into the hook (refer to bundlerAddress,
authorizationTarget, and useBundlerAuthorizationStep to locate the code), and
ensure downstream logic handles the undefined/absent-authorization case via
early returns or disabled flows.
In `@src/hooks/useLeverageTransaction.ts`:
- Around line 66-72: The bundlerAddress memo currently falls back to
resolveErc4626RouteBundler when route is null, which wires the hook before a
route exists; update useLeverageTransaction's bundlerAddress logic to fail
closed by returning undefined (or null) when route is missing: inside the
useMemo for bundlerAddress, first check if (!route) return undefined; then if
(route.kind === 'swap') return route.bundler3Address; otherwise call
resolveErc4626RouteBundler(market.morphoBlue.chain.id, market.uniqueKey). Keep
the same deps ([route, market.uniqueKey, market.morphoBlue.chain.id]) and
reference bundlerAddress and resolveErc4626RouteBundler in your change.
---
Nitpick comments:
In `@src/hooks/useSupplyMarket.ts`:
- Around line 223-238: The local variable minSharesSlippageAmount is redundant
because it simply duplicates the constant MIN_SHARES_SLIPPAGE_AMOUNT; remove the
local declaration and replace uses of minSharesSlippageAmount with
MIN_SHARES_SLIPPAGE_AMOUNT where constructing the morphoSupply call (see
encodeFunctionData and the morphoSupply args) to keep the code cleaner and avoid
an unnecessary assignment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f49e9097-3553-4734-ae8d-e8a5d82e9bd7
📒 Files selected for processing (18)
app/tools/page.tsxsrc/hooks/deleverage/deleverageWithErc4626Redeem.tssrc/hooks/deleverage/deleverageWithSwap.tssrc/hooks/deleverage/transaction-shared.tssrc/hooks/leverage/bundler3.tssrc/hooks/leverage/leverageWithErc4626Deposit.tssrc/hooks/leverage/leverageWithSwap.tssrc/hooks/leverage/math.tssrc/hooks/leverage/transaction-shared.tssrc/hooks/leverage/velora-transaction.tssrc/hooks/useBorrowTransaction.tssrc/hooks/useDeleverageTransaction.tssrc/hooks/useLeverageTransaction.tssrc/hooks/useMultiMarketSupply.tssrc/hooks/useSupplyMarket.tssrc/modals/borrow/components/withdraw-collateral-and-repay.tsxsrc/modals/leverage/components/add-collateral-and-leverage.tsxsrc/modals/leverage/components/remove-collateral-and-deleverage.tsx
Comment on lines
+84
to
+107
| const loanAssetsOutSlippageFloor = withSlippageFloor(flashLoanAmount, slippageBps); | ||
| const callbackTxs: `0x${string}`[] = [ | ||
| encodeFunctionData({ | ||
| abi: morphoBundlerAbi, | ||
| functionName: 'morphoRepay', | ||
| args: [ | ||
| marketParams, | ||
| useCloseRoute ? 0n : flashLoanAmount, | ||
| useCloseRoute ? repayBySharesAmount : 0n, | ||
| bundlerV2RepaySlippageAmount, | ||
| account, | ||
| '0x', | ||
| ], | ||
| }), | ||
| encodeFunctionData({ | ||
| abi: morphoBundlerAbi, | ||
| functionName: 'morphoWithdrawCollateral', | ||
| // Withdraw ERC4626 shares onto the bundler because the same bundler multicall redeems them immediately. | ||
| args: [marketParams, withdrawCollateralAmount, bundlerAddress], | ||
| }), | ||
| encodeFunctionData({ | ||
| abi: morphoBundlerAbi, | ||
| functionName: 'erc4626Redeem', | ||
| args: [route.collateralVault, withdrawCollateralAmount, loanAssetsOutSlippageFloor, bundlerAddress, bundlerAddress], |
There was a problem hiding this comment.
Close-route redeem needs a hard floor of flashLoanAmount.
With non-zero slippage, withSlippageFloor(flashLoanAmount, slippageBps) is below flashLoanAmount. In close mode that lets erc4626Redeem() succeed while still returning too little to settle the flash loan, so the bundle reverts at the end.
Possible fix
- const loanAssetsOutSlippageFloor = withSlippageFloor(flashLoanAmount, slippageBps);
+ const loanAssetsOutSlippageFloor = useCloseRoute
+ ? flashLoanAmount
+ : withSlippageFloor(flashLoanAmount, slippageBps);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const loanAssetsOutSlippageFloor = withSlippageFloor(flashLoanAmount, slippageBps); | |
| const callbackTxs: `0x${string}`[] = [ | |
| encodeFunctionData({ | |
| abi: morphoBundlerAbi, | |
| functionName: 'morphoRepay', | |
| args: [ | |
| marketParams, | |
| useCloseRoute ? 0n : flashLoanAmount, | |
| useCloseRoute ? repayBySharesAmount : 0n, | |
| bundlerV2RepaySlippageAmount, | |
| account, | |
| '0x', | |
| ], | |
| }), | |
| encodeFunctionData({ | |
| abi: morphoBundlerAbi, | |
| functionName: 'morphoWithdrawCollateral', | |
| // Withdraw ERC4626 shares onto the bundler because the same bundler multicall redeems them immediately. | |
| args: [marketParams, withdrawCollateralAmount, bundlerAddress], | |
| }), | |
| encodeFunctionData({ | |
| abi: morphoBundlerAbi, | |
| functionName: 'erc4626Redeem', | |
| args: [route.collateralVault, withdrawCollateralAmount, loanAssetsOutSlippageFloor, bundlerAddress, bundlerAddress], | |
| const loanAssetsOutSlippageFloor = useCloseRoute | |
| ? flashLoanAmount | |
| : withSlippageFloor(flashLoanAmount, slippageBps); | |
| const callbackTxs: `0x${string}`[] = [ | |
| encodeFunctionData({ | |
| abi: morphoBundlerAbi, | |
| functionName: 'morphoRepay', | |
| args: [ | |
| marketParams, | |
| useCloseRoute ? 0n : flashLoanAmount, | |
| useCloseRoute ? repayBySharesAmount : 0n, | |
| bundlerV2RepaySlippageAmount, | |
| account, | |
| '0x', | |
| ], | |
| }), | |
| encodeFunctionData({ | |
| abi: morphoBundlerAbi, | |
| functionName: 'morphoWithdrawCollateral', | |
| // Withdraw ERC4626 shares onto the bundler because the same bundler multicall redeems them immediately. | |
| args: [marketParams, withdrawCollateralAmount, bundlerAddress], | |
| }), | |
| encodeFunctionData({ | |
| abi: morphoBundlerAbi, | |
| functionName: 'erc4626Redeem', | |
| args: [route.collateralVault, withdrawCollateralAmount, loanAssetsOutSlippageFloor, bundlerAddress, bundlerAddress], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/deleverage/deleverageWithErc4626Redeem.ts` around lines 84 - 107,
The current loanAssetsOutSlippageFloor computed via
withSlippageFloor(flashLoanAmount, slippageBps) can be below flashLoanAmount in
close mode causing erc4626Redeem to return too little to cover the flash loan;
change the logic so loanAssetsOutSlippageFloor = max(flashLoanAmount,
withSlippageFloor(flashLoanAmount, slippageBps)) (or if useCloseRoute is true,
set it to flashLoanAmount) and use that value in the erc4626Redeem args; also
ensure the callbackTxs bundle includes a final explicit sweep transfer from the
bundler/adapter balances to the intended recipient (bundlerAddress/account) so
execute-time asset routing matches quote-time slippage assumptions.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hooks/useDeleverageTransaction.ts (1)
150-175: 🛠️ Refactor suggestion | 🟠 MajorDo sync deleverage preflight before
tracking.start().
authorizeAndDeleverage()still opens the stepper before the zero-input, stale-limit, missing-quote, and missing-close-share checks insideexecuteDeleverage(). Those branches fail cleanly now, but they still show a failed transaction for a local validation error. Mirror the leverage-side preflight pattern here and calltracking.start(...)only after preflight passes.Also applies to: 241-283
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDeleverageTransaction.ts` around lines 150 - 175, authorizeAndDeleverage currently calls tracking.start() before executeDeleverage's local preflight checks, causing the stepper to open for synchronous validation errors; move the synchronous preflight validation (the checks for account, route, bundlerAddress, withdrawCollateralAmount/flashLoanAmount > 0n, withdrawCollateralAmount <= maxWithdrawCollateralAmount, and useCloseRoute implies repayBySharesAmount > 0n, plus presence of swapSellPriceRoute when route.kind === 'swap') to run immediately inside authorizeAndDeleverage (or invoke executeDeleverage's preflight helper) and only call tracking.start(...) after these validations succeed, mirroring the leverage-side preflight pattern so the stepper is started only for real on-chain flows and not for local validation failures.src/hooks/useLeverageTransaction.ts (1)
80-93:⚠️ Potential issue | 🟠 MajorDon't resolve an ERC4626 bundler before the route exists.
The fallback branch still calls
resolveErc4626RouteBundler(...)whenrouteisnull, so unsupported or unresolved markets can throw during render before preflight runs. Even when it does not throw, the auth/approval hooks get seeded with an ERC4626 spender for a route that does not exist yet. Keep this unresolved untilroute?.kind === 'erc4626', then fail in preflight instead of from render.As per coding guidelines: "Guard null/undefined/stale API and contract fields in all tx-critical paths so malformed data degrades safely"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLeverageTransaction.ts` around lines 80 - 93, The bundlerAddress useMemo currently calls resolveErc4626RouteBundler even when route is null, causing throws at render; change bundlerAddress so it only calls resolveErc4626RouteBundler when route?.kind === 'erc4626' (otherwise return undefined/null), and adjust authorizationTarget logic to keep returning route.generalAdapterAddress for swap but otherwise use the possibly-undefined bundlerAddress (do not resolve ERC4626 bundler eagerly); this ensures unresolved ERC4626 routes remain unset until preflight (use symbols bundlerAddress, authorizationTarget, resolveErc4626RouteBundler, route, market).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useDeleverageTransaction.ts`:
- Around line 199-215: The ERC4626 path is receiving only
withdrawCollateralAmount and thus loses the quote-derived close amount
(maxCollateralForDebtRepay) when useCloseRoute is true; update the call to
deleverageWithErc4626Redeem in useDeleverageTransaction (the block invoking
deleverageWithErc4626Redeem) to compute collateralToRedeem = useCloseRoute ?
maxCollateralForDebtRepay : withdrawCollateralAmount and pass that value into
the ERC4626 helper (either by replacing the withdrawCollateralAmount argument or
adding a new parameter name the helper expects, e.g., collateralToRedeem), and
ensure deleverageWithErc4626Redeem (and its internal logic in
deleverageWithErc4626Redeem.ts) uses this collateralToRedeem when deciding how
many shares to redeem vs. return to the user (respecting
autoWithdrawCollateralAmount).
---
Outside diff comments:
In `@src/hooks/useDeleverageTransaction.ts`:
- Around line 150-175: authorizeAndDeleverage currently calls tracking.start()
before executeDeleverage's local preflight checks, causing the stepper to open
for synchronous validation errors; move the synchronous preflight validation
(the checks for account, route, bundlerAddress,
withdrawCollateralAmount/flashLoanAmount > 0n, withdrawCollateralAmount <=
maxWithdrawCollateralAmount, and useCloseRoute implies repayBySharesAmount > 0n,
plus presence of swapSellPriceRoute when route.kind === 'swap') to run
immediately inside authorizeAndDeleverage (or invoke executeDeleverage's
preflight helper) and only call tracking.start(...) after these validations
succeed, mirroring the leverage-side preflight pattern so the stepper is started
only for real on-chain flows and not for local validation failures.
In `@src/hooks/useLeverageTransaction.ts`:
- Around line 80-93: The bundlerAddress useMemo currently calls
resolveErc4626RouteBundler even when route is null, causing throws at render;
change bundlerAddress so it only calls resolveErc4626RouteBundler when
route?.kind === 'erc4626' (otherwise return undefined/null), and adjust
authorizationTarget logic to keep returning route.generalAdapterAddress for swap
but otherwise use the possibly-undefined bundlerAddress (do not resolve ERC4626
bundler eagerly); this ensures unresolved ERC4626 routes remain unset until
preflight (use symbols bundlerAddress, authorizationTarget,
resolveErc4626RouteBundler, route, market).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 54f81c6c-83c7-442b-97b8-1461b68e67a3
📒 Files selected for processing (7)
AGENTS.mdsrc/hooks/deleverage/deleverageWithErc4626Redeem.tssrc/hooks/leverage/leverageWithErc4626Deposit.tssrc/hooks/useDeleverageQuote.tssrc/hooks/useDeleverageTransaction.tssrc/hooks/useLeverageQuote.tssrc/hooks/useLeverageTransaction.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useLeverageTransaction.ts (1)
85-98:⚠️ Potential issue | 🟠 MajorShort-circuit bundler resolution until a route exists.
routeis nullable in this hook, but this memo falls through toresolveErc4626RouteBundler(...)for every non-swap case, includingnull. That can throw during render on unsupported markets and also primes the auth/approval hooks with a bogus target beforegetLeverageExecutionPreflight()gets a chance to bail out. Mirror the guarded lookup already used insrc/hooks/useDeleverageTransaction.ts.As per coding guidelines: Guard null/undefined/stale API and contract fields in all tx-critical paths so malformed data degrades safely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLeverageTransaction.ts` around lines 85 - 98, The bundler resolution memo (bundlerAddress useMemo) calls resolveErc4626RouteBundler even when route is null, which can throw and prime downstream hooks; change bundlerAddress to short-circuit and return undefined (or a safe no-op Address) when route is null/undefined, and only call resolveErc4626RouteBundler when route exists and route.kind !== 'swap'; likewise ensure authorizationTarget useMemo continues to return route.generalAdapterAddress for swap and otherwise uses the guarded bundlerAddress; mirror the null-guarding pattern used in useDeleverageTransaction.ts and keep dependencies [route, market.uniqueKey, market.morphoBlue.chain.id] and [route, bundlerAddress] intact.
♻️ Duplicate comments (1)
src/hooks/useDeleverageTransaction.ts (1)
166-182:⚠️ Potential issue | 🟠 MajorDo the new deleverage preflight checks before
tracking.start().
executeDeleverage()now has synchronous throws for stale close-route inputs and missing swap quotes. BecauseauthorizeAndDeleverage()still starts tracking before calling it, those cases will still open the stepper and then immediately fail it instead of never starting tracking. This should be split into a preflight path the same wayuseLeverageTransaction()now does.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDeleverageTransaction.ts` around lines 166 - 182, The preflight synchronous checks inside executeDeleverage (checks for repayBySharesAmount, collateralToRedeem, withdrawCollateralAmount and missing swapSellPriceRoute) must run before tracking.start() to avoid opening the stepper and immediately failing; refactor by extracting those validations into a preflight function or inline them at the top of authorizeAndDeleverage (the caller that currently invokes tracking.start()), call that preflight before any tracking.start() invocation, and only call executeDeleverage or start tracking after the preflight succeeds (mirror the pattern used in useLeverageTransaction).
🧹 Nitpick comments (2)
src/hooks/useDeleverageTransaction.ts (1)
206-221: Only forward-step the deleverage tracker.When
initialStepis already an auth step,src/hooks/deleverage/deleverageWithErc4626Redeem.tsimmediately emits that same auth step again at Lines 57-58 and 72-73. Passing rawtracking.updatehere breaks the monotonic-step contract thatuseLeverageTransaction()now enforces withupdateTrackedStep.Based on learnings: Applies to /*{transaction,tracking,hook}/**.{ts,tsx} : Use useTransactionTracking as the progress-bar/stepper chokepoint, define explicit ordered steps per flow, and call tracking.update(...) only when advancing to a strictly later step (never backwards or out of order).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDeleverageTransaction.ts` around lines 206 - 221, The deleverage flow is forwarding the raw tracking.update into deleverageWithErc4626Redeem which allows duplicate/backward auth steps to be emitted; wrap or replace the passed tracker so only forward-progressing steps are applied (use the existing updateTrackedStep or a small wrapper that compares currentStep and only calls tracking.update when the new step is strictly later). Specifically, in useDeleverageTransaction.ts where deleverageWithErc4626Redeem(...) is invoked, stop passing tracking.update directly and instead pass an advance-only updater (or call useTransactionTracking’s updateTrackedStep) so deleverageWithErc4626Redeem and its auth emissions cannot regress or re-emit the same step out of order.src/hooks/useLeverageQuote.ts (1)
92-100: Surface quote loading state in the preview.The hook returns
isLoading, but the modal never checks it. TanStack Query persists data across argument changes by default—the?? 0nfallbacks won't trigger during normal refetch. However, while quotes fetch (especially swap routes), the modal should checkquote.isLoadingand provide visual feedback rather than leaving the preview inert.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLeverageQuote.ts` around lines 92 - 100, The preview is using fallbacks like "?? 0n" which mask TanStack Query's persisted data during refetch; surface the loading state instead by making the hook and preview respect quote.isLoading: in useLeverageQuote (symbols: initialCapitalCollateralTokenAmount, previewDepositCollateralSharesFromUserLoanAssets) return undefined/null (not 0n) while the relevant quote is loading, or explicitly expose quote.isLoading from the hook, and then update the modal to check quote.isLoading and show a loading indicator/placeholder rather than rendering zero values; ensure any logic that currently assumes 0n when route.kind === 'erc4626' uses the loading sentinel so the UI can react to in-flight fetches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modals/leverage/components/add-collateral-and-leverage.tsx`:
- Around line 84-86: The useLoanAssetInput boolean is stored in local state so
it resets on modal reopen; move it into the shared settings hook
(useAppSettings) so the toggle persists like leverageUseTargetLtvInput: replace
the useState declaration for useLoanAssetInput/setUseLoanAssetInput with a value
and setter from useAppSettings (create a new key if needed), update all places
that read/write useLoanAssetInput to call the app-settings getter/setter, and
remove the local state import/usage; ensure the new setting follows the same
naming and types/pattern as leverageUseTargetLtvInput so the user's choice
survives modal reopen and retries.
---
Outside diff comments:
In `@src/hooks/useLeverageTransaction.ts`:
- Around line 85-98: The bundler resolution memo (bundlerAddress useMemo) calls
resolveErc4626RouteBundler even when route is null, which can throw and prime
downstream hooks; change bundlerAddress to short-circuit and return undefined
(or a safe no-op Address) when route is null/undefined, and only call
resolveErc4626RouteBundler when route exists and route.kind !== 'swap'; likewise
ensure authorizationTarget useMemo continues to return
route.generalAdapterAddress for swap and otherwise uses the guarded
bundlerAddress; mirror the null-guarding pattern used in
useDeleverageTransaction.ts and keep dependencies [route, market.uniqueKey,
market.morphoBlue.chain.id] and [route, bundlerAddress] intact.
---
Duplicate comments:
In `@src/hooks/useDeleverageTransaction.ts`:
- Around line 166-182: The preflight synchronous checks inside executeDeleverage
(checks for repayBySharesAmount, collateralToRedeem, withdrawCollateralAmount
and missing swapSellPriceRoute) must run before tracking.start() to avoid
opening the stepper and immediately failing; refactor by extracting those
validations into a preflight function or inline them at the top of
authorizeAndDeleverage (the caller that currently invokes tracking.start()),
call that preflight before any tracking.start() invocation, and only call
executeDeleverage or start tracking after the preflight succeeds (mirror the
pattern used in useLeverageTransaction).
---
Nitpick comments:
In `@src/hooks/useDeleverageTransaction.ts`:
- Around line 206-221: The deleverage flow is forwarding the raw tracking.update
into deleverageWithErc4626Redeem which allows duplicate/backward auth steps to
be emitted; wrap or replace the passed tracker so only forward-progressing steps
are applied (use the existing updateTrackedStep or a small wrapper that compares
currentStep and only calls tracking.update when the new step is strictly later).
Specifically, in useDeleverageTransaction.ts where
deleverageWithErc4626Redeem(...) is invoked, stop passing tracking.update
directly and instead pass an advance-only updater (or call
useTransactionTracking’s updateTrackedStep) so deleverageWithErc4626Redeem and
its auth emissions cannot regress or re-emit the same step out of order.
In `@src/hooks/useLeverageQuote.ts`:
- Around line 92-100: The preview is using fallbacks like "?? 0n" which mask
TanStack Query's persisted data during refetch; surface the loading state
instead by making the hook and preview respect quote.isLoading: in
useLeverageQuote (symbols: initialCapitalCollateralTokenAmount,
previewDepositCollateralSharesFromUserLoanAssets) return undefined/null (not 0n)
while the relevant quote is loading, or explicitly expose quote.isLoading from
the hook, and then update the modal to check quote.isLoading and show a loading
indicator/placeholder rather than rendering zero values; ensure any logic that
currently assumes 0n when route.kind === 'erc4626' uses the loading sentinel so
the UI can react to in-flight fetches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 668a3454-ac78-4a73-84b5-6cbd7bac37f7
📒 Files selected for processing (8)
AGENTS.mdsrc/hooks/deleverage/deleverageWithErc4626Redeem.tssrc/hooks/leverage/leverageWithErc4626Deposit.tssrc/hooks/leverage/leverageWithSwap.tssrc/hooks/useDeleverageTransaction.tssrc/hooks/useLeverageQuote.tssrc/hooks/useLeverageTransaction.tssrc/modals/leverage/components/add-collateral-and-leverage.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/hooks/useLeverageQuote.ts (1)
285-306: Unused dependency in error memo.
swapExecutionAddressis in the dependency array (line 298) but not referenced in the memo body. Safe to remove.Proposed fix
}, [ route, - swapExecutionAddress, userAddress, initialCapitalInputAmount, isLoanAssetInput,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLeverageQuote.ts` around lines 285 - 306, The memo computing error in useLeverageQuote includes an unused dependency swapExecutionAddress; remove swapExecutionAddress from the useMemo dependency array so the dependencies match referenced values (route, userAddress, initialCapitalInputAmount, isLoanAssetInput, swapLoanInputCombinedQuoteQuery.error, swapCollateralInputQuoteQuery.error, erc4626DepositError, erc4626MintError) and ensure the memo still returns the same error logic in the error constant defined by useMemo.src/hooks/useDeleverageQuote.ts (1)
89-92: Type assertion on batch read data could be tightened.When
allowFailure: false,useReadContractsreturnsdataas the raw tuple of results. The cast works but relies on runtime shape matching. Consider extracting with index guards or inline checks for safer destructuring.Minor — current code is functionally correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDeleverageQuote.ts` around lines 89 - 92, The destructuring of erc4626PreviewData into previewRedeemLoanAssetsFromCollateralShares and previewWithdrawCollateralSharesForBufferedDebtAssets assumes the tuple shape; add a runtime guard before destructuring by checking that erc4626PreviewData is an array with length 2 (e.g., Array.isArray(erc4626PreviewData) && erc4626PreviewData.length === 2) and that entries are of the expected type (bigint or convertible), and only then assign the two values, otherwise fall back to [0n, 0n]; this uses the existing symbols erc4626PreviewData, previewRedeemLoanAssetsFromCollateralShares, previewWithdrawCollateralSharesForBufferedDebtAssets and leaves useReadContracts with allowFailure: false unchanged.src/hooks/leverage/leverageWithSwap.ts (1)
107-107: Multiple sleep() calls for UX pacing.These delays (800-900ms) space out step updates for user visibility. Not blocking correctness, but consider whether these should be configurable or removed in tests. Low priority.
Also applies to: 137-137, 165-165, 344-344
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/leverage/leverageWithSwap.ts` at line 107, The multiple hardcoded sleep(800/900) calls in leverageWithSwap.ts are used only for UX pacing and should be made configurable so tests and consumers can opt out; update the function(s) that call sleep (e.g., leverageWithSwap and any helper step/update functions referenced in this file) to accept an optional config flag or options object (e.g., { enablePacing?: boolean, pacingMs?: number }) or check a shared runtime flag (e.g., process.env.SKIP_PACING or a isTestMode helper) and, when pacing is disabled, bypass or use a zero-duration sleep; ensure all occurrences (the current sleep calls) use that option or helper so tests can run without the artificial delays.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/hooks/leverage/leverageWithSwap.ts`:
- Line 107: The multiple hardcoded sleep(800/900) calls in leverageWithSwap.ts
are used only for UX pacing and should be made configurable so tests and
consumers can opt out; update the function(s) that call sleep (e.g.,
leverageWithSwap and any helper step/update functions referenced in this file)
to accept an optional config flag or options object (e.g., { enablePacing?:
boolean, pacingMs?: number }) or check a shared runtime flag (e.g.,
process.env.SKIP_PACING or a isTestMode helper) and, when pacing is disabled,
bypass or use a zero-duration sleep; ensure all occurrences (the current sleep
calls) use that option or helper so tests can run without the artificial delays.
In `@src/hooks/useDeleverageQuote.ts`:
- Around line 89-92: The destructuring of erc4626PreviewData into
previewRedeemLoanAssetsFromCollateralShares and
previewWithdrawCollateralSharesForBufferedDebtAssets assumes the tuple shape;
add a runtime guard before destructuring by checking that erc4626PreviewData is
an array with length 2 (e.g., Array.isArray(erc4626PreviewData) &&
erc4626PreviewData.length === 2) and that entries are of the expected type
(bigint or convertible), and only then assign the two values, otherwise fall
back to [0n, 0n]; this uses the existing symbols erc4626PreviewData,
previewRedeemLoanAssetsFromCollateralShares,
previewWithdrawCollateralSharesForBufferedDebtAssets and leaves useReadContracts
with allowFailure: false unchanged.
In `@src/hooks/useLeverageQuote.ts`:
- Around line 285-306: The memo computing error in useLeverageQuote includes an
unused dependency swapExecutionAddress; remove swapExecutionAddress from the
useMemo dependency array so the dependencies match referenced values (route,
userAddress, initialCapitalInputAmount, isLoanAssetInput,
swapLoanInputCombinedQuoteQuery.error, swapCollateralInputQuoteQuery.error,
erc4626DepositError, erc4626MintError) and ensure the memo still returns the
same error logic in the error constant defined by useMemo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0bcb4a89-62d9-498e-b719-71e90034df6c
📒 Files selected for processing (7)
AGENTS.mdsrc/features/swap/api/velora.tssrc/hooks/deleverage/deleverageWithSwap.tssrc/hooks/leverage/leverageWithSwap.tssrc/hooks/useDeleverageQuote.tssrc/hooks/useLeverageQuote.tssrc/modals/leverage/components/add-collateral-and-leverage.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/deleverage/deleverageWithSwap.ts
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 by CodeRabbit
New Features
Bug Fixes
UI Improvements
Documentation