Skip to content

Commit 344f947

Browse files
jiexiffmcgee725adonesky1
authored
fix: fix MM Connect account switching via homepage account selector (#22163)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Fixes a bug with account switching for MM Connect where changing the account selector value from the homepage did not result in the dapp receiving an accountsChanged event. This was because when the app cold started, there was no BackgroundBridge instantiated for any MMC connections unless there were pending messages from the relay (for performance reasons), and because the homepage account selector was no longer updating the PreferencesController state because the PreferencesController.selectedAddress is deprecated. To fix this, BackgroundBridge is now always instantiated on app start up for all MMC connections. This is suboptimal, but can be rectified later with TTL enforcement. BackgroundBridge now reads AccountsController for selected account state. This is also technically deprecated state, but it's out of the scope of what we are trying to achieve to fully migrate the BackgroundBridge to read the selected account group instead. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null No changelog necessary as MM Connect has not been publicly released to users yet. ## **Related issues** Fixes: ## **Manual testing steps** 1. Using the [EVM MMC test dapp](MetaMask/connect-monorepo#21) in the Native Browser 2. Connect 2 accounts 3. See which account is listed first in the test dapp 4. Go back to the wallet and change the account in the main wallet view to the other permitted account 5. Return to the native browser 6. See that the newly selected account is listed first in the test dapp ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Ensure MMC/remote account changes emit by eagerly initializing the RPC bridge and reading selected account from AccountsController instead of PreferencesController. > > - **BackgroundBridge**: > - Subscribe to `AccountsController:selectedAccountChange` and unsubscribe on disconnect. > - Replace `PreferencesController.selectedAddress` with `AccountsController.getSelectedAccount().address` for `selectedAddress` in `getState` and change detection in `onStateUpdate`. > - For WalletConnect/remote, emit `accountsChanged` when the AccountsController-selected address differs; update `addressSent` accordingly. > - **SDKConnectV2**: > - `rpc-bridge-adapter`: call `ensureInitialized()` in constructor to instantiate `BackgroundBridge` on startup, enabling timely event delivery. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bbd4de4. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: ffmcgee <joao.carlos@consensys.net> Co-authored-by: Alex Donesky <adonesky@gmail.com>
1 parent d42d736 commit 344f947

File tree

2 files changed

+21
-8
lines changed

2 files changed

+21
-8
lines changed

app/core/BackgroundBridge/BackgroundBridge.js

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,11 @@ export class BackgroundBridge extends EventEmitter {
187187
this.sendStateUpdate,
188188
);
189189

190+
Engine.controllerMessenger.subscribe(
191+
'AccountsController:selectedAccountChange',
192+
this.sendStateUpdate,
193+
);
194+
190195
Engine.controllerMessenger.subscribe(
191196
'PreferencesController:stateChange',
192197
this.sendStateUpdate,
@@ -423,13 +428,16 @@ export class BackgroundBridge extends EventEmitter {
423428
}
424429
// ONLY NEEDED FOR WC FOR NOW, THE BROWSER HANDLES THIS NOTIFICATION BY ITSELF
425430
if (this.isWalletConnect || this.isRemoteConn) {
431+
const accountControllerSelectedAddress = toFormattedAddress(
432+
Engine.context.AccountsController.getSelectedAccount().address,
433+
);
426434
if (
427435
this.addressSent != null &&
428-
memState.selectedAddress != null &&
429-
!areAddressesEqual(this.addressSent, memState.selectedAddress)
436+
accountControllerSelectedAddress != null &&
437+
!areAddressesEqual(this.addressSent, accountControllerSelectedAddress)
430438
) {
431-
this.addressSent = memState.selectedAddress;
432-
this.notifySelectedAddressChanged(memState.selectedAddress);
439+
this.addressSent = accountControllerSelectedAddress;
440+
this.notifySelectedAddressChanged(accountControllerSelectedAddress);
433441
}
434442
}
435443
}
@@ -480,6 +488,10 @@ export class BackgroundBridge extends EventEmitter {
480488
'PreferencesController:stateChange',
481489
this.sendStateUpdate,
482490
);
491+
controllerMessenger.tryUnsubscribe(
492+
'AccountsController:selectedAccountChange',
493+
this.sendStateUpdate,
494+
);
483495

484496
// Enable multichain functionality for all connections except for WalletConnect and MMSDK v1.
485497
if (!(this.isMMSDK && this.sdkVersion === 'v1') && !this.isWalletConnect) {
@@ -1133,14 +1145,14 @@ export class BackgroundBridge extends EventEmitter {
11331145
*/
11341146
getState() {
11351147
const vault = Engine.context.KeyringController.state.vault;
1136-
const {
1137-
PreferencesController: { selectedAddress },
1138-
} = Engine.datamodel.state;
1148+
const accountControllerSelectedAddress = toFormattedAddress(
1149+
Engine.context.AccountsController.getSelectedAccount().address,
1150+
);
11391151
return {
11401152
isInitialized: !!vault,
11411153
isUnlocked: true,
11421154
network: legacyNetworkId(),
1143-
selectedAddress,
1155+
selectedAddress: accountControllerSelectedAddress,
11441156
};
11451157
}
11461158

app/core/SDKConnectV2/adapters/rpc-bridge-adapter.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export class RPCBridgeAdapter
2525
super();
2626
this.connInfo = connInfo;
2727
this.processQueue = this.processQueue.bind(this);
28+
this.ensureInitialized();
2829
}
2930

3031
/**

0 commit comments

Comments
 (0)