Skip to content

feat(wallet): add AccountsController and ConnectivityController#8924

Open
grypez wants to merge 29 commits into
mainfrom
grypez/wallet-account-controllers
Open

feat(wallet): add AccountsController and ConnectivityController#8924
grypez wants to merge 29 commits into
mainfrom
grypez/wallet-account-controllers

Conversation

@grypez
Copy link
Copy Markdown

@grypez grypez commented May 28, 2026

Explanation

Adds AccountsController and ConnectivityController as default initialized controllers in the wallet package, following the InitializationConfiguration pattern established by KeyringController. The required messenger delegation for AccountsController is wired up (keyring actions, SnapKeyring events, and MultichainNetworkController:networkDidChange). A temporary AlwaysOnlineAdapter is introduced for ConnectivityController pending a real platform adapter, and a createSecretRecoveryPhrase utility is added alongside the existing importSecretRecoveryPhrase.

AccountsController is deprecated in favour of AccountTreeController and MultichainAccountService, but both of those still consume AccountsController actions/events at the messenger level, so it must remain present as infrastructure. Migration is tracked via a TODO on the initialization file.

References

N/A

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Breaking wallet API (required connectivity adapter) plus account/keyring messenger wiring; scope is bounded to wallet initialization and a small accounts-controller constructor tweak.

Overview
Breaking change for @metamask/wallet consumers: default initialization now includes AccountsController and ConnectivityController, and instanceOptions.connectivityController.connectivityAdapter is required (tests use the new AlwaysOnlineAdapter stub until a real platform adapter is injected).

AccountsController is registered via the existing InitializationConfiguration pattern, with messenger delegation for keyring actions/events (including KeyringController:stateChange, Snap keyring account updates, and MultichainNetworkController:networkDidChange). Wallet tests assert keyring-created addresses appear in AccountsController state.

ConnectivityController is wired the same way, taking the injected adapter at init. @metamask/accounts-controller makes constructor state optional (defaults when omitted). Package deps, TS project references, README dependency graph, and CODEOWNERS paths are updated accordingly.

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

@grypez grypez force-pushed the grypez/wallet-account-controllers branch 4 times, most recently from d35a344 to 83241f6 Compare May 29, 2026 00:49
grypez and others added 3 commits May 29, 2026 15:45
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ctivityController

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ller initialization

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@grypez grypez force-pushed the grypez/wallet-account-controllers branch from ce86ff2 to e85be4e Compare May 30, 2026 12:26
@grypez grypez marked this pull request as ready for review May 30, 2026 12:47
@grypez grypez requested a review from a team as a code owner May 30, 2026 12:47
@grypez grypez temporarily deployed to default-branch May 30, 2026 12:47 — with GitHub Actions Inactive
Comment thread packages/wallet/src/initialization/instances/connectivity-controller.ts Outdated
Comment thread packages/wallet/src/initialization/instances/accounts-controller.ts Outdated
Copy link
Copy Markdown
Member

@FrederikBolding FrederikBolding Jun 1, 2026

Choose a reason for hiding this comment

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

Let's add a CODEOWNER rule for this that matches the controller (same applies to ConnectivityController).

Additionally, let's have the accounts team review the addition.

Comment thread packages/wallet/src/utilities.test.ts Outdated
Comment thread packages/wallet/src/initialization/instances/connectivity-controller.ts Outdated
Comment thread packages/wallet/src/Wallet.test.ts Outdated
grypez and others added 4 commits June 1, 2026 08:59
Use explicit type annotation instead of verbose generic type parameters,
leveraging the exported AccountsControllerMessenger type.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… init

Add connectivityAdapter to instanceOptions so mobile and extension can
supply a platform-specific adapter. Fall back to AlwaysOnlineAdapter when
none is provided. Call controller.init() from the factory so the initial
connectivity status is always fetched on construction.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ired init

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread packages/wallet/src/initialization/instances/connectivity-controller.ts Outdated
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
grypez and others added 5 commits June 1, 2026 12:43
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…explicit dependencies

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…OWNERS

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread packages/wallet/src/types.ts Outdated
Comment thread packages/wallet/src/initialization/instances/accounts-controller.ts Outdated
Comment thread packages/wallet/src/initialization/instances/connectivity-controller.ts Outdated
Comment thread packages/wallet/src/initialization/instances/accounts-controller.ts Outdated
grypez and others added 4 commits June 1, 2026 13:39
…vityController factory

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ller options are provided

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@grypez grypez requested a review from a team as a code owner June 1, 2026 19:37
grypez and others added 3 commits June 1, 2026 16:55
Comment thread packages/wallet/src/utilities.ts Outdated
grypez and others added 2 commits June 1, 2026 19:44
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…angelog

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@grypez
Copy link
Copy Markdown
Author

grypez commented Jun 2, 2026

Given mandatory connectivity adaptor is a breaking change, will need to prepare integration PRs in mobile and extension

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 488b4c6. Configure here.

Comment thread packages/wallet/CHANGELOG.md Outdated
Comment thread .github/CODEOWNERS Outdated
Comment thread packages/wallet/src/Wallet.ts Outdated
}
}

export const connectivityController: InitializationConfiguration<
Copy link
Copy Markdown
Member

@FrederikBolding FrederikBolding Jun 2, 2026

Choose a reason for hiding this comment

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

Do we not need a way to call init?

Perhaps we can just have an async function on the Wallet class that iterates each instance and calls init if it exists

Comment thread packages/wallet/src/types.ts Outdated
grypez and others added 6 commits June 2, 2026 05:16
Pass the connectivity adapter through `instanceOptions.connectivityController`
instead of a top-level `WalletOptions.connectivityAdapter` field, and make the
`connectivityController` instance options required. This drops the constructor
glue that re-wrapped the top-level option and keeps the wallet defaults
compatible across mobile and extension, since each consumer supplies its own
adapter.

Supersedes the top-level field introduced in 878145e, per PR review feedback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop @MetaMask/core-platform as co-owner of the wallet accounts-controller
initialization file so it matches the ownership of the accounts-controller
package itself (@MetaMask/accounts-engineers), per PR review feedback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tor state

Document that the `AccountsController` constructor's `state` option is now
optional, introduced in this PR alongside the wallet default-controller work.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-controllers

# Conflicts:
#	.github/CODEOWNERS
#	README.md
#	packages/wallet/CHANGELOG.md
#	packages/wallet/package.json
#	packages/wallet/src/initialization/instances/index.ts
#	packages/wallet/src/types.ts
#	packages/wallet/tsconfig.build.json
#	yarn.lock
…directory convention

Move the AccountsController and ConnectivityController initialization
configs into their own directories to match the per-controller layout
established by the ApprovalController instance, so the package isn't left
in two styles.

- accounts-controller/accounts-controller.ts
- connectivity-controller/{connectivity-controller,types,always-online-adapter}.ts
  with colocated tests, pulling AlwaysOnlineAdapter into its own sibling
  file (mirroring the keyring encryptor split) and extracting the
  connectivity instance options into a ConnectivityControllerInstanceOptions
  type.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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