feat: let users cap their repay approval, and show the amount needed - #3090
feat: let users cap their repay approval, and show the amount needed#3090sammdec wants to merge 2 commits into
Conversation
Repay's approval button has only ever requested MAX_UINT: RepayActions never passes `amountToApprove`, so generateApproval falls through to its unlimited default. For tokens outside permitConfig (cbBTC on mainnet, among others) tryPermit is false, which also hides the "Approve with" control entirely - leaving no way at all to cap an allowance. Users who capped the allowance by hand in their wallet then hit a second problem. On a full repay the gate compares against debt * 1.0025, not the debt, and never shows that figure. A reported case approved 32.7 cbBTC against 32.68074616 of debt and was asked to approve again indefinitely, because the gate wanted 32.76244803. The repay that eventually went through pulled 32.68074630 - the capped approval would have been fine. Adds an ApprovalAmount preference (persisted per account, defaulting to Unlimited so nothing changes for existing users) and surfaces it in the existing cog menu, which now renders for non-permit tokens too. Permit is untouched: it already signs for an exact amount. The approval is built from the same binding the gate checks, plus a margin. maxApproveNeeded tracks live debt and creeps upward while the approval is in flight, so approving exactly what the gate asked for at t0 would fall short at t1 and recreate the same loop with our own button. Unit tests pin that invariant, including the drift case. Also fixes addTransaction logging MAX_UINT_AMOUNT for every approval, and switches maxApproveNeeded to toString(10) now that it feeds parseUnits, which would throw on exponential notation for dust-sized debts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mjolnir Security ReviewNew commits since last review at 11 PRs reviewed |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
…he shared menu Replaces the approach in the previous commit. Bolting exact/unlimited onto ApprovalMethodToggleButton meant four conditional branches in a component seven flows depend on, a persisted preference mirroring the method one, and a footer reading "Approve with Transaction · unlimited". The shared menu is generic across flows, so it cannot show the amount. That amount is the whole problem: the gate wants debt * 1.0025 and never says so, which is what left the reported user re-approving an allowance they had every reason to think was ample. A Repay-owned control knows the figure and puts it on screen - "Allowance 32.76244803 cbBTC" - so the transparency fix and the finite-approval option land together rather than as two pieces of work. ApprovalMethodToggleButton, RightHelperText and walletSlice are back to their state on main. TxActionsWrapper trades the boolean for an `approvalOptions` slot, so no other flow changes behaviour and the existing Cypress selectors are untouched. The preference is now component state rather than persisted per account. It is chosen inside a modal the user is already standing in, and dropping it removes the second localStorage key and its refresh plumbing. Full precision is shown rather than a rounded FormattedNumber: rendering 32.7624 while approving 32.76244803 understates the allowance being granted, which defeats the point. Checked against an 18-decimal amount - it still fits the footer on one line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sammdec
changed the title
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
| decimals: poolReserve.decimals, | ||
| signatureAmount: amountToRepay, | ||
| amountToApprove: approveExactAmount | ||
| ? parseUnits(amountToApprove, poolReserve.decimals).toString() |
There was a problem hiding this comment.
This runs during render, and getRepayAmountToApprove returns the typed string untouched on the partial branch. The repay input has no decimalScale (AssetInput.tsx:39), so USDC (6dp) + 1.1234567 + "Exact amount" throws fractional component exceeds decimals and takes the modal down. The existing parseUnits calls are inside action()'s try/catch, so today the same input just fails the tx.
.decimalPlaces(decimals, BigNumber.ROUND_UP).toString(10) on both branches fixes it, and stops the footer showing more decimals than the token has.
| * live debt, so that target creeps upward while the approval is in flight; approving | ||
| * exactly what the gate asked for would leave the user re-approving forever. | ||
| */ | ||
| export const REPAY_ALL_APPROVAL_MARGIN = '1.0025'; |
There was a problem hiding this comment.
The margin leaves a residual allowance, which breaks USDT on Ethereum. Exact on a max repay approves debt * 1.0025 * 1.0025 but only pulls the debt, so ~0.5% survives. USDT reverts approve(spender, non-zero) over a non-zero allowance, and the reset path returns early when signatureAmount === '-1' (useApprovalTx.tsx:79), which is the max-repay case. The next full repay fails at gas estimation.
Unlimited never hit this since requiresApproval stays false after one max approval. Either hide Exact for USDT-on-Ethereum, or key needsUSDTApprovalReset off the approval amount rather than signatureAmount.
| txState: 'success', | ||
| asset: assetAddress, | ||
| amount: MAX_UINT_AMOUNT, | ||
| amount: amountToApprove ?? MAX_UINT_AMOUNT, |
There was a problem hiding this comment.
amountToApprove is base units, so the Amplitude tokenAmount on GENERAL.TRANSACTION will report 3276244803 next to the 32.68 every other caller sends (RepayActions.tsx:209, SupplyActions.tsx:276). formatUnits(amountToApprove, decimals) keeps it consistent.
Not repay-only either: Bridge, Umbrella stake and unstake already pass amountToApprove, so their approval events change shape too.
2 participants
General Changes
addTransactionrecordingMAX_UINT_AMOUNTfor every approval regardless of what was actually approvedBackground
From a support report on Aave V3 Ethereum (cbBTC, wallet
0xCA68…487a). The user capped their repay approval and the UI would not accept it, prompting for approval over and over. They eventually approved unlimited, repaid, then revoked.Two independent problems produced that:
1. There was no way to approve a finite amount.
RepayActionsnever passedamountToApprove, sogenerateApprovalfell through toMAX_UINT_AMOUNT. cbBTC's underlying token isn't inpermitConfigfor mainnet (onlyacbBTCis), sotryPermitwas false and the "Approve with" cog never rendered either. The user was capping the spend limit by hand in their wallet because the app offered nothing else.2. A full repay demands more than the debt, and never says so. The gate compares the allowance against
debt * 1.0025:32.6807461632.7000000032.7624480332.68074630Their approval was comfortably sufficient on-chain — the repay that eventually landed moved less than they had approved. Only the frontend rejected it, and it never showed the number it was asking for.
Approach
The control is Repay's own (
RepayAllowanceControl) rather than an option on the sharedApprovalMethodToggleButton. That component is generic across seven flows, so it has no access to the amount — and the amount is the whole problem. A Repay-owned control can renderAllowance 32.76244803 cbBTCin the footer, so the transparency fix ships with the finite-approval option instead of trailing it as separate work.ApprovalMethodToggleButton,RightHelperTextandwalletSliceare untouched.TxActionsWrappergains anapprovalOptionsslot; no other flow changes behaviour and the existing Cypress selectors still work.The choice is component state rather than a persisted preference — it's made inside a modal the user is already in.
Developer Notes
The margin is load-bearing, please don't remove it.
maxApproveNeededis derived from live debt, so it creeps upward while an approval is in flight (~18 raw units/minute for this position). Approving exactly what the gate asked for at t0 falls short by t1 and recreates the reported loop using our own button.getRepayAmountToApproveappliesREPAY_ALL_APPROVAL_MARGINon top ofREPAY_ALL_BUFFER, giving ~0.5% total on a full repay — weeks of accrual headroom.src/components/transactions/__tests__/utils.test.tspins this, and the drift test fails if the margin is dropped.Both the gate and the approval read from one
amountRequiringApprovalbinding inRepayActions, so they can't diverge.Permit is untouched — it already signs for an exact amount, so the control is hidden when permit is in use.
Full precision is deliberate. Rendering
32.7624viaFormattedNumberwhile approving32.76244803understates the allowance being granted, which defeats the purpose. Checked against an 18-decimal amount — it still fits the footer on one line.Drive-by:
maxApproveNeededswitches fromtoString()totoString(10). It feedsparseUnitsfor the first time, which throws on the exponential notation BigNumber emits for dust-sized debts.Scope: Repay only. Supply, Withdraw & Unwrap, Umbrella and the staking flows still approve unlimited. The
approvalOptionsslot is generic, so extending is straightforward if we want it.Design input welcome on the "Allowance" wording and the menu's secondary lines — that copy hasn't had a designer look at it.
Verification
main(ESM transform innetworksConfig)tsc --noEmit,eslintandpnpm buildall cleanReviewer Checklist
Please ensure you, as the reviewer(s), have gone through this checklist to ensure that the code changes are ready to ship safely and to help mitigate any downstream issues that may occur.
.env.examplefile as well as the pertinant.github/actions/*files🤖 Generated with Claude Code