-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
chore: use native utils for crypto functions (#23270) #23746
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
base: main
Are you sure you want to change the base?
Conversation
## **Description** This PR implements new `@metamask/native-utils` package for C++ cryptography instead of JS implementation. That provides significant performance improvements all across the app. Most visible improvements are during app startup and SRP imports, but for example `keccak256` helps also in many other places. This PR should also have really nice synergy with MetaMask/core#6654 that could shave of another tens of percent for login times. For now I am patching `@metamask/key-tree`, once MetaMask/key-tree#223 is done, patch could be removed, but it may take a while. ### Performance Optimization Results Device: Pixel 4a 5G Tested on SRP with 200+ accounts. | Metric | Before Optimization* | After Optimization | Improvement | % Faster | | :--- | :--- | :--- | :--- | :--- | | **App Loaded to Login screen** | 7s 333ms | **4s 750ms** | ⚡️ 2.58s faster | **35.2%** | | **Dashboard Loaded** | 14s 0ms | **6s 333ms** | ⚡️ 7.67s faster | **54.8%** | | **App is Responsive (60 FPS)** | 18s 783ms | **12s 166ms** | ⚡️ 6.62s faster | **35.2%** | | **SRP Import (Discovery)** | 276s 616ms | **203s 450ms** | ⚡️ 73.17s faster | **26.5%** | _* Before optimalization version has `@metamask/native-utils` completely removed (including secp256k1 that was merged before)._ There should be around 200 - 300ms improvement in account creation time but I am not including this in result because I did many measurements but spread was too big to conclude any results from it. Another big improvement for acccount creation should be MetaMask/core#6654 ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** 1. All accounts are discovered 2. Balances for tokens and total balance is correct 3. Correct receive addresses are generated ## **Screenshots/Recordings** ### Startup + Login https://github.com/user-attachments/assets/24c8ca90-5475-4fa8-9062-30f6fa5133b2 ### SRP Import Observe also FPS counter, as you can see optimized version is maintaining higher FPS (around ~20) compared to non-optimized (around ~10). That should be enough to make app usable even on very slow device during running accounts discovery. To improve FPS even more we need to optimize rerenders and some selectors. In order to reduce discovery total time even more it would require different strategies of discovery, for example instead doing account detail requests one by one, until you find empty one, you could for example request them in batches of 3 which should improve total time significantly. https://github.com/user-attachments/assets/9f6c9825-5d97-415e-903c-8f4327273a2d ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Replaces JS crypto with native implementations via @metamask/native-utils, adds runtime perf shims, patches dependencies, updates iOS pods/Jest config, and optimizes account sorting. > > - **Performance/crypto integration** > - Add `shimPerf.js` to monkey‑patch `@noble/*` and `js-sha3` with native `getPublicKey`, `hmacSha512`, and `keccak256` from `@metamask/native-utils`. > - Update `shim.js` to load the new perf shim. > - **Library patches** > - Patch `@ethereumjs/util` to export `pubToAddress` from `@metamask/native-utils`. > - Patch `@metamask/key-tree` Ed25519 to use `nativeUtils.getPublicKeyEd25519`. > - **Encryption** > - Replace `Buffer.from(..., 'utf-8')` with `TextEncoder().encode(...)` in `quick-crypto.ts` for key derivation and encryption inputs. > - **Selectors** > - Optimize `selectInternalAccounts` sorting by precomputing address indices to avoid repeated `toFormattedAddress` calls. > - **Testing** > - Add Jest mock `app/__mocks__/@metamask/native-utils.js` and map it in `jest.config.js`; expand `transformIgnorePatterns` for native modules. > - **iOS/Pods** > - Bump `NativeUtils` pod to `0.8.0`. > - **Dependencies** > - Upgrade `@metamask/native-utils` to `^0.8.0`; add `[email protected]`, `@noble/[email protected]`. > - Add Yarn patches/resolutions for `@ethereumjs/[email protected]` and `@metamask/[email protected]`; update `yarn.lock`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 7ff47ad. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsThis PR introduces fundamental changes to the cryptographic infrastructure of the MetaMask Mobile wallet:
These changes affect virtually every operation that involves:
Given the foundational nature of these crypto changes, ALL test suites should run to ensure no regressions in any wallet functionality. |
|
Cal-L
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm



Description
This PR is from an external contributor. Initial review and context here
Credit @Nodonisko
This PR implements new
@metamask/native-utilspackage for C++ cryptography instead of JS implementation. That provides significant performance improvements all across the app. Most visible improvements are during app startup and SRP imports, but for examplekeccak256helps also in many other places.This PR should also have really nice synergy with
MetaMask/core#6654 that could shave of another tens of percent for login times.
For now I am patching
@metamask/key-tree, onceMetaMask/key-tree#223 is done, patch could be removed, but it may take a while.
Performance Optimization Results
Device: Pixel 4a 5G
Tested on SRP with 200+ accounts.
* Before optimalization version has
@metamask/native-utilscompletely removed (including secp256k1 that was merged before).There should be around 200 - 300ms improvement in account creation time but I am not including this in result because I did many measurements but spread was too big to conclude any results from it. Another big improvement for acccount creation should be
MetaMask/core#6654
Changelog
CHANGELOG entry:
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Startup + Login
NativeUtilsLoginComparison.mp4
SRP Import
Observe also FPS counter, as you can see optimized version is maintaining higher FPS (around ~20) compared to non-optimized (around ~10). That should be enough to make app usable even on very slow device during running accounts discovery. To improve FPS even more we need to optimize rerenders and some selectors.
In order to reduce discovery total time even more it would require different strategies of discovery, for example instead doing account detail requests one by one, until you find empty one, you could for example request them in batches of 3 which should improve total time significantly.
NativeUtilsSRPComparison_small.mp4
Pre-merge author checklist
Standards.
Pre-merge reviewer checklist
Note
Replaces JS crypto with native implementations via @metamask/native-utils, adds perf shims, patches key libs, and updates tests/config to support it.
shimPerfto monkey-patch@nobleandjs-sha3(secp256k1.getPublicKey,hmacSha512,keccak256) to use native C++ via@metamask/native-utils.shim.jsto loadshimPerf.quick-cryptostring encoding toTextEncoderfor key derivation/encryption.@ethereumjs/utilto exportpubToAddressfrom@metamask/native-utils.@metamask/key-treeed25519to usenative-utils.getPublicKeyEd25519.selectInternalAccountssorting to pre-compute indices and reducetoFormattedAddresscalls.app/__mocks__/@metamask/native-utils.jsand map injest.config.js; extendtransformIgnorePatterns.@metamask/native-utilsto^0.8.0(iOSNativeUtilspod 0.8.0).[email protected], pin@noble/[email protected], add Yarn patches/locks for@ethereumjs/[email protected]and@metamask/[email protected].Written by Cursor Bugbot for commit 95728ed. This will update automatically on new commits. Configure here.
Description
Changelog
CHANGELOG entry:
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist