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

Rounding negative FixedNumbers which are not using the default format results in an incompatible format error #1749

Closed
alexanderwende opened this issue Jul 5, 2021 · 5 comments
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@alexanderwende
Copy link

Hi, I think there's still a small bug in FixedNumber.round(), when working with negative numbers...

Describe the bug

Rounding negative FixedNumbers which are not in the default format results in an incompatible format error.

Reproduction steps

FixedNumber.from('-0.01101100205516751', 'fixed128x17').round(2)

Error produced

Uncaught Error: incompatible format; use fixedNumber.toFormat (argument="other", value={"format":{"signed":true,"width":128,"decimals":18,"name":"fixed128x18","_multiplier":"1000000000000000000"},"_hex":"0x0de0b6b3a7640000","_value":"1.0","_isFixedNumber":true}, code=INVALID_ARGUMENT, version=bignumber/5.4.0)
    at Logger.makeError (index.ts:213)
    at Logger.throwError (index.ts:225)
    at Logger.throwArgumentError (index.ts:229)
    at FixedNumber._checkFormat (fixednumber.ts:217)
    at FixedNumber.subUnsafe (fixednumber.ts:229)
    at FixedNumber.floor (fixednumber.ts:257)
    at FixedNumber.round (fixednumber.ts:294)

Environment

browser

Notes

When removing the minus sign from the number, no error is thrown.

Originally posted by @alexanderwende in #1629 (comment)

@ricmoo ricmoo added investigate Under investigation and may be a bug. on-deck This Enhancement or Bug is currently being worked on. labels Jul 5, 2021
@ricmoo ricmoo added bug Verified to be an issue. and removed investigate Under investigation and may be a bug. labels Jul 22, 2021
@ricmoo
Copy link
Member

ricmoo commented Jul 22, 2021

The bug is in the .floor() method which is an intermediate step of rounding.

It is fixed locally and will be going out in the next release.

Thanks! :)

@ricmoo
Copy link
Member

ricmoo commented Jul 23, 2021

This has been fixed in 5.4.2. Let me know if you still have any issues with it.

@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 Jul 23, 2021
@alexanderwende
Copy link
Author

@ricmoo Awesome, thanks a bunch! Will give it a shot soonish and will let you know.

@ricmoo
Copy link
Member

ricmoo commented Jul 28, 2021

Closing this now. If you find it is still an issue, please re-open.

Thanks! :)

@ricmoo ricmoo closed this as completed Jul 28, 2021
@alexanderwende
Copy link
Author

Hi, just go around to testing this out today. I haven't found any issues, had a test suite to run against, which was using a workaround for rounding negative numbers. After updating ethers to 5.4.2 and removing the workaround everything was green. So yeah, this one is good! :)

Thanks again for your quick help with this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

2 participants