Skip to content

Commit

Permalink
feat: Add metrics for provider calls coming from ppom on extension (#…
Browse files Browse the repository at this point in the history
…21482)

## **Description**
We need to add metrics to help measure and estimate the usage and cost
of Infura by ppom.

This fix uses the `providerRequestsCount` object returned as part of the
securityAlertResponse of transaction object and gets only the keys that
matches the below specified requests.

### Proposal
Add new properties to Transactions and Signature events where which
property will count the number of rpc requests made by ppom to evaluate
that specific transaction or signature.

- [x] `ppom_eth_call_count` - counts the number of eth_call rpc requests
made by ppom to evaluate that transaction or signature
- [x] `ppom_eth_createAccessList_count` - counts the number of eth_call
rpc requests made by ppom to evaluate that transaction or signature
- [x] `ppom_eth_getStorageAt_count` - counts the number of eth_call rpc
requests made by ppom to evaluate that transaction or signature
- [x] `ppom_eth_getCode_count` - counts the number of eth_call rpc
requests made by ppom to evaluate that transaction or signature
- [x] `ppom_eth_getTrasanctionCount_count` - counts the number of
eth_call rpc requests made by ppom to evaluate that transaction or
signature
- [x] `ppom_eth_getBalance_count` - counts the number of eth_call rpc
requests made by ppom to evaluate that transaction or signature
- [x] `ppom_trace_call_count` - counts the number of eth_call rpc
requests made by ppom to evaluate that transaction or signature

### References
- [ppom provider usage
benchmark](https://wobbly-nutmeg-8a5.notion.site/MM-JSON-RPC-Benchmark-37d4a3bd74914b0fbe9147582c0cde51)
- [ppom required
json-rpcs](https://docs.google.com/document/d/1aOjVIsGHc0twc96V82OcXmpduGKrPjRScL_jwfgjkj0/edit?usp=sharing)


## **Manual testing steps**

1. Start extension with blockaid enabled
2. Go to test-dapp and initiate a blockaid transaction
3. When blockaid banner is shown, check that the above keys (or some of
it at least) are part of the metrics transaction or signature events

Fixes [#1357](https://github.com/MetaMask/MetaMask-planning/issues/1357)

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained:
  - [x] What problem this PR is solving.
  - [x] How this problem was solved.
  - [x] How reviewers can test my changes.
- [x] I’ve indicated what issue this PR is linked to: Fixes #???
- [ ] I’ve included tests if applicable.
- [x] I’ve documented any added code.
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
- [x] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
segun authored Oct 25, 2023
1 parent 2205735 commit 0a261a8
Show file tree
Hide file tree
Showing 4 changed files with 401 additions and 231 deletions.
10 changes: 10 additions & 0 deletions app/scripts/lib/createRPCMethodTrackingMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,16 @@ export default function createRPCMethodTrackingMiddleware({
const paramsExamplePassword = req?.params?.[2];

///: BEGIN:ONLY_INCLUDE_IN(blockaid)
if (req.securityAlertResponse?.providerRequestsCount) {
Object.keys(req.securityAlertResponse.providerRequestsCount).forEach(
(key) => {
const metricKey = `ppom_${key}_count`;
eventProperties[metricKey] =
req.securityAlertResponse.providerRequestsCount[key];
},
);
}

eventProperties.security_alert_response =
req.securityAlertResponse?.result_type ??
BlockaidResultType.NotApplicable;
Expand Down
41 changes: 41 additions & 0 deletions app/scripts/lib/createRPCMethodTrackingMiddleware.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,47 @@ describe('createRPCMethodTrackingMiddleware', () => {
});
});

it(`should track an event with correct blockaid parameters when providerRequestsCount is provided`, async () => {
const req = {
method: MESSAGE_TYPE.ETH_SIGN,
origin: 'some.dapp',
securityAlertResponse: {
result_type: BlockaidResultType.Malicious,
reason: BlockaidReason.maliciousDomain,
providerRequestsCount: {
eth_call: 5,
eth_getCode: 3,
},
},
};

const res = {
error: null,
};
const { next } = getNext();
await handler(req, res, next);
expect(trackEvent).toHaveBeenCalledTimes(1);
/**
* TODO:
* toMatchObject matches even if the the matched object does not contain some of the properties of the expected object
* I'm not sure why toMatchObject is used but we should probably check the other tests in this file for correctness in
* another PR.
*
*/
expect(trackEvent.mock.calls[0][0]).toStrictEqual({
category: 'inpage_provider',
event: MetaMetricsEventName.SignatureRequested,
properties: {
signature_type: MESSAGE_TYPE.ETH_SIGN,
security_alert_response: BlockaidResultType.Malicious,
security_alert_reason: BlockaidReason.maliciousDomain,
ppom_eth_call_count: 5,
ppom_eth_getCode_count: 3,
},
referrer: { url: 'some.dapp' },
});
});

it(`should track a ${MetaMetricsEventName.SignatureApproved} event if the user approves`, async () => {
const req = {
method: MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V4,
Expand Down
Loading

0 comments on commit 0a261a8

Please sign in to comment.