Skip to content

Commit

Permalink
fix: cp-12.13.0 Disable origin throttling middleware if cause is "rej…
Browse files Browse the repository at this point in the history
…ectAllApprovals" (#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]>
  • Loading branch information
OGPoyraz and metamaskbot authored Feb 19, 2025
1 parent 468ab8f commit 42bd11d
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -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);
9 changes: 8 additions & 1 deletion app/scripts/lib/approval/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
33 changes: 32 additions & 1 deletion app/scripts/lib/createOriginThrottlingMiddleware.test.ts
Original file line number Diff line number Diff line change
@@ -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, {
Expand Down Expand Up @@ -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<Json>;

mockGetThrottledOriginState.mockReturnValueOnce({
rejections: 0,
lastRejection: 0,
});

await middleware(req, responseWithUserRejectedError, next, end);

expect(mockUpdateThrottledOriginState).not.toHaveBeenCalled();
expect(nextCallback).toHaveBeenCalled();
});
});
7 changes: 7 additions & 0 deletions app/scripts/lib/createOriginThrottlingMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
38 changes: 36 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 42bd11d

Please sign in to comment.