-
-
Notifications
You must be signed in to change notification settings - Fork 270
feat: add network fallback mechanism to IncomingTransactionHelper #7759
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
Conversation
…ingTransactionHelper
packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts
Outdated
Show resolved
Hide resolved
packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts
Outdated
Show resolved
Hide resolved
packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts
Outdated
Show resolved
Hide resolved
packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts
Outdated
Show resolved
Hide resolved
packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts
Outdated
Show resolved
Hide resolved
packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts
Show resolved
Hide resolved
packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts
Show resolved
Hide resolved
|
|
||
| this.#onInterval().catch((error) => { | ||
| log('Initial polling failed', error); | ||
| log('Polling failed', error); |
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.
This was intentional to distinguish that the first attempt failed, rather than a future interval based one.
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.
PR is updated
packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts
Show resolved
Hide resolved
| }); | ||
| } | ||
| } else if (!this.#chainsToPoll.includes(hexChainId)) { | ||
| // status === 'down' |
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.
Should we be explicit in case they ever add more statuses?
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.
PR is updated
packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts
Show resolved
Hide resolved
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Explanation
In incoming transaction helper if any chain goes down on web sockets, start polling for it.
References
Fixes #12345
Checklist
Note
Medium Risk
Medium risk because it changes when/what chains are polled for incoming transactions and adds new event-driven state around network availability, which could impact transaction discovery if chain ID conversion or status handling is off.
Overview
When WebSocket-based incoming transaction retrieval is enabled,
IncomingTransactionHelpernow listens forAccountActivityService:statusChangedand starts/stops fallback polling based on which supported networks are reported down, polling only the affected chains.This threads an optional
chainIdsfilter throughRemoteTransactionSourceRequestand updatesAccountsApiRemoteTransactionSourceto query only those chains, plus adds tests and updatesTransactionController’s allowed events/types to include the new status event (and updates the changelog).Written by Cursor Bugbot for commit 72bf51c. This will update automatically on new commits. Configure here.