-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: Max balance for dApp erc20 approval #13881
base: main
Are you sure you want to change the base?
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. |
|
ed9a2e3
to
8527fc0
Compare
|
|
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.
added a suggestion to remove conditional
It could also be helpful to add tests to check the full balance is shown
if (maxSelected) setValue(unroundedAccountBalance || accountBalance); | ||
}, [maxSelected, accountBalance, unroundedAccountBalance]); |
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.
if (maxSelected) setValue(unroundedAccountBalance || accountBalance); | |
}, [maxSelected, accountBalance, unroundedAccountBalance]); | |
if (maxSelected) setValue(unroundedAccountBalance); | |
}, [maxSelected, unroundedAccountBalance]); |
I think we may no longer need accountBalance. Else I think since unroundedAccountBalance may be an empty string, || accountBalance may never be reached
Description
When the user clicks "Max" on ERC20 token approvals, the account token balance is set as the spending cap. However, this number is rounded to 5 decimals places, which sometimes results in a value smaller than the actual token balance.
This PR fixes this bug by setting the unrounded account token balance instead.
Related issues
Fixes: #13122
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist