Skip to content

Commit

Permalink
fix: exclude smart transaction status page from rate limiting (#30537)
Browse files Browse the repository at this point in the history
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.

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **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](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30537?quickstart=1)

## **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
```
2. 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
```
8. Install the updated MetaMask extension build containing the fix
9. Connect to Ethereum Mainnet
10. Navigate to the test dapp
11. Connect your MetaMask wallet to the dapp
12. Click the "Send 3 Sequential Transactions" button
13. Approve each transaction when prompted by MetaMask
14. Verify no console errors containing 
```
Request of type 'smartTransaction:showSmartTransactionStatusPage' already pending
```
15. Confirm that each transaction is processed correctly
16. Verify that the status page for each transaction is displayed
properly
17. 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**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/user-attachments/assets/efb9c501-835c-4259-a3ec-20400a80cf53

### **After**


https://github.com/user-attachments/assets/92772ca7-7da5-4a3e-8cee-46fd16784ea4

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.
  • Loading branch information
httpJunkie authored Feb 26, 2025
1 parent 4e2b1d4 commit 838e4cb
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ import { MILLISECOND, MINUTE, SECOND } from '../../shared/constants/time';
import {
ORIGIN_METAMASK,
POLLING_TOKEN_ENVIRONMENT_TYPES,
SMART_TRANSACTION_CONFIRMATION_TYPES,
} from '../../shared/constants/app';
import {
MetaMetricsEventCategory,
Expand Down Expand Up @@ -527,6 +528,8 @@ export default class MetamaskController extends EventEmitter {
ApprovalType.WatchAsset,
ApprovalType.EthGetEncryptionPublicKey,
ApprovalType.EthDecrypt,
// Exclude Smart TX Status Page from rate limiting to allow sequential transactions
SMART_TRANSACTION_CONFIRMATION_TYPES.showSmartTransactionStatusPage,
],
});

Expand Down

0 comments on commit 838e4cb

Please sign in to comment.