diff --git a/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java b/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java index b93388b7a3b..83df96c3af1 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java @@ -1226,6 +1226,10 @@ private UInt128 mulSubOverflow(final long v1, final long v0) { long carry = u0 - 1 + ((Long.compareUnsigned(v0, z0) <= 0) ? 1 : 0); long z1 = v1 + u1 - carry; + // q = MAX may still be 1 too high; check if result >= modulus (i.e. negative wrapped) + if (Long.compareUnsigned(z1, u1) > 0 || (z1 == u1 && Long.compareUnsigned(z0, u0) >= 0)) { + return addBack(z1, z0); + } return new UInt128(z1, z0); } @@ -1427,6 +1431,13 @@ private UInt192 mulSubOverflow(final long v2, final long v1, final long v0) { carry = u1 - 1 + ((Long.compareUnsigned(v1, res) < 0) ? 1 : 0); long z2 = v2 - carry + u2 - borrow; + // q = MAX may still be 1 too high; check if result >= modulus (i.e. negative wrapped) + if (Long.compareUnsigned(z2, u2) > 0 + || (z2 == u2 + && (Long.compareUnsigned(z1, u1) > 0 + || (z1 == u1 && Long.compareUnsigned(z0, u0) >= 0)))) { + return addBack(z2, z1, z0); + } return new UInt192(z2, z1, z0); } @@ -1439,26 +1450,39 @@ private UInt192 reduceStep( } private UInt256 reduceNormalised(final UInt256 that, final int shift, final long inv) { - UInt192 r; UInt320 v = that.shiftLeftWide(shift); - if (v.u4 != 0 || Long.compareUnsigned(v.u3, u2) >= 0) { - r = reduceStep(v.u4, v.u3, v.u2, v.u1, inv); - r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); - } else { - r = reduceStep(v.u3, v.u2, v.u1, v.u0, inv); + if (Long.compareUnsigned(v.u4, u2) < 0) { + UInt192 r; + if (v.u4 != 0 || Long.compareUnsigned(v.u3, u2) >= 0) { + r = reduceStep(v.u4, v.u3, v.u2, v.u1, inv); + r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); + } else { + r = reduceStep(v.u3, v.u2, v.u1, v.u0, inv); + } + return new UInt256(0, r.u2, r.u1, r.u0).shiftRight(shift); } - return new UInt256(0, r.u2, r.u1, r.u0).shiftRight(shift); + return reduceNormalisedSlowPath(v, shift, inv); } private UInt256 reduceNormalised(final UInt257 that, final int shift, final long inv) { - UInt192 r; UInt320 v = that.shiftLeftWide(shift); - if (v.u4 != 0 || Long.compareUnsigned(v.u3, u2) >= 0) { - r = reduceStep(v.u4, v.u3, v.u2, v.u1, inv); - r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); - } else { - r = reduceStep(v.u3, v.u2, v.u1, v.u0, inv); + if (Long.compareUnsigned(v.u4, u2) < 0) { + UInt192 r; + if (v.u4 != 0 || Long.compareUnsigned(v.u3, u2) >= 0) { + r = reduceStep(v.u4, v.u3, v.u2, v.u1, inv); + r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); + } else { + r = reduceStep(v.u3, v.u2, v.u1, v.u0, inv); + } + return new UInt256(0, r.u2, r.u1, r.u0).shiftRight(shift); } + return reduceNormalisedSlowPath(v, shift, inv); + } + + private UInt256 reduceNormalisedSlowPath(final UInt320 v, final int shift, final long inv) { + UInt192 r = reduceStep(0, v.u4, v.u3, v.u2, inv); + r = reduceStep(r.u2, r.u1, r.u0, v.u1, inv); + r = reduceStep(r.u2, r.u1, r.u0, v.u0, inv); return new UInt256(0, r.u2, r.u1, r.u0).shiftRight(shift); } @@ -1660,6 +1684,15 @@ private UInt256 mulSubOverflow(final long v3, final long v2, final long v1, fina carry = u2 - 1 + ((Long.compareUnsigned(v2, res) < 0) ? 1 : 0); long z3 = v3 + u3 - carry - borrow; + // q = MAX may still be 1 too high; check if result >= modulus (i.e. negative wrapped) + if (Long.compareUnsigned(z3, u3) > 0 + || (z3 == u3 + && (Long.compareUnsigned(z2, u2) > 0 + || (z2 == u2 + && (Long.compareUnsigned(z1, u1) > 0 + || (z1 == u1 && Long.compareUnsigned(z0, u0) >= 0)))))) { + return addBack(z3, z2, z1, z0); + } return new UInt256(z3, z2, z1, z0); } diff --git a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java index fa9db2b6b23..e296cf22085 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java @@ -544,6 +544,69 @@ public void mulMod() { } } + @Test + public void addModTestReduceNormalisedTopLimb() { + UInt256 a = + UInt256.fromBytesBE( + new BigInteger("62d900c9700000000000000000023f00bc1814ff00000000000000ca22300806", 16) + .toByteArray()); + UInt256 b = + UInt256.fromBytesBE( + new BigInteger("ffffffffffffffffb4fffff4befff4f4f4d4f4f504f4f4bef5f5100b0bf4f5f6", 16) + .toByteArray()); + UInt256 m = + UInt256.fromBytesBE(new BigInteger("13464637e8bdc0e53b895d7b79348a784", 16).toByteArray()); + BigInteger A = new BigInteger(1, a.toBytesBE()); + BigInteger B = new BigInteger(1, b.toBytesBE()); + BigInteger M = new BigInteger(1, m.toBytesBE()); + BigInteger expected = A.add(B).mod(M); + assertThat(new BigInteger(1, a.addMod(b, m).toBytesBE())).isEqualTo(expected); + } + + @Test + public void mulSubOverflowWithAddBackBug() { + // When the dividend's leading limb equals the modulus's leading limb, the trial quotient + // overflows and is clamped to 2^64-1. Verify correctness for each Modulus size. + + // Modulus192 path (b.u3==0, b.u2!=0) + UInt256 a1 = + UInt256.fromBytesBE( + new BigInteger("7effffff8000000000000000000000000000000000000000d900000000000001", 16) + .toByteArray()); + UInt256 b1 = + UInt256.fromBytesBE( + new BigInteger("7effffff800000007effffff800000008000ff0000010000", 16).toByteArray()); + BigInteger expected1 = new BigInteger("7effffff800000007dff00feffff0001d901fe0000020001", 16); + assertThat(new BigInteger(1, a1.mod(b1).toBytesBE())).isEqualTo(expected1); + + // Modulus128 path (b.u3==0, b.u2==0, b.u1!=0) + UInt256 a2 = + UInt256.fromBytesBE( + new BigInteger("7effffff800000000000000000000000d900000000000001", 16).toByteArray()); + UInt256 b2 = + UInt256.fromBytesBE(new BigInteger("7effffff800000007fffffffffffffff", 16).toByteArray()); + BigInteger aBI2 = new BigInteger(1, a2.toBytesBE()); + BigInteger bBI2 = new BigInteger(1, b2.toBytesBE()); + BigInteger expected2 = aBI2.mod(bBI2); + assertThat(new BigInteger(1, a2.mod(b2).toBytesBE())).isEqualTo(expected2); + + // Modulus256 path (b.u3!=0) via mulMod + UInt256 a3 = + UInt256.fromBytesBE( + new BigInteger("7effffff8000000000000000000000000000000000000000d900000000000001", 16) + .toByteArray()); + UInt256 x3 = UInt256.fromBytesBE(new BigInteger("10000000000000000", 16).toByteArray()); // 2^64 + UInt256 m3 = + UInt256.fromBytesBE( + new BigInteger("7effffff800000007effffff800000008000ff00000100007effffff80000000", 16) + .toByteArray()); + BigInteger aBI3 = new BigInteger(1, a3.toBytesBE()); + BigInteger xBI3 = new BigInteger(1, x3.toBytesBE()); + BigInteger mBI3 = new BigInteger(1, m3.toBytesBE()); + BigInteger expected3 = aBI3.multiply(xBI3).mod(mBI3); + assertThat(new BigInteger(1, a3.mulMod(x3, m3).toBytesBE())).isEqualTo(expected3); + } + @Test public void signedMod() { final Random random = new Random(432);