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

Add bitwise operations to BigNumber #781

Closed
SilentCicero opened this issue Apr 11, 2020 · 16 comments
Closed

Add bitwise operations to BigNumber #781

SilentCicero opened this issue Apr 11, 2020 · 16 comments
Labels
enhancement New feature or improvement.

Comments

@SilentCicero
Copy link

Hey Richard,

Does ethers expose the plain BN object anywhere, there were some BN specific methods I need and it seems your wrapper doesn't include them (i think it was shifting).

Regardless, might be good to just expose the plain BN object for these pinch cases.

Best,
Nick

@ricmoo
Copy link
Member

ricmoo commented Apr 11, 2020

No, it’s not exposed. If you npm the same version in your project, it should reuse the same copy during rollup though.

The reason to not expose it is that some day I plan to remove it, and if people depend on it from ethers, than I can’t remove it without breaking them. It is very much meant to be internal. :)

What functionality do you need? There shouldn’t be any methods normal ethereum development should need beyond was is exposed, I think?

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Apr 11, 2020
@SilentCicero
Copy link
Author

SilentCicero commented Apr 11, 2020

Very understandable, what's the long term BN strategy?

I think it was missing some Bit operations that I needed, shift right/left/pow etc:

Bit operations

a.shln(b) - shift left (i, u, iu)
a.shrn(b) - shift right (i, u, iu)

Would be nice to just use ethers here, and it's just a few methods I couldn't seem to find in ethers. Maybe I just didn't see them?

@SilentCicero
Copy link
Author

I think something like this should do it #782

@ricmoo
Copy link
Member

ricmoo commented Apr 11, 2020

The pow function is present, but bitshifts are not. Other than maskn (which is required for the ABI coder), I’ve avoided bitwise operations, because they aren’t usually required, and for arbitrary precision some operations are not well-defined and many implementations try to “find a way” (the prime example being bitwise not).

That said, I can imaging people using a uint256 as a bit field (I have), so and, or, shl, shr and sar probably make sense to add. There won’t be a not though and I’ll add notes to the docs as to why the other bitwise operations were added but not that one...

It prolly won’t happen right away though. Still trying to get the new docs ready first so I can finally release v5. :)

@SilentCicero
Copy link
Author

Nice! I think a hefty explainer on the concerns of Bit Wise implementation concerns would be a nice addition to the docs.

Secondly, what are you using for the docs? Same tech, or a new platform?

@SilentCicero
Copy link
Author

One last thing...

I really, really hate looking at and typing bigNumberify every time.

Why not just big, please one day consider big. It can be our secret.

@ricmoo
Copy link
Member

ricmoo commented Apr 11, 2020

In v5, it’s BigNumber.from. Moving (slowly) away from magic functions. :)

For the most part though, you shouldn’t need to explicitly convert numbers to BigNumber, since most functions do that for you.

The new docs are written in Flatworm, because as much as I advocate against re-inventing the wheel, I often fail to heed my own advice. I’m almost done refactoring it, so it will be easier to add full-text search and PDF output. :)

@SilentCicero
Copy link
Author

@ricmoo I might use flatworm also, looks good, my own problem is it has too many dependancies.

Why don't you trim it down a bit ;)

@ricmoo ricmoo changed the title Does ethers expose BN Add bitwise operations to BN Apr 15, 2020
@ricmoo ricmoo added enhancement New feature or improvement. on-deck This Enhancement or Bug is currently being worked on. and removed discussion Questions, feedback and general information. labels Apr 15, 2020
@ricmoo ricmoo changed the title Add bitwise operations to BN Add bitwise operations to BigNumber Apr 15, 2020
@ricmoo
Copy link
Member

ricmoo commented Apr 15, 2020

Tests are running right now.

I've restricted all operations to positive numbers.

The and, or and xor operations are complicated (or not well defined) for negative values of arbitrary bit width.

The shift operations shl and shr require a positive shift offset (technically a negative offset could referent to an offset of the opposite direction, but for now I'm making the decision to not allow this).

I also omitted arithmetic right shift, since it is not common and I need to think more about what it means in the context of arbitrary bit width.

The mod also only allows a positive modulo.

Hopefully these all make sense and help. :)

@ricmoo
Copy link
Member

ricmoo commented Apr 15, 2020

Published to 5.0.0-beta.181.

Try it out and let me know if you have any problems.

@ricmoo ricmoo removed the on-deck This Enhancement or Bug is currently being worked on. label Apr 15, 2020
@SilentCicero
Copy link
Author

SilentCicero commented Apr 16, 2020 via email

@ricmoo
Copy link
Member

ricmoo commented Apr 17, 2020

Coolio. Closing this now.

Thanks! :)

@ricmoo ricmoo closed this as completed Apr 17, 2020
@samlaf
Copy link

samlaf commented Jul 27, 2022

I don't see shl or any other of the bit operations documented in the docs (https://docs.ethers.io/v5/api/utils/bignumber/#BigNumber). Would be nice/essential to have.

@ricmoo
Copy link
Member

ricmoo commented Jul 27, 2022

Agreed. I’ll add them on the next docs pass.

@barakman
Copy link

So what's the status on this? Still no bit-wise operations in the doc.

@Mouradif
Copy link

Still no bitwise operations in the doc (for people who still want to use v5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement.
Projects
None yet
Development

No branches or pull requests

5 participants