-
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
fix: 0 token balance in send flow #28136
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. |
@@ -2670,7 +2670,7 @@ export function updateSendAsset( | |||
|
|||
if (details.standard === TokenStandard.ERC20) { | |||
asset.balance = | |||
details.balance && typeof details.decimals === 'number' | |||
details.balance && details.decimals !== undefined |
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.
or maybe parseInt(details.decimals) >= 0
. I'm not sure when this can be something unexpected. The number check was introduced to replace a truthiness check, which did not work for 0 decimal tokens.
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.
I did verify this PR still works for 0 decimal tokens
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 will also be false when balance === 0
- should change the details.balance
to typeof details.balance
, or is it typed well enough that we know that details.balance
is never a number
representation?
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.
I dont really know. Trying to make minimal changes when hotfixing things. The decimal check is the only bit that changed recently.
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.
its true that balance is a string in every place i can reproduce though
Builds ready [40d7d3f]
Page Load Metrics (2081 ± 74 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
Description
Fixes an issue where token balance showed as 0 during send flow. This occurred when clicking the token in the token list, then clicking the send button from the token details page.
When going send first and then picking a token, picking
decimals
was a number:But when going token first and then clicking send ,
decimals
was a string and skipped calculating the balance.calcTokenAmount
seems to work with either string or number, so changing logic from #27083 which introduced the number checkRelated issues
Fixes: #28112
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist