Skip to content

Commit d53e126

Browse files
authored
fix: allow disable last remaining network on namespace (#6499)
## 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 <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent b78915b commit d53e126

File tree

3 files changed

+8
-55
lines changed

3 files changed

+8
-55
lines changed

packages/network-enablement-controller/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515

1616
### Changed
1717

18+
- **BREAKING:** Allow disabling the last remaining network in a namespace to align with BIP-44, where account groups shouldn't be forced to always keep at least one active network ([#6499](https://github.com/MetaMask/core/pull/6499))
1819
- Bump `@metamask/base-controller` from `^8.2.0` to `^8.3.0` ([#6465](https://github.com/MetaMask/core/pull/6465))
1920

2021
## [0.4.0]

packages/network-enablement-controller/src/NetworkEnablementController.test.ts

Lines changed: 7 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -198,34 +198,6 @@ describe('NetworkEnablementController', () => {
198198
});
199199
});
200200

201-
it('subscribes to TransactionController:transactionSubmitted and enables network', async () => {
202-
const { controller, messenger } = setupInitializedController();
203-
204-
// Initially disable Polygon network (it should not exist)
205-
expect(controller.isNetworkEnabled('0x89')).toBe(false);
206-
207-
// Publish a transaction submitted event with Polygon chainId
208-
messenger.publish('TransactionController:transactionSubmitted', {
209-
transactionMeta: {
210-
chainId: '0x89', // Polygon
211-
networkClientId: 'polygon-network',
212-
id: 'test-tx-id',
213-
status: TransactionStatus.submitted,
214-
time: Date.now(),
215-
txParams: {
216-
from: '0x123',
217-
to: '0x456',
218-
value: '0x0',
219-
},
220-
} as TransactionMeta, // Simplified structure for testing
221-
});
222-
223-
await advanceTime({ clock, duration: 1 });
224-
225-
// The Polygon network should now be enabled
226-
expect(controller.isNetworkEnabled('0x89')).toBe(true);
227-
});
228-
229201
it('handles TransactionController:transactionSubmitted with missing chainId gracefully', async () => {
230202
const { controller, messenger } = setupInitializedController();
231203

@@ -1136,16 +1108,16 @@ describe('NetworkEnablementController', () => {
11361108
});
11371109
});
11381110

1139-
it('does not disable a Solana network using CAIP chain ID as it is the only enabled network on the namespace', () => {
1111+
it('does disable a Solana network using CAIP chain ID as it is the only enabled network on the namespace', () => {
11401112
const { controller } = setupController();
11411113

11421114
// Try to disable a Solana network using CAIP chain ID
11431115
expect(() =>
11441116
controller.disableNetwork('solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'),
1145-
).toThrow('Cannot disable the last remaining enabled network');
1117+
).not.toThrow();
11461118
});
11471119

1148-
it('prevents disabling the last active network for an EVM namespace', () => {
1120+
it('disables the last active network for an EVM namespace', () => {
11491121
const { controller } = setupInitializedController();
11501122

11511123
// disable all networks except one
@@ -1169,9 +1141,7 @@ describe('NetworkEnablementController', () => {
11691141
});
11701142

11711143
// Try to disable the last active network
1172-
expect(() => controller.disableNetwork('0x1')).toThrow(
1173-
'Cannot disable the last remaining enabled network',
1174-
);
1144+
expect(() => controller.disableNetwork('0x1')).not.toThrow();
11751145
});
11761146

11771147
it('handles disabling non-existent network gracefully', () => {
@@ -1424,13 +1394,11 @@ describe('NetworkEnablementController', () => {
14241394
expect(controller.isNetworkEnabled(BtcScope.Testnet)).toBe(false);
14251395
});
14261396

1427-
it('prevents disabling the last Bitcoin network', () => {
1397+
it('allows disabling the last Bitcoin network', () => {
14281398
const { controller } = setupController();
14291399

14301400
// Only Bitcoin mainnet is enabled by default in the BIP122 namespace
1431-
expect(() => controller.disableNetwork(BtcScope.Mainnet)).toThrow(
1432-
'Cannot disable the last remaining enabled network',
1433-
);
1401+
expect(() => controller.disableNetwork(BtcScope.Mainnet)).not.toThrow();
14341402
});
14351403

14361404
it('allows disabling Bitcoin mainnet when testnet is enabled', () => {
@@ -1490,7 +1458,7 @@ describe('NetworkEnablementController', () => {
14901458
// Disable Solana network - this should fail as it's the only one in its namespace
14911459
expect(() =>
14921460
controller.disableNetwork('solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'),
1493-
).toThrow('Cannot disable the last remaining enabled network');
1461+
).not.toThrow();
14941462

14951463
// Bitcoin should still be enabled
14961464
expect(controller.isNetworkEnabled(BtcScope.Mainnet)).toBe(true);

packages/network-enablement-controller/src/NetworkEnablementController.ts

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -177,18 +177,6 @@ export class NetworkEnablementController extends BaseController<
177177
messenger.subscribe('NetworkController:networkRemoved', ({ chainId }) => {
178178
this.#removeNetworkEntry(chainId);
179179
});
180-
181-
// Listen for confirmed staking transactions
182-
messenger.subscribe(
183-
'TransactionController:transactionSubmitted',
184-
(transactionMeta) => {
185-
if (transactionMeta?.transactionMeta?.chainId) {
186-
this.enableNetwork(
187-
transactionMeta.transactionMeta.chainId as Hex | CaipChainId,
188-
);
189-
}
190-
},
191-
);
192180
}
193181

194182
/**
@@ -385,10 +373,6 @@ export class NetworkEnablementController extends BaseController<
385373
const derivedKeys = deriveKeys(chainId);
386374
const { namespace, storageKey } = derivedKeys;
387375

388-
if (isOnlyNetworkEnabledInNamespace(this.state, derivedKeys)) {
389-
throw new Error('Cannot disable the last remaining enabled network');
390-
}
391-
392376
this.update((s) => {
393377
s.enabledNetworkMap[namespace][storageKey] = false;
394378
});

0 commit comments

Comments
 (0)