-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add networkClientId to DetectTokensController.detectNewTokens() #22898
Add networkClientId to DetectTokensController.detectNewTokens() #22898
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
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.
LGTM!
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #22898 +/- ##
===========================================
- Coverage 68.54% 68.54% -0.00%
===========================================
Files 1088 1088
Lines 42900 42904 +4
Branches 11418 11419 +1
===========================================
+ Hits 29403 29405 +2
- Misses 13497 13499 +2 ☔ View full report in Codecov by Sentry. |
Builds ready [2cd0d7a]
Page Load Metrics (989 ± 48 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
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.
LGTM
Description
Currently networkClientId token based polling in the
DetectTokensController
does not make thegetBalancesInSingleCall
contract call with the provider for the networkClientId. This is because networkClientId is not being passed from_executePoll
togetBalancesInSingleCall
.This PR adds an optional networkClientId param to
detectNewTokens
which is used to:getBalancesInSingleCall
calls with the correct providerRelated issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2097
Manual testing steps
Nothing should change as this does not affect the current single globally selected network flow and the multichain polling for this has not been integrated yet.
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist