diff --git a/crates/float/src/lib.rs b/crates/float/src/lib.rs index 6bc060e6..5830f8a1 100644 --- a/crates/float/src/lib.rs +++ b/crates/float/src/lib.rs @@ -1510,7 +1510,7 @@ mod tests { assert!(matches!( err, - FloatError::DecimalFloat(DecimalFloatErrors::MulDivOverflow(_)) + FloatError::DecimalFloat(DecimalFloatErrors::DivisionByZero(_)) )); } diff --git a/src/error/ErrDecimalFloat.sol b/src/error/ErrDecimalFloat.sol index a1f7abe4..e91bb87a 100644 --- a/src/error/ErrDecimalFloat.sol +++ b/src/error/ErrDecimalFloat.sol @@ -33,3 +33,11 @@ error ZeroNegativePower(Float b); /// @dev Thrown when mulDiv internal to division overflows. error MulDivOverflow(uint256 x, uint256 y, uint256 denominator); + +/// @dev Thrown when a maximize overflows where it is not appropriate. +error MaximizeOverflow(int256 signedCoefficient, int256 exponent); + +/// @dev Thrown when dividing by zero. +/// @param signedCoefficient The signed coefficient of the numerator. +/// @param exponent The exponent of the numerator. +error DivisionByZero(int256 signedCoefficient, int256 exponent); diff --git a/src/lib/format/LibFormatDecimalFloat.sol b/src/lib/format/LibFormatDecimalFloat.sol index 255cf5cc..7acf1e6f 100644 --- a/src/lib/format/LibFormatDecimalFloat.sol +++ b/src/lib/format/LibFormatDecimalFloat.sol @@ -57,7 +57,7 @@ library LibFormatDecimalFloat { uint256 scaleExponent; uint256 scale = 0; if (scientific) { - (signedCoefficient, exponent) = LibDecimalFloatImplementation.maximize(signedCoefficient, exponent); + (signedCoefficient, exponent) = LibDecimalFloatImplementation.maximizeFull(signedCoefficient, exponent); bool isAtLeastE76 = signedCoefficient / 1e76 != 0; scaleExponent = isAtLeastE76 ? uint256(76) : uint256(75); diff --git a/src/lib/implementation/LibDecimalFloatImplementation.sol b/src/lib/implementation/LibDecimalFloatImplementation.sol index b944146a..0be38a18 100644 --- a/src/lib/implementation/LibDecimalFloatImplementation.sol +++ b/src/lib/implementation/LibDecimalFloatImplementation.sol @@ -2,7 +2,14 @@ // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd pragma solidity ^0.8.25; -import {ExponentOverflow, Log10Negative, Log10Zero, MulDivOverflow} from "../../error/ErrDecimalFloat.sol"; +import { + ExponentOverflow, + Log10Negative, + Log10Zero, + MulDivOverflow, + DivisionByZero, + MaximizeOverflow +} from "../../error/ErrDecimalFloat.sol"; import { LOG_TABLES, LOG_TABLES_SMALL, @@ -223,16 +230,21 @@ library LibDecimalFloatImplementation { function div(int256 signedCoefficientA, int256 exponentA, int256 signedCoefficientB, int256 exponentB) internal pure - returns (int256 signedCoefficient, int256 exponent) + returns (int256, int256) { - if (signedCoefficientA == 0 && signedCoefficientB != 0) { - signedCoefficient = MAXIMIZED_ZERO_SIGNED_COEFFICIENT; - exponent = MAXIMIZED_ZERO_EXPONENT; + if (signedCoefficientB == 0) { + revert DivisionByZero(signedCoefficientA, exponentA); + } else if (signedCoefficientA == 0) { + return (MAXIMIZED_ZERO_SIGNED_COEFFICIENT, MAXIMIZED_ZERO_EXPONENT); } else { + int256 signedCoefficient; + int256 exponent; + bool fullA; + bool fullB; // Move both coefficients into the e75/e76 range, so that the result // of division will not cause a mulDiv overflow. - (signedCoefficientA, exponentA) = maximize(signedCoefficientA, exponentA); - (signedCoefficientB, exponentB) = maximize(signedCoefficientB, exponentB); + (signedCoefficientA, exponentA, fullA) = maximize(signedCoefficientA, exponentA); + (signedCoefficientB, exponentB, fullB) = maximize(signedCoefficientB, exponentB); // mulDiv only works with unsigned integers, so get the absolute // values of the coefficients. @@ -249,25 +261,78 @@ library LibDecimalFloatImplementation { // fit in 256 bits by the division of a denominator that is larger // than the scale up. if (signedCoefficientBAbs < scale) { - scale = 1e75; - adjustExponent = 75; + if (fullB) { + scale = 1e75; + adjustExponent = 75; + } else { + // This is potentially quite a slow edge case. + while (signedCoefficientBAbs < scale) { + unchecked { + scale /= 10; + adjustExponent -= 1; + } + } + } + if (!fullA) { + revert MaximizeOverflow(signedCoefficientA, exponentA); + } } - // The order of subtraction matters in edge cases. For non-negative - // exponentA, apply the adjust exponent first to move the value - // towards 0 before exponentB is applied. This reduces the chance of - // a transient overflow in the intermediate subtraction. - if (exponentA >= 0) { - exponent = exponentA - adjustExponent - exponentB; - } else { - exponent = exponentA - exponentB - adjustExponent; + + // Attempt to apply the exponent adjustment. + // First we try to apply it to exponentA. + // If we cannot fully apply it we try to apply the rest to exponentB. + // If we still have some left over then we just return zero as + // the difference in exponents is too large to represent in + // a single result negative exponent. + unchecked { + if (exponentA >= type(int256).min + adjustExponent) { + exponentA -= adjustExponent; + } else { + adjustExponent -= exponentA - type(int256).min; + exponentA = type(int256).min; + + if (adjustExponent > 0) { + if (exponentB <= type(int256).max - adjustExponent) { + exponentB += adjustExponent; + } else { + return (MAXIMIZED_ZERO_SIGNED_COEFFICIENT, MAXIMIZED_ZERO_EXPONENT); + } + } + } } - (signedCoefficient, exponent) = unabsUnsignedMulOrDivLossy( - signedCoefficientA, - signedCoefficientB, - mulDiv(signedCoefficientAAbs, scale, signedCoefficientBAbs), - exponent - ); + int256 underflowExponentBy = 0; + + unchecked { + // This is the only case that can underflow. + if (exponentA < 0 && exponentB > 0) { + int256 headroom = exponentA - type(int256).min; + underflowExponentBy = exponentB > headroom ? exponentB - headroom : int256(0); + } + + exponent = exponentA + underflowExponentBy - exponentB; + + (signedCoefficient, exponent) = unabsUnsignedMulOrDivLossy( + signedCoefficientA, + signedCoefficientB, + mulDiv(signedCoefficientAAbs, scale, signedCoefficientBAbs), + exponent + ); + + if (underflowExponentBy > 0) { + if (underflowExponentBy > 76) { + // This means the exponent is too small to represent even if + // we truncate and downscale the signed coefficient. + return (MAXIMIZED_ZERO_SIGNED_COEFFICIENT, MAXIMIZED_ZERO_EXPONENT); + } + + signedCoefficient /= int256(10 ** uint256(underflowExponentBy)); + if (signedCoefficient == 0) { + exponent = MAXIMIZED_ZERO_EXPONENT; + } + } + return (signedCoefficient, exponent); + } } } @@ -440,8 +505,8 @@ library LibDecimalFloatImplementation { // Maximizing A and B gives us similar coefficients, which simplifies // detecting when their exponents are too far apart to add without // simply ignoring one of them. - (signedCoefficientA, exponentA) = maximize(signedCoefficientA, exponentA); - (signedCoefficientB, exponentB) = maximize(signedCoefficientB, exponentB); + (signedCoefficientA, exponentA) = maximizeFull(signedCoefficientA, exponentA); + (signedCoefficientB, exponentB) = maximizeFull(signedCoefficientB, exponentB); // We want A to represent the larger exponent. If this is not the case // then swap them. @@ -556,14 +621,17 @@ library LibDecimalFloatImplementation { { unchecked { { + int256 unmaximizedCoefficient = signedCoefficient; + int256 unmaximizedExponent = exponent; + (signedCoefficient, exponent) = maximizeFull(signedCoefficient, exponent); + if (signedCoefficient <= 0) { if (signedCoefficient == 0) { revert Log10Zero(); } else { - revert Log10Negative(signedCoefficient, exponent); + revert Log10Negative(unmaximizedCoefficient, unmaximizedExponent); } } - (signedCoefficient, exponent) = maximize(signedCoefficient, exponent); } // all powers of 10 look like 1 with a different exponent @@ -724,37 +792,40 @@ library LibDecimalFloatImplementation { } } - function maximize(int256 signedCoefficient, int256 exponent) internal pure returns (int256, int256) { + /// @return signedCoefficient The maximized signed coefficient. + /// @return exponent The maximized exponent. + /// @return full `true` if the result is fully maximized, `false` if it was + /// not possible to maximize without overflow. + function maximize(int256 signedCoefficient, int256 exponent) internal pure returns (int256, int256, bool) { unchecked { if (signedCoefficient == 0) { - return (MAXIMIZED_ZERO_SIGNED_COEFFICIENT, MAXIMIZED_ZERO_EXPONENT); + return (MAXIMIZED_ZERO_SIGNED_COEFFICIENT, MAXIMIZED_ZERO_EXPONENT, true); } - int256 initialExponent = exponent; // Check if already maximized before dropping into a block full of // jumps. if (signedCoefficient / 1e75 == 0) { - if (signedCoefficient / 1e38 == 0) { + if (signedCoefficient / 1e38 == 0 && exponent >= type(int256).min + 38) { signedCoefficient *= 1e38; exponent -= 38; } - if (signedCoefficient / 1e57 == 0) { + if (signedCoefficient / 1e57 == 0 && exponent >= type(int256).min + 19) { signedCoefficient *= 1e19; exponent -= 19; } - if (signedCoefficient / 1e66 == 0) { + if (signedCoefficient / 1e66 == 0 && exponent >= type(int256).min + 10) { signedCoefficient *= 1e10; exponent -= 10; } - while (signedCoefficient / 1e74 == 0) { + while (signedCoefficient / 1e74 == 0 && exponent >= type(int256).min + 2) { signedCoefficient *= 1e2; exponent -= 2; } - if (signedCoefficient / 1e75 == 0) { + if (signedCoefficient / 1e75 == 0 && exponent >= type(int256).min + 1) { signedCoefficient *= 10; exponent -= 1; } @@ -764,17 +835,21 @@ library LibDecimalFloatImplementation { // know until we try. This pushes us into [1e76,type(int256).max] and // [-type(int256).max,-1e76] ranges, if that's possible. int256 trySignedCoefficient = signedCoefficient * 10; - if (signedCoefficient == trySignedCoefficient / 10) { + if (signedCoefficient == trySignedCoefficient / 10 && exponent >= type(int256).min + 1) { signedCoefficient = trySignedCoefficient; exponent -= 1; } - if (initialExponent < exponent) { - revert ExponentOverflow(signedCoefficient, initialExponent); - } + return (signedCoefficient, exponent, signedCoefficient / 1e75 != 0); + } + } - return (signedCoefficient, exponent); + function maximizeFull(int256 signedCoefficient, int256 exponent) internal pure returns (int256, int256) { + (int256 trySignedCoefficient, int256 tryExponent, bool full) = maximize(signedCoefficient, exponent); + if (!full) { + revert MaximizeOverflow(signedCoefficient, exponent); } + return (trySignedCoefficient, tryExponent); } /// Rescale two floats so that they are possible to directly compare using diff --git a/test/src/lib/LibDecimalFloat.sub.t.sol b/test/src/lib/LibDecimalFloat.sub.t.sol index 79148f4a..e2c6e2da 100644 --- a/test/src/lib/LibDecimalFloat.sub.t.sol +++ b/test/src/lib/LibDecimalFloat.sub.t.sol @@ -22,16 +22,24 @@ contract LibDecimalFloatSubTest is Test { return LibDecimalFloat.sub(floatA, floatB); } + function packLossyExternal(int256 signedCoefficient, int256 exponent) external pure returns (Float, bool) { + return LibDecimalFloat.packLossy(signedCoefficient, exponent); + } + function testSubPacked(Float a, Float b) external { (int256 signedCoefficientA, int256 exponentA) = a.unpack(); (int256 signedCoefficientB, int256 exponentB) = b.unpack(); try this.subExternal(signedCoefficientA, exponentA, signedCoefficientB, exponentB) returns ( int256 signedCoefficient, int256 exponent ) { - Float float = this.subExternal(a, b); - (Float floatImplementation, bool lossless) = LibDecimalFloat.packLossy(signedCoefficient, exponent); - (lossless); - assertTrue(float.eq(floatImplementation)); + try this.packLossyExternal(signedCoefficient, exponent) returns (Float float, bool lossless) { + (lossless); + Float floatImplementation = this.subExternal(a, b); + assertTrue(float.eq(floatImplementation)); + } catch (bytes memory err) { + vm.expectRevert(err); + this.packLossyExternal(signedCoefficient, exponent); + } } catch (bytes memory err) { vm.expectRevert(err); this.subExternal(a, b); diff --git a/test/src/lib/implementation/LibDecimalFloatImplementation.add.t.sol b/test/src/lib/implementation/LibDecimalFloatImplementation.add.t.sol index c101c138..62216172 100644 --- a/test/src/lib/implementation/LibDecimalFloatImplementation.add.t.sol +++ b/test/src/lib/implementation/LibDecimalFloatImplementation.add.t.sol @@ -153,9 +153,9 @@ contract LibDecimalFloatImplementationAddTest is Test { vm.assume(signedCoefficientB != 0); (int256 normalizedSignedCoefficientA, int256 normalizedExponentA) = - LibDecimalFloatImplementation.maximize(signedCoefficientA, exponentA); + LibDecimalFloatImplementation.maximizeFull(signedCoefficientA, exponentA); (int256 expectedSignedCoefficient, int256 expectedExponent) = - LibDecimalFloatImplementation.maximize(signedCoefficientB, exponentB); + LibDecimalFloatImplementation.maximizeFull(signedCoefficientB, exponentB); vm.assume(normalizedSignedCoefficientA != 0); vm.assume(expectedSignedCoefficient != 0); @@ -251,8 +251,8 @@ contract LibDecimalFloatImplementationAddTest is Test { int256 exponentB; int256 signedCoefficientAMaximized; int256 signedCoefficientBMaximized; - (signedCoefficientAMaximized, exponentA) = LibDecimalFloatImplementation.maximize(signedCoefficientA, 0); - (signedCoefficientBMaximized, exponentB) = LibDecimalFloatImplementation.maximize(signedCoefficientB, 0); + (signedCoefficientAMaximized, exponentA) = LibDecimalFloatImplementation.maximizeFull(signedCoefficientA, 0); + (signedCoefficientBMaximized, exponentB) = LibDecimalFloatImplementation.maximizeFull(signedCoefficientB, 0); if (signedCoefficientA == 0 || signedCoefficientB == 0) { exponentA = 0; diff --git a/test/src/lib/implementation/LibDecimalFloatImplementation.div.t.sol b/test/src/lib/implementation/LibDecimalFloatImplementation.div.t.sol index 9ffa1ee6..bebf82be 100644 --- a/test/src/lib/implementation/LibDecimalFloatImplementation.div.t.sol +++ b/test/src/lib/implementation/LibDecimalFloatImplementation.div.t.sol @@ -7,7 +7,7 @@ import { LibDecimalFloatImplementation, EXPONENT_MIN, EXPONENT_MAX, - MulDivOverflow + DivisionByZero } from "src/lib/implementation/LibDecimalFloatImplementation.sol"; import {THREES, ONES} from "../../../lib/LibCommonResults.sol"; @@ -36,23 +36,14 @@ contract LibDecimalFloatImplementationDivTest is Test { function testDivZero(int256 signedCoefficient, int256 exponent) external { exponent = bound(exponent, type(int256).min / 2, type(int256).max); - (int256 signedCoefficientMaximized, int256 exponentMaximized) = - LibDecimalFloatImplementation.maximize(signedCoefficient, exponent); - if (signedCoefficient == 0) { - vm.expectRevert(stdError.divisionError); - } else { - vm.expectRevert( - abi.encodeWithSelector( - MulDivOverflow.selector, - LibDecimalFloatImplementation.absUnsignedSignedCoefficient(signedCoefficientMaximized), - 1e75, - 0 - ) - ); - } + vm.expectRevert(abi.encodeWithSelector(DivisionByZero.selector, signedCoefficient, exponent)); this.divExternal(signedCoefficient, exponent, 0, 0); } + function testDivMaxPositiveValueDenominatorNotRevert(int256 signedCoefficient, int256 exponent) external pure { + LibDecimalFloatImplementation.div(signedCoefficient, exponent, type(int256).max, type(int32).max); + } + /// 1 / 3 gas by parts 10 function testDiv1Over3Gas10() external pure { (int256 c, int256 e) = LibDecimalFloatImplementation.div(1, 0, 3e37, -37); @@ -132,7 +123,7 @@ contract LibDecimalFloatImplementationDivTest is Test { function testDivBy1(int256 signedCoefficient, int256 exponent) external pure { exponent = bound(exponent, type(int256).min + 76, type(int256).max); (int256 expectedCoefficient, int256 expectedExponent) = - LibDecimalFloatImplementation.maximize(signedCoefficient, exponent); + LibDecimalFloatImplementation.maximizeFull(signedCoefficient, exponent); int256 one = 1; for (int256 oneExponent = 0; oneExponent >= -76; --oneExponent) { @@ -147,7 +138,7 @@ contract LibDecimalFloatImplementationDivTest is Test { function testDivByNegativeOneFloat(int256 signedCoefficient, int256 exponent) external pure { exponent = bound(exponent, type(int256).min + 76, type(int256).max - 1); (int256 expectedCoefficient, int256 expectedExponent) = - LibDecimalFloatImplementation.maximize(signedCoefficient, exponent); + LibDecimalFloatImplementation.maximizeFull(signedCoefficient, exponent); (expectedCoefficient, expectedExponent) = LibDecimalFloatImplementation.minus(expectedCoefficient, expectedExponent); diff --git a/test/src/lib/implementation/LibDecimalFloatImplementation.inv.t.sol b/test/src/lib/implementation/LibDecimalFloatImplementation.inv.t.sol index b08d6e97..43269ef2 100644 --- a/test/src/lib/implementation/LibDecimalFloatImplementation.inv.t.sol +++ b/test/src/lib/implementation/LibDecimalFloatImplementation.inv.t.sol @@ -8,7 +8,7 @@ import { LibDecimalFloatImplementation, EXPONENT_MIN, EXPONENT_MAX, - MulDivOverflow + DivisionByZero } from "src/lib/implementation/LibDecimalFloatImplementation.sol"; contract LibDecimalFloatImplementationInvTest is Test { @@ -42,7 +42,7 @@ contract LibDecimalFloatImplementationInvTest is Test { } function testInv0() external { - vm.expectRevert(abi.encodeWithSelector(MulDivOverflow.selector, 1e76, 1e75, 0)); + vm.expectRevert(abi.encodeWithSelector(DivisionByZero.selector, 1e76, -76)); this.invExternal(0, 0); } } diff --git a/test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol b/test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol index 15e06504..73be8237 100644 --- a/test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol +++ b/test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol @@ -34,7 +34,7 @@ contract LibDecimalFloatImplementationMaximizeTest is Test { function testMaximizedEverything(int256 signedCoefficient, int256 exponent) external pure { exponent = bound(exponent, EXPONENT_MIN, EXPONENT_MAX); (int256 actualSignedCoefficient, int256 actualExponent) = - LibDecimalFloatImplementation.maximize(signedCoefficient, exponent); + LibDecimalFloatImplementation.maximizeFull(signedCoefficient, exponent); assertTrue(isMaximized(actualSignedCoefficient, actualExponent)); } @@ -45,7 +45,7 @@ contract LibDecimalFloatImplementationMaximizeTest is Test { int256 expectedExponent ) internal pure { (int256 actualSignedCoefficient, int256 actualExponent) = - LibDecimalFloatImplementation.maximize(signedCoefficient, exponent); + LibDecimalFloatImplementation.maximizeFull(signedCoefficient, exponent); assertEq(actualSignedCoefficient, expectedCoefficient); assertEq(actualExponent, expectedExponent); } @@ -75,9 +75,9 @@ contract LibDecimalFloatImplementationMaximizeTest is Test { function testMaximizedIdempotent(int256 signedCoefficient, int256 exponent) external pure { exponent = bound(exponent, EXPONENT_MIN, EXPONENT_MAX); (int256 maximizedSignedCoefficient, int256 maximizedExponent) = - LibDecimalFloatImplementation.maximize(signedCoefficient, exponent); + LibDecimalFloatImplementation.maximizeFull(signedCoefficient, exponent); (int256 actualSignedCoefficient, int256 actualExponent) = - LibDecimalFloatImplementation.maximize(maximizedSignedCoefficient, maximizedExponent); + LibDecimalFloatImplementation.maximizeFull(maximizedSignedCoefficient, maximizedExponent); assertEq(actualSignedCoefficient, maximizedSignedCoefficient); assertEq(actualExponent, maximizedExponent); } @@ -86,7 +86,7 @@ contract LibDecimalFloatImplementationMaximizeTest is Test { function testMaximizedReference(int256 signedCoefficient, int256 exponent) external pure { exponent = bound(exponent, EXPONENT_MIN, EXPONENT_MAX); (int256 actualSignedCoefficient, int256 actualExponent) = - LibDecimalFloatImplementation.maximize(signedCoefficient, exponent); + LibDecimalFloatImplementation.maximizeFull(signedCoefficient, exponent); (int256 expectedSignedCoefficient, int256 expectedExponent) = LibDecimalFloatSlow.maximizeSlow(signedCoefficient, exponent); assertEq(actualSignedCoefficient, expectedSignedCoefficient);