BN-Chai assertions fail

[LayneHaber]

Switching over to use the native BigNumber library from this package, and even though it wraps the bn.ts, it seems that it doesn't work with the bn-chai assertions.

My guess is it has something to do with the casting that happens in the constructor in your package, but not natively in the bn package (though I'm really not sure).

[ricmoo]

Oh, the BN.js objects are only used internally during actual maths operations. Outside of that, the BigNumber only keeps a normalized hex string representation.

You should be able to add any methods you need to the BigNumber prototype yourself though. What method are you trying to use?

[LayneHaber]

I was using the equal assert (more or less) like this in the tests:

const should = require("chai");
const BN = require("bn.js");

should
  .use(require("chai-as-promised"))
  .use(require("bn-chai")(BN))
  .should();


describe("eq", () => {
  it("should work", () => {
    const test1 = ethers.utils.bigNumberify("2")
    const test2 = ethers.utils.bigNumberify("2")
    test1.should.be.eq.BN(test2)
  })
})

[ricmoo]

Wow... So, Chai looks insane, and I don't know that API or syntax at all. :p

Is it easy to write plugins? Once I get v5 up, I would fully welcome a plugin for Chai with ethers, but that code above most certainly will not work, as the BN is not accessible in any sense like that. For now you will need to use something more akin to: assert(test1.eq(test2)), which is what I assume that .should.be.eq is doing? Out of curiosity, what is the advantage of that over the assert?

[LayneHaber]

I'll throw it on the list, and if it becomes enough of an add-on for me, I'll let you know if I get around to it.

I've never written a plugin for chai, but I don't think its too hard. And I agree! I was surprised this worked as well, but these tests were passing before making the switch to ethers :). As far as I understand it, the only real advantage of the should syntax is readability.

Using the more traditional assert. is what we switched to (and we stopped using the bn-chai ), so its not mission critical.

[ricmoo]

Awesome possum!

I'm going to close this now, but if you ever get around to opening a PR for this, feel free to add it as an ancillary package for v5. I'll get docs up soon on how to do that; once I figure out the best way to prevent the namespace from becoming too cluttered. Depending on how it works, it may also be a candidate for the @ethersproject/tests package.

Thanks! :)

[wighawag]

Hey, was just looking at issues related to BN as I am used to that lib.
Would like to know why ethers.js decided to wrap bn.js and not use it directly ?

[ricmoo]

@wighawag Heya! It's a very common question, feel free to peruse the issues (just search for BigNumber) as many of them cover various reasons.

Here is one from a while ago that hits most of the bullet points, I believe: #312 (comment)

tl;dr: immutability, control over used/exposed functionality (to simplify testing coverage and facilitate swapping out the underlying implementation if needed, or if switching to another ECC library) and in general I try to keep external dependencies un-exposed, so it is easier/possible to safeguard against certain attack vectors.

I'll be including a larger explanation in the v5 documentation. :)