BigNumber.js -> bn.js

[debris]

I'm trying to migrate from BigNumber.js to bn.js, but I'm having troubles with 4 tests (formatting real numbers)
@ethers @wanderer can you help me? Check failing tests on travis.

fixes #99

[debris]

here

[wanderer]

is k256 ok for the max int? Do you have a max int that ethereum.js can handle defined somewhere?

[debris]

so BigNumber.js was accepting floats on input. bn.js is not (or I do not know how to do this properly)

[wanderer]

@debris i haven't went through the codebase to see where formatOutputUReal is being used but can you tell me what values formatOutputUReal is suppose to support?

[debris]

floats

assert.equal(parser.test([2.125]),  "0000000000000000000000000000000220000000000000000000000000000000");
assert.equal(parser.test([8.5]),    "0000000000000000000000000000000880000000000000000000000000000000");

[chevdor]

Travis shows only linting errors.
You are lucky however with 4 errors or you are not using the latest I made in master.
After a quick s/BigNumber/bn/ I get 13 tests failing in the browser.

[wanderer]

this should be value.cmpn(new BN(0))

[debris]

you are right, I will fix this

[wanderer]

@debris on the floats issue BN will always around down. So Just multiply by how many decimal places of accuracy you need. Make sense?

[chevdor]

@wanderer I tried this idea, it breaks always somewhere like when you want to convert 1111111111111111111 wei into ether for instance or for any units that are 'far' from each other and require the numbers to be expressed in BN. That should be 1.1 and BN as far as I see cannot support decimal. The only way I found (still not acceptable) is to do this:

if (number.cmpn(divider)>0){
        number = number.div(divider);
        result = (isBigNumber) ? number : number.toString(10);
    }
    else{
        result = 1/parseFloat(number.div(divider).toString(10));
    }

The first block is what is done today and it works fine. This is the non decimal part.
The second block gives you decimal but no longer big numbers compatible. In other words, it works for a small BN (lol) but fails for real BN. It then returns Infinity.

@debris did you progress on the topic? I don´t really see how to solve that.
bn.js does not support decimal so all the tests related to that will fail. I think method such as fromWei should be avoided. Everything should be in wei so we don´t loose anything in the calculations and we only display a more readable version of the amount with the existing method: utils.toEth()

[wanderer]

@chevdor if you are converting units around you probably don't need bignum at all. Thats should be easy todo with pure string manipulation since all the operations result in a simple decimal shift.

[chevdor]

Hello @wanderer,
from what I tested and saw in the existing tests this is not 100% correct unfortunately.

If you take units that are close to each other (ie wei and Kwei), I am totally with you and the "limited solution I explained" works. However, if you take units far away from each other, then it depends if number a > number b. If the div is a large number, then the BN lib does the trick very well. The other way around is the problem.

I think the main question is: do we want to keep some of the methods and keep them accurate. I see neither a way nor a need to express 1 wei into Mether (for instance).

Stephan - I think - was suggesting to stay in wei as much as possible (= all the time) for the calculations (and using BN then works all fine) and just making conversion (with loss) for the user and only for display purposes.

In short I would be fine saying that 1 wei = 0 Mether instead of 0.000....001 Mether.
Knowing that we do have function today that can provide a user friendly representation of 1000.000 wei in whatever unit is the most appropriate for display.

I think there is an important decision to make here. Once everyone agrees, we can all move forward but I feel like @debris at the moment. I try to solve the problem he mentioned on my side and I get to the same issues he explained.

My take: remove functions and tests that end up with decimal BN (since that does not exist) and aimed at anything else than display.

[debris]

temporary closing