Skip to content

feat: disable simulation for MetaMask Pay transactions - #34853

Open
matthewwalsh0 wants to merge 3 commits into
mainfrom
feat/simulation-disabled-mm-pay
Open

feat: disable simulation for MetaMask Pay transactions#34853
matthewwalsh0 wants to merge 3 commits into
mainfrom
feat/simulation-disabled-mm-pay

Conversation

@matthewwalsh0

@matthewwalsh0 matthewwalsh0 commented Aug 16, 2026

Copy link
Copy Markdown
Member

Description

Transaction simulation was previously enabled or disabled solely based on the user's useTransactionSimulations preference, with no awareness of the transaction being processed. MetaMask Pay transactions (deposits, orders, conversions, claims, withdrawals) do not benefit from simulation and should never be simulated regardless of that preference.

This wires the transaction meta through to the isSimulationEnabled callback and returns false whenever the transaction matches one of the MetaMask Pay transaction types (MM_PAY_TRANSACTION_TYPES), falling through to the existing preference-based behaviour for all other transactions.

This depends on the extended isSimulationEnabled callback signature in @metamask/transaction-controller, consumed here via a preview build (69.5.2-preview-7585e3a) from MetaMask/core#9800 until that change is released.

Changelog

CHANGELOG entry: null

Related issues

Fixes:

Manual testing steps

Feature: Simulation disabled for MetaMask Pay transactions

  Scenario: user initiates a MetaMask Pay transaction with simulations enabled
    Given transaction simulations are enabled in settings
    When user initiates a MetaMask Pay transaction (e.g. a Perps deposit)
    Then the confirmation does not run or display a simulation

  Scenario: user initiates a non-Pay transaction with simulations enabled
    Given transaction simulations are enabled in settings
    When user initiates a standard transaction (e.g. a token transfer)
    Then the confirmation runs and displays a simulation as before

Screenshots/Recordings

Before

N/A

After

N/A

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

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

Medium Risk
Touches confirmation/simulation behavior for money-account and pay flows; dependency is a preview transaction-controller build rather than a stable release.

Overview
Transaction simulation now considers the transaction being confirmed, not only the useTransactionSimulations preference.

isSimulationEnabled accepts optional transactionMeta and forces simulation off when the type is in MM_PAY_TRANSACTION_TYPES (deposits, withdrawals, conversions, claims, perps/predict flows, etc.). All other transactions still follow the existing preference.

This relies on a preview @metamask/transaction-controller (69.5.2-preview-7585e3a) that passes meta into the callback until the core change ships. Unit tests cover non–MM Pay (simulation on when enabled) and each MM Pay type (simulation off).

Reviewed by Cursor Bugbot for commit c5909ba. Bugbot is set up for automated code reviews on this repo. Configure here.

@metamask-ci metamask-ci Bot added the team-confirmations Push issues to confirmations team label Aug 16, 2026
@metamask-ci

metamask-ci Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Related issues section is empty. Add Fixes: #123 / Closes: <URL> / Refs: <Jira key>, or write a short rationale after the colon.
  • Pre-merge author checklist has unchecked items (e.g. "I've tested on Android"). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@socket-security

socket-security Bot commented Aug 16, 2026

Copy link
Copy Markdown

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

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​@​metamask/​core-backend@​8.1.1 ⏵ 8.1.27810010099 +1100
Updatednpm/​@​metamask/​multichain-account-service@​13.0.1 ⏵ 13.0.299 +110079 +199100
Addednpm/​@​metamask-previews/​transaction-controller@​69.5.2-preview-7585e3a891008198100

View full report

@socket-security

socket-security Bot commented Aug 16, 2026

Copy link
Copy Markdown

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-previews/transaction-controller in module globalThis["fetch"]

Module: globalThis["fetch"]

Location: Package overview

From: package.jsonnpm/@metamask-previews/transaction-controller@69.5.2-preview-7585e3a

ℹ 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 support@socket.dev.

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-previews/transaction-controller@69.5.2-preview-7585e3a. 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.

Warn Low
Potential code anomaly (AI signal): npm @metamask-previews/transaction-controller is 75.0% likely to have a medium risk anomaly

Notes: The code performs straightforward signature verification using ethers.js, returning true when the recovered signer matches the provided publicKey. While generally safe, the silent catch and potential mismatch between data formatting and signing process should be addressed to avoid silent failures. Overall, a benign utility with moderate input-format sensitivity.

Confidence: 0.75

Severity: 0.50

From: package.jsonnpm/@metamask-previews/transaction-controller@69.5.2-preview-7585e3a

ℹ Read more on: This package | This alert | What is an AI-detected potential code anomaly?

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 support@socket.dev.

Suggestion: An AI system found a low-risk anomaly in this package. It may still be fine to use, but you should check that it is safe before proceeding.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@metamask-previews/transaction-controller@69.5.2-preview-7585e3a. 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
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/core/Engine/wallet-init/instance-options/transaction-controller.test.ts 0/140 0/179 0/382

AI-detected flaky patterns

app/core/Engine/wallet-init/instance-options/transaction-controller.test.ts

  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The test file uses jest.clearAllMocks() in the beforeEach block, which is good for clearing mock call counts. However, it does not reset the mock implementations, which can lead to shared state between tests causing intermittent failures. This can result in tests passing or failing based on the order they are run.
    • Suggested fix in app/core/Engine/wallet-init/instance-options/transaction-controller.test.ts:1:
      -beforeEach(() => {
      -       jest.clearAllMocks();
      -       isSendBundleSupportedMock.mockResolvedValue(false);
      -       accountSupports7702Mock.mockResolvedValue(true);
      -     });
      +beforeEach(() => {
      +       jest.clearAllMocks();
      +       jest.resetAllMocks(); // Reset mock implementations
      +       isSendBundleSupportedMock.mockResolvedValue(false);
      +       accountSupports7702Mock.mockResolvedValue(true);
      +     });

This check is informational only and does not block merging.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeConfirmations, SmokeNetworkAbstractions, SmokeNetworkExpansion, SmokeSwap, SmokeStake, SmokeWalletPlatform, SmokeMoney, SmokePerps, SmokeMultiChainAPI, SmokePredictions, SmokeSeedlessOnboarding, SmokeBrowser, SmokeSnaps, SmokeMMConnect
  • Selected Performance tags: @PerformancePreps, @PerformancePredict, @PerformanceMoney
  • Risk Level: high
  • AI Confidence: 100%
click to see 🤖 AI reasoning details

E2E Test Selection:
Hard rule (controller-version-update): @MetaMask controller package version updated in package.json: @metamask/transaction-controller. Running all tests.

Performance Test Selection:
The isSimulationEnabled change disables transaction simulations for MM Pay transaction types (perps, predict, money account). Disabling simulation could affect the rendering and loading time of confirmation screens for these flows. Additionally, the transaction-controller package update to a preview version could introduce performance changes. The @PerformancePreps, @PerformancePredict, and @PerformanceMoney tags cover the affected flows (perps add funds, predict market deposits, money account operations) and should be run to detect any performance regressions introduced by these changes.

View GitHub Actions results

@matthewwalsh0
matthewwalsh0 marked this pull request as ready for review August 16, 2026 10:45
@matthewwalsh0 matthewwalsh0 added the DO-NOT-MERGE Pull requests that should not be merged label Aug 16, 2026
@github-actions github-actions Bot added the risk:high AI analysis: high risk label Aug 16, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c5909ba. Configure here.


return initMessenger.call('PreferencesController:getState')
.useTransactionSimulations;
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirm stuck after skipping simulation

High Severity

isSimulationEnabled now returns false for MetaMask Pay types while confirmation UI still reads useTransactionSimulations as the simulation signal. useIsGaslessLoading treats missing gasFeeTokens as still loading when that preference is on, and the footer disables confirm while that flag is true. For Pay flows with insufficient native balance on gasless-supported chains, gasFeeTokens never arrive, so confirm can stay disabled.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c5909ba. Configure here.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Performance Test Results

ℹ️ Performance test results are currently non-blocking and will not block this PR.

1 test failed · 7 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

❌ Failed Tests (1)

🔬 App profiling vs main is included under each failed scenario that has a prior baseline.

@mm-perps-engineering-team

Perps open position and close it

Platform Device Reason Recording
Android Google Pixel 8 Pro (v14.0) no_performance_metrics 📹 Watch

🔬 App profiling check · Current run 31941804590 · Baseline (last run on main (scenario also failing)) run 30897750395 @ 67486d2

⚠️ No green baseline on main — comparing against the latest usable profiling.

Summary: ⚠️ 4 metrics over +10%: CPU max (+6.62 (+35.2%)), Memory avg (+276.18 (+49.7%)), Memory max (+397.16 (+65.2%)), Slow frames (+13.33 (+258.3%))

ℹ️ API calls unavailable: Network logs API error: Bad Request

Full metric table (+10% variance rules)

Disclaimer — allowed variance: a +10% margin over the baseline is permitted.

  • If Current <= Baseline + 10%, treated as acceptable noise.
  • If Current > Baseline + 10%, Current and variance % are highlighted with ⚠️.
Metric Baseline Current Δ
CPU avg 10.44% 7.28% -3.16 (-30.3%)
CPU max 18.83% 25.45% +6.62 (+35.2%) ⚠️
Memory avg 555.64 MB 831.82 MB +276.18 (+49.7%) ⚠️
Memory max 609.03 MB 1006.19 MB +397.16 (+65.2%) ⚠️
Slow frames 5.16% 18.49% +13.33 (+258.3%) ⚠️
Frozen frames 0% 0% 0 (0%)
ANRs 0 0 0 (0%)
Issues 2 2 0 (0%)
Critical issues 1 1 0 (0%)
App size 328.67 MB 329.55 MB +0.88 (+0.3%)
✅ Passed Tests (6)
Test Platform Device Duration Team Recording
Money Home after fresh wallet creation with empty balance Android Google Pixel 8 Pro (v14.0) 3.56s @mm-earn-team 📹 Watch
Money Home after importing SRP with funded balance Android Google Pixel 8 Pro (v14.0) 3.28s @mm-earn-team 📹 Watch
Predict Available Balance - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 0.86s @team-predict 📹 Watch
Perps add funds Android Google Pixel 8 Pro (v14.0) 6.66s @mm-perps-engineering-team 📹 Watch
Predict Deposit - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 9.88s @team-predict 📹 Watch
Predict Market Details - Complete Flow Performance Android Google Pixel 8 Pro (v14.0) 5.55s @team-predict 📹 Watch

Branch: feat/simulation-disabled-mm-pay · Build: E2E · Commit: b715bf6 · View full run

@matthewwalsh0
matthewwalsh0 requested a review from jpuri August 17, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DO-NOT-MERGE Pull requests that should not be merged risk:high AI analysis: high risk size-S team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants