Skip to content
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

fix: exclude smart transaction status page from rate limiting #30537

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

httpJunkie
Copy link
Contributor

@httpJunkie httpJunkie commented Feb 24, 2025

This change adds smartTransaction:showSmartTransactionStatusPage to the typesExcludedFromRateLimiting array when initializing the ApprovalController.

This prevents the "Request of type 'smartTransaction:showSmartTransactionStatusPage' already pending" error that was occurring when users executed sequential transactions in quick succession.

Description

Before the fix, MetaMask's ApprovalController was rate-limiting sequential smartTransaction:showSmartTransactionStatusPage requests, causing errors when users attempted multiple transactions.

What is being Rate Limited by the typesExcludedFromRateLimiting?

The rate limiting is happening on API request types within MetaMask's internal architecture. Specifically: When a user initiates a transaction ie: eth_sendTransaction, MetaMask makes an internal request of type smartTransaction:showSmartTransactionStatusPage.

A "request type" in this context refers to a specific category of internal API calls within MetaMask's architecture. Each request type is a string identifier (like smartTransaction:showSmartTransactionStatusPage) that represents an operation that extension needs to perform. The ApprovalController tracks these "request types" to manage permissions, prevent spam, and handle confirmations from users.

The ApprovalController component in MetaMask was designed to prevent many identical "request types" from the same source in a short time period, ie: "rate limiting".

This means that when a user tried to make multiple transactions quickly (simulated by the test dapp included below), they could see the status page for the first transaction, but subsequent status pages were being BLOCKED by this rate-limiting mechanism.

Our fix tells the ApprovalController to exclude this particular request type from its rate limiting checks, so that multiple transaction status pages can be displayed in sequence without error.

Open in GitHub Codespaces

Related issues

Fixes: #30387

Manual testing steps

Repro Setup:

  1. Run the following command to clone and run the test dapp that reproduces this error:
git clone https://github.com/httpJunkie/metamask-sequential-tx-repro.git &&
cd metamask-sequential-tx-repro && yarn && yarn dev
  1. Cmd+Click on the localhost URL in terminal to run the dapp

Manually Test the Extension:

  1. Load a version 12.13.0 or earlier version of MetaMask
  2. Connect to Ethereum Mainnet (testnet seems harder to repro this bug)
  3. Navigate to the test dapp
  4. Connect your MetaMask wallet to the dapp
  5. Click the "Send 3 Sequential Transactions" button
  6. Approve transactions as prompted by MetaMask
  7. Verify console errors containing
Request of type 'smartTransaction:showSmartTransactionStatusPage' already pending
  1. Install the updated MetaMask extension build containing the fix
  2. Connect to Ethereum Mainnet
  3. Navigate to the test dapp
  4. Connect your MetaMask wallet to the dapp
  5. Click the "Send 3 Sequential Transactions" button
  6. Approve each transaction when prompted by MetaMask
  7. Verify no console errors containing
Request of type 'smartTransaction:showSmartTransactionStatusPage' already pending
  1. Confirm that each transaction is processed correctly
  2. Verify that the status page for each transaction is displayed properly
  3. Ensure all three transactions complete successfully

Expected Results

  • All transactions should execute without errors
  • Each transaction should display its status page
  • No errors should appear in the console

Screenshots/Recordings

Before

before-seq-tx.mov

After

after-seq-tx.mov

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.

This change adds 'smartTransaction:showSmartTransactionStatusPage' to the
typesExcludedFromRateLimiting array when initializing the ApprovalController.

This prevents the "Request of type 'smartTransaction:showSmartTransactionStatusPage'
already pending" error that was occurring when users executed sequential
transactions in quick succession.

Fixes TXL-679
@httpJunkie httpJunkie added team-transactions Transactions team release-12.14.0 Issue or pull request that will be included in release 12.14.0 labels Feb 24, 2025
@httpJunkie httpJunkie self-assigned this Feb 24, 2025
Copy link
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.

@metamaskbot
Copy link
Collaborator

Builds ready [8a9a42a]
Page Load Metrics (1726 ± 61 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint36719461643314151
domContentLoaded14771917168912158
load15311990172612661
domInteractive269343209
backgroundConnect1287412311
firstReactRender1575402211
getState681242311
initialActions01000
loadScripts1095143312449847
setupStore76513126
uiStartup17382208197515474
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 50 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@httpJunkie
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@metamaskbot
Copy link
Collaborator

Builds ready [646d34e]
Page Load Metrics (1645 ± 60 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14501867164512359
domContentLoaded14431832161911756
load14511869164512460
domInteractive256734115
backgroundConnect877322210
firstReactRender1478282110
getState45111115
initialActions01000
loadScripts1052135511989445
setupStore76518199
uiStartup16792154187514469
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 50 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-12.14.0 Issue or pull request that will be included in release 12.14.0 team-transactions Transactions team
Projects
None yet
2 participants