Conversation
…oney Account deposits
…ame-token gas budget
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e81860e. Configure here.
| const sourceAmount = new BigNumber(promotedQuote.sourceAmount.raw); | ||
| const sourceTokenIsNative = | ||
| request.sourceTokenAddress.toLowerCase() === | ||
| getNativeToken(request.sourceChainId).toLowerCase(); |
There was a problem hiding this comment.
Spend cap misses normalized native
Medium Severity
After normalizeRequest, Polygon native is rewritten to NATIVE_TOKEN_ADDRESS, but getPromotedSourceCost and the earlier spend check only treat getNativeToken as native. Same-token gas is then omitted from the cap, so a promoted max quote can exceed the user's spendable native balance.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e81860e. Configure here.
| log('Normalized requests', normalizedRequests); | ||
|
|
||
| const quotes = await Promise.all( | ||
| quotes = await Promise.all( |
There was a problem hiding this comment.
Validation uses stale parent transaction
Medium Severity
Promotion rewrites nested calldata and requiredAssets only on a local clone, then validateRelayQuotes runs against the original parent. Validation rebuilds the execute delegation from stale txParams.data, so caveats can disagree with the promoted quote and fail closed or simulate the wrong batch.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e81860e. Configure here.
| * and `detail` on {@link QuoteErrorInfo}. | ||
| */ | ||
| export type QuoteErrorReason = | ||
| | 'atomic-promotion-failed' |
There was a problem hiding this comment.
Do we need an explicit reason for this? Or is this a subset of no-quotes and we could use our error prefix pattern to get a good error message?
| } | ||
|
|
||
| function isSubsidizedRelayQuote(quote: RelayQuote): boolean { | ||
| export function isSubsidizedRelayQuote(quote: RelayQuote): boolean { |
There was a problem hiding this comment.
Minor, exported above private?
| } catch (error) { | ||
| log('Error fetching quotes', { error }); | ||
|
|
||
| if (quotes.some(isPromotedSubsidizedMaxMoneyAccountQuote)) { |
There was a problem hiding this comment.
Can we be sure any error under these conditions is because of those conditions?
Should we instead just add an error prefix around the re-quote?
| discoveryQuote: TransactionPayQuote<RelayQuote>, | ||
| ): boolean { | ||
| return ( | ||
| isMoneyAccountDepositTransaction(transaction) && |
There was a problem hiding this comment.
Is this needed?
Can we avoid the coupling as atomic and subsidised should always be okay?
Could this be a feature flag keyed by transaction type?
| }); | ||
| } | ||
|
|
||
| async function maybePromoteSubsidizedMaxMoneyAccountQuote({ |
There was a problem hiding this comment.
Minor, can some of these utils go in relay-max-gas-station.ts?
Maybe we rename to relay-max.ts?
| transaction: transactionClone, | ||
| }); | ||
|
|
||
| const promotedQuote = await getSingleQuote( |
There was a problem hiding this comment.
We discussed which way to default this, as if the subsidized flow is the more common, we could do the initial quote with atomic and then do the second quote only if resulting quote is not subsidized?
And we could "hint" this from the client by setting atomic based on fixed spreads feature flag and checking route, so it's already off if route cannot be subsidized?
| } | ||
|
|
||
| if (discoveryQuote.original.details.currencyOut.amount !== targetAmount) { | ||
| throw new Error('Discovery quote target amount changed before promotion'); |
There was a problem hiding this comment.
I don't think we're handling enough error states 😄
| } | ||
|
|
||
| /* istanbul ignore next: re-quote hardcodes atomic true; defensive guard. */ | ||
| if (promotedQuote.request.atomic !== true) { |
There was a problem hiding this comment.
Are some of these necessary? Are we guarding against Relay returning a response that simply ignores our request params?
|
|
||
| const budget = new BigNumber(request.sourceTokenAmount); | ||
|
|
||
| if (sourceCost.isGreaterThan(budget)) { |
There was a problem hiding this comment.
This is already validated in quote validation phase generically, checking the amount in the quote is less or equal to balance.
2 participants
Explanation
Max deposits to the Money Account currently use
EXACT_INPUTquoting plus a separate MetaMask-sponsored vault transaction (approve +teller.deposit) submitted as a second leg.This PR promotes subsidized (fixed-spread) max direct Money Account deposits to an atomic
EXACT_OUTPUTRelay re-quote with the vault calls embedded, reusing the existinggetAmountData/getDelegationTransactionmachinery and the standard atomic submit path (no second vault transaction). Non-subsidized max behavior is unchanged.Fail-closed by design: any promotion failure (amount rewrite, re-quote error, subsidy loss, spend-cap exceed including same-token gas, final validation failure) blocks terminally with the new
atomic-promotion-failedquote error reason instead of silently falling back to a non-atomic quote.quote.request.atomicon the selected quote already drives submission, so no double-vault is possible.No public API signatures changed; no client code changes required — consumers pick this up via package bump.
Screenshot & recordings
Exact Input for Max ARB token:
Exact output for Max USDC token:
Recording:
Simulator.Screen.Recording.-.iPhone.17.Pro.Max.-.2026-09-14.at.14.57.04.mov
References
Checklist
Note
High Risk
Changes max-deposit quoting, spend-cap math, and terminal error handling on a subsidized Money Account path; a promotion bug could block deposits or mis-size amounts instead of falling back.
Overview
Subsidized max direct Money Account Relay deposits now run a two-step quote path: after the usual max “discovery” quote, eligible subsidized flows are promoted to an atomic
EXACT_OUTPUTre-quote with vault/deposit calldata embedded in the Relay request (viagetAmountData+ delegation), and the returned quote keepsatomic: trueandisMaxAmount: true. Non-subsidized max deposits are unchanged.Promotion is fail-closed: subsidy loss, invalid amount rewrites, spend-cap violations (including same-token gas), or post-promotion validation failures surface a new terminal
atomic-promotion-failedQuoteErrorReasoninstead of falling back to a non-atomic quote. Strategy selection inquotes.tsstops trying alternate strategies on that reason and clears quotes/batch state; validation errors during promotion are wrapped asatomic-promotion-failedwhen a promoted quote was in play.isSubsidizedRelayQuoteis exported for reuse in promotion gating. Submission tests assert promoted atomic max quotes go through Relay execute with embedded steps and skip the separate vault helper; non-atomic max behavior is preserved.Reviewed by Cursor Bugbot for commit e81860e. Bugbot is set up for automated code reviews on this repo. Configure here.