Skip to content

Conversation

@micaelae
Copy link
Member

@micaelae micaelae commented Nov 19, 2025

Description

Changelog

CHANGELOG entry:

Related issues

Fixes:

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Allow submitting bridge transactions while quotes are still loading and propagate quote warnings through analytics and transaction submission, upgrading bridge controllers.

  • Bridge UI (BridgeView):
    • Enable submit while quotes are loading by removing isLoading from isSubmitDisabled.
    • Compute warnings via useMemo and pass to useBridgeQuoteEvents and submitBridgeTx; also pass isLoading to submitBridgeTx.
  • Analytics (useBridgeQuoteEvents):
    • Refactor to accept warnings and generate event payload via getQuotesReceivedProperties.
  • Transaction submission (useSubmitBridgeTx):
    • Extend submitBridgeTx to accept { isLoading, warnings } and forward to BridgeStatusController.submitTx.
  • Tests:
    • Update BridgeView and hooks tests to assert new warnings/isLoading plumbing.
  • Dependencies:
    • Bump @metamask/bridge-controller and @metamask/bridge-status-controller to ^63.0.0.

Written by Cursor Bugbot for commit a4c5251. This will update automatically on new commits. Configure here.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Metamask Bridge Controller Dependency Mismatch

The @metamask/assets-controllers dependency is pinned to ^88.0.0, but the preview versions of @metamask/bridge-controller and @metamask/bridge-status-controller have a peer dependency requirement of ^89.0.0. This creates a version mismatch that will likely cause runtime errors or unexpected behavior. Update @metamask/assets-controllers to ^89.0.0 to satisfy the peer dependency constraints of the new bridge controller versions.

package.json#L169-L170

"@metamask/keyring-controller/@ethereumjs/tx": "npm:@ethereumjs/[email protected]",
"metro/image-size": "^1.2.1",

Fix in Cursor Fix in Web


github-merge-queue bot pushed a commit to MetaMask/core that referenced this pull request Nov 19, 2025
…itted (#7182)

## Explanation

Publishes the `QuotesReceived` event when submitting a trade before all
quotes load

Extension PR: MetaMask/metamask-extension#37963
Mobilie PR: MetaMask/metamask-mobile#22905

<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain?
* If your primary goal was to update one package but you found you had
to update another one along the way, why did you do so?
* If you had to upgrade a dependency, why did you do so?
-->

## 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
-->

Fixes https://consensyssoftware.atlassian.net/browse/SWAPS-3427

## 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/contributing.md#updating-changelogs),
highlighting breaking changes as necessary
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes



<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Publishes a QuotesReceived event on early trade submission and adds a
helper and types to standardize its payload and warnings.
> 
> - **Bridge Status Controller**:
> - Change `submitTx` to accept optional `isLoading` and `warnings`;
when `isLoading=true`, publish `Unified SwapBridge Quotes Received`
using `getQuotesReceivedProperties` before stopping quote polling.
>   - Allow tracking of `QuotesReceived` in internal tracking helper.
> - **Bridge Controller**:
> - Add and export `getQuotesReceivedProperties` to build QuotesReceived
metrics payload; re-export from `index.ts`.
> - Introduce `QuoteWarning` type and use it for `warnings` across
events/tests; update snapshots to standardized values (e.g.,
`low_return`, `insufficient_balance`).
> - Selector/tests: refine `gasIncluded` vs `gasIncluded_7702` handling
and add scenario where fees come from dest token under 7702.
> - Tests: add BTC fee error handling (return `undefined` fees on
failure) and validate BTC/SOL fee behaviors.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
89bb4b5. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@micaelae micaelae force-pushed the swaps3024-submit-while-loading branch from 84e4f6b to 254cad9 Compare November 19, 2025 00:51
@github-actions github-actions bot added size-S and removed size-XS labels Nov 19, 2025
@github-actions github-actions bot added size-M and removed size-S labels Nov 19, 2025
@micaelae micaelae marked this pull request as ready for review November 20, 2025 22:12
@micaelae micaelae requested a review from a team as a code owner November 20, 2025 22:12
await submitBridgeTx({
quoteResponse: activeQuote,
isLoading,
warnings,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Navigation occurs even when transaction submission fails

The navigation.navigate(Routes.TRANSACTIONS_VIEW) call in the finally block (line 347) executes regardless of whether the transaction was actually submitted. If a user clicks "Continue" while activeQuote is undefined (e.g., before quotes finish loading, which is now possible since isLoading was removed from isSubmitDisabled), the if (activeQuote) check fails silently and nothing is submitted. However, the finally block still navigates away, causing the user to be taken to an empty transactions view instead of remaining on the bridge screen.

Fix in Cursor Fix in Web

@micaelae micaelae force-pushed the swaps3024-submit-while-loading branch from c56283c to 464a5d9 Compare November 20, 2025 23:03
@micaelae micaelae marked this pull request as draft November 24, 2025 20:17
@micaelae micaelae marked this pull request as ready for review November 26, 2025 16:09
@micaelae micaelae marked this pull request as ready for review November 26, 2025 16:09
@micaelae micaelae marked this pull request as draft November 26, 2025 16:09
@socket-security
Copy link

socket-security bot commented Nov 26, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​@​metamask/​bridge-status-controller@​63.0.0 ⏵ 63.1.0991007898 +1100
Updatednpm/​@​metamask/​bridge-controller@​63.0.0 ⏵ 63.2.09810079 +198 +1100

View full report

@socket-security
Copy link

socket-security bot commented Nov 26, 2025

Caution

MetaMask internal reviewing guidelines:

  • Do not ignore-all
  • Each alert has instructions on how to review if you don't know what it means. If lost, ask your Security Liaison or the supply-chain group
  • Copy-paste ignore lines for specific packages or a group of one kind with a note on what research you did to deem it safe.
    @SocketSecurity ignore npm/PACKAGE@VERSION
Action Severity Alert  (click "▶" to expand/collapse)
Block Medium
Network access: npm @metamask/bridge-controller in module globalThis["fetch"]

Module: globalThis["fetch"]

Location: Package overview

From: package.jsonnpm/@metamask/[email protected]

ℹ Read more on: This package | This alert | What is network access?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected].

Suggestion: Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@metamask/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Block Medium
Network access: npm @metamask/bridge-status-controller in module globalThis["fetch"]

Module: globalThis["fetch"]

Location: Package overview

From: package.jsonnpm/@metamask/[email protected]

ℹ Read more on: This package | This alert | What is network access?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected].

Suggestion: Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@metamask/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@github-actions
Copy link
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeTrade
  • Risk Level: medium
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

Summary of Changes

The PR updates the bridge-related controllers and refactors how bridge quote warnings and analytics events are tracked:

Package Updates (package.json)

  • Updated @metamask/bridge-controller from v61.0.0 to v63.2.0 (major version bump)
  • Updated @metamask/bridge-status-controller from v61.0.0 to v63.1.0 (major version bump)

Code Changes

All changes are focused on the Bridge functionality:

  1. BridgeView Component (app/components/UI/Bridge/Views/BridgeView/index.tsx):

    • Added QuoteWarning import from bridge-controller
    • Refactored warning state management by consolidating individual boolean flags into a unified warnings array using useMemo
    • Changed how warnings are passed to hooks - now using structured warnings array instead of individual boolean props
    • Updated submitBridgeTx call to include the warnings array
  2. useBridgeQuoteEvents Hook (app/components/UI/Bridge/hooks/useBridgeQuoteEvents/index.ts):

    • Simplified hook interface to accept warnings array instead of multiple boolean parameters
    • Removed local warnings computation (moved to BridgeView)
    • Updated event tracking to use getQuotesReceivedProperties utility function from bridge-controller
    • Cleaner separation of concerns
  3. useSubmitBridgeTx Hook (app/util/bridge/hooks/useSubmitBridgeTx.ts):

    • Added warnings parameter to submitBridgeTx function
    • Now passes warnings along with quote response when submitting transactions
    • Includes warnings in analytics event properties via getQuotesReceivedProperties
  4. Test Updates: All test files updated to match new function signatures with warnings array parameter

Impact Analysis

Direct Impact:

  • Bridge functionality is the primary and only area directly affected
  • Changes are primarily refactoring/improvements to warning handling and analytics tracking
  • The bridge controller version bump suggests API changes that are now properly integrated

Risk Assessment:

  • Medium Risk because:
    • Major version bump of bridge controllers (v61 → v63) suggests breaking API changes
    • Core bridge transaction flow modified (warnings now passed through submission)
    • Analytics event properties structure changed
    • However, changes are well-tested with updated unit tests
    • Scope is contained to bridge-specific code paths
    • No changes to core wallet, accounts, network switching, or other critical flows

Why SmokeTrade Tag:

  • The E2E test file e2e/specs/swaps/bridge-action-smoke.spec.ts is explicitly tagged with SmokeTrade
  • Bridge functionality falls under the trade/swap category
  • This tag covers "Token swaps, bridge, DEX trading" per the description
  • The test validates end-to-end bridge transactions from ETH (Mainnet) to ETH (Base Network)

Other Tags Considered but Not Selected:

  • SmokeWalletPlatform: Not needed - no changes to core wallet, accounts, or network switching
  • SmokeCore: Not needed - no changes to core framework, app state, or navigation
  • SmokeConfirmationsRedesigned: Not needed - no changes to confirmation UI flows
  • SmokeAnalytics: While analytics event properties changed, these are specific to bridge events and will be validated by the bridge E2E test

Confidence: 85%

High confidence because:

  • Clear scope: all changes are isolated to bridge-specific code
  • Direct E2E test exists for bridge functionality with SmokeTrade tag
  • Unit tests were updated to match changes
  • Risk is contained and well-understood

Lower than 90% because:

  • Major version bump of controllers could have unexpected side effects
  • Didn't verify if bridge-controller v63 introduced additional breaking changes beyond what's visible in code

View GitHub Actions results

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size-M team-swaps-and-bridge Swaps and Bridge team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants