[pull] main from MetaMask:main#575
Merged
Merged
Conversation
## Explanation
The fiat strategy had three issues that prevented correct end-to-end
behavior:
1. **Provider code extraction**: The submit flow was parsing the
provider code from the order ID string
(`/providers/{provider}/orders/{orderCode}`), but the order ID format no
longer contains the provider prefix. Now extracts the provider code
directly from the stored `rampsQuote.provider` field and passes the raw
order ID as `orderCode`.
2. **Fiat asset tracking**: Instead of calling
`RampsController:setSelectedToken` (which could silently fail if tokens
weren't loaded), the fiat asset's `caipAssetId` is now stored directly
on the `fiatPayment` state when the payment method changes.
3. **Totals calculation**: When a fiat strategy quote is present, the
totals pipeline should use `targetAmount` (the actual bridged output)
rather than `amountFiat`/`amountUsd` from token metadata — same behavior
already applied for `isMaxAmount` transactions.
## References
- Follows up on fiat strategy submit implementation from #8347
## Checklist
- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Changes modify the fiat strategy execution path and totals
computation, which can affect order polling, relay submission, and
user-visible pricing. Risk is mitigated by updated unit tests but
touches core payment flow state and calculations.
>
> **Overview**
> Fixes the fiat strategy end-to-end flow by **deriving the on-ramp
provider code from the stored `rampsQuote.provider`** (instead of
parsing the `orderId`) and passing the raw `orderId` through as the
`orderCode` when polling `RampsController:getOrder`.
>
> When the fiat payment method changes, the controller now **persists
the derived fiat on-ramp asset as `fiatPayment.caipAssetId`** (new
field) rather than calling `RampsController:setSelectedToken`.
>
> Updates totals so that when any quote uses
`TransactionPayStrategy.Fiat`, **totals use `quote.targetAmount`**
(matching max-amount behavior) instead of summing token fiat/USD
amounts.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
8971e1a. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
## Explanation Feature release for `json-rpc-engine` introducing `assertExpectedHooks`. ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Primarily version bumps and changelog updates; no functional code changes in this diff beyond updating dependency resolution. > > **Overview** > Bumps the monorepo release to `973.0.0` and publishes `@metamask/json-rpc-engine@10.5.0`, documenting the new `assertExpectedHooks` export in the engine changelog. > > Updates all in-repo consumers to depend on `@metamask/json-rpc-engine@^10.5.0`, refreshes related package changelogs, and updates `yarn.lock` accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 7a7ce4c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## Explanation I forgot to remove this event in my previous PR: - #8715 ## References N/A ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk cleanup that only tightens messenger event typings/delegations and updates related lint suppression/changelog entries; behavioral impact should be limited to consumers that still attempted to delegate/subscribe to `KeyringController:stateChange`. > > **Overview** > Removes the unused `KeyringController:stateChange` subscription from `multichain-account-service` by dropping it from the test messenger delegation list and from the service’s `AllowedEvents` type. > > Updates supporting metadata: deletes the now-unneeded ESLint suppression for `src/tests/messenger.ts` and tweaks the changelog entry to reflect that `KeyringController:stateChange` is no longer required (and references the follow-up PR). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c7eb0e9. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )