Open
Conversation
Contributor
|
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. |
Contributor
✨ Files requiring CODEOWNER review ✨✅ @MetaMask/confirmations (21 files, +776 -251)
👨🔧 @MetaMask/core-extension-ux (1 files, +1 -13)
📜 @MetaMask/policy-reviewers (4 files, +308 -4)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. 🧪 @MetaMask/qa (1 files, +87 -0)
|
Contributor
Builds ready [d67145b]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
Builds ready [f8b973b]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
Builds ready [3d4bdab]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
7 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
|
Review — posted on behalf of @cryptotavares I reviewed this PR as part of tracking Q1 confirmation work. Architecture in issue-7051 was written by the implementor and has no recorded team sign-off or discussion comments. The spec is detailed, but there is no evidence of collaborative pre-approval before 4 reviewer teams were engaged. The confirmations team review is especially critical here. Beyond the process concern, the diff contains two bugs that I believe will block users from submitting cancel/speedup on production: BUG 1 — HIGH: EIP-1559 cancel/speedup confirm button permanently disabledFile: const hasEIP1559Gas =
txParams?.maxFeePerGas &&
txParams?.maxPriorityFeePerGas &&
txParams?.gasLimit; // ← wrong field
The fix is already demonstrated elsewhere in the same PR ( txParams?.gasLimit ?? txParams?.gasBUG 2 — HIGH: Legacy (non-EIP-1559) cancel/speedup always blockedSame A separate path is needed to set BUG 3 — MEDIUM: Tests assert known-wrong fee valuesFile: The test file contains inline comments acknowledging that the expected fee constants are incorrect (the author notes the intended values should be higher). Merging these tests locks in a broken fee display as the regression baseline. Either fix the underlying fee calculation and update the constants, or do not assert specific fee strings until the logic is correct. Minor: Dead stateFile:
Bugs 1 and 2 together mean the cancel/speedup flow would be broken for all users on both EIP-1559 and legacy networks. Would recommend blocking merge until these are resolved. |
ui/pages/confirmations/components/gas-timing/gas-timing.component.js
Outdated
Show resolved
Hide resolved
Contributor
Builds ready [c72ecd4]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
ui/pages/confirmations/components/edit-gas-fee-popover/network-statistics/network-statistics.js
Show resolved
Hide resolved
Contributor
Builds ready [bd1817d]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
7 tasks
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| const speedUpCancelModal = new SpeedUpAndCancelModal(driver); | ||
| await speedUpCancelModal.waitForModal(); | ||
| await speedUpCancelModal.checkSpeedUpTitleVisible(); | ||
| await speedUpCancelModal.checkSpeedRowShowsSiteSuggested(); |
There was a problem hiding this comment.
E2E test asserts transient "Site suggested" before gas update
Medium Severity
checkSpeedRowShowsSiteSuggested() asserts "Site suggested" in the speed row, but useCancelSpeedupInitialGas applies the initial gas rule on modal open, which changes userFeeLevel from dappSuggested to medium or tenPercentIncreased. The assertion only passes because it catches the transient first-render state before the store update propagates. This makes the E2E tests flaky — if the Redux update completes before the waitForSelector check, the text will already be "Medium" or "⬆ 10% increase" and the test will timeout.
Additional Locations (1)
|
Contributor
Builds ready [6fe2d50]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
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
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.
Description
This PR modernizes the cancel/speed-up transaction UI to use the same GasFeeModal context and gas-details patterns as the main confirmation flow, and fixes the "useConfirmContext must be used within ConfirmContextProvider" error when opening the gas modal from cancel-speedup.
What was done
transactionMetaandeditGasMode.GasFeeModalWrapperwraps modal content inConfirmContextProviderwithcurrentConfirmationOverride={transactionMeta}when opened from cancel-speedup so gas modals can useuseConfirmContext().GasFeeModalContextProvider(transactionMeta, editGasMode),useCancelSpeedupGasState, anduseCancelSpeedupInitialGas. Renders shared gas UI (e.g. GasFeesSection) and opens the gas modal via context.AdvancedEIP1559Modal,AdvancedGasPriceModal, anduseGasFeeEstimateLevelOptionsuseuseConfirmContext()again; nouseTransactionForGasModalin the modal flow.ConfirmContextProvidersupports optionalcurrentConfirmationOverride; when set, route sync and navigate-on-dismiss are skipped so the gas modal can be used outside the confirm route.useCancelSpeedupGasStateprovides effective transaction and actions;useCancelSpeedupInitialGasruns the initial gas rule (medium vs tenPercentIncreased) when the cancel-speedup modal is open.Changelog
CHANGELOG entry: Fixed gas edit opening from cancel/speed-up transaction flow and aligned cancel-speedup gas UI with the main confirmation flow.
Related issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/7051
Manual testing steps
Screenshots/Recordings
cancel_new_ex.webm
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Touches transaction replacement gas calculation and confirmation/gas modal context wiring, which can affect whether cancel/speed-up transactions submit and display fees correctly. Changes are well-covered with new unit and e2e tests but span multiple confirmation components and hooks.
Overview
Refactors the cancel/speed-up transaction modal to use the shared
GasFeeModalContext/ConfirmContextflow (including acurrentConfirmationOverride) so gas editing can be opened from this modal withoutuseConfirmContextprovider errors, and aligns fee/timing display with the main confirmation UI.Strengthens replacement-transaction behavior by persisting
previousGason modal open and introducinggetGasValuesForReplacement()to enforce minimummaxFeePerGas/maxPriorityFeePerGasbased onpreviousGas × (CANCEL_RATE|SPEED_UP_RATE), preventing underpriced cancel/speed-up submissions.Updates gas UI components to be more context-optional (e.g.,
useDappSwapContextOptional,NetworkStatisticsfallback estimates,GasTiminglabeling/override), adds new hooks/tests and e2e coverage for the modal, and expands MV3 LavaMoat policies for additional transaction/gas-related packages.Written by Cursor Bugbot for commit 6fe2d50. This will update automatically on new commits. Configure here.