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

Add isDecodeSignatureRequestEnabled to SignatureController constructor #4903

Merged
merged 6 commits into from
Nov 8, 2024
Merged
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
78 changes: 75 additions & 3 deletions packages/signature-controller/src/SignatureController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,10 @@ describe('SignatureController', () => {
},
],
};
const { controller } = createController();
const { controller } = createController({
decodingApiUrl: 'www.test.com',
isDecodeSignatureRequestEnabled: () => true,
});

jest
.spyOn(DecodingDataUtils, 'decodeSignature')
Expand All @@ -948,8 +951,74 @@ describe('SignatureController', () => {
).toStrictEqual(MOCK_STATE_CHANGES);
});

it('does not invoke decodeSignature if decodingApiUrl is not defined', async () => {
const { controller } = createController({
decodingApiUrl: undefined,
isDecodeSignatureRequestEnabled: () => true,
});

await controller.newUnsignedTypedMessage(
PERMIT_PARAMS_MOCK,
PERMIT_REQUEST_MOCK,
SignTypedDataVersion.V4,
{ parseJsonData: false },
);

expect(
controller.state.signatureRequests[ID_MOCK].decodingLoading,
).toBeUndefined();
expect(
controller.state.signatureRequests[ID_MOCK].decodingData,
).toBeUndefined();
});

it('does not invoke decodeSignature if featureFLag disableDecodingApi is true', async () => {
const { controller } = createController({
decodingApiUrl: 'www.test.com',
isDecodeSignatureRequestEnabled: () => false,
});

await controller.newUnsignedTypedMessage(
PERMIT_PARAMS_MOCK,
PERMIT_REQUEST_MOCK,
SignTypedDataVersion.V4,
{ parseJsonData: false },
);

expect(
controller.state.signatureRequests[ID_MOCK].decodingLoading,
).toBeUndefined();
expect(
controller.state.signatureRequests[ID_MOCK].decodingData,
).toBeUndefined();
});

it('does not invoke decodeSignature if isDecodeSignatureRequestEnabled is not defined', async () => {
const { controller } = createController({
decodingApiUrl: 'www.test.com',
isDecodeSignatureRequestEnabled: undefined,
});

await controller.newUnsignedTypedMessage(
PERMIT_PARAMS_MOCK,
PERMIT_REQUEST_MOCK,
SignTypedDataVersion.V4,
{ parseJsonData: false },
);

expect(
controller.state.signatureRequests[ID_MOCK].decodingLoading,
).toBeUndefined();
expect(
controller.state.signatureRequests[ID_MOCK].decodingData,
).toBeUndefined();
});

it('correctly set decoding data if decodeSignature fails', async () => {
const { controller } = createController();
const { controller } = createController({
decodingApiUrl: 'www.test.com',
isDecodeSignatureRequestEnabled: () => true,
});

jest
.spyOn(DecodingDataUtils, 'decodeSignature')
Expand All @@ -973,7 +1042,10 @@ describe('SignatureController', () => {
});

it('set decodingLoading to true while api request is in progress', async () => {
const { controller } = createController();
const { controller } = createController({
decodingApiUrl: 'www.test.com',
isDecodeSignatureRequestEnabled: () => true,
});

jest
.spyOn(DecodingDataUtils, 'decodeSignature')
Expand Down
15 changes: 14 additions & 1 deletion packages/signature-controller/src/SignatureController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ export type SignatureControllerOptions = {
*/
decodingApiUrl?: string;

/**
* Function to check if decoding signature request is enabled
*/
isDecodeSignatureRequestEnabled?: () => boolean;

/**
* Initial state of the controller.
*/
Expand All @@ -184,19 +189,23 @@ export class SignatureController extends BaseController<

#decodingApiUrl?: string;

#isDecodeSignatureRequestEnabled?: () => boolean;

#trace: TraceCallback;

/**
* Construct a Sign controller.
*
* @param options - The controller options.
* @param options.decodingApiUrl - Api used to get decoded data for permits.
* @param options.isDecodeSignatureRequestEnabled - Function to check is decoding signature request is enabled.
* @param options.messenger - The restricted controller messenger for the sign controller.
* @param options.state - Initial state to set on this controller.
* @param options.trace - Callback to generate trace information.
* @param options.decodingApiUrl - Api used to get decoded data for permits.
*/
constructor({
decodingApiUrl,
isDecodeSignatureRequestEnabled,
messenger,
state,
trace,
Expand All @@ -214,6 +223,7 @@ export class SignatureController extends BaseController<
this.hub = new EventEmitter();
this.#trace = trace ?? (((_request, fn) => fn?.()) as TraceCallback);
this.#decodingApiUrl = decodingApiUrl;
this.#isDecodeSignatureRequestEnabled = isDecodeSignatureRequestEnabled;
}

/**
Expand Down Expand Up @@ -902,6 +912,9 @@ export class SignatureController extends BaseController<
request: OriginalRequest,
chainId: string,
) {
if (!this.#isDecodeSignatureRequestEnabled?.() || !this.#decodingApiUrl) {
return;
}
this.#updateMetadata(signatureRequestId, (draftMetadata) => {
draftMetadata.decodingLoading = true;
});
Expand Down
10 changes: 0 additions & 10 deletions packages/signature-controller/src/constants.ts

This file was deleted.

32 changes: 22 additions & 10 deletions packages/signature-controller/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,27 @@ import type { SIWEMessage } from '@metamask/controller-utils';
import type { SignTypedDataVersion } from '@metamask/keyring-controller';
import type { Hex, Json } from '@metamask/utils';

/**
* Supported signature methods.
*/
export enum EthMethod {
PersonalSign = 'personal_sign',
SignTransaction = 'eth_signTransaction',
SignTypedDataV1 = 'eth_signTypedData_v1',
SignTypedDataV3 = 'eth_signTypedData_v3',
SignTypedDataV4 = 'eth_signTypedData_v4',
}

/** Different decoding data state change types */
export enum DecodingDataChangeType {
Receive = 'RECEIVE',
Transfer = 'TRANSFER',
Approve = 'APPROVE',
Revoke = 'REVOKE_APPROVE',
Bidding = 'BIDDING',
Listing = 'LISTING',
}

/** Original client request that triggered the signature request. */
export type OriginalRequest = {
/** Unique ID to identify the client request. */
Expand Down Expand Up @@ -81,19 +102,10 @@ export type MessageParamsTyped = MessageParams & {
version?: string;
};

/** Different decoding data state change types */
export type DecodingDataChangeType =
| 'RECEIVE'
| 'TRANSFER'
| 'APPROVE'
| 'REVOKE_APPROVE'
| 'BIDDING'
| 'LISTING';

/** Information about a single state change returned by decoding api. */
export type DecodingDataStateChange = {
assetType: string;
changeType: DecodingDataChangeType;
changeType: (typeof DecodingDataChangeType)[keyof typeof DecodingDataChangeType];
address: string;
amount: string;
contractAddress: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { EthMethod } from '../constants';
import { type OriginalRequest } from '../types';
import { EthMethod, type OriginalRequest } from '../types';
import { decodeSignature } from './decoding-api';

const PERMIT_REQUEST_MOCK = {
Expand Down
8 changes: 2 additions & 6 deletions packages/signature-controller/src/utils/decoding-api.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { EthMethod } from '../constants';
import { type OriginalRequest } from '../types';
import { EthMethod, type OriginalRequest } from '../types';
import { convertNumericValuesToQuotedString } from './normalize';

export const DECODING_API_ERRORS = {
Expand All @@ -18,11 +17,8 @@ export const DECODING_API_ERRORS = {
export async function decodeSignature(
request: OriginalRequest,
chainId: string,
decodingApiUrl?: string,
decodingApiUrl: string,
) {
if (!decodingApiUrl) {
return undefined;
}
try {
const { method, origin, params } = request;
if (request.method === EthMethod.SignTypedDataV4) {
Expand Down
Loading