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.

Sign up for GitHub

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

Jump to bottom

Exchange amounts should not be represented in scientific notation #5271

Closed
tarikbellamine opened this issue Oct 6, 2020 · 3 comments · Fixed by #6212
Closed

Exchange amounts should not be represented in scientific notation #5271

tarikbellamine opened this issue Oct 6, 2020 · 3 comments · Fixed by #6212
Assignees
@i1skn
Labels
bug Something isn't working Component: CELO Priority: P2 Major quick-fix Things that should be quick fixes- Good for first issues & ramping up wallet
Milestone
Milestone 8

Comments

@tarikbellamine
Copy link
Contributor

Expected Behavior

Amounts are not represented in scientific notation.

Current Behavior

Amounts are sometimes represented in scientific notation when there are many decimals. This results in an "invalid number value" error.

https://sentry.io/organizations/celo/issues/1936609246/?project=1250733
@gnardini
Copy link
Contributor

gnardini commented Nov 6, 2020

I can't reproduce this and on Sentry there appears to be only 1 user. Wondering if it's really a P1 given the low number of affected users.

@nityas nityas added tmp and removed tmp labels Nov 11, 2020
@nityas nityas added bug Something isn't working quick-fix Things that should be quick fixes- Good for first issues & ramping up Priority: P2 Major Component: CELO and removed Priority: P1 Critical labels Nov 25, 2020
@i1skn i1skn mentioned this issue Dec 11, 2020
Merged
@i1skn i1skn added this to the Milestone 8 milestone Dec 11, 2020
@i1skn i1skn self-assigned this Dec 11, 2020
@jeanregisser
Copy link
Contributor

Small note, we also usually need to limit to 18 decimals when passing to a smart contract.

@mergify mergify bot closed this as completed in #6212 Dec 14, 2020
mergify bot pushed a commit that referenced this issue Dec 14, 2020
@i1skn
[Wallet] <BigNumber>.toString() almost never return exponential notat…
0c86f76 
…ion (#6212)

### Description

There is quite a few usages of `<BigNumber>.toString()` around the codebase, which potentially can return exponential notation, which AFAIK not needed anywhere in the app. This change would make  `<BigNumber>.toString()` almost identical to `<BigNumber>.toFixed()`, which always returns fixed notation.

For reference: https://mikemcl.github.io/bignumber.js/#exponential-at

Also, removed unused param in `createStandbyTx` function.

### Related issues

- Fixes #5271

### Backwards compatibility

Yes
@sentry-io
Copy link

sentry-io bot commented Dec 15, 2020

Sentry issue: CELO-MOBILE-2NR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@i1skn i1skn

Labels
bug Something isn't working Component: CELO Priority: P2 Major quick-fix Things that should be quick fixes- Good for first issues & ramping up wallet
Projects
None yet
Milestone
Milestone 8
Development

Successfully merging a pull request may close this issue.

[Wallet] <BigNumber>.toString() almost never return exponential notation celo-org/celo-monorepo
5 participants
@jeanregisser @nityas @gnardini @i1skn @tarikbellamine