-
-
Notifications
You must be signed in to change notification settings - Fork 250
feat(core-backend): update websocket behavior #6819
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
base: main
Are you sure you want to change the base?
Conversation
61d7fa7
to
1a6bf0c
Compare
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.
Approving for the assets codeowner part.
8f51798
to
e7dac94
Compare
@metamaskbot publish-preview |
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions.
|
e7dac94
to
0189b42
Compare
@metamaskbot publish-preview |
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions.
|
Explanation
Current State & Motivation
The
core-backend
package had several architectural issues that needed to be addressed:Inflexible Chain Management:
AccountActivityService
was proactively fetching supported chains from the Accounts API (/v2/supportedNetworks
) with a hardcoded fallback list. This approach was:Incomplete Connection Management:
BackendWebSocketService
only subscribed toAuthenticationController:stateChange
events but didn't directly respond to wallet lock/unlock events fromKeyringController
. This created a dependency where connection management relied solely on authentication state changes.Reconnection Logic Based on Close Codes: The service used WebSocket close codes to determine reconnection behavior, which was fragile and didn't properly distinguish between manual disconnects (user action) and unexpected disconnects (network issues).
Type Mismatches: The
Transaction
andAsset
types didn't match the backend API contract, causing potential integration issues.No Performance Monitoring: There was no way to trace WebSocket operations for performance analysis in production environments.
Solution
1. System Notification-Driven Chain Tracking (
AccountActivityService
)Before: Client proactively fetches and caches supported chains
After: Client reacts to backend system notifications
Benefits:
Flow:
{chainIds: ['eip155:1', 'eip155:137'], status: 'up'}
2. Simplified Reconnection Logic (
BackendWebSocketService
)Before: Reconnection based on close codes
After: Reconnection based on manual disconnect flag
Benefits:
3. KeyringController Event Integration
Added direct subscriptions to wallet lock state:
Connection Requirements (all must be true):
isEnabled()
callback)AuthenticationController.isSignedIn
)KeyringController
state)4. Type Alignment with Backend API
Transaction.hash
→Transaction.id
(matches backend field name)Asset
now requiresdecimals: number
(needed for proper token formatting)5. Performance Tracing Integration
Added optional
traceFn
parameter to enable performance monitoring:Benefits:
Integration:
trace
fromshared/lib/trace.ts
(Sentry-backed)trace
fromapp/util/trace.ts
(Sentry-backed)Example usage:
Breaking Changes Impact
For
AccountActivityService
consumers:getSupportedChains()
method - consumers should subscribe toAccountActivityService:statusChanged
events insteadFor type definitions:
Transaction
objects must useid
instead ofhash
Asset
objects must includedecimals
fieldTesting Updates
nock
dependency (no longer fetching from external API)References
Checklist
Note
Revamps WebSocket lifecycle and reconnection with lock/unlock integration and tracing, replaces API-based chain support with system-notification tracking, and aligns types (Transaction.id, Asset.decimals).
traceFn
for performance tracing; trace connection, disconnect, notifications.KeyringController:lock/unlock
events; idempotentconnect()
/disconnect()
.#manualDisconnect
flag; always auto-reconnect on unexpected closes.timestamp
handling; exposechannelType
; refineconnectedAt
/state.#chainsUp
set.down
; resubscribe on reconnect.statusChanged
with optionaltimestamp
; passchannelType
on subscribe.Transaction.hash
→Transaction.id
.Asset
adds requireddecimals
.@metamask/keyring-controller
(dev/peer); TS references updated.TokenBalancesController.test.ts
to includeasset.decimals
in balance updates.Written by Cursor Bugbot for commit 0189b42. This will update automatically on new commits. Configure here.