Skip to content

Commit

Permalink
perf: Improve Snaps execution performance (#13420)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Improve Snaps execution performance by bumping the Snaps packages to
latest. The latest packages have improved caching for Snap state and
changes the Snaps execution on mobile to run each Snap in a separate
WebView. This PR adapts `SnapsExecutionWebView` to support this new
execution architecture and also attempts to speed up key derivation by
using a native implementation of `hmacSha512`. Furthermore, the idle
timeout for Snaps is bumped to 5 minutes.

## **Related issues**

Fixes: MetaMask/snaps#2873

## **Manual testing steps**

Run any Snap and make sure it still works!
  • Loading branch information
FrederikBolding authored Feb 20, 2025
1 parent 8886b52 commit a9cd3af
Show file tree
Hide file tree
Showing 11 changed files with 354 additions and 181 deletions.
23 changes: 23 additions & 0 deletions app/core/Encryptor/hmac.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { NativeModules } from 'react-native';
import { hmacSha512 } from './hmac';
import { hmac as nobleHmac } from '@noble/hashes/hmac';
import { sha512 as nobleSha512 } from '@noble/hashes/sha512';
import { bytesToHex, hexToBytes, stringToBytes } from '@metamask/utils';

describe('hmacSha512', () => {
NativeModules.Aes.hmac512 = jest
.fn()
.mockImplementation((data, key) =>
bytesToHex(nobleHmac(nobleSha512, hexToBytes(key), stringToBytes(data))),
);

it('returns hash from native module', async () => {
const key = new Uint8Array(32);
const data = new Uint8Array(32);

const result = await hmacSha512(key, data);
expect(bytesToHex(result)).toBe(
'0xbae46cebebbb90409abc5acf7ac21fdb339c01ce15192c52fb9e8aa11a8de9a4ea15a045f2be245fbb98916a9ae81b353e33b9c42a55380c5158241daeb3c6dd',
);
});
});
16 changes: 16 additions & 0 deletions app/core/Encryptor/hmac.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import {
bytesToHex,
bytesToString,
hexToBytes,
remove0x,
} from '@metamask/utils';
import { NativeModules } from 'react-native';

export async function hmacSha512(key: Uint8Array, data: Uint8Array) {
const Aes = NativeModules.Aes;
const bytes = await Aes.hmac512(
bytesToString(data),
remove0x(bytesToHex(key)),
);
return hexToBytes(bytes);
}
1 change: 1 addition & 0 deletions app/core/Encryptor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ export {
DERIVATION_OPTIONS_DEFAULT_OWASP2023,
pbkdf2,
};
export * from './hmac';
33 changes: 23 additions & 10 deletions app/core/Engine/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ import {

import { WebViewExecutionService } from '@metamask/snaps-controllers/react-native';
import type { NotificationArgs } from '@metamask/snaps-rpc-methods/dist/restricted/notify.cjs';
import { getSnapsWebViewPromise } from '../../lib/snaps';
import { createWebView, removeWebView } from '../../lib/snaps';
import {
buildSnapEndowmentSpecifications,
buildSnapRestrictedMethodSpecifications,
Expand All @@ -85,7 +85,12 @@ import {
LedgerMobileBridge,
LedgerTransportMiddleware,
} from '@metamask/eth-ledger-bridge-keyring';
import { Encryptor, LEGACY_DERIVATION_OPTIONS, pbkdf2 } from '../Encryptor';
import {
Encryptor,
hmacSha512,
LEGACY_DERIVATION_OPTIONS,
pbkdf2,
} from '../Encryptor';
import {
isMainnetByChainId,
isTestNet,
Expand Down Expand Up @@ -139,7 +144,15 @@ import {
SignatureController,
SignatureControllerOptions,
} from '@metamask/signature-controller';
import { hasProperty, Hex, Json } from '@metamask/utils';
import {
///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps)
Duration,
inMilliseconds,
///: END:ONLY_INCLUDE_IF
hasProperty,
Hex,
Json,
} from '@metamask/utils';
import { providerErrors } from '@metamask/rpc-errors';

import { PPOM, ppomInit } from '../../lib/ppom/PPOMView';
Expand Down Expand Up @@ -323,7 +336,6 @@ export class Engine {
selectBasicFunctionalityEnabled(store.getState());

const approvalController = new ApprovalController({
// @ts-expect-error TODO: Resolve mismatch between base-controller versions.
messenger: this.controllerMessenger.getRestricted({
name: 'ApprovalController',
allowedEvents: [],
Expand Down Expand Up @@ -645,7 +657,6 @@ export class Engine {
);

const phishingController = new PhishingController({
// @ts-expect-error TODO: Resolve mismatch between base-controller versions.
messenger: this.controllerMessenger.getRestricted({
name: 'PhishingController',
allowedActions: [],
Expand Down Expand Up @@ -674,7 +685,7 @@ export class Engine {

const hdKeyringBuilder = () =>
new HDKeyring({
cryptographicFunctions: { pbkdf2Sha512: pbkdf2 },
cryptographicFunctions: { pbkdf2Sha512: pbkdf2, hmacSha512 },
});
hdKeyringBuilder.type = HDKeyring.type;
additionalKeyrings.push(hdKeyringBuilder);
Expand Down Expand Up @@ -821,7 +832,7 @@ export class Engine {
origin,
target,
),
getClientCryptography: () => ({ pbkdf2Sha512: pbkdf2 }),
getClientCryptography: () => ({ pbkdf2Sha512: pbkdf2, hmacSha512 }),
};
///: END:ONLY_INCLUDE_IF

Expand Down Expand Up @@ -1009,7 +1020,6 @@ export class Engine {
const requireAllowlist = process.env.METAMASK_BUILD_TYPE === 'main';
const disableSnapInstallation = process.env.METAMASK_BUILD_TYPE === 'main';
const allowLocalSnaps = process.env.METAMASK_BUILD_TYPE === 'flask';
// @ts-expect-error TODO: Resolve mismatch between base-controller versions.
const snapsRegistryMessenger: SnapsRegistryMessenger =
this.controllerMessenger.getRestricted({
name: 'SnapsRegistry',
Expand All @@ -1029,14 +1039,14 @@ export class Engine {
});

this.snapExecutionService = new WebViewExecutionService({
// @ts-expect-error TODO: Resolve mismatch between base-controller versions.
messenger: this.controllerMessenger.getRestricted({
name: 'ExecutionService',
allowedActions: [],
allowedEvents: [],
}),
setupSnapProvider: setupSnapProvider.bind(this),
getWebView: () => getSnapsWebViewPromise,
createWebView,
removeWebView,
});

const snapControllerMessenger = this.controllerMessenger.getRestricted({
Expand All @@ -1045,6 +1055,7 @@ export class Engine {
'ExecutionService:unhandledError',
'ExecutionService:outboundRequest',
'ExecutionService:outboundResponse',
'KeyringController:lock',
],
allowedActions: [
`${approvalController.name}:addRequest`,
Expand Down Expand Up @@ -1094,6 +1105,7 @@ export class Engine {
// TODO: Replace "any" with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
messenger: snapControllerMessenger as any,
maxIdleTime: inMilliseconds(5, Duration.Minute),
detectSnapLocation,
//@ts-expect-error types need to be aligned with snaps-controllers
preinstalledSnaps: PREINSTALLED_SNAPS,
Expand All @@ -1105,6 +1117,7 @@ export class Engine {
}),
clientCryptography: {
pbkdf2Sha512: pbkdf2,
hmacSha512,
},
});

Expand Down
5 changes: 4 additions & 1 deletion app/core/Permissions/specifications.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('PermissionController specifications', () => {
describe('caveat specifications', () => {
it('getCaveatSpecifications returns the expected specifications object', () => {
const caveatSpecifications = getCaveatSpecifications({});
expect(Object.keys(caveatSpecifications)).toHaveLength(13);
expect(Object.keys(caveatSpecifications)).toHaveLength(14);
expect(
caveatSpecifications[CaveatTypes.restrictReturnedAccounts].type,
).toStrictEqual(CaveatTypes.restrictReturnedAccounts);
Expand Down Expand Up @@ -60,6 +60,9 @@ describe('PermissionController specifications', () => {
expect(caveatSpecifications.lookupMatchers.type).toStrictEqual(
SnapCaveatType.LookupMatchers,
);
expect(caveatSpecifications.protocolSnapScopes.type).toStrictEqual(
SnapCaveatType.ProtocolSnapScopes,
);
});

describe('restrictReturnedAccounts', () => {
Expand Down
40 changes: 40 additions & 0 deletions app/lib/snaps/SnapsExecutionWebView.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import React from 'react';
import { render } from '@testing-library/react-native';

import {
createWebView,
removeWebView,
SnapsExecutionWebView,
} from './SnapsExecutionWebView';

describe('SnapsExecutionWebView', () => {
it('should render correctly', () => {
const wrapper = render(<SnapsExecutionWebView />);
expect(wrapper).toMatchInlineSnapshot(`
<RCTScrollView>
<View>
<View
style={
{
"height": 0,
}
}
/>
</View>
</RCTScrollView>
`);
});

it('should create and remove WebViews correctly', async () => {
const wrapper = render(<SnapsExecutionWebView />);
createWebView('foo');
createWebView('bar');
wrapper.rerender(<SnapsExecutionWebView />);
expect(await wrapper.queryByTestId('foo')).toBeTruthy();
expect(await wrapper.queryByTestId('bar')).toBeTruthy();
removeWebView('foo');
wrapper.rerender(<SnapsExecutionWebView />);
expect(await wrapper.queryByTestId('foo')).toBeNull();
expect(await wrapper.queryByTestId('bar')).toBeTruthy();
});
});
Loading

0 comments on commit a9cd3af

Please sign in to comment.