Skip to content

Conversation

@mcmire
Copy link
Contributor

@mcmire mcmire commented Nov 3, 2025

Explanation

In debugging a long-standing issue where SelectedNetworkController sometimes requests invalid network client IDs from NetworkController, one bug was discovered in NetworkController. Namely, when a network is removed, its data is removed from networkConfigurationsByChainId, but not networksMetadata. This commit fixes removeNetwork to do this, and ensures that when the controller is initialized, if networksMetadata contains invalid network client IDs, they are removed.

References

Fixes https://consensyssoftware.atlassian.net/browse/WPC-151.

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, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

Note

Cleans networksMetadata of invalid network client IDs on initialization and when removing networks, with tests and changelog updates.

  • Network Controller:
    • On init, correctInitialState removes invalid network client IDs from networksMetadata and logs via ErrorReportingService:captureException.
    • removeNetwork now deletes networksMetadata for all RPC endpoints in the removed configuration.
  • Tests:
    • Add/adjust tests to verify correction of selectedNetworkClientId, metadata cleanup, and error logging in tests/NetworkController.test.ts.
    • Update test helpers to pre-register and delegate ErrorReportingService:captureException and expose handler injection (tests/helpers.ts).
    • Update GasFeeController.test.ts to register and delegate ErrorReportingService:captureException in messenger setup.
  • Docs:
    • Update packages/network-controller/CHANGELOG.md under Unreleased → Fixed to document metadata cleanup behavior.

Written by Cursor Bugbot for commit deeb2e2. This will update automatically on new commits. Configure here.

@mcmire mcmire force-pushed the correct-invalid-network-metadata branch from befa4e1 to 6933895 Compare November 3, 2025 18:56
@mcmire mcmire changed the title Ensure networksMetadata always reflects available networks Ensure networksMetadata never references old network client IDs Nov 3, 2025
@mcmire mcmire force-pushed the correct-invalid-network-metadata branch from 6933895 to e5a0e22 Compare November 3, 2025 18:57
In debugging a long-standing issue where SelectedNetworkController
sometimes requests invalid network client IDs from NetworkController,
one bug was discovered in NetworkController. Namely, when a network is
removed, its data is removed from `networkConfigurationsByChainId`, but
not `networksMetadata`. This commit fixes `removeNetwork` to do this,
and ensures that when the controller is initialized, if
`networksMetadata` contains invalid network client IDs, they are
removed.
@mcmire mcmire force-pushed the correct-invalid-network-metadata branch from e5a0e22 to d80de28 Compare November 3, 2025 19:33
@mcmire mcmire marked this pull request as ready for review November 3, 2025 20:36
@mcmire mcmire requested review from a team as code owners November 3, 2025 20:36
cursor[bot]

This comment was marked as outdated.


const getRootMessenger = (): RootMessenger => {
return new Messenger({ namespace: MOCK_ANY_NAMESPACE });
const rootMessenger = new Messenger<
Copy link
Contributor Author

@mcmire mcmire Nov 3, 2025

Choose a reason for hiding this comment

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

Since the tests for GasFeeController instantiate a real NetworkController, we also need to make sure that we register ErrorReportingService:captureException, as NetworkController may call it during initialization if the initial state is incoherent. These changes should have been here all along, but they have not been needed so far. The reason they are needed now is that in some tests, the initial NetworkController state sets networksMetadata for networks that aren't present in networkConfigurationsByChainId. We could make the initial state more accurate, but it doesn't really matter here, we just need to register ErrorReportingService:captureException with a dummy handler.

btoa,
}),
});
it('corrects an invalid selectedNetworkClientId to the default RPC endpoint of the first chain, logging this fact', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two tests were combined into one for simplicity.

}),
'0x2': buildCustomNetworkConfiguration({ chainId: '0x2' }),
'0x3': buildCustomNetworkConfiguration({ chainId: '0x3' }),
it('removes invalid network client IDs from networksMetadata, logging this fact', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new test.

},
networksMetadata: {
mainnet: {
[TESTNET.networkType]: {
Copy link
Contributor Author

@mcmire mcmire Nov 3, 2025

Choose a reason for hiding this comment

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

Some of the NetworkController tests used initial state which is now invalid, so they were corrected.

@mcmire mcmire enabled auto-merge (squash) November 4, 2025 15:01
@mcmire mcmire disabled auto-merge November 4, 2025 15:01
@mcmire mcmire enabled auto-merge (squash) November 4, 2025 15:04
@mcmire mcmire merged commit 80b9a0f into main Nov 4, 2025
266 of 267 checks passed
@mcmire mcmire deleted the correct-invalid-network-metadata branch November 4, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants