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

Utils functions mention BigNumber instead of BN in error messages #2468

Closed
rhlsthrm opened this issue Mar 6, 2019 · 5 comments · Fixed by #2705
Closed

Utils functions mention BigNumber instead of BN in error messages #2468

rhlsthrm opened this issue Mar 6, 2019 · 5 comments · Fixed by #2705
Labels
Bug Addressing a bug

Comments

@rhlsthrm
Copy link

rhlsthrm commented Mar 6, 2019

Description

Error messages for utils functions toWei, fromWei, etc. throw the following error:

Please pass numbers as strings or BigNumber objects to avoid precision errors.

The source itself checks if they are BN, not BigNumber. Relevant source (others in this file as well): https://github.com/ethereum/web3.js/blob/1.0/packages/web3-utils/src/index.js#L193

Steps to reproduce the behavior

  1. Import web3
  2. Call web3.utils.toWei(5) (i.e. pass in a number or other non-allowed type).

Error Logs

Gists

Versions

  • web3.js: 1.0.0 Beta 48
@nivida
Copy link
Contributor

nivida commented Mar 7, 2019

Yes, the documentation is wrong it's currently returning the BigNumber object but was always named BN. I will update the documentation and the related function documentation blocks.

@nivida nivida added the Documentation Relates to project wiki or documentation label Mar 7, 2019
@rhlsthrm
Copy link
Author

rhlsthrm commented Mar 7, 2019

It's also the thrown error messages (not sure if you are counting this in "documentation").

@interfect
Copy link

interfect commented Mar 15, 2019

Since there is an isBigNumber in the source, with auto-conversion to BN e.g. here, why shouldn't toWei and fromWei accept BigNumber objects? Why force a manual conversion with toBN?

@nivida nivida added Needs Clarification Requires additional input and removed Documentation Relates to project wiki or documentation labels Apr 3, 2019
@nivida
Copy link
Contributor

nivida commented Apr 16, 2019

@rhlsthrm @interfect I can't really answer that because I do not the thoughts the former maintainer had. The whole BigNumber handling here in Web3.js will get improved asap.

@mqklin
Copy link

mqklin commented Apr 24, 2019

@nivida are you going to use bignumber.js or BN? I'm migrating to beta now, and now I have to convert all BN objects to BigNumber, because bignumber is more convenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Addressing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants