fix: Fixes BigNumber error due to incorrect number type#296
fix: Fixes BigNumber error due to incorrect number type#296amaury1093 merged 8 commits intomasterfrom
Conversation
Fixes error `Uncaught Error: lt() number type has more than 15 significant` that occurs when for example the users balance is 0.999368704999999998, the tx fee is 0.000063 (i.e. 3Gwei * 21000)
Tbaut
left a comment
There was a problem hiding this comment.
Thanks, I think I've had this error before, but didn't know how to reproduce.
I'd let a pro confirm before merging.
|
@ltfschoen I merged your PRs chronologically, but this one has some conflicts with 2 previous ones. Could you rebase this PR on top of latest master? Feature-wise, we can re-test everything from this PR. |
…alidation if input more than token dps
|
@amaurymartiny I've fixed the merge conflicts and used BigNumber in a couple of other places. The main change that resolves the error is |
amaury1093
left a comment
There was a problem hiding this comment.
Works well, tiny comment
| const amountBn = new BigNumber(amount.toString()); | ||
|
|
||
| if (!amount || isNaN(amount)) { | ||
| if (!amount || amountBn.isNaN()) { |
There was a problem hiding this comment.
!amount is not useful here, since amount.toString() would throw an error before.
Could you add the test if (!amount) {...} before the new BigNumber initialisation?
There was a problem hiding this comment.
i've pushed a commit with this change
| @@ -161,11 +161,11 @@ class Send extends Component { | |||
| preValidate = values => { | |||
| const { balance, token } = this.props; | |||
| const amount = +values.amount; | |||
There was a problem hiding this comment.
If we use BigNumbers here, the + is useless
There was a problem hiding this comment.
I found that when the user enters a zero value (i.e. 0, 0.0, etc), the following returned different values:
!values.amountreturnedfalse!+values.amountreturnedtrueamountBn.isNaN()returnedfalse
So if I replaced +values.amount with values.amount or just relied upon amountBn.isNaN() it would allow zero values (which would deviate from the current implementation where zero values are not permitted).
So in the commit that I've pushed I've added a .isZero() check as well.
Fixes error
Uncaught Error: lt() number type has more than 15 significant. It occurred when for example the users balance was 0.999368704999999998 ETH, the tx fee was 0.000063 (i.e. 3Gwei * 21000)