Skip to content

feat(snaps-rpc-methods)!: update usage of withKeyring to withKeyringV2Unsafe#4009

Open
hmalik88 wants to merge 5 commits into
mainfrom
hm/mul-1824
Open

feat(snaps-rpc-methods)!: update usage of withKeyring to withKeyringV2Unsafe#4009
hmalik88 wants to merge 5 commits into
mainfrom
hm/mul-1824

Conversation

@hmalik88
Copy link
Copy Markdown
Contributor

@hmalik88 hmalik88 commented May 26, 2026

This PR updates usage of withKeyring to the new withKeyringV2. The plan is to convert withKeyringV2 to be withKeyring once usage across the codebase has been converted to v2.


Note

Medium Risk
Touches messenger paths that read HD keyring mnemonic/seed for Snap entropy RPCs; logic is unchanged but clients must expose the new action or entropy methods will fail at runtime.

Overview
This PR replaces all KeyringController:withKeyring messenger usage with KeyringController:withKeyringV2Unsafe as part of the broader KeyringController v2 migration.

In @metamask/snaps-rpc-methods, the shared type is renamed to KeyringControllerWithKeyringV2UnsafeAction, and entropy-related restricted methods (snap_getEntropy, snap_getBip32*, snap_getBip44Entropy) declare and call the new action. getMnemonic / getMnemonicSeed in utils.ts now invoke withKeyringV2Unsafe for primary and alternate entropy sources. Tests and Unreleased changelog entries are updated accordingly.

@metamask/snaps-simulation registers a mock handler for withKeyringV2Unsafe instead of withKeyring. snaps-controllers only updates a comment on WithSnapKeyringFunction to document the v2 unsafe API.

Behavior is intended to stay the same; this is primarily an integration rename so Snaps align with the new KeyringController surface before withKeyringV2 becomes the default withKeyring.

Reviewed by Cursor Bugbot for commit 408113b. Bugbot is set up for automated code reviews on this repo. Configure here.

@hmalik88 hmalik88 marked this pull request as ready for review May 26, 2026 20:36
@hmalik88 hmalik88 requested a review from a team as a code owner May 26, 2026 20:36
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.58%. Comparing base (7998f0d) to head (408113b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4009   +/-   ##
=======================================
  Coverage   98.58%   98.58%           
=======================================
  Files         425      425           
  Lines       12370    12370           
  Branches     1949     1949           
=======================================
  Hits        12195    12195           
  Misses        175      175           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


### Changed

- Update `withKeyring` to `withKeyringV2Unsafe` ([#4009](https://github.com/MetaMask/snaps/pull/4009))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Update `withKeyring` to `withKeyringV2Unsafe` ([#4009](https://github.com/MetaMask/snaps/pull/4009))
- Use `withKeyringV2Unsafe` for accessing entropy ([#4009](https://github.com/MetaMask/snaps/pull/4009))


### Changed

- Update `withKeyring` to `withKeyringV2Unsafe` ([#4009](https://github.com/MetaMask/snaps/pull/4009))
Copy link
Copy Markdown
Member

@FrederikBolding FrederikBolding May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Update `withKeyring` to `withKeyringV2Unsafe` ([#4009](https://github.com/MetaMask/snaps/pull/4009))
- **BREAKING:** Use `withKeyringV2Unsafe` for accessing entropy ([#4009](https://github.com/MetaMask/snaps/pull/4009))

@FrederikBolding FrederikBolding changed the title feat: update usage of withKeyring to withKeyringV2 feat!(snaps-rpc-methods): update usage of withKeyring to withKeyringV2Unsafe May 27, 2026
};

// Expecting a bound function that calls KeyringController.withKeyring selecting the Snap keyring
// Expecting a bound function that calls KeyringController.withKeyringV2Unsafe selecting the Snap keyring
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just revert this

'KeyringController:withKeyring',
'KeyringController:withKeyringV2Unsafe',
{
type: HD_KEYRING,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like type was changed, we use that for filtering, but also assert it below.

Please adjust tests as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though it seems that KeyringController expects a V1 type for filtering but then returns V2 when using withKeyringV2. Unsure if that is a bug that needs fixing first?

@FrederikBolding FrederikBolding changed the title feat!(snaps-rpc-methods): update usage of withKeyring to withKeyringV2Unsafe feat(snaps-rpc-methods)!: update usage of withKeyring to withKeyringV2Unsafe May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants