Skip to content

fix: correct divRound rounding for negative operands - #320

Merged
fanatid merged 4 commits into
indutny:masterfrom
spokodev:fix/divround-negative-operands
Jul 9, 2026
Merged

fanatid merged 4 commits into
indutny:masterfrom
spokodev:fix/divround-negative-operands

Conversation

@spokodev

@spokodev spokodev commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

divRound computed the half-way threshold and the round-up step from the raw operands, which only works when both are positive. With a negative dividend or divisor it compared a signed remainder against a value taken from the signed divisor, stepped the quotient the wrong way, and dropped the sign when the truncated quotient was zero. For example new BN(-40).divRound(new BN(-13)) returned 4 instead of 3, and new BN(-7).divRound(new BN(2)) returned -3 instead of -4.

This compares the magnitude of the remainder against half of the absolute divisor and steps the quotient away from zero using the sign of the result, so rounding matches the positive case for every sign. Verified against BigInt over the full sign range. Adds a test covering the sign combinations.

spokodev and others added 4 commits July 8, 2026 14:55
divRound computed the half-way threshold and the round-up direction
from the raw operands, which only works when both are positive. With a
negative dividend or divisor it compared a signed remainder against a
value derived from the signed divisor, rounded in the wrong direction,
and lost the sign when the truncated quotient was zero.

Compare the magnitude of the remainder against half of the absolute
divisor and step the quotient away from zero using the sign of the
result, so rounding matches the positive case for every sign.
* fix: normalize sign when imuln multiplies by zero

Multiplying a negative number by the plain number 0 zeroed the words
but left the negative flag set, producing a negative zero. That value
reported isNeg() as true, was not eq() to zero, compared as less than
zero, and printed as -0. Clear the sign along with the length when the
factor is zero so the result is a plain zero.

* use _normSign()

---------

Co-authored-by: Kirill Fomichev <fanatid@ya.ru>
…bn.js into spokodev-fix/divround-negative-operands
@fanatid
fanatid merged commit 73fb9a4 into indutny:master Jul 9, 2026
2 checks passed
fanatid added a commit that referenced this pull request Jul 9, 2026
* fix: correct divRound rounding for negative operands

divRound computed the half-way threshold and the round-up direction
from the raw operands, which only works when both are positive. With a
negative dividend or divisor it compared a signed remainder against a
value derived from the signed divisor, rounded in the wrong direction,
and lost the sign when the truncated quotient was zero.

Compare the magnitude of the remainder against half of the absolute
divisor and step the quotient away from zero using the sign of the
result, so rounding matches the positive case for every sign.

* fix: normalize sign when imuln multiplies by zero (#321)

* fix: normalize sign when imuln multiplies by zero

Multiplying a negative number by the plain number 0 zeroed the words
but left the negative flag set, producing a negative zero. That value
reported isNeg() as true, was not eq() to zero, compared as less than
zero, and printed as -0. Clear the sign along with the length when the
factor is zero so the result is a plain zero.

* use _normSign()

---------

Co-authored-by: Kirill Fomichev <fanatid@ya.ru>

* add more tests

---------

Co-authored-by: Kirill Fomichev <fanatid@ya.ru>
(cherry picked from commit 73fb9a4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants