-
-
Notifications
You must be signed in to change notification settings - Fork 255
fix: allow disable last remaining network on namespace #6499
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.
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
Conversation
<!-- 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** BIP-44: introduce the unified network selector scoped to an account group (EVM + Solana, BTC later) with support for popular and custom networks, CAIP-19 display, and parity with existing feature flags. **Why** • BIP-44 groups multiple accounts under one “account group.” Users need a single selector that reflects all enabled networks for that group instead of EVM-only. core PR: MetaMask/core#6499 <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **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: Added a unified network selector for BIP-44 account groups, showing EVM and Solana networks in one place and supporting custom networks. ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: BIP-44 network selector Background: Given I have an account group with at least one EVM account and one Solana account enabled And popular networks are enabled in settings And I am on the wallet home screen ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** https://github.com/user-attachments/assets/27b6ef8f-0cd3-49df-8d89-a139b9474a63 <!-- [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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Network Enablement Feature Removed
The NetworkEnablementController no longer subscribes to TransactionController:transactionSubmitted events. This removes the automatic network enablement feature when users submit transactions. Additionally, several existing tests that publish these events are now ineffective and misleading, as the controller no longer processes them.
packages/network-enablement-controller/src/NetworkEnablementController.test.ts#L200-L300
core/packages/network-enablement-controller/src/NetworkEnablementController.test.ts
Lines 200 to 300 in c93bc57
| it('handles TransactionController:transactionSubmitted with missing chainId gracefully', async () => { | |
| const { controller, messenger } = setupInitializedController(); | |
| const initialState = { ...controller.state }; | |
| // Publish a transaction submitted event without chainId | |
| messenger.publish('TransactionController:transactionSubmitted', { | |
| transactionMeta: { | |
| networkClientId: 'test-network', | |
| id: 'test-tx-id', | |
| status: TransactionStatus.submitted, | |
| time: Date.now(), | |
| txParams: { | |
| from: '0x123', | |
| to: '0x456', | |
| value: '0x0', | |
| }, | |
| // chainId is missing | |
| } as TransactionMeta, // Simplified structure for testing | |
| }); | |
| await advanceTime({ clock, duration: 1 }); | |
| // State should remain unchanged | |
| expect(controller.state).toStrictEqual(initialState); | |
| }); | |
| it('handles TransactionController:transactionSubmitted with malformed structure gracefully', async () => { | |
| const { controller, messenger } = setupInitializedController(); | |
| const initialState = { ...controller.state }; | |
| // Publish a transaction submitted event with malformed structure | |
| // @ts-expect-error - Testing runtime safety for malformed payload | |
| messenger.publish('TransactionController:transactionSubmitted', { | |
| // Missing transactionMeta entirely | |
| }); | |
| await advanceTime({ clock, duration: 1 }); | |
| // State should remain unchanged | |
| expect(controller.state).toStrictEqual(initialState); | |
| }); | |
| it('handles TransactionController:transactionSubmitted with null/undefined transactionMeta gracefully', async () => { | |
| const { controller, messenger } = setupInitializedController(); | |
| const initialState = { ...controller.state }; | |
| // Test with null transactionMeta | |
| messenger.publish('TransactionController:transactionSubmitted', { | |
| // @ts-expect-error - Testing runtime safety for null transactionMeta | |
| transactionMeta: null, | |
| }); | |
| await advanceTime({ clock, duration: 1 }); | |
| // State should remain unchanged | |
| expect(controller.state).toStrictEqual(initialState); | |
| // Test with undefined transactionMeta | |
| messenger.publish('TransactionController:transactionSubmitted', { | |
| // @ts-expect-error - Testing runtime safety for undefined transactionMeta | |
| transactionMeta: undefined, | |
| }); | |
| await advanceTime({ clock, duration: 1 }); | |
| // State should still remain unchanged | |
| expect(controller.state).toStrictEqual(initialState); | |
| }); | |
| it('does fallback to ethereum when removing the last enabled network', async () => { | |
| const { controller, messenger } = setupInitializedController(); | |
| // disable all networks except linea | |
| controller.disableNetwork('0x1'); // Ethereum Mainnet | |
| controller.disableNetwork('0x2105'); // Base Mainnet | |
| // Publish an update with linea network removed | |
| messenger.publish('NetworkController:networkRemoved', { | |
| chainId: '0xe708', // Linea Mainnet | |
| blockExplorerUrls: [], | |
| defaultRpcEndpointIndex: 0, | |
| name: 'Linea', | |
| nativeCurrency: 'ETH', | |
| rpcEndpoints: [ | |
| { | |
| url: 'https://linea-mainnet.infura.io/v3/1234567890', | |
| networkClientId: 'id', | |
| type: RpcEndpointType.Custom, | |
| }, | |
| ], | |
| }); | |
| await advanceTime({ clock, duration: 1 }); | |
| expect(controller.state).toStrictEqual({ | |
| enabledNetworkMap: { | |
| [KnownCaipNamespace.Eip155]: { |
packages/network-enablement-controller/src/NetworkEnablementController.ts#L179-L192
core/packages/network-enablement-controller/src/NetworkEnablementController.ts
Lines 179 to 192 in c93bc57
| }); | |
| } | |
| /** | |
| * Enables or disables a network for the user. | |
| * | |
| * This method accepts either a Hex chain ID (for EVM networks) or a CAIP-2 chain ID | |
| * (for any blockchain network). The method will automatically convert Hex chain IDs | |
| * to CAIP-2 format internally. This dual parameter support allows for backward | |
| * compatibility with existing EVM chain ID formats while supporting newer | |
| * multi-chain standards. | |
| * | |
| * When enabling a non-popular network, this method will disable all other networks | |
| * to ensure only one network is active at a time (exclusive mode). |
Explanation
This change allows the last remaining network in a namespace to be disabled. The reason is to align with BIP-44, where account groups shouldn’t be forced to always keep at least one active network
References
Checklist