fix: MMPay fiat payment fixes #8987
Open
OGPoyraz wants to merge 19 commits into
Open
Conversation
Add optional getAmountData callback that re-encodes nested transaction calldata for a given token amount. Used by transaction types with non-standard nested data (e.g. vault approve + deposit) that require client-side context (vault config, RPC providers) to encode.
OGPoyraz
changed the title
MoneyAccountDeposit
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
4 tasks
After fiat order settlement, use a three-phase relay flow: 1. Discovery quote (EXACT_INPUT) to find settled target token output 2. Re-encode nested calldata via getAmountData callback 3. Real relay quote with delegation (EXACT_OUTPUT) for execution Also removes validateRelaySlippage which incorrectly compared outputs from relay quotes made with different source amounts, and removes isMaxAmount:true which caused delegation errors with nested transactions.
OGPoyraz
force-pushed
the
ogp/fiat-money-account-deposit-fix
branch
from
3f77b67 to
0560a35
Compare
- Fix unnecessary type assertion on requiredAssets hex amount - Add JSDoc @param tags to validateRelayRateDrift - Update fiat-submit tests for three-phase relay flow - Add getAmountData controller tests - Add rate drift, stale calldata, and discovery quote error tests - 100% test coverage maintained
A better post-settlement rate benefits the user and should not block fiat completion. Remove .abs() so only positive drift (rate worsened) is rejected.
OGPoyraz
force-pushed
the
ogp/fiat-money-account-deposit-fix
branch
from
b3b05dd to
8ca96e8
Compare
- 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.
| const state = messenger.call('TransactionPayController:getState'); | ||
| const transactionData = state.transactionData[transactionId]; | ||
| const walletAddress = (transactionData?.accountOverride ?? | ||
| transaction.txParams.from) as Hex | undefined; |
There was a problem hiding this comment.
Fiat poll wallet diverges
Medium Severity
submitFiatQuotes derives the wallet for RampsController:getOrder from current transactionData.accountOverride (or txParams.from), while submitRelayAfterFiatCompletion resolves the settled source amount using quotes[0].request.from. If pay state and the executed quote disagree, order polling can target the wrong address and time out even though relay steps use the quote wallet.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2333bcd. Configure here.
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.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ 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 7e8e999. Configure here.
OGPoyraz
changed the title
MoneyAccountDeposit
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.
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.References
ogp/fiat-money-account-deposit-fix)Checklist