Skip to content

Commit 42bd11d

Browse files
fix: cp-12.13.0 Disable origin throttling middleware if cause is "rejectAllApprovals" (#30388)
<!-- 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** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> This PR aims to fix `release-blocker` in `v12.13.0` which disables origin throttling middleware if `cause` is `rejectAllApprovals`. Patch note: Currently bumping `transaction-controller` is blocked because it's breaking e2e tests https://consensys.slack.com/archives/CTQAGKY5V/p1739968847746929?thread_ts=1739869734.799669&cid=CTQAGKY5V To avoid applying version newer version of `transaction-controller` needed change patched in this PR. Required change is already in the core repository MetaMask/core#5355. And will most likely be released in version `46.0.1`. Then we will most likely bump `transaction-controller` in `main`. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30388?quickstart=1) ## **Related issues** Fixes: #30333 ## **Manual testing steps** See the task details ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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** - [ ] 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. --------- Co-authored-by: MetaMask Bot <[email protected]>
1 parent 468ab8f commit 42bd11d

6 files changed

+101
-5
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
diff --git a/dist/TransactionController.cjs b/dist/TransactionController.cjs
2+
index c96d2962d81c4c63c7b626bc1ca18b196b20a154..236b451e5f94584d4ce42b6bfaf6b7225b8ae7f9 100644
3+
--- a/dist/TransactionController.cjs
4+
+++ b/dist/TransactionController.cjs
5+
@@ -1152,7 +1152,11 @@ class TransactionController extends base_controller_1.BaseController {
6+
if (!isTxCompleted) {
7+
if (error?.code === rpc_errors_1.errorCodes.provider.userRejectedRequest) {
8+
this.cancelTransaction(transactionId, actionId);
9+
- throw rpc_errors_1.providerErrors.userRejectedRequest('MetaMask Tx Signature: User denied transaction signature.');
10+
+ throw rpc_errors_1.providerErrors.userRejectedRequest({
11+
+ message:
12+
+ 'MetaMask Tx Signature: User denied transaction signature.',
13+
+ data: error?.data,
14+
+ });
15+
}
16+
else {
17+
this.failTransaction(meta, error, actionId);

app/scripts/lib/approval/utils.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,14 @@ export function rejectAllApprovals({
4949

5050
default:
5151
log('Rejecting pending approval', { id });
52-
approvalController.reject(id, providerErrors.userRejectedRequest());
52+
approvalController.reject(
53+
id,
54+
providerErrors.userRejectedRequest({
55+
data: {
56+
cause: 'rejectAllApprovals',
57+
},
58+
}),
59+
);
5360
break;
5461
}
5562
}

app/scripts/lib/createOriginThrottlingMiddleware.test.ts

+32-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { errorCodes } from '@metamask/rpc-errors';
1+
import { errorCodes, providerErrors } from '@metamask/rpc-errors';
22
import { JsonRpcResponse } from '@metamask/utils';
33
import type { Json } from '@metamask/utils';
44
import createOriginThrottlingMiddleware, {
@@ -118,4 +118,35 @@ describe('createOriginThrottlingMiddleware', () => {
118118
});
119119
expect(nextCallback).toHaveBeenCalled();
120120
});
121+
122+
it('does not update throttling state if response has userRejected error and rejectAllApprovals is in the error data', async () => {
123+
const req = {
124+
method: 'eth_sendTransaction',
125+
origin: 'testOrigin',
126+
} as unknown as ExtendedJSONRPCRequest;
127+
const nextCallback = jest.fn();
128+
const next = jest
129+
.fn()
130+
.mockImplementation((callback) => callback(nextCallback));
131+
const end = jest.fn();
132+
const responseWithUserRejectedError = {
133+
error: providerErrors.userRejectedRequest({
134+
data: {
135+
cause: 'rejectAllApprovals',
136+
},
137+
}),
138+
id: 1,
139+
jsonrpc: '2.0',
140+
} as unknown as JsonRpcResponse<Json>;
141+
142+
mockGetThrottledOriginState.mockReturnValueOnce({
143+
rejections: 0,
144+
lastRejection: 0,
145+
});
146+
147+
await middleware(req, responseWithUserRejectedError, next, end);
148+
149+
expect(mockUpdateThrottledOriginState).not.toHaveBeenCalled();
150+
expect(nextCallback).toHaveBeenCalled();
151+
});
121152
});

app/scripts/lib/createOriginThrottlingMiddleware.ts

+7
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,13 @@ export default function createOriginThrottlingMiddleware({
8383

8484
next((callback: () => void) => {
8585
if ('error' in res && res.error && isUserRejectedError(res.error)) {
86+
const extraData = res.error?.data as { cause?: string };
87+
// Any rejection caused by rejectAllApprovals is not evaluated as user rejection for now
88+
if (extraData?.cause === 'rejectAllApprovals') {
89+
callback();
90+
return;
91+
}
92+
8693
// User rejected the request
8794
const throttledOriginState = getThrottledOriginState(origin) || {
8895
rejections: 0,

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@
359359
"@metamask/snaps-sdk": "^6.18.0",
360360
"@metamask/snaps-utils": "^9.0.0",
361361
"@metamask/solana-wallet-snap": "^1.2.0",
362-
"@metamask/transaction-controller": "^45.0.0",
362+
"@metamask/transaction-controller": "patch:@metamask/transaction-controller@npm%3A45.0.0#~/.yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch",
363363
"@metamask/user-operation-controller": "^24.0.1",
364364
"@metamask/utils": "^10.0.1",
365365
"@ngraveio/bc-ur": "^1.1.12",

yarn.lock

+36-2
Original file line numberDiff line numberDiff line change
@@ -6491,7 +6491,7 @@ __metadata:
64916491
languageName: node
64926492
linkType: hard
64936493

6494-
"@metamask/transaction-controller@npm:^45.0.0":
6494+
"@metamask/transaction-controller@npm:45.0.0":
64956495
version: 45.0.0
64966496
resolution: "@metamask/transaction-controller@npm:45.0.0"
64976497
dependencies:
@@ -6525,6 +6525,40 @@ __metadata:
65256525
languageName: node
65266526
linkType: hard
65276527

6528+
"@metamask/transaction-controller@patch:@metamask/transaction-controller@npm%3A45.0.0#~/.yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch":
6529+
version: 45.0.0
6530+
resolution: "@metamask/transaction-controller@patch:@metamask/transaction-controller@npm%3A45.0.0#~/.yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch::version=45.0.0&hash=e00dfe"
6531+
dependencies:
6532+
"@ethereumjs/common": "npm:^3.2.0"
6533+
"@ethereumjs/tx": "npm:^4.2.0"
6534+
"@ethereumjs/util": "npm:^8.1.0"
6535+
"@ethersproject/abi": "npm:^5.7.0"
6536+
"@ethersproject/contracts": "npm:^5.7.0"
6537+
"@ethersproject/providers": "npm:^5.7.0"
6538+
"@metamask/base-controller": "npm:^7.1.1"
6539+
"@metamask/controller-utils": "npm:^11.5.0"
6540+
"@metamask/eth-query": "npm:^4.0.0"
6541+
"@metamask/metamask-eth-abis": "npm:^3.1.1"
6542+
"@metamask/nonce-tracker": "npm:^6.0.0"
6543+
"@metamask/rpc-errors": "npm:^7.0.2"
6544+
"@metamask/utils": "npm:^11.1.0"
6545+
async-mutex: "npm:^0.5.0"
6546+
bn.js: "npm:^5.2.1"
6547+
eth-method-registry: "npm:^4.0.0"
6548+
fast-json-patch: "npm:^3.1.1"
6549+
lodash: "npm:^4.17.21"
6550+
uuid: "npm:^8.3.2"
6551+
peerDependencies:
6552+
"@babel/runtime": ^7.0.0
6553+
"@metamask/accounts-controller": ^23.0.0
6554+
"@metamask/approval-controller": ^7.0.0
6555+
"@metamask/eth-block-tracker": ">=9"
6556+
"@metamask/gas-fee-controller": ^22.0.0
6557+
"@metamask/network-controller": ^22.0.0
6558+
checksum: 10/b65d8d04031c3bbba2f95dcf66d22bcd0ef83c66fb3b341943ff6d81befbf370f826e9b9f0d73fccf3686a6550c2c0d3e80ad394618273964cc5efc1cb1d31fe
6559+
languageName: node
6560+
linkType: hard
6561+
65286562
"@metamask/user-operation-controller@npm:^24.0.1":
65296563
version: 24.0.1
65306564
resolution: "@metamask/user-operation-controller@npm:24.0.1"
@@ -26876,7 +26910,7 @@ __metadata:
2687626910
"@metamask/solana-wallet-snap": "npm:^1.2.0"
2687726911
"@metamask/test-bundler": "npm:^1.0.0"
2687826912
"@metamask/test-dapp": "npm:9.0.0"
26879-
"@metamask/transaction-controller": "npm:^45.0.0"
26913+
"@metamask/transaction-controller": "patch:@metamask/transaction-controller@npm%3A45.0.0#~/.yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch"
2688026914
"@metamask/user-operation-controller": "npm:^24.0.1"
2688126915
"@metamask/utils": "npm:^10.0.1"
2688226916
"@ngraveio/bc-ur": "npm:^1.1.12"

0 commit comments

Comments
 (0)