-
Notifications
You must be signed in to change notification settings - Fork 5.4k
fix: cp-13.11.0 sending quote request for only valid swaps #38121
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
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
✨ Files requiring CODEOWNER review ✨✅ @MetaMask/confirmations (2 files, +5 -12)
|
| if (swapComparisonDisplayed) { | ||
| dappSwapComparisonDisplayed = true; | ||
| captureDappSwapComparisonDisplayProperties({ | ||
| swap_mm_cta_displayed: 'true', |
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.
Hey @bschorchit : for swap_mm_cta_displayed we need to take missing value as false.
...confirmations/components/confirm/dapp-swap-comparison-banner/dapp-swap-comparison-banner.tsx
Show resolved
Hide resolved
Builds ready [99d90fe]
UI Startup Metrics (1215 ± 91 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
|
||
| import { decodeCommandV3Path } from '../decoding'; | ||
|
|
||
| const DEFAULT_QUOTEFEE = 250; |
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.
Should we fetch this from a remote flag?
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.
PR is updated.
Builds ready [ecfc276]
UI Startup Metrics (1266 ± 121 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
…ask-extension into dapp_swap_req_fix
Builds ready [c0543d6]
UI Startup Metrics (1242 ± 120 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [ca6ffaf]
UI Startup Metrics (1351 ± 136 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Description
Request for quotes should only be send for valid confirmation
Changelog
CHANGELOG entry:
Related issues
Fixes: MetaMask/metamask-mobile#23100
Manual testing steps
Screenshots/Recordings
NA
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Gate dapp-swap quote fetching behind a remote flag, move fee/walletAddress injection to middleware, simplify getDataFromSwap, and update UI metrics/tests accordingly.
dappSwapMetricsFlagand short-circuits when disabled; prevents quote fetching when off.quotesInputviagetDataFromSwap(chainId, data)and augments it withwalletAddressandfee(from flag or default) beforefetchQuotes.metamask-controllerwithremoteFeatureFlags.dappSwapMetrics.dapp-swap-comparison-utils):getDataFromSwapsignature (removewalletAddress/feeconcerns) and return rawquotesInput.GenericQuoteRequest/default fee from utils.useDappSwapComparisonInfo: update to use newgetDataFromSwap(chainId, data)and handle possibly undefinedcommands.feeandwalletAddressadded in middleware and newgetDataFromSwapshape.Written by Cursor Bugbot for commit ca6ffaf. This will update automatically on new commits. Configure here.