refactor: reconcile native Transak fees in RampsController - #10238
Open
saustrie-consensys wants to merge 4 commits into
Open
saustrie-consensys wants to merge 4 commits into
saustrie-consensys wants to merge 4 commits into
Conversation
Follow-up to #9317 addressing review comments. - Add RampsController:getQuoteWithFees, which returns the best on-ramp quote with fees reconciled to the resolved provider. When the provider is Transak Native it fetches the native buy quote via the stateless TransakService:getBuyQuote (so it does not touch Unified Buy's shared buy-quote state) and folds the native total into the quote fee fields, keeping the aggregator networkFee on the network line and the remainder in the provider fee so the breakdown survives and the total is unchanged. - TransactionPayController consumes the single action instead of owning the provider check and native lookup; drop the now-unused TransakService:getBuyQuote delegation from its messenger. - Fall back to the entered fiat amount (not the crypto amountOut) for the direct mUSD fiat target when amountOutInFiat is missing.
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
makinpaper
approved these changes
Sep 15, 2026
saustrie-consensys
added a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Sep 15, 2026
Core moved the native Transak fee lookup and reconciliation into RampsController:getQuoteWithFees (MetaMask/core#10238), which TransactionPayController now calls instead of owning the native lookup. - Delegate RampsController:getQuoteWithFees to the TransactionPayController messenger; drop the no-longer-needed RampsController:getQuotes, RampsController:transakGetBuyQuote, and stateless TransakService:getBuyQuote delegations. - Consume the core preview build b7ffa92bf for @metamask/ramps-controller and @metamask/transaction-pay-controller.
meltingice1337
previously approved these changes
Sep 15, 2026
![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 1 potential issue.
❌ 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 37756a5. Configure here.
The native Transak lookup in getQuoteWithFees used paymentMethods[0] from the request, but the aggregator may price a different method (or the caller may omit the list). Use the resolved quote's own paymentMethod so the native lookup matches the quote being reconciled.
saustrie-consensys
enabled auto-merge
September 15, 2026 10:48
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
saustrie-consensys
disabled auto-merge
September 15, 2026 12:33
saustrie-consensys
enabled auto-merge
September 15, 2026 17:43
amitabh94
reviewed
Sep 15, 2026
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
5 participants
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
Follow-up to #9317, addressing the review comments left by @OGPoyraz after that PR merged. Since #9317 is unreleased, this keeps the net behavior the same while moving ownership to the right package and fixing one fiat-field bug.
Three changes:
Own the native Transak fee lookup in
@metamask/ramps-controller#9317 had
TransactionPayControllerdetect the native provider, parse the asset id, and issue a second Transak buy-quote lookup to correct the fee. That is ramps domain, not payment domain. This addsRampsController:getQuoteWithFees, which returns the best on-ramp quote with its fees already reconciled to the resolved provider. When the resolved provider is Transak Native it fetches the native buy quote and foldstotalFeeinto the returned quote's fee fields.The native lookup uses the stateless
TransakService:getBuyQuote, not the statefulRampsController:transakGetBuyQuote, so it does not write the sharednativeProviders.transak.buyQuotestate that Unified Buy relies on.TransactionPayControllernow calls this one action through its existinggetRampsQuotehelper and consumes the quote's fee fields. TheTransakService:getBuyQuotedelegation it previously required is removed from its messenger (AllowedActions).Keep the fee breakdown when the native fee is applied
Transak Native returns a single total fee, so #9317 put the whole native total in the provider bucket and forced the network line to
0.getQuoteWithFeesnow keeps the aggregator'snetworkFeeon the network line (clamped to the native total) and puts the remainder in the provider fee, soproviderFee + networkFeestill equals the native total. Totals are unchanged; the breakdown survives.Do not put a crypto amount in a fiat field
getDirectMusdTargetAmountFiatfell back toquote.amountOut(a crypto amount) whenamountOutInFiatwas missing, which placed crypto units in the fiat target. It now falls back to the entered fiat amount.A new
bignumber.jsdependency is added to@metamask/ramps-controllerfor the fee split (the package had no decimal-math dependency before).Notes
RampsController:getQuoteWithFeesto theTransactionPayControllermessenger and drop theTransakService:getBuyQuotedelegation. That change will be folded into that open PR.@metamask/ramps-controllerhas no logging subsystem, so the native-lookup fallback branches degrade silently (as the rest of the controller does). Left as-is rather than introducing logging here.References
Refs: TRAM-3651
Follow-up to: #9317
Consumer PR: MetaMask/metamask-mobile#35527
Checklist
🤖 Generated with Claude Code
Note
Medium Risk
Changes MM Pay and on-ramp fee estimation and requires a new messenger delegation; behavior is guarded by fallbacks to aggregator quotes when native lookup fails.
Overview
Moves Transak Native fee reconciliation from MM Pay into
RampsController:getQuoteWithFees, which returns the best on-ramp quote and, fortransak-nativeproviders, rewrites fee fields using the statelessTransakService:getBuyQuotelookup (without touching Unified Buy’s shared native buy-quote state).Fee split behavior changes versus the prior MM Pay approach: the native total is applied while keeping the aggregator
networkFeeon the network line (clamped to the native total) and putting the remainder inproviderFee, so totals stay correct but the breakdown is preserved.TransactionPayControllernow routes fiat quoting throughgetQuoteWithFeesviagetRampsQuote, dropsgetNativeTransakRampsFeeand theTransakService:getBuyQuotemessenger action, and maps reconciled provider/network fees directly into fee buckets.Direct mUSD fixes
getDirectMusdTargetAmountFiatto fall back to the entered fiat amount whenamountOutInFiatis missing (instead ofamountOut, which is crypto).Adds
bignumber.jsto@metamask/ramps-controllerfor the fee math; changelogs note clients must delegateRampsController:getQuoteWithFeesto the pay controller messenger.Reviewed by Cursor Bugbot for commit 56ced5c. Bugbot is set up for automated code reviews on this repo. Configure here.