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 15f20924670..b5d28af181d 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java @@ -992,7 +992,7 @@ private QR256 mulSub(final long v3, final long v2, final long v1, final long v0, return new QR256(q, new UInt256(z3, z2, z1, z0)); } - private UInt256 mulSubOverflow(final long v3, final long v2, final long v1, final long v0) { + private QR256 mulSubOverflow(final long v3, final long v2, final long v1, final long v0) { // Overflow case: div2by1 quotient would be <1, 0>, but adjusts to <0, -1> // = -1 * u0 = long res, borrow; @@ -1011,21 +1011,22 @@ 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) + // q = MAX may still be 1 too high - need to pass q - 1 (-2L) to addBack; 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, 1L).r; + return addBack(z3, z2, z1, z0, -2L); } - return new UInt256(z3, z2, z1, z0); + return new QR256(-1L, new UInt256(z3, z2, z1, z0)); } private QR256 reduceStep( final long v4, final long v3, final long v2, final long v1, final long v0, final long inv) { - if (v4 == u3) return new QR256(-1L, mulSubOverflow(v3, v2, v1, v0)); + if (v4 == u3) return mulSubOverflow(v3, v2, v1, v0); QR64 qr = div2by1(v4, v3, u3, inv); if (qr.q != 0) return mulSub(qr.r, v2, v1, v0, qr.q); return new QR256(0, new UInt256(v3, v2, v1, v0)); @@ -1175,8 +1176,10 @@ private static QR64 div2by1(final long x1, final long x0, final long y, final lo // -------------------------------------------------------------------------- // endregion - // region Records - // -------------------------------------------------------------------------- + // region Quotient / Remainder types + // These types are used to store the result of division. Due to the nature of the algorithm + // (div2by1) quotient (q) can't + // ever exceed 64 bits while remainder (r) can range from 64 to 256 bits. private record QR64(long q, long r) {} private record QR128(long q, UInt128 r) {} @@ -1185,6 +1188,11 @@ private record QR192(long q, UInt192 r) {} private record QR256(long q, UInt256 r) {} + // -------------------------------------------------------------------------- + // endregion + + // region UInt* types + // -------------------------------------------------------------------------- record UInt64(long u0) { UInt64 shiftLeft(final int shift) { return (shift == 0) ? this : new UInt64(u0 << shift); @@ -1414,22 +1422,23 @@ private QR128 mulSub(final long x1, final long x0, final long q) { return new QR128(q, new UInt128(z1, z0)); } - private UInt128 mulSubOverflow(final long v1, final long v0) { + private QR128 mulSubOverflow(final long v1, final long v0) { // Overflow case: div2by1 quotient would be <1, 0>, but adjusts to <0, MAX> // = -1 * u0 = long z0 = v0 + u0; 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) + // q = MAX may still be 1 too high - need to pass q - 1 (-2L) to addBack; 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, 1L).r; + return addBack(z1, z0, -2L); } - return new UInt128(z1, z0); + return new QR128(-1L, new UInt128(z1, z0)); } private QR128 reduceStep(final long v2, final long v1, final long v0, final long inv) { - if (v2 == u1) return new QR128(-1L, mulSubOverflow(v1, v0)); + if (v2 == u1) return mulSubOverflow(v1, v0); QR64 qr = div2by1(v2, v1, u1, inv); if (qr.q != 0) return mulSub(qr.r, v0, qr.q); return new QR128(0, new UInt128(qr.r, v0)); @@ -1640,7 +1649,7 @@ private QR192 mulSub(final long v2, final long v1, final long v0, final long q) return new QR192(q, new UInt192(z2, z1, z0)); } - private UInt192 mulSubOverflow(final long v2, final long v1, final long v0) { + private QR192 mulSubOverflow(final long v2, final long v1, final long v0) { // Overflow case: div2by1 quotient would be <1, 0>, but adjusts to <0, -1> // = -1 * u0 = long z0 = v0 + u0; @@ -1652,19 +1661,21 @@ 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) + // q = MAX may still be 1 too high - need to pass q - 1 (-2L) to addBack; 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, 1L).r; + + return addBack(z2, z1, z0, -2L); } - return new UInt192(z2, z1, z0); + return new QR192(-1L, new UInt192(z2, z1, z0)); } private QR192 reduceStep( final long v3, final long v2, final long v1, final long v0, final long inv) { - if (v3 == u2) return new QR192(-1L, mulSubOverflow(v2, v1, v0)); + if (v3 == u2) return mulSubOverflow(v2, v1, v0); QR64 qr = div2by1(v3, v2, u2, inv); if (qr.q != 0) return mulSub(qr.r, v1, v0, qr.q); return new QR192(0, new UInt192(v2, v1, v0)); diff --git a/evm/src/test/java/org/hyperledger/besu/evm/UInt256PropertyBasedTest.java b/evm/src/test/java/org/hyperledger/besu/evm/UInt256PropertyBasedTest.java index b2e352b0e5d..e373ebaa58e 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/UInt256PropertyBasedTest.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/UInt256PropertyBasedTest.java @@ -87,6 +87,7 @@ Arbitrary bytes0to64_shaped() { Tuple.of(4, Arbitraries.of(Pattern.ALL_ZERO)), Tuple.of(4, Arbitraries.of(Pattern.ALL_FF)), Tuple.of(4, Arbitraries.of(Pattern.LIMB_SIGN_BITS)), + Tuple.of(4, Arbitraries.of(Pattern.FIXED_TOP_LIMBS)), Tuple.of(10, Arbitraries.of(Pattern.RANDOM))); return lengths.flatMap( @@ -129,29 +130,38 @@ private enum Pattern { ALL_ZERO, ALL_FF, LIMB_SIGN_BITS, + FIXED_TOP_LIMBS, RANDOM } private static byte[] applyPattern(final byte[] bytes, final Pattern pat) { - switch (pat) { - case ALL_ZERO: + return switch (pat) { + case ALL_ZERO -> { Arrays.fill(bytes, (byte) 0x00); - return bytes; - case ALL_FF: + yield bytes; + } + case ALL_FF -> { Arrays.fill(bytes, (byte) 0xFF); - return bytes; - case LIMB_SIGN_BITS: + yield bytes; + } + case LIMB_SIGN_BITS -> { Arrays.fill(bytes, (byte) 0x00); forceMsbAtIndexIfPresent(bytes, 0); forceMsbAtIndexIfPresent(bytes, 8); forceMsbAtIndexIfPresent(bytes, 16); forceMsbAtIndexIfPresent(bytes, 24); forceMsbAtIndexIfPresent(bytes, bytes.length - 1); - return bytes; - case RANDOM: - default: - return bytes; - } + yield bytes; + } + case FIXED_TOP_LIMBS -> { + int size = (int) Math.ceil(bytes.length / 8D) * 8; + final byte[] newArray = new byte[size]; + System.arraycopy(bytes, 0, newArray, size - bytes.length, bytes.length); + Arrays.fill(newArray, 0, 8, (byte) 0x80); + yield newArray; + } + case RANDOM -> bytes; + }; } private static void forceMsbAtIndexIfPresent(final byte[] bytes, final int index) { 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 0a29d8d2d24..9dd578f21f2 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java @@ -601,7 +601,19 @@ static Collection testCases() { "0x0700b2d7adda7612da7f95" }, {"0xbf1256135bb3f72de074d0f237", "0x8b63235ac1765530"}, - {"0x5b35862b0027a502b1d4cbc4a09e25", "0x932542f4003763"} + {"0x5b35862b0027a502b1d4cbc4a09e25", "0x932542f4003763"}, + { + // Multiply and subtract overflows and we need to decrement quotient estimation - + // UInt192 case + "0x8200000000000000000000000000000000000000000000000000000000000000", + "0x8200000000000000fe000004000000ffff000000fffff700" + }, + { + // Multiply and subtract overflows and we need to decrement quotient estimation - + // UInt128 case + "0x820000000000000000000000000000000000000000000000", + "0x8200000000000000fe00000000000001" + }, }) .flatMap( inputs ->