Skip to content

Commit

Permalink
chore(runway): cherry-pick fix: replace legacy eth-json-rpc deps (#12700
Browse files Browse the repository at this point in the history
)

- fix: replace legacy eth-json-rpc deps (#11952)

## **Description**

- This PR is a rebase of #10098, including:
    * #9925
    * #9930
- Bump `@metamask/eth-json-rpc-filters` to `^7.0.0`
- Bump `@metamask/json-rpc-engine` to `^10.0.0`
- Bump `@metamask/eth-json-rpc-middleware` to `^15.0.0`
- Migrate from `json-rpc-middleware-stream` to
`@metamask/json-rpc-middleware-stream`
- Upgrade `@metamask/providers` from v13 to v16
  - Also broken out separately as #12085 
- Revert `Internal JSON-RPC error` message change to accomodate for
`@metamask/rpc-errors` v7


## **Related issues**

Expected to fix the following issues:

- [x] #11163
- [x] #11129
- [ ] #11105
- [ ] #9715
- [ ] #8308
- [x] #7926
- [x] #4621
- [x] #4646
- [ ] #12634

#### Blocked by
- [x] #12085
- [x] #12047
  - [x] #12024
    - [x] #11980 
- [x] #12008
- [x] #11978 

## **Manual testing steps**

1. Go to in-app browser
2. Test connect with multiple dapps
3. Perform transaciton on test dapp
1. Go to this page...

## **Screenshots/Recordings**



https://github.com/MetaMask/metamask-mobile/assets/46944231/c608d957-6684-40e2-8963-67a11dc610df

### **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 Mobile
Coding

Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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: Aslau Mario-Daniel <[email protected]>
Co-authored-by: tommasini <[email protected]>
Co-authored-by: kylanhurt <[email protected]>
Co-authored-by: sethkfman <[email protected]>
Co-authored-by: Nicolas MASSART <[email protected]>
Co-authored-by: Nico MASSART <[email protected]>
[d967a76](d967a76)

Co-authored-by: legobeat <[email protected]>
Co-authored-by: Aslau Mario-Daniel <[email protected]>
Co-authored-by: tommasini <[email protected]>
Co-authored-by: kylanhurt <[email protected]>
Co-authored-by: sethkfman <[email protected]>
Co-authored-by: Nicolas MASSART <[email protected]>
Co-authored-by: Nico MASSART <[email protected]>
  • Loading branch information
8 people authored Dec 13, 2024
1 parent 17c1611 commit 2e566df
Show file tree
Hide file tree
Showing 31 changed files with 120 additions and 147 deletions.
12 changes: 4 additions & 8 deletions app/core/BackgroundBridge/BackgroundBridge.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/* eslint-disable import/no-commonjs */
import URL from 'url-parse';
import { JsonRpcEngine } from 'json-rpc-engine';
import {
createSelectedNetworkMiddleware,
METAMASK_DOMAIN,
} from '@metamask/selected-network-controller';
import EthQuery from '@metamask/eth-query';
import { JsonRpcEngine } from '@metamask/json-rpc-engine';
import MobilePortStream from '../MobilePortStream';
import { setupMultiplex } from '../../util/streams';
import {
Expand All @@ -25,10 +25,10 @@ import snapMethodMiddlewareBuilder from '../Snaps/SnapsMethodMiddleware';
import { SubjectType } from '@metamask/permission-controller';
///: END:ONLY_INCLUDE_IF

import { createEngineStream } from '@metamask/json-rpc-middleware-stream';
const createFilterMiddleware = require('@metamask/eth-json-rpc-filters');
const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager');
const { providerAsMiddleware } = require('@metamask/eth-json-rpc-middleware');
import { createEngineStream } from '@metamask/json-rpc-middleware-stream';
import { providerAsMiddleware } from '@metamask/eth-json-rpc-middleware';
const pump = require('pump');
// eslint-disable-next-line import/no-nodejs-modules
const EventEmitter = require('events').EventEmitter;
Expand Down Expand Up @@ -388,11 +388,7 @@ export class BackgroundBridge extends EventEmitter {

pump(outStream, providerStream, outStream, (err) => {
// handle any middleware cleanup
this.engine._middleware.forEach((mid) => {
if (mid.destroy && typeof mid.destroy === 'function') {
mid.destroy();
}
});
this.engine.destroy();
if (err) Logger.log('Error with provider stream conn', err);
});
}
Expand Down
2 changes: 1 addition & 1 deletion app/core/Encryptor/pbkdf2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('pbkdf2', () => {
afterEach(() => {
jest.restoreAllMocks();
});

it('uses the native implementation of pbkdf2 with main aes', async () => {
NativeModules.Aes.pbkdf2 = jest
.fn()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export { createRemoteFeatureFlagController } from './utils';

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { RemoteFeatureFlagControllerMessenger, RemoteFeatureFlagControllerState

export interface RemoteFeatureFlagInitParamTypes {
state?: RemoteFeatureFlagControllerState;
messenger: RemoteFeatureFlagControllerMessenger,
disabled: boolean
messenger: RemoteFeatureFlagControllerMessenger;
disabled: boolean;
}

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ describe('RemoteFeatureFlagController utils', () => {

describe('createRemoteFeatureFlagController', () => {
it('creates controller with initial undefined state', () => {

const controller = createRemoteFeatureFlagController({
state: undefined,
messenger,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export const createRemoteFeatureFlagController = ({
messenger,
disabled,
}: RemoteFeatureFlagInitParamTypes) => {

const remoteFeatureFlagController = new RemoteFeatureFlagController({
messenger,
state,
Expand All @@ -58,4 +57,3 @@ export const createRemoteFeatureFlagController = ({
}
return remoteFeatureFlagController;
};

1 change: 0 additions & 1 deletion app/core/Ledger/Ledger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,4 +251,3 @@ export const unlockLedgerWalletAccount = async (index: number) => {
}
};


32 changes: 19 additions & 13 deletions app/core/RPCMethods/RPCMethodMiddleware.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import {
JsonRpcEngine,
import { JsonRpcEngine, JsonRpcMiddleware } from '@metamask/json-rpc-engine';
import type {
Json,
JsonRpcFailure,
JsonRpcMiddleware,
JsonRpcParams,
JsonRpcRequest,
JsonRpcResponse,
JsonRpcSuccess,
} from 'json-rpc-engine';
} from '@metamask/utils';
import { type JsonRpcError, providerErrors, rpcErrors } from '@metamask/rpc-errors';
import type { TransactionParams } from '@metamask/transaction-controller';
import { providerErrors, rpcErrors } from '@metamask/rpc-errors';
import Engine from '../Engine';
import { store } from '../../store';
import { getPermittedAccounts } from '../Permissions';
Expand Down Expand Up @@ -106,8 +107,8 @@ const jsonrpc = '2.0' as const;
* @throws If the given value is not a valid {@link JsonRpcSuccess} object.
*/
function assertIsJsonRpcSuccess(
response: JsonRpcResponse<unknown>,
): asserts response is JsonRpcSuccess<unknown> {
response: JsonRpcResponse<Json>,
): asserts response is JsonRpcSuccess<Json> {
if ('error' in response) {
throw new Error(`Response failed with error '${JSON.stringify('error')}'`);
} else if (!('result' in response)) {
Expand Down Expand Up @@ -208,8 +209,8 @@ async function callMiddleware({
middleware,
request,
}: {
middleware: JsonRpcMiddleware<unknown, unknown>;
request: JsonRpcRequest<unknown>;
middleware: JsonRpcMiddleware<JsonRpcParams, Json>;
request: JsonRpcRequest<JsonRpcParams>;
}) {
const engine = new JsonRpcEngine();
engine.push(middleware);
Expand Down Expand Up @@ -415,7 +416,6 @@ describe('getRpcMethodMiddleware', () => {
permissionController.createPermissionMiddleware({
origin: hostMock,
});
// @ts-expect-error JsonRpcId (number | string | void) doesn't match PS middleware's id, which is (string | number | null)
engine.push(permissionMiddleware);
const middleware = getRpcMethodMiddleware(getMinimalOptions());
engine.push(middleware);
Expand Down Expand Up @@ -1096,7 +1096,8 @@ describe('getRpcMethodMiddleware', () => {
it('returns a JSON-RPC error if an error is thrown when adding this transaction', async () => {
// Omit `from` and `chainId` here to skip validation for simplicity
// Downcast needed here because `from` is required by this type
const mockTransactionParameters = {} as TransactionParams;
const mockTransactionParameters = {} as (TransactionParams &
JsonRpcParams)[];
// Transaction fails before returning a result
mockAddTransaction.mockImplementation(async () => {
throw new Error('Failed to add transaction');
Expand All @@ -1119,12 +1120,17 @@ describe('getRpcMethodMiddleware', () => {
expect((response as JsonRpcFailure).error.message).toBe(
expectedError.message,
);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
expect(((response as JsonRpcFailure).error as JsonRpcError<any>).data.cause.message).toBe(
expectedError.message,
);
});

it('returns a JSON-RPC error if an error is thrown after approval', async () => {
// Omit `from` and `chainId` here to skip validation for simplicity
// Downcast needed here because `from` is required by this type
const mockTransactionParameters = {} as TransactionParams;
const mockTransactionParameters = {} as (TransactionParams &
JsonRpcParams)[];
setupGlobalState({
addTransactionResult: Promise.reject(
new Error('Failed to process transaction'),
Expand Down Expand Up @@ -1233,7 +1239,7 @@ describe('getRpcMethodMiddleware', () => {
jsonrpc,
id: 1,
method: 'personal_ecRecover',
params: [undefined, helloWorldSignature],
params: [undefined, helloWorldSignature] as JsonRpcParams,
};
const expectedError = rpcErrors.internal('Missing data parameter');

Expand Down
2 changes: 1 addition & 1 deletion app/core/RPCMethods/RPCMethodMiddleware.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Alert } from 'react-native';
import { getVersion } from 'react-native-device-info';
import { createAsyncMiddleware } from 'json-rpc-engine';
import { createAsyncMiddleware } from '@metamask/json-rpc-engine';
import { providerErrors, rpcErrors } from '@metamask/rpc-errors';
import {
EndFlowOptions,
Expand Down
10 changes: 9 additions & 1 deletion app/core/RPCMethods/createLegacyMethodMiddleware/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
JsonRpcEngine,
JsonRpcEngineEndCallback,
JsonRpcEngineNextCallback,
} from 'json-rpc-engine';
} from '@metamask/json-rpc-engine';
import {
Json,
JsonRpcParams,
Expand Down Expand Up @@ -150,6 +150,10 @@ describe('createLegacyMethodMiddleware', () => {
});
assertIsJsonRpcFailure(response);

// Type assertion for the error not having cause object
const errorData = response.error.data as { cause?: Error };

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

Expand All @@ -166,6 +170,10 @@ describe('createLegacyMethodMiddleware', () => {
});
assertIsJsonRpcFailure(response);

// Type assertion for the error not having cause object
const errorData = response.error.data as { cause?: Error };

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { rpcErrors } from '@metamask/rpc-errors';
import type { JsonRpcMiddleware } from 'json-rpc-engine';
import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine';

import { UNSUPPORTED_RPC_METHODS } from '../utils';
import { Json, JsonRpcParams } from '@metamask/utils';

/**
* Creates a middleware that rejects explicitly unsupported RPC methods with the
* appropriate error.
*/
const createUnsupportedMethodMiddleware = (): JsonRpcMiddleware<
unknown,
void
JsonRpcParams,
Json
> =>
async function unsupportedMethodMiddleware(req, _res, next, end) {
if ((UNSUPPORTED_RPC_METHODS as Set<string>).has(req.method)) {
Expand Down
15 changes: 11 additions & 4 deletions app/core/RPCMethods/eth_sendTransaction.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
// eslint-disable-next-line import/no-nodejs-modules
import { inspect } from 'util';
import type { JsonRpcRequest, PendingJsonRpcResponse } from 'json-rpc-engine';
import type {
Json,
JsonRpcParams,
JsonRpcRequest,
PendingJsonRpcResponse,
} from '@metamask/utils';
import type {
TransactionParams,
TransactionController,
Expand Down Expand Up @@ -50,8 +55,8 @@ const NETWORK_CLIENT_ID_MOCK = 'testNetworkClientId';
* @returns The JSON-RPC request.
*/
function constructSendTransactionRequest(
params: unknown,
): JsonRpcRequest<unknown> & {
params: [TransactionParams & JsonRpcParams],
): JsonRpcRequest<[TransactionParams & JsonRpcParams]> & {
method: 'eth_sendTransaction';
networkClientId: string;
} {
Expand All @@ -69,7 +74,7 @@ function constructSendTransactionRequest(
*
* @returns A pending JSON-RPC response.
*/
function constructPendingJsonRpcResponse(): PendingJsonRpcResponse<unknown> {
function constructPendingJsonRpcResponse(): PendingJsonRpcResponse<Json> {
return {
jsonrpc: '2.0',
id: 1,
Expand Down Expand Up @@ -181,6 +186,7 @@ describe('eth_sendTransaction', () => {
async () =>
await eth_sendTransaction({
hostname: 'example.metamask.io',
//@ts-expect-error - invalid parameters forced
req: constructSendTransactionRequest(invalidParameter),
res: constructPendingJsonRpcResponse(),
sendTransaction: getMockAddTransaction({
Expand All @@ -203,6 +209,7 @@ describe('eth_sendTransaction', () => {
async () =>
await eth_sendTransaction({
hostname: 'example.metamask.io',
//@ts-expect-error - invalid parameters forced
req: constructSendTransactionRequest(invalidParameter),
res: constructPendingJsonRpcResponse(),
sendTransaction: getMockAddTransaction({
Expand Down
28 changes: 20 additions & 8 deletions app/core/RPCMethods/eth_sendTransaction.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import type { JsonRpcRequest, PendingJsonRpcResponse } from 'json-rpc-engine';
import type {
Json,
JsonRpcParams,
JsonRpcRequest,
PendingJsonRpcResponse,
} from '@metamask/utils';
import {
TransactionController,
TransactionParams,
WalletDevice,
} from '@metamask/transaction-controller';
import { rpcErrors } from '@metamask/rpc-errors';
Expand Down Expand Up @@ -46,6 +52,11 @@ const hasProperty = <
): objectToCheck is ObjectToCheck & Record<Property, unknown> =>
Object.hasOwnProperty.call(objectToCheck, name);

interface SendArgs {
from: string;
chainId?: number;
}

/**
* Handle a `eth_sendTransaction` request.
*
Expand All @@ -66,16 +77,13 @@ async function eth_sendTransaction({
validateAccountAndChainId,
}: {
hostname: string;
req: JsonRpcRequest<unknown> & {
req: JsonRpcRequest<[TransactionParams & JsonRpcParams]> & {
method: 'eth_sendTransaction';
networkClientId: string;
};
res: PendingJsonRpcResponse<unknown>;
res: PendingJsonRpcResponse<Json>;
sendTransaction: TransactionController['addTransaction'];
validateAccountAndChainId: (args: {
from: string;
chainId?: number;
}) => Promise<void>;
validateAccountAndChainId: (args: SendArgs) => Promise<void>;
}) {
if (
!Array.isArray(req.params) &&
Expand All @@ -91,9 +99,13 @@ async function eth_sendTransaction({
message: `Invalid parameters: expected the first parameter to be an object`,
});
}
// TODO: Normalize chainId to Hex string
const nChainId = typeof req.params[0].chainId === 'number'
? req.params[0].chainId
: parseInt(req.params[0].chainId || '0x0', 16);
await validateAccountAndChainId({
from: req.params[0].from,
chainId: req.params[0].chainId,
chainId: nChainId,
});

const { result, transactionMeta } = await sendTransaction(req.params[0], {
Expand Down
7 changes: 3 additions & 4 deletions app/core/RPCMethods/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { query } from '@metamask/controller-utils';
import Engine from '../Engine';
import { selectHooks } from '@metamask/snaps-rpc-methods';
import { OptionalDataWithOptionalCause, rpcErrors } from '@metamask/rpc-errors';
import { JsonRpcMiddleware } from 'json-rpc-engine';
import { JsonRpcMiddleware } from '@metamask/json-rpc-engine';
import { PermittedHandlerExport } from '@metamask/permission-controller';
import { Json, JsonRpcParams, hasProperty } from '@metamask/utils';
import EthQuery from '@metamask/eth-query';
Expand Down Expand Up @@ -76,7 +76,7 @@ export function makeMethodMiddlewareMaker<U>(
const makeMethodMiddleware = (hooks: Record<string, unknown>) => {
assertExpectedHook(hooks, expectedHookNames);

const methodMiddleware: JsonRpcMiddleware<JsonRpcParams, unknown> = async (
const methodMiddleware: JsonRpcMiddleware<JsonRpcParams, Json> = async (
req,
res,
next,
Expand All @@ -88,12 +88,11 @@ export function makeMethodMiddlewareMaker<U>(
try {
// Implementations may or may not be async, so we must await them.
return await implementation(
// @ts-expect-error JsonRpcId (number | string | void) doesn't match the permission middleware's id, which is (string | number | null)
req,
res,
next,
end,
selectHooks(hooks, hookNames),
selectHooks(hooks, hookNames) as U,
);
} catch (error) {
if (process.env.METAMASK_DEBUG) {
Expand Down
2 changes: 1 addition & 1 deletion app/core/RPCMethods/wallet_watchAsset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
} from '../../constants/error';
import { selectChainId, selectNetworkClientId } from '../../selectors/networkController';
import { isValidAddress } from 'ethereumjs-util';
import { JsonRpcRequest, PendingJsonRpcResponse } from 'json-rpc-engine';
import { toChecksumHexAddress } from '@metamask/controller-utils';
import { JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils';

const wallet_watchAsset = async ({
req,
Expand Down
Loading

0 comments on commit 2e566df

Please sign in to comment.