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.

Sign up for GitHub

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

feat: account sync - primary SRP filtering, bulk accounts creation and synced accounts limit removal #30317 #30529

Merged
merged 2 commits into from
Feb 24, 2025
+19 −17
Merged

feat: account sync - primary SRP filtering, bulk accounts creation and synced accounts limit removal #30317 #30529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e047229
feat: account sync - primary SRP filtering, bulk accounts creation an…
mathieuartu Feb 24, 2025
015c562
fix: yarn.lock
mathieuartu Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/scripts/metamask-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ import {
BridgeUserAction,
BridgeBackgroundAction,
} from '../../shared/types/bridge';
import { isProduction } from '../../shared/modules/environment';
import {
///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
handleMMITransactionUpdate,
Expand Down Expand Up @@ -1267,6 +1268,7 @@ export default class MetamaskController extends EventEmitter {
state: initState.UserStorageController,
config: {
accountSyncing: {
maxNumberOfAccountsToAdd: isProduction() ? undefined : 100,
Copy link
Contributor

@mirceanis mirceanis Feb 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This jumps out at me as something that we won't detect as a problem some months down the line because in tests it doesn't show up.
I suspect I half know the reason for this but I wonder if we are still bound by the same constraints here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mirceanis The QA team would know, believe me 😂 When we didn't have a limit, we (righfully) received a lot of complaints.
But yeah, jokes aside, there's no clear way of testing this because we don't expose the result of this config publicly from the controller.

But all in all, I'd say last week incident was in fact caught by @cmd-ob so I'm confident to include this line as well

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, but can we make it a bigger number, and something unique that would raise eyebrows, like 420 or 1337?

onAccountAdded: (profileId) => {
this.metaMetricsController.trackEvent({
category: MetaMetricsEventCategory.ProfileSyncing,
Expand Down Expand Up @@ -1312,7 +1314,7 @@ export default class MetamaskController extends EventEmitter {
name: 'UserStorageController',
allowedActions: [
'KeyringController:getState',
'KeyringController:addNewAccount',
'KeyringController:withKeyring',
'SnapController:handleRequest',
'AuthenticationController:getBearerToken',
'AuthenticationController:getSessionProfile',
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@
"@metamask/post-message-stream": "^9.0.0",
"@metamask/ppom-validator": "0.36.0",
"@metamask/preinstalled-example-snap": "^0.3.0",
"@metamask/profile-sync-controller": "^7.0.1",
"@metamask/profile-sync-controller": "^8.1.1",
"@metamask/providers": "^20.0.0",
"@metamask/queued-request-controller": "^7.0.1",
"@metamask/rate-limit-controller": "^6.0.3",
Expand Down
30 changes: 15 additions & 15 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5564,9 +5564,9 @@ __metadata:
languageName: node
linkType: hard

"@metamask/keyring-controller@npm:^19.0.7, @metamask/keyring-controller@npm:^19.2.0":
version: 19.2.0
resolution: "@metamask/keyring-controller@npm:19.2.0"
"@metamask/keyring-controller@npm:^19.2.0, @metamask/keyring-controller@npm:^19.2.1":
version: 19.2.1
resolution: "@metamask/keyring-controller@npm:19.2.1"
dependencies:
"@ethereumjs/util": "npm:^8.1.0"
"@keystonehq/metamask-airgapped-keyring": "npm:^0.14.1"
Expand All @@ -5575,18 +5575,18 @@ __metadata:
"@metamask/eth-hd-keyring": "npm:^10.0.0"
"@metamask/eth-sig-util": "npm:^8.2.0"
"@metamask/eth-simple-keyring": "npm:^8.1.0"
"@metamask/keyring-api": "npm:^17.0.0"
"@metamask/keyring-internal-api": "npm:^4.0.1"
"@metamask/keyring-api": "npm:^17.2.0"
"@metamask/keyring-internal-api": "npm:^4.0.3"
"@metamask/utils": "npm:^11.2.0"
async-mutex: "npm:^0.5.0"
ethereumjs-wallet: "npm:^1.0.1"
immer: "npm:^9.0.6"
ulid: "npm:^2.3.0"
checksum: 10/ba8e874ef37cf2dda112aa7265c01c21291d9143d72a2748fcf1e056a3734c4ca1712ffe8d026889bbcea5ccbdfe1bc8e71a40c5ffb1ef618e472d009135a5a7
checksum: 10/e9715a0743689c54843fd68f58ba1a577fd2f1a3ff104bd2e3d592c9246c56098f90288fab6c0d511e4a7115d2e9b835862885f37c3495aa2dd12db7df673326
languageName: node
linkType: hard

"@metamask/keyring-internal-api@npm:^4.0.1, @metamask/keyring-internal-api@npm:^4.0.3":
"@metamask/keyring-internal-api@npm:^4.0.3":
version: 4.0.3
resolution: "@metamask/keyring-internal-api@npm:4.0.3"
dependencies:
Expand Down Expand Up @@ -6032,13 +6032,13 @@ __metadata:
languageName: node
linkType: hard

"@metamask/profile-sync-controller@npm:^7.0.1":
version: 7.0.1
resolution: "@metamask/profile-sync-controller@npm:7.0.1"
"@metamask/profile-sync-controller@npm:^8.1.1":
version: 8.1.1
resolution: "@metamask/profile-sync-controller@npm:8.1.1"
dependencies:
"@metamask/base-controller": "npm:^8.0.0"
"@metamask/keyring-api": "npm:^17.0.0"
"@metamask/keyring-controller": "npm:^19.0.7"
"@metamask/keyring-api": "npm:^17.2.0"
"@metamask/keyring-controller": "npm:^19.2.1"
"@metamask/network-controller": "npm:^22.2.1"
"@metamask/snaps-sdk": "npm:^6.17.1"
"@metamask/snaps-utils": "npm:^8.10.0"
Expand All @@ -6048,13 +6048,13 @@ __metadata:
loglevel: "npm:^1.8.1"
siwe: "npm:^2.3.2"
peerDependencies:
"@metamask/accounts-controller": ^23.0.0
"@metamask/accounts-controller": ^24.0.0
"@metamask/keyring-controller": ^19.0.0
"@metamask/network-controller": ^22.0.0
"@metamask/providers": ^18.1.0
"@metamask/snaps-controllers": ^9.19.0
webextension-polyfill: ^0.10.0 || ^0.11.0 || ^0.12.0
checksum: 10/752a3ccf9ccc498ca05ba0272dbf7995a28a005749c7dd33b4b927d23f415150ea505fe76883311a38121df196f40dc771df63e3ba2239e0b3108e98a1836fdf
checksum: 10/4b18a546c0d78f010fbf1cf16644001ba0139c60941ceb70b83c7850e2fafcd5d17b8148f8020dcfe0e8c3f897259480077fa6b4402f4d270ed68e248d628369
languageName: node
linkType: hard

Expand Down Expand Up @@ -26988,7 +26988,7 @@ __metadata:
"@metamask/ppom-validator": "npm:0.36.0"
"@metamask/preferences-controller": "npm:^15.0.2"
"@metamask/preinstalled-example-snap": "npm:^0.3.0"
"@metamask/profile-sync-controller": "npm:^7.0.1"
"@metamask/profile-sync-controller": "npm:^8.1.1"
"@metamask/providers": "npm:^20.0.0"
"@metamask/queued-request-controller": "npm:^7.0.1"
"@metamask/rate-limit-controller": "npm:^6.0.3"
Expand Down
Loading