From 42bd11deff7aa61fbcdf08c3e1bdc731198866ba Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 19 Feb 2025 21:49:20 +0100 Subject: [PATCH] fix: cp-12.13.0 Disable origin throttling middleware if cause is "rejectAllApprovals" (#30388) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** 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 https://github.com/MetaMask/core/pull/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: https://github.com/MetaMask/metamask-extension/issues/30333 ## **Manual testing steps** See the task details ## **Screenshots/Recordings** ### **Before** ### **After** ## **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 --- ...ion-controller-npm-45.0.0-010fef9da6.patch | 17 +++++++++ app/scripts/lib/approval/utils.ts | 9 ++++- .../createOriginThrottlingMiddleware.test.ts | 33 +++++++++++++++- .../lib/createOriginThrottlingMiddleware.ts | 7 ++++ package.json | 2 +- yarn.lock | 38 ++++++++++++++++++- 6 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 .yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch diff --git a/.yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch b/.yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch new file mode 100644 index 000000000000..2e4df34a64f1 --- /dev/null +++ b/.yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch @@ -0,0 +1,17 @@ +diff --git a/dist/TransactionController.cjs b/dist/TransactionController.cjs +index c96d2962d81c4c63c7b626bc1ca18b196b20a154..236b451e5f94584d4ce42b6bfaf6b7225b8ae7f9 100644 +--- a/dist/TransactionController.cjs ++++ b/dist/TransactionController.cjs +@@ -1152,7 +1152,11 @@ class TransactionController extends base_controller_1.BaseController { + if (!isTxCompleted) { + if (error?.code === rpc_errors_1.errorCodes.provider.userRejectedRequest) { + this.cancelTransaction(transactionId, actionId); +- throw rpc_errors_1.providerErrors.userRejectedRequest('MetaMask Tx Signature: User denied transaction signature.'); ++ throw rpc_errors_1.providerErrors.userRejectedRequest({ ++ message: ++ 'MetaMask Tx Signature: User denied transaction signature.', ++ data: error?.data, ++ }); + } + else { + this.failTransaction(meta, error, actionId); diff --git a/app/scripts/lib/approval/utils.ts b/app/scripts/lib/approval/utils.ts index 5bd44b5db079..538478ea4652 100644 --- a/app/scripts/lib/approval/utils.ts +++ b/app/scripts/lib/approval/utils.ts @@ -49,7 +49,14 @@ export function rejectAllApprovals({ default: log('Rejecting pending approval', { id }); - approvalController.reject(id, providerErrors.userRejectedRequest()); + approvalController.reject( + id, + providerErrors.userRejectedRequest({ + data: { + cause: 'rejectAllApprovals', + }, + }), + ); break; } } diff --git a/app/scripts/lib/createOriginThrottlingMiddleware.test.ts b/app/scripts/lib/createOriginThrottlingMiddleware.test.ts index 820447b8bb64..93c5bf37276b 100644 --- a/app/scripts/lib/createOriginThrottlingMiddleware.test.ts +++ b/app/scripts/lib/createOriginThrottlingMiddleware.test.ts @@ -1,4 +1,4 @@ -import { errorCodes } from '@metamask/rpc-errors'; +import { errorCodes, providerErrors } from '@metamask/rpc-errors'; import { JsonRpcResponse } from '@metamask/utils'; import type { Json } from '@metamask/utils'; import createOriginThrottlingMiddleware, { @@ -118,4 +118,35 @@ describe('createOriginThrottlingMiddleware', () => { }); expect(nextCallback).toHaveBeenCalled(); }); + + it('does not update throttling state if response has userRejected error and rejectAllApprovals is in the error data', async () => { + const req = { + method: 'eth_sendTransaction', + origin: 'testOrigin', + } as unknown as ExtendedJSONRPCRequest; + const nextCallback = jest.fn(); + const next = jest + .fn() + .mockImplementation((callback) => callback(nextCallback)); + const end = jest.fn(); + const responseWithUserRejectedError = { + error: providerErrors.userRejectedRequest({ + data: { + cause: 'rejectAllApprovals', + }, + }), + id: 1, + jsonrpc: '2.0', + } as unknown as JsonRpcResponse; + + mockGetThrottledOriginState.mockReturnValueOnce({ + rejections: 0, + lastRejection: 0, + }); + + await middleware(req, responseWithUserRejectedError, next, end); + + expect(mockUpdateThrottledOriginState).not.toHaveBeenCalled(); + expect(nextCallback).toHaveBeenCalled(); + }); }); diff --git a/app/scripts/lib/createOriginThrottlingMiddleware.ts b/app/scripts/lib/createOriginThrottlingMiddleware.ts index 066da679ae8a..570276746f44 100644 --- a/app/scripts/lib/createOriginThrottlingMiddleware.ts +++ b/app/scripts/lib/createOriginThrottlingMiddleware.ts @@ -83,6 +83,13 @@ export default function createOriginThrottlingMiddleware({ next((callback: () => void) => { if ('error' in res && res.error && isUserRejectedError(res.error)) { + const extraData = res.error?.data as { cause?: string }; + // Any rejection caused by rejectAllApprovals is not evaluated as user rejection for now + if (extraData?.cause === 'rejectAllApprovals') { + callback(); + return; + } + // User rejected the request const throttledOriginState = getThrottledOriginState(origin) || { rejections: 0, diff --git a/package.json b/package.json index 173f1c240d7c..8f92fc96de22 100644 --- a/package.json +++ b/package.json @@ -359,7 +359,7 @@ "@metamask/snaps-sdk": "^6.18.0", "@metamask/snaps-utils": "^9.0.0", "@metamask/solana-wallet-snap": "^1.2.0", - "@metamask/transaction-controller": "^45.0.0", + "@metamask/transaction-controller": "patch:@metamask/transaction-controller@npm%3A45.0.0#~/.yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch", "@metamask/user-operation-controller": "^24.0.1", "@metamask/utils": "^10.0.1", "@ngraveio/bc-ur": "^1.1.12", diff --git a/yarn.lock b/yarn.lock index 875664b1f752..6a33351cc594 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6491,7 +6491,7 @@ __metadata: languageName: node linkType: hard -"@metamask/transaction-controller@npm:^45.0.0": +"@metamask/transaction-controller@npm:45.0.0": version: 45.0.0 resolution: "@metamask/transaction-controller@npm:45.0.0" dependencies: @@ -6525,6 +6525,40 @@ __metadata: languageName: node linkType: hard +"@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 + 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" + dependencies: + "@ethereumjs/common": "npm:^3.2.0" + "@ethereumjs/tx": "npm:^4.2.0" + "@ethereumjs/util": "npm:^8.1.0" + "@ethersproject/abi": "npm:^5.7.0" + "@ethersproject/contracts": "npm:^5.7.0" + "@ethersproject/providers": "npm:^5.7.0" + "@metamask/base-controller": "npm:^7.1.1" + "@metamask/controller-utils": "npm:^11.5.0" + "@metamask/eth-query": "npm:^4.0.0" + "@metamask/metamask-eth-abis": "npm:^3.1.1" + "@metamask/nonce-tracker": "npm:^6.0.0" + "@metamask/rpc-errors": "npm:^7.0.2" + "@metamask/utils": "npm:^11.1.0" + async-mutex: "npm:^0.5.0" + bn.js: "npm:^5.2.1" + eth-method-registry: "npm:^4.0.0" + fast-json-patch: "npm:^3.1.1" + lodash: "npm:^4.17.21" + uuid: "npm:^8.3.2" + peerDependencies: + "@babel/runtime": ^7.0.0 + "@metamask/accounts-controller": ^23.0.0 + "@metamask/approval-controller": ^7.0.0 + "@metamask/eth-block-tracker": ">=9" + "@metamask/gas-fee-controller": ^22.0.0 + "@metamask/network-controller": ^22.0.0 + checksum: 10/b65d8d04031c3bbba2f95dcf66d22bcd0ef83c66fb3b341943ff6d81befbf370f826e9b9f0d73fccf3686a6550c2c0d3e80ad394618273964cc5efc1cb1d31fe + languageName: node + linkType: hard + "@metamask/user-operation-controller@npm:^24.0.1": version: 24.0.1 resolution: "@metamask/user-operation-controller@npm:24.0.1" @@ -26876,7 +26910,7 @@ __metadata: "@metamask/solana-wallet-snap": "npm:^1.2.0" "@metamask/test-bundler": "npm:^1.0.0" "@metamask/test-dapp": "npm:9.0.0" - "@metamask/transaction-controller": "npm:^45.0.0" + "@metamask/transaction-controller": "patch:@metamask/transaction-controller@npm%3A45.0.0#~/.yarn/patches/@metamask-transaction-controller-npm-45.0.0-010fef9da6.patch" "@metamask/user-operation-controller": "npm:^24.0.1" "@metamask/utils": "npm:^10.0.1" "@ngraveio/bc-ur": "npm:^1.1.12"