Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8bd9c80
chore: nodejs 20.18.0
legobeat Oct 22, 2024
a20e6b3
fix(deps): eth-json-rpc-filters@4.2.2 -> @metamask/eth-json-rpc-filte…
legobeat Apr 16, 2024
608b160
deps: json-rpc-engine@^6.1.0 -> @metamask/json-rpc-engine@^7.1.0
legobeat Apr 17, 2024
850c29a
Update JsonRpcRequest typings
legobeat Apr 17, 2024
14c6230
chainId is now hex string, not number
legobeat Apr 18, 2024
7e76a90
chore(test): eth_sendTransaction test type workarounds
legobeat May 20, 2024
a3ab34e
fixup: console.error in test
legobeat May 20, 2024
96ff71c
fix(test): test for error cause message in RPCMethodMIddleware
legobeat May 20, 2024
b839c67
fix(deps): `metamask-rpc-middleware` -> `@metamask/eth-json-rpc-middl…
MarioAslau Jun 10, 2024
658d0f0
chore: update .iyarc
legobeat Jun 11, 2024
6c40511
background bridge cleanup fix
tommasini Jun 24, 2024
e76f790
fix depcheck issues
tommasini Jun 24, 2024
080117d
fix lint issues
tommasini Jun 24, 2024
81c5f5c
fix lint and fix ts error
tommasini Jun 24, 2024
cce823e
fix on snap bridge
tommasini Jun 25, 2024
c7e8773
feat: update createLegacyMethodMiddleware tests
MarioAslau Jun 25, 2024
16a7052
feat: remove type
MarioAslau Jun 25, 2024
326050d
feat: removed unused imports
MarioAslau Jun 25, 2024
10584a6
fix: revert `Internal JSON-RPC error` message change
legobeat Oct 22, 2024
afa4d2d
deps: @metamask/rpc-errors@^6.2.1->^7.0.0
legobeat Oct 22, 2024
560a93b
chore: fixup test for @metamask/rpc-errors 7.0.0+
legobeat Oct 22, 2024
435d214
chore: migrate patch for @metamask/post-message-stream
legobeat Oct 22, 2024
8dbb040
chore: migrate patch for @metamask/rpc-errors
legobeat Oct 22, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .iyarc
Original file line number Diff line number Diff line change
@@ -1,3 +0,0 @@
# ReDoS vulnerability, no impact to this application, and fix not backported yet to the versions we use
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍😍😍😍😍😍


GHSA-c2qf-rxjj-qqgw
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20.12.2
20.18.0
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ jest.mock('../../hooks/useStakeContext.ts', () => ({
const stakeContext: Stake = {
setSdkType: jest.fn(),
sdkService: undefined
}
return stakeContext
};
return stakeContext;
})
}))
}));

jest.mock('../../hooks/useBalance', () => ({
__esModule: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const StakeInputView = () => {


const { sdkService } = useStakeContext();

const navigateToLearnMoreModal = () => {
navigation.navigate('StakeModals', {
screen: Routes.STAKING.MODALS.LEARN_MORE,
Expand Down
2 changes: 1 addition & 1 deletion app/components/UI/Stake/sdk/stakeSdkProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,4 @@ export const StakeSDKProvider: React.FC<PropsWithChildren<StakeProviderProps>> =
{children}
</StakeContext.Provider>
);
};
};
14 changes: 5 additions & 9 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 @@ -26,9 +26,9 @@ import snapMethodMiddlewareBuilder from '../Snaps/SnapsMethodMiddleware';
import { SubjectType } from '@metamask/permission-controller';
///: END:ONLY_INCLUDE_IF

const createFilterMiddleware = require('eth-json-rpc-filters');
const createSubscriptionManager = require('eth-json-rpc-filters/subscriptionManager');
const providerAsMiddleware = require('eth-json-rpc-middleware/providerAsMiddleware');
const createFilterMiddleware = require('@metamask/eth-json-rpc-filters');
const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager');
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
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 @@ -416,7 +417,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 @@ -1097,7 +1097,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 @@ -1120,12 +1121,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 @@ -1234,7 +1240,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
9 changes: 5 additions & 4 deletions 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 All @@ -16,6 +16,7 @@ import {
permissionRpcMethods,
} from '@metamask/permission-controller';
import { blockTagParamIndex, getAllNetworks } from '../../util/networks';
import type { Hex } from '@metamask/utils';
import { polyfillGasPrice } from './utils';
import {
processOriginThrottlingRejection,
Expand Down Expand Up @@ -112,7 +113,7 @@ export const checkActiveAccountAndChainId = async ({
isWalletConnect,
}: {
address?: string;
chainId?: number;
chainId?: Hex;
channelId?: string;
hostname: string;
isWalletConnect: boolean;
Expand Down Expand Up @@ -224,7 +225,7 @@ const generateRawSignature = async ({
title: { current: string };
icon: { current: string | undefined };
analytics: { [key: string]: string | boolean };
chainId: number;
chainId: Hex;
isMMSDK: boolean;
channelId?: string;
getSource: () => string;
Expand Down Expand Up @@ -521,7 +522,7 @@ export const getRpcMethodMiddleware = ({
chainId,
}: {
from?: string;
chainId?: number;
chainId?: Hex;
}) => {
// TODO this needs to be modified for per dapp selected network
await checkActiveAccountAndChainId({
Expand Down
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
21 changes: 16 additions & 5 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 @@ -48,8 +53,10 @@ jest.mock('../../util/transaction-controller', () => ({
* @returns The JSON-RPC request.
*/
function constructSendTransactionRequest(
params: unknown,
): JsonRpcRequest<unknown> & { method: 'eth_sendTransaction' } {
params: [TransactionParams & JsonRpcParams],
): JsonRpcRequest<[TransactionParams & JsonRpcParams]> & {
method: 'eth_sendTransaction';
} {
return {
jsonrpc: '2.0',
id: 1,
Expand All @@ -63,7 +70,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 @@ -144,7 +151,9 @@ function getMockAddTransaction({
describe('eth_sendTransaction', () => {
it('sends the transaction and returns the resulting hash', async () => {
const mockAddress = '0x0000000000000000000000000000000000000001';
const mockTransactionParameters = { from: mockAddress };
const mockTransactionParameters = {
from: mockAddress,
};
const expectedResult = 'fake-hash';
const pendingResult = constructPendingJsonRpcResponse();

Expand Down Expand Up @@ -172,6 +181,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 @@ -194,6 +204,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
25 changes: 18 additions & 7 deletions app/core/RPCMethods/eth_sendTransaction.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import type { JsonRpcRequest, PendingJsonRpcResponse } from 'json-rpc-engine';
import type {
Hex,
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 +53,11 @@ const hasProperty = <
): objectToCheck is ObjectToCheck & Record<Property, unknown> =>
Object.hasOwnProperty.call(objectToCheck, name);

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

/**
* Handle a `eth_sendTransaction` request.
*
Expand All @@ -66,13 +78,12 @@ async function eth_sendTransaction({
validateAccountAndChainId,
}: {
hostname: string;
req: JsonRpcRequest<unknown> & { method: 'eth_sendTransaction' };
res: PendingJsonRpcResponse<unknown>;
req: JsonRpcRequest<[TransactionParams & JsonRpcParams]> & {
method: 'eth_sendTransaction';
};
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 Down
8 changes: 4 additions & 4 deletions app/core/RPCMethods/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ 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 +77,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 +89,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
Loading
Loading