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: unwrap method call validator errors and add tests #4808

Open
wants to merge 4 commits into
base: caip-multichain-api
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import type {
MethodCallValidationError,
MethodCallValidatorNoSchemaError,
} from './multichainMethodCallValidator';
import { multichainMethodCallValidator } from './multichainMethodCallValidator';

describe('multichainMethodCallValidator', () => {
it('should validate method calls with no params', async () => {
const errors = await multichainMethodCallValidator('wallet_getSession', {});
expect(errors).toBe(false);
});

it('should validate method calls with invalid params when required and return errors', async () => {
const errors = await multichainMethodCallValidator('wallet_createSession', {
requiredScopes: true,
});
expect(errors).toHaveLength(1);
expect((errors as MethodCallValidationError[])[0].message).toBe(
'requiredScopes is not of a type(s) object',
);
});

it('should have no errors when params are valid', async () => {
const errors = await multichainMethodCallValidator('wallet_createSession', {
requiredScopes: {
'eip155:1337': {
methods: ['eth_sendTransaction'],
notifications: [],
},
},
optionalScopes: {
'eip155:1337': {
methods: ['eth_sendTransaction'],
notifications: [],
},
},
});
expect(errors).toBe(false);
});

describe('invalid number of params', () => {
it('should validate method calls with invalid number of params when required and return errors', async () => {
const errors = await multichainMethodCallValidator(
'wallet_createSession',
{
chainId: 'eip155:1',
accountAddress: '0x0',
foo: 'bar',
baz: 'potato',
},
);
expect(errors).toHaveLength(1);
expect((errors as MethodCallValidationError[])[0].message).toBe(
'Invalid number of parameters.',
);
expect((errors as MethodCallValidatorNoSchemaError[])[0].expected).toBe(
3,
);
expect((errors as MethodCallValidationError[])[0].got).toStrictEqual({
chainId: 'eip155:1',
accountAddress: '0x0',
foo: 'bar',
baz: 'potato',
});
});
});
});
101 changes: 81 additions & 20 deletions packages/multichain/src/middlewares/multichainMethodCallValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,7 @@ import { MultiChainOpenRPCDocument } from '@metamask/api-specs';
import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine';
import { rpcErrors } from '@metamask/rpc-errors';
import { isObject } from '@metamask/utils';
import type {
Json,
JsonRpcError,
JsonRpcParams,
JsonRpcRequest,
} from '@metamask/utils';
import type { Json, JsonRpcParams } from '@metamask/utils';
import type {
ContentDescriptorObject,
MethodObject,
Expand All @@ -18,45 +13,96 @@ import { makeCustomResolver } from '@open-rpc/schema-utils-js/build/parse-open-r
import type { Schema, ValidationError } from 'jsonschema';
import { Validator } from 'jsonschema';

export type MethodCallValidationSchemaError = {
message: string;
param: string;
path: (string | number)[];
schema: string | Schema;
got: unknown;
};

export type MethodCallValidatorNoSchemaError = {
message: string;
expected?: Json;
got: unknown;
};

export type MethodCallValidationError =
| MethodCallValidationSchemaError
| MethodCallValidatorNoSchemaError;

const transformError = (
error: ValidationError,
param: ContentDescriptorObject,
got: unknown,
) => {
): MethodCallValidationError => {
// if there is a path, add it to the message
const message = `${param.name}${
error.path.length > 0 ? `.${error.path.join('.')}` : ''
} ${error.message}`;

return {
code: -32602, // TODO: could be a different error code or not wrapped in json-rpc error, since this will also be wrapped in a -32602 invalid params error
message,
data: {
param: param.name,
path: error.path,
schema: error.schema,
got,
},
param: param.name,
path: error.path,
schema: error.schema,
got,
};
};

const checkForInvalidParams = (
params: JsonRpcParams | undefined,
paramsToCheck: ContentDescriptorObject[],
) => {
const errors: MethodCallValidationError[] = [];
const numRequiredParams = (paramsToCheck as ContentDescriptorObject[]).filter(
(p) => p.required,
).length;

let paramsLength = 0;
if (Array.isArray(params)) {
paramsLength = params.length;
} else if (isObject(params)) {
paramsLength = Object.keys(params).length;
}

if (numRequiredParams > paramsLength && numRequiredParams > 0) {
errors.push({
message: `Invalid number of parameters.`,
expected: numRequiredParams,
got: params,
});
} else if (paramsLength > paramsToCheck.length) {
errors.push({
message: `Invalid number of parameters.`,
expected: paramsToCheck.length,
shanejonas marked this conversation as resolved.
Show resolved Hide resolved
got: params,
});
}

return errors;
};

const v = new Validator();

const dereffedPromise = dereferenceDocument(
MultiChainOpenRPCDocument as unknown as OpenrpcDocument,
makeCustomResolver({}),
);
const multichainMethodCallValidator = async (
export const multichainMethodCallValidator = async (
method: string,
params: JsonRpcParams | undefined,
) => {
const dereffed = await dereffedPromise;
const methodToCheck = dereffed.methods.find(
(m) => (m as unknown as ContentDescriptorObject).name === method,
);
const errors: JsonRpcError[] = [];
const errors: MethodCallValidationError[] = [];

const paramsToCheck = (methodToCheck as unknown as MethodObject).params;

// check each param and aggregate errors
(methodToCheck as unknown as MethodObject).params.forEach((param, i) => {
paramsToCheck.forEach((param, i) => {
let paramToCheck: Json | undefined;
const p = param as ContentDescriptorObject;
if (isObject(params)) {
Expand All @@ -72,28 +118,43 @@ const multichainMethodCallValidator = async (
if (result.errors) {
errors.push(
...result.errors.map((e) => {
return transformError(e, p, paramToCheck) as JsonRpcError;
return transformError(e, p, paramToCheck);
}),
);
}
});

const invalidParamsErrors = checkForInvalidParams(
params,
paramsToCheck as ContentDescriptorObject[],
);

if (invalidParamsErrors.length > 0) {
errors.push(...invalidParamsErrors);
}

if (errors.length > 0) {
return errors;
}

// feels like this should return true to indicate that its valid but i'd rather check the falsy value since errors
// would be an array and return true if it's empty
return false;
};

export const multichainMethodCallValidatorMiddleware: JsonRpcMiddleware<
JsonRpcRequest,
JsonRpcParams,
Json
> = function (request, _response, next, end) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
multichainMethodCallValidator(request.method, request.params).then(
(errors) => {
if (errors) {
return end(rpcErrors.invalidParams<JsonRpcError[]>({ data: errors }));
return end(
rpcErrors.invalidParams({
data: errors as Json[],
}),
);
}
return next();
},
Expand Down
Loading