fix: correct divRound rounding for negative operands (#320)

* 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)

Author: spokodev

lib/bn.js

@@ -2469,17 +2469,19 @@
     // Fast case - exact division
     if (dm.mod.isZero()) return dm.div;
 
-    var mod = dm.div.negative !== 0 ? dm.mod.isub(num) : dm.mod;
+    var mod = dm.mod.abs();
 
-    var half = num.ushrn(1);
-    var r2 = num.andln(1);
+    var half = num.abs().iushrn(1);
+    var r2 = num.words[0] & 1;
     var cmp = mod.cmp(half);
 
     // Round down
     if (cmp < 0 || r2 === 1 && cmp === 0) return dm.div;
 
-    // Round up
-    return dm.div.negative !== 0 ? dm.div.isubn(1) : dm.div.iaddn(1);
+    // Round up, away from zero
+    var up = new BN(1);
+    up.negative = this.negative ^ num.negative;
+    return dm.div.iadd(up);
   };
 
   BN.prototype.modn = function modn (num) {

test/arithmetic-test.js

@@ -443,8 +443,70 @@ describe('BN.js/Arithmetic', function () {
         '-8');
     });
 
+    it('should round with negative operands', function () {
+      assert.equal(new BN(-40).divRound(new BN(-13)).toString(10), '3');
+      assert.equal(new BN(40).divRound(new BN(-13)).toString(10), '-3');
+      assert.equal(new BN(-40).divRound(new BN(13)).toString(10), '-3');
+      assert.equal(new BN(-30).divRound(new BN(40)).toString(10), '-1');
+      assert.equal(new BN(30).divRound(new BN(-40)).toString(10), '-1');
+      assert.equal(new BN(-7).divRound(new BN(2)).toString(10), '-4');
+      assert.equal(new BN(7).divRound(new BN(-2)).toString(10), '-4');
+      assert.equal(new BN(-25).divRound(new BN(10)).toString(10), '-3');
+      // odd divisor, remainder exactly floor(|divisor| / 2): round toward zero
+      assert.equal(new BN(-4).divRound(new BN(3)).toString(10), '-1');
+      assert.equal(new BN(4).divRound(new BN(-3)).toString(10), '-1');
+      assert.equal(new BN(-7).divRound(new BN(5)).toString(10), '-1');
+      assert.equal(new BN(7).divRound(new BN(-5)).toString(10), '-1');
+    });
+
+    it('should round values smaller than the divisor', function () {
+      assert.equal(new BN(4).divRound(new BN(10)).toString(10), '0');
+      assert.equal(new BN(-4).divRound(new BN(10)).toString(10), '0');
+      assert.equal(new BN(4).divRound(new BN(-10)).toString(10), '0');
+      assert.equal(new BN(6).divRound(new BN(10)).toString(10), '1');
+      assert.equal(new BN(-6).divRound(new BN(10)).toString(10), '-1');
+      assert.equal(new BN(6).divRound(new BN(-10)).toString(10), '-1');
+    });
+
+    it('should round with large multi-word operands', function () {
+      assert.equal(
+        new BN('123456789012345678901234567890')
+          .divRound(new BN('98765432109876543210')).toString(10),
+        '1249999989');
+      assert.equal(
+        new BN('-123456789012345678901234567890')
+          .divRound(new BN('98765432109876543210')).toString(10),
+        '-1249999989');
+      assert.equal(
+        new BN('123456789012345678901234567890')
+          .divRound(new BN('-98765432109876543210')).toString(10),
+        '-1249999989');
+      assert.equal(
+        new BN('-123456789012345678901234567890')
+          .divRound(new BN('-98765432109876543210')).toString(10),
+        '1249999989');
+    });
+
+    it('should round large multi-word ties away from zero', function () {
+      assert.equal(
+        new BN('3048315750647767350192043944016156078651272672090')
+          .divRound(new BN('246913578024691357802469135780')).toString(10),
+        '12345678901234567891');
+      assert.equal(
+        new BN('-3048315750647767350192043944016156078651272672090')
+          .divRound(new BN('246913578024691357802469135780')).toString(10),
+        '-12345678901234567891');
+      assert.equal(
+        new BN('3048315750647767350192043944016156078651272672090')
+          .divRound(new BN('-246913578024691357802469135780')).toString(10),
+        '-12345678901234567891');
+    });
+
     it('should return 1 on exact division', function () {
       assert.equal(new BN(144).divRound(new BN(144)).toString(10), '1');
+      assert.equal(new BN(-144).divRound(new BN(144)).toString(10), '-1');
+      assert.equal(new BN(144).divRound(new BN(-144)).toString(10), '-1');
+      assert.equal(new BN(-144).divRound(new BN(-144)).toString(10), '1');
     });
   });