Skip to content

refactor: rename Controller types to MessengerClient types#28610

Merged
cryptodev-2s merged 1 commit into
mainfrom
rename-controller-types-to-messenger-client
Apr 10, 2026
Merged

refactor: rename Controller types to MessengerClient types#28610
cryptodev-2s merged 1 commit into
mainfrom
rename-controller-types-to-messenger-client

Conversation

@cryptodev-2s
Copy link
Copy Markdown
Contributor

@cryptodev-2s cryptodev-2s commented Apr 9, 2026

Description

Rename type definitions in Engine/types.ts to reflect that the init system handles all messenger clients (controllers + services), not just controllers. All consumer imports updated accordingly.

PR 1 of 4 in a series that updates the init system API to use "messenger client" terminology:

  • PR 1 (this): Core type renames (ControllerMessengerClient, ControllerInitFunctionMessengerClientInitFunction, etc.)
  • PR 2: CONTROLLER_MESSENGERSMESSENGER_FACTORIES
  • PR 3: initModularizedControllersinitMessengerClients + utils/Engine.ts renames
  • PR 4: Property renames (controllermessengerClient, getControllergetMessengerClient) in all init files + tests

Relates to WPC-916.

Changelog

CHANGELOG entry: null

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/WPC-916

Manual testing steps

N/A — no runtime behavior change.

Screenshots/Recordings

N/A — no UI changes.

Before

N/A

After

N/A

Pre-merge author checklist

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.

Note

Medium Risk
Broad mechanical type renames across many engine init modules and tests; low behavioral risk but moderate integration risk if any downstream code still expects the old Controller* types.

Overview
Updates the Engine init type API to use messenger client terminology: Controllers �� MessengerClients, ControllerName �� MessengerClientName, ControllerInitRequest/ControllerInitFunction �� MessengerClientInitRequest/MessengerClientInitFunction, and related mapping/request types in app/core/Engine/types.ts.

Propagates the rename through all affected init implementations and unit tests (e.g., accountsControllerInit, bridgeControllerInit, TransactionControllerInit, etc.) plus test utilities, without changing initialization logic beyond updated type annotations.

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2026

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.

@metamaskbot metamaskbot added the team-core-platform Core Platform team label Apr 9, 2026
@github-actions github-actions Bot added the size-L label Apr 9, 2026
@cryptodev-2s cryptodev-2s marked this pull request as ready for review April 9, 2026 19:18
@cryptodev-2s cryptodev-2s requested review from a team as code owners April 9, 2026 19:18
@cryptodev-2s cryptodev-2s force-pushed the rename-controller-types-to-messenger-client branch from cae0b4f to 69d60d0 Compare April 10, 2026 06:31
@github-actions github-actions Bot added the risk-low Low testing needed · Low bug introduction risk label Apr 10, 2026
@cryptodev-2s cryptodev-2s force-pushed the rename-controller-types-to-messenger-client branch from 69d60d0 to e97d56c Compare April 10, 2026 07:22
@github-actions github-actions Bot added risk-low Low testing needed · Low bug introduction risk and removed risk-low Low testing needed · Low bug introduction risk labels Apr 10, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeConfirmations, SmokeWalletPlatform
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 92%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR is a pure TypeScript type rename refactoring across 100 controller init files in app/core/Engine/controllers/. The change is exclusively:

  • ControllerInitFunctionMessengerClientInitFunction
  • ControllerInitRequestMessengerClientInitRequest

Every single diff examined shows ONLY this import rename and the corresponding type annotation update. No controller logic, initialization order, state schemas, messenger configurations, Redux sync, or runtime behavior was changed.

This is a compile-time only change - TypeScript type aliases are erased at runtime. The actual controller initialization functions remain identical in behavior.

Risk factors:

  1. The change touches 100 files across ALL controllers (network, keyring, accounts, permissions, bridge, perps, predict, card, ramps, identity, etc.)
  2. If the rename was incomplete or introduced a type mismatch, it could cause TypeScript compilation errors that would prevent the app from building
  3. However, since TypeScript would catch any issues at build time (not runtime), E2E tests are not the right validation mechanism

Conservative approach: Running a minimal set of smoke tests covering core wallet functionality (accounts, confirmations, wallet platform) to verify the app initializes correctly and core flows work. This covers the most critical controller domains (KeyringController, AccountsController, NetworkController, TransactionController) that were renamed.

No performance tests are needed as this is a pure type rename with zero runtime impact on rendering, data loading, or app performance.

Performance Test Selection:
This is a pure TypeScript type rename refactoring (ControllerInitFunction → MessengerClientInitFunction) with no runtime behavior changes. Type aliases are erased at compile time and have zero impact on app performance, rendering, data loading, or any measurable performance metric. No performance tests are warranted.

View GitHub Actions results

@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown
Contributor

E2E Fixture Validation — Schema is up to date
16 value mismatches detected (expected — fixture represents an existing user).
View details

@cryptodev-2s cryptodev-2s enabled auto-merge April 10, 2026 08:21
Copy link
Copy Markdown
Contributor

@ccharly ccharly left a comment

Choose a reason for hiding this comment

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

LGTM for accounts

Copy link
Copy Markdown
Contributor

@ieow ieow left a comment

Choose a reason for hiding this comment

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

lgtm

@saustrie-consensys
Copy link
Copy Markdown
Contributor

Looks like a simple change. I would like to locally test this via my iOS simulator. What user flow journey would this affect?

@cryptodev-2s
Copy link
Copy Markdown
Contributor Author

Looks like a simple change. I would like to locally test this via my iOS simulator. What user flow journey would this affect?

These are purely variable renames. The flows should not be impacted in any case. The build would have failed if we were still using the old names in some files. I’d suggest running a simple test, but for this kind of change, where a variable used in almost all flows is being renamed, I would rely more on reviewing the code itself to catch anything that might still be using the old name

Copy link
Copy Markdown
Contributor

@saustrie-consensys saustrie-consensys left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Contributor

@NicolasMassart NicolasMassart left a comment

Choose a reason for hiding this comment

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

found 2 naming inconsistencies that could be fixed in this PR

Comment thread app/core/Engine/utils/test-utils.ts
Comment thread app/core/Engine/utils/utils.ts
@cryptodev-2s
Copy link
Copy Markdown
Contributor Author

found 2 naming inconsistencies that could be fixed in this PR

Here are the follow-up PRs that are currently waiting for this one to merge:

Could I address them in #28641 instead?

Otherwise, it’s going to be much harder to get 13 reviews again.

Copy link
Copy Markdown
Contributor

@NicolasMassart NicolasMassart left a comment

Choose a reason for hiding this comment

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

approved given the changes will be done in followup PRs

Copy link
Copy Markdown
Contributor

@caieu caieu left a comment

Choose a reason for hiding this comment

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

LGTM

@cryptodev-2s cryptodev-2s added this pull request to the merge queue Apr 10, 2026
Merged via the queue into main with commit 86b6748 Apr 10, 2026
103 checks passed
@cryptodev-2s cryptodev-2s deleted the rename-controller-types-to-messenger-client branch April 10, 2026 13:23
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 10, 2026
@metamaskbot metamaskbot added the release-7.74.0 Issue or pull request that will be included in release 7.74.0 label Apr 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.74.0 Issue or pull request that will be included in release 7.74.0 risk-low Low testing needed · Low bug introduction risk size-L team-core-platform Core Platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.