Skip to content

Commit

Permalink
chore(deps): upgrade from json-rpc-engine to @metamask/json-rpc-engine (
Browse files Browse the repository at this point in the history
#22875)

## **Description**

## **Related issues**

- #27784
- MetaMask/eth-json-rpc-middleware#335
- #27917
- #18510
- #15250
- MetaMask/metamask-improvement-proposals#36

### Blocked by
- [x] #24496

### Follow-up to
- #24496

## **Manual testing steps**

## **Screenshots/Recordings**

### **Before**

### **After**

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
  • Loading branch information
legobeat authored and vinistevam committed Oct 20, 2024
1 parent fcce0cc commit dd34756
Show file tree
Hide file tree
Showing 33 changed files with 535 additions and 242 deletions.
3 changes: 1 addition & 2 deletions app/scripts/controllers/preferences-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ import {
AccountsControllerSetSelectedAccountAction,
AccountsControllerState,
} from '@metamask/accounts-controller';
import { Hex } from '@metamask/utils';
import { Hex, Json } from '@metamask/utils';
import {
BaseController,
ControllerGetStateAction,
ControllerStateChangeEvent,
RestrictedControllerMessenger,
} from '@metamask/base-controller';
import { Json } from 'json-rpc-engine';
import { NetworkControllerGetStateAction } from '@metamask/network-controller';
import {
ETHERSCAN_SUPPORTED_CHAIN_IDS,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { jsonrpc2 } from '@metamask/utils';
import { jsonrpc2, Json } from '@metamask/utils';
import { BtcAccountType, EthAccountType } from '@metamask/keyring-api';
import { Json } from 'json-rpc-engine';
import type { JsonRpcParams, JsonRpcRequest } from '@metamask/utils';
import createEvmMethodsToNonEvmAccountReqFilterMiddleware, {
EvmMethodsToNonEvmAccountFilterMessenger,
} from './createEvmMethodsToNonEvmAccountReqFilterMiddleware';

describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
const getMockRequest = (method: string, params?: Json) => ({
const getMockRequest = (method: string, params: Json) => ({
jsonrpc: jsonrpc2,
id: 1,
method,
Expand All @@ -20,185 +20,221 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
{
accountType: BtcAccountType.P2wpkh,
method: 'eth_accounts',
params: undefined,
calledNext: false,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'eth_sendRawTransaction',
params: undefined,
calledNext: false,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'eth_sendTransaction',
params: undefined,
calledNext: false,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'eth_signTypedData',
params: undefined,
calledNext: false,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'eth_signTypedData_v1',
params: undefined,
calledNext: false,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'eth_signTypedData_v3',
params: undefined,
calledNext: false,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'eth_signTypedData_v4',
params: undefined,
calledNext: false,
},
{
accountType: EthAccountType.Eoa,
method: 'eth_accounts',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'eth_sendRawTransaction',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'eth_sendTransaction',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'eth_signTypedData',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'eth_signTypedData_v1',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'eth_signTypedData_v3',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'eth_signTypedData_v4',
params: undefined,
calledNext: true,
},

// EVM requests not associated with an account
{
accountType: BtcAccountType.P2wpkh,
method: 'eth_blockNumber',
params: undefined,
calledNext: true,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'eth_chainId',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'eth_blockNumber',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'eth_chainId',
params: undefined,
calledNext: true,
},

// other requests
{
accountType: BtcAccountType.P2wpkh,
method: 'wallet_getSnaps',
params: undefined,
calledNext: true,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'wallet_invokeSnap',
params: undefined,
calledNext: true,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'wallet_requestSnaps',
params: undefined,
calledNext: true,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'snap_getClientStatus',
params: undefined,
calledNext: true,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'wallet_addEthereumChain',
params: undefined,
calledNext: true,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'wallet_getPermissions',
params: undefined,
calledNext: true,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'wallet_requestPermissions',
params: undefined,
calledNext: true,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'wallet_revokePermissions',
params: undefined,
calledNext: true,
},
{
accountType: BtcAccountType.P2wpkh,
method: 'wallet_switchEthereumChain',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'wallet_getSnaps',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'wallet_invokeSnap',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'wallet_requestSnaps',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'snap_getClientStatus',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'wallet_addEthereumChain',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'wallet_getPermissions',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'wallet_requestPermissions',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'wallet_revokePermissions',
params: undefined,
calledNext: true,
},
{
accountType: EthAccountType.Eoa,
method: 'wallet_switchEthereumChain',
params: undefined,
calledNext: true,
},

Expand Down Expand Up @@ -250,7 +286,7 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
}: {
accountType: EthAccountType | BtcAccountType;
method: string;
params?: Json;
params: Json;
calledNext: number;
}) => {
const filterFn = createEvmMethodsToNonEvmAccountReqFilterMiddleware({
Expand All @@ -262,7 +298,7 @@ describe('createEvmMethodsToNonEvmAccountReqFilterMiddleware', () => {
const mockEnd = jest.fn();

filterFn(
getMockRequest(method, params),
getMockRequest(method, params) as JsonRpcRequest<JsonRpcParams>,
getMockResponse(),
mockNext,
mockEnd,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { isEvmAccountType } from '@metamask/keyring-api';
import { RestrictedControllerMessenger } from '@metamask/base-controller';
import { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller';
import { JsonRpcMiddleware } from 'json-rpc-engine';
import { JsonRpcMiddleware } from '@metamask/json-rpc-engine';
import type { Json, JsonRpcParams } from '@metamask/utils';
import { RestrictedEthMethods } from '../../../shared/constants/permissions';
import { unrestrictedEthSigningMethods } from '../controllers/permissions';

Expand Down Expand Up @@ -32,7 +33,7 @@ export default function createEvmMethodsToNonEvmAccountReqFilterMiddleware({
messenger,
}: {
messenger: EvmMethodsToNonEvmAccountFilterMessenger;
}): JsonRpcMiddleware<unknown, void> {
}): JsonRpcMiddleware<JsonRpcParams, Json> {
return function filterEvmRequestToNonEvmAccountsMiddleware(
req,
_res,
Expand Down Expand Up @@ -74,7 +75,13 @@ export default function createEvmMethodsToNonEvmAccountReqFilterMiddleware({
// TODO: Convert this to superstruct schema
const isWalletRequestPermission =
req.method === 'wallet_requestPermissions';
if (isWalletRequestPermission && req?.params && Array.isArray(req.params)) {
if (
isWalletRequestPermission &&
req?.params &&
Array.isArray(req.params) &&
req.params.length > 0 &&
req.params[0]
) {
const permissionsMethodRequest = Object.keys(req.params[0]);

const isEvmPermissionRequest = METHODS_TO_CHECK.some((method) =>
Expand Down
5 changes: 4 additions & 1 deletion app/scripts/lib/createMetamaskMiddleware.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { createScaffoldMiddleware, mergeMiddleware } from 'json-rpc-engine';
import {
createScaffoldMiddleware,
mergeMiddleware,
} from '@metamask/json-rpc-engine';
import { createWalletMiddleware } from '@metamask/eth-json-rpc-middleware';
import {
createPendingNonceMiddleware,
Expand Down
10 changes: 8 additions & 2 deletions app/scripts/lib/createRPCMethodTrackingMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
PRIMARY_TYPES_PERMIT,
} from '../../../shared/constants/signatures';
import { SIGNING_METHODS } from '../../../shared/constants/transaction';
import { getErrorMessage } from '../../../shared/modules/error';
import {
generateSignatureUniqueId,
getBlockaidMetricsProps,
Expand Down Expand Up @@ -419,15 +420,20 @@ export default function createRPCMethodTrackingMiddleware({
const location = res.error?.data?.location;

let event;

const errorMessage = getErrorMessage(res.error);

if (res.error?.code === errorCodes.provider.userRejectedRequest) {
event = eventType.REJECTED;
} else if (
res.error?.code === errorCodes.rpc.internal &&
res.error?.message === 'Request rejected by user or snap.'
[errorMessage, res.error.message].includes(
'Request rejected by user or snap.',
)
) {
// The signature was approved in MetaMask but rejected in the snap
event = eventType.REJECTED;
eventProperties.status = res.error.message;
eventProperties.status = errorMessage;
} else {
event = eventType.APPROVED;
}
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/lib/middleware/pending.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createAsyncMiddleware } from 'json-rpc-engine';
import { createAsyncMiddleware } from '@metamask/json-rpc-engine';
import { formatTxMetaForRpcResult } from '../util';

export function createPendingNonceMiddleware({ getPendingNonce }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function makeMethodMiddlewareMaker(handlers) {
*
* @param {Record<string, (...args: unknown[]) => unknown | Promise<unknown>>} hooks - Required "hooks" into our
* controllers.
* @returns {import('json-rpc-engine').JsonRpcMiddleware<unknown, unknown>} The method middleware function.
* @returns {import('@metamask/json-rpc-engine').JsonRpcMiddleware<unknown, unknown>} The method middleware function.
*/
const makeMethodMiddleware = (hooks) => {
assertExpectedHook(hooks, expectedHookNames);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { JsonRpcEngine } from 'json-rpc-engine';
import { JsonRpcEngine } from '@metamask/json-rpc-engine';
import {
assertIsJsonRpcFailure,
assertIsJsonRpcSuccess,
Expand Down Expand Up @@ -140,6 +140,7 @@ describe.each([
assertIsJsonRpcFailure(response);

expect(response.error.message).toBe('test error');
expect(response.error.data.cause.message).toBe('test error');
});

it('should handle errors thrown by the implementation', async () => {
Expand All @@ -156,6 +157,7 @@ describe.each([
assertIsJsonRpcFailure(response);

expect(response.error.message).toBe('test error');
expect(response.error.data.cause.message).toBe('test error');
});

it('should handle non-errors thrown by the implementation', async () => {
Expand Down
Loading

0 comments on commit dd34756

Please sign in to comment.