fix: MMPay fiat payment fixes #8987
Conversation
OGPoyraz
changed the title
MoneyAccountDeposit
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
OGPoyraz
force-pushed
the
ogp/fiat-money-account-deposit-fix
branch
from
3f77b67 to
0560a35
Compare
OGPoyraz
force-pushed
the
ogp/fiat-money-account-deposit-fix
branch
from
b3b05dd to
8ca96e8
Compare
OGPoyraz
changed the title
MoneyAccountDeposit 
| ### Added | ||
|
|
||
| - Add optional `getAmountData` callback to `TransactionPayControllerOptions` for client-side nested calldata re-encoding ([#8987](https://github.com/MetaMask/core/pull/8987)) | ||
| - Add `TransactionPayController:getAmountData` messenger action ([#8987](https://github.com/MetaMask/core/pull/8987)) |
There was a problem hiding this comment.
Can we combine / nest some or all of these?
|
|
||
| export type GetAmountDataRequest = { | ||
| /** Raw token amount (atomic units) to encode into calldata. */ | ||
| amount: string; |
There was a problem hiding this comment.
Minor, I think getPaymentOverrideData provides the human / formatted amount. Not sure which is easier for the client to handle.
There was a problem hiding this comment.
buildMoneyAccountDepositBatch in client was expecting raw amount hence I passed as is.
For reference: https://github.com/MetaMask/metamask-mobile/pull/31009/files#diff-c2390f54bc74c73163ad5949d6accb3524100bf2dd425db4dd82c1ed73d7076f
| refundTo, | ||
| sourceAmounts, | ||
| tokens, | ||
| fiatPayment, |
| ? targetAmount.fiat | ||
| : amountFiat, | ||
| ) | ||
| .plus(paymentAmountFiat) |
There was a problem hiding this comment.
For clarity and consistency, could we call this sourceAmountX and rename the function to getSourceAmount?
| const amountFiat = transactionData?.fiatPayment?.amountFiat; | ||
| const walletAddress = transaction.txParams.from as Hex; | ||
| const walletAddress = | ||
| transactionData?.accountOverride ?? (transaction.txParams.from as Hex); |
There was a problem hiding this comment.
As discussed, this can always use request.from as it's the same calculation?
| const amountFiat = transactionData?.fiatPayment?.amountFiat; | ||
| const walletAddress = transaction.txParams.from as Hex; | ||
| const walletAddress = | ||
| transactionData?.accountOverride ?? (transaction.txParams.from as Hex); |
There was a problem hiding this comment.
On line 95, below, it looks like we were already added the relay fee to the fiat amount?
So was it just the UI that wasn't reflecting that?
There was a problem hiding this comment.
Yes but this is just a quote, real order flow (headless buy) starts when we click "Add funds" in UI where we need the amount again (in useFiatConfirm).
You may ask "why we are not passing the fiat quote amount while starting headless buy", but somehow I never see what amount we requested originally in the quote, either less or more amount saved in fiat quote entity, not sure why. Hence I subtract the fiat provider fee from totals.total in client
https://github.com/MetaMask/metamask-mobile/pull/31009/files#diff-aafbaf61a37d99028f87dea7fbe81be4508bc20412d534db932c1a1cd83e59b6
| (tx) => { | ||
| for (const { nestedTransactionIndex, data } of updates) { | ||
| if (tx.nestedTransactions?.[nestedTransactionIndex]) { | ||
| tx.nestedTransactions[nestedTransactionIndex].data = data; |
There was a problem hiding this comment.
Do we not to update data also? In which case should we look at updateAtomicData in the transaction controller? Or have the new callback return the full data also?
Or are we just updating enough so the Relay quote can generate the delegation and include in the quote?
There was a problem hiding this comment.
Just updating enough parts so only nestedTransactions as other parts will be unnecessary for final quote happening in the Phase3 below.
| const relayRequest: QuoteRequest = { | ||
| ...baseRequest, | ||
| isMaxAmount: false, | ||
| isPostQuote: false, |
There was a problem hiding this comment.
Does this not need to be conditional?
If we're doing a Perps or Predict deposit for example, we won't need transaction data, so we will want to use EXACT_INPUT instead of EXACT_OUTPUT so we can ensure nothing left over and cheaper Relay fees.
We won't even need the first discovery quote in that case so can avoid an entire additional request?
There was a problem hiding this comment.
Very valid point, I will make sure perps predict cases to use EXACT_OUTPUT, thanks.
|
|
||
| // Phase 3: Real relay quote with delegation (standard crypto-like flow). | ||
| const relayRequest: QuoteRequest = { | ||
| ...baseRequest, |
There was a problem hiding this comment.
If we're doing an EXACT_OUTPUT for money account deposit, what do we do with any remaining funds on the selected account after the fiat quote?
There was a problem hiding this comment.
There is no clean way to avoid it, unfortunately it will leave some dust.
If we do the EXACT_INPUT for everything then the vault deposit calldata amount won't match what Relay delivers, causing reverts
| targetAmountMinimum: settledTargetRaw, | ||
| }; | ||
|
|
||
| const relayQuotes = await getRelayQuotes({ |
There was a problem hiding this comment.
Suppose we do a Ramps quote that gets $100 into the EOA.
And we get an EXACT_INPUT quote that tell us we can get $95.
And then we get our final EXACT_OUPUT quote targeting $95... Except the fees are higher in general on EXACT_OUTPUT, plus we now have our additional transaction data included. So what if it says the required source amount is $101?
Do we need to leave a buffer for additional cost so intentionally ask for less in the second quote? But that means we always have funds left over.
Or do we retry with repeating lower amounts until it's less than our balance? Meaning we have to repeatedly re-encode the amount.
Or do we just throw if we can't afford it? But that's not the user's fault so no way for them to remedy.
Essentially we have to consider the slippage twice, from original to discovery, and discovery to final.
There was a problem hiding this comment.
Understood, so depends on the spread between EXACT_INPUT and EXACT_OUTPUT fees + the delegation gas overhead. I didn't foresee this point as I always did my tests on already delegated account.
There is no easy solution for that. But we can try adding potential buffer here for the non-delegated cases, which we can assume discovery phase determine partially (for fees) that. But not sure how can we determine the cost of delegation, maybe we can apply some dedicated amount for that. Would it make sense?
- Consolidate duplicate changelog entries for #8987 - Alphabetize destructured properties in updateQuotes - Rename getPaymentAmount to getSourceAmount for clarity - Simplify walletAddress in fiat-quotes to use transaction.txParams.from
- Consolidate duplicate changelog entries for #8987 - Alphabetize destructured properties in updateQuotes - Rename getPaymentAmount to getSourceAmount for clarity - Simplify walletAddress in fiat-quotes to use transaction.txParams.from
OGPoyraz
force-pushed
the
ogp/fiat-money-account-deposit-fix
branch
from
5ce7386 to
d9d35fd
Compare
|
@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. |
A better post-settlement rate benefits the user and should not block fiat completion. Remove .abs() so only positive drift (rate worsened) is rejected.
- Remove unused args parameter - Replace non-null assertions with optional chaining
The execute flow uses Relay's relayer to handle the source-side transaction, so the user's EOA does not need to hold the source tokens at submit time. This was causing fiat moneyAccountDeposit to fail with 'Insufficient source token balance' after Transak settlement.
The fiat quoting and submission flows used transaction.txParams.from as the wallet address. For moneyAccountDeposit, txParams.from is the money account address on the target chain, not the user's EOA. This caused: - Ramps/Transak to receive the wrong deposit address - resolveSourceAmountRaw to look for on-chain ETH at the wrong address - Relay quotes to use the wrong from/user address - Balance validation to check the wrong account Use accountOverride (the user's selected EVM account) when available, matching the pattern already used in quotes.ts. Also revert the isExecute balance skip (no longer needed with correct address) and remove hasFiatStrategy from totals calculation.
Reverts the test changes from the isExecute balance skip commit since the source code was also reverted. Restores the original test that validates source balance for execute flows.
The test validated the removed hasFiatStrategy path in calculateTotals. With fiat strategy now using amountFiat consistently, this test case is no longer applicable.
Pass fiatPayment.amountFiat from quote context into calculateTotals so the fiat flow uses the user-entered fiat amount for the total instead of deriving it from token amounts or targetAmount.
- Consolidate duplicate changelog entries for #8987 - Alphabetize destructured properties in updateQuotes - Rename getPaymentAmount to getSourceAmount for clarity - Simplify walletAddress in fiat-quotes to use transaction.txParams.from
Transactions without nested calldata (e.g. Perps, Predict deposits) now use a single EXACT_INPUT relay quote after fiat settlement instead of the three-phase discovery + re-encoding + delegation flow. This reduces relay calls from 3 to 1, uses cheaper EXACT_INPUT fees, and avoids leftover dust on the source chain. The three-phase flow is preserved for moneyAccountDeposit and other transactions with nested calldata that requires re-encoding.
…iat submit - Add fee-as-buffer strategy to prevent EXACT_OUTPUT cost overruns after fiat settlement by reserving the original relay fee from the discovery source amount and adding the discovery fee back to the final target. - Simple deposits (Perps, Predict) skip the three-phase discovery flow and use a single EXACT_INPUT relay quote for cheaper fees and no dust. - Split fiat-submit.ts into focused modules: - fiat-submit.ts: orchestration (polling, validation, routing) - fiat-submit-simple.ts: single EXACT_INPUT relay path - fiat-submit-with-calldata.ts: three-phase flow with fee buffer - utils.ts: shared validateRelayRateDrift, extractProviderCode - Add configurable feeReserveMultiplier and maxRateDriftPercent via confirmations_pay_fiat feature flag with safe defaults (1 and 10%).
…-submit-with-calldata
…ubmit-with-calldata
OGPoyraz
force-pushed
the
ogp/fiat-money-account-deposit-fix
branch
from
124114b to
57a14f2
Compare
2 participants
Explanation
Fiat payments for
moneyAccountDeposittransactions fail with multiple sequential errors after the on-ramp order settles:isMaxAmount: truein the fiat re-quote — after Transak settles,submitRelayAfterFiatCompletionre-quotes withisMaxAmount: true, isPostQuote: false. This callsprocessTransactions, which throws"Max amount quotes do not support included transactions"because moneyAccountDeposit has nested txs (approve + deposit) that require delegation.validateRelaySlippagecompares wrong amounts — it comparescurrencyOut.amountfrom two relay quotes made with different source amounts (theoretical $5 quoting phase vs actual ~$4.95 post-Transak settlement). This produces ~25% apparent "slippage" that is not real slippage — it is just a smaller input producing a smaller output.Nested calldata has zero amounts — the fiat path in
handleDoneonly setsamountFiatand never callsupdateTokenAmount(), sorequiredAssets.amountstays0x0and the nested approve + deposit calldata encodes zero amounts.Wrong wallet address in fiat flow —
fiat-quotes.tsandfiat-submit.tsusedtransaction.txParams.fromas the wallet address. FormoneyAccountDeposit,txParams.fromis the money account address on the target chain (Monad), not the user's EOA. This caused Ramps/Transak to receive the wrong deposit address,resolveSourceAmountRawto look for on-chain ETH at the wrong address, and all relay quotes to use the wrongfrom/useraddress — resulting inTRANSFER_FROM_FAILEDreverts.Fiat total calculation using wrong amount —
calculateTotalsderived the payment amount from token amounts ortargetAmount, which is incorrect for fiat flows where the user enters a specific fiat amount.Changes
fiat-quotes.ts— usesaccountOverridewhen available forwalletAddress, matching the existing pattern inquotes.ts. This ensures Ramps/Transak receives the user's actual EOA address, not the money account address.fiat-submit.ts— two wallet address fixes plus three-phase relay flow:submitFiatQuotes: usesaccountOverride ?? txParams.fromfor order polling wallet addresssubmitRelayAfterFiatCompletion: usesbaseRequest.from(already accountOverride-aware from the quote) for on-chain amount lookupisPostQuote: true, EXACT_INPUT) with the settled source amount to learncurrencyOut.minimumAmountgetAmountDatavia messenger to delegate calldata re-encoding to the client, then patches nested tx data +requiredAssets[0].amountisPostQuote: false, EXACT_OUTPUT) with delegationrelay-submit.ts— reverts theisExecutebalance skip (no longer needed with correct wallet address). The balance check now correctly validates the user's EOA balance.validateRelaySlippage→validateRelayRateDrift— compares USD exchange rate ratios (output_usd / input_usd) between original and discovery quotes instead of absolute amounts, normalising for different source amounts.TransactionPayController— adds optionalgetAmountDatacallback on the constructor, exposed via messenger asTransactionPayController:getAmountData. Keeps ABI knowledge on the client side.totals.ts— addsfiatPaymentAmountparameter tocalculateTotals. When a fiat strategy quote is present, uses the user-entered fiat payment amount directly instead of deriving it from token amounts or relaytargetAmount. Falls back to'0'if fiat amount is unavailable.quotes.ts— passesfiatPayment.amountFiatfrom the transaction pay state intocalculateTotalsasfiatPaymentAmount.relay-quotes.ts(recipient routing fix) — addsskipProcessTransactionstoQuoteRequesttype. Defaults toisPostQuotewhen not set. The simple fiat path (fiat-submit-simple.ts) setsskipProcessTransactions: falseto forceprocessTransactionsto run and extract thetransfer(to, amount)recipient from calldata — fixing Predict/Perps deposits where Relay sent swapped tokens to the user's EOA instead of the target contract.relay-quotes.ts(7702 batch gas estimation) — for post-quote flows, includes the original transaction in batch gas estimation alongside relay params. Previously, a single relay step was estimated alone, gotis7702: false, and the batch fell back to separate type-0x2 transactions — breaking zero-balance fiat-funded accounts that need all native tokens for the swap.relay-quotes.ts(native gas subtraction) — extends the phase-2 gas subtraction to trigger when the source token is the native gas token (e.g. POL on Polygon), not only whenisSourceGasFeeTokenis true. For zero-balance accounts where the source IS the native token, gas must be reserved from the source amount to avoidinsufficient funds for gas * price + value.relay-submit.ts(gas price alignment) — the prepended original transaction in post-quote batch submissions now carries the relay step'smaxFeePerGasandmaxPriorityFeePerGas. This ensures the top-level 7702 batch uses the same gas price as the relay quote, matching the gas cost computed during the phase-2 gas subtraction.eip7702.ts(transaction-controller) —generateEIP7702BatchTransactionnow sums native values from all nested calls and sets the result as the top-level transactionvalue. Previously the top-level value was always0x0, causing 7702 batches with native value transfers (e.g. POL swap) to revert because the delegation contract had no native tokens to forward.fiat-quotes.ts(accountOverride fix) — usesaccountOverride ?? txParams.fromfor the wallet address, matching the pattern infiat-submit.ts. For moneyAccountDeposit,txParams.fromis the money account on the target chain, not the user's EOA. This caused relay quotes to use the wrongfrom/useraddress, and Relay Execute to pull funds from an empty account.relay-submit.ts(isExecute balance skip) — skips the source balance validation when the Relay Execute flow is active (isExecute: true). In Execute flows, Relay's relayer handles the source-side transaction — the user's account doesn't need to hold the source tokens at submit time.fiat-submit-with-calldata.ts(rate drift moved) — moves thevalidateRelayRateDriftcheck from after the discovery quote (phase 1) to after the final EXACT_OUTPUT quote (phase 3). The discovery quote uses a reduced source amount (after fee reserve), causing an unfair rate comparison with the original quote on cross-chain routes where relay fees are a large percentage. The final quote uses the full source amount, making the comparison accurate.feature-flags.ts(fee reserve default) — bumpsDEFAULT_FEE_RESERVE_MULTIPLIERfrom1to1.2. On cross-chain routes with high relay fees (ETH→Monad), a 1x reserve was too tight — relay fees could increase slightly between the discovery and final quotes, causing the final EXACT_OUTPUT to overshoot the settled balance. 1.2x provides 20% headroom.Feature Flags (LaunchDarkly)
This PR reads optional properties from two feature flags. No new LaunchDarkly flags need to be created — these are properties on existing flag objects.
confirmations_pay_fiatfeeReserveMultipliernumber1.21.5) if EXACT_OUTPUT quotes consistently exceed the settled balance after settlement on cross-chain routes.maxRateDriftPercentnumber10confirmations_pay_post_quotegasBuffernumber1.1insufficient funds.No action required for launch — all properties default to safe values when absent. They exist as remote safety valves to tune post-settlement behavior without a code release.
References
ogp/fiat-money-account-deposit-fix)Checklist
Note
High Risk
Touches money movement after fiat settlement (relay quoting/submit, wallet overrides, gas reservation, 7702 batch value) across transaction-pay and transaction-controller; regressions could block deposits or mis-route funds.
Overview
Fixes fiat-funded pay flows (especially
moneyAccountDeposit) after on-ramp settlement by correcting wallet routing, relay submit/quote behavior, totals, and EIP-7702 batch native value.Transaction Pay — fiat submit replaces the single post-settlement
isMaxAmountrelay re-quote with two paths: simple deposits use one EXACT_INPUT post-quote relay (submitSimpleRelay); nested approve+deposit use a three-phase flow (submitWithCalldataReEncoding): fee-reserved discovery quote → clientgetAmountDatamessenger callback to patch nested calldata andrequiredAssets→ final EXACT_OUTPUT relay. Slippage checks becomevalidateRelayRateDrift(USD rate ratio) with tunablefeeReserveMultiplier/maxRateDriftPercent. Fiat quoting/submit now useaccountOverride ?? txParams.fromfor ramps and on-chain resolution.Relay layer adds optional
skipProcessTransactionson quote requests; post-quote gas handling reserves gas when the source is native (not only fee-token gas) with agetPostQuoteGasBuffermultiplier; post-quote batch gas estimation includes the original tx (unlessfromdiverges fromtxParams.from) so 7702 batches work for zero-balance fiat users; prepended original txs inherit relay max fee fields; Relay Execute skips EOA balance checks at submit.Totals use
fiatPayment.amountFiatfor the payment line when a fiat strategy quote is present.Transaction controller —
generateEIP7702BatchTransactionsets top-levelvalueto the sum of nested call values when non-zero, fixing native transfers in 7702 batches (e.g. POL swaps).Reviewed by Cursor Bugbot for commit 0f35336. Bugbot is set up for automated code reviews on this repo. Configure here.