Repository navigation
feat(keyring-eth-money-mpc): skip MFA for whitelisted signatures - #670
Draft
matthiasgeihs wants to merge 40 commits into
Draft
matthiasgeihs wants to merge 40 commits into
matthiasgeihs wants to merge 40 commits into
Conversation
Introduce a 2-party client/server MPC keyring with DKLS23 TSS, cloud backup sync, and vendored MFA wallet libs. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Pass convertV so recovery id 27|28 becomes yParity or EIP-155 v. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Replace backupId with shareEpoch so create/rotate append and activate epochs only after backup readiness, matching the robust MPC architecture. Co-authored-by: Cursor <cursoragent@cursor.com>
Prevent mid-sign rotate/sync from being overwritten by stale Co-authored-by: Cursor <cursoragent@cursor.com> #applyKeyState snapshots from concurrent signing.
Pass the create/rotate session nonce to storeKeyShareBackup so a delayed backup from an overwritten retry cannot attach to a newer share. Co-authored-by: Cursor <cursoragent@cursor.com>
<!--
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 issues or other links reviewers should consult to
understand this pull request better? For instance:
* Fixes #12345
* See: #67890
-->
## Summary
- MPC cloud HTTP calls now send the profile token in an `Authorization:
Bearer <token>` header instead of including `token` in JSON request
bodies.
- The shared `postJson` helper accepts the token as a separate argument
and applies it consistently across all MPC backend requests.
- Token-only endpoints (`getNetId`, `checkKeyShareBackupId`,
`loadKeyShareBackup`) still use POST but now send an empty JSON body
(`{}`).
## Test plan
- [x] Run `packages/keyring-eth-mpc` unit tests (`cloud.test.ts`)
- [x] Verify `getNetId` requests include the Bearer header and send `{}`
as the body
- [x] Verify sign and backup helpers include the Bearer header and omit
`token` from the JSON payload
- [ ] Confirm MPC backend accepts Bearer authentication for affected
endpoints before merge
<!--
Are there any examples of this change being used in another repository?
When considering changes to the MetaMask module template, it's strongly
preferred that the change be experimented with in another repository
first. This gives reviewers a better sense of how the change works,
making it less likely the change will need to be reverted or adjusted
later.
-->
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Auth moves from JSON to Bearer headers across all MPC cloud calls;
clients and the backend must agree before deploy or requests will fail.
>
> **Overview**
> MPC cloud HTTP clients no longer put the profile `token` in JSON
bodies. **`postJson`** now takes the token separately and sets
**`Authorization: Bearer <token>`** on every backend POST.
>
> Endpoints that previously sent only `{ token }` (**`getNetId`**,
**`checkKeyShare`**, **`loadKeyShareBackup`**) now POST **`{}`** with
auth in the header. Create/sign/rotate/backup/register calls keep their
payloads but drop the **`token`** field. Unit tests in
**`cloud.test.ts`** assert the header and that **`body.token`** is
absent.
>
> The unreleased **CHANGELOG** entry is shortened to a single
initial-release line (detailed feature bullets removed).
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
400e5b1. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: Matthias Geihs <matthias.geihs@consensys.net>
<!-- 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 issues or other links reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Summary - Add a shared `parseJsonResponse` helper in `packages/keyring-eth-mpc/src/cloud.ts` to centralize JSON parsing and non-OK error handling for cloud API responses. - Introduce `getJson` for Bearer-authenticated GET requests and refactor `postJson` to reuse `parseJsonResponse`. - Update `loadKeyShareBackup` to call `getJson` with GET instead of POST with an empty body. <!-- Are there any examples of this change being used in another repository? When considering changes to the MetaMask module template, it's strongly preferred that the change be experimented with in another repository first. This gives reviewers a better sense of how the change works, making it less likely the change will need to be reverted or adjusted later. --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes the HTTP method for loading encrypted key-share backups; behavior depends on the MPC backend accepting GET on that route. > > **Overview** > Refactors MPC cloud HTTP helpers and fixes how encrypted key-share backups are fetched. > > **`cloud.ts`** adds **`parseJsonResponse`** for shared non-OK handling and JSON/empty-body parsing, introduces **`getJson`** for Bearer-authenticated GETs, and routes **`postJson`** through the same parser (generic renamed to **`Result`**). **`loadKeyShareBackup`** now calls **`getJson`** on `/load-key-share-backup` instead of **`postJson`** with an empty body. > > **`cloud.test.ts`** asserts the backup load uses **GET** with **`Authorization: Bearer`** and no POST body. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 2690e54. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Matthias Geihs <matthias.geihs@consensys.net> Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
- Implemented a new function `waitForDoneStatus` to ensure the backend has completed the create and rotate operations by checking for a `status: done` message. - Updated the `MPCKeyring` class to call this function after initiating create and rotate sessions. - Enhanced tests to verify behavior when receiving non-done status messages during the create process. <!-- 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 issues or other links reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Examples <!-- Are there any examples of this change being used in another repository? When considering changes to the MetaMask module template, it's strongly preferred that the change be experimented with in another repository first. This gives reviewers a better sense of how the change works, making it less likely the change will need to be reverted or adjusted later. --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes ordering and failure modes for MPC key creation, rotation, and signing against the backend; mistakes could block wallet setup or drop signatures, though signing is explicitly lenient. > > **Overview** > MPC key create and share rotation now **block until the cloud peer sends a `status` message with payload `done`**, via new `waitForDoneStatus`, before disconnecting and continuing with backup storage / epoch activation. If the backend reports anything else (e.g. `running`), **create and rotate fail** with `Expected status done, received …`, so backups are not written when persistence is incomplete. > > **Signing** still requests the same status after TSS sign, but **non-`done` status is ignored** so a late backend signal does not discard an already-computed signature (signing does not mutate shares). Tests and the changelog document this split behavior. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d8b464a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
A backend that never responds would otherwise hold the keyring mutex until the client restarts. Co-authored-by: Cursor <cursoragent@cursor.com>
A failed sign may have consumed the cached setup, so the next sign rebuilds it with the peer. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the 2-of-2 threshold next to the other share constants. Co-authored-by: Cursor <cursoragent@cursor.com>
Match the Dkls23-style casing. The persisted keyring type string stays 'MPC Keyring'. Co-authored-by: Cursor <cursoragent@cursor.com>
Public instance methods already had JSDoc. This covers the class, static type, and constructor. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Reject a drifted cloud body or malformed keyring state before it is used. Co-authored-by: Cursor <cursoragent@cursor.com>
Run init inside #serializeOp with the initialized check inside the queue, so overlapping init calls run key generation or import once. The second call waits, sees the initialized state and returns. Co-authored-by: Cursor <cursoragent@cursor.com>
The SDK dropped the -lib suffix and the util package. The keyring now uses the network package's identity serializer and scoped session id. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the V1 MPC keyring implementation together with the V2 adapter already on main. Co-authored-by: Cursor <cursoragent@cursor.com>
Changes required after the changes made to the MFA SDK.
Map relative .js specifiers back to TypeScript sources so the MPC keyring tests can load. Also point the skipLibCheck note at the MFA network package. Co-authored-by: Cursor <cursoragent@cursor.com>
…etup mode A payload with invalid initialized fields and mode was accepted as setup because extra keys are ignored, which dropped the key material. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop publicToAddressHex and compare recovered keys with publicKeyToAddressHex, which accepts raw and SEC1 keys. Co-authored-by: Cursor <cursoragent@cursor.com>
Reuse the shared AES-GCM helpers for key-share backup encryption. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
matthiasgeihs
marked this pull request as draft
October 13, 2026 15:22
…ounts (#669) ## Summary - Rename the package from `keyring-eth-mpc` / `@metamask/eth-mpc-keyring` to `keyring-eth-money-mpc` / `@metamask/eth-money-mpc-keyring`. - Rename the V1 and V2 keyring classes to `MpcMoneyKeyring`, and the controller type to `MPC Money Keyring`. - Update the related public types, source filenames, and monorepo references (README, tsconfig, Yarn constraints, PR title scope). ## Test plan - [ ] `yarn workspace @metamask/eth-money-mpc-keyring test` - [ ] Confirm the controller still registers the keyring under `MPC Money Keyring` - [ ] Confirm `KeyringType.Mpc` and `KeyringAccountEntropyTypeOption.Mpc` are unchanged Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
Money accounts do not sign transactions, so drop signTransaction and the helpers that only existed for it. Co-authored-by: Cursor <cursoragent@cursor.com>
Ask the whitelist before prompting, and retry once with a 2FA token when the backend still returns mfa_required. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
matthiasgeihs
force-pushed
the
mg/money-mpc-keyring
branch
from
October 13, 2026 20:02
40c1573 to
58d90f9
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@metamask/money-account-mfa-whitelistallows, usinggetMfaWhitelistConfigfor the pinned config./sign. If the backend responds with{ "code": "mfa_required" }, request a 2FA profile token and call/signonce more without the request.@metamask/money-account-mfa-whitelistis consumed from the preview build@metamask-previews/money-account-mfa-whitelist@0.0.0-preview-6ccf7a055. Replace that with a semver range once the package is published.Test plan
getProfileTokenis called withtwoFactor: falseand/signincludes the request.{ "code": "mfa_required" }from/signand confirm the keyring prompts for 2FA and retries once with a new nonce and no request.mfa_required, ormfa_requiredafter a token that already has 2FA, throws and does not open a signing session.yarn workspace @metamask/eth-money-mpc-keyring test