-
-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -7,6 +7,35 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||||
|
||||||
## [Unreleased] | ||||||
|
||||||
### Changed | ||||||
|
||||||
- **BREAKING**: `BackendWebSocketService` - Simplified connection management and added KeyringController event integration ([#6819](https://github.com/MetaMask/core/pull/6819)) | ||||||
- Added `KeyringController:lock` and `KeyringController:unlock` event subscriptions to automatically manage WebSocket connections based on wallet lock state | ||||||
- Renamed internal method `setupAuthentication()` to `subscribeEvents()` to reflect broader event handling responsibilities | ||||||
- Simplified reconnection logic: auto-reconnect on any unexpected disconnect, stay disconnected on manual disconnects (tracked via `#manualDisconnect` flag) | ||||||
- Updated `connect()` to reset manual disconnect flag, allowing reconnection after previous manual disconnects | ||||||
- Updated `disconnect()` to set manual disconnect flag, preventing automatic reconnection | ||||||
- Improved error handling in `connect()` to properly rethrow errors to callers | ||||||
- **BREAKING**: `AccountActivityService` - Replaced API-based chain support detection with system notification-driven chain tracking ([#6819](https://github.com/MetaMask/core/pull/6819)) | ||||||
- Added internal `#chainsUp` Set to track chains reported as 'up' via system notifications | ||||||
- Updated system notification handler to dynamically track chain status (add to set when 'up', remove when 'down') | ||||||
- Updated WebSocket state change handler to flush all tracked chains as 'down' on disconnect/error (instead of using hardcoded list) | ||||||
- Chain status is now entirely driven by backend system notifications rather than proactive API calls | ||||||
- **BREAKING**: Updated `Transaction` type definition - renamed `hash` field to `id` for consistency with backend API ([#6819](https://github.com/MetaMask/core/pull/6819)) | ||||||
- **BREAKING**: Updated `Asset` type definition - added required `decimals` field for proper token amount formatting ([#6819](https://github.com/MetaMask/core/pull/6819)) | ||||||
- `BackendWebSocketService` - Added optional `traceFn` parameter to constructor for performance tracing integration (e.g., Sentry) | ||||||
- Enables tracing of WebSocket operations including connect, disconnect methods | ||||||
- Trace function receives operation metadata and callback to wrap for performance monitoring | ||||||
- Updated documentation (README.md) to reflect new connection management model and chain tracking behavior ([#6819](https://github.com/MetaMask/core/pull/6819)) | ||||||
- Added "WebSocket Connection Management" section explaining connection requirements and behavior | ||||||
- Updated sequence diagram to show system notification-driven chain status flow | ||||||
- Updated key flow characteristics to reflect internal chain tracking mechanism | ||||||
|
||||||
### Removed | ||||||
|
||||||
- **BREAKING**: Removed `getSupportedChains()` public method and all related API fetching logic | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we specify where this was removed from? We are also missing a PR link. We also don't need to mention implementation details.
Suggested change
|
||||||
- **BREAKING**: Removed hardcoded `DEFAULT_SUPPORTED_CHAINS` fallback list and cache expiration mechanism | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't a public constant. Do we need to mention this in the changelog?
Suggested change
|
||||||
|
||||||
## [1.0.1] | ||||||
|
||||||
### Changed | ||||||
|
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.
A few notes:
connect
,disconnect
, etc. (And to be clear, if a change is "breaking" it means that there is some change that needs to be made on the client side to adjust to the change or else an error at compile-time or runtime will occur.)Putting this together, what if we do something like this: