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

Remove default to 18 decimals in quotesToRenderableData method #10212

Merged
merged 1 commit into from
Jan 19, 2021

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Jan 19, 2021

Fixes an issue where tokens with 0 decimals would show incorrect amounts on the view token screen.

The default to 18 decimals was unnecessary, as we validate /tokens api responses in swaps.util.js to verify that decimals are defined and either a number or a string of decimal digits between 0 and 36. If the decimals were somehow undefined by the time quotesToRenderableData in view-quote.js, then it would be better that a user hit an error than be shown an incorrect default.

A future improvement will be to add data validation to the quotesToRenderableData method

@danjm danjm requested a review from a team as a code owner January 19, 2021 16:52
@danjm danjm requested a review from darkwing January 19, 2021 16:52
@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.

Copy link
Contributor

@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.

LGTM

@metamaskbot
Copy link
Collaborator

Builds ready [87883ea]
Page Load Metrics (475 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint316443105
domContentLoaded30858947410148
load30959047510048
domInteractive30858947410048

@danjm danjm merged commit 5b6e524 into develop Jan 19, 2021
@danjm danjm deleted the fix-default-to-decimals-swaps-token branch January 19, 2021 17:06
@github-actions github-actions bot locked and limited conversation to collaborators Jan 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants