Skip to content
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

[Bug]: Balance does not update on duplicate chainId network #13690 #14245

Merged
merged 1 commit into from Nov 4, 2022

Conversation

dragana8
Copy link
Contributor

Explanation

Currently, the balance of an account won't update if the network selected has a chainId that is already present in Metamask. That happens because in account-tracker.js there is a check for chainId and if it belonged to known networks ( as in this case it was Ropsten network) it called the method that checked balance on Ropsten network and not localhost as it should.

In order to solve this problem, I added if statement that checks if rpcUrl is localhost and in that case the right method (updateBalance) is being called.

More information

Screenshots/Screencaps

Before

Screenshot 2022-03-29 at 13 00 34

After

Screenshot 2022-03-29 at 12 32 47

@github-actions
Copy link
Contributor

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.

@metamaskbot
Copy link
Collaborator

Builds ready [fd59007]
Page Load Metrics (1293 ± 34 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6814188199
domContentLoaded1194146412897335
load1202146412937234
domInteractive1194146412897335

@mirjanaKukic
Copy link
Contributor

Verified by QA

@dragana8 dragana8 marked this pull request as ready for review March 30, 2022 11:34
@dragana8 dragana8 requested a review from a team as a code owner March 30, 2022 11:34
segun
segun previously approved these changes May 24, 2022
Copy link
Contributor

@segun segun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danjm danjm added the PR - P1 identifies PRs deemed priority for Extension team label Jul 20, 2022
brad-decker
brad-decker previously approved these changes Aug 25, 2022
@brad-decker
Copy link
Member

@dragana Could you rebase this please and fix conflicts? we'll get it landed after that

Ellajoke
Ellajoke previously approved these changes Aug 25, 2022
@dragana8 dragana8 dismissed stale reviews from Ellajoke, brad-decker, ghost , and segun via bbee415 August 29, 2022 07:33
@metamaskbot
Copy link
Collaborator

Builds ready [bbee415]
Page Load Metrics (1633 ± 39 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint873521195828
domContentLoaded1515189216128842
load1532189216338139
domInteractive1515189216128842

@dragana8
Copy link
Contributor Author

@brad-decker done

@metamaskbot
Copy link
Collaborator

Builds ready [276ec39]
Page Load Metrics (1904 ± 64 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint91155121168
domContentLoaded16802170188313565
load16802170190413464
domInteractive16802170188313565

brad-decker
brad-decker previously approved these changes Sep 5, 2022
NiranjanaBinoy
NiranjanaBinoy previously approved these changes Nov 4, 2022
Copy link
Contributor

@NiranjanaBinoy NiranjanaBinoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me, this PR can be merged once it is rebased.

@brad-decker brad-decker dismissed stale reviews from NiranjanaBinoy and themself via 0884411 November 4, 2022 15:20
Copy link
Member

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rebased the pr for @dragana8, please reapprove when ready!

@metamaskbot
Copy link
Collaborator

Builds ready [0884411]
Page Load Metrics (2660 ± 164 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint962511353517
domContentLoaded208630802631325156
load208631062660341164
domInteractive208630802631325156

@brad-decker brad-decker merged commit 02b34dc into develop Nov 4, 2022
@brad-decker brad-decker deleted the balance-duplicate-chainId branch November 4, 2022 16:29
@github-actions github-actions bot locked and limited conversation to collaborators Nov 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR - P1 identifies PRs deemed priority for Extension team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Balance does not update on duplicate chainId network
8 participants