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

isBN breaks in production #1777

Closed
epiqueras opened this issue Jul 13, 2018 · 6 comments · Fixed by #2793
Closed

isBN breaks in production #1777

epiqueras opened this issue Jul 13, 2018 · 6 comments · Fixed by #2793
Assignees
Labels
Bug Addressing a bug
Projects

Comments

@epiqueras
Copy link

Minifying the code makes the BN instance's constructor.name !== 'BN' so it breaks web3's implementation of isBN.

@nivida nivida self-assigned this Aug 10, 2018
@nivida nivida added the Bug Addressing a bug label Aug 10, 2018
@nivida
Copy link
Contributor

nivida commented Aug 13, 2018

Hay @epiqueras I've tested it with the minified version of web3 and cant reproduce your case.

Tested with:

var BN = web3.utils.BN;
var bigNumber = new BN(1234);
console.log(web3.utils.isBN(bigNumber));

The result I got:
true

@nivida nivida added more information needed and removed Bug Addressing a bug labels Aug 13, 2018
@loliconer
Copy link

@nivida
There is no problem when using web3.min.js. But error occurs when using webpack to minify code.

import Web3 from 'web3

console.log(Web3.utils.isBN(Web3.utils.toBN(100000000))) // false, because the constructor.name becomes 'o', so 'o' !== 'BN'
console.log(Web3.utils.isBN(new Web3.utils.BN(100000000))) // true

@nivida
Copy link
Contributor

nivida commented Nov 28, 2018

I think it should be solved with the PR #2000

@nivida nivida added this to In progress in 1.0 Nov 28, 2018
@such
Copy link

such commented Dec 11, 2018

@nivida what makes you think that? I have just looked at the code and isBN still uses constructor.name that causes the issue here. But I'm not a minification expert! Is there anything I'm missing?

@rhlsthrm
Copy link

Ran into this issue in a create-react-app implementation. Googling around I found this: indutny/bn.js#133.

Web3 should use the isBN function from the BN.js library directly.

@nivida nivida moved this from In progress to Proposed To Do's in 1.0 Apr 15, 2019
@nivida nivida added Bug Addressing a bug and removed more information needed labels Apr 15, 2019
@nivida nivida moved this from Proposed To Do's to Accepted To Do's in 1.0 Apr 15, 2019
@chapati23
Copy link

This is still a problem. Bumping the version of bn.js to 4.11.8 fixed it for me, temporary fix is possible via yarn resolutions, just add the following to your package.json:

  "resolutions": {
    "bn.js": "4.11.8"
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Addressing a bug
Projects
No open projects
1.0
  
Done
Development

Successfully merging a pull request may close this issue.

6 participants