Skip to content

Commit d095ab1

Browse files
feat: Shield: log transaction and signature (#6633)
## Explanation Context: ShieldController Once a transaction is submitted or a signature request is fulfilled, the transaction hash or signature, respectively, are logged on the backend. <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 7d08c95 commit d095ab1

File tree

8 files changed

+385
-15
lines changed

8 files changed

+385
-15
lines changed

packages/shield-controller/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Add two new controller state metadata properties: `includeInStateLogs` and `usedInUi` ([#6504](https://github.com/MetaMask/core/pull/6504))
1313
- Add signature coverage checking ([#6501](https://github.com/MetaMask/core/pull/6501))
14+
- Add transaction and signature logging ([#6633](https://github.com/MetaMask/core/pull/6633))
1415

1516
### Changed
1617

packages/shield-controller/src/ShieldController.test.ts

Lines changed: 179 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
import { deriveStateFromMetadata } from '@metamask/base-controller';
2-
import type { SignatureControllerState } from '@metamask/signature-controller';
3-
import type { TransactionControllerState } from '@metamask/transaction-controller';
2+
import type { SignatureRequest } from '@metamask/signature-controller';
3+
import {
4+
SignatureRequestStatus,
5+
type SignatureControllerState,
6+
} from '@metamask/signature-controller';
7+
import type { TransactionMeta } from '@metamask/transaction-controller';
8+
import {
9+
TransactionStatus,
10+
type TransactionControllerState,
11+
} from '@metamask/transaction-controller';
412

513
import { ShieldController } from './ShieldController';
6-
import { createMockBackend } from '../tests/mocks/backend';
14+
import { createMockBackend, MOCK_COVERAGE_ID } from '../tests/mocks/backend';
715
import { createMockMessenger } from '../tests/mocks/messenger';
816
import {
917
generateMockSignatureRequest,
@@ -218,6 +226,174 @@ describe('ShieldController', () => {
218226
);
219227
});
220228

229+
describe('logSignature', () => {
230+
/**
231+
* Run a test that logs a signature.
232+
*
233+
* @param components - An object containing the messenger and base messenger.
234+
* @param options - An object containing optional parameters.
235+
* @param options.updateSignatureRequest - A function that updates the signature request.
236+
*/
237+
async function runTest(
238+
components: ReturnType<typeof setup>,
239+
options?: {
240+
updateSignatureRequest?: (signatureRequest: SignatureRequest) => void;
241+
},
242+
) {
243+
const { messenger, baseMessenger } = components;
244+
245+
// Create a promise that resolves when the state changes
246+
const stateUpdated = new Promise((resolve) =>
247+
messenger.subscribe('ShieldController:stateChange', resolve),
248+
);
249+
250+
// Publish a signature request
251+
const signatureRequest = generateMockSignatureRequest();
252+
baseMessenger.publish(
253+
'SignatureController:stateChange',
254+
{
255+
signatureRequests: { [signatureRequest.id]: signatureRequest },
256+
} as SignatureControllerState,
257+
undefined as never,
258+
);
259+
260+
// Wait for state to be updated
261+
await stateUpdated;
262+
263+
// Update signature request
264+
const updatedSignatureRequest = { ...signatureRequest };
265+
updatedSignatureRequest.status = SignatureRequestStatus.Signed;
266+
updatedSignatureRequest.rawSig = '0x00';
267+
options?.updateSignatureRequest?.(updatedSignatureRequest);
268+
baseMessenger.publish(
269+
'SignatureController:stateChange',
270+
{
271+
signatureRequests: { [signatureRequest.id]: updatedSignatureRequest },
272+
} as SignatureControllerState,
273+
undefined as never,
274+
);
275+
}
276+
277+
it('logs a signature', async () => {
278+
const components = setup();
279+
280+
await runTest(components);
281+
282+
// Check that backend was called
283+
expect(components.backend.logSignature).toHaveBeenCalledWith({
284+
coverageId: MOCK_COVERAGE_ID,
285+
signature: '0x00',
286+
status: 'shown',
287+
});
288+
});
289+
290+
it('does not log when coverageId is missing', async () => {
291+
const components = setup();
292+
293+
components.backend.checkSignatureCoverage.mockResolvedValue({
294+
coverageId: undefined,
295+
status: 'unknown',
296+
});
297+
298+
await runTest(components);
299+
300+
// Check that backend was not called
301+
expect(components.backend.logSignature).not.toHaveBeenCalled();
302+
});
303+
304+
it('does not log when signature is missing', async () => {
305+
const components = setup();
306+
307+
await runTest(components, {
308+
updateSignatureRequest: (signatureRequest) => {
309+
signatureRequest.rawSig = undefined;
310+
},
311+
});
312+
313+
// Check that backend was not called
314+
expect(components.backend.logSignature).not.toHaveBeenCalled();
315+
});
316+
});
317+
318+
describe('logTransaction', () => {
319+
/**
320+
* Runs a test that logs a transaction.
321+
*
322+
* @param components - An object containing the messenger and base messenger.
323+
* @param options - Options for the test.
324+
* @param options.updateTransaction - A function that updates the transaction.
325+
*/
326+
async function runTest(
327+
components: ReturnType<typeof setup>,
328+
options?: { updateTransaction: (txMeta: TransactionMeta) => void },
329+
) {
330+
const { messenger, baseMessenger } = components;
331+
// Create a promise that resolves when the state changes
332+
const stateUpdated = new Promise((resolve) =>
333+
messenger.subscribe('ShieldController:stateChange', resolve),
334+
);
335+
336+
// Publish a transaction
337+
const txMeta = generateMockTxMeta();
338+
baseMessenger.publish(
339+
'TransactionController:stateChange',
340+
{ transactions: [txMeta] } as TransactionControllerState,
341+
undefined as never,
342+
);
343+
344+
// Wait for state to be updated
345+
await stateUpdated;
346+
347+
// Update transaction
348+
const updatedTxMeta = { ...txMeta };
349+
updatedTxMeta.status = TransactionStatus.submitted;
350+
updatedTxMeta.hash = '0x00';
351+
options?.updateTransaction(updatedTxMeta);
352+
baseMessenger.publish(
353+
'TransactionController:stateChange',
354+
{ transactions: [updatedTxMeta] } as TransactionControllerState,
355+
undefined as never,
356+
);
357+
}
358+
359+
it('logs a transaction', async () => {
360+
const components = setup();
361+
await runTest(components);
362+
363+
// Check that backend was called
364+
expect(components.backend.logTransaction).toHaveBeenCalledWith({
365+
coverageId: MOCK_COVERAGE_ID,
366+
status: 'shown',
367+
transactionHash: '0x00',
368+
});
369+
});
370+
371+
it('does not log when coverageId is missing', async () => {
372+
const components = setup();
373+
374+
components.backend.checkCoverage.mockResolvedValue({
375+
coverageId: undefined,
376+
status: 'unknown',
377+
});
378+
379+
await runTest(components);
380+
381+
// Check that backend was not called
382+
expect(components.backend.logTransaction).not.toHaveBeenCalled();
383+
});
384+
385+
it('does not log when hash is missing', async () => {
386+
const components = setup();
387+
388+
await runTest(components, {
389+
updateTransaction: (txMeta) => delete txMeta.hash,
390+
});
391+
392+
// Check that backend was not called
393+
expect(components.backend.logTransaction).not.toHaveBeenCalled();
394+
});
395+
});
396+
221397
describe('metadata', () => {
222398
it('includes expected state in debug snapshots', () => {
223399
const { controller } = setup();

packages/shield-controller/src/ShieldController.ts

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ import type {
44
RestrictedMessenger,
55
} from '@metamask/base-controller';
66
import {
7+
SignatureRequestStatus,
78
SignatureRequestType,
89
type SignatureRequest,
910
type SignatureStateChange,
1011
} from '@metamask/signature-controller';
11-
import type {
12-
TransactionControllerStateChangeEvent,
13-
TransactionMeta,
12+
import {
13+
TransactionStatus,
14+
type TransactionControllerStateChangeEvent,
15+
type TransactionMeta,
1416
} from '@metamask/transaction-controller';
1517

1618
import { controllerName } from './constants';
@@ -230,6 +232,17 @@ export class ShieldController extends BaseController<
230232
(error) => log('Error checking coverage:', error),
231233
);
232234
}
235+
236+
// Log signature once the signature request has been fulfilled.
237+
if (
238+
signatureRequest.status === SignatureRequestStatus.Signed &&
239+
signatureRequest.status !== previousSignatureRequest?.status
240+
) {
241+
this.#logSignature(signatureRequest).catch(
242+
// istanbul ignore next
243+
(error) => log('Error logging signature:', error),
244+
);
245+
}
233246
}
234247
}
235248

@@ -256,6 +269,17 @@ export class ShieldController extends BaseController<
256269
(error) => log('Error checking coverage:', error),
257270
);
258271
}
272+
273+
// Log transaction once it has been submitted.
274+
if (
275+
transaction.status === TransactionStatus.submitted &&
276+
transaction.status !== previousTransaction?.status
277+
) {
278+
this.#logTransaction(transaction).catch(
279+
// istanbul ignore next
280+
(error) => log('Error logging transaction:', error),
281+
);
282+
}
259283
}
260284
}
261285

@@ -346,4 +370,49 @@ export class ShieldController extends BaseController<
346370
}
347371
});
348372
}
373+
374+
async #logSignature(signatureRequest: SignatureRequest) {
375+
const coverageId = this.#getLatestCoverageId(signatureRequest.id);
376+
if (!coverageId) {
377+
throw new Error('Coverage ID not found');
378+
}
379+
380+
const sig = signatureRequest.rawSig;
381+
if (!sig) {
382+
throw new Error('Signature not found');
383+
}
384+
385+
await this.#backend.logSignature({
386+
coverageId,
387+
signature: sig,
388+
// Status is 'shown' because the coverageId can only be retrieved after
389+
// the result is in the state. If the result is in the state, we assume
390+
// that it has been shown.
391+
status: 'shown',
392+
});
393+
}
394+
395+
async #logTransaction(txMeta: TransactionMeta) {
396+
const coverageId = this.#getLatestCoverageId(txMeta.id);
397+
if (!coverageId) {
398+
throw new Error('Coverage ID not found');
399+
}
400+
401+
const txHash = txMeta.hash;
402+
if (!txHash) {
403+
throw new Error('Transaction hash not found');
404+
}
405+
await this.#backend.logTransaction({
406+
coverageId,
407+
transactionHash: txHash,
408+
// Status is 'shown' because the coverageId can only be retrieved after
409+
// the result is in the state. If the result is in the state, we assume
410+
// that it has been shown.
411+
status: 'shown',
412+
});
413+
}
414+
415+
#getLatestCoverageId(itemId: string) {
416+
return this.state.coverageResults[itemId]?.results[0]?.coverageId;
417+
}
349418
}

0 commit comments

Comments
 (0)