-
Notifications
You must be signed in to change notification settings - Fork 5.4k
feat: enable tx submission before swap quotes are loaded #37963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✨ Files requiring CODEOWNER review ✨🧩 @MetaMask/extension-devs (6 files, +218 -36)
📜 @MetaMask/policy-reviewers (6 files, +218 -36)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. 🔗 @MetaMask/supply-chain (6 files, +218 -36)
🔄 @MetaMask/swaps-engineers (11 files, +205 -101)
|
…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 -->
Builds ready [41f851b]
UI Startup Metrics (1258 ± 97 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [91015ce]
UI Startup Metrics (1100 ± 90 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [154fea4]
UI Startup Metrics (1242 ± 95 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [0a2b3f4]
UI Startup Metrics (1245 ± 93 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
@metamaskbot update-policies |
Builds ready [741e2f9]
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
Builds ready [8589241]
UI Startup Metrics (1233 ± 95 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [25df354]
UI Startup Metrics (1212 ± 94 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [1423536]
UI Startup Metrics (1251 ± 92 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [40d80c7]
UI Startup Metrics (1211 ± 99 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [6dc835b]
UI Startup Metrics (1194 ± 98 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| }), | ||
| trackUnifiedSwapBridgeEvent( | ||
| UnifiedSwapBridgeEventName.QuotesReceived, | ||
| getQuotesReceivedProperties( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More of a non-blocking question about analytics, but will this analytics event fire if I submit on the first streamed quote and then click away?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so, maybe fire the event immediately if activeQuote is present when the user clicks "Swap", rather than waiting for the stream to finish
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one only fires at the end of the stream if the trade has not been submitted. Otherwise, the bridge controller publishes the event and cancels the stream on submission if it hasn't finished yet
Builds ready [7d668fe]
UI Startup Metrics (1269 ± 111 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [b94b788]
UI Startup Metrics (1201 ± 100 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [2cfeca6]
UI Startup Metrics (1366 ± 146 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [5aadff0]
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [bbf2923]
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
Caution MetaMask internal reviewing guidelines:
|
|
@metamaskbot update-policies |
Description
Replaces the LowReturn banner with a warning font-color change to prevent swap page from auto-scrolling while streaming quotes. Also added an e2e test to verify the QuotesReceived event is published if the trade is submitted before quotes finish loading
Changelog
CHANGELOG entry: feat: enable tx submission before swap quotes are loaded
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/SWAPS-3421, https://consensyssoftware.atlassian.net/browse/SWAPS-3427
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Enables submitting swaps before quotes finish loading, replaces the low-return banner with in-line warning styling, updates analytics payloads, adds e2e coverage, and bumps bridge controllers with corresponding LavaMoat policy updates.
isLoadingfromdisabledlogic inbridge-cta-button.tsxand related tests.multichain-bridge-quote-card.tsx; remove auto-scroll tied to that banner.isLoadingandwarningstosubmitBridgeTx; addgetWarningLabelsselector and use it across submission and analytics.getQuotesReceivedPropertiesand trackQuotesReceivedduring streaming..once()forgetQuoteStream; remove redundant mocks; update snapshots.@metamask/bridge-controllerand@metamask/bridge-status-controllerto^62.0.0(and related packages like controller-utils, gas-fee, polling, multichain-network); updateyarn.lock.@metamask/assets-controllers>@metamask/polling-controllerand add policy blocks for@metamask/bridge-status-controller>@metamask/polling-controllerand@metamask/bridge-controller>@metamask/multichain-network-controlleracross Browserify/Webpack policies.Written by Cursor Bugbot for commit 7d668fe. This will update automatically on new commits. Configure here.