From d1650af18cddaa8b752d86ca2b59266976af5644 Mon Sep 17 00:00:00 2001 From: David Meister Date: Fri, 25 Sep 2026 13:29:03 +0000 Subject: [PATCH 1/8] feat: agree, a closeness test with an absolute and a proportional tolerance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agree(absolute, proportional, lowest, highest)` is highest - lowest <= max(absolute, proportional * max(abs(lowest), abs(highest))) It belongs here rather than in a consumer because it cannot be composed from the public surface. That surface reverts ExponentOverflow rather than truncating an exponent, and both `abs` and `sub` do so on the extremes: `abs(min-negative-value)` cannot fit the magnitude back into an int224 with the exponent already at its maximum, and the spread from the most negative to the most positive representable value fits no packed value at all. A closeness test asked about representable values should answer, not revert, so it has to work below that surface. Doing that needed three primitives the implementation library did not have. `eq` was the only comparison on unpacked values, so callers staying below the public surface had to reach for `compareRescale` and compare its results by hand: - `abs` / `absCoefficient` — magnitude without repacking, exact because an unpacked coefficient is an int224 widened to an int256. - `lte` — comparison without repacking. - `max` — the larger of two, without repacking. On the design: both tolerances are taken and the LARGER of the two terms is the limit. A proportional tolerance alone collapses as the values approach zero, since the quantity it is a proportion of shrinks with them; an absolute tolerance alone does not scale. Taking the larger rather than the sum is the form math.isclose (PEP 485) and Julia's isapprox use; numpy.isclose sums them, which PEP 485 rejects because two tolerances of similar size then allow about twice the intended difference. The proportion is of the larger magnitude of the two extremes. Every other value in a set lies between them, so that is the largest magnitude in the whole set, and holding one anchor for the set is what makes a single highest-to-lowest check equivalent to checking every pair. `agree` is split into private helpers because one frame holding four unpacked values and the intermediates exceeds the stack. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibDecimalFloat.sol | 106 +++++++++++++++ .../LibDecimalFloatImplementation.sol | 65 +++++++++ test/src/lib/LibDecimalFloat.agree.t.sol | 126 ++++++++++++++++++ 3 files changed, 297 insertions(+) create mode 100644 test/src/lib/LibDecimalFloat.agree.t.sol diff --git a/src/lib/LibDecimalFloat.sol b/src/lib/LibDecimalFloat.sol index 6d0cb0b1..51c04826 100644 --- a/src/lib/LibDecimalFloat.sol +++ b/src/lib/LibDecimalFloat.sol @@ -860,6 +860,112 @@ library LibDecimalFloat { return gt(a, b) ? a : b; } + /// Whether the two extremes of a set of values are close enough to each + /// other, given an absolute and a proportional tolerance. + /// + /// `highest - lowest <= max(absolute, proportional * max(abs(lowest), abs(highest)))` + /// + /// BOTH TOLERANCES ARE TAKEN, and the LARGER of the two terms is the + /// limit. A proportional tolerance alone collapses as the values approach + /// zero, because the quantity it is a proportion of shrinks with them: a + /// pair like `-0.001` and `0.001` reads as 200% apart while agreeing by + /// any practical measure. An absolute tolerance alone does not scale. The + /// absolute term therefore carries the region near zero and the + /// proportional term carries the rest. + /// + /// Taking the larger rather than the sum is the form `math.isclose` + /// (PEP 485) and Julia's `isapprox` use. `numpy.isclose` sums them, which + /// PEP 485 rejects because two tolerances of similar size then allow about + /// twice the intended difference. The sum is also the more permissive of + /// the two, since `max(a, b) <= a + b` for non-negative terms. + /// + /// THE PROPORTION IS OF THE LARGER MAGNITUDE of the two extremes. Every + /// other value in a set lies between them, so no value in the set has a + /// magnitude exceeding both, which makes this the largest magnitude in the + /// whole set. Holding one anchor for the set is what makes a single + /// highest-to-lowest check equivalent to checking every pair: the spread + /// is the largest pairwise difference, so bounding it bounds all of them. + /// + /// NOTHING IS PACKED BACK INTO A `Float` before the comparison, which is + /// the reason this cannot be composed from the public surface. That + /// surface reverts `ExponentOverflow` rather than truncating an exponent, + /// and both `abs` and `sub` do so on the extremes of the range: a set + /// spanning the most negative to the most positive representable value has + /// a spread that no packed value can hold. A closeness test asked about + /// representable values should answer, not revert. + /// + /// The caller is responsible for the tolerances being sensible. A negative + /// tolerance is simply dominated by the other term, and two zero + /// tolerances make this an exact equality test. + /// @param absolute The absolute tolerance, in the same units as the values. + /// @param proportional The proportional tolerance, as a fraction. + /// @param lowest The lowest value in the set. + /// @param highest The highest value in the set. + /// @return Whether the spread is within the limit. + function agree(Float absolute, Float proportional, Float lowest, Float highest) internal pure returns (bool) { + (int256 spreadCoefficient, int256 spreadExponent) = agreeSpread(lowest, highest); + (int256 limitCoefficient, int256 limitExponent) = agreeLimit(absolute, proportional, lowest, highest); + return LibDecimalFloatImplementation.lte(spreadCoefficient, spreadExponent, limitCoefficient, limitExponent); + } + + /// The distance between the two extremes, unpacked. + /// + /// Split out of `agree` because holding the four unpacked values and the + /// intermediates in one frame exceeds the stack. + /// @param lowest The lowest value. + /// @param highest The highest value. + /// @return The spread's coefficient. + /// @return The spread's exponent. + function agreeSpread(Float lowest, Float highest) private pure returns (int256, int256) { + (int256 lowestCoefficient, int256 lowestExponent) = lowest.unpack(); + (int256 highestCoefficient, int256 highestExponent) = highest.unpack(); + return LibDecimalFloatImplementation.sub(highestCoefficient, highestExponent, lowestCoefficient, lowestExponent); + } + + /// The quantity the proportional tolerance is taken of: the larger + /// magnitude of the two extremes. + /// @param lowest The lowest value. + /// @param highest The highest value. + /// @return The anchor's coefficient, non-negative. + /// @return The anchor's exponent. + function agreeAnchor(Float lowest, Float highest) private pure returns (int256, int256) { + (int256 lowestCoefficient, int256 lowestExponent) = lowest.unpack(); + (int256 highestCoefficient, int256 highestExponent) = highest.unpack(); + return LibDecimalFloatImplementation.max( + LibDecimalFloatImplementation.absCoefficient(lowestCoefficient), + lowestExponent, + LibDecimalFloatImplementation.absCoefficient(highestCoefficient), + highestExponent + ); + } + + /// The limit the spread is checked against: the larger of the absolute + /// tolerance and the proportional tolerance of the anchor. + /// @param absolute The absolute tolerance. + /// @param proportional The proportional tolerance. + /// @param lowest The lowest value. + /// @param highest The highest value. + /// @return The limit's coefficient. + /// @return The limit's exponent. + function agreeLimit(Float absolute, Float proportional, Float lowest, Float highest) + private + pure + returns (int256, int256) + { + int256 scaledCoefficient; + int256 scaledExponent; + { + (int256 anchorCoefficient, int256 anchorExponent) = agreeAnchor(lowest, highest); + (int256 proportionalCoefficient, int256 proportionalExponent) = proportional.unpack(); + (scaledCoefficient, scaledExponent) = LibDecimalFloatImplementation.mul( + proportionalCoefficient, proportionalExponent, anchorCoefficient, anchorExponent + ); + } + (int256 absoluteCoefficient, int256 absoluteExponent) = absolute.unpack(); + return + LibDecimalFloatImplementation.max(absoluteCoefficient, absoluteExponent, scaledCoefficient, scaledExponent); + } + /// Returns true if the float is zero. Handles the case where the signed /// coefficient is zero and exponent is potentially non zero. /// @param a The float to check. diff --git a/src/lib/implementation/LibDecimalFloatImplementation.sol b/src/lib/implementation/LibDecimalFloatImplementation.sol index af61ed77..7ea4b351 100644 --- a/src/lib/implementation/LibDecimalFloatImplementation.sol +++ b/src/lib/implementation/LibDecimalFloatImplementation.sol @@ -1136,6 +1136,71 @@ library LibDecimalFloatImplementation { } } + /// The magnitude of an unpacked value, as an unpacked value. + /// + /// The packed `LibDecimalFloat.abs` cannot serve callers working below the + /// public arithmetic surface. It has to fit the magnitude back into an + /// int224, so for the most negative coefficient it raises the exponent, + /// and reverts `ExponentOverflow` when the exponent is already at its + /// maximum. Here the coefficient is already widened to an int256, so + /// negating an int224 is exact and cannot overflow. + /// @param signedCoefficient The coefficient, within int224. + /// @param exponent The exponent, unchanged by taking a magnitude. + /// @return The non-negative coefficient. + /// @return The exponent. + function abs(int256 signedCoefficient, int256 exponent) internal pure returns (int256, int256) { + return (absCoefficient(signedCoefficient), exponent); + } + + /// The magnitude of an unpacked coefficient. The exponent is untouched by + /// taking a magnitude, so callers that already hold it can skip carrying + /// it through. + /// @param signedCoefficient The coefficient, within int224. + /// @return The non-negative coefficient. + function absCoefficient(int256 signedCoefficient) internal pure returns (int256) { + return signedCoefficient < 0 ? -signedCoefficient : signedCoefficient; + } + + /// Whether A is less than or equal to B, without packing either. + /// + /// `eq` is the only comparison this library offered on unpacked values, so + /// callers staying below the public surface had to reach for + /// `compareRescale` and compare its results by hand. + /// @param signedCoefficientA The first coefficient. + /// @param exponentA The first exponent. + /// @param signedCoefficientB The second coefficient. + /// @param exponentB The second exponent. + /// @return Whether A <= B. + function lte(int256 signedCoefficientA, int256 exponentA, int256 signedCoefficientB, int256 exponentB) + internal + pure + returns (bool) + { + (int256 rescaledA, int256 rescaledB) = + compareRescale(signedCoefficientA, exponentA, signedCoefficientB, exponentB); + return rescaledA <= rescaledB; + } + + /// The larger of two unpacked values, without packing either. + /// + /// Ties return B, so that `max(x, x)` is stable whichever representation + /// of a numerically equal pair is passed second. + /// @param signedCoefficientA The first coefficient. + /// @param exponentA The first exponent. + /// @param signedCoefficientB The second coefficient. + /// @param exponentB The second exponent. + /// @return The larger value's coefficient. + /// @return The larger value's exponent. + function max(int256 signedCoefficientA, int256 exponentA, int256 signedCoefficientB, int256 exponentB) + internal + pure + returns (int256, int256) + { + return lte(signedCoefficientA, exponentA, signedCoefficientB, exponentB) + ? (signedCoefficientB, exponentB) + : (signedCoefficientA, exponentA); + } + /// Sets the coefficient so that exponent is the target exponent. Truncates /// the coefficient if shrinking, will error on overflow when growing. /// @param signedCoefficient The signed coefficient. diff --git a/test/src/lib/LibDecimalFloat.agree.t.sol b/test/src/lib/LibDecimalFloat.agree.t.sol new file mode 100644 index 00000000..e2c93c8f --- /dev/null +++ b/test/src/lib/LibDecimalFloat.agree.t.sol @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.1/src/Test.sol"; +import {Float, LibDecimalFloat} from "src/lib/LibDecimalFloat.sol"; + +contract LibDecimalFloatAgreeTest is Test { + using LibDecimalFloat for Float; + + function f(int256 signedCoefficient, int256 exponent) internal pure returns (Float) { + return LibDecimalFloat.packLossless(signedCoefficient, exponent); + } + + function minNegative() internal pure returns (Float) { + return LibDecimalFloat.packLossless(type(int224).min, type(int32).max); + } + + function maxPositive() internal pure returns (Float) { + return LibDecimalFloat.packLossless(type(int224).max, type(int32).max); + } + + /// The proportional tolerance is of the LARGER MAGNITUDE of the two + /// extremes, so with positive values it is a proportion of the highest. + /// + /// 100 - 99 == 1 == 0.01 * 100, so this sits exactly on the limit and is + /// accepted, because the check is `<=`. Anchoring on the lower value would + /// give 0.01 * 99 == 0.99 and reject it, so these distinguish the two + /// anchors rather than merely exercising one. + function testAgreeProportionalBoundary() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(99, 0), f(100, 0))); + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(98999, -3), f(100, 0))); + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(99001, -3), f(100, 0))); + } + + /// The absolute tolerance is in the same units as the values and does not + /// scale with them. + function testAgreeAbsoluteBoundary() external pure { + assertTrue(LibDecimalFloat.agree(f(1, 0), f(0, 0), f(100, 0), f(101, 0))); + assertFalse(LibDecimalFloat.agree(f(1, 0), f(0, 0), f(100, 0), f(101001, -3))); + assertTrue(LibDecimalFloat.agree(f(1, 0), f(0, 0), f(100, 0), f(100999, -3))); + } + + /// The limit is the LARGER of the two terms, not their sum. + /// + /// With an absolute tolerance of 0.5, a proportional tolerance of 0.01 and + /// a highest of 100, the larger term is 1 and the sum would be 1.5. A + /// spread of 1.2 falls between them, so it distinguishes the two forms + /// rather than merely exercising one. + function testAgreeLimitIsTheLargerNotTheSum() external pure { + assertTrue(LibDecimalFloat.agree(f(5, -1), f(1, -2), f(99, 0), f(100, 0))); + assertFalse(LibDecimalFloat.agree(f(5, -1), f(1, -2), f(988, -1), f(100, 0))); + } + + /// Either tolerance alone carries the check when the other is zero. + function testAgreeOneToleranceZero() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(99, 0), f(100, 0))); + assertTrue(LibDecimalFloat.agree(f(1, 0), f(0, 0), f(100, 0), f(101, 0))); + } + + /// The magnitude is taken, so negatives behave as positives reflected + /// through zero rather than being rejected outright. + function testAgreeNegativeValues() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(-100, 0), f(-99, 0))); + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(-100, 0), f(-98999, -3))); + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(-100, 0), f(-100, 0))); + } + + /// Values symmetric about zero are the furthest apart anything can be + /// relative to its own magnitude: the spread is twice the anchor, so a + /// proportional tolerance has to reach 200%. + function testAgreeSymmetricAboutZero() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), f(2, 0), f(-1, 0), f(1, 0))); + assertFalse(LibDecimalFloat.agree(f(0, 0), f(199, -2), f(-1, 0), f(1, 0))); + assertTrue(LibDecimalFloat.agree(f(2, 0), f(0, 0), f(-1, 0), f(1, 0))); + } + + /// WHY BOTH TOLERANCES EXIST. A proportional tolerance collapses as the + /// values approach zero, because the quantity it is a proportion of + /// shrinks with them. The absolute term is what covers that region. + function testAgreeNearZero() external pure { + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(-1, -3), f(1, -3))); + assertTrue(LibDecimalFloat.agree(f(1, -2), f(0, 0), f(-1, -3), f(1, -3))); + } + + /// THE REASON THIS CANNOT BE COMPOSED FROM THE PUBLIC SURFACE. + /// + /// Both `abs` and `sub` revert `ExponentOverflow` on these values, because + /// the exponent is already at its maximum and neither the magnitude of the + /// most negative coefficient nor the spread across the whole range fits a + /// packed value. `agree` answers instead of reverting, which is only + /// possible by staying below that surface. + function testAgreeAcrossTheWholeRange() external pure { + // A spread this wide is not within a 1% proportional tolerance. + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, -2), minNegative(), maxPositive())); + // The most negative value against itself has a zero spread, so it + // agrees. The packed `abs` cannot even be asked this question. + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), minNegative(), minNegative())); + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), maxPositive(), maxPositive())); + } + + /// Identical values agree under any non negative tolerance, and a zero + /// spread is the only thing two zero tolerances accept. + function testAgreeZeroSpread() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), f(0, 0), f(100, 0), f(100, 0))); + assertFalse(LibDecimalFloat.agree(f(0, 0), f(0, 0), f(100, 0), f(101, 0))); + } + + /// The comparison is numerical, so the representation of the tolerances + /// and the values does not change the answer. + function testAgreeNumericalEquality() external pure { + // 0.01 written three ways, against the same boundary. + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(99, 0), f(100, 0))); + assertTrue(LibDecimalFloat.agree(f(0, 0), f(10, -3), f(99, 0), f(100, 0))); + assertTrue(LibDecimalFloat.agree(f(0, 0), f(100, -4), f(990, -1), f(1000, -1))); + } + + /// A tolerance at the top of the range accepts rather than reverting: the + /// multiplication scales the coefficient down and raises the exponent + /// instead of overflowing, and nothing is packed back. + function testAgreeExtremeTolerance() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), maxPositive(), f(1, 0), f(2, 0))); + assertTrue(LibDecimalFloat.agree(maxPositive(), f(0, 0), f(1, 0), f(2, 0))); + assertTrue(LibDecimalFloat.agree(f(0, 0), maxPositive(), minNegative(), maxPositive())); + } +} From 33d5c86659bf2e84deb5651bcc5bd90bdde1b778 Mon Sep 17 00:00:00 2001 From: David Meister Date: Fri, 25 Sep 2026 13:52:49 +0000 Subject: [PATCH 2/8] fix: destructure agree's tuple returns for slither Slither's unused-return detector reads `return f(...)` on a tuple-returning call as an ignored return value, failing the static CI job on all three agree helpers. Each now binds the result to locals and returns those. No behaviour change; agree stays at 11 passing and slither goes from 3 results to 0. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibDecimalFloat.sol | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/lib/LibDecimalFloat.sol b/src/lib/LibDecimalFloat.sol index 51c04826..27cd0b91 100644 --- a/src/lib/LibDecimalFloat.sol +++ b/src/lib/LibDecimalFloat.sol @@ -919,7 +919,11 @@ library LibDecimalFloat { function agreeSpread(Float lowest, Float highest) private pure returns (int256, int256) { (int256 lowestCoefficient, int256 lowestExponent) = lowest.unpack(); (int256 highestCoefficient, int256 highestExponent) = highest.unpack(); - return LibDecimalFloatImplementation.sub(highestCoefficient, highestExponent, lowestCoefficient, lowestExponent); + // Destructured rather than returned directly because slither reads + // `return f(...)` on a tuple-returning call as an ignored return. + (int256 spreadCoefficient, int256 spreadExponent) = + LibDecimalFloatImplementation.sub(highestCoefficient, highestExponent, lowestCoefficient, lowestExponent); + return (spreadCoefficient, spreadExponent); } /// The quantity the proportional tolerance is taken of: the larger @@ -931,12 +935,13 @@ library LibDecimalFloat { function agreeAnchor(Float lowest, Float highest) private pure returns (int256, int256) { (int256 lowestCoefficient, int256 lowestExponent) = lowest.unpack(); (int256 highestCoefficient, int256 highestExponent) = highest.unpack(); - return LibDecimalFloatImplementation.max( + (int256 anchorCoefficient, int256 anchorExponent) = LibDecimalFloatImplementation.max( LibDecimalFloatImplementation.absCoefficient(lowestCoefficient), lowestExponent, LibDecimalFloatImplementation.absCoefficient(highestCoefficient), highestExponent ); + return (anchorCoefficient, anchorExponent); } /// The limit the spread is checked against: the larger of the absolute @@ -962,8 +967,9 @@ library LibDecimalFloat { ); } (int256 absoluteCoefficient, int256 absoluteExponent) = absolute.unpack(); - return + (int256 limitCoefficient, int256 limitExponent) = LibDecimalFloatImplementation.max(absoluteCoefficient, absoluteExponent, scaledCoefficient, scaledExponent); + return (limitCoefficient, limitExponent); } /// Returns true if the float is zero. Handles the case where the signed From 02a19202b135ac95789adf760812442631b18b06 Mon Sep 17 00:00:00 2001 From: David Meister Date: Fri, 25 Sep 2026 13:54:40 +0000 Subject: [PATCH 3/8] test: port the arithmetic cases agree left behind, and add an oracle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first cut of these tests carried 11 cases where the rainlang opcode has 37. Most of that gap is correct — integrity, parse errors, operand handling, the stack walk and the list-level property are all opcode concerns that do not belong here. Four things were arithmetic and should have moved: - straddling zero: the anchor is whichever end is further from zero, not simply the highest. - a zero anchor collapses the proportional term whatever its size. - the proportional term scales with the values and the absolute does not, which is the reason for taking both. And the strongest test did not move at all. Everything here was a hand-derived assertion, so a misread of the formula would have been written into both the code and its expectations with nothing to catch it. `testAgreeAgainstIntegerOracle` computes the predicate in plain int256 arithmetic sharing nothing with this library, over a domain where both paths are exact, so the two can disagree. That also gives the file its first fuzz coverage; every case before this was fixed. Co-Authored-By: Claude Opus 5 (1M context) --- test/src/lib/LibDecimalFloat.agree.t.sol | 76 ++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/test/src/lib/LibDecimalFloat.agree.t.sol b/test/src/lib/LibDecimalFloat.agree.t.sol index e2c93c8f..ff472677 100644 --- a/test/src/lib/LibDecimalFloat.agree.t.sol +++ b/test/src/lib/LibDecimalFloat.agree.t.sol @@ -115,6 +115,82 @@ contract LibDecimalFloatAgreeTest is Test { assertTrue(LibDecimalFloat.agree(f(0, 0), f(100, -4), f(990, -1), f(1000, -1))); } + /// The anchor is whichever end is further from zero, so for values + /// straddling zero it is not simply the highest. + function testAgreeStraddlingZero() external pure { + // Spread 101, anchor 100, so 1.01 reaches it and 1 does not. + assertTrue(LibDecimalFloat.agree(f(0, 0), f(101, -2), f(-1, 0), f(100, 0))); + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, 0), f(100, 0))); + // Reflected: the negative end is now the larger magnitude. + assertTrue(LibDecimalFloat.agree(f(0, 0), f(101, -2), f(-100, 0), f(1, 0))); + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-100, 0), f(1, 0))); + } + + /// A zero anchor collapses the proportional term whatever its size, so + /// only the absolute term can accept a spread there. + function testAgreeZeroAnchor() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(0, 0), f(0, 0))); + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1000, 0), f(0, 0), f(0, 0))); + // The anchor is the larger magnitude, so with a highest of 1 it is 1, + // not 0. + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(0, 0), f(1, 0))); + assertFalse(LibDecimalFloat.agree(f(0, 0), f(99, -2), f(0, 0), f(1, 0))); + } + + /// The proportional term scales with the values and the absolute term + /// does not, which is the whole reason for taking both. + function testAgreeScaling() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(1000, 0), f(1001, 0))); + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, -2), f(10, 0), f(11, 0))); + assertTrue(LibDecimalFloat.agree(f(1, 0), f(0, 0), f(1000, 0), f(1001, 0))); + assertTrue(LibDecimalFloat.agree(f(1, 0), f(0, 0), f(10, 0), f(11, 0))); + } + + /// AN INDEPENDENT ORACLE, composed in plain integer arithmetic sharing + /// nothing with this library. + /// + /// Everything else here is a hand-derived assertion, so a misread of the + /// formula would be written into both the code and the expectations. This + /// computes the predicate as exact integers at a fixed scale, so the two + /// can disagree. + /// + /// THE DOMAIN IS RESTRICTED ON PURPOSE. Values and tolerances are integers + /// at exponent 0 within a range where the products fit an int256 exactly, + /// so both paths are lossless and must agree exactly. Widening it would + /// surface rounding differences rather than formula differences. + function testAgreeAgainstIntegerOracle( + int256 lowestSeed, + int256 highestSeed, + uint256 absoluteSeed, + uint256 proportionalSeed + ) external pure { + int256 lowestValue = bound(lowestSeed, -1e12, 1e12); + int256 highestValue = bound(highestSeed, -1e12, 1e12); + if (highestValue < lowestValue) { + (lowestValue, highestValue) = (highestValue, lowestValue); + } + // Absolute is an integer; proportional is hundredths, so the product + // with an anchor up to 1e12 stays exact. + int256 absoluteValue = int256(bound(absoluteSeed, 0, 1e12)); + int256 proportionalHundredths = int256(bound(proportionalSeed, 0, 100000)); + + int256 spread = highestValue - lowestValue; + int256 anchor = highestValue < 0 + ? -lowestValue + : (lowestValue < 0 && -lowestValue > highestValue ? -lowestValue : highestValue); + // Clear the denominator: spread*100 <= max(absolute*100, proportional*anchor). + int256 proportionalTerm = proportionalHundredths * anchor; + int256 absoluteTerm = absoluteValue * 100; + bool expected = spread * 100 <= (absoluteTerm > proportionalTerm ? absoluteTerm : proportionalTerm); + + assertEq( + LibDecimalFloat.agree( + f(absoluteValue, 0), f(proportionalHundredths, -2), f(lowestValue, 0), f(highestValue, 0) + ), + expected + ); + } + /// A tolerance at the top of the range accepts rather than reverting: the /// multiplication scales the coefficient down and raises the exponent /// instead of overflowing, and nothing is packed back. From b84cf083bdaa6762a2e00591019f75971b7c1bb9 Mon Sep 17 00:00:00 2001 From: David Meister Date: Fri, 25 Sep 2026 13:58:33 +0000 Subject: [PATCH 4/8] test: cover the new unpacked primitives directly, and drop a dead abs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things this repo's conventions expect that the first cut skipped. `abs(coefficient, exponent)` was added and never called — `agree` uses `absCoefficient`. Removed, with its rationale folded into the function that survives. `absCoefficient`, `lte` and `max` had no test files, against a clear per-function convention here that 15 existing files follow. They were reachable only through `agree`, so a defect in one would have surfaced as a confusing `agree` failure rather than where it lived. Each now has its own file covering the case that motivates it: `absCoefficient` on the most negative coefficient, where the packed form cannot produce the answer at all; `lte` and `max` on operands with different exponents, where the larger coefficient is not the larger value. The mutation probe shows the difference. `lte`'s boundary and `max` returning the smaller were previously caught only downstream in `agree`; they are now killed by the tests for the functions themselves. Co-Authored-By: Claude Opus 5 (1M context) --- .../LibDecimalFloatImplementation.sol | 14 +-- ...alFloatImplementation.absCoefficient.t.sol | 46 ++++++++++ .../LibDecimalFloatImplementation.lte.t.sol | 71 ++++++++++++++++ .../LibDecimalFloatImplementation.max.t.sol | 85 +++++++++++++++++++ 4 files changed, 204 insertions(+), 12 deletions(-) create mode 100644 test/src/lib/implementation/LibDecimalFloatImplementation.absCoefficient.t.sol create mode 100644 test/src/lib/implementation/LibDecimalFloatImplementation.lte.t.sol create mode 100644 test/src/lib/implementation/LibDecimalFloatImplementation.max.t.sol diff --git a/src/lib/implementation/LibDecimalFloatImplementation.sol b/src/lib/implementation/LibDecimalFloatImplementation.sol index 7ea4b351..cab89046 100644 --- a/src/lib/implementation/LibDecimalFloatImplementation.sol +++ b/src/lib/implementation/LibDecimalFloatImplementation.sol @@ -1136,7 +1136,8 @@ library LibDecimalFloatImplementation { } } - /// The magnitude of an unpacked value, as an unpacked value. + /// The magnitude of an unpacked coefficient. The exponent is untouched by + /// taking a magnitude, so there is nothing to return alongside it. /// /// The packed `LibDecimalFloat.abs` cannot serve callers working below the /// public arithmetic surface. It has to fit the magnitude back into an @@ -1145,17 +1146,6 @@ library LibDecimalFloatImplementation { /// maximum. Here the coefficient is already widened to an int256, so /// negating an int224 is exact and cannot overflow. /// @param signedCoefficient The coefficient, within int224. - /// @param exponent The exponent, unchanged by taking a magnitude. - /// @return The non-negative coefficient. - /// @return The exponent. - function abs(int256 signedCoefficient, int256 exponent) internal pure returns (int256, int256) { - return (absCoefficient(signedCoefficient), exponent); - } - - /// The magnitude of an unpacked coefficient. The exponent is untouched by - /// taking a magnitude, so callers that already hold it can skip carrying - /// it through. - /// @param signedCoefficient The coefficient, within int224. /// @return The non-negative coefficient. function absCoefficient(int256 signedCoefficient) internal pure returns (int256) { return signedCoefficient < 0 ? -signedCoefficient : signedCoefficient; diff --git a/test/src/lib/implementation/LibDecimalFloatImplementation.absCoefficient.t.sol b/test/src/lib/implementation/LibDecimalFloatImplementation.absCoefficient.t.sol new file mode 100644 index 00000000..4b93aaf5 --- /dev/null +++ b/test/src/lib/implementation/LibDecimalFloatImplementation.absCoefficient.t.sol @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.1/src/Test.sol"; +import {LibDecimalFloatImplementation} from "src/lib/implementation/LibDecimalFloatImplementation.sol"; + +contract LibDecimalFloatImplementationAbsCoefficientTest is Test { + function testAbsCoefficientPositiveUnchanged(int256 signedCoefficient) external pure { + signedCoefficient = bound(signedCoefficient, 0, type(int224).max); + assertEq(LibDecimalFloatImplementation.absCoefficient(signedCoefficient), signedCoefficient); + } + + function testAbsCoefficientNegativeNegated(int256 signedCoefficient) external pure { + signedCoefficient = bound(signedCoefficient, type(int224).min, -1); + assertEq(LibDecimalFloatImplementation.absCoefficient(signedCoefficient), -signedCoefficient); + } + + /// THE REASON THIS EXISTS. The packed `abs` has to fit the magnitude back + /// into an int224, which the most negative coefficient does not, so it + /// raises the exponent and reverts once that is at its maximum. Here the + /// value is already an int256, so negating an int224 is exact. + function testAbsCoefficientMostNegativeIsExact() external pure { + assertEq(LibDecimalFloatImplementation.absCoefficient(type(int224).min), -int256(type(int224).min)); + // And the result does not fit an int224, which is precisely why the + // packed form cannot produce it. + assertGt(LibDecimalFloatImplementation.absCoefficient(type(int224).min), type(int224).max); + } + + function testAbsCoefficientZero() external pure { + assertEq(LibDecimalFloatImplementation.absCoefficient(0), 0); + } + + /// Idempotent: taking a magnitude twice is taking it once. + function testAbsCoefficientIdempotent(int256 signedCoefficient) external pure { + signedCoefficient = bound(signedCoefficient, type(int224).min, type(int224).max); + int256 once = LibDecimalFloatImplementation.absCoefficient(signedCoefficient); + assertEq(LibDecimalFloatImplementation.absCoefficient(once), once); + } + + /// Never negative, for any input in range. + function testAbsCoefficientNeverNegative(int256 signedCoefficient) external pure { + signedCoefficient = bound(signedCoefficient, type(int224).min, type(int224).max); + assertGe(LibDecimalFloatImplementation.absCoefficient(signedCoefficient), 0); + } +} diff --git a/test/src/lib/implementation/LibDecimalFloatImplementation.lte.t.sol b/test/src/lib/implementation/LibDecimalFloatImplementation.lte.t.sol new file mode 100644 index 00000000..292468be --- /dev/null +++ b/test/src/lib/implementation/LibDecimalFloatImplementation.lte.t.sol @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.1/src/Test.sol"; +import {LibDecimalFloatImplementation} from "src/lib/implementation/LibDecimalFloatImplementation.sol"; + +contract LibDecimalFloatImplementationLteTest is Test { + function lte(int256 coefficientA, int256 exponentA, int256 coefficientB, int256 exponentB) + internal + pure + returns (bool) + { + return LibDecimalFloatImplementation.lte(coefficientA, exponentA, coefficientB, exponentB); + } + + /// Same exponent reduces to comparing coefficients. + function testLteSameExponent() external pure { + assertTrue(lte(1, 0, 2, 0)); + assertTrue(lte(2, 0, 2, 0)); + assertFalse(lte(3, 0, 2, 0)); + assertTrue(lte(-2, 0, -1, 0)); + assertFalse(lte(-1, 0, -2, 0)); + } + + /// THE POINT OF THE FUNCTION: different exponents are rescaled before + /// comparing, so a smaller coefficient can be the larger value. + function testLteDifferentExponents() external pure { + // 1e1 == 10 > 9e0, despite the coefficient being smaller. + assertFalse(lte(1, 1, 9, 0)); + assertTrue(lte(9, 0, 1, 1)); + // 1e-1 == 0.1 < 1e0. + assertTrue(lte(1, -1, 1, 0)); + assertFalse(lte(1, 0, 1, -1)); + } + + /// Numerically equal values compare equal whichever way they are packed, + /// and `lte` holds in both directions. + function testLteNumericallyEqual() external pure { + assertTrue(lte(100, 0, 1, 2)); + assertTrue(lte(1, 2, 100, 0)); + assertTrue(lte(10, 1, 100, 0)); + assertTrue(lte(100, 0, 10, 1)); + } + + /// Every value is <= itself. + function testLteReflexive(int256 signedCoefficient, int256 exponent) external pure { + signedCoefficient = bound(signedCoefficient, type(int224).min, type(int224).max); + exponent = bound(exponent, -50, 50); + assertTrue(lte(signedCoefficient, exponent, signedCoefficient, exponent)); + } + + /// For any pair, at least one direction holds, and both hold only when + /// they are numerically equal. + function testLteTotalOrder(int256 coefficientA, int256 exponentA, int256 coefficientB, int256 exponentB) + external + pure + { + coefficientA = bound(coefficientA, type(int224).min, type(int224).max); + coefficientB = bound(coefficientB, type(int224).min, type(int224).max); + exponentA = bound(exponentA, -50, 50); + exponentB = bound(exponentB, -50, 50); + + bool forward = lte(coefficientA, exponentA, coefficientB, exponentB); + bool backward = lte(coefficientB, exponentB, coefficientA, exponentA); + assertTrue(forward || backward); + assertEq( + forward && backward, LibDecimalFloatImplementation.eq(coefficientA, exponentA, coefficientB, exponentB) + ); + } +} diff --git a/test/src/lib/implementation/LibDecimalFloatImplementation.max.t.sol b/test/src/lib/implementation/LibDecimalFloatImplementation.max.t.sol new file mode 100644 index 00000000..f926a728 --- /dev/null +++ b/test/src/lib/implementation/LibDecimalFloatImplementation.max.t.sol @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.1/src/Test.sol"; +import {LibDecimalFloatImplementation} from "src/lib/implementation/LibDecimalFloatImplementation.sol"; + +contract LibDecimalFloatImplementationMaxTest is Test { + function max(int256 coefficientA, int256 exponentA, int256 coefficientB, int256 exponentB) + internal + pure + returns (int256, int256) + { + return LibDecimalFloatImplementation.max(coefficientA, exponentA, coefficientB, exponentB); + } + + function assertMaxIs( + int256 coefficientA, + int256 exponentA, + int256 coefficientB, + int256 exponentB, + int256 expectedCoefficient, + int256 expectedExponent + ) internal pure { + (int256 coefficient, int256 exponent) = max(coefficientA, exponentA, coefficientB, exponentB); + assertEq(coefficient, expectedCoefficient); + assertEq(exponent, expectedExponent); + } + + /// Same exponent reduces to comparing coefficients. + function testMaxSameExponent() external pure { + assertMaxIs(1, 0, 2, 0, 2, 0); + assertMaxIs(2, 0, 1, 0, 2, 0); + assertMaxIs(-2, 0, -1, 0, -1, 0); + } + + /// THE POINT OF THE FUNCTION: different exponents are rescaled first, so + /// the larger coefficient is not necessarily the larger value. + function testMaxDifferentExponents() external pure { + // 1e1 == 10 beats 9e0, despite the smaller coefficient. + assertMaxIs(1, 1, 9, 0, 1, 1); + assertMaxIs(9, 0, 1, 1, 1, 1); + // 1e-1 == 0.1 loses to 1e0. + assertMaxIs(1, -1, 1, 0, 1, 0); + } + + /// Ties return B, so the result is stable regardless of which + /// representation of a numerically equal pair is passed second. + function testMaxTiesReturnB() external pure { + assertMaxIs(100, 0, 1, 2, 1, 2); + assertMaxIs(1, 2, 100, 0, 100, 0); + } + + /// The result is always one of the two inputs, and is never less than + /// either of them. + function testMaxIsOneOfTheInputsAndNotLess( + int256 coefficientA, + int256 exponentA, + int256 coefficientB, + int256 exponentB + ) external pure { + coefficientA = bound(coefficientA, type(int224).min, type(int224).max); + coefficientB = bound(coefficientB, type(int224).min, type(int224).max); + exponentA = bound(exponentA, -50, 50); + exponentB = bound(exponentB, -50, 50); + + (int256 coefficient, int256 exponent) = max(coefficientA, exponentA, coefficientB, exponentB); + + bool isA = coefficient == coefficientA && exponent == exponentA; + bool isB = coefficient == coefficientB && exponent == exponentB; + assertTrue(isA || isB); + + assertTrue(LibDecimalFloatImplementation.lte(coefficientA, exponentA, coefficient, exponent)); + assertTrue(LibDecimalFloatImplementation.lte(coefficientB, exponentB, coefficient, exponent)); + } + + /// Selecting a larger value is exact — nothing is rescaled into the + /// result, so the winner comes back byte for byte as it went in. + function testMaxDoesNotRescaleTheWinner(int256 coefficient, int256 exponent) external pure { + coefficient = bound(coefficient, 1, type(int224).max); + exponent = bound(exponent, -50, 50); + // Against a value that is unambiguously smaller. + assertMaxIs(0, 0, coefficient, exponent, coefficient, exponent); + } +} From e5f32e7cb34ccc9bb87876aae0048f394ae80a9f Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 26 Sep 2026 10:29:18 +0000 Subject: [PATCH 5/8] docs: cut the tolerance-form comparison down to a reference The natspec argued for taking the larger of the two tolerance terms by comparing `math.isclose`, `isapprox` and `numpy.isclose` and relaying what PEP 485 says about summing. Those are other languages' libraries; they are not why this library does it, and quoting their reasoning made an appeal do work the paragraph above it already does on this library's own terms. The external implementations stay as a one-line pointer for a reader who wants prior art. The discussion comparing them is gone. Comments only. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibDecimalFloat.sol | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/lib/LibDecimalFloat.sol b/src/lib/LibDecimalFloat.sol index 82daebb8..422bc644 100644 --- a/src/lib/LibDecimalFloat.sol +++ b/src/lib/LibDecimalFloat.sol @@ -886,11 +886,8 @@ library LibDecimalFloat { /// absolute term therefore carries the region near zero and the /// proportional term carries the rest. /// - /// Taking the larger rather than the sum is the form `math.isclose` - /// (PEP 485) and Julia's `isapprox` use. `numpy.isclose` sums them, which - /// PEP 485 rejects because two tolerances of similar size then allow about - /// twice the intended difference. The sum is also the more permissive of - /// the two, since `max(a, b) <= a + b` for non-negative terms. + /// The same form as `math.isclose` (PEP 485) and Julia's `isapprox`. + /// `numpy.isclose` sums the two terms instead. /// /// THE PROPORTION IS OF THE LARGER MAGNITUDE of the two extremes. Every /// other value in a set lies between them, so no value in the set has a From 8973bfcf416f650c2126a702efa4a1f54f4a4f3a Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 26 Sep 2026 12:56:08 +0000 Subject: [PATCH 6/8] refactor: give the unpacked surface the whole comparison set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agree` needed a comparison on unpacked values, because the spread it checks may exceed what packs, and it added `lte` alone. That left the library with `lte` unpacked and `lt`/`gt`/`gte` only packed: the same question answered at different levels depending on which operator you reach for, and an open invitation to bolt on the missing three the next time something needs one. The set now lives at the unpacked level. `lt`, `gt` and `gte` join `eq` and `lte` there, plus `min` beside `max`, and the packed `lt`, `gt`, `lte` and `gte` unpack and delegate — which is how packed `eq` already worked. The four packed bodies each hand-rolled `compareRescale` and a bare operator; that logic now exists once. No behaviour change: the 72 existing packed comparison tests pass untouched. Tests for the four new unpacked functions, including the dualities that pin them to each other rather than leaving each independently plausible (`gt(a,b) == lt(b,a)`, `gte == !lt`, `lte == lt || eq`), `lt` transitivity across rescaling, and that `min`/`max` partition the pair they are given. Full suite: 53 suites, 485 tests, 0 failed. Lint, fmt and slither clean. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibDecimalFloat.sol | 17 +- .../LibDecimalFloatImplementation.sol | 72 ++++++++- ...cimalFloatImplementation.comparisons.t.sol | 146 ++++++++++++++++++ 3 files changed, 218 insertions(+), 17 deletions(-) create mode 100644 test/src/lib/implementation/LibDecimalFloatImplementation.comparisons.t.sol diff --git a/src/lib/LibDecimalFloat.sol b/src/lib/LibDecimalFloat.sol index 422bc644..43be8827 100644 --- a/src/lib/LibDecimalFloat.sol +++ b/src/lib/LibDecimalFloat.sol @@ -605,10 +605,7 @@ library LibDecimalFloat { function lt(Float a, Float b) internal pure returns (bool) { (int256 signedCoefficientA, int256 exponentA) = a.unpack(); (int256 signedCoefficientB, int256 exponentB) = b.unpack(); - (signedCoefficientA, signedCoefficientB) = - LibDecimalFloatImplementation.compareRescale(signedCoefficientA, exponentA, signedCoefficientB, exponentB); - - return signedCoefficientA < signedCoefficientB; + return LibDecimalFloatImplementation.lt(signedCoefficientA, exponentA, signedCoefficientB, exponentB); } /// Numeric greater than for floats. @@ -620,9 +617,7 @@ library LibDecimalFloat { function gt(Float a, Float b) internal pure returns (bool) { (int256 signedCoefficientA, int256 exponentA) = a.unpack(); (int256 signedCoefficientB, int256 exponentB) = b.unpack(); - (signedCoefficientA, signedCoefficientB) = - LibDecimalFloatImplementation.compareRescale(signedCoefficientA, exponentA, signedCoefficientB, exponentB); - return signedCoefficientA > signedCoefficientB; + return LibDecimalFloatImplementation.gt(signedCoefficientA, exponentA, signedCoefficientB, exponentB); } /// Numeric less than or equal to for floats. @@ -635,9 +630,7 @@ library LibDecimalFloat { function lte(Float a, Float b) internal pure returns (bool) { (int256 signedCoefficientA, int256 exponentA) = a.unpack(); (int256 signedCoefficientB, int256 exponentB) = b.unpack(); - (signedCoefficientA, signedCoefficientB) = - LibDecimalFloatImplementation.compareRescale(signedCoefficientA, exponentA, signedCoefficientB, exponentB); - return signedCoefficientA <= signedCoefficientB; + return LibDecimalFloatImplementation.lte(signedCoefficientA, exponentA, signedCoefficientB, exponentB); } /// Numeric greater than or equal to for floats. @@ -650,9 +643,7 @@ library LibDecimalFloat { function gte(Float a, Float b) internal pure returns (bool) { (int256 signedCoefficientA, int256 exponentA) = a.unpack(); (int256 signedCoefficientB, int256 exponentB) = b.unpack(); - (signedCoefficientA, signedCoefficientB) = - LibDecimalFloatImplementation.compareRescale(signedCoefficientA, exponentA, signedCoefficientB, exponentB); - return signedCoefficientA >= signedCoefficientB; + return LibDecimalFloatImplementation.gte(signedCoefficientA, exponentA, signedCoefficientB, exponentB); } /// Integer component of a float. diff --git a/src/lib/implementation/LibDecimalFloatImplementation.sol b/src/lib/implementation/LibDecimalFloatImplementation.sol index 8d83ddb0..7d53dba0 100644 --- a/src/lib/implementation/LibDecimalFloatImplementation.sol +++ b/src/lib/implementation/LibDecimalFloatImplementation.sol @@ -1153,11 +1153,23 @@ library LibDecimalFloatImplementation { return signedCoefficient < 0 ? -signedCoefficient : signedCoefficient; } + /// Whether A is less than B, without packing either. + /// @param signedCoefficientA The first coefficient. + /// @param exponentA The first exponent. + /// @param signedCoefficientB The second coefficient. + /// @param exponentB The second exponent. + /// @return Whether A < B. + function lt(int256 signedCoefficientA, int256 exponentA, int256 signedCoefficientB, int256 exponentB) + internal + pure + returns (bool) + { + (int256 rescaledA, int256 rescaledB) = + compareRescale(signedCoefficientA, exponentA, signedCoefficientB, exponentB); + return rescaledA < rescaledB; + } + /// Whether A is less than or equal to B, without packing either. - /// - /// `eq` is the only comparison this library offered on unpacked values, so - /// callers staying below the public surface had to reach for - /// `compareRescale` and compare its results by hand. /// @param signedCoefficientA The first coefficient. /// @param exponentA The first exponent. /// @param signedCoefficientB The second coefficient. @@ -1173,6 +1185,58 @@ library LibDecimalFloatImplementation { return rescaledA <= rescaledB; } + /// Whether A is greater than B, without packing either. + /// @param signedCoefficientA The first coefficient. + /// @param exponentA The first exponent. + /// @param signedCoefficientB The second coefficient. + /// @param exponentB The second exponent. + /// @return Whether A > B. + function gt(int256 signedCoefficientA, int256 exponentA, int256 signedCoefficientB, int256 exponentB) + internal + pure + returns (bool) + { + (int256 rescaledA, int256 rescaledB) = + compareRescale(signedCoefficientA, exponentA, signedCoefficientB, exponentB); + return rescaledA > rescaledB; + } + + /// Whether A is greater than or equal to B, without packing either. + /// @param signedCoefficientA The first coefficient. + /// @param exponentA The first exponent. + /// @param signedCoefficientB The second coefficient. + /// @param exponentB The second exponent. + /// @return Whether A >= B. + function gte(int256 signedCoefficientA, int256 exponentA, int256 signedCoefficientB, int256 exponentB) + internal + pure + returns (bool) + { + (int256 rescaledA, int256 rescaledB) = + compareRescale(signedCoefficientA, exponentA, signedCoefficientB, exponentB); + return rescaledA >= rescaledB; + } + + /// The smaller of two unpacked values, without packing either. + /// + /// Ties return B, so that `min(x, x)` is stable whichever representation + /// of a numerically equal pair is passed second. + /// @param signedCoefficientA The first coefficient. + /// @param exponentA The first exponent. + /// @param signedCoefficientB The second coefficient. + /// @param exponentB The second exponent. + /// @return The smaller value's coefficient. + /// @return The smaller value's exponent. + function min(int256 signedCoefficientA, int256 exponentA, int256 signedCoefficientB, int256 exponentB) + internal + pure + returns (int256, int256) + { + return gte(signedCoefficientA, exponentA, signedCoefficientB, exponentB) + ? (signedCoefficientB, exponentB) + : (signedCoefficientA, exponentA); + } + /// The larger of two unpacked values, without packing either. /// /// Ties return B, so that `max(x, x)` is stable whichever representation diff --git a/test/src/lib/implementation/LibDecimalFloatImplementation.comparisons.t.sol b/test/src/lib/implementation/LibDecimalFloatImplementation.comparisons.t.sol new file mode 100644 index 00000000..33e52971 --- /dev/null +++ b/test/src/lib/implementation/LibDecimalFloatImplementation.comparisons.t.sol @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.1/src/Test.sol"; +import {LibDecimalFloatImplementation} from "src/lib/implementation/LibDecimalFloatImplementation.sol"; + +/// `lt`, `gt`, `gte` and `min` on unpacked values. `lte` and `max` have their +/// own files; these are the rest of the set, so the unpacked surface carries +/// every comparison the packed surface does rather than a subset of them. +contract LibDecimalFloatImplementationComparisonsTest is Test { + function lt(int256 cA, int256 eA, int256 cB, int256 eB) internal pure returns (bool) { + return LibDecimalFloatImplementation.lt(cA, eA, cB, eB); + } + + function gt(int256 cA, int256 eA, int256 cB, int256 eB) internal pure returns (bool) { + return LibDecimalFloatImplementation.gt(cA, eA, cB, eB); + } + + function gte(int256 cA, int256 eA, int256 cB, int256 eB) internal pure returns (bool) { + return LibDecimalFloatImplementation.gte(cA, eA, cB, eB); + } + + function lte(int256 cA, int256 eA, int256 cB, int256 eB) internal pure returns (bool) { + return LibDecimalFloatImplementation.lte(cA, eA, cB, eB); + } + + function eq(int256 cA, int256 eA, int256 cB, int256 eB) internal pure returns (bool) { + return LibDecimalFloatImplementation.eq(cA, eA, cB, eB); + } + + /// Same exponent reduces to comparing coefficients. + function testSameExponent() external pure { + assertTrue(lt(1, 0, 2, 0)); + assertFalse(lt(2, 0, 2, 0)); + assertTrue(gt(3, 0, 2, 0)); + assertFalse(gt(2, 0, 2, 0)); + assertTrue(gte(2, 0, 2, 0)); + assertTrue(gte(3, 0, 2, 0)); + assertFalse(gte(1, 0, 2, 0)); + } + + /// THE POINT OF THE FUNCTIONS: different exponents are rescaled before + /// comparing, so a smaller coefficient can be the larger value. + function testDifferentExponents() external pure { + // 1e1 == 10 > 9e0, despite the coefficient being smaller. + assertTrue(gt(1, 1, 9, 0)); + assertFalse(lt(1, 1, 9, 0)); + assertTrue(lt(9, 0, 1, 1)); + // 1e-1 == 0.1 < 1e0. + assertTrue(lt(1, -1, 1, 0)); + assertTrue(gt(1, 0, 1, -1)); + } + + /// Numerically equal values are neither less nor greater, whichever way + /// they are packed, and the inclusive forms hold both directions. + function testNumericallyEqual() external pure { + assertFalse(lt(100, 0, 1, 2)); + assertFalse(gt(100, 0, 1, 2)); + assertTrue(gte(100, 0, 1, 2)); + assertTrue(gte(1, 2, 100, 0)); + assertFalse(lt(10, 1, 100, 0)); + assertFalse(gt(10, 1, 100, 0)); + } + + /// Nothing is less than or greater than itself; every value is >= itself. + function testIrreflexive(int256 signedCoefficient, int256 exponent) external pure { + signedCoefficient = bound(signedCoefficient, type(int224).min, type(int224).max); + exponent = bound(exponent, -50, 50); + assertFalse(lt(signedCoefficient, exponent, signedCoefficient, exponent)); + assertFalse(gt(signedCoefficient, exponent, signedCoefficient, exponent)); + assertTrue(gte(signedCoefficient, exponent, signedCoefficient, exponent)); + } + + /// `gt` is `lt` with the operands swapped, and `gte` is `lte` swapped, for + /// every pair. This is what pins the four to one another rather than each + /// being independently plausible. + function testDualities(int256 cA, int256 eA, int256 cB, int256 eB) external pure { + cA = bound(cA, type(int224).min, type(int224).max); + cB = bound(cB, type(int224).min, type(int224).max); + eA = bound(eA, -50, 50); + eB = bound(eB, -50, 50); + + assertEq(gt(cA, eA, cB, eB), lt(cB, eB, cA, eA)); + assertEq(gte(cA, eA, cB, eB), lte(cB, eB, cA, eA)); + // Exactly one of <, ==, > holds. + assertEq(lt(cA, eA, cB, eB), !gte(cA, eA, cB, eB)); + assertEq(gt(cA, eA, cB, eB), !lte(cA, eA, cB, eB)); + // The inclusive forms are the strict form or equality. + assertEq(lte(cA, eA, cB, eB), lt(cA, eA, cB, eB) || eq(cA, eA, cB, eB)); + assertEq(gte(cA, eA, cB, eB), gt(cA, eA, cB, eB) || eq(cA, eA, cB, eB)); + } + + /// `lt` is transitive across rescaling. + function testLtTransitive(int256 cA, int256 cB, int256 cC) external pure { + cA = bound(cA, -1e30, 1e30); + cB = bound(cB, -1e30, 1e30); + cC = bound(cC, -1e30, 1e30); + // Deliberately different exponents so rescaling is exercised. + if (lt(cA, 3, cB, 0) && lt(cB, 0, cC, -3)) { + assertTrue(lt(cA, 3, cC, -3)); + } + } + + /// `min` returns an operand, and one that is <= both. + function testMin(int256 cA, int256 eA, int256 cB, int256 eB) external pure { + cA = bound(cA, type(int224).min, type(int224).max); + cB = bound(cB, type(int224).min, type(int224).max); + eA = bound(eA, -50, 50); + eB = bound(eB, -50, 50); + + (int256 c, int256 e) = LibDecimalFloatImplementation.min(cA, eA, cB, eB); + + // It is one of the two operands, unchanged. + assertTrue((c == cA && e == eA) || (c == cB && e == eB)); + // And it is no greater than either. + assertTrue(lte(c, e, cA, eA)); + assertTrue(lte(c, e, cB, eB)); + } + + /// `min` and `max` pick opposite ends of the same pair: the pair of results + /// is the pair of inputs, whichever order they arrive in. + function testMinMaxPartitionThePair(int256 cA, int256 eA, int256 cB, int256 eB) external pure { + cA = bound(cA, type(int224).min, type(int224).max); + cB = bound(cB, type(int224).min, type(int224).max); + eA = bound(eA, -50, 50); + eB = bound(eB, -50, 50); + + (int256 lowC, int256 lowE) = LibDecimalFloatImplementation.min(cA, eA, cB, eB); + (int256 highC, int256 highE) = LibDecimalFloatImplementation.max(cA, eA, cB, eB); + + assertTrue(lte(lowC, lowE, highC, highE)); + // Numerically the multiset {min, max} is the multiset {A, B}. + assertTrue( + (eq(lowC, lowE, cA, eA) && eq(highC, highE, cB, eB)) || (eq(lowC, lowE, cB, eB) && eq(highC, highE, cA, eA)) + ); + } + + /// Ties return B, matching `max`, so the choice is stable and documented + /// rather than incidental. + function testMinTieReturnsB() external pure { + (int256 c, int256 e) = LibDecimalFloatImplementation.min(100, 0, 1, 2); + assertEq(c, 1); + assertEq(e, 2); + } +} From fb40a914ada96a7b3ee00111e670ce57492fa197 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 26 Sep 2026 15:14:22 +0000 Subject: [PATCH 7/8] docs: state agree's precision boundary, and pin it with tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agree` accepts a spread that an exact comparison would refuse, when the spread lands exactly on the limit and the subtraction discarded digits that would have carried it over. `agree(0, 1, -1e-100, 1)` is such a case: the real spread is 1 + 1e-100, needing 101 significant digits against the coefficient's 76, so `sub` returns exactly 1 and 1 <= 1 holds. Documented rather than changed. `sub` reports that same spread as exactly 1 when asked directly, so resolving the boundary the other way would put `agree` at odds with the library's own arithmetic, and the excess it admits is bounded by one unit in the last place of the aligned coefficient — under 1e-76 of the spread's own magnitude. Nine tests, because a documented claim with nothing asserting it rots: - The cliff is LOCATED rather than sampled. Walking the exponent gap 1..120 pins the flip at ADD_MAX_EXPONENT_DIFF + 1 and asserts the transition happens exactly once, so a scatter of accepts and refusals would fail where sampling two points could not tell the difference. - The cliff is scale invariant across seven orders of magnitude, which is what distinguishes a relative precision limit from an absolute one. - The loss is one directional: with both extremes positive the discarded term shrinks the spread, and the truncated and exact answers agree, so the imprecision cannot cause a refusal. - A refusal is always backed by `sub` reporting a spread over the limit, fuzzed across gaps straddling the cliff. - The mechanism is asserted directly, not inferred: `sub` on those operands returns exactly 1. - Inside precision the check is exactly the integer comparison, which bounds the imprecision to the cliff instead of leaving it as something that might apply anywhere. The gap constant is named once rather than cast twice. Plain `//` and not natspec, because solc rejects a doc comment on a file level constant and `-D warnings` makes that fatal. Full suite 53 suites, 494 tests. The one failure, `testSubPacked` reverting ExponentUnderflow, reproduces on main at the same fuzz seed with the same counterexample and is untouched by this change. Lint, fmt and slither clean. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibDecimalFloat.sol | 17 +++ test/src/lib/LibDecimalFloat.agree.t.sol | 177 +++++++++++++++++++++++ 2 files changed, 194 insertions(+) diff --git a/src/lib/LibDecimalFloat.sol b/src/lib/LibDecimalFloat.sol index 43be8827..2c2d251d 100644 --- a/src/lib/LibDecimalFloat.sol +++ b/src/lib/LibDecimalFloat.sol @@ -895,6 +895,23 @@ library LibDecimalFloat { /// a spread that no packed value can hold. A closeness test asked about /// representable values should answer, not revert. /// + /// THE COMPARISON IS EXACT ONLY TO REPRESENTABLE PRECISION. The spread is a + /// subtraction, and a subtraction aligns exponents by discarding the + /// smaller operand's low digits; past a gap of `ADD_MAX_EXPONENT_DIFF` the + /// smaller operand is dropped whole. When those discarded digits would have + /// carried the spread above the limit, and the spread as computed lands + /// exactly on the limit, this returns true where an exact comparison would + /// return false. `agree(0, 1, -1e-100, 1)` is such a case: the real spread + /// is `1 + 1e-100`, needing 101 significant digits against the + /// coefficient's 76, so `sub` returns exactly `1` and `1 <= 1` holds. + /// + /// That is the rounding every other operation here performs, and `sub` + /// reports the same spread as exactly `1` when asked directly. Resolving + /// the boundary the other way would put this function at odds with the + /// library's own arithmetic. The excess it admits is bounded by one unit in + /// the last place of the aligned coefficient, so a spread accepted at the + /// boundary exceeds the limit by less than `1e-76` of its own magnitude. + /// /// The caller is responsible for the tolerances being sensible. A negative /// tolerance is simply dominated by the other term, and two zero /// tolerances make this an exact equality test. diff --git a/test/src/lib/LibDecimalFloat.agree.t.sol b/test/src/lib/LibDecimalFloat.agree.t.sol index ff472677..eaee422b 100644 --- a/test/src/lib/LibDecimalFloat.agree.t.sol +++ b/test/src/lib/LibDecimalFloat.agree.t.sol @@ -4,6 +4,20 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.1/src/Test.sol"; import {Float, LibDecimalFloat} from "src/lib/LibDecimalFloat.sol"; +import { + LibDecimalFloatImplementation, + ADD_MAX_EXPONENT_DIFF +} from "src/lib/implementation/LibDecimalFloatImplementation.sol"; + +// The exponent gap at which the spread subtraction stops seeing the smaller +// operand at all, so the spread reads as exactly the larger one. `add` aligns +// without loss up to `ADD_MAX_EXPONENT_DIFF` and drops the smaller operand one +// past it. +// +// `ADD_MAX_EXPONENT_DIFF` is a `uint256` of 76 and the exponent walk works in +// `int256`, so the cast is exact and cannot truncate. +//forge-lint: disable-next-line(unsafe-typecast) +int256 constant BOUNDARY_CLIFF_GAP = int256(ADD_MAX_EXPONENT_DIFF) + 1; contract LibDecimalFloatAgreeTest is Test { using LibDecimalFloat for Float; @@ -199,4 +213,167 @@ contract LibDecimalFloatAgreeTest is Test { assertTrue(LibDecimalFloat.agree(maxPositive(), f(0, 0), f(1, 0), f(2, 0))); assertTrue(LibDecimalFloat.agree(f(0, 0), maxPositive(), minNegative(), maxPositive())); } + + /// The comparison is exact only to representable precision, and this pins + /// that so the natspec claim cannot drift silently. + /// + /// The real spread of `-1e-100` and `1` is `1 + 1e-100`, which needs 101 + /// significant digits against the coefficient's 76. The subtraction + /// discards the small term, the spread reads as exactly `1`, and a limit of + /// `1` is therefore met. An exact comparison would refuse all three of + /// these. + function testAgreeBoundaryExactOnlyToRepresentablePrecision() external pure { + // A proportional tolerance of 1 against an anchor of 1 is a limit of 1. + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, -100), f(1, 0))); + // The same limit reached by the absolute term instead. + assertTrue(LibDecimalFloat.agree(f(1, 0), f(0, 0), f(-1, -100), f(1, 0))); + // And both terms at once, so neither branch of the `max` escapes it. + assertTrue(LibDecimalFloat.agree(f(1, 0), f(1, 0), f(-1, -100), f(1, 0))); + } + + /// The mechanism behind the boundary, asserted directly rather than + /// inferred: `sub` itself reports the spread as exactly `1`, so `agree` is + /// agreeing with the library's own arithmetic rather than departing from + /// it. + function testAgreeBoundarySpreadIsExactlyOne() external pure { + (int256 lowestCoefficient, int256 lowestExponent) = f(-1, -100).unpack(); + (int256 highestCoefficient, int256 highestExponent) = f(1, 0).unpack(); + (int256 spreadCoefficient, int256 spreadExponent) = + LibDecimalFloatImplementation.sub(highestCoefficient, highestExponent, lowestCoefficient, lowestExponent); + (int256 oneCoefficient, int256 oneExponent) = f(1, 0).unpack(); + assertTrue( + LibDecimalFloatImplementation.eq(spreadCoefficient, spreadExponent, oneCoefficient, oneExponent), + "spread is not exactly one" + ); + } + + /// Just inside the coefficient's reach the small term survives, the spread + /// exceeds the limit, and the check refuses. This is what shows the + /// boundary is a precision limit rather than `agree` ignoring small terms + /// in general. + function testAgreeSmallTermRefusedWhenRepresentable() external pure { + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, -60), f(1, 0))); + assertFalse(LibDecimalFloat.agree(f(1, 0), f(0, 0), f(-1, -60), f(1, 0))); + } + + /// Walks the exponent gap to LOCATE the precision cliff rather than + /// sampling a point either side of it. Below the cliff the small term + /// survives and the spread exceeds the limit; at and past it the term is + /// discarded and the spread reads as exactly the limit. + /// + /// Also asserts the transition happens exactly once. A scatter of accepts + /// and refusals would mean something other than a precision limit is + /// deciding, which sampling two points could never distinguish. + function testAgreeBoundaryCliffLocated() external pure { + bool seenAccepted = false; + int256 firstAccepted = 0; + for (int256 n = 1; n <= 120; n++) { + bool accepted = LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, -n), f(1, 0)); + if (accepted) { + if (!seenAccepted) { + seenAccepted = true; + firstAccepted = n; + } + } else { + // A term that has already vanished cannot reappear as the gap + // widens further. + assertFalse(seenAccepted, "acceptance is not monotone in the exponent gap"); + } + } + assertTrue(seenAccepted, "never accepted anywhere in the walk"); + // Both operands are maximized to the same order of magnitude before + // alignment, so the gap the alignment sees is the exponent difference, + // and it gives up one past ADD_MAX_EXPONENT_DIFF. + assertEq(firstAccepted, BOUNDARY_CLIFF_GAP, "the cliff moved"); + } + + /// The cliff is a RELATIVE precision limit, so scaling the whole problem by + /// a power of ten moves it not at all. Anchoring on an absolute exponent + /// instead would shift the cliff with the scale. + function testAgreeBoundaryCliffIsScaleInvariant() external pure { + int256 cliff = BOUNDARY_CLIFF_GAP; + for (int256 k = -30; k <= 30; k += 10) { + // One below the cliff: the small term survives, so it is refused. + assertFalse( + LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, k - cliff + 1), f(1, k)), "refused side moved with scale" + ); + // At the cliff: the term is discarded and the spread reads exactly + // as the limit. + assertTrue( + LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, k - cliff), f(1, k)), "accepted side moved with scale" + ); + } + } + + /// The same spread reached with the operands placed the other way about + /// zero behaves identically. `-1e-100` against `1` and `-1` against + /// `1e-100` are both a real spread of `1 + 1e-100` with an anchor of `1`, + /// so neither the sign of the larger operand nor which side carries the + /// tiny magnitude changes the outcome. + function testAgreeBoundaryMirroredAboutZero() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, -100), f(1, 0))); + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, 0), f(1, -100))); + // And one below the cliff both ways round. + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, -60), f(1, 0))); + assertFalse(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(-1, 0), f(1, -60))); + } + + /// The loss is ONE DIRECTIONAL. When the discarded term would have made the + /// spread SMALLER, the truncated answer and the exact answer agree, so the + /// imprecision cannot cause a refusal. + /// + /// Here both extremes are positive, so the spread `1 - 1e-100` is slightly + /// under the limit of `1`, and it reads as exactly `1`. Both the truncated + /// and the exact comparison accept, unlike the opposite-sign case where + /// they differ. + function testAgreeBoundaryLossIsOneDirectional() external pure { + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(1, -100), f(1, 0))); + // Well inside precision the same shape is still accepted, because the + // spread is genuinely below the limit rather than rounded to it. + assertTrue(LibDecimalFloat.agree(f(0, 0), f(1, 0), f(1, -60), f(1, 0))); + } + + /// A refusal is always sound. Truncation only ever reduces the spread's + /// magnitude, so if the computed spread already exceeds the limit then the + /// exact spread does too. Fuzzed across gaps that straddle the cliff, a + /// refusal must be backed by `sub` reporting a spread above the limit. + function testAgreeRefusalIsAlwaysBackedByTheSpread(int256 gap, int256 anchorExponent) external pure { + gap = bound(gap, 1, 120); + anchorExponent = bound(anchorExponent, -40, 40); + + Float lowest = f(-1, anchorExponent - gap); + Float highest = f(1, anchorExponent); + + if (LibDecimalFloat.agree(f(0, 0), f(1, 0), lowest, highest)) { + return; + } + + (int256 lowestCoefficient, int256 lowestExponent) = lowest.unpack(); + (int256 highestCoefficient, int256 highestExponent) = highest.unpack(); + (int256 spreadCoefficient, int256 spreadExponent) = + LibDecimalFloatImplementation.sub(highestCoefficient, highestExponent, lowestCoefficient, lowestExponent); + (int256 limitCoefficient, int256 limitExponent) = highest.unpack(); + assertTrue( + LibDecimalFloatImplementation.gt(spreadCoefficient, spreadExponent, limitCoefficient, limitExponent), + "refused without the spread exceeding the limit" + ); + } + + /// Away from the cliff the check is exactly the integer comparison, for + /// every gap the coefficient can hold. This bounds the imprecision to the + /// cliff rather than leaving it as a property that might apply anywhere. + function testAgreeMatchesIntegerComparisonInsidePrecision(int256 spreadUnits, int256 limitUnits) external pure { + spreadUnits = bound(spreadUnits, 0, 1e18); + limitUnits = bound(limitUnits, 1, 1e18); + + // lowest = 0, highest = spreadUnits, so the spread is exact and the + // anchor is the highest. An absolute tolerance keeps the limit exact + // too, so the whole comparison is representable. + bool expected = spreadUnits <= limitUnits; + assertEq( + LibDecimalFloat.agree(f(limitUnits, 0), f(0, 0), f(0, 0), f(spreadUnits, 0)), + expected, + "diverged from the integer comparison inside precision" + ); + } } From 6d131b488eb563cfc82fd576f08cea1a64f63321 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 26 Sep 2026 18:59:28 +0000 Subject: [PATCH 8/8] test: assert agree never reverts, which nothing was checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agree`'s natspec promises an ANSWER rather than a revert for representable values, and that promise is the whole reason the function works unpacked. Nothing asserted it. Two tests. A fuzz over arbitrary bit patterns in all four operands, tolerances included, so nothing about the packing is assumed sane; and the exact counterexample `testSubPacked` fails on, driven through `agree`. That second one is the case worth pinning. `sub` reverting `ExponentUnderflow` is a live bug in this library, but it is in the PACKED wrapper, which packs the result. `agree` never packs back, so it cannot inherit it. That now rests on a test rather than on reading the call graph, and a future change to the packing path cannot quietly make `agree` revert. Mutation coverage for the comparison set added in the previous commit: 12/12 killed against a baseline the probe proved green at 157, over every suite that exercises the changed code. The six new mutants cover `lt`, `gt`, `gte` and `min` at their equality boundaries, `min` returning the larger, and two MIS-WIRINGS of the packed delegations — packed `lt` routed to unpacked `gt`, packed `gte` routed to `lte`. The wiring mutants are the point: four packed functions now route through one unpacked set, so a mis-wire is a single token that still compiles and still looks right. Co-Authored-By: Claude Opus 5 (1M context) --- test/src/lib/LibDecimalFloat.agree.t.sol | 33 ++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/src/lib/LibDecimalFloat.agree.t.sol b/test/src/lib/LibDecimalFloat.agree.t.sol index eaee422b..7f8ca039 100644 --- a/test/src/lib/LibDecimalFloat.agree.t.sol +++ b/test/src/lib/LibDecimalFloat.agree.t.sol @@ -376,4 +376,37 @@ contract LibDecimalFloatAgreeTest is Test { "diverged from the integer comparison inside precision" ); } + + /// `agree` promises an ANSWER rather than a revert for representable values, + /// and that promise is the reason it works unpacked. Fuzzed over arbitrary + /// bit patterns in all four operands, including tolerances, so nothing about + /// the packing is assumed sane. + /// + /// `sub` reverting `ExponentUnderflow` is a live bug in this library + /// (`testSubPacked`), but it lives in the PACKED wrapper, which packs the + /// result. `agree` never packs back, so it cannot inherit it — this asserts + /// that rather than relying on it. + /// forge-config: default.fuzz.runs = 20000 + function testAgreeNeverReverts(bytes32 absolute, bytes32 proportional, bytes32 lowest, bytes32 highest) + external + pure + { + bool result = LibDecimalFloat.agree( + Float.wrap(absolute), Float.wrap(proportional), Float.wrap(lowest), Float.wrap(highest) + ); + // Only that it returned. The value is whatever the operands imply. + assertTrue(result || !result); + } + + /// The exact counterexample `testSubPacked` fails on, driven through + /// `agree`. Pinned as a case in its own right so a future change to the + /// packing path cannot quietly make `agree` revert. + function testAgreeSurvivesTheSubUnderflowCounterexample() external pure { + Float a = Float.wrap(0x8000000000000000000000000000000000000000000000000000000000000009); + Float b = Float.wrap(0x8000000000000000000000000000000000000000000000000000000000000003); + bool forward = LibDecimalFloat.agree(f(0, 0), f(1, -2), a, b); + bool backward = LibDecimalFloat.agree(f(0, 0), f(1, -2), b, a); + assertTrue(forward || !forward); + assertTrue(backward || !backward); + } }