Skip to content
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

refactor: split keyring-api #24

Merged
merged 56 commits into from
Dec 6, 2024
Merged

refactor: split keyring-api #24

merged 56 commits into from
Dec 6, 2024

Conversation

ccharly
Copy link
Collaborator

@ccharly ccharly commented Sep 9, 2024

Description

Split of the keyring-api into smaller packages.

Fixes: https://github.com/MetaMask/accounts-planning/issues/729

@ccharly ccharly force-pushed the refactor/split-keyring-api branch 5 times, most recently from 4678982 to 55f15b4 Compare September 10, 2024 19:46
@ccharly
Copy link
Collaborator Author

ccharly commented Sep 10, 2024

@metamaskbot publish-preview

1 similar comment
@ccharly
Copy link
Collaborator Author

ccharly commented Sep 11, 2024

@metamaskbot publish-preview

Copy link

Preview builds have been published. See these instructions (from the core monorepo) for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/keyring-api": "8.1.0-67f3b77",
  "@metamask-previews/eth-hd-keyring": "7.0.1-67f3b77",
  "@metamask-previews/eth-ledger-bridge-keyring": "4.1.1-67f3b77",
  "@metamask-previews/eth-simple-keyring": "6.0.2-67f3b77",
  "@metamask-previews/eth-trezor-keyring": "3.1.0-67f3b77",
  "@metamask-previews/keyring-internal-api": "0.0.1-67f3b77",
  "@metamask-previews/eth-snap-keyring": "4.3.2-67f3b77",
  "@metamask-previews/keyring-snap-client": "0.0.1-67f3b77",
  "@metamask-previews/keyring-snap-sdk": "0.0.1-67f3b77",
  "@metamask-previews/keyring-utils": "0.0.1-67f3b77"
}

@ccharly ccharly force-pushed the refactor/split-keyring-api branch from 67f3b77 to 672cc7b Compare September 11, 2024 16:37
@ccharly
Copy link
Collaborator Author

ccharly commented Sep 11, 2024

@metamaskbot publish-preview

Copy link

Preview builds have been published. See these instructions (from the core monorepo) for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/keyring-api": "8.1.0-672cc7b",
  "@metamask-previews/eth-hd-keyring": "7.0.1-672cc7b",
  "@metamask-previews/eth-ledger-bridge-keyring": "4.1.1-672cc7b",
  "@metamask-previews/eth-simple-keyring": "6.0.2-672cc7b",
  "@metamask-previews/eth-trezor-keyring": "3.1.0-672cc7b",
  "@metamask-previews/keyring-internal-api": "0.0.1-672cc7b",
  "@metamask-previews/eth-snap-keyring": "4.3.2-672cc7b",
  "@metamask-previews/keyring-snap-client": "0.0.1-672cc7b",
  "@metamask-previews/keyring-snap-sdk": "0.0.1-672cc7b",
  "@metamask-previews/keyring-utils": "0.0.1-672cc7b"
}

@ccharly
Copy link
Collaborator Author

ccharly commented Sep 13, 2024

@metamaskbot publish-preview

Copy link

Preview builds have been published. See these instructions (from the core monorepo) for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/keyring-api": "8.1.0-b9b21be",
  "@metamask-previews/eth-hd-keyring": "7.0.1-b9b21be",
  "@metamask-previews/eth-ledger-bridge-keyring": "4.1.1-b9b21be",
  "@metamask-previews/eth-simple-keyring": "6.0.2-b9b21be",
  "@metamask-previews/eth-trezor-keyring": "3.1.0-b9b21be",
  "@metamask-previews/keyring-internal-api": "0.0.1-b9b21be",
  "@metamask-previews/eth-snap-keyring": "4.3.2-b9b21be",
  "@metamask-previews/keyring-snap-client": "0.0.1-b9b21be",
  "@metamask-previews/keyring-snap-sdk": "0.0.1-b9b21be",
  "@metamask-previews/keyring-utils": "0.0.1-b9b21be"
}

Copy link

This PR is marked as stale because it has been open for 60 days with no activity. Please remove the stale label or leave a comment, or it will be closed in 14 days.

@github-actions github-actions bot added the Stale label Nov 18, 2024
@danroc danroc removed the Stale label Nov 25, 2024
@ccharly
Copy link
Collaborator Author

ccharly commented Dec 5, 2024

@metamaskbot publish-preview

@ccharly
Copy link
Collaborator Author

ccharly commented Dec 5, 2024

@metamaskbot publish-preview

Copy link

github-actions bot commented Dec 5, 2024

Preview builds have been published. See these instructions (from the core monorepo) for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/keyring-api": "11.1.0-388ed88",
  "@metamask-previews/eth-hd-keyring": "9.0.0-388ed88",
  "@metamask-previews/eth-ledger-bridge-keyring": "7.0.0-388ed88",
  "@metamask-previews/eth-simple-keyring": "8.0.0-388ed88",
  "@metamask-previews/eth-trezor-keyring": "6.0.0-388ed88",
  "@metamask-previews/keyring-internal-api": "0.0.1-388ed88",
  "@metamask-previews/eth-snap-keyring": "6.0.0-388ed88",
  "@metamask-previews/keyring-snap-client": "0.0.1-388ed88",
  "@metamask-previews/keyring-snap-internal-client": "0.0.1-388ed88",
  "@metamask-previews/keyring-snap-sdk": "0.0.1-388ed88",
  "@metamask-previews/keyring-utils": "0.0.1-388ed88"
}

@ccharly
Copy link
Collaborator Author

ccharly commented Dec 5, 2024

@metamaskbot publish-preview

Copy link

github-actions bot commented Dec 5, 2024

Preview builds have been published. See these instructions (from the core monorepo) for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/keyring-api": "11.1.0-f78dac4",
  "@metamask-previews/eth-hd-keyring": "9.0.0-f78dac4",
  "@metamask-previews/eth-ledger-bridge-keyring": "7.0.0-f78dac4",
  "@metamask-previews/eth-simple-keyring": "8.0.0-f78dac4",
  "@metamask-previews/eth-trezor-keyring": "6.0.0-f78dac4",
  "@metamask-previews/keyring-internal-api": "0.0.1-f78dac4",
  "@metamask-previews/eth-snap-keyring": "6.0.0-f78dac4",
  "@metamask-previews/keyring-snap-client": "0.0.1-f78dac4",
  "@metamask-previews/keyring-snap-internal-client": "0.0.1-f78dac4",
  "@metamask-previews/keyring-snap-sdk": "0.0.1-f78dac4",
  "@metamask-previews/keyring-utils": "0.0.1-f78dac4"
}

@ccharly ccharly enabled auto-merge December 6, 2024 08:55
@ccharly ccharly added this pull request to the merge queue Dec 6, 2024
Merged via the queue into main with commit 0488168 Dec 6, 2024
29 checks passed
@ccharly ccharly deleted the refactor/split-keyring-api branch December 6, 2024 09:09
ccharly added a commit to MetaMask/core that referenced this pull request Dec 11, 2024
)

## Explanation

The `keyring-api` has been split out into multiple smaller packages,
this introduce some breaking changes since some exports have been moved
elsewhere.

This was done to reduce the number of required dependencies for the
`keyring-api`. The `InternalAccount` type (widely used internally by
some controllers) has been moved to a new package:
`@metamask/keyring-internal-api`.

## References

Relates to:
- MetaMask/accounts#24

Basic testing done through the CI + preview builds:
- MetaMask/metamask-extension#28861

## Changelog

### `@metamask/keyring-controller`

- **CHANGED**: Remove use of peer dependency `@metamask/providers`
- **CHANGED**: Bump `@metamask/keyring-api` to `^12.0.0`
- **CHANGED**: Use `@metamask/keyring-internal-api@^1.0.0`

> Even if we are bumping the major of the `keyring-api`, this is not
breaking since all
> types from the split are compatible. The only difference now here is
the `Keyring`
> interface which has a new optional method `listAccountTransactions`,
optional, thus
> non-breaking (and this type is not used there anyway)

### `@metamask/accounts-controller`

- **CHANGED**: Bump `@metamask/keyring-api` to `^12.0.0`
- **CHANGED**: Bump `@metamask/eth-snap-keyring` to `^7.0.0`
- **CHANGED**: Use `@metamask/keyring-internal-api@^1.0.0`

> `eth-snap-keyring` has been bumped to `7.0.0` but this was a mistake
since no breaking changes has been introduced there either.
> The version `6.0.0` also introduces `ts-bridge` builds, but this is
already compatible with this repo, so not breaking here either.

### `@metamask/assets-controllers`

- **CHANGED**: Remove use of `@metamask/keyring-api`
- **CHANGED**: Use `@metamask/keyring-internal-api@^1.0.0`

### `@metamask/chain-controller`

- **CHANGED**: Remove use of `@metamask/keyring-api`
- **CHANGED**: Use `@metamask/keyring-internal-api@^1.0.0`
- **CHANGED**: Use `@metamask/keyring-utils@^1.0.0`

### `@metamask/profile-sync-controller`

- **CHANGED**: Remove use of `@metamask/keyring-api`
- **CHANGED**: Use `@metamask/keyring-internal-api@^1.0.0`

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
github-merge-queue bot pushed a commit to MetaMask/metamask-mobile that referenced this pull request Jan 16, 2025
# **Description**

The primary purpose of this PR is to update the accounts controller and
the assets-controller to the latest version. In doing that, there were
several other package updates that were needed. Here is a summary of the
updates...


# 🔴  Major updates 🔴 

### "@metamask/accounts-controller": "^20.0.1" -> ^21.0.0
-
[changelog](https://github.com/MetaMask/core/blob/main/packages/accounts-controller/CHANGELOG.md#2100)
- Breaking changes
- BREAKING: Add scopes field to KeyringAccount
(MetaMask/core#5066),
(MetaMask/core#5136)
This field is now required and will be used to identify the supported
chains (using CAIP-2 chain IDs) for every accounts.
- Changes
- Bump @metamask/base-controller from ^7.0.0 to ^7.1.1
(MetaMask/core#5079),
(MetaMask/core#5135)
- Bump @metamask/utils to ^11.0.1
(MetaMask/core#5080)
- Bump @metamask/rpc-errors to ^7.0.2
(MetaMask/core#5080)
- Use new @metamask/keyring-internal-api@^1.0.0
(MetaMask/core#4695)
This package has been split out from the Keyring API.
- Bump @metamask/keyring-api from ^10.1.0 to ^12.0.0
(MetaMask/core#4695)
- Bump @metamask/eth-snap-keyring from ^5.0.1 to ^7.0.0
(MetaMask/core#4695)
ESM/CommonJS support.

### "@metamask/assets-controllers": "^45.1.1", ->  ^46.0.0",
-
[changelog](https://github.com/MetaMask/core/blob/main/packages/assets-controllers/CHANGELOG.md#4600)
- Breaking changes
- BREAKING: Bump @metamask/accounts-controller peer dependency from
^20.0.0 to ^21.0.0 (MetaMask/core#5140)
- Changes
- Remove use of @metamask/keyring-api
(MetaMask/core#4695)
@metamask/providers and webextension-polyfill peer dependencies are no
longer required.
- Use new @metamask/keyring-internal-api@^1.0.0
(MetaMask/core#4695)
This package has been split out from the Keyring API. Its types are
compatible with the @metamask/keyring-api package used previously.
- Bump @metamask/base-controller from ^7.0.0 to ^7.1.1
(MetaMask/core#5079),
(MetaMask/core#5135)
- Bump @metamask/keyring-api from ^12.0.0 to ^13.0.0
(MetaMask/core#5066)
- Bump @metamask/utils to ^11.0.1
(MetaMask/core#5080)
- Bump @metamask/rpc-errors to ^7.0.2
(MetaMask/core#5080)
- Added
- Add new MultichainBalancesController
(MetaMask/core#4965)
This controller has been migrated from the MetaMask extension codebase.

### "@metamask/utils": "^10.0.1" -> ^11.0.1"
-
[changelog](https://github.com/MetaMask/utils/blob/main/CHANGELOG.md#1101)
- breaking changes
- BREAKING: generateRandomMnemonic now returns Promise<void> instead of
void (MetaMask/utils#222)

### Added "@metamask/keyring-internal-api": "^2.0.0",
-
[changelog](https://github.com/MetaMask/accounts/blob/main/packages/keyring-internal-api/CHANGELOG.md#200)
- This package was a peer dep on the latest accounts controller and
assets controller. Given this I figured we should add it now anyway.
- Changes needed
- update imports from `@metamask/keyring-api` to
`@metamask/keyring-internal-api`
   - add support for scopes in the InternalAccount object
- added migration (066.ts) to backfill the scopes to existing accounts.
- This change is based off a [similar change made in the
extension](MetaMask/metamask-extension#29195)
made by @ccharly

### Added @metamask/keyring-snap-client: "^2.0.0"
-
[changelog](https://github.com/MetaMask/accounts/blob/main/packages/keyring-snap-client/CHANGELOG.md#200)
- Added because KeyringClient is now exported from
`@metamask/keyring-snap-client` instead of `@metamask/keyring-api'`. See
`app/components/Views/AddAccountActions/AddAccountActions.tsx` for
changes.

### "@metamask/keyring-api": "^10.1.0", -> ^13.0.0"
-
[changelog](https://github.com/MetaMask/accounts/blob/main/packages/keyring-api/CHANGELOG.md#1300)
- Breaking changes
- BREAKING: Add scopes field to KeyringAccount
(MetaMask/accounts#101)
- BREAKING: Split into several smaller packages
(MetaMask/accounts#24)
        - This should improve dependencies management.
- Internal related types (internal to both clients) have been moved to
keyring-internal-* packages.
- Keyring API clients (mainly used by dapps) have been moved to
keyring-snap-client package.
        - Common utils have been moevd to keyring-utils package.

### "@metamask/eth-snap-keyring": "^5.0.1" -> ^7.0.0"
-
[changelog](https://github.com/MetaMask/accounts/blob/main/packages/keyring-eth-simple/CHANGELOG.md#700)
- breaking changes
- BREAKING: Increase minimum Node.js version to 16
(MetaMask/eth-simple-keyring#152)
- BREAKING: Bump @metamask/eth-sig-util from ^6.0.1 to ^7.0.0
(MetaMask/eth-simple-keyring#156)
- Bump @metamask/utils from ^5.0.0 to ^8.1.0
(MetaMask/eth-simple-keyring#153)
- Bump ethereum-cryptography from ^1.2.0 to ^2.1.2
(MetaMask/eth-simple-keyring#153)
- BREAKING: Bump @metamask/eth-sig-util dependency from ^7.0.3 to ^8.0.0
(MetaMask/accounts#79)
- signTypedData no longer support number for addresses, see
[here](https://github.com/MetaMask/eth-sig-util/blob/main/CHANGELOG.md#800).

## 🟡 Minor updates 🟡 

### "@metamask/base-controller": "^7.0.1", -> ^7.1.1
-
[changelog](https://github.com/MetaMask/core/blob/main/packages/base-controller/CHANGELOG.md#711)
- Changes
    -  Bump @metamask/utils from ^10.0.0 to ^11.0.1
- Rename ControllerMessenger to Messenger
(MetaMask/core#5050)
        - ControllerMessenger has been renamed to Messenger
- RestrictedControllerMessengerConstraint has been renamed to
RestrictedMessengerConstraint
- RestrictedControllerMessenger has been renamed to RestrictedMessenger
- The RestrictedMessenger constructor parameter controllerMessenger has
been renamed to messenger, though the old name is still accepted
- The old names remain exported as deprecated aliases of the new names,
so this is not a breaking change.

### "@metamask/providers": "^18.1.0" -> ^18.3.1"
-
[changelog](https://github.com/MetaMask/providers/blob/main/CHANGELOG.md#1831)
- Changes
- Bump @metamask/json-rpc-engine from ^10.0.1 to ^10.0.2
(MetaMask/providers#397)
- Bump @metamask/json-rpc-middleware-stream from ^8.0.5 to ^8.0.6
(MetaMask/providers#397)
- Bump @metamask/rpc-errors from ^7.0.1 to ^7.0.2
(MetaMask/providers#397)
- Bump @metamask/utils from ^10.0.0 to ^11.0.1
(MetaMask/providers#397)


## 🟢  Patch updates 🟢 

### "@metamask/json-rpc-engine": "^10.0.0" -> ^10.0.2",
-
[changelog](https://github.com/MetaMask/core/blob/main/packages/json-rpc-engine/CHANGELOG.md#1002)

### "@metamask/json-rpc-middleware-stream": "^8.0.2" -> 8.0.6"
-
[changelog](https://github.com/MetaMask/core/blob/main/packages/json-rpc-middleware-stream/CHANGELOG.md#806)
- changed
- Bump @metamask/json-rpc-engine from ^10.0.1 to ^10.0.2
(MetaMask/core#5082)
- Bump @metamask/utils from ^10.0.0 to ^11.0.1
(MetaMask/core#5080)

### "@metamask/rpc-errors": "^7.0.1" -> ^7.0.2"
-
[changelog](https://github.com/MetaMask/rpc-errors/blob/main/CHANGELOG.md#702)
- changes
- Bump @metamask/utils from ^10.0.0 to ^11.0.1
(MetaMask/rpc-errors#166)


## **Related issues**

Fixes: #12967
Fixes: #12966

Fixes: MetaMask/accounts-planning#758

Unblocks: #12599

## **Manual testing steps**

#### Basic import account flow with tokens


https://github.com/user-attachments/assets/1a8d3e59-34e4-413e-a3e8-7dbdc5f7424a



## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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.
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.

3 participants