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

ethers.BigNumber.from does not work with negative stringified bignumber #1010

Closed
KillariDev opened this issue Aug 20, 2020 · 6 comments
Closed
Labels
discussion Questions, feedback and general information. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@KillariDev
Copy link

I have need to stringify a BigNumber and restore it. This seems to work with only with positive numbers and not negative

This fails:

ethers.BigNumber.from({ _hex: "-0x01", _isBigNumber: true })

while this works:

ethers.BigNumber.from({ _hex: "0x01", _isBigNumber: true })

Also this fails:

ethers.BigNumber.from(JSON.parse(JSON.stringify(ethers.BigNumber.from(-1))))

while this works:

ethers.BigNumber.from(JSON.parse(JSON.stringify(ethers.BigNumber.from(1))))

The workaround is to get the _hex string of the object and work with that, but I think it would be nice if this would work with the whole object.

@ricmoo
Copy link
Member

ricmoo commented Aug 20, 2020

The _ properties are meant to be internal and instantiation isn’t meant to operate quite in that way. Those properties may change in the future and they should not be depended upon. For example, I may soon add internal BigInt support for platforms that support it.

If your are serializing, you should consider using just value.toString() which would return the decimal value as a string or value.toHexString() which will return the hexadecimal value As a string. These strings should both work as you’d expect in BigNumber.from.

In general, you should not expect stringify to work on arbitrary classes...

Make sense?

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Aug 20, 2020
@KillariDev
Copy link
Author

I understand that they are intended to be internal, so the workaround there is not too nice.

I am using bigints in a bigger javascript object and its quite hand to be able to stringify whole thing and then get it back. The better workaround would be to iterate throught the structure and convert bigints using .toString(), or just use strings there all the time (but not as nice from dev experience perspective).

Is there are reason JSON.stringify couldn't be made to work or why it wouldn't be supported? If the support is not wanted, it would be handy to have .from() reject "{ _hex: "0x01", _isBigNumber: true }" as well so this would'nt cause weird bugs where it sometimes work and sometimes not.

@ricmoo
Copy link
Member

ricmoo commented Aug 23, 2020

I’ll look into adding a .toJSON method and test compatibility across implementations.

I plan to overhaul the internals of BigNumber, so relying on the above behaviour could just fail at some point in the future. I’m surprised that negative numbers don’t work (so I’ll prolly fix that too), but I’ll try getting a more sustainable solution in, in general. :)

@KillariDev
Copy link
Author

That sounds like a good plan! I appreciate it :)

@ricmoo ricmoo added the on-deck This Enhancement or Bug is currently being worked on. label Aug 24, 2020
@ricmoo
Copy link
Member

ricmoo commented Aug 26, 2020

Added in 5.0.9. Let me know if it works for you.

I've added JSON support to the BigNumber class as well as support negative _hex_ just incase someone stored a serialized value in the old format. The _hex` support will be dropped in v6 (next summer).

Thanks! :)

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Aug 26, 2020
@ricmoo
Copy link
Member

ricmoo commented Aug 26, 2020

Closing this now, but if you have any issues, please re-open.

Thanks! :)

@ricmoo ricmoo closed this as completed Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Questions, feedback and general information. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

2 participants