Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,10 @@ describe('useNetworkConnectionBanner', () => {
expect(typeof result.current.switchToInfura).toBe('function');
});

it('should call Engine.lookupEnabledNetworks on mount', () => {
it('should NOT call Engine.lookupEnabledNetworks on mount', () => {
renderHookWithProvider();

expect(mockEngine.lookupEnabledNetworks).toHaveBeenCalledTimes(1);
expect(mockEngine.lookupEnabledNetworks).not.toHaveBeenCalled();
});
});

Expand Down Expand Up @@ -903,6 +903,36 @@ describe('useNetworkConnectionBanner', () => {
});
});

it('should not show banner for networks with unknown status (stale persisted state)', () => {
// Networks with unknown status are treated the same as no data to avoid
// false-positive banners when the startup probe is removed and stale
// unknown entries remain from a previous session.
const unknownNetworkMetadata = {
[NETWORK_CLIENT_ID_1]: { status: NetworkStatus.Unknown },
[NETWORK_CLIENT_ID_89]: { status: NetworkStatus.Unknown },
};

// @ts-expect-error - Mocking Engine for testing
Engine.context = {
NetworkController: {
...mockNetworkController,
state: { networksMetadata: unknownNetworkMetadata },
},
};

jest.mocked(selectNetworkConnectionBannerState).mockReturnValue({
visible: false,
});

renderHookWithProvider();

act(() => {
jest.advanceTimersByTime(30000);
});

expect(store.getActions()).toHaveLength(0);
});

it('should not show banner for available networks', () => {
const availableNetworkMetadata = {
[NETWORK_CLIENT_ID_1]: { status: NetworkStatus.Available },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,6 @@ const useNetworkConnectionBanner = (): {

const evmEnabledNetworksChainIds = useSelector(selectEVMEnabledNetworks);

useEffect(() => {
Engine.lookupEnabledNetworks();
}, []);

function updateRpc(
rpcUrl: string,
status: NetworkConnectionBannerStatus,
Expand Down Expand Up @@ -152,7 +148,10 @@ const useNetworkConnectionBanner = (): {

totalEnabled += 1;

if (networkMetadata.status === NetworkStatus.Available) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Stale RPC status shows banner

Medium Severity

Removing the startup network probe stops refreshing persisted networksMetadata. The banner timer still treats non-Available statuses (e.g. Unavailable) as failures, while only Unknown is ignored. After a prior outage, users can see a connection banner on unlock even when RPC is healthy again.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit afba082. Configure here.

if (
networkMetadata.status === NetworkStatus.Available ||
networkMetadata.status === NetworkStatus.Unknown
) {
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import { MOCK_ANY_NAMESPACE, MockAnyNamespace } from '@metamask/messenger';
import { selectAssetsAccountApiBalancesEnabled } from '../../../selectors/featureFlagController/assetsAccountApiBalances';
import { selectBasicFunctionalityEnabled } from '../../../selectors/settings';
import { selectIsControllerDeprecated } from '../../../selectors/featureFlagController/assetsUnifyState';

jest.mock('@metamask/assets-controllers');

Expand All @@ -25,6 +26,14 @@
selectBasicFunctionalityEnabled: jest.fn().mockReturnValue(true),
}));

jest.mock('../../../selectors/featureFlagController/assetsUnifyState', () => ({
selectIsControllerDeprecated: jest.fn().mockReturnValue(() => false),
}));

jest.mock('../../../store', () => ({
store: { getState: jest.fn().mockReturnValue({}) },
}));

function getInitRequestMock(): jest.Mocked<
MessengerClientInitRequest<AccountTrackerControllerMessenger>
> {
Expand Down Expand Up @@ -68,6 +77,7 @@
accountsApiChainIds: expect.any(Function),
allowExternalServices: expect.any(Function),
isHomepageSectionsV1Enabled: expect.any(Function),
isDeprecated: expect.any(Function),
}),
);
});
Expand Down Expand Up @@ -113,4 +123,35 @@
expect(allowExternalServices()).toBe(false);
expect(selectBasicFunctionalityEnabled).toHaveBeenCalled();
});

describe('isDeprecated', () => {
it('returns false when AccountTrackerController is not deprecated', () => {
jest.mocked(selectIsControllerDeprecated).mockReturnValue(() => false);

Check failure on line 129 in app/core/Engine/controllers/account-tracker-controller-init.test.ts

View workflow job for this annotation

GitHub Actions / Run `lint:tsc`

Argument of type '() => false' is not assignable to parameter of type '((state: StateWithPartialEngine) => boolean) & { clearCache: () => void; resultsCount: () => number; resetResultsCount: () => void; } & { resultFunc: (resultFuncArgs_0: { [x: string]: Json; }) => boolean; ... 6 more ...; resetDependencyRecomputations: () => void; } & { ...; }'.

accountTrackerControllerInit(getInitRequestMock());

const controllerMock = jest.mocked(AccountTrackerController);
const { isDeprecated } = controllerMock.mock.calls[0][0] as {
isDeprecated: () => boolean;
};

expect(isDeprecated()).toBe(false);
expect(selectIsControllerDeprecated).toHaveBeenCalledWith(
'AccountTrackerController',
);
});

it('returns true when AccountTrackerController is deprecated', () => {
jest.mocked(selectIsControllerDeprecated).mockReturnValue(() => true);

Check failure on line 145 in app/core/Engine/controllers/account-tracker-controller-init.test.ts

View workflow job for this annotation

GitHub Actions / Run `lint:tsc`

Argument of type '() => true' is not assignable to parameter of type '((state: StateWithPartialEngine) => boolean) & { clearCache: () => void; resultsCount: () => number; resetResultsCount: () => void; } & { resultFunc: (resultFuncArgs_0: { [x: string]: Json; }) => boolean; ... 6 more ...; resetDependencyRecomputations: () => void; } & { ...; }'.

accountTrackerControllerInit(getInitRequestMock());

const controllerMock = jest.mocked(AccountTrackerController);
const { isDeprecated } = controllerMock.mock.calls[0][0] as {
isDeprecated: () => boolean;
};

expect(isDeprecated()).toBe(true);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import {
} from '@metamask/assets-controllers';
import { selectAssetsAccountApiBalancesEnabled } from '../../../selectors/featureFlagController/assetsAccountApiBalances';
import { selectBasicFunctionalityEnabled } from '../../../selectors/settings';
import { selectIsControllerDeprecated } from '../../../selectors/featureFlagController/assetsUnifyState';
import { store } from '../../../store';

/**
* Initialize the accountTracker controller.
*
Expand Down Expand Up @@ -34,6 +37,10 @@ export const accountTrackerControllerInit: MessengerClientInitFunction<
selectAssetsAccountApiBalancesEnabled(getState()) as `0x${string}`[],
allowExternalServices: () => selectBasicFunctionalityEnabled(getState()),
isHomepageSectionsV1Enabled: () => true,
isDeprecated: () =>
selectIsControllerDeprecated('AccountTrackerController')(
store.getState(),
),
});

return {
Expand Down
Loading