diff --git a/packages/shield-controller/CHANGELOG.md b/packages/shield-controller/CHANGELOG.md index 98e99b2e963..bc06fe8eb61 100644 --- a/packages/shield-controller/CHANGELOG.md +++ b/packages/shield-controller/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add two new controller state metadata properties: `includeInStateLogs` and `usedInUi` ([#6504](https://github.com/MetaMask/core/pull/6504)) - Add signature coverage checking ([#6501](https://github.com/MetaMask/core/pull/6501)) +- Add transaction and signature logging ([#6633](https://github.com/MetaMask/core/pull/6633)) ### Changed diff --git a/packages/shield-controller/src/ShieldController.test.ts b/packages/shield-controller/src/ShieldController.test.ts index 00fb0e53477..94e77ce8a7c 100644 --- a/packages/shield-controller/src/ShieldController.test.ts +++ b/packages/shield-controller/src/ShieldController.test.ts @@ -1,9 +1,17 @@ import { deriveStateFromMetadata } from '@metamask/base-controller'; -import type { SignatureControllerState } from '@metamask/signature-controller'; -import type { TransactionControllerState } from '@metamask/transaction-controller'; +import type { SignatureRequest } from '@metamask/signature-controller'; +import { + SignatureRequestStatus, + type SignatureControllerState, +} from '@metamask/signature-controller'; +import type { TransactionMeta } from '@metamask/transaction-controller'; +import { + TransactionStatus, + type TransactionControllerState, +} from '@metamask/transaction-controller'; import { ShieldController } from './ShieldController'; -import { createMockBackend } from '../tests/mocks/backend'; +import { createMockBackend, MOCK_COVERAGE_ID } from '../tests/mocks/backend'; import { createMockMessenger } from '../tests/mocks/messenger'; import { generateMockSignatureRequest, @@ -218,6 +226,174 @@ describe('ShieldController', () => { ); }); + describe('logSignature', () => { + /** + * Run a test that logs a signature. + * + * @param components - An object containing the messenger and base messenger. + * @param options - An object containing optional parameters. + * @param options.updateSignatureRequest - A function that updates the signature request. + */ + async function runTest( + components: ReturnType, + options?: { + updateSignatureRequest?: (signatureRequest: SignatureRequest) => void; + }, + ) { + const { messenger, baseMessenger } = components; + + // Create a promise that resolves when the state changes + const stateUpdated = new Promise((resolve) => + messenger.subscribe('ShieldController:stateChange', resolve), + ); + + // Publish a signature request + const signatureRequest = generateMockSignatureRequest(); + baseMessenger.publish( + 'SignatureController:stateChange', + { + signatureRequests: { [signatureRequest.id]: signatureRequest }, + } as SignatureControllerState, + undefined as never, + ); + + // Wait for state to be updated + await stateUpdated; + + // Update signature request + const updatedSignatureRequest = { ...signatureRequest }; + updatedSignatureRequest.status = SignatureRequestStatus.Signed; + updatedSignatureRequest.rawSig = '0x00'; + options?.updateSignatureRequest?.(updatedSignatureRequest); + baseMessenger.publish( + 'SignatureController:stateChange', + { + signatureRequests: { [signatureRequest.id]: updatedSignatureRequest }, + } as SignatureControllerState, + undefined as never, + ); + } + + it('logs a signature', async () => { + const components = setup(); + + await runTest(components); + + // Check that backend was called + expect(components.backend.logSignature).toHaveBeenCalledWith({ + coverageId: MOCK_COVERAGE_ID, + signature: '0x00', + status: 'shown', + }); + }); + + it('does not log when coverageId is missing', async () => { + const components = setup(); + + components.backend.checkSignatureCoverage.mockResolvedValue({ + coverageId: undefined, + status: 'unknown', + }); + + await runTest(components); + + // Check that backend was not called + expect(components.backend.logSignature).not.toHaveBeenCalled(); + }); + + it('does not log when signature is missing', async () => { + const components = setup(); + + await runTest(components, { + updateSignatureRequest: (signatureRequest) => { + signatureRequest.rawSig = undefined; + }, + }); + + // Check that backend was not called + expect(components.backend.logSignature).not.toHaveBeenCalled(); + }); + }); + + describe('logTransaction', () => { + /** + * Runs a test that logs a transaction. + * + * @param components - An object containing the messenger and base messenger. + * @param options - Options for the test. + * @param options.updateTransaction - A function that updates the transaction. + */ + async function runTest( + components: ReturnType, + options?: { updateTransaction: (txMeta: TransactionMeta) => void }, + ) { + const { messenger, baseMessenger } = components; + // Create a promise that resolves when the state changes + const stateUpdated = new Promise((resolve) => + messenger.subscribe('ShieldController:stateChange', resolve), + ); + + // Publish a transaction + const txMeta = generateMockTxMeta(); + baseMessenger.publish( + 'TransactionController:stateChange', + { transactions: [txMeta] } as TransactionControllerState, + undefined as never, + ); + + // Wait for state to be updated + await stateUpdated; + + // Update transaction + const updatedTxMeta = { ...txMeta }; + updatedTxMeta.status = TransactionStatus.submitted; + updatedTxMeta.hash = '0x00'; + options?.updateTransaction(updatedTxMeta); + baseMessenger.publish( + 'TransactionController:stateChange', + { transactions: [updatedTxMeta] } as TransactionControllerState, + undefined as never, + ); + } + + it('logs a transaction', async () => { + const components = setup(); + await runTest(components); + + // Check that backend was called + expect(components.backend.logTransaction).toHaveBeenCalledWith({ + coverageId: MOCK_COVERAGE_ID, + status: 'shown', + transactionHash: '0x00', + }); + }); + + it('does not log when coverageId is missing', async () => { + const components = setup(); + + components.backend.checkCoverage.mockResolvedValue({ + coverageId: undefined, + status: 'unknown', + }); + + await runTest(components); + + // Check that backend was not called + expect(components.backend.logTransaction).not.toHaveBeenCalled(); + }); + + it('does not log when hash is missing', async () => { + const components = setup(); + + await runTest(components, { + updateTransaction: (txMeta) => delete txMeta.hash, + }); + + // Check that backend was not called + expect(components.backend.logTransaction).not.toHaveBeenCalled(); + }); + }); + describe('metadata', () => { it('includes expected state in debug snapshots', () => { const { controller } = setup(); diff --git a/packages/shield-controller/src/ShieldController.ts b/packages/shield-controller/src/ShieldController.ts index 4e8a31fc4d4..1578b258280 100644 --- a/packages/shield-controller/src/ShieldController.ts +++ b/packages/shield-controller/src/ShieldController.ts @@ -4,13 +4,15 @@ import type { RestrictedMessenger, } from '@metamask/base-controller'; import { + SignatureRequestStatus, SignatureRequestType, type SignatureRequest, type SignatureStateChange, } from '@metamask/signature-controller'; -import type { - TransactionControllerStateChangeEvent, - TransactionMeta, +import { + TransactionStatus, + type TransactionControllerStateChangeEvent, + type TransactionMeta, } from '@metamask/transaction-controller'; import { controllerName } from './constants'; @@ -230,6 +232,17 @@ export class ShieldController extends BaseController< (error) => log('Error checking coverage:', error), ); } + + // Log signature once the signature request has been fulfilled. + if ( + signatureRequest.status === SignatureRequestStatus.Signed && + signatureRequest.status !== previousSignatureRequest?.status + ) { + this.#logSignature(signatureRequest).catch( + // istanbul ignore next + (error) => log('Error logging signature:', error), + ); + } } } @@ -256,6 +269,17 @@ export class ShieldController extends BaseController< (error) => log('Error checking coverage:', error), ); } + + // Log transaction once it has been submitted. + if ( + transaction.status === TransactionStatus.submitted && + transaction.status !== previousTransaction?.status + ) { + this.#logTransaction(transaction).catch( + // istanbul ignore next + (error) => log('Error logging transaction:', error), + ); + } } } @@ -346,4 +370,49 @@ export class ShieldController extends BaseController< } }); } + + async #logSignature(signatureRequest: SignatureRequest) { + const coverageId = this.#getLatestCoverageId(signatureRequest.id); + if (!coverageId) { + throw new Error('Coverage ID not found'); + } + + const sig = signatureRequest.rawSig; + if (!sig) { + throw new Error('Signature not found'); + } + + await this.#backend.logSignature({ + coverageId, + signature: sig, + // Status is 'shown' because the coverageId can only be retrieved after + // the result is in the state. If the result is in the state, we assume + // that it has been shown. + status: 'shown', + }); + } + + async #logTransaction(txMeta: TransactionMeta) { + const coverageId = this.#getLatestCoverageId(txMeta.id); + if (!coverageId) { + throw new Error('Coverage ID not found'); + } + + const txHash = txMeta.hash; + if (!txHash) { + throw new Error('Transaction hash not found'); + } + await this.#backend.logTransaction({ + coverageId, + transactionHash: txHash, + // Status is 'shown' because the coverageId can only be retrieved after + // the result is in the state. If the result is in the state, we assume + // that it has been shown. + status: 'shown', + }); + } + + #getLatestCoverageId(itemId: string) { + return this.state.coverageResults[itemId]?.results[0]?.coverageId; + } } diff --git a/packages/shield-controller/src/backend.test.ts b/packages/shield-controller/src/backend.test.ts index 6ef470bbe74..c5647e48f92 100644 --- a/packages/shield-controller/src/backend.test.ts +++ b/packages/shield-controller/src/backend.test.ts @@ -49,6 +49,7 @@ describe('ShieldRemoteBackend', () => { const { backend, fetchMock, getAccessToken } = setup(); // Mock init coverage check. + const coverageId = 'coverageId'; fetchMock.mockResolvedValueOnce({ status: 200, json: jest.fn().mockResolvedValue({ coverageId: 'coverageId' }), @@ -63,7 +64,7 @@ describe('ShieldRemoteBackend', () => { const txMeta = generateMockTxMeta(); const coverageResult = await backend.checkCoverage(txMeta); - expect(coverageResult).toStrictEqual({ status }); + expect(coverageResult).toStrictEqual({ coverageId, status }); expect(fetchMock).toHaveBeenCalledTimes(2); expect(getAccessToken).toHaveBeenCalledTimes(2); }); @@ -74,9 +75,10 @@ describe('ShieldRemoteBackend', () => { }); // Mock init coverage check. + const coverageId = 'coverageId'; fetchMock.mockResolvedValueOnce({ status: 200, - json: jest.fn().mockResolvedValue({ coverageId: 'coverageId' }), + json: jest.fn().mockResolvedValue({ coverageId }), } as unknown as Response); // Mock get coverage result: result unavailable. @@ -94,7 +96,7 @@ describe('ShieldRemoteBackend', () => { const txMeta = generateMockTxMeta(); const coverageResult = await backend.checkCoverage(txMeta); - expect(coverageResult).toStrictEqual({ status }); + expect(coverageResult).toStrictEqual({ coverageId, status }); expect(fetchMock).toHaveBeenCalledTimes(3); expect(getAccessToken).toHaveBeenCalledTimes(2); }); @@ -151,9 +153,10 @@ describe('ShieldRemoteBackend', () => { const { backend, fetchMock, getAccessToken } = setup(); // Mock init coverage check. + const coverageId = 'coverageId'; fetchMock.mockResolvedValueOnce({ status: 200, - json: jest.fn().mockResolvedValue({ coverageId: 'coverageId' }), + json: jest.fn().mockResolvedValue({ coverageId }), } as unknown as Response); // Mock get coverage result. @@ -166,7 +169,7 @@ describe('ShieldRemoteBackend', () => { const signatureRequest = generateMockSignatureRequest(); const coverageResult = await backend.checkSignatureCoverage(signatureRequest); - expect(coverageResult).toStrictEqual({ status }); + expect(coverageResult).toStrictEqual({ coverageId, status }); expect(fetchMock).toHaveBeenCalledTimes(2); expect(getAccessToken).toHaveBeenCalledTimes(2); }); @@ -181,4 +184,64 @@ describe('ShieldRemoteBackend', () => { ).rejects.toThrow('Signature data must be a string'); }); }); + + describe('logSignature', () => { + it('logs signature', async () => { + const { backend, fetchMock, getAccessToken } = setup(); + + fetchMock.mockResolvedValueOnce({ status: 200 } as unknown as Response); + + await backend.logSignature({ + coverageId: 'coverageId', + signature: '0x00', + status: 'shown', + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(getAccessToken).toHaveBeenCalledTimes(1); + }); + + it('throws on status 500', async () => { + const { backend, fetchMock } = setup(); + + fetchMock.mockResolvedValueOnce({ status: 500 } as unknown as Response); + + await expect( + backend.logSignature({ + coverageId: 'coverageId', + signature: '0x00', + status: 'shown', + }), + ).rejects.toThrow('Failed to log signature: 500'); + }); + }); + + describe('logTransaction', () => { + it('logs transaction', async () => { + const { backend, fetchMock, getAccessToken } = setup(); + + fetchMock.mockResolvedValueOnce({ status: 200 } as unknown as Response); + + await backend.logTransaction({ + coverageId: 'coverageId', + transactionHash: '0x00', + status: 'shown', + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(getAccessToken).toHaveBeenCalledTimes(1); + }); + + it('throws on status 500', async () => { + const { backend, fetchMock } = setup(); + + fetchMock.mockResolvedValueOnce({ status: 500 } as unknown as Response); + + await expect( + backend.logTransaction({ + coverageId: 'coverageId', + transactionHash: '0x00', + status: 'shown', + }), + ).rejects.toThrow('Failed to log transaction: 500'); + }); + }); }); diff --git a/packages/shield-controller/src/backend.ts b/packages/shield-controller/src/backend.ts index 184d742c6df..662be563f4c 100644 --- a/packages/shield-controller/src/backend.ts +++ b/packages/shield-controller/src/backend.ts @@ -1,7 +1,13 @@ import type { SignatureRequest } from '@metamask/signature-controller'; import type { TransactionMeta } from '@metamask/transaction-controller'; -import type { CoverageResult, CoverageStatus, ShieldBackend } from './types'; +import type { + CoverageResult, + CoverageStatus, + LogSignatureRequest, + LogTransactionRequest, + ShieldBackend, +} from './types'; export type InitCoverageCheckRequest = { txParams: [ @@ -88,7 +94,8 @@ export class ShieldRemoteBackend implements ShieldBackend { reqBody, ); - return this.#getCoverageResult(coverageId); + const coverageResult = await this.#getCoverageResult(coverageId); + return { coverageId, status: coverageResult.status }; } async checkSignatureCoverage( @@ -111,7 +118,36 @@ export class ShieldRemoteBackend implements ShieldBackend { reqBody, ); - return this.#getCoverageResult(coverageId); + const coverageResult = await this.#getCoverageResult(coverageId); + return { coverageId, status: coverageResult.status }; + } + + async logSignature(req: LogSignatureRequest): Promise { + const res = await this.#fetch( + `${this.#baseUrl}/v1/signature/coverage/log`, + { + method: 'POST', + headers: await this.#createHeaders(), + body: JSON.stringify(req), + }, + ); + if (res.status !== 200) { + throw new Error(`Failed to log signature: ${res.status}`); + } + } + + async logTransaction(req: LogTransactionRequest): Promise { + const res = await this.#fetch( + `${this.#baseUrl}/v1/transaction/coverage/log`, + { + method: 'POST', + headers: await this.#createHeaders(), + body: JSON.stringify(req), + }, + ); + if (res.status !== 200) { + throw new Error(`Failed to log transaction: ${res.status}`); + } } async #initCoverageCheck( diff --git a/packages/shield-controller/src/index.ts b/packages/shield-controller/src/index.ts index b435824513d..07ebbb3b149 100644 --- a/packages/shield-controller/src/index.ts +++ b/packages/shield-controller/src/index.ts @@ -1,4 +1,8 @@ -export type { CoverageStatus } from './types'; +export type { + CoverageStatus, + LogSignatureRequest, + LogTransactionRequest, +} from './types'; export type { ShieldControllerActions, ShieldControllerEvents, diff --git a/packages/shield-controller/src/types.ts b/packages/shield-controller/src/types.ts index eb602e0fb6e..5506c0e39ad 100644 --- a/packages/shield-controller/src/types.ts +++ b/packages/shield-controller/src/types.ts @@ -2,13 +2,28 @@ import type { SignatureRequest } from '@metamask/signature-controller'; import type { TransactionMeta } from '@metamask/transaction-controller'; export type CoverageResult = { + coverageId: string; status: CoverageStatus; }; export const coverageStatuses = ['covered', 'malicious', 'unknown'] as const; export type CoverageStatus = (typeof coverageStatuses)[number]; +export type LogSignatureRequest = { + coverageId: string; + signature: string; + status: string; +}; + +export type LogTransactionRequest = { + coverageId: string; + transactionHash: string; + status: string; +}; + export type ShieldBackend = { + logSignature: (req: LogSignatureRequest) => Promise; + logTransaction: (req: LogTransactionRequest) => Promise; checkCoverage: (txMeta: TransactionMeta) => Promise; checkSignatureCoverage: ( signatureRequest: SignatureRequest, diff --git a/packages/shield-controller/tests/mocks/backend.ts b/packages/shield-controller/tests/mocks/backend.ts index 3963f3aecb9..53e4afecc5f 100644 --- a/packages/shield-controller/tests/mocks/backend.ts +++ b/packages/shield-controller/tests/mocks/backend.ts @@ -1,3 +1,5 @@ +export const MOCK_COVERAGE_ID = '1'; + /** * Create a mock backend. * @@ -6,10 +8,14 @@ export function createMockBackend() { return { checkCoverage: jest.fn().mockResolvedValue({ + coverageId: MOCK_COVERAGE_ID, status: 'covered', }), checkSignatureCoverage: jest.fn().mockResolvedValue({ + coverageId: MOCK_COVERAGE_ID, status: 'covered', }), + logSignature: jest.fn(), + logTransaction: jest.fn(), }; }