ethereumjs / fixed-bn.js

a bn.js wrapper that constrains numbers to a fixed width
7 stars 9 forks source link

JSBI from google #9

Open nivida opened 5 years ago

nivida commented 5 years ago

Hey,

I've checked the ethereum-js repositories because of a utility method I was looking for and saw the fixed-bn.js. I've done some research about the coming BigInt of JS and found the following polyfill which also has a babel plugin. It would be possible to extend from the JSBI and to create a fixed BigInt which is following the tc39 proposal interface. BigInt is already supported in chrome and exists as an experimental feature for Firefox.

JSBI: https://github.com/GoogleChromeLabs/jsbi Google blog post: https://developers.google.com/web/updates/2018/05/bigint

Is there also an idea around for a BigDecimals implementation as mentioned in the BigInt blog post of google? I would like to implement such an object with extending from the JSBI.

axic commented 5 years ago

Wow, that sounds interesting. I think we should definitely investigate, @s1na @holgerd77 @fanatid.

s1na commented 5 years ago

Yeah that definitely seems very promising. @axic just recently moved fixed-bn.js from ewasm organization here, and we're trying to revive it (see #8).

It's currently using bn.js, but if such a polyfill already exists and is more performant than bn.js, we could either fully switch to BigInt soon, or provide the same API with two alternative backens, bn.js and BigInt. The good thing is, once the other libraries are using these fixed-precision integers, if the backend changes, they won't need much modification.

Do you have specific use-cases for BigDecimals in mind? I'm not sure if I've felt the need for decimals in ethereumjs so far. Would you be interested in a joint effort?

holgerd77 commented 5 years ago

Maybe we directly concentrate on the polyfill version? At the end this (or at least: the native implementation) will be the much more performant choice.

s1na commented 5 years ago

Maybe we directly concentrate on the polyfill version?

I was thinking it might not be as fully-featured as bn.js, but on a second look, it seems to support most of the essentials. There might be a few convenience methods missing, but I don't as of yet see any road-blocker. Will give it a try and report back.

nivida commented 5 years ago

The use-cases for BigDecimals are for example in FinTec related DApps. We have an open issue about it in the Web3.js repository: https://github.com/ethereum/web3.js/issues/2171.

Yes, I would like to join. But currently, I have to focus on the coming stable release. But I will definitely do some tests if I find the time.

s1na commented 5 years ago

I gave JSBI a try, and my first impression is that it's still a bit clunky and it wasn't playing nice with typescript. Some utility methods (e.g. conversions to and from Buffer, determining byte length for checking width, etc., alternative reduction algorithms) are not there and we'll have to implement them ourselves. The repo also doesn't seem to be as active. Up to now my opinion is to continue with bn.js for now but keep a close eye on BigInt. However if someone is extra-motivated about JSBI and can help with the additional efforts I'll happily go with it.

holgerd77 commented 5 years ago

Ok, then let's continue with fixed-bn.js from here. I think this is a good choice and has enough validation. This is also some sort of problem where time will ease things, it's likely just a bit early for a switch.

@s1na is there anything I should review here already?

s1na commented 5 years ago

@holgerd77 #8 should be mostly ready (at least for the first round). I'm planning to test it further by trying it out in VM and figure out if there are problems. But those problems could also be addressed in future PRs. Some comments concerning the API and general structure could be helpful.

s1na commented 5 years ago

Hi @no2chem, I just noticed that you folks are using BigInt in merkle-patricia-tree, and have also worked on some utilities, e.g. bigint-buffer. I was curious about your experience of this transition so far, as we're also considering doing it.

nivida commented 5 years ago

We could probably move all the discussions in all the different issues to the tc39 repository with a clear issue of our needs.

no2chem commented 5 years ago

Hi @s1na - our experience is that bigints are nicely supported in typescript (since ~3.3) and much faster than comparing buffers. There are some annoying issues though. For example, we often use bigints where we want a fixed-byte-width number (for example, in Ethereum, we want a 20byte account). This causes some issues; for example, the to account 0 (for contract creations) is equivalent to the account 0000000000..n. These would normally be two different buffers.

Our reason for not using bn is mainly a performance motivated one; and honestly, for a special use-case like Ethereum, if one cared about performance, it would seem worthwhile to create a custom binding that was efficient (ie, that only handled 20 byte and 32byte numbers), since we actually don't need arbitrary precision but fixed precision numbers.

s1na commented 5 years ago

Thanks for sharing your experience.

we actually don't need arbitrary precision but fixed precision numbers.

We're trying to do something similar in this repo. For now it's a wrapper around BN that checks width on initialization and after operations (ie not the efficient binding you mentioned). But hopefully it'll evolve in that direction.

Just to clarify, you're only targeting node at the moment? Have you by any chance experimented with the jsbi polyfill?

nivida commented 5 years ago

I've added a comment to the Web3.js issue with my findings. https://github.com/ethereum/web3.js/issues/2171#issuecomment-488334795

holgerd77 commented 5 years ago

@nivida impressive summary! :+1:

no2chem commented 5 years ago

@s1na Currently, we only looked at node. I haven't looked at jsbi, I assume that for webpack+babel or some other mechanism would automatically implement the appropriate polyfill for it, as I think having bigint literals helps produce cleaner, more readable code.

s1na commented 5 years ago

From @nivida 's summary I got a good impression of BigInteger.js, specially because it polyfills to native BigInt (when present). I'll be doing some experiments with it.