Skip to content

Commit dc2b77e

Browse files
Kriys94Kriys94
committed
feat(core-backend): clean code
1 parent aa6f1aa commit dc2b77e

File tree

9 files changed

+135
-160
lines changed

9 files changed

+135
-160
lines changed

‎packages/assets-controllers/CHANGELOG.md‎

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

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Fix address casing in WebSocket-based token balance updates to ensure consistency ([#6819](https://github.com/MetaMask/core/pull/6819))
13+
1014
## [80.0.0]
1115

1216
### Added

‎packages/assets-controllers/src/TokenBalancesController.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -844,12 +844,10 @@ export class TokenBalancesController extends StaticIntervalPollingController<{
844844
account: ChecksumAddress,
845845
chainId: ChainIdHex,
846846
): boolean {
847-
const normalizedAddress = tokenAddress.toLowerCase();
848-
849847
// Check if token exists in allTokens
850848
if (
851849
this.#allTokens?.[chainId]?.[account.toLowerCase()]?.some(
852-
(token) => token.address.toLowerCase() === normalizedAddress,
850+
(token) => token.address === tokenAddress,
853851
)
854852
) {
855853
return true;
@@ -858,7 +856,7 @@ export class TokenBalancesController extends StaticIntervalPollingController<{
858856
// Check if token exists in allIgnoredTokens
859857
if (
860858
this.#allIgnoredTokens?.[chainId]?.[account.toLowerCase()]?.some(
861-
(addr) => addr.toLowerCase() === normalizedAddress,
859+
(token) => token === tokenAddress,
862860
)
863861
) {
864862
return true;
@@ -1124,23 +1122,26 @@ export class TokenBalancesController extends StaticIntervalPollingController<{
11241122
updates: BalanceUpdate[];
11251123
}) => {
11261124
const chainId = caipChainIdToHex(chain);
1127-
const account = checksum(address);
1125+
const checksummedAccount = checksum(address);
11281126

11291127
try {
11301128
// Process all balance updates at once
11311129
const { tokenBalances, newTokens, nativeBalanceUpdates } =
1132-
this.#prepareBalanceUpdates(updates, account, chainId);
1130+
this.#prepareBalanceUpdates(updates, checksummedAccount, chainId);
11331131

11341132
// Update state once with all token balances
11351133
if (tokenBalances.length > 0) {
11361134
this.update((state) => {
1137-
// Initialize account and chain structure
1138-
state.tokenBalances[account] ??= {};
1139-
state.tokenBalances[account][chainId] ??= {};
1135+
// Temporary until ADR to normalize all keys - tokenBalances state requires: account in lowercase, token in checksum
1136+
const lowercaseAccount =
1137+
checksummedAccount.toLowerCase() as ChecksumAddress;
1138+
state.tokenBalances[lowercaseAccount] ??= {};
1139+
state.tokenBalances[lowercaseAccount][chainId] ??= {};
11401140

11411141
// Apply all token balance updates
11421142
for (const { tokenAddress, balance } of tokenBalances) {
1143-
state.tokenBalances[account][chainId][tokenAddress] = balance;
1143+
state.tokenBalances[lowercaseAccount][chainId][tokenAddress] =
1144+
balance;
11441145
}
11451146
});
11461147
}

‎packages/assets-controllers/src/TokenDetectionController.test.ts‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3535,6 +3535,8 @@ describe('TokenDetectionController', () => {
35353535
describe('addDetectedTokensViaWs', () => {
35363536
it('should add tokens detected from websocket with metadata from cache', async () => {
35373537
const mockTokenAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
3538+
const checksummedTokenAddress =
3539+
'0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
35383540
const chainId = '0x1';
35393541

35403542
await withController(
@@ -3581,7 +3583,7 @@ describe('TokenDetectionController', () => {
35813583
'TokensController:addTokens',
35823584
[
35833585
{
3584-
address: mockTokenAddress,
3586+
address: checksummedTokenAddress,
35853587
decimals: 6,
35863588
symbol: 'USDC',
35873589
aggregators: [],
@@ -3652,7 +3654,11 @@ describe('TokenDetectionController', () => {
36523654

36533655
it('should add all tokens provided without filtering (filtering is caller responsibility)', async () => {
36543656
const mockTokenAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
3657+
const checksummedTokenAddress =
3658+
'0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
36553659
const secondTokenAddress = '0x1f573d6fb3f13d689ff844b4ce37794d79a7ff1c';
3660+
const checksummedSecondTokenAddress =
3661+
'0x1F573D6Fb3F13d689FF844B4cE37794d79a7FF1C';
36563662
const chainId = '0x1';
36573663
const selectedAccount = createMockInternalAccount({
36583664
address: '0x0000000000000000000000000000000000000001',
@@ -3718,7 +3724,7 @@ describe('TokenDetectionController', () => {
37183724
'TokensController:addTokens',
37193725
[
37203726
{
3721-
address: mockTokenAddress,
3727+
address: checksummedTokenAddress,
37223728
decimals: 6,
37233729
symbol: 'USDC',
37243730
aggregators: [],
@@ -3727,7 +3733,7 @@ describe('TokenDetectionController', () => {
37273733
name: 'USD Coin',
37283734
},
37293735
{
3730-
address: secondTokenAddress,
3736+
address: checksummedSecondTokenAddress,
37313737
decimals: 18,
37323738
symbol: 'BNT',
37333739
aggregators: [],
@@ -3744,6 +3750,8 @@ describe('TokenDetectionController', () => {
37443750

37453751
it('should track metrics when adding tokens from websocket', async () => {
37463752
const mockTokenAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
3753+
const checksummedTokenAddress =
3754+
'0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
37473755
const chainId = '0x1';
37483756
const mockTrackMetricsEvent = jest.fn();
37493757

@@ -3793,7 +3801,7 @@ describe('TokenDetectionController', () => {
37933801
event: 'Token Detected',
37943802
category: 'Wallet',
37953803
properties: {
3796-
tokens: [`USDC - ${mockTokenAddress}`],
3804+
tokens: [`USDC - ${checksummedTokenAddress}`],
37973805
token_standard: 'ERC20',
37983806
asset_type: 'TOKEN',
37993807
},
@@ -3810,6 +3818,8 @@ describe('TokenDetectionController', () => {
38103818

38113819
it('should be callable directly as a public method on the controller instance', async () => {
38123820
const mockTokenAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
3821+
const checksummedTokenAddress =
3822+
'0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
38133823
const chainId = '0x1';
38143824

38153825
await withController(
@@ -3857,7 +3867,7 @@ describe('TokenDetectionController', () => {
38573867
'TokensController:addTokens',
38583868
[
38593869
{
3860-
address: mockTokenAddress,
3870+
address: checksummedTokenAddress,
38613871
decimals: 6,
38623872
symbol: 'USDC',
38633873
aggregators: [],

‎packages/assets-controllers/src/TokenDetectionController.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
ERC20,
1616
safelyExecute,
1717
isEqualCaseInsensitive,
18+
toChecksumHexAddress,
1819
} from '@metamask/controller-utils';
1920
import type {
2021
KeyringControllerGetStateAction,
@@ -1042,27 +1043,28 @@ export class TokenDetectionController extends StaticIntervalPollingController<To
10421043
const tokensWithBalance: Token[] = [];
10431044
const eventTokensDetails: string[] = [];
10441045

1045-
for (const nonZeroTokenAddress of tokensSlice) {
1046-
// Check map of validated tokens
1046+
for (const tokenAddress of tokensSlice) {
1047+
// Normalize addresses explicitly (don't assume input format)
1048+
const lowercaseTokenAddress = tokenAddress.toLowerCase();
1049+
const checksummedTokenAddress = toChecksumHexAddress(tokenAddress);
1050+
1051+
// Check map of validated tokens (cache keys are lowercase)
10471052
const tokenData =
1048-
this.#tokensChainsCache[chainId]?.data?.[
1049-
nonZeroTokenAddress.toLowerCase()
1050-
];
1053+
this.#tokensChainsCache[chainId]?.data?.[lowercaseTokenAddress];
10511054

10521055
if (!tokenData) {
10531056
console.warn(
1054-
`Token metadata not found in cache for ${nonZeroTokenAddress} on chain ${chainId}`,
1057+
`Token metadata not found in cache for ${tokenAddress} on chain ${chainId}`,
10551058
);
10561059
continue;
10571060
}
10581061

1059-
const { decimals, symbol, aggregators, iconUrl, name, address } =
1060-
tokenData;
1062+
const { decimals, symbol, aggregators, iconUrl, name } = tokenData;
10611063

1062-
// Push to lists
1063-
eventTokensDetails.push(`${symbol} - ${address}`);
1064+
// Push to lists with checksummed address (for allTokens storage)
1065+
eventTokensDetails.push(`${symbol} - ${checksummedTokenAddress}`);
10641066
tokensWithBalance.push({
1065-
address,
1067+
address: checksummedTokenAddress,
10661068
decimals,
10671069
symbol,
10681070
aggregators,

‎packages/core-backend/CHANGELOG.md‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,34 +7,38 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10-
### Changed
10+
### Added
1111

12-
- **BREAKING**: `BackendWebSocketService` - Simplified connection management and added KeyringController event integration ([#6819](https://github.com/MetaMask/core/pull/6819))
13-
- Added `KeyringController:lock` and `KeyringController:unlock` event subscriptions to automatically manage WebSocket connections based on wallet lock state
14-
- Renamed internal method `setupAuthentication()` to `subscribeEvents()` to reflect broader event handling responsibilities
15-
- Simplified reconnection logic: auto-reconnect on any unexpected disconnect, stay disconnected on manual disconnects (tracked via `#manualDisconnect` flag)
16-
- Updated `connect()` to reset manual disconnect flag, allowing reconnection after previous manual disconnects
17-
- Updated `disconnect()` to set manual disconnect flag, preventing automatic reconnection
18-
- Improved error handling in `connect()` to properly rethrow errors to callers
19-
- **BREAKING**: `AccountActivityService` - Replaced API-based chain support detection with system notification-driven chain tracking ([#6819](https://github.com/MetaMask/core/pull/6819))
20-
- Added internal `#chainsUp` Set to track chains reported as 'up' via system notifications
21-
- Updated system notification handler to dynamically track chain status (add to set when 'up', remove when 'down')
22-
- Updated WebSocket state change handler to flush all tracked chains as 'down' on disconnect/error (instead of using hardcoded list)
23-
- Chain status is now entirely driven by backend system notifications rather than proactive API calls
24-
- **BREAKING**: Updated `Transaction` type definition - renamed `hash` field to `id` for consistency with backend API ([#6819](https://github.com/MetaMask/core/pull/6819))
25-
- **BREAKING**: Updated `Asset` type definition - added required `decimals` field for proper token amount formatting ([#6819](https://github.com/MetaMask/core/pull/6819))
26-
- `BackendWebSocketService` - Added optional `traceFn` parameter to constructor for performance tracing integration (e.g., Sentry)
12+
- **BREAKING:** Add required argument `channelType` to `BackendWebSocketService.subscribe` method ([#6819](https://github.com/MetaMask/core/pull/6819))
13+
- Add `channelType` to argument of the `BackendWebSocketService:subscribe` messenger action
14+
- Add `channelType` to `WebSocketSubscription` type
15+
- **BREAKING**: Update `Asset` type definition: add required `decimals` field for proper token amount formatting ([#6819](https://github.com/MetaMask/core/pull/6819))
16+
- Add optional `traceFn` parameter to `BackendWebSocketService` constructor for performance tracing integration (e.g., Sentry) ([#6819](https://github.com/MetaMask/core/pull/6819))
2717
- Enables tracing of WebSocket operations including connect, disconnect methods
2818
- Trace function receives operation metadata and callback to wrap for performance monitoring
29-
- Updated documentation (README.md) to reflect new connection management model and chain tracking behavior ([#6819](https://github.com/MetaMask/core/pull/6819))
30-
- Added "WebSocket Connection Management" section explaining connection requirements and behavior
31-
- Updated sequence diagram to show system notification-driven chain status flow
32-
- Updated key flow characteristics to reflect internal chain tracking mechanism
19+
- Add optional `timestamp` property to `ServerNotificationMessage` and `SystemNoticationData` types ([#6819](https://github.com/MetaMask/core/pull/6819))
20+
- Add optional `timestamp` property to `AccountActivityService:statusChanged` event and corresponding event type ([#6819](https://github.com/MetaMask/core/pull/6819))
21+
22+
### Changed
23+
24+
- **BREAKING:** Update `BackendWebSocketService` to automatically manage WebSocket connections based on wallet lock state ([#6819](https://github.com/MetaMask/core/pull/6819))
25+
- `KeyringController:lock` and `KeyringController:unlock` are now required events in the `BackendWebSocketService` messenger
26+
- **BREAKING**: Update `Transaction` type definition: rename `hash` field to `id` for consistency with backend API ([#6819](https://github.com/MetaMask/core/pull/6819))
27+
- **BREAKING:** Add peer dependency on `@metamask/keyring-controller` (^23.0.0) ([#6819](https://github.com/MetaMask/core/pull/6819))
28+
- Update `BackendWebSocketService` to simplify reconnection logic: auto-reconnect on any unexpected disconnect (not just code 1000), stay disconnected when manually disconnecting via `disconnect` ([#6819](https://github.com/MetaMask/core/pull/6819))
29+
- Improve error handling in `BackendWebSocketService.connect()` to properly rethrow errors to callers ([#6819](https://github.com/MetaMask/core/pull/6819))
30+
- Update `AccountActivityService` to replace API-based chain support detection with system notification-driven chain tracking ([#6819](https://github.com/MetaMask/core/pull/6819))
31+
- Instead of hardcoding a list of supported chains, assume that the backend has the list
32+
- When receiving a system notification, capture the backend-tracked status of each chain instead of assuming it is up or down
33+
- Flush all tracked chains as 'down' on disconnect/error (instead of using hardcoded list)
34+
- Update documentation in `README.md` to reflect new connection management model and chain tracking behavior ([#6819](https://github.com/MetaMask/core/pull/6819))
35+
- Add "WebSocket Connection Management" section explaining connection requirements and behavior
36+
- Update sequence diagram to show system notification-driven chain status flow
37+
- Update key flow characteristics to reflect internal chain tracking mechanism
3338

3439
### Removed
3540

36-
- **BREAKING**: Removed `getSupportedChains()` public method and all related API fetching logic
37-
- **BREAKING**: Removed hardcoded `DEFAULT_SUPPORTED_CHAINS` fallback list and cache expiration mechanism
41+
- **BREAKING**: Remove `getSupportedChains` method from `AccountActivityService` ([#6819](https://github.com/MetaMask/core/pull/6819))
3842

3943
## [1.0.1]
4044

‎packages/core-backend/src/AccountActivityService.test.ts‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -574,8 +574,6 @@ describe('AccountActivityService', () => {
574574
const publishSpy = jest.spyOn(messenger, 'publish');
575575
const systemCallback = getSystemNotificationCallback(mocks);
576576

577-
publishSpy.mockClear();
578-
579577
// Simulate chains coming up
580578
const timestamp1 = 1760344704595;
581579
systemCallback({
@@ -597,8 +595,6 @@ describe('AccountActivityService', () => {
597595
},
598596
);
599597

600-
publishSpy.mockClear();
601-
602598
// Simulate one chain going down
603599
const timestamp2 = 1760344704696;
604600
systemCallback({
@@ -630,9 +626,6 @@ describe('AccountActivityService', () => {
630626

631627
mocks.getSelectedAccount.mockReturnValue(null);
632628

633-
// Clear any publish calls from service initialization
634-
publishSpy.mockClear();
635-
636629
// First, simulate receiving a system notification with chains up
637630
const systemCallback = getSystemNotificationCallback(mocks);
638631
systemCallback({
@@ -645,15 +638,17 @@ describe('AccountActivityService', () => {
645638
timestamp: 1760344704595,
646639
});
647640

648-
publishSpy.mockClear();
649-
650641
// Publish WebSocket ERROR state event - should flush tracked chains as down
651642
await rootMessenger.publish(
652643
'BackendWebSocketService:connectionStateChanged',
653644
{
654645
state: WebSocketState.ERROR,
655646
url: 'ws://test',
656647
reconnectAttempts: 2,
648+
timeout: 10000,
649+
reconnectDelay: 500,
650+
maxReconnectDelay: 5000,
651+
requestTimeout: 30000,
657652
},
658653
);
659654
await completeAsyncOperations(100);
@@ -664,6 +659,7 @@ describe('AccountActivityService', () => {
664659
{
665660
chainIds: ['eip155:1', 'eip155:137', 'eip155:56'],
666661
status: 'down',
662+
timestamp: expect.any(Number),
667663
},
668664
);
669665
});
@@ -675,16 +671,17 @@ describe('AccountActivityService', () => {
675671

676672
mocks.getSelectedAccount.mockReturnValue(null);
677673

678-
// Clear any publish calls from service initialization
679-
publishSpy.mockClear();
680-
681674
// Publish WebSocket ERROR state event without any tracked chains
682675
await rootMessenger.publish(
683676
'BackendWebSocketService:connectionStateChanged',
684677
{
685678
state: WebSocketState.ERROR,
686679
url: 'ws://test',
687680
reconnectAttempts: 2,
681+
timeout: 10000,
682+
reconnectDelay: 500,
683+
maxReconnectDelay: 5000,
684+
requestTimeout: 30000,
688685
},
689686
);
690687
await completeAsyncOperations(100);
@@ -787,6 +784,10 @@ describe('AccountActivityService', () => {
787784
state: WebSocketState.CONNECTED,
788785
url: 'ws://test',
789786
reconnectAttempts: 0,
787+
timeout: 10000,
788+
reconnectDelay: 500,
789+
maxReconnectDelay: 5000,
790+
requestTimeout: 30000,
790791
},
791792
);
792793
// Wait for async handler to complete
@@ -935,6 +936,10 @@ describe('AccountActivityService', () => {
935936
state: WebSocketState.CONNECTED,
936937
url: 'ws://test',
937938
reconnectAttempts: 0,
939+
timeout: 10000,
940+
reconnectDelay: 500,
941+
maxReconnectDelay: 5000,
942+
requestTimeout: 30000,
938943
},
939944
);
940945
await completeAsyncOperations();

‎packages/core-backend/src/AccountActivityService.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,7 @@ export class AccountActivityService {
438438
this.#messenger.publish(`AccountActivityService:statusChanged`, {
439439
chainIds: chainsToMarkDown,
440440
status: 'down',
441+
timestamp: Date.now(),
441442
});
442443

443444
log(

0 commit comments

Comments
 (0)