Skip to content

Commit 346acb2

Browse files
Prithpal-SooriyaPrithpal-Sooriya
committed
chore(runway): cherry-pick fix: (cp-12.17.1) patch notification services controller (#32391)
## **Description** Wow, huge oopsy on my side. Urgh, a little annoyed that I didn't spot this earlier. After doing some thorough debugging, I found an infinite state update loop from the NotificationServicesController. I didn't find the full context but this controller: - Listens to `KeyringController:stateChange` events - When receiving an event we perform a `KeyringController:withKeyring` action. Somewhere above (or potentially another controller firing events or actions?) will then fire another `KeyringController:stateChange` and the loop repeats. This prevents other state updates and redux updates. Controller fix: MetaMask/core#5731 However this patch can be an interim for existing releases or until the CORE package is released. <details><summary>Screenshot of background logs</summary> ![Screenshot 2025-04-29 at 22 43 03](https://github.com/user-attachments/assets/a1f07672-d471-4e60-8124-e6644df0b5f4) </details> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/32391?quickstart=1) ## **Related issues** Fixes: Potentially fixes #32416 #31422 ## **Manual testing steps** 1. Turn on notifications 2. Try dismissing the carousel banners 3. Try switching networks 4. Try the send flow where you send sepolia ## **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** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/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.
1 parent 22a89b5 commit 346acb2

File tree

3 files changed

+78
-3
lines changed

3 files changed

+78
-3
lines changed

‎.yarn/patches/@metamask-notification-services-controller-npm-6.0.0-d0cefeaee1.patch

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
diff --git a/dist/NotificationServicesController/NotificationServicesController.cjs b/dist/NotificationServicesController/NotificationServicesController.cjs
2+
index 2c0e69468c3bab825ca6d9e6d78d9fb3b4372081..2f6214553fc6dcfecaa8a614e05980dfd8118e10 100644
3+
--- a/dist/NotificationServicesController/NotificationServicesController.cjs
4+
+++ b/dist/NotificationServicesController/NotificationServicesController.cjs
5+
@@ -307,8 +307,11 @@ class NotificationServicesController extends base_controller_1.BaseController {
6+
* And call effects to subscribe/unsubscribe to notifications.
7+
*/
8+
subscribe: () => {
9+
- this.messagingSystem.subscribe('KeyringController:stateChange', async () => {
10+
- if (!this.state.isNotificationServicesEnabled) {
11+
+ this.messagingSystem.subscribe('KeyringController:stateChange', async (totalAccounts, prevTotalAccounts) => {
12+
+ // Fixed in a release that contains https://github.com/MetaMask/core/pull/5731
13+
+ const hasTotalAccountsChanged = totalAccounts !== prevTotalAccounts;
14+
+ if (!this.state.isNotificationServicesEnabled ||
15+
+ !hasTotalAccountsChanged) {
16+
return;
17+
}
18+
const { accountsAdded, accountsRemoved } = await __classPrivateFieldGet(this, _NotificationServicesController_accounts, "f").listAccounts();
19+
@@ -320,6 +323,9 @@ class NotificationServicesController extends base_controller_1.BaseController {
20+
promises.push(this.deleteOnChainTriggersByAccount(accountsRemoved));
21+
}
22+
await Promise.all(promises);
23+
+ }, (state) => {
24+
+ return (state?.keyrings?.flatMap?.((keyring) => keyring.accounts)?.length ??
25+
+ 0);
26+
});
27+
},
28+
});
29+
diff --git a/dist/NotificationServicesController/NotificationServicesController.mjs b/dist/NotificationServicesController/NotificationServicesController.mjs
30+
index 1eeede947b3a8875589a81c140f001f358d8bcc1..9a1c68b32169e2e0a7574e4cc270e63d6e07f326 100644
31+
--- a/dist/NotificationServicesController/NotificationServicesController.mjs
32+
+++ b/dist/NotificationServicesController/NotificationServicesController.mjs
33+
@@ -285,8 +285,11 @@ class NotificationServicesController extends BaseController {
34+
* And call effects to subscribe/unsubscribe to notifications.
35+
*/
36+
subscribe: () => {
37+
- this.messagingSystem.subscribe('KeyringController:stateChange', async () => {
38+
- if (!this.state.isNotificationServicesEnabled) {
39+
+ this.messagingSystem.subscribe('KeyringController:stateChange', async (totalAccounts, prevTotalAccounts) => {
40+
+ // Fixed in a release that contains https://github.com/MetaMask/core/pull/5731
41+
+ const hasTotalAccountsChanged = totalAccounts !== prevTotalAccounts;
42+
+ if (!this.state.isNotificationServicesEnabled ||
43+
+ !hasTotalAccountsChanged) {
44+
return;
45+
}
46+
const { accountsAdded, accountsRemoved } = await __classPrivateFieldGet(this, _NotificationServicesController_accounts, "f").listAccounts();
47+
@@ -298,6 +301,9 @@ class NotificationServicesController extends BaseController {
48+
promises.push(this.deleteOnChainTriggersByAccount(accountsRemoved));
49+
}
50+
await Promise.all(promises);
51+
+ }, (state) => {
52+
+ return (state?.keyrings?.flatMap?.((keyring) => keyring.accounts)?.length ??
53+
+ 0);
54+
});
55+
},
56+
});

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@
303303
"@metamask/multichain-transactions-controller": "^0.9.0",
304304
"@metamask/name-controller": "^8.0.3",
305305
"@metamask/network-controller": "patch:@metamask/network-controller@npm%3A23.2.0#~/.yarn/patches/@metamask-network-controller-npm-23.2.0-0223efb98a.patch",
306-
"@metamask/notification-services-controller": "^6.0.0",
306+
"@metamask/notification-services-controller": "patch:@metamask/notification-services-controller@npm%3A6.0.0#~/.yarn/patches/@metamask-notification-services-controller-npm-6.0.0-d0cefeaee1.patch",
307307
"@metamask/object-multiplex": "^2.0.0",
308308
"@metamask/obs-store": "^9.0.0",
309309
"@metamask/permission-controller": "^11.0.6",

‎yarn.lock

+21-2
Original file line numberDiff line numberDiff line change
@@ -6039,7 +6039,7 @@ __metadata:
60396039
languageName: node
60406040
linkType: hard
60416041

6042-
"@metamask/notification-services-controller@npm:^6.0.0":
6042+
"@metamask/notification-services-controller@npm:6.0.0":
60436043
version: 6.0.0
60446044
resolution: "@metamask/notification-services-controller@npm:6.0.0"
60456045
dependencies:
@@ -6058,6 +6058,25 @@ __metadata:
60586058
languageName: node
60596059
linkType: hard
60606060

6061+
"@metamask/notification-services-controller@patch:@metamask/notification-services-controller@npm%3A6.0.0#~/.yarn/patches/@metamask-notification-services-controller-npm-6.0.0-d0cefeaee1.patch":
6062+
version: 6.0.0
6063+
resolution: "@metamask/notification-services-controller@patch:@metamask/notification-services-controller@npm%3A6.0.0#~/.yarn/patches/@metamask-notification-services-controller-npm-6.0.0-d0cefeaee1.patch::version=6.0.0&hash=7b1708"
6064+
dependencies:
6065+
"@contentful/rich-text-html-renderer": "npm:^16.5.2"
6066+
"@metamask/base-controller": "npm:^8.0.0"
6067+
"@metamask/controller-utils": "npm:^11.7.0"
6068+
"@metamask/utils": "npm:^11.2.0"
6069+
bignumber.js: "npm:^9.1.2"
6070+
firebase: "npm:^11.2.0"
6071+
loglevel: "npm:^1.8.1"
6072+
uuid: "npm:^8.3.2"
6073+
peerDependencies:
6074+
"@metamask/keyring-controller": ^21.0.0
6075+
"@metamask/profile-sync-controller": ^12.0.0
6076+
checksum: 10/f5cd1036edda31c33b95213e24d7a55d8c1995dfe47020708efff6f73e6b7a1a3a5619c56339a2e96e25d1ac4f63371240e4a700583c06d6b9900e3600f77af5
6077+
languageName: node
6078+
linkType: hard
6079+
60616080
"@metamask/number-to-bn@npm:^1.7.1":
60626081
version: 1.7.1
60636082
resolution: "@metamask/number-to-bn@npm:1.7.1"
@@ -30037,7 +30056,7 @@ __metadata:
3003730056
"@metamask/multichain-transactions-controller": "npm:^0.9.0"
3003830057
"@metamask/name-controller": "npm:^8.0.3"
3003930058
"@metamask/network-controller": "patch:@metamask/network-controller@npm%3A23.2.0#~/.yarn/patches/@metamask-network-controller-npm-23.2.0-0223efb98a.patch"
30040-
"@metamask/notification-services-controller": "npm:^6.0.0"
30059+
"@metamask/notification-services-controller": "patch:@metamask/notification-services-controller@npm%3A6.0.0#~/.yarn/patches/@metamask-notification-services-controller-npm-6.0.0-d0cefeaee1.patch"
3004130060
"@metamask/object-multiplex": "npm:^2.0.0"
3004230061
"@metamask/obs-store": "npm:^9.0.0"
3004330062
"@metamask/permission-controller": "npm:^11.0.6"

0 commit comments

Comments
 (0)