Skip to content

Commit

Permalink
Refactor RPC method params and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
david0xd committed Nov 7, 2022
1 parent 554f3b4 commit 060f80c
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 19 deletions.
8 changes: 4 additions & 4 deletions packages/rpc-methods/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ module.exports = {
coverageReporters: ['clover', 'json', 'lcov', 'text', 'json-summary'],
coverageThreshold: {
global: {
branches: 43.52,
functions: 59.74,
lines: 48.27,
statements: 48.04,
branches: 51.78,
functions: 64.93,
lines: 54.61,
statements: 54.28,
},
},
moduleFileExtensions: ['js', 'json', 'jsx', 'ts', 'tsx', 'node'],
Expand Down
6 changes: 3 additions & 3 deletions packages/rpc-methods/src/restricted/invokeSnap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('implementation', () => {
await implementation({
context: { origin: MOCK_ORIGIN },
method: `wallet_snap_${MOCK_SNAP_ID}`,
params: [{ method: 'foo', params: {} }],
params: { method: 'foo', params: {} },
});

expect(hooks.getSnap).toHaveBeenCalledTimes(1);
Expand All @@ -73,7 +73,7 @@ describe('implementation', () => {
implementation({
context: { origin: MOCK_ORIGIN },
method: `wallet_snap_${MOCK_SNAP_ID}`,
params: [{ method: 'foo', params: {} }],
params: { method: 'foo', params: {} },
}),
).rejects.toThrow(
`The snap "${MOCK_SNAP_ID}" is not installed. This is a bug, please report it.`,
Expand All @@ -92,7 +92,7 @@ describe('implementation', () => {
implementation({
context: { origin: MOCK_ORIGIN },
method: `wallet_snap_${MOCK_SNAP_ID}`,
params: [{}],
params: {},
}),
).rejects.toThrow(
'Must specify a valid JSON-RPC request object as single parameter.',
Expand Down
7 changes: 3 additions & 4 deletions packages/rpc-methods/src/restricted/invokeSnap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,11 @@ export function getInvokeSnapImplementation({
handleSnapRpcRequest,
}: InvokeSnapMethodHooks) {
return async function invokeSnap(
options: RestrictedMethodOptions<[Record<string, Json>]>,
options: RestrictedMethodOptions<Record<string, Json>>,
): Promise<Json> {
const { params = [], method, context } = options;
const rawRequest = params[0];
const { params = {}, method, context } = options;

const request = { jsonrpc: '2.0', id: nanoid(), ...rawRequest };
const request = { jsonrpc: '2.0', id: nanoid(), ...params };

if (!isJsonRpcRequest(request)) {
throw ethErrors.rpc.invalidParams({
Expand Down
133 changes: 133 additions & 0 deletions packages/rpc-methods/src/restricted/notify.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import {
getImplementation,
getValidatedParams,
NotificationType,
} from './notify';

describe('snap_notify', () => {
const validParams = {
type: 'inApp',
message: 'Some message',
};

describe('getImplementation', () => {
it('shows inApp notification', async () => {
const showNativeNotification = jest.fn().mockResolvedValueOnce(true);
const showInAppNotification = jest.fn().mockResolvedValueOnce(true);

const notificationImplementation = getImplementation({
showNativeNotification,
showInAppNotification,
});

await notificationImplementation({
context: {
origin: 'extension',
},
method: '',
params: {
type: NotificationType.inApp,
message: 'Some message',
},
});

expect(showInAppNotification).toHaveBeenCalledWith('extension', {
type: NotificationType.inApp,
message: 'Some message',
});
});

it('shows native notification', async () => {
const showNativeNotification = jest.fn().mockResolvedValueOnce(true);
const showInAppNotification = jest.fn().mockResolvedValueOnce(true);

const notificationImplementation = getImplementation({
showNativeNotification,
showInAppNotification,
});

await notificationImplementation({
context: {
origin: 'extension',
},
method: '',
params: {
type: NotificationType.native,
message: 'Some message',
},
});

expect(showNativeNotification).toHaveBeenCalledWith('extension', {
type: NotificationType.native,
message: 'Some message',
});
});

it('throws an error if the notification type is invalid', async () => {
const showNativeNotification = jest.fn().mockResolvedValueOnce(true);
const showInAppNotification = jest.fn().mockResolvedValueOnce(true);

const notificationImplementation = getImplementation({
showNativeNotification,
showInAppNotification,
});

await expect(
notificationImplementation({
context: {
origin: 'extension',
},
method: '',
params: {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
type: 'invalid-type',
message: 'Some message',
},
}),
).rejects.toThrow('Must specify a valid notification "type".');
});
});

describe('getValidatedParams', () => {
it('throws an error if the params is not an object', () => {
expect(() => getValidatedParams([])).toThrow(
'Expected params to be a single object.',
);
});

it('throws an error if the type is missing from params object', () => {
expect(() =>
getValidatedParams({ type: undefined, message: 'Something happened.' }),
).toThrow('Must specify a valid notification "type".');
});

it('throws an error if the message is empty', () => {
expect(() => getValidatedParams({ type: 'inApp', message: '' })).toThrow(
'Must specify a non-empty string "message" less than 50 characters long.',
);
});

it('throws an error if the message is not a string', () => {
expect(() => getValidatedParams({ type: 'inApp', message: 123 })).toThrow(
'Must specify a non-empty string "message" less than 50 characters long.',
);
});

it('throws an error if the message is larger than 50 characters', () => {
expect(() =>
getValidatedParams({
type: 'inApp',
message:
'test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg_test_msg',
}),
).toThrow(
'Must specify a non-empty string "message" less than 50 characters long.',
);
});

it('returns valid parameters', () => {
expect(getValidatedParams(validParams)).toStrictEqual(validParams);
});
});
});
16 changes: 8 additions & 8 deletions packages/rpc-methods/src/restricted/notify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ type Specification = ValidPermissionSpecification<{
* @param options.methodHooks - The RPC method hooks needed by the method implementation.
* @returns The specification for the `snap_notify` permission.
*/
const specificationBuilder: PermissionSpecificationBuilder<
export const specificationBuilder: PermissionSpecificationBuilder<
PermissionType.RestrictedMethod,
SpecificationBuilderOptions,
Specification
Expand Down Expand Up @@ -99,12 +99,12 @@ export const notifyBuilder = Object.freeze({
* @returns The method implementation which returns `null` on success.
* @throws If the params are invalid.
*/
function getImplementation({
export function getImplementation({
showNativeNotification,
showInAppNotification,
}: NotifyMethodHooks) {
return async function implementation(
args: RestrictedMethodOptions<[NotificationArgs]>,
args: RestrictedMethodOptions<NotificationArgs>,
): Promise<null> {
const {
params,
Expand Down Expand Up @@ -133,14 +133,14 @@ function getImplementation({
* @param params - The unvalidated params object from the method request.
* @returns The validated method parameter object.
*/
function getValidatedParams(params: unknown): NotificationArgs {
if (!Array.isArray(params) || !isObject(params[0])) {
export function getValidatedParams(params: unknown): NotificationArgs {
if (!isObject(params)) {
throw ethErrors.rpc.invalidParams({
message: 'Expected array params with single object.',
message: 'Expected params to be a single object.',
});
}

const { type, message } = params[0];
const { type, message } = params;

if (
!type ||
Expand All @@ -160,5 +160,5 @@ function getValidatedParams(params: unknown): NotificationArgs {
});
}

return params[0] as NotificationArgs;
return params as NotificationArgs;
}

0 comments on commit 060f80c

Please sign in to comment.