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: add source when local PPOM fails #12460

Merged
merged 5 commits into from
Nov 29, 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
57 changes: 54 additions & 3 deletions app/lib/ppom/ppom-util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ import {
RpcEndpointType,
} from '@metamask/network-controller';
import { NETWORKS_CHAIN_ID } from '../../constants/network';
import {
Reason,
ResultType,
SecurityAlertSource,
} from '../../components/Views/confirmations/components/BlockaidBanner/BlockaidBanner.types';
import Logger from '../../util/Logger';

const CHAIN_ID_MOCK = '0x1';

Expand Down Expand Up @@ -176,8 +182,8 @@ describe('PPOM Utils', () => {
MockEngine.context.PreferencesController.state.securityAlertsEnabled =
false;
await PPOMUtil.validateRequest(mockRequest, CHAIN_ID_MOCK);
expect(MockEngine.context.PPOMController?.usePPOM).toBeCalledTimes(0);
expect(spyTransactionAction).toBeCalledTimes(0);
expect(MockEngine.context.PPOMController?.usePPOM).toHaveBeenCalledTimes(0);
expect(spyTransactionAction).toHaveBeenCalledTimes(0);
});

it('should not validate if request is send to users own account ', async () => {
Expand Down Expand Up @@ -307,6 +313,22 @@ describe('PPOM Utils', () => {
});
});

it('logs error if normalization fails', async () => {
const error = new Error('Test Error');
normalizeTransactionParamsMock.mockImplementation(() => {
throw error;
});

const spyLogger = jest.spyOn(Logger, 'log');

await PPOMUtil.validateRequest(mockRequest, CHAIN_ID_MOCK);

expect(spyLogger).toHaveBeenCalledTimes(1);
expect(spyLogger).toHaveBeenCalledWith(
`Error validating JSON RPC using PPOM: ${error}`,
);
});

it('normalizes transaction request origin before validation', async () => {
const validateMock = jest.fn();

Expand Down Expand Up @@ -385,5 +407,34 @@ describe('PPOM Utils', () => {
await PPOMUtil.validateRequest(mockRequest, CHAIN_ID_MOCK);
expect(spy).toHaveBeenCalledTimes(2);
});

it('sets security alerts response to failed when security alerts API and controller PPOM throws', async () => {
const spy = jest.spyOn(
TransactionActions,
'setTransactionSecurityAlertResponse',
);

const validateMock = new Error('Test Error');

const ppomMock = {
validateJsonRpc: validateMock,
};

MockEngine.context.PPOMController?.usePPOM.mockImplementation(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(callback: any) => callback(ppomMock),
);

await PPOMUtil.validateRequest(mockRequest, CHAIN_ID_MOCK);
expect(spy).toHaveBeenCalledTimes(2);
expect(spy).toHaveBeenCalledWith(CHAIN_ID_MOCK, {
chainId: CHAIN_ID_MOCK,
req: mockRequest,
result_type: ResultType.Failed,
reason: Reason.failed,
description: 'Validating the confirmation failed by throwing error.',
source: SecurityAlertSource.Local,
});
});
});
});
});
19 changes: 12 additions & 7 deletions app/lib/ppom/ppom-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,19 @@ async function validateWithController(
ppomController: PPOMController,
request: PPOMRequest,
): Promise<SecurityAlertResponse> {
const response = (await ppomController.usePPOM((ppom) =>
ppom.validateJsonRpc(request as unknown as Record<string, unknown>),
)) as SecurityAlertResponse;
try{
const response = (await ppomController.usePPOM((ppom) =>
ppom.validateJsonRpc(request as unknown as Record<string, unknown>),
)) as SecurityAlertResponse;

return {
...response,
source: SecurityAlertSource.Local,
};
return {
...response,
source: SecurityAlertSource.Local,
};
} catch (e) {
Logger.log(`Error validating request with PPOM: ${e}`);
return {...SECURITY_ALERT_RESPONSE_FAILED, source: SecurityAlertSource.Local,};
}
}

async function validateWithAPI(
Expand Down
Loading