From 7ab9709373468ad295a891eb8c8567f994279a28 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 20 Jan 2026 19:59:59 -0500 Subject: [PATCH 01/11] Add canonical "scale" computation to Number - Requires a template for STAmount and Asset. - Update tests and computeMinScale from #6217 to use scale. - Convert a few other places to use "scale" correctly. --- include/xrpl/basics/Number.h | 32 ++++++++ src/test/app/Invariants_test.cpp | 76 ++++++++++++++++++- src/xrpld/app/misc/LendingHelpers.h | 3 +- src/xrpld/app/tx/detail/InvariantCheck.cpp | 16 +--- .../app/tx/detail/LoanBrokerCoverWithdraw.cpp | 2 +- 5 files changed, 110 insertions(+), 19 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index d1ef7497844..e43daca8ba4 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -109,6 +109,10 @@ template concept Integral64 = std::is_same_v || std::is_same_v; +template +concept CanUseAsScale = requires(Asset a, Number n) { STAmount(a, n); } && + requires(STAmount s) { s.exponent(); }; + /** Number is a floating point type that can represent a wide range of values. * * It can represent all values that can be represented by an STAmount - @@ -268,6 +272,26 @@ class Number constexpr int exponent() const noexcept; + /** Get the scale of this Number for the given asset. + * + * "scale" is similar to "exponent", but from the perspective of STAmount, + * which has different rules for determining the exponent than Number. + * + * Because Number does not have access to STAmount or Asset, this function + * is implemented as a template, with the expectation that it will only be + * used by those types. Any types that fit the requirements will work, + * though, if there's a need. + * + * @tparam STAmount The STAmount type. + * @tparam Asset The Asset type. + * @param asset The asset to use for determining the scale. + * @return The scale of this Number for the given asset. + */ + template + int + scale(Asset const& asset) const + requires CanUseAsScale; + constexpr Number operator+() const noexcept; constexpr Number @@ -602,6 +626,14 @@ Number::exponent() const noexcept return e; } +template +int +Number::scale(Asset const& asset) const + requires CanUseAsScale +{ + return STAmount{asset, *this}.exponent(); +} + inline constexpr Number Number::operator+() const noexcept { diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 8051ab1cbe9..c30d0320702 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -20,8 +21,6 @@ #include -#include "xrpld/app/tx/detail/InvariantCheck.h" - #include #include @@ -3908,6 +3907,8 @@ class Invariants_test : public beast::unit_test::suite std::initializer_list values; }; + NumberMantissaScaleGuard g{MantissaRange::large}; + auto const testCases = std::vector{ { .name = "No values", @@ -3916,14 +3917,23 @@ class Invariants_test : public beast::unit_test::suite }, { .name = "Mixed integer and Number values", - .expectedMinScale = 0, + .expectedMinScale = -15, .values = {1, -1, Number{10, -1}}, }, { .name = "Mixed scales", - .expectedMinScale = -2, + .expectedMinScale = -17, .values = {Number{1, -2}, Number{5, -3}, Number{3, -2}}, }, + { + .name = "Mixed mantissa sizes", + .expectedMinScale = -12, + .values = + {Number{1}, + Number{1234, -3}, + Number{12345, -6}, + Number{123, 1}}, + }, }; for (auto const& tc : testCases) @@ -3937,6 +3947,64 @@ class Invariants_test : public beast::unit_test::suite actualScale == tc.expectedMinScale, "expected: " + std::to_string(tc.expectedMinScale) + ", actual: " + std::to_string(actualScale)); + for (auto const& num : tc.values) + { + // None of these scales are far enough apart that rounding the + // values would lose information, so check that the rounded + // value matches the original. + auto const actualRounded = + roundToAsset(vaultAsset, num, actualScale); + BEAST_EXPECTS( + actualRounded == num, + "number " + to_string(num) + " rounded to scale " + + std::to_string(actualScale) + " is " + + to_string(actualRounded)); + } + } + + auto const testCases2 = std::vector{ + { + .name = "False equivalence", + .expectedMinScale = -15, + .values = + { + Number{1234567890123456789, -18}, + Number{12345, -4}, + Number{1}, + }, + }, + }; + + // Unlike the first set of test cases, the values in these test could + // look equivalent if using the wrong scale. + for (auto const& tc : testCases2) + { + testcase("vault computeMinScale: " + tc.name); + + auto const actualScale = + ValidVault::computeMinScale(vaultAsset, tc.values); + + BEAST_EXPECTS( + actualScale == tc.expectedMinScale, + "expected: " + std::to_string(tc.expectedMinScale) + + ", actual: " + std::to_string(actualScale)); + std::optional first; + Number firstRounded; + for (auto const& num : tc.values) + { + if (!first) + { + first = num; + firstRounded = roundToAsset(vaultAsset, num, actualScale); + continue; + } + auto const numRounded = + roundToAsset(vaultAsset, num, actualScale); + BEAST_EXPECTS( + numRounded != firstRounded, + "at a scale of " + std::to_string(actualScale) + " " + + to_string(num) + " == " + to_string(*first)); + } } } diff --git a/src/xrpld/app/misc/LendingHelpers.h b/src/xrpld/app/misc/LendingHelpers.h index 79fc6175697..5780f764280 100644 --- a/src/xrpld/app/misc/LendingHelpers.h +++ b/src/xrpld/app/misc/LendingHelpers.h @@ -176,8 +176,7 @@ getAssetsTotalScale(SLE::const_ref vaultSle) { if (!vaultSle) return Number::minExponent - 1; // LCOV_EXCL_LINE - return STAmount{vaultSle->at(sfAsset), vaultSle->at(sfAssetsTotal)} - .exponent(); + return vaultSle->at(sfAssetsTotal).scale(vaultSle->at(sfAsset)); } TER diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index d40c58152d8..6595e6d7291 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -3658,20 +3658,12 @@ ValidVault::computeMinScale( // zeros from its mantissa auto const getNatScale = [](Asset const& asset, Number const& value) -> std::int32_t { - if (value == beast::zero || asset.integral()) + if (asset.integral()) return 0; + if (value == beast::zero) + return value.exponent(); - auto mantissa = std::abs(value.mantissa()); - auto scale = value.exponent(); - - // Remove trailing zeros from mantissa, adjusting scale accordingly - while (mantissa % 10 == 0) - { - mantissa /= 10; - ++scale; - } - - return scale; + return value.scale(asset); }; std::vector natScales; diff --git a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp index 5d4d2053ed3..baafc6f2c08 100644 --- a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp +++ b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp @@ -125,7 +125,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) tenthBipsOfValue( currentDebtTotal, TenthBips32(sleBroker->at(sfCoverRateMinimum))), - currentDebtTotal.exponent()); + currentDebtTotal.scale(vaultAsset)); }(); if (coverAvail < amount) return tecINSUFFICIENT_FUNDS; From 040bf342570127e1f74142d7329df3bdee8199a2 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 21 Jan 2026 14:21:58 -0500 Subject: [PATCH 02/11] ValidVault tracks scale of original operands alongside deltas --- src/test/app/Invariants_test.cpp | 51 +++--- src/xrpld/app/tx/detail/InvariantCheck.cpp | 178 ++++++++++++++------- src/xrpld/app/tx/detail/InvariantCheck.h | 12 +- 3 files changed, 161 insertions(+), 80 deletions(-) diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index b8a8e5d55ef..c7327542b44 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3904,11 +3904,16 @@ class Invariants_test : public beast::unit_test::suite { std::string name; std::int32_t expectedMinScale; - std::initializer_list values; + std::initializer_list values; }; NumberMantissaScaleGuard g{MantissaRange::large}; + auto makeDelta = + [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { + return {n, n.scale(vaultAsset.raw())}; + }; + auto const testCases = std::vector{ { .name = "No values", @@ -3918,26 +3923,33 @@ class Invariants_test : public beast::unit_test::suite { .name = "Mixed integer and Number values", .expectedMinScale = -15, - .values = {1, -1, Number{10, -1}}, + .values = + {makeDelta(1), makeDelta(-1), makeDelta(Number{10, -1})}, }, { .name = "Mixed scales", .expectedMinScale = -17, - .values = {Number{1, -2}, Number{5, -3}, Number{3, -2}}, + .values = + {makeDelta(Number{1, -2}), + makeDelta(Number{5, -3}), + makeDelta(Number{3, -2})}, }, { .name = "Equal scales", - .expectedMinScale = -14, - .values = {Number{1, -1}, Number{5, -1}, Number{1, -1}}, + .expectedMinScale = -16, + .values = + {makeDelta(Number{1, -1}), + makeDelta(Number{5, -1}), + makeDelta(Number{1, -1})}, }, { .name = "Mixed mantissa sizes", .expectedMinScale = -12, .values = - {Number{1}, - Number{1234, -3}, - Number{12345, -6}, - Number{123, 1}}, + {makeDelta(Number{1}), + makeDelta(Number{1234, -3}), + makeDelta(Number{12345, -6}), + makeDelta(Number{123, 1})}, }, }; @@ -3958,10 +3970,10 @@ class Invariants_test : public beast::unit_test::suite // values would lose information, so check that the rounded // value matches the original. auto const actualRounded = - roundToAsset(vaultAsset, num, actualScale); + roundToAsset(vaultAsset, num.delta, actualScale); BEAST_EXPECTS( - actualRounded == num, - "number " + to_string(num) + " rounded to scale " + + actualRounded == num.delta, + "number " + to_string(num.delta) + " rounded to scale " + std::to_string(actualScale) + " is " + to_string(actualRounded)); } @@ -3973,9 +3985,9 @@ class Invariants_test : public beast::unit_test::suite .expectedMinScale = -15, .values = { - Number{1234567890123456789, -18}, - Number{12345, -4}, - Number{1}, + makeDelta(Number{1234567890123456789, -18}), + makeDelta(Number{12345, -4}), + makeDelta(Number{1}), }, }, }; @@ -3999,16 +4011,17 @@ class Invariants_test : public beast::unit_test::suite { if (!first) { - first = num; - firstRounded = roundToAsset(vaultAsset, num, actualScale); + first = num.delta; + firstRounded = + roundToAsset(vaultAsset, num.delta, actualScale); continue; } auto const numRounded = - roundToAsset(vaultAsset, num, actualScale); + roundToAsset(vaultAsset, num.delta, actualScale); BEAST_EXPECTS( numRounded != firstRounded, "at a scale of " + std::to_string(actualScale) + " " + - to_string(num) + " == " + to_string(*first)); + to_string(num.delta) + " == " + to_string(*first)); } } } diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 84a40df4d0d..88b75c75133 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -2669,7 +2669,7 @@ ValidVault::visitEntry( // state (zero if created) and "after" state (zero if destroyed), so the // invariants can validate that the change in account balances matches the // change in vault balances, stored to deltas_ at the end of this function. - Number balanceDelta{}; + DeltaInfo balanceDelta{numZero, STAmount::cMinOffset - 1}; std::int8_t sign = 0; if (before) @@ -2683,20 +2683,35 @@ ValidVault::visitEntry( // At this moment we have no way of telling if this object holds // vault shares or something else. Save it for finalize. beforeMPTs_.push_back(Shares::make(*before)); - balanceDelta = static_cast( + balanceDelta.delta = static_cast( before->getFieldU64(sfOutstandingAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = 1; break; case ltMPTOKEN: - balanceDelta = + balanceDelta.delta = static_cast(before->getFieldU64(sfMPTAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = -1; break; case ltACCOUNT_ROOT: - case ltRIPPLE_STATE: - balanceDelta = before->getFieldAmount(sfBalance); + balanceDelta.delta = before->getFieldAmount(sfBalance); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; sign = -1; break; + case ltRIPPLE_STATE: { + auto const amount = before->getFieldAmount(sfBalance); + balanceDelta.delta = amount; + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + balanceDelta.scale = amount.exponent(); + sign = -1; + break; + } default:; } } @@ -2712,20 +2727,36 @@ ValidVault::visitEntry( // At this moment we have no way of telling if this object holds // vault shares or something else. Save it for finalize. afterMPTs_.push_back(Shares::make(*after)); - balanceDelta -= Number(static_cast( + balanceDelta.delta -= Number(static_cast( after->getFieldU64(sfOutstandingAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = 1; break; case ltMPTOKEN: - balanceDelta -= Number( + balanceDelta.delta -= Number( static_cast(after->getFieldU64(sfMPTAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = -1; break; case ltACCOUNT_ROOT: - case ltRIPPLE_STATE: - balanceDelta -= Number(after->getFieldAmount(sfBalance)); + balanceDelta.delta -= Number(after->getFieldAmount(sfBalance)); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; + sign = -1; + break; + case ltRIPPLE_STATE: { + auto const amount = after->getFieldAmount(sfBalance); + balanceDelta.delta -= Number(amount); + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + balanceDelta.scale = + std::max(balanceDelta.scale, amount.exponent()); sign = -1; break; + } default:; } } @@ -2737,7 +2768,10 @@ ValidVault::visitEntry( // transferred to the account. We intentionally do not compare balanceDelta // against zero, to avoid missing such updates. if (sign != 0) - deltas_[key] = balanceDelta * sign; + { + balanceDelta.delta *= sign; + deltas_[key] = balanceDelta; + } } bool @@ -3017,13 +3051,15 @@ ValidVault::finalize( } auto const& vaultAsset = afterVault.asset; - auto const deltaAssets = [&](AccountID const& id) -> std::optional { + auto const deltaAssets = + [&](AccountID const& id) -> std::optional { auto const get = // - [&](auto const& it, std::int8_t sign = 1) -> std::optional { + [&](auto const& it, + std::int8_t sign = 1) -> std::optional { if (it == deltas_.end()) return std::nullopt; - return it->second * sign; + return DeltaInfo{it->second.delta * sign, it->second.scale}; }; return std::visit( @@ -3044,7 +3080,7 @@ ValidVault::finalize( }, vaultAsset.value()); }; - auto const deltaAssetsTxAccount = [&]() -> std::optional { + auto const deltaAssetsTxAccount = [&]() -> std::optional { auto ret = deltaAssets(tx[sfAccount]); // Nothing returned or not XRP transaction if (!ret.has_value() || !vaultAsset.native()) @@ -3055,13 +3091,14 @@ ValidVault::finalize( delegate.has_value() && *delegate != tx[sfAccount]) return ret; - *ret += fee.drops(); - if (*ret == zero) + ret->delta += fee.drops(); + if (ret->delta == zero) return std::nullopt; return ret; }; - auto const deltaShares = [&](AccountID const& id) -> std::optional { + auto const deltaShares = + [&](AccountID const& id) -> std::optional { auto const it = [&]() { if (id == afterVault.pseudoId) return deltas_.find( @@ -3069,7 +3106,7 @@ ValidVault::finalize( return deltas_.find(keylet::mptoken(afterVault.shareMPTID, id).key); }(); - return it != deltas_.end() ? std::optional(it->second) + return it != deltas_.end() ? std::optional(it->second) : std::nullopt; }; @@ -3211,19 +3248,31 @@ ValidVault::finalize( } // Get the coarsest scale to round calculations to + DeltaInfo totalDelta{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + afterVault.assetsTotal.scale(vaultAsset), + beforeVault.assetsTotal.scale(vaultAsset))}; + DeltaInfo availableDelta{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + afterVault.assetsAvailable.scale(vaultAsset), + beforeVault.assetsAvailable.scale( + vaultAsset))}; auto const minScale = computeMinScale( vaultAsset, { *maybeVaultDeltaAssets, - afterVault.assetsTotal - beforeVault.assetsTotal, - afterVault.assetsAvailable - - beforeVault.assetsAvailable, + totalDelta, + availableDelta, }); - auto const vaultDeltaAssets = - roundToAsset(vaultAsset, *maybeVaultDeltaAssets, minScale); + auto const vaultDeltaAssets = roundToAsset( + vaultAsset, maybeVaultDeltaAssets->delta, minScale); + auto const txAmount = + roundToAsset(vaultAsset, tx[sfAmount], minScale); - if (vaultDeltaAssets > tx[sfAmount]) + if (vaultDeltaAssets > txAmount) { JLOG(j.fatal()) << // "Invariant failed: deposit must not change vault " @@ -3261,7 +3310,7 @@ ValidVault::finalize( computeMinScale(vaultAsset, {*maybeAccDeltaAssets})); auto const accountDeltaAssets = roundToAsset( - vaultAsset, *maybeAccDeltaAssets, localMinScale); + vaultAsset, maybeAccDeltaAssets->delta, localMinScale); auto const localVaultDeltaAssets = roundToAsset( vaultAsset, vaultDeltaAssets, localMinScale); @@ -3301,7 +3350,7 @@ ValidVault::finalize( } // We don't need to round shares, they are integral MPT auto const& accountDeltaShares = *maybeAccDeltaShares; - if (accountDeltaShares <= zero) + if (accountDeltaShares.delta <= zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must increase depositor " @@ -3311,7 +3360,8 @@ ValidVault::finalize( auto const maybeVaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!maybeVaultDeltaShares || *maybeVaultDeltaShares == zero) + if (!maybeVaultDeltaShares || + maybeVaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault shares"; @@ -3320,7 +3370,7 @@ ValidVault::finalize( // We don't need to round shares, they are integral MPT auto const& vaultDeltaShares = *maybeVaultDeltaShares; - if (vaultDeltaShares * -1 != accountDeltaShares) + if (vaultDeltaShares.delta * -1 != accountDeltaShares.delta) { JLOG(j.fatal()) << // "Invariant failed: deposit must change depositor and " @@ -3372,14 +3422,23 @@ ValidVault::finalize( } // Get the most coarse scale to round calculations to + auto const totalDelta = DeltaInfo{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + afterVault.assetsTotal.scale(vaultAsset), + beforeVault.assetsTotal.scale(vaultAsset))}; + auto const availableDelta = DeltaInfo{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + afterVault.assetsAvailable.scale(vaultAsset), + beforeVault.assetsAvailable.scale( + vaultAsset))}; auto const minScale = computeMinScale( vaultAsset, - {*maybeVaultDeltaAssets, - afterVault.assetsTotal - beforeVault.assetsTotal, - afterVault.assetsAvailable - beforeVault.assetsAvailable}); + {*maybeVaultDeltaAssets, totalDelta, availableDelta}); - auto const vaultPseudoDeltaAssets = - roundToAsset(vaultAsset, *maybeVaultDeltaAssets, minScale); + auto const vaultPseudoDeltaAssets = roundToAsset( + vaultAsset, maybeVaultDeltaAssets->delta, minScale); if (vaultPseudoDeltaAssets >= zero) { @@ -3401,7 +3460,7 @@ ValidVault::finalize( { auto const maybeAccDelta = deltaAssetsTxAccount(); auto const maybeOtherAccDelta = - [&]() -> std::optional { + [&]() -> std::optional { if (auto const destination = tx[~sfDestination]; destination && *destination != tx[sfAccount]) return deltaAssets(*destination); @@ -3427,7 +3486,7 @@ ValidVault::finalize( computeMinScale(vaultAsset, {destinationDelta})); auto const roundedDestinationDelta = roundToAsset( - vaultAsset, destinationDelta, localMinScale); + vaultAsset, destinationDelta.delta, localMinScale); if (roundedDestinationDelta <= zero) { @@ -3457,7 +3516,7 @@ ValidVault::finalize( return false; } - if (*accountDeltaShares >= zero) + if (accountDeltaShares->delta >= zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must decrease depositor " @@ -3466,14 +3525,14 @@ ValidVault::finalize( } // We don't need to round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + if (vaultDeltaShares->delta * -1 != accountDeltaShares->delta) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change depositor " @@ -3536,14 +3595,25 @@ ValidVault::finalize( deltaAssets(afterVault.pseudoId); if (maybeVaultDeltaAssets) { + auto const totalDelta = DeltaInfo{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + afterVault.assetsTotal.scale(vaultAsset), + beforeVault.assetsTotal.scale( + vaultAsset))}; + auto const availableDelta = DeltaInfo{ + afterVault.assetsAvailable - + beforeVault.assetsAvailable, + std::max( + afterVault.assetsAvailable.scale( + vaultAsset), + beforeVault.assetsAvailable.scale( + vaultAsset))}; auto const minScale = computeMinScale( vaultAsset, - {*maybeVaultDeltaAssets, - afterVault.assetsTotal - beforeVault.assetsTotal, - afterVault.assetsAvailable - - beforeVault.assetsAvailable}); + {*maybeVaultDeltaAssets, totalDelta, availableDelta}); auto const vaultDeltaAssets = roundToAsset( - vaultAsset, *maybeVaultDeltaAssets, minScale); + vaultAsset, maybeVaultDeltaAssets->delta, minScale); if (vaultDeltaAssets >= zero) { JLOG(j.fatal()) << // @@ -3592,7 +3662,7 @@ ValidVault::finalize( "Invariant failed: clawback must change holder shares"; return false; // That's all we can do } - if (*maybeAccountDeltaShares >= zero) + if (maybeAccountDeltaShares->delta >= zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must decrease holder " @@ -3602,14 +3672,15 @@ ValidVault::finalize( // We don't need to round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *maybeAccountDeltaShares) + if (vaultDeltaShares->delta * -1 != + maybeAccountDeltaShares->delta) { JLOG(j.fatal()) << // "Invariant failed: clawback must change holder and " @@ -3650,28 +3721,17 @@ ValidVault::finalize( [[nodiscard]] std::int32_t ValidVault::computeMinScale( Asset const& asset, - std::initializer_list numbers) + std::initializer_list numbers) { if (numbers.size() == 0) return 0; - // Helper to get the minimal scale of an STAmount by removing trailing - // zeros from its mantissa - auto const getNatScale = [](Asset const& asset, - Number const& value) -> std::int32_t { - if (asset.integral()) - return 0; - if (value == beast::zero) - return value.exponent(); - - return value.scale(asset); - }; std::vector natScales; std::transform( numbers.begin(), numbers.end(), std::back_inserter(natScales), - [&](Number const& n) { return getNatScale(asset, n); }); + [&](DeltaInfo const& n) { return n.scale; }); return *std::max_element(natScales.begin(), natScales.end()); } diff --git a/src/xrpld/app/tx/detail/InvariantCheck.h b/src/xrpld/app/tx/detail/InvariantCheck.h index 3f61588cacd..2d1dbc0990c 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.h +++ b/src/xrpld/app/tx/detail/InvariantCheck.h @@ -880,11 +880,19 @@ class ValidVault Shares static make(SLE const&); }; +public: + struct DeltaInfo final + { + Number delta = numZero; + int scale = 0; + }; + +private: std::vector afterVault_ = {}; std::vector afterMPTs_ = {}; std::vector beforeVault_ = {}; std::vector beforeMPTs_ = {}; - std::unordered_map deltas_ = {}; + std::unordered_map deltas_ = {}; public: void @@ -905,7 +913,7 @@ class ValidVault [[nodiscard]] static std::int32_t computeMinScale( Asset const& asset, - std::initializer_list numbers); + std::initializer_list numbers); }; // additional invariant checks can be declared above and then added to this From cd4f915e40f90679b68850806cce7dd1ccfaec21 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 22 Jan 2026 18:27:29 -0500 Subject: [PATCH 03/11] Update src/xrpld/app/tx/detail/InvariantCheck.cpp Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> --- src/xrpld/app/tx/detail/InvariantCheck.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 7dd75ddc590..91a00236a09 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -2664,7 +2664,12 @@ ValidVault::visitEntry( // Number balanceDelta will capture the difference (delta) between "before" // state (zero if created) and "after" state (zero if destroyed), so the // invariants can validate that the change in account balances matches the - // change in vault balances, stored to deltas_ at the end of this function. + // balanceDelta captures the difference (delta) between "before" + // state (zero if created) and "after" state (zero if destroyed), and + // preserves value scale (exponent) to round values to the same scale during + // validation. It is used to validate that the change in account + // balances matches the change in vault balances, stored to deltas_ at the + // end of this function. DeltaInfo balanceDelta{numZero, STAmount::cMinOffset - 1}; std::int8_t sign = 0; From accbe7343d2951515e97d2e7bcc6e1e77a61469e Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 22 Jan 2026 19:45:22 -0500 Subject: [PATCH 04/11] Change ValidVault::DeltaInfo::scale to an optional --- src/xrpld/app/tx/detail/InvariantCheck.cpp | 23 +++++++++++++--------- src/xrpld/app/tx/detail/InvariantCheck.h | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 91a00236a09..26895cdd2fe 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -2670,7 +2670,7 @@ ValidVault::visitEntry( // validation. It is used to validate that the change in account // balances matches the change in vault balances, stored to deltas_ at the // end of this function. - DeltaInfo balanceDelta{numZero, STAmount::cMinOffset - 1}; + DeltaInfo balanceDelta{numZero, std::nullopt}; std::int8_t sign = 0; if (before) @@ -2753,8 +2753,8 @@ ValidVault::visitEntry( balanceDelta.delta -= Number(amount); // Trust Line balances are STAmounts, so we can use the exponent // directly to get the scale. - balanceDelta.scale = - std::max(balanceDelta.scale, amount.exponent()); + if (amount.exponent() > balanceDelta.scale) + balanceDelta.scale = amount.exponent(); sign = -1; break; } @@ -2770,6 +2770,10 @@ ValidVault::visitEntry( // against zero, to avoid missing such updates. if (sign != 0) { + XRPL_ASSERT_PARTS( + balanceDelta.scale, + "xrpl::ValidVault::visitEntry", + "scale initialized"); balanceDelta.delta *= sign; deltas_[key] = balanceDelta; } @@ -3727,14 +3731,15 @@ ValidVault::computeMinScale( if (numbers.size() == 0) return 0; - std::vector natScales; - std::transform( + auto const max = std::max_element( numbers.begin(), numbers.end(), - std::back_inserter(natScales), - [&](DeltaInfo const& n) { return n.scale; }); - - return *std::max_element(natScales.begin(), natScales.end()); + [](auto const& a, auto const& b) -> bool { return a.scale < b.scale; }); + XRPL_ASSERT_PARTS( + max->scale, + "xrpl::ValidVault::computeMinScale", + "scale set for destinationDelta"); + return max->scale.value_or(STAmount::cMaxOffset); } } // namespace xrpl diff --git a/src/xrpld/app/tx/detail/InvariantCheck.h b/src/xrpld/app/tx/detail/InvariantCheck.h index e0d3a95e765..574c8f7a061 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.h +++ b/src/xrpld/app/tx/detail/InvariantCheck.h @@ -883,7 +883,7 @@ class ValidVault struct DeltaInfo final { Number delta = numZero; - int scale = 0; + std::optional scale; }; private: From 5f8acbc001b972db6373edd009e46688617156e7 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Fri, 23 Jan 2026 15:51:38 -0500 Subject: [PATCH 05/11] Use 2^63-1 as maxMantissa for large range - That makes minMantissa 2^63/10+1. - Simplifies many of the existing operations, and removes the need for the accessors (mantissa() & exponent()) to do any math. --- include/xrpl/basics/Number.h | 171 ++++++--- include/xrpl/protocol/Protocol.h | 2 +- include/xrpl/protocol/SystemParameters.h | 2 +- src/libxrpl/basics/Number.cpp | 88 +++-- src/test/basics/Number_test.cpp | 419 +++++++++++++++-------- 5 files changed, 447 insertions(+), 235 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 118714460e6..45856aec3b4 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -17,18 +17,37 @@ class Number; std::string to_string(Number const& amount); +/** Returns a rough estimate of log10(value). + * + * The return value is a pair (log, rem), where log is the estimated log10, + * and rem is value divided by 10^log. If rem is 1, then value is an exact + * power of ten, and log is the exact log10(value). + * + * This function only works for positive values. + */ template -constexpr std::optional -logTen(T value) +constexpr std::pair +logTenEstimate(T value) { int log = 0; - while (value >= 10 && value % 10 == 0) + T remainder = value; + while (value >= 10) { + if (value % 10 == 0) + remainder = remainder / 10; value /= 10; ++log; } - if (value == 1) - return log; + return {log, remainder}; +} + +template +constexpr std::optional +logTen(T value) +{ + auto const est = logTenEstimate(value); + if (est.second == 1) + return est.first; return std::nullopt; } @@ -42,12 +61,10 @@ isPowerOfTen(T value) /** MantissaRange defines a range for the mantissa of a normalized Number. * * The mantissa is in the range [min, max], where - * * min is a power of 10, and - * * max = min * 10 - 1. * * The mantissa_scale enum indicates whether the range is "small" or "large". * This intentionally restricts the number of MantissaRanges that can be - * instantiated to two: one for each scale. + * used to two: one for each scale. * * The "small" scale is based on the behavior of STAmount for IOUs. It has a min * value of 10^15, and a max value of 10^16-1. This was sufficient for @@ -61,8 +78,8 @@ isPowerOfTen(T value) * "large" scale. * * The "large" scale is intended to represent all values that can be represented - * by an STAmount - IOUs, XRP, and MPTs. It has a min value of 10^18, and a max - * value of 10^19-1. + * by an STAmount - IOUs, XRP, and MPTs. It has a min value of 2^63/10+1 + * (truncated), and a max value of 2^63-1. * * Note that if the mentioned amendments are eventually retired, this class * should be left in place, but the "small" scale option should be removed. This @@ -74,28 +91,42 @@ struct MantissaRange enum mantissa_scale { small, large }; explicit constexpr MantissaRange(mantissa_scale scale_) - : min(getMin(scale_)) - , max(min * 10 - 1) - , log(logTen(min).value_or(-1)) + : max(getMax(scale_)) + , min(computeMin(max)) + , referenceMin(getReferenceMin(scale_, min)) + , log(computeLog(min)) , scale(scale_) { + // Since this is constexpr, if any of these throw, it won't compile + if (min * 10 <= max) + throw std::out_of_range("min * 10 <= max"); + if (max / 10 >= min) + throw std::out_of_range("max / 10 >= min"); + if ((min - 1) * 10 > max) + throw std::out_of_range("(min - 1) * 10 > max"); + // This is a little hacky + if ((max + 10) / 10 < min) + throw std::out_of_range("(max + 10) / 10 < min"); } - rep min; rep max; + rep min; + // This is not a great name. Used to determine if mantissas are in range, + // but have fewer digits than max + rep referenceMin; int log; mantissa_scale scale; private: static constexpr rep - getMin(mantissa_scale scale_) + getMax(mantissa_scale scale) { - switch (scale_) + switch (scale) { case small: - return 1'000'000'000'000'000ULL; + return 9'999'999'999'999'999ULL; case large: - return 1'000'000'000'000'000'000ULL; + return std::numeric_limits::max(); default: // Since this can never be called outside a non-constexpr // context, this throw assures that the build fails if an @@ -103,6 +134,38 @@ struct MantissaRange throw std::runtime_error("Unknown mantissa scale"); } } + + static constexpr rep + computeMin(rep max) + { + auto min = max + 1; + auto const r = min % 10; + min /= 10; + if (r != 0) + ++min; + return min; + } + + static constexpr rep + getReferenceMin(mantissa_scale scale, rep min) + { + switch (scale) + { + case large: + return 1'000'000'000'000'000'000ULL; + default: + if (isPowerOfTen(min)) + return min; + throw std::runtime_error("Unknown/bad mantissa scale"); + } + } + + static constexpr rep + computeLog(rep min) + { + auto const estimate = logTenEstimate(min); + return estimate.first + (estimate.second == 1 ? 0 : 1); + } }; // Like std::integral, but only 64-bit integral types. @@ -141,9 +204,7 @@ concept CanUseAsScale = requires(Asset a, Number n) { STAmount(a, n); } && * 1. Normalization can be disabled by using the "unchecked" ctor tag. This * should only be used at specific conversion points, some constexpr * values, and in unit tests. - * 2. The max of the "large" range, 10^19-1, is the largest 10^X-1 value that - * fits in an unsigned 64-bit number. (10^19-1 < 2^64-1 and - * 10^20-1 > 2^64-1). This avoids under- and overflows. + * 2. The max of the "large" range, 2^63-1, TODO: explain the large range. * * ---- External Interface ---- * @@ -157,7 +218,7 @@ concept CanUseAsScale = requires(Asset a, Number n) { STAmount(a, n); } && * * Note: * 1. 2^63-1 is between 10^18 and 10^19-1, which are the limits of the "large" - * mantissa range. + * mantissa range. TODO: update this explanation. * 2. The functions mantissa() and exponent() return the external view of the * Number value, specifically using a signed 63-bit mantissa. This may * require altering the internal representation to fit into that range @@ -217,6 +278,7 @@ class Number using rep = std::int64_t; using internalrep = MantissaRange::rep; + // TODO: Get rid of negative_ and convert mantissa back to rep bool negative_{false}; internalrep mantissa_{0}; int exponent_{std::numeric_limits::lowest()}; @@ -226,9 +288,11 @@ class Number constexpr static int minExponent = -32768; constexpr static int maxExponent = 32768; +#if MAXREP constexpr static internalrep maxRep = std::numeric_limits::max(); static_assert(maxRep == 9'223'372'036'854'775'807); static_assert(-maxRep == std::numeric_limits::min() + 1); +#endif // May need to make unchecked private struct unchecked @@ -481,16 +545,31 @@ class Number static_assert(isPowerOfTen(smallRange.min)); static_assert(smallRange.min == 1'000'000'000'000'000LL); static_assert(smallRange.max == 9'999'999'999'999'999LL); + static_assert(smallRange.referenceMin == smallRange.min); static_assert(smallRange.log == 15); +#if MAXREP static_assert(smallRange.min < maxRep); static_assert(smallRange.max < maxRep); +#endif constexpr static MantissaRange largeRange{MantissaRange::large}; - static_assert(isPowerOfTen(largeRange.min)); - static_assert(largeRange.min == 1'000'000'000'000'000'000ULL); - static_assert(largeRange.max == internalrep(9'999'999'999'999'999'999ULL)); + static_assert(!isPowerOfTen(largeRange.min)); + static_assert(largeRange.min == 922'337'203'685'477'581ULL); + static_assert(largeRange.max == internalrep(9'223'372'036'854'775'807ULL)); + static_assert(largeRange.max == std::numeric_limits::max()); + static_assert(largeRange.referenceMin == 1'000'000'000'000'000'000ULL); static_assert(largeRange.log == 18); + // There are 2 values that will not fit in largeRange without some extra + // work + // * 9223372036854775808 + // * 9223372036854775809 + // They both end up < min, but with a leftover. If they round up, everything + // will be fine. If they don't, well need to bring them up into range. + // Guard::bringIntoRange handles this situation. + +#if MAXREP static_assert(largeRange.min < maxRep); static_assert(largeRange.max > maxRep); +#endif // The range for the mantissa when normalized. // Use reference_wrapper to avoid making copies, and prevent accidentally @@ -540,7 +619,16 @@ class Number static internalrep externalToInternal(rep mantissa); + // Safely convert Number to the internal rep where the mantissa always has + // the same number of digits + template + std::tuple + toInternal() const; + class Guard; + +public: + constexpr static internalrep largestMantissa = largeRange.max; }; inline constexpr Number::Number( @@ -594,17 +682,8 @@ inline Number::Number(rep mantissa) : Number{mantissa, 0} inline constexpr Number::rep Number::mantissa() const noexcept { - auto m = mantissa_; - if (m > maxRep) - { - XRPL_ASSERT_PARTS( - !isnormal() || (m % 10 == 0 && m / 10 <= maxRep), - "xrpl::Number::mantissa", - "large normalized mantissa has no remainder"); - m /= 10; - } auto const sign = negative_ ? -1 : 1; - return sign * static_cast(m); + return sign * static_cast(mantissa_); } /** Returns the exponent of the external view of the Number. @@ -615,16 +694,7 @@ Number::mantissa() const noexcept inline constexpr int Number::exponent() const noexcept { - auto e = exponent_; - if (mantissa_ > maxRep) - { - XRPL_ASSERT_PARTS( - !isnormal() || (mantissa_ % 10 == 0 && mantissa_ / 10 <= maxRep), - "xrpl::Number::exponent", - "large normalized mantissa has no remainder"); - ++e; - } - return e; + return exponent_; } template @@ -728,15 +798,13 @@ Number::min() noexcept inline Number Number::max() noexcept { - return Number{ - false, std::min(range_.get().max, maxRep), maxExponent, unchecked{}}; + return Number{false, range_.get().max, maxExponent, unchecked{}}; } inline Number Number::lowest() noexcept { - return Number{ - true, std::min(range_.get().max, maxRep), maxExponent, unchecked{}}; + return Number{true, range_.get().max, maxExponent, unchecked{}}; } inline bool @@ -745,9 +813,8 @@ Number::isnormal() const noexcept MantissaRange const& range = range_; auto const abs_m = mantissa_; return *this == Number{} || - (range.min <= abs_m && abs_m <= range.max && - (abs_m <= maxRep || abs_m % 10 == 0) && minExponent <= exponent_ && - exponent_ <= maxExponent); + (range.min <= abs_m && abs_m <= range.max && // + minExponent <= exponent_ && exponent_ <= maxExponent); } template diff --git a/include/xrpl/protocol/Protocol.h b/include/xrpl/protocol/Protocol.h index 43be9d3b45c..30065b38bfa 100644 --- a/include/xrpl/protocol/Protocol.h +++ b/include/xrpl/protocol/Protocol.h @@ -233,7 +233,7 @@ std::size_t constexpr maxMPTokenMetadataLength = 1024; /** The maximum amount of MPTokenIssuance */ std::uint64_t constexpr maxMPTokenAmount = 0x7FFF'FFFF'FFFF'FFFFull; -static_assert(Number::maxRep >= maxMPTokenAmount); +static_assert(Number::largestMantissa >= maxMPTokenAmount); /** The maximum length of Data payload */ std::size_t constexpr maxDataPayloadLength = 256; diff --git a/include/xrpl/protocol/SystemParameters.h b/include/xrpl/protocol/SystemParameters.h index c2f66e9ea11..fe81ca9bbd8 100644 --- a/include/xrpl/protocol/SystemParameters.h +++ b/include/xrpl/protocol/SystemParameters.h @@ -24,7 +24,7 @@ systemName() /** Number of drops in the genesis account. */ constexpr XRPAmount INITIAL_XRP{100'000'000'000 * DROPS_PER_XRP}; static_assert(INITIAL_XRP.drops() == 100'000'000'000'000'000); -static_assert(Number::maxRep >= INITIAL_XRP.drops()); +static_assert(Number::largestMantissa >= INITIAL_XRP.drops()); /** Returns true if the amount does not exceed the initial XRP in existence. */ inline bool diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 436ebf67790..aa88c682102 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -260,7 +260,7 @@ Number::Guard::doRoundUp( ++mantissa; // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". - if (mantissa > maxMantissa || mantissa > maxRep) + if (mantissa > maxMantissa) { mantissa /= 10; ++exponent; @@ -299,7 +299,7 @@ Number::Guard::doRound(rep& drops, std::string location) auto r = round(); if (r == 1 || (r == 0 && (drops & 1) == 1)) { - if (drops >= maxRep) + if (drops >= maxMantissa()) { static_assert(sizeof(internalrep) == sizeof(rep)); // This should be impossible, because it's impossible to represent @@ -341,12 +341,36 @@ Number::externalToInternal(rep mantissa) return static_cast(-temp); } +template +std::tuple +Number::toInternal() const +{ + auto exponent = exponent_; + Rep mantissa = mantissa_; + bool const negative = negative_; + + auto const referenceMin = Number::range_.get().referenceMin; + + if (mantissa != 0 && mantissa < referenceMin) + { + // Ensure the mantissa has the correct number of digits + mantissa *= 10; + --exponent; + } + XRPL_ASSERT_PARTS( + mantissa >= referenceMin && mantissa < referenceMin * 10, + "ripple::Number::toInternal()", + "Number is within reference range and has 'log' digits"); + + return {negative, mantissa, exponent}; +} + constexpr Number Number::oneSmall() { return Number{ false, - Number::smallRange.min, + Number::smallRange.referenceMin, -Number::smallRange.log, Number::unchecked{}}; }; @@ -358,7 +382,7 @@ Number::oneLarge() { return Number{ false, - Number::largeRange.min, + Number::largeRange.referenceMin, -Number::largeRange.log, Number::unchecked{}}; }; @@ -387,18 +411,18 @@ doNormalize( { auto constexpr minExponent = Number::minExponent; auto constexpr maxExponent = Number::maxExponent; - auto constexpr maxRep = Number::maxRep; using Guard = Number::Guard; constexpr Number zero = Number{}; - if (mantissa_ == 0) + if (mantissa_ == 0 || (mantissa_ < minMantissa && exponent_ <= minExponent)) { mantissa_ = zero.mantissa_; exponent_ = zero.exponent_; negative = zero.negative_; return; } + auto m = mantissa_; while ((m < minMantissa) && (exponent_ > minExponent)) { @@ -416,7 +440,7 @@ doNormalize( m /= 10; ++exponent_; } - if ((exponent_ < minExponent) || (m < minMantissa)) + if ((exponent_ < minExponent) || (m == 0)) { mantissa_ = zero.mantissa_; exponent_ = zero.exponent_; @@ -424,33 +448,8 @@ doNormalize( return; } - // When using the largeRange, "m" needs fit within an int64, even if - // the final mantissa_ is going to end up larger to fit within the - // MantissaRange. Cut it down here so that the rounding will be done while - // it's smaller. - // - // Example: 9,900,000,000,000,123,456 > 9,223,372,036,854,775,807, - // so "m" will be modified to 990,000,000,000,012,345. Then that value - // will be rounded to 990,000,000,000,012,345 or - // 990,000,000,000,012,346, depending on the rounding mode. Finally, - // mantissa_ will be "m*10" so it fits within the range, and end up as - // 9,900,000,000,000,123,450 or 9,900,000,000,000,123,460. - // mantissa() will return mantissa_ / 10, and exponent() will return - // exponent_ + 1. - if (m > maxRep) - { - if (exponent_ >= maxExponent) - throw std::overflow_error("Number::normalize 1.5"); - g.push(m % 10); - m /= 10; - ++exponent_; - } - // Before modification, m should be within the min/max range. After - // modification, it must be less than maxRep. In other words, the original - // value should have been no more than maxRep * 10. - // (maxRep * 10 > maxMantissa) XRPL_ASSERT_PARTS( - m <= maxRep, + m <= maxMantissa, "xrpl::doNormalize", "intermediate mantissa fits in int64"); mantissa_ = m; @@ -462,6 +461,7 @@ doNormalize( minMantissa, maxMantissa, "Number::normalize 2"); + XRPL_ASSERT_PARTS( mantissa_ >= minMantissa && mantissa_ <= maxMantissa, "xrpl::doNormalize", @@ -560,13 +560,10 @@ Number::operator+=(Number const& y) // Need to use uint128_t, because large mantissas can overflow when added // together. - bool xn = negative_; - uint128_t xm = mantissa_; - auto xe = exponent_; + auto [xn, xm, xe] = toInternal(); + + auto [yn, ym, ye] = y.toInternal(); - bool yn = y.negative_; - uint128_t ym = y.mantissa_; - auto ye = y.exponent_; Guard g; if (xe < ye) { @@ -598,7 +595,7 @@ Number::operator+=(Number const& y) if (xn == yn) { xm += ym; - if (xm > maxMantissa || xm > maxRep) + if (xm > maxMantissa) { g.push(xm % 10); xm /= 10; @@ -619,7 +616,7 @@ Number::operator+=(Number const& y) xe = ye; xn = yn; } - while (xm < minMantissa && xm * 10 <= maxRep) + while (xm < minMantissa) { xm *= 10; xm -= g.pop(); @@ -701,7 +698,7 @@ Number::operator*=(Number const& y) auto const& minMantissa = range.min; auto const& maxMantissa = range.max; - while (zm > maxMantissa || zm > maxRep) + while (zm > maxMantissa) { // The following is optimization for: // g.push(static_cast(zm % 10)); @@ -843,7 +840,7 @@ Number::operator rep() const } for (; offset > 0; --offset) { - if (drops > maxRep / 10) + if (drops > largeRange.max / 10) throw std::overflow_error("Number::operator rep() overflow"); drops *= 10; } @@ -878,9 +875,8 @@ to_string(Number const& amount) if (amount == zero) return "0"; - auto exponent = amount.exponent_; - auto mantissa = amount.mantissa_; - bool const negative = amount.negative_; + // The mantissa must have a set number of decimal places for this to work + auto [negative, mantissa, exponent] = amount.toInternal(); // Use scientific notation for exponents that are too small or too large auto const rangeLog = Number::mantissaLog(); diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 1fa5ae6e8fb..db169fc1f9b 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -32,9 +32,10 @@ class Number_test : public beast::unit_test::suite test_limits() { auto const scale = Number::getMantissaScale(); - testcase << "test_limits " << to_string(scale); - bool caught = false; auto const minMantissa = Number::minMantissa(); + + testcase << "test_limits " << to_string(scale) << ", " << minMantissa; + bool caught = false; try { Number x = @@ -62,8 +63,9 @@ class Number_test : public beast::unit_test::suite Number{}, __LINE__); test( + // Use 1501 to force rounding up Number{false, minMantissa, 32000, Number::normalized{}} * 1'000 + - Number{false, 1'500, 32000, Number::normalized{}}, + Number{false, 1'501, 32000, Number::normalized{}}, Number{false, minMantissa + 2, 32003, Number::normalized{}}, __LINE__); // 9,223,372,036,854,775,808 @@ -198,12 +200,12 @@ class Number_test : public beast::unit_test::suite 9'999'999'999'999'999'990ULL, -19, Number::normalized{}}}, - {Number{Number::maxRep}, + {Number{Number::largestMantissa}, Number{6, -1}, - Number{Number::maxRep / 10, 1}}, - {Number{Number::maxRep - 1}, + Number{Number::largestMantissa / 10, 1}}, + {Number{Number::largestMantissa - 1}, Number{1, 0}, - Number{Number::maxRep}}, + Number{Number::largestMantissa}}, // Test extremes { // Each Number operand rounds up, so the actual mantissa is @@ -221,11 +223,11 @@ class Number_test : public beast::unit_test::suite Number{2, 19}, }, { - // Does not round. Mantissas are going to be > maxRep, so if - // added together as uint64_t's, the result will overflow. - // With addition using uint128_t, there's no problem. After - // normalizing, the resulting mantissa ends up less than - // maxRep. + // Does not round. Mantissas are going to be > + // largestMantissa, so if added together as uint64_t's, the + // result will overflow. With addition using uint128_t, + // there's no problem. After normalizing, the resulting + // mantissa ends up less than largestMantissa. Number{ false, 9'999'999'999'999'999'990ULL, @@ -352,16 +354,24 @@ class Number_test : public beast::unit_test::suite {Number{1'000'000'000'000'000'001, -18}, Number{1'000'000'000'000'000'000, -18}, Number{1'000'000'000'000'000'000, -36}}, - {Number{Number::maxRep}, + {Number{Number::largestMantissa}, Number{6, -1}, - Number{Number::maxRep - 1}}, - {Number{false, Number::maxRep + 1, 0, Number::normalized{}}, + Number{Number::largestMantissa - 1}}, + {Number{ + false, + Number::largestMantissa + 1, + 0, + Number::normalized{}}, Number{1, 0}, - Number{Number::maxRep / 10 + 1, 1}}, - {Number{false, Number::maxRep + 1, 0, Number::normalized{}}, + Number{Number::largestMantissa / 10 + 1, 1}}, + {Number{ + false, + Number::largestMantissa + 1, + 0, + Number::normalized{}}, Number{3, 0}, - Number{Number::maxRep}}, - {power(2, 63), Number{3, 0}, Number{Number::maxRep}}, + Number{Number::largestMantissa}}, + {power(2, 63), Number{3, 0}, Number{Number::largestMantissa}}, }); auto test = [this](auto const& c) { for (auto const& [x, y, z] : c) @@ -384,14 +394,16 @@ class Number_test : public beast::unit_test::suite auto const scale = Number::getMantissaScale(); testcase << "test_mul " << to_string(scale); - using Case = std::tuple; + // Case: Factor 1, Factor 2, Expected product, Line number + using Case = std::tuple; auto test = [this](auto const& c) { - for (auto const& [x, y, z] : c) + for (auto const& [x, y, z, line] : c) { auto const result = x * y; std::stringstream ss; ss << x << " * " << y << " = " << result << ". Expected: " << z; - BEAST_EXPECTS(result == z, ss.str()); + BEAST_EXPECTS( + result == z, ss.str() + " line: " + std::to_string(line)); } }; auto tests = [&](auto const& cSmall, auto const& cLarge) { @@ -401,78 +413,105 @@ class Number_test : public beast::unit_test::suite test(cLarge); }; auto const maxMantissa = Number::maxMantissa(); + auto const maxInternalMantissa = + static_cast( + static_cast(power(10, Number::mantissaLog()))) * + 10 - + 1; saveNumberRoundMode save{Number::setround(Number::to_nearest)}; { auto const cSmall = std::to_array({ - {Number{7}, Number{8}, Number{56}}, + {Number{7}, Number{8}, Number{56}, __LINE__}, {Number{1414213562373095, -15}, Number{1414213562373095, -15}, - Number{2000000000000000, -15}}, + Number{2000000000000000, -15}, + __LINE__}, {Number{-1414213562373095, -15}, Number{1414213562373095, -15}, - Number{-2000000000000000, -15}}, + Number{-2000000000000000, -15}, + __LINE__}, {Number{-1414213562373095, -15}, Number{-1414213562373095, -15}, - Number{2000000000000000, -15}}, + Number{2000000000000000, -15}, + __LINE__}, {Number{3214285714285706, -15}, Number{3111111111111119, -15}, - Number{1000000000000000, -14}}, + Number{1000000000000000, -14}, + __LINE__}, {Number{1000000000000000, -32768}, Number{1000000000000000, -32768}, - Number{0}}, + Number{0}, + __LINE__}, // Maximum mantissa range {Number{9'999'999'999'999'999, 0}, Number{9'999'999'999'999'999, 0}, - Number{9'999'999'999'999'998, 16}}, + Number{9'999'999'999'999'998, 16}, + __LINE__}, }); auto const cLarge = std::to_array({ // Note that items with extremely large mantissas need to be // calculated, because otherwise they overflow uint64. Items // from C with larger mantissa - {Number{7}, Number{8}, Number{56}}, + {Number{7}, Number{8}, Number{56}, __LINE__}, {Number{1414213562373095, -15}, Number{1414213562373095, -15}, - Number{1999999999999999862, -18}}, + Number{1999999999999999862, -18}, + __LINE__}, {Number{-1414213562373095, -15}, Number{1414213562373095, -15}, - Number{-1999999999999999862, -18}}, + Number{-1999999999999999862, -18}, + __LINE__}, {Number{-1414213562373095, -15}, Number{-1414213562373095, -15}, - Number{1999999999999999862, -18}}, + Number{1999999999999999862, -18}, + __LINE__}, {Number{3214285714285706, -15}, Number{3111111111111119, -15}, Number{ false, 9'999'999'999'999'999'579ULL, -18, - Number::normalized{}}}, + Number::normalized{}}, + __LINE__}, {Number{1000000000000000000, -32768}, Number{1000000000000000000, -32768}, - Number{0}}, + Number{0}, + __LINE__}, // Items from cSmall expanded for the larger mantissa, // except duplicates. Sadly, it looks like sqrt(2)^2 != 2 // with higher precision {Number{1414213562373095049, -18}, Number{1414213562373095049, -18}, - Number{2000000000000000001, -18}}, + Number{2000000000000000001, -18}, + __LINE__}, {Number{-1414213562373095048, -18}, Number{1414213562373095048, -18}, - Number{-1999999999999999998, -18}}, + Number{-1999999999999999998, -18}, + __LINE__}, {Number{-1414213562373095048, -18}, Number{-1414213562373095049, -18}, - Number{1999999999999999999, -18}}, + Number{1999999999999999999, -18}, + __LINE__}, {Number{3214285714285714278, -18}, Number{3111111111111111119, -18}, - Number{10, 0}}, - // Maximum mantissa range - rounds up to 1e19 + Number{10, 0}, + __LINE__}, + // Maximum internal mantissa range - rounds up to 1e19 + {Number{false, maxInternalMantissa, 0, Number::normalized{}}, + Number{false, maxInternalMantissa, 0, Number::normalized{}}, + Number{1, 38}, + __LINE__}, + // Maximum actual mantissa range - same as int64 range {Number{false, maxMantissa, 0, Number::normalized{}}, Number{false, maxMantissa, 0, Number::normalized{}}, - Number{1, 38}}, + Number{85'070'591'730'234'615'85, 19}, + __LINE__}, // Maximum int64 range - {Number{Number::maxRep, 0}, - Number{Number::maxRep, 0}, - Number{85'070'591'730'234'615'85, 19}}, + {Number{Number::largestMantissa, 0}, + Number{Number::largestMantissa, 0}, + Number{85'070'591'730'234'615'85, 19}, + __LINE__}, }); tests(cSmall, cLarge); } @@ -481,76 +520,101 @@ class Number_test : public beast::unit_test::suite << " towards_zero"; { auto const cSmall = std::to_array( - {{Number{7}, Number{8}, Number{56}}, + {{Number{7}, Number{8}, Number{56}, __LINE__}, {Number{1414213562373095, -15}, Number{1414213562373095, -15}, - Number{1999999999999999, -15}}, + Number{1999999999999999, -15}, + __LINE__}, {Number{-1414213562373095, -15}, Number{1414213562373095, -15}, - Number{-1999999999999999, -15}}, + Number{-1999999999999999, -15}, + __LINE__}, {Number{-1414213562373095, -15}, Number{-1414213562373095, -15}, - Number{1999999999999999, -15}}, + Number{1999999999999999, -15}, + __LINE__}, {Number{3214285714285706, -15}, Number{3111111111111119, -15}, - Number{9999999999999999, -15}}, + Number{9999999999999999, -15}, + __LINE__}, {Number{1000000000000000, -32768}, Number{1000000000000000, -32768}, - Number{0}}}); + Number{0}, + __LINE__}}); auto const cLarge = std::to_array( // Note that items with extremely large mantissas need to be // calculated, because otherwise they overflow uint64. Items // from C with larger mantissa { - {Number{7}, Number{8}, Number{56}}, + {Number{7}, Number{8}, Number{56}, __LINE__}, {Number{1414213562373095, -15}, Number{1414213562373095, -15}, - Number{1999999999999999861, -18}}, + Number{1999999999999999861, -18}, + __LINE__}, {Number{-1414213562373095, -15}, Number{1414213562373095, -15}, - Number{-1999999999999999861, -18}}, + Number{-1999999999999999861, -18}, + __LINE__}, {Number{-1414213562373095, -15}, Number{-1414213562373095, -15}, - Number{1999999999999999861, -18}}, + Number{1999999999999999861, -18}, + __LINE__}, {Number{3214285714285706, -15}, Number{3111111111111119, -15}, Number{ false, 9999999999999999579ULL, -18, - Number::normalized{}}}, + Number::normalized{}}, + __LINE__}, {Number{1000000000000000000, -32768}, Number{1000000000000000000, -32768}, - Number{0}}, + Number{0}, + __LINE__}, // Items from cSmall expanded for the larger mantissa, // except duplicates. Sadly, it looks like sqrt(2)^2 != 2 // with higher precision {Number{1414213562373095049, -18}, Number{1414213562373095049, -18}, - Number{2, 0}}, + Number{2, 0}, + __LINE__}, {Number{-1414213562373095048, -18}, Number{1414213562373095048, -18}, - Number{-1999999999999999997, -18}}, + Number{-1999999999999999997, -18}, + __LINE__}, {Number{-1414213562373095048, -18}, Number{-1414213562373095049, -18}, - Number{1999999999999999999, -18}}, + Number{1999999999999999999, -18}, + __LINE__}, {Number{3214285714285714278, -18}, Number{3111111111111111119, -18}, - Number{10, 0}}, - // Maximum mantissa range - rounds down to maxMantissa/10e1 + Number{10, 0}, + __LINE__}, + // Maximum internal mantissa range - rounds down to + // maxMantissa/10e1 // 99'999'999'999'999'999'800'000'000'000'000'000'100 - {Number{false, maxMantissa, 0, Number::normalized{}}, - Number{false, maxMantissa, 0, Number::normalized{}}, + {Number{ + false, maxInternalMantissa, 0, Number::normalized{}}, + Number{ + false, maxInternalMantissa, 0, Number::normalized{}}, Number{ false, - maxMantissa / 10 - 1, + maxInternalMantissa / 10 - 1, 20, - Number::normalized{}}}, + Number::normalized{}}, + __LINE__}, + // Maximum actual mantissa range - same as int64 + // 99'999'999'999'999'999'800'000'000'000'000'000'100 + {Number{false, maxMantissa, 0, Number::normalized{}}, + Number{false, maxMantissa, 0, Number::normalized{}}, + Number{85'070'591'730'234'615'84, 19}, + __LINE__}, // Maximum int64 range // 85'070'591'730'234'615'847'396'907'784'232'501'249 - {Number{Number::maxRep, 0}, - Number{Number::maxRep, 0}, - Number{85'070'591'730'234'615'84, 19}}, + {Number{Number::largestMantissa, 0}, + Number{Number::largestMantissa, 0}, + Number{85'070'591'730'234'615'84, 19}, + __LINE__}, }); tests(cSmall, cLarge); } @@ -559,76 +623,100 @@ class Number_test : public beast::unit_test::suite << " downward"; { auto const cSmall = std::to_array( - {{Number{7}, Number{8}, Number{56}}, + {{Number{7}, Number{8}, Number{56}, __LINE__}, {Number{1414213562373095, -15}, Number{1414213562373095, -15}, - Number{1999999999999999, -15}}, + Number{1999999999999999, -15}, + __LINE__}, {Number{-1414213562373095, -15}, Number{1414213562373095, -15}, - Number{-2000000000000000, -15}}, + Number{-2000000000000000, -15}, + __LINE__}, {Number{-1414213562373095, -15}, Number{-1414213562373095, -15}, - Number{1999999999999999, -15}}, + Number{1999999999999999, -15}, + __LINE__}, {Number{3214285714285706, -15}, Number{3111111111111119, -15}, - Number{9999999999999999, -15}}, + Number{9999999999999999, -15}, + __LINE__}, {Number{1000000000000000, -32768}, Number{1000000000000000, -32768}, - Number{0}}}); + Number{0}, + __LINE__}}); auto const cLarge = std::to_array( // Note that items with extremely large mantissas need to be // calculated, because otherwise they overflow uint64. Items // from C with larger mantissa { - {Number{7}, Number{8}, Number{56}}, + {Number{7}, Number{8}, Number{56}, __LINE__}, {Number{1414213562373095, -15}, Number{1414213562373095, -15}, - Number{1999999999999999861, -18}}, + Number{1999999999999999861, -18}, + __LINE__}, {Number{-1414213562373095, -15}, Number{1414213562373095, -15}, - Number{-1999999999999999862, -18}}, + Number{-1999999999999999862, -18}, + __LINE__}, {Number{-1414213562373095, -15}, Number{-1414213562373095, -15}, - Number{1999999999999999861, -18}}, + Number{1999999999999999861, -18}, + __LINE__}, {Number{3214285714285706, -15}, Number{3111111111111119, -15}, Number{ false, 9'999'999'999'999'999'579ULL, -18, - Number::normalized{}}}, + Number::normalized{}}, + __LINE__}, {Number{1000000000000000000, -32768}, Number{1000000000000000000, -32768}, - Number{0}}, + Number{0}, + __LINE__}, // Items from cSmall expanded for the larger mantissa, // except duplicates. Sadly, it looks like sqrt(2)^2 != 2 // with higher precision {Number{1414213562373095049, -18}, Number{1414213562373095049, -18}, - Number{2, 0}}, + Number{2, 0}, + __LINE__}, {Number{-1414213562373095048, -18}, Number{1414213562373095048, -18}, - Number{-1999999999999999998, -18}}, + Number{-1999999999999999998, -18}, + __LINE__}, {Number{-1414213562373095048, -18}, Number{-1414213562373095049, -18}, - Number{1999999999999999999, -18}}, + Number{1999999999999999999, -18}, + __LINE__}, {Number{3214285714285714278, -18}, Number{3111111111111111119, -18}, - Number{10, 0}}, - // Maximum mantissa range - rounds down to maxMantissa/10e1 + Number{10, 0}, + __LINE__}, + // Maximum internal mantissa range - rounds down to + // maxMantissa/10-1 // 99'999'999'999'999'999'800'000'000'000'000'000'100 - {Number{false, maxMantissa, 0, Number::normalized{}}, - Number{false, maxMantissa, 0, Number::normalized{}}, + {Number{ + false, maxInternalMantissa, 0, Number::normalized{}}, + Number{ + false, maxInternalMantissa, 0, Number::normalized{}}, Number{ false, - maxMantissa / 10 - 1, + maxInternalMantissa / 10 - 1, 20, - Number::normalized{}}}, + Number::normalized{}}, + __LINE__}, + // Maximum mantissa range - same as int64 + {Number{false, maxMantissa, 0, Number::normalized{}}, + Number{false, maxMantissa, 0, Number::normalized{}}, + Number{85'070'591'730'234'615'84, 19}, + __LINE__}, // Maximum int64 range // 85'070'591'730'234'615'847'396'907'784'232'501'249 - {Number{Number::maxRep, 0}, - Number{Number::maxRep, 0}, - Number{85'070'591'730'234'615'84, 19}}, + {Number{Number::largestMantissa, 0}, + Number{Number::largestMantissa, 0}, + Number{85'070'591'730'234'615'84, 19}, + __LINE__}, }); tests(cSmall, cLarge); } @@ -637,68 +725,91 @@ class Number_test : public beast::unit_test::suite << " upward"; { auto const cSmall = std::to_array( - {{Number{7}, Number{8}, Number{56}}, + {{Number{7}, Number{8}, Number{56}, __LINE__}, {Number{1414213562373095, -15}, Number{1414213562373095, -15}, - Number{2000000000000000, -15}}, + Number{2000000000000000, -15}, + __LINE__}, {Number{-1414213562373095, -15}, Number{1414213562373095, -15}, - Number{-1999999999999999, -15}}, + Number{-1999999999999999, -15}, + __LINE__}, {Number{-1414213562373095, -15}, Number{-1414213562373095, -15}, - Number{2000000000000000, -15}}, + Number{2000000000000000, -15}, + __LINE__}, {Number{3214285714285706, -15}, Number{3111111111111119, -15}, - Number{1000000000000000, -14}}, + Number{1000000000000000, -14}, + __LINE__}, {Number{1000000000000000, -32768}, Number{1000000000000000, -32768}, - Number{0}}}); + Number{0}, + __LINE__}}); auto const cLarge = std::to_array( // Note that items with extremely large mantissas need to be // calculated, because otherwise they overflow uint64. Items // from C with larger mantissa { - {Number{7}, Number{8}, Number{56}}, + {Number{7}, Number{8}, Number{56}, __LINE__}, {Number{1414213562373095, -15}, Number{1414213562373095, -15}, - Number{1999999999999999862, -18}}, + Number{1999999999999999862, -18}, + __LINE__}, {Number{-1414213562373095, -15}, Number{1414213562373095, -15}, - Number{-1999999999999999861, -18}}, + Number{-1999999999999999861, -18}, + __LINE__}, {Number{-1414213562373095, -15}, Number{-1414213562373095, -15}, - Number{1999999999999999862, -18}}, + Number{1999999999999999862, -18}, + __LINE__}, {Number{3214285714285706, -15}, Number{3111111111111119, -15}, - Number{999999999999999958, -17}}, + Number{999999999999999958, -17}, + __LINE__}, {Number{1000000000000000000, -32768}, Number{1000000000000000000, -32768}, - Number{0}}, + Number{0}, + __LINE__}, // Items from cSmall expanded for the larger mantissa, // except duplicates. Sadly, it looks like sqrt(2)^2 != 2 // with higher precision {Number{1414213562373095049, -18}, Number{1414213562373095049, -18}, - Number{2000000000000000001, -18}}, + Number{2000000000000000001, -18}, + __LINE__}, {Number{-1414213562373095048, -18}, Number{1414213562373095048, -18}, - Number{-1999999999999999997, -18}}, + Number{-1999999999999999997, -18}, + __LINE__}, {Number{-1414213562373095048, -18}, Number{-1414213562373095049, -18}, - Number{2, 0}}, + Number{2, 0}, + __LINE__}, {Number{3214285714285714278, -18}, Number{3111111111111111119, -18}, - Number{1000000000000000001, -17}}, - // Maximum mantissa range - rounds up to minMantissa*10 - // 1e19*1e19=1e38 + Number{1000000000000000001, -17}, + __LINE__}, + // Maximum internal mantissa range - rounds up to + // minMantissa*10 1e19*1e19=1e38 + {Number{ + false, maxInternalMantissa, 0, Number::normalized{}}, + Number{ + false, maxInternalMantissa, 0, Number::normalized{}}, + Number{1, 38}, + __LINE__}, + // Maximum mantissa range - same as int64 {Number{false, maxMantissa, 0, Number::normalized{}}, Number{false, maxMantissa, 0, Number::normalized{}}, - Number{1, 38}}, + Number{85'070'591'730'234'615'85, 19}, + __LINE__}, // Maximum int64 range // 85'070'591'730'234'615'847'396'907'784'232'501'249 - {Number{Number::maxRep, 0}, - Number{Number::maxRep, 0}, - Number{85'070'591'730'234'615'85, 19}}, + {Number{Number::largestMantissa, 0}, + Number{Number::largestMantissa, 0}, + Number{85'070'591'730'234'615'85, 19}, + __LINE__}, }); tests(cSmall, cLarge); } @@ -971,6 +1082,13 @@ class Number_test : public beast::unit_test::suite }; */ + auto const maxMantissa = Number::maxMantissa(); + auto const maxInternalMantissa = + static_cast( + static_cast(power(10, Number::mantissaLog()))) * + 10 - + 1; + auto const cSmall = std::to_array( {{Number{2}, 2, Number{1414213562373095049, -18}}, {Number{2'000'000}, 2, Number{1414213562373095049, -15}}, @@ -982,17 +1100,17 @@ class Number_test : public beast::unit_test::suite {Number{0}, 5, Number{0}}, {Number{5625, -4}, 2, Number{75, -2}}}); auto const cLarge = std::to_array({ - {Number{false, Number::maxMantissa() - 9, -1, Number::normalized{}}, + {Number{false, maxInternalMantissa - 9, -1, Number::normalized{}}, 2, Number{false, 999'999'999'999'999'999, -9, Number::normalized{}}}, - {Number{false, Number::maxMantissa() - 9, 0, Number::normalized{}}, + {Number{false, maxInternalMantissa - 9, 0, Number::normalized{}}, 2, Number{ false, 3'162'277'660'168'379'330, -9, Number::normalized{}}}, - {Number{Number::maxRep}, + {Number{Number::largestMantissa}, 2, Number{false, 3'037'000'499'976049692, -9, Number::normalized{}}}, - {Number{Number::maxRep}, + {Number{Number::largestMantissa}, 4, Number{false, 55'108'98747006743627, -14, Number::normalized{}}}, }); @@ -1051,7 +1169,7 @@ class Number_test : public beast::unit_test::suite Number{5, -1}, Number{0}, Number{5625, -4}, - Number{Number::maxRep}, + Number{Number::largestMantissa}, }); test(cSmall); bool caught = false; @@ -1417,20 +1535,20 @@ class Number_test : public beast::unit_test::suite case MantissaRange::large: // Test the edges // ((exponent < -(28)) || (exponent > -(8))))) - test(Number::min(), "1e-32750"); + test(Number::min(), "922337203685477581e-32768"); test(Number::max(), "9223372036854775807e32768"); test(Number::lowest(), "-9223372036854775807e32768"); { NumberRoundModeGuard mg(Number::towards_zero); auto const maxMantissa = Number::maxMantissa(); - BEAST_EXPECT(maxMantissa == 9'999'999'999'999'999'999ULL); + BEAST_EXPECT(maxMantissa == 9'223'372'036'854'775'807ULL); test( Number{false, maxMantissa, 0, Number::normalized{}}, - "9999999999999999990"); + "9223372036854775807"); test( Number{true, maxMantissa, 0, Number::normalized{}}, - "-9999999999999999990"); + "-9223372036854775807"); test( Number{std::numeric_limits::max(), 0}, @@ -1671,7 +1789,7 @@ class Number_test : public beast::unit_test::suite Number const initalXrp{INITIAL_XRP}; BEAST_EXPECT(initalXrp.exponent() > 0); - Number const maxInt64{Number::maxRep}; + Number const maxInt64{Number::largestMantissa}; BEAST_EXPECT(maxInt64.exponent() > 0); // 85'070'591'730'234'615'865'843'651'857'942'052'864 - 38 digits BEAST_EXPECT( @@ -1691,7 +1809,7 @@ class Number_test : public beast::unit_test::suite Number const initalXrp{INITIAL_XRP}; BEAST_EXPECT(initalXrp.exponent() <= 0); - Number const maxInt64{Number::maxRep}; + Number const maxInt64{Number::largestMantissa}; BEAST_EXPECT(maxInt64.exponent() <= 0); // 85'070'591'730'234'615'847'396'907'784'232'501'249 - 38 digits BEAST_EXPECT( @@ -1699,16 +1817,47 @@ class Number_test : public beast::unit_test::suite NumberRoundModeGuard mg(Number::towards_zero); - auto const maxMantissa = Number::maxMantissa(); - Number const max = - Number{false, maxMantissa, 0, Number::normalized{}}; - BEAST_EXPECT(max.mantissa() == maxMantissa / 10); - BEAST_EXPECT(max.exponent() == 1); - // 99'999'999'999'999'999'800'000'000'000'000'000'100 - also 38 - // digits - BEAST_EXPECT(( - power(max, 2) == - Number{false, maxMantissa / 10 - 1, 20, Number::normalized{}})); + { + auto const maxInternalMantissa = + static_cast(static_cast( + power(10, Number::mantissaLog()))) * + 10 - + 1; + + // Rounds down to fit under 2^63 + Number const max = + Number{false, maxInternalMantissa, 0, Number::normalized{}}; + // No alterations by the accessors + BEAST_EXPECT(max.mantissa() == maxInternalMantissa / 10); + BEAST_EXPECT(max.exponent() == 1); + // 99'999'999'999'999'999'800'000'000'000'000'000'100 - also 38 + // digits + BEAST_EXPECT( + (power(max, 2) == + Number{ + false, + maxInternalMantissa / 10 - 1, + 20, + Number::normalized{}})); + } + + { + auto const maxMantissa = Number::maxMantissa(); + Number const max = + Number{false, maxMantissa, 0, Number::normalized{}}; + // No alterations by the accessors + BEAST_EXPECT(max.mantissa() == maxMantissa); + BEAST_EXPECT(max.exponent() == 0); + // 85'070'591'730'234'615'847'396'907'784'232'501'249 - also 38 + // digits + BEAST_EXPECT( + (power(max, 2) == + Number{ + false, + 85'070'591'730'234'615'84, + 19, + Number::normalized{}})); + } } } From e7f81c26467fccfdc4eccc76ccde76931cde687d Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 26 Jan 2026 18:48:12 -0500 Subject: [PATCH 06/11] Remove the _ suffixes from doNormalize function parameters --- include/xrpl/basics/Number.h | 4 ++-- src/libxrpl/basics/Number.cpp | 39 +++++++++++++++++++---------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 45856aec3b4..2ba0f9b5230 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -598,8 +598,8 @@ class Number friend void doNormalize( bool& negative, - T& mantissa_, - int& exponent_, + T& mantissa, + int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa); diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index aa88c682102..541920045a9 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -399,13 +399,12 @@ Number::one() } // Use the member names in this static function for now so the diff is cleaner -// TODO: Rename the function parameters to get rid of the "_" suffix template void doNormalize( bool& negative, - T& mantissa_, - int& exponent_, + T& mantissa, + int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa) { @@ -415,35 +414,35 @@ doNormalize( using Guard = Number::Guard; constexpr Number zero = Number{}; - if (mantissa_ == 0 || (mantissa_ < minMantissa && exponent_ <= minExponent)) + if (mantissa == 0 || (mantissa < minMantissa && exponent <= minExponent)) { - mantissa_ = zero.mantissa_; - exponent_ = zero.exponent_; + mantissa = zero.mantissa_; + exponent = zero.exponent_; negative = zero.negative_; return; } - auto m = mantissa_; - while ((m < minMantissa) && (exponent_ > minExponent)) + auto m = mantissa; + while ((m < minMantissa) && (exponent > minExponent)) { m *= 10; - --exponent_; + --exponent; } Guard g; if (negative) g.set_negative(); while (m > maxMantissa) { - if (exponent_ >= maxExponent) + if (exponent >= maxExponent) throw std::overflow_error("Number::normalize 1"); g.push(m % 10); m /= 10; - ++exponent_; + ++exponent; } - if ((exponent_ < minExponent) || (m == 0)) + if ((exponent < minExponent) || (m == 0)) { - mantissa_ = zero.mantissa_; - exponent_ = zero.exponent_; + mantissa = zero.mantissa_; + exponent = zero.exponent_; negative = zero.negative_; return; } @@ -452,20 +451,24 @@ doNormalize( m <= maxMantissa, "xrpl::doNormalize", "intermediate mantissa fits in int64"); - mantissa_ = m; + mantissa = m; g.doRoundUp( negative, - mantissa_, - exponent_, + mantissa, + exponent, minMantissa, maxMantissa, "Number::normalize 2"); XRPL_ASSERT_PARTS( - mantissa_ >= minMantissa && mantissa_ <= maxMantissa, + mantissa >= minMantissa && mantissa <= maxMantissa, "xrpl::doNormalize", "final mantissa fits in range"); + XRPL_ASSERT_PARTS( + exponent >= minExponent && exponent <= maxExponent, + "xrpl::doNormalize", + "final exponent fits in range"); } template <> From a48182c65d3a631b33e421d6c614c70fc800e15c Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 26 Jan 2026 19:50:38 -0500 Subject: [PATCH 07/11] Convert "bool negative_ & uint64_t mantissa_" combo back to "rep mantissa_" --- include/xrpl/basics/Number.h | 54 +++++++--------- src/libxrpl/basics/Number.cpp | 108 +++++++++++++++++++++----------- src/test/basics/Number_test.cpp | 1 - 3 files changed, 91 insertions(+), 72 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 2ba0f9b5230..983c816571c 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -138,12 +138,7 @@ struct MantissaRange static constexpr rep computeMin(rep max) { - auto min = max + 1; - auto const r = min % 10; - min /= 10; - if (r != 0) - ++min; - return min; + return max / 10 + 1; } static constexpr rep @@ -278,9 +273,7 @@ class Number using rep = std::int64_t; using internalrep = MantissaRange::rep; - // TODO: Get rid of negative_ and convert mantissa back to rep - bool negative_{false}; - internalrep mantissa_{0}; + rep mantissa_{0}; int exponent_{std::numeric_limits::lowest()}; public: @@ -398,8 +391,7 @@ class Number friend constexpr bool operator==(Number const& x, Number const& y) noexcept { - return x.negative_ == y.negative_ && x.mantissa_ == y.mantissa_ && - x.exponent_ == y.exponent_; + return x.mantissa_ == y.mantissa_ && x.exponent_ == y.exponent_; } friend constexpr bool @@ -413,8 +405,8 @@ class Number { // If the two amounts have different signs (zero is treated as positive) // then the comparison is true iff the left is negative. - bool const lneg = x.negative_; - bool const rneg = y.negative_; + bool const lneg = x.mantissa_ < 0; + bool const rneg = y.mantissa_ < 0; if (lneg != rneg) return lneg; @@ -442,7 +434,7 @@ class Number constexpr int signum() const noexcept { - return negative_ ? -1 : (mantissa_ ? 1 : 0); + return mantissa_ < 0 ? -1 : (mantissa_ ? 1 : 0); } Number @@ -625,6 +617,11 @@ class Number std::tuple toInternal() const; + // Set the Number from an internal representation + template + void + fromInternal(bool negative, Rep mantissa, int exponent); + class Guard; public: @@ -636,7 +633,8 @@ inline constexpr Number::Number( internalrep mantissa, int exponent, unchecked) noexcept - : negative_(negative), mantissa_{mantissa}, exponent_{exponent} + : mantissa_{(negative ? -1 : 1) * static_cast(mantissa)} + , exponent_{exponent} { } @@ -650,16 +648,6 @@ inline constexpr Number::Number( constexpr static Number numZero{}; -inline Number::Number( - bool negative, - internalrep mantissa, - int exponent, - normalized) - : Number(negative, mantissa, exponent, unchecked{}) -{ - normalize(); -} - inline Number::Number(internalrep mantissa, int exponent, normalized) : Number(false, mantissa, exponent, normalized{}) { @@ -682,8 +670,7 @@ inline Number::Number(rep mantissa) : Number{mantissa, 0} inline constexpr Number::rep Number::mantissa() const noexcept { - auto const sign = negative_ ? -1 : 1; - return sign * static_cast(mantissa_); + return mantissa_; } /** Returns the exponent of the external view of the Number. @@ -717,7 +704,7 @@ Number::operator-() const noexcept if (mantissa_ == 0) return Number{}; auto x = *this; - x.negative_ = !x.negative_; + x.mantissa_ = -1 * x.mantissa_; return x; } @@ -811,7 +798,8 @@ inline bool Number::isnormal() const noexcept { MantissaRange const& range = range_; - auto const abs_m = mantissa_; + auto const abs_m = mantissa_ < 0 ? -mantissa_ : mantissa_; + return *this == Number{} || (range.min <= abs_m && abs_m <= range.max && // minExponent <= exponent_ && exponent_ <= maxExponent); @@ -821,8 +809,9 @@ template std::pair Number::normalizeToRange(T minMantissa, T maxMantissa) const { - bool negative = negative_; - internalrep mantissa = mantissa_; + bool negative = mantissa_ < 0; + auto const sign = negative ? -1 : 1; + internalrep mantissa = sign * mantissa_; int exponent = exponent_; if constexpr (std::is_unsigned_v) @@ -832,8 +821,7 @@ Number::normalizeToRange(T minMantissa, T maxMantissa) const "Number is non-negative for unsigned range."); Number::normalize(negative, mantissa, exponent, minMantissa, maxMantissa); - auto const sign = negative ? -1 : 1; - return std::make_pair(static_cast(sign * mantissa), exponent); + return std::make_pair(sign * static_cast(mantissa), exponent); } inline constexpr Number diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 541920045a9..f35263852c5 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -238,7 +238,7 @@ Number::Guard::bringIntoRange( { constexpr Number zero = Number{}; - negative = zero.negative_; + negative = false; mantissa = zero.mantissa_; exponent = zero.exponent_; } @@ -346,25 +346,54 @@ std::tuple Number::toInternal() const { auto exponent = exponent_; - Rep mantissa = mantissa_; - bool const negative = negative_; + bool const negative = mantissa_ < 0; + auto const sign = negative ? -1 : 1; + Rep mantissa = static_cast(sign * mantissa_); - auto const referenceMin = Number::range_.get().referenceMin; + auto const& range = Number::range_.get(); + auto const referenceMin = range.referenceMin; + auto const minMantissa = range.min; - if (mantissa != 0 && mantissa < referenceMin) + if (mantissa != 0 && mantissa >= minMantissa && mantissa < referenceMin) { // Ensure the mantissa has the correct number of digits mantissa *= 10; --exponent; + XRPL_ASSERT_PARTS( + mantissa >= referenceMin && mantissa < referenceMin * 10, + "ripple::Number::toInternal()", + "Number is within reference range and has 'log' digits"); } - XRPL_ASSERT_PARTS( - mantissa >= referenceMin && mantissa < referenceMin * 10, - "ripple::Number::toInternal()", - "Number is within reference range and has 'log' digits"); return {negative, mantissa, exponent}; } +template +void +Number::fromInternal(bool negative, Rep mantissa, int exponent) +{ + auto const sign = negative ? -1 : 1; + + auto const& range = Number::range_.get(); + auto const maxMantissa = range.max; + + XRPL_ASSERT_PARTS( + mantissa <= maxMantissa, + "ripple::Number::fromInternal", + "mantissa in range"); + if constexpr (std::is_signed_v) + XRPL_ASSERT_PARTS( + mantissa >= -maxMantissa, + "xrpl::Number::fromInternal", + "negative mantissa in range"); + + mantissa_ = sign * static_cast(mantissa); + exponent_ = exponent; + + XRPL_ASSERT_PARTS( + isnormal(), "ripple::Number::fromInternal", "Number is normalized"); +} + constexpr Number Number::oneSmall() { @@ -418,7 +447,7 @@ doNormalize( { mantissa = zero.mantissa_; exponent = zero.exponent_; - negative = zero.negative_; + negative = false; return; } @@ -443,7 +472,7 @@ doNormalize( { mantissa = zero.mantissa_; exponent = zero.exponent_; - negative = zero.negative_; + negative = false; return; } @@ -511,7 +540,12 @@ void Number::normalize() { auto const& range = range_.get(); - normalize(negative_, mantissa_, exponent_, range.min, range.max); + + auto [negative, mantissa, exponent] = toInternal(); + + normalize(negative, mantissa, exponent, range.min, range.max); + + fromInternal(negative, mantissa, exponent); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -521,6 +555,9 @@ Number Number::shiftExponent(int exponentDelta) const { XRPL_ASSERT_PARTS(isnormal(), "xrpl::Number::shiftExponent", "normalized"); + + auto const [negative, mantissa, exponent] = toInternal(); + auto const newExponent = exponent_ + exponentDelta; if (newExponent >= maxExponent) throw std::overflow_error("Number::shiftExponent"); @@ -528,7 +565,9 @@ Number::shiftExponent(int exponentDelta) const { return Number{}; } - Number const result{negative_, mantissa_, newExponent, unchecked{}}; + + Number const result{negative, mantissa, newExponent, unchecked{}}; + XRPL_ASSERT_PARTS( result.isnormal(), "xrpl::Number::shiftExponent", @@ -536,6 +575,13 @@ Number::shiftExponent(int exponentDelta) const return result; } +Number::Number(bool negative, internalrep mantissa, int exponent, normalized) +{ + auto const& range = range_.get(); + normalize(negative, mantissa, exponent, range.min, range.max); + fromInternal(negative, mantissa, exponent); +} + Number& Number::operator+=(Number const& y) { @@ -628,10 +674,8 @@ Number::operator+=(Number const& y) g.doRoundDown(xn, xm, xe, minMantissa); } - negative_ = xn; - mantissa_ = static_cast(xm); - exponent_ = xe; - normalize(); + normalize(xn, xm, xe, minMantissa, maxMantissa); + fromInternal(xn, xm, xe); return *this; } @@ -679,15 +723,11 @@ Number::operator*=(Number const& y) // *m = mantissa // *e = exponent - bool xn = negative_; + auto [xn, xm, xe] = toInternal(); int xs = xn ? -1 : 1; - internalrep xm = mantissa_; - auto xe = exponent_; - bool yn = y.negative_; + auto [yn, ym, ye] = y.toInternal(); int ys = yn ? -1 : 1; - internalrep ym = y.mantissa_; - auto ye = y.exponent_; auto zm = uint128_t(xm) * uint128_t(ym); auto ze = xe + ye; @@ -718,11 +758,9 @@ Number::operator*=(Number const& y) minMantissa, maxMantissa, "Number::multiplication overflow : exponent is " + std::to_string(xe)); - negative_ = zn; - mantissa_ = xm; - exponent_ = xe; - normalize(); + normalize(zn, xm, xe, minMantissa, maxMantissa); + fromInternal(zn, xm, xe); return *this; } @@ -741,15 +779,11 @@ Number::operator/=(Number const& y) // *m = mantissa // *e = exponent - bool np = negative_; + auto [np, nm, ne] = toInternal(); int ns = (np ? -1 : 1); - auto nm = mantissa_; - auto ne = exponent_; - bool dp = y.negative_; + auto [dp, dm, de] = y.toInternal(); int ds = (dp ? -1 : 1); - auto dm = y.mantissa_; - auto de = y.exponent_; auto const& range = range_.get(); auto const& minMantissa = range.min; @@ -815,9 +849,7 @@ Number::operator/=(Number const& y) } } normalize(zn, zm, ze, minMantissa, maxMantissa); - negative_ = zn; - mantissa_ = static_cast(zm); - exponent_ = ze; + fromInternal(zn, zm, ze); XRPL_ASSERT_PARTS( isnormal(), "xrpl::Number::operator/=", "result is normalized"); @@ -831,7 +863,7 @@ Number::operator rep() const Guard g; if (drops != 0) { - if (negative_) + if (drops < 0) { g.set_negative(); drops = -drops; @@ -843,7 +875,7 @@ Number::operator rep() const } for (; offset > 0; --offset) { - if (drops > largeRange.max / 10) + if (drops >= largeRange.min) throw std::overflow_error("Number::operator rep() overflow"); drops *= 10; } diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index db169fc1f9b..bf269178007 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -1082,7 +1082,6 @@ class Number_test : public beast::unit_test::suite }; */ - auto const maxMantissa = Number::maxMantissa(); auto const maxInternalMantissa = static_cast( static_cast(power(10, Number::mantissaLog()))) * From 7e2c2573d25e1fdbbd4377c50751012adb8f05f2 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 27 Jan 2026 12:56:16 -0500 Subject: [PATCH 08/11] Fix bugs - Simplify shiftExponent(). - Clean up to_string() to prevent integers from including "e0". - Fix root() and root2() computations by ensuring the mantissas have a consistent length. --- include/xrpl/basics/Number.h | 50 +++++++++-- src/libxrpl/basics/Number.cpp | 153 +++++++++++++++++++------------- src/test/basics/Number_test.cpp | 6 ++ 3 files changed, 144 insertions(+), 65 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 983c816571c..0e140ef816b 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -10,6 +10,10 @@ #include #include +#ifdef _MSC_VER +#include +#endif // !defined(_MSC_VER) + namespace xrpl { class Number; @@ -109,6 +113,14 @@ struct MantissaRange throw std::out_of_range("(max + 10) / 10 < min"); } + // Explicitly delete copy and move operations + MantissaRange(MantissaRange const&) = delete; + MantissaRange(MantissaRange&&) = delete; + MantissaRange& + operator=(MantissaRange const&) = delete; + MantissaRange& + operator=(MantissaRange&&) = delete; + rep max; rep min; // This is not a great name. Used to determine if mantissas are in range, @@ -172,6 +184,20 @@ template concept CanUseAsScale = requires(Asset a, Number n) { STAmount(a, n); } && requires(STAmount s) { s.exponent(); }; +namespace detail { +#ifdef _MSC_VER +using uint128_t = boost::multiprecision::uint128_t; +using int128_t = boost::multiprecision::int128_t; +#else // !defined(_MSC_VER) +using uint128_t = __uint128_t; +using int128_t = __int128_t; +#endif // !defined(_MSC_VER) + +template +concept UnsignedMantissa = + std::is_unsigned_v || std::is_same_v; +} // namespace detail + /** Number is a floating point type that can represent a wide range of values. * * It can represent all values that can be represented by an STAmount - @@ -611,14 +637,28 @@ class Number static internalrep externalToInternal(rep mantissa); - // Safely convert Number to the internal rep where the mantissa always has - // the same number of digits - template + /** Breaks down the number into components, potentially de-normalizing it. + * + * Ensures that the mantissa always has range_.log digits. + * + */ + template std::tuple toInternal() const; - // Set the Number from an internal representation - template + /** Rebuilds the number from components. + * + * If "normalized" is true, the values are expected to be normalized - all + * in their valid ranges. + * + * If "normalized" is false, the values are expected to be "near + * normalized", meaning that the mantissa has to be modified at most once to + * bring it back into range. + * + */ + template < + bool expectNormal = true, + detail::UnsignedMantissa Rep = internalrep> void fromInternal(bool negative, Rep mantissa, int exponent); diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index f35263852c5..755dbe31fdc 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -16,13 +16,10 @@ #ifdef _MSC_VER #pragma message("Using boost::multiprecision::uint128_t and int128_t") -#include -using uint128_t = boost::multiprecision::uint128_t; -using int128_t = boost::multiprecision::int128_t; -#else // !defined(_MSC_VER) -using uint128_t = __uint128_t; -using int128_t = __int128_t; -#endif // !defined(_MSC_VER) +#endif + +using uint128_t = ripple::detail::uint128_t; +using int128_t = ripple::detail::int128_t; namespace xrpl { @@ -62,10 +59,6 @@ Number::setMantissaScale(MantissaRange::mantissa_scale scale) // precision to an operation. This enables the final result // to be correctly rounded to the internal precision of Number. -template -concept UnsignedMantissa = - std::is_unsigned_v || std::is_same_v; - class Number::Guard { std::uint64_t digits_; // 16 decimal guard digits @@ -101,7 +94,7 @@ class Number::Guard round() noexcept; // Modify the result to the correctly rounded value - template + template void doRoundUp( bool& negative, @@ -112,7 +105,7 @@ class Number::Guard std::string location); // Modify the result to the correctly rounded value - template + template void doRoundDown( bool& negative, @@ -128,7 +121,7 @@ class Number::Guard void doPush(unsigned d) noexcept; - template + template void bringIntoRange( bool& negative, @@ -219,7 +212,7 @@ Number::Guard::round() noexcept return 0; } -template +template void Number::Guard::bringIntoRange( bool& negative, @@ -244,7 +237,7 @@ Number::Guard::bringIntoRange( } } -template +template void Number::Guard::doRoundUp( bool& negative, @@ -271,7 +264,7 @@ Number::Guard::doRoundUp( throw std::overflow_error(location); } -template +template void Number::Guard::doRoundDown( bool& negative, @@ -341,7 +334,12 @@ Number::externalToInternal(rep mantissa) return static_cast(-temp); } -template +/** Breaks down the number into components, potentially de-normalizing it. + * + * Ensures that the mantissa always has range_.log digits. + * + */ +template std::tuple Number::toInternal() const { @@ -368,24 +366,46 @@ Number::toInternal() const return {negative, mantissa, exponent}; } -template +/** Rebuilds the number from components. + * + * If "normalized" is true, the values are expected to be normalized - all in + * their valid ranges. + * + * If "normalized" is false, the values are expected to be "near normalized", + * meaning that the mantissa has to be modified at most once to bring it back + * into range. + * + */ +template void Number::fromInternal(bool negative, Rep mantissa, int exponent) { - auto const sign = negative ? -1 : 1; + if constexpr (std::is_same_v< + std::bool_constant, + std::false_type>) + { + auto const& range = Number::range_.get(); - auto const& range = Number::range_.get(); - auto const maxMantissa = range.max; + auto const maxMantissa = range.max; + auto const minMantissa = range.min; - XRPL_ASSERT_PARTS( - mantissa <= maxMantissa, - "ripple::Number::fromInternal", - "mantissa in range"); - if constexpr (std::is_signed_v) XRPL_ASSERT_PARTS( - mantissa >= -maxMantissa, - "xrpl::Number::fromInternal", - "negative mantissa in range"); + mantissa >= minMantissa, + "ripple::Number::fromInternal", + "mantissa large enough"); + + if (mantissa > maxMantissa || mantissa < minMantissa) + { + normalize(negative, mantissa, exponent, range.min, maxMantissa); + } + + XRPL_ASSERT_PARTS( + mantissa >= minMantissa && mantissa <= maxMantissa, + "ripple::Number::fromInternal", + "mantissa in range"); + } + + auto const sign = negative ? -1 : 1; mantissa_ = sign * static_cast(mantissa); exponent_ = exponent; @@ -556,22 +576,17 @@ Number::shiftExponent(int exponentDelta) const { XRPL_ASSERT_PARTS(isnormal(), "xrpl::Number::shiftExponent", "normalized"); - auto const [negative, mantissa, exponent] = toInternal(); + Number result = *this; + + result.exponent_ += exponentDelta; - auto const newExponent = exponent_ + exponentDelta; - if (newExponent >= maxExponent) + if (result.exponent_ >= maxExponent) throw std::overflow_error("Number::shiftExponent"); - if (newExponent < minExponent) + if (result.exponent_ < minExponent) { return Number{}; } - Number const result{negative, mantissa, newExponent, unchecked{}}; - - XRPL_ASSERT_PARTS( - result.isnormal(), - "xrpl::Number::shiftExponent", - "result is normalized"); return result; } @@ -915,9 +930,10 @@ to_string(Number const& amount) // Use scientific notation for exponents that are too small or too large auto const rangeLog = Number::mantissaLog(); - if (((exponent != 0) && + if (((exponent != 0 && amount.exponent() != 0) && ((exponent < -(rangeLog + 10)) || (exponent > -(rangeLog - 10))))) { + // Remove trailing zeroes from the mantissa. while (mantissa != 0 && mantissa % 10 == 0 && exponent < Number::maxExponent) { @@ -926,8 +942,11 @@ to_string(Number const& amount) } std::string ret = negative ? "-" : ""; ret.append(std::to_string(mantissa)); - ret.append(1, 'e'); - ret.append(std::to_string(exponent)); + if (exponent != 0) + { + ret.append(1, 'e'); + ret.append(std::to_string(exponent)); + } return ret; } @@ -1045,20 +1064,28 @@ root(Number f, unsigned d) if (f == zero) return f; - // Scale f into the range (0, 1) such that f's exponent is a multiple of d - auto e = f.exponent_ + Number::mantissaLog() + 1; - auto const di = static_cast(d); - auto ex = [e = e, di = di]() // Euclidean remainder of e/d - { - int k = (e >= 0 ? e : e - (di - 1)) / di; - int k2 = e - k * di; - if (k2 == 0) - return 0; - return di - k2; + auto const [e, di] = [&]() { + auto const [negative, mantissa, exponent] = f.toInternal(); + + // Scale f into the range (0, 1) such that the scale change (e) is a + // multiple of the root (d) + auto e = exponent + Number::mantissaLog() + 1; + auto const di = static_cast(d); + auto ex = [e = e, di = di]() // Euclidean remainder of e/d + { + int k = (e >= 0 ? e : e - (di - 1)) / di; + int k2 = e - k * di; + if (k2 == 0) + return 0; + return di - k2; + }(); + e += ex; + f = f.shiftExponent(-e); // f /= 10^e; + return std::make_tuple(e, di); }(); - e += ex; - f = f.shiftExponent(-e); // f /= 10^e; + XRPL_ASSERT_PARTS( + e % di == 0, "xrpl::root(Number, unsigned)", "e is divisible by d"); XRPL_ASSERT_PARTS( f.isnormal(), "xrpl::root(Number, unsigned)", "f is normalized"); bool neg = false; @@ -1113,11 +1140,17 @@ root2(Number f) if (f == zero) return f; - // Scale f into the range (0, 1) such that f's exponent is a multiple of d - auto e = f.exponent_ + Number::mantissaLog() + 1; - if (e % 2 != 0) - ++e; - f = f.shiftExponent(-e); // f /= 10^e; + auto const e = [&]() { + auto const [negative, mantissa, exponent] = f.toInternal(); + + // Scale f into the range (0, 1) such that f's exponent is a + // multiple of d + auto e = exponent + Number::mantissaLog() + 1; + if (e % 2 != 0) + ++e; + f = f.shiftExponent(-e); // f /= 10^e; + return e; + }(); XRPL_ASSERT_PARTS(f.isnormal(), "xrpl::root2(Number)", "f is normalized"); // Quadratic least squares curve fit of f^(1/d) in the range [0, 1] diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index bf269178007..078c2fa57c8 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -1159,6 +1159,9 @@ class Number_test : public beast::unit_test::suite } }; + auto const maxInternalMantissa = + power(10, Number::mantissaLog()) * 10 - 1; + auto const cSmall = std::to_array({ Number{2}, Number{2'000'000}, @@ -1169,6 +1172,9 @@ class Number_test : public beast::unit_test::suite Number{0}, Number{5625, -4}, Number{Number::largestMantissa}, + maxInternalMantissa, + Number{Number::minMantissa(), 0, Number::unchecked{}}, + Number{Number::maxMantissa(), 0, Number::unchecked{}}, }); test(cSmall); bool caught = false; From f0eef6ec63b10a0617825821cf205ba4141d6839 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 27 Jan 2026 16:31:14 -0500 Subject: [PATCH 09/11] Reduce expensive(?) accesses to thread_local MantissaRange --- include/xrpl/basics/Number.h | 55 +++++++++- src/libxrpl/basics/Number.cpp | 182 +++++++++++++++++++++++----------- 2 files changed, 179 insertions(+), 58 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 0e140ef816b..a0a6e671d6b 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -499,6 +499,9 @@ class Number friend Number root2(Number f); + friend Number + power(Number const& f, unsigned n, unsigned d); + // Thread local rounding control. Default is to_nearest enum rounding_mode { to_nearest, towards_zero, downward, upward }; static rounding_mode @@ -594,6 +597,17 @@ class Number // changing the values inside the range. static thread_local std::reference_wrapper range_; + // And one is needed because it needs to choose between oneSmall and + // oneLarge based on the current range + static Number + one(MantissaRange const& range); + + static Number + root(MantissaRange const& range, Number f, unsigned d); + + void + normalize(MantissaRange const& range); + void normalize(); @@ -621,6 +635,9 @@ class Number MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa); + bool + isnormal(MantissaRange const& range) const noexcept; + bool isnormal() const noexcept; @@ -637,6 +654,15 @@ class Number static internalrep externalToInternal(rep mantissa); + /** Breaks down the number into components, potentially de-normalizing it. + * + * Ensures that the mantissa always has range_.log digits. + * + */ + template + std::tuple + toInternal(MantissaRange const& range) const; + /** Breaks down the number into components, potentially de-normalizing it. * * Ensures that the mantissa always has range_.log digits. @@ -646,6 +672,26 @@ class Number std::tuple toInternal() const; + /** Rebuilds the number from components. + * + * If "normalized" is true, the values are expected to be normalized - all + * in their valid ranges. + * + * If "normalized" is false, the values are expected to be "near + * normalized", meaning that the mantissa has to be modified at most once to + * bring it back into range. + * + */ + template < + bool expectNormal = true, + detail::UnsignedMantissa Rep = internalrep> + void + fromInternal( + bool negative, + Rep mantissa, + int exponent, + MantissaRange const* pRange); + /** Rebuilds the number from components. * * If "normalized" is true, the values are expected to be normalized - all @@ -835,9 +881,8 @@ Number::lowest() noexcept } inline bool -Number::isnormal() const noexcept +Number::isnormal(MantissaRange const& range) const noexcept { - MantissaRange const& range = range_; auto const abs_m = mantissa_ < 0 ? -mantissa_ : mantissa_; return *this == Number{} || @@ -845,6 +890,12 @@ Number::isnormal() const noexcept minExponent <= exponent_ && exponent_ <= maxExponent); } +inline bool +Number::isnormal() const noexcept +{ + return isnormal(range_); +} + template std::pair Number::normalizeToRange(T minMantissa, T maxMantissa) const diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 755dbe31fdc..84b97eecac3 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -341,14 +341,13 @@ Number::externalToInternal(rep mantissa) */ template std::tuple -Number::toInternal() const +Number::toInternal(MantissaRange const& range) const { auto exponent = exponent_; bool const negative = mantissa_ < 0; auto const sign = negative ? -1 : 1; Rep mantissa = static_cast(sign * mantissa_); - auto const& range = Number::range_.get(); auto const referenceMin = range.referenceMin; auto const minMantissa = range.min; @@ -366,25 +365,43 @@ Number::toInternal() const return {negative, mantissa, exponent}; } +/** Breaks down the number into components, potentially de-normalizing it. + * + * Ensures that the mantissa always has range_.log digits. + * + */ +template +std::tuple +Number::toInternal() const +{ + return toInternal(range_); +} + /** Rebuilds the number from components. * - * If "normalized" is true, the values are expected to be normalized - all in - * their valid ranges. + * If "normalized" is true, the values are expected to be normalized - all + * in their valid ranges. * - * If "normalized" is false, the values are expected to be "near normalized", - * meaning that the mantissa has to be modified at most once to bring it back - * into range. + * If "normalized" is false, the values are expected to be "near + * normalized", meaning that the mantissa has to be modified at most once to + * bring it back into range. * */ template void -Number::fromInternal(bool negative, Rep mantissa, int exponent) +Number::fromInternal( + bool negative, + Rep mantissa, + int exponent, + MantissaRange const* pRange) { if constexpr (std::is_same_v< std::bool_constant, std::false_type>) { - auto const& range = Number::range_.get(); + if (!pRange) + throw std::runtime_error("Missing range to Number::fromInternal!"); + auto const& range = *pRange; auto const maxMantissa = range.max; auto const minMantissa = range.min; @@ -411,7 +428,34 @@ Number::fromInternal(bool negative, Rep mantissa, int exponent) exponent_ = exponent; XRPL_ASSERT_PARTS( - isnormal(), "ripple::Number::fromInternal", "Number is normalized"); + (pRange && isnormal(*pRange)) || isnormal(), + "ripple::Number::fromInternal", + "Number is normalized"); +} + +/** Rebuilds the number from components. + * + * If "normalized" is true, the values are expected to be normalized - all in + * their valid ranges. + * + * If "normalized" is false, the values are expected to be "near normalized", + * meaning that the mantissa has to be modified at most once to bring it back + * into range. + * + */ +template +void +Number::fromInternal(bool negative, Rep mantissa, int exponent) +{ + MantissaRange const* pRange = nullptr; + if constexpr (std::is_same_v< + std::bool_constant, + std::false_type>) + { + pRange = &Number::range_.get(); + } + + fromInternal(negative, mantissa, exponent, pRange); } constexpr Number @@ -439,14 +483,20 @@ Number::oneLarge() constexpr Number oneLrg = Number::oneLarge(); Number -Number::one() +Number::one(MantissaRange const& range) { - if (&range_.get() == &smallRange) + if (&range == &smallRange) return oneSml; - XRPL_ASSERT(&range_.get() == &largeRange, "Number::one() : valid range_"); + XRPL_ASSERT(&range == &largeRange, "Number::one() : valid range"); return oneLrg; } +Number +Number::one() +{ + return one(range_); +} + // Use the member names in this static function for now so the diff is cleaner template void @@ -557,15 +607,19 @@ Number::normalize( } void -Number::normalize() +Number::normalize(MantissaRange const& range) { - auto const& range = range_.get(); - - auto [negative, mantissa, exponent] = toInternal(); + auto [negative, mantissa, exponent] = toInternal(range); normalize(negative, mantissa, exponent, range.min, range.max); - fromInternal(negative, mantissa, exponent); + fromInternal(negative, mantissa, exponent, &range); +} + +void +Number::normalize() +{ + normalize(range_); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -594,12 +648,14 @@ Number::Number(bool negative, internalrep mantissa, int exponent, normalized) { auto const& range = range_.get(); normalize(negative, mantissa, exponent, range.min, range.max); - fromInternal(negative, mantissa, exponent); + fromInternal(negative, mantissa, exponent, &range); } Number& Number::operator+=(Number const& y) { + auto const& range = range_.get(); + constexpr Number zero = Number{}; if (y == zero) return *this; @@ -615,7 +671,7 @@ Number::operator+=(Number const& y) } XRPL_ASSERT( - isnormal() && y.isnormal(), + isnormal(range) && y.isnormal(range), "xrpl::Number::operator+=(Number) : is normal"); // *n = negative // *s = sign @@ -624,9 +680,9 @@ Number::operator+=(Number const& y) // Need to use uint128_t, because large mantissas can overflow when added // together. - auto [xn, xm, xe] = toInternal(); + auto [xn, xm, xe] = toInternal(range); - auto [yn, ym, ye] = y.toInternal(); + auto [yn, ym, ye] = y.toInternal(range); Guard g; if (xe < ye) @@ -652,7 +708,6 @@ Number::operator+=(Number const& y) } while (xe > ye); } - auto const& range = range_.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; @@ -690,7 +745,7 @@ Number::operator+=(Number const& y) } normalize(xn, xm, xe, minMantissa, maxMantissa); - fromInternal(xn, xm, xe); + fromInternal(xn, xm, xe, &range); return *this; } @@ -725,6 +780,8 @@ divu10(uint128_t& u) Number& Number::operator*=(Number const& y) { + auto const& range = range_.get(); + constexpr Number zero = Number{}; if (*this == zero) return *this; @@ -738,10 +795,10 @@ Number::operator*=(Number const& y) // *m = mantissa // *e = exponent - auto [xn, xm, xe] = toInternal(); + auto [xn, xm, xe] = toInternal(range); int xs = xn ? -1 : 1; - auto [yn, ym, ye] = y.toInternal(); + auto [yn, ym, ye] = y.toInternal(range); int ys = yn ? -1 : 1; auto zm = uint128_t(xm) * uint128_t(ym); @@ -752,7 +809,6 @@ Number::operator*=(Number const& y) if (zn) g.set_negative(); - auto const& range = range_.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; @@ -775,13 +831,15 @@ Number::operator*=(Number const& y) "Number::multiplication overflow : exponent is " + std::to_string(xe)); normalize(zn, xm, xe, minMantissa, maxMantissa); - fromInternal(zn, xm, xe); + fromInternal(zn, xm, xe, &range); return *this; } Number& Number::operator/=(Number const& y) { + auto const& range = range_.get(); + constexpr Number zero = Number{}; if (y == zero) throw std::overflow_error("Number: divide by 0"); @@ -794,13 +852,12 @@ Number::operator/=(Number const& y) // *m = mantissa // *e = exponent - auto [np, nm, ne] = toInternal(); + auto [np, nm, ne] = toInternal(range); int ns = (np ? -1 : 1); - auto [dp, dm, de] = y.toInternal(); + auto [dp, dm, de] = y.toInternal(range); int ds = (dp ? -1 : 1); - auto const& range = range_.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; @@ -812,7 +869,7 @@ Number::operator/=(Number const& y) // f can be up to 10^(38-19) = 10^19 safely static_assert(smallRange.log == 15); static_assert(largeRange.log == 18); - bool small = Number::getMantissaScale() == MantissaRange::small; + bool small = range.scale == MantissaRange::small; uint128_t const f = small ? 100'000'000'000'000'000 : 10'000'000'000'000'000'000ULL; XRPL_ASSERT_PARTS( @@ -864,9 +921,9 @@ Number::operator/=(Number const& y) } } normalize(zn, zm, ze, minMantissa, maxMantissa); - fromInternal(zn, zm, ze); + fromInternal(zn, zm, ze, &range); XRPL_ASSERT_PARTS( - isnormal(), "xrpl::Number::operator/=", "result is normalized"); + isnormal(range), "xrpl::Number::operator/=", "result is normalized"); return *this; } @@ -920,16 +977,18 @@ Number::truncate() const noexcept std::string to_string(Number const& amount) { + auto const& range = Number::range_.get(); + // keep full internal accuracy, but make more human friendly if possible constexpr Number zero = Number{}; if (amount == zero) return "0"; // The mantissa must have a set number of decimal places for this to work - auto [negative, mantissa, exponent] = amount.toInternal(); + auto [negative, mantissa, exponent] = amount.toInternal(range); // Use scientific notation for exponents that are too small or too large - auto const rangeLog = Number::mantissaLog(); + auto const rangeLog = range.log; if (((exponent != 0 && amount.exponent() != 0) && ((exponent < -(rangeLog + 10)) || (exponent > -(rangeLog - 10))))) { @@ -1034,20 +1093,11 @@ power(Number const& f, unsigned n) return r; } -// Returns f^(1/d) -// Uses Newton–Raphson iterations until the result stops changing -// to find the non-negative root of the polynomial g(x) = x^d - f - -// This function, and power(Number f, unsigned n, unsigned d) -// treat corner cases such as 0 roots as advised by Annex F of -// the C standard, which itself is consistent with the IEEE -// floating point standards. - Number -root(Number f, unsigned d) +Number::root(MantissaRange const& range, Number f, unsigned d) { constexpr Number zero = Number{}; - auto const one = Number::one(); + auto const one = Number::one(range); if (f == one || d == 1) return f; @@ -1065,7 +1115,7 @@ root(Number f, unsigned d) return f; auto const [e, di] = [&]() { - auto const [negative, mantissa, exponent] = f.toInternal(); + auto const [negative, mantissa, exponent] = f.toInternal(range); // Scale f into the range (0, 1) such that the scale change (e) is a // multiple of the root (d) @@ -1087,7 +1137,7 @@ root(Number f, unsigned d) XRPL_ASSERT_PARTS( e % di == 0, "xrpl::root(Number, unsigned)", "e is divisible by d"); XRPL_ASSERT_PARTS( - f.isnormal(), "xrpl::root(Number, unsigned)", "f is normalized"); + f.isnormal(range), "xrpl::root(Number, unsigned)", "f is normalized"); bool neg = false; if (f < zero) { @@ -1121,17 +1171,34 @@ root(Number f, unsigned d) // return r * 10^(e/d) to reverse scaling auto const result = r.shiftExponent(e / di); XRPL_ASSERT_PARTS( - result.isnormal(), + result.isnormal(range), "xrpl::root(Number, unsigned)", "result is normalized"); return result; } +// Returns f^(1/d) +// Uses Newton–Raphson iterations until the result stops changing +// to find the non-negative root of the polynomial g(x) = x^d - f + +// This function, and power(Number f, unsigned n, unsigned d) +// treat corner cases such as 0 roots as advised by Annex F of +// the C standard, which itself is consistent with the IEEE +// floating point standards. + +Number +root(Number f, unsigned d) +{ + auto const& range = Number::range_.get(); + return Number::root(range, f, d); +} + Number root2(Number f) { + auto const& range = Number::range_.get(); constexpr Number zero = Number{}; - auto const one = Number::one(); + auto const one = Number::one(range); if (f == one) return f; @@ -1141,7 +1208,7 @@ root2(Number f) return f; auto const e = [&]() { - auto const [negative, mantissa, exponent] = f.toInternal(); + auto const [negative, mantissa, exponent] = f.toInternal(range); // Scale f into the range (0, 1) such that f's exponent is a // multiple of d @@ -1151,7 +1218,8 @@ root2(Number f) f = f.shiftExponent(-e); // f /= 10^e; return e; }(); - XRPL_ASSERT_PARTS(f.isnormal(), "xrpl::root2(Number)", "f is normalized"); + XRPL_ASSERT_PARTS( + f.isnormal(range), "xrpl::root2(Number)", "f is normalized"); // Quadratic least squares curve fit of f^(1/d) in the range [0, 1] auto const D = 105; @@ -1174,7 +1242,7 @@ root2(Number f) // return r * 10^(e/2) to reverse scaling auto const result = r.shiftExponent(e / 2); XRPL_ASSERT_PARTS( - result.isnormal(), "xrpl::root2(Number)", "result is normalized"); + result.isnormal(range), "xrpl::root2(Number)", "result is normalized"); return result; } @@ -1184,8 +1252,10 @@ root2(Number f) Number power(Number const& f, unsigned n, unsigned d) { + auto const& range = Number::range_.get(); + constexpr Number zero = Number{}; - auto const one = Number::one(); + auto const one = Number::one(range); if (f == one) return f; @@ -1207,7 +1277,7 @@ power(Number const& f, unsigned n, unsigned d) d /= g; if ((n % 2) == 1 && (d % 2) == 0 && f < zero) throw std::overflow_error("Number::power nan"); - return root(power(f, n), d); + return Number::root(range, power(f, n), d); } } // namespace xrpl From 0364d61b427b07aa5f02d7c3765a24b556d2a427 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 27 Jan 2026 19:07:08 -0500 Subject: [PATCH 10/11] Clean-ups and tweaks --- include/xrpl/basics/Number.h | 4 ++-- src/libxrpl/basics/Number.cpp | 7 ++++--- src/test/basics/Number_test.cpp | 20 +++++++++++++++++++- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index a0a6e671d6b..48f7da17500 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -656,7 +656,7 @@ class Number /** Breaks down the number into components, potentially de-normalizing it. * - * Ensures that the mantissa always has range_.log digits. + * Ensures that the mantissa always has range_.log + 1 digits. * */ template @@ -665,7 +665,7 @@ class Number /** Breaks down the number into components, potentially de-normalizing it. * - * Ensures that the mantissa always has range_.log digits. + * Ensures that the mantissa always has range_.log + 1 digits. * */ template diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 84b97eecac3..f570bc2bb6c 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -292,7 +292,8 @@ Number::Guard::doRound(rep& drops, std::string location) auto r = round(); if (r == 1 || (r == 0 && (drops & 1) == 1)) { - if (drops >= maxMantissa()) + auto const& range = range_.get(); + if (drops >= range.max) { static_assert(sizeof(internalrep) == sizeof(rep)); // This should be impossible, because it's impossible to represent @@ -1119,7 +1120,7 @@ Number::root(MantissaRange const& range, Number f, unsigned d) // Scale f into the range (0, 1) such that the scale change (e) is a // multiple of the root (d) - auto e = exponent + Number::mantissaLog() + 1; + auto e = exponent + range.log + 1; auto const di = static_cast(d); auto ex = [e = e, di = di]() // Euclidean remainder of e/d { @@ -1212,7 +1213,7 @@ root2(Number f) // Scale f into the range (0, 1) such that f's exponent is a // multiple of d - auto e = exponent + Number::mantissaLog() + 1; + auto e = exponent + range.log + 1; if (e % 2 != 0) ++e; f = f.shiftExponent(-e); // f /= 10^e; diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 078c2fa57c8..314e0f58e6b 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -228,6 +228,25 @@ class Number_test : public beast::unit_test::suite // result will overflow. With addition using uint128_t, // there's no problem. After normalizing, the resulting // mantissa ends up less than largestMantissa. + Number{ + false, + Number::largestMantissa, + 0, + Number::normalized{}}, + Number{ + false, + Number::largestMantissa, + 0, + Number::normalized{}}, + Number{ + false, + Number::largestMantissa * 2, + 0, + Number::normalized{}}, + }, + { + // These mantissas round down, so adding them together won't + // have any consequences. Number{ false, 9'999'999'999'999'999'990ULL, @@ -604,7 +623,6 @@ class Number_test : public beast::unit_test::suite Number::normalized{}}, __LINE__}, // Maximum actual mantissa range - same as int64 - // 99'999'999'999'999'999'800'000'000'000'000'000'100 {Number{false, maxMantissa, 0, Number::normalized{}}, Number{false, maxMantissa, 0, Number::normalized{}}, Number{85'070'591'730'234'615'84, 19}, From 7c499aacab28c4ef8e30363c555f46e68009612d Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 28 Jan 2026 17:28:27 -0500 Subject: [PATCH 11/11] Add unit tests for normalizeToRange - Steal changes from @pratik's #6150 to avoid UB --- include/xrpl/basics/Number.h | 14 +++ include/xrpl/protocol/STAmount.h | 4 + src/libxrpl/basics/Number.cpp | 28 ++--- src/test/basics/Number_test.cpp | 201 +++++++++++++++++++++++++++++++ 4 files changed, 230 insertions(+), 17 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 48f7da17500..8d91fbe05e8 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -906,12 +906,26 @@ Number::normalizeToRange(T minMantissa, T maxMantissa) const int exponent = exponent_; if constexpr (std::is_unsigned_v) + { XRPL_ASSERT_PARTS( !negative, "xrpl::Number::normalizeToRange", "Number is non-negative for unsigned range."); + // To avoid logical errors in release builds, throw if the Number is + // negative for an unsigned range. + if (negative) + throw std::runtime_error( + "Number::normalizeToRange: Number is negative for " + "unsigned range."); + } Number::normalize(negative, mantissa, exponent, minMantissa, maxMantissa); + // Cast mantissa to signed type first (if T is a signed type) to avoid + // unsigned integer overflow when multiplying by negative sign + T signedMantissa = static_cast(mantissa); + if (negative) + signedMantissa = -signedMantissa; + return std::make_pair(signedMantissa, exponent); return std::make_pair(sign * static_cast(mantissa), exponent); } diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index 4d86aed2eca..0c86bc5747a 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -568,6 +568,10 @@ STAmount::fromNumber(A const& a, Number const& number) return STAmount{asset, intValue, 0, negative}; } + XRPL_ASSERT_PARTS( + working.signum() >= 0, + "ripple::STAmount::fromNumber", + "non-negative Number to normalize"); auto const [mantissa, exponent] = working.normalizeToRange(cMinValue, cMaxValue); diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index f570bc2bb6c..5a1d84c8d3b 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -102,7 +103,7 @@ class Number::Guard int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - std::string location); + std::string_view location); // Modify the result to the correctly rounded value template @@ -115,7 +116,7 @@ class Number::Guard // Modify the result to the correctly rounded value void - doRound(rep& drops, std::string location); + doRound(rep& drops, std::string_view location); private: void @@ -245,7 +246,7 @@ Number::Guard::doRoundUp( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - std::string location) + std::string_view location) { auto r = round(); if (r == 1 || (r == 0 && (mantissa & 1) == 1)) @@ -261,7 +262,7 @@ Number::Guard::doRoundUp( } bringIntoRange(negative, mantissa, exponent, minMantissa); if (exponent > maxExponent) - throw std::overflow_error(location); + throw std::overflow_error(std::string{location}); } template @@ -287,7 +288,7 @@ Number::Guard::doRoundDown( // Modify the result to the correctly rounded value void -Number::Guard::doRound(rep& drops, std::string location) +Number::Guard::doRound(rep& drops, std::string_view location) { auto r = round(); if (r == 1 || (r == 0 && (drops & 1) == 1)) @@ -302,7 +303,7 @@ Number::Guard::doRound(rep& drops, std::string location) // or "(maxRep + 1) / 10", neither of which will round up when // converting to rep, though the latter might overflow _before_ // rounding. - throw std::overflow_error(location); // LCOV_EXCL_LINE + throw std::overflow_error(std::string{location}); // LCOV_EXCL_LINE } ++drops; } @@ -322,17 +323,10 @@ Number::externalToInternal(rep mantissa) // If the mantissa is already positive, just return it if (mantissa >= 0) return mantissa; - // If the mantissa is negative, but fits within the positive range of rep, - // return it negated - if (mantissa >= -std::numeric_limits::max()) - return -mantissa; - - // If the mantissa doesn't fit within the positive range, convert to - // int128_t, negate that, and cast it back down to the internalrep - // In practice, this is only going to cover the case of - // std::numeric_limits::min(). - int128_t temp = mantissa; - return static_cast(-temp); + + // Cast to unsigned before negating to avoid undefined behavior + // when v == INT64_MIN (negating INT64_MIN in signed is UB) + return -static_cast(mantissa); } /** Breaks down the number into components, potentially de-normalizing it. diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 314e0f58e6b..4da06214394 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -1884,6 +1884,206 @@ class Number_test : public beast::unit_test::suite } } + void + testNormalizeToRange() + { + // Test edge-cases of normalizeToRange + auto const scale = Number::getMantissaScale(); + testcase << "normalizeToRange " << to_string(scale); + + auto test = [this]( + Number const& n, + auto const rangeMin, + auto const rangeMax, + auto const expectedMantissa, + auto const expectedExponent, + auto const line) { + auto const normalized = n.normalizeToRange(rangeMin, rangeMax); + BEAST_EXPECTS( + normalized.first == expectedMantissa, + "Number " + to_string(n) + " scaled to " + + std::to_string(rangeMax) + + ". Expected mantissa:" + std::to_string(expectedMantissa) + + ", got: " + std::to_string(normalized.first) + " @ " + + std::to_string(line)); + BEAST_EXPECTS( + normalized.second == expectedExponent, + "Number " + to_string(n) + " scaled to " + + std::to_string(rangeMax) + + ". Expected exponent:" + std::to_string(expectedExponent) + + ", got: " + std::to_string(normalized.second) + " @ " + + std::to_string(line)); + }; + + std::int64_t constexpr iRangeMin = 100; + std::int64_t constexpr iRangeMax = 999; + + std::uint64_t constexpr uRangeMin = 100; + std::uint64_t constexpr uRangeMax = 999; + + constexpr static MantissaRange largeRange{MantissaRange::large}; + + std::int64_t constexpr iBigMin = largeRange.min; + std::int64_t constexpr iBigMax = largeRange.max; + + auto const testSuite = [&](Number const& n, + auto const expectedSmallMantissa, + auto const expectedSmallExponent, + auto const expectedLargeMantissa, + auto const expectedLargeExponent, + auto const line) { + test( + n, + iRangeMin, + iRangeMax, + expectedSmallMantissa, + expectedSmallExponent, + line); + test( + n, + iBigMin, + iBigMax, + expectedLargeMantissa, + expectedLargeExponent, + line); + + // Only test non-negative. testing a negative number with an + // unsigned range will assert, and asserts can't be tested. + if (n.signum() >= 0) + { + test( + n, + uRangeMin, + uRangeMax, + expectedSmallMantissa, + expectedSmallExponent, + line); + test( + n, + largeRange.min, + largeRange.max, + expectedLargeMantissa, + expectedLargeExponent, + line); + } + }; + + { + // zero + Number const n{0}; + + testSuite( + n, + 0, + std::numeric_limits::lowest(), + 0, + std::numeric_limits::lowest(), + __LINE__); + } + { + // Small positive number + Number const n{2}; + + testSuite(n, 200, -2, 2'000'000'000'000'000'000, -18, __LINE__); + } + { + // Negative number + Number const n{-2}; + + testSuite(n, -200, -2, -2'000'000'000'000'000'000, -18, __LINE__); + } + { + // Biggest valid mantissa + Number const n{Number::largestMantissa, 0, Number::normalized{}}; + + if (scale == MantissaRange::small) + // With the small mantissa range, the value rounds up. Because + // it rounds up, when scaling up to the full int64 range, it + // can't go over the max, so it is one digit smaller than the + // full value. + testSuite(n, 922, 16, 922'337'203'685'477'600, 1, __LINE__); + else + testSuite(n, 922, 16, Number::largestMantissa, 0, __LINE__); + } + { + // Biggest valid mantissa + 1 + Number const n{ + Number::largestMantissa + 1, 0, Number::normalized{}}; + + if (scale == MantissaRange::small) + // With the small mantissa range, the value rounds up. Because + // it rounds up, when scaling up to the full int64 range, it + // can't go over the max, so it is one digit smaller than the + // full value. + testSuite(n, 922, 16, 922'337'203'685'477'600, 1, __LINE__); + else + testSuite(n, 922, 16, 922'337'203'685'477'581, 1, __LINE__); + } + { + // Biggest valid mantissa + 2 + Number const n{ + Number::largestMantissa + 2, 0, Number::normalized{}}; + + if (scale == MantissaRange::small) + // With the small mantissa range, the value rounds up. Because + // it rounds up, when scaling up to the full int64 range, it + // can't go over the max, so it is one digit smaller than the + // full value. + testSuite(n, 922, 16, 922'337'203'685'477'600, 1, __LINE__); + else + testSuite(n, 922, 16, 922'337'203'685'477'581, 1, __LINE__); + } + { + // Biggest valid mantissa + 3 + Number const n{ + Number::largestMantissa + 3, 0, Number::normalized{}}; + + if (scale == MantissaRange::small) + // With the small mantissa range, the value rounds up. Because + // it rounds up, when scaling up to the full int64 range, it + // can't go over the max, so it is one digit smaller than the + // full value. + testSuite(n, 922, 16, 922'337'203'685'477'600, 1, __LINE__); + else + testSuite(n, 922, 16, 922'337'203'685'477'581, 1, __LINE__); + } + { + // int64 min + Number const n{std::numeric_limits::min(), 0}; + + if (scale == MantissaRange::small) + testSuite(n, -922, 16, -922'337'203'685'477'600, 1, __LINE__); + else + testSuite(n, -922, 16, -922'337'203'685'477'581, 1, __LINE__); + } + { + // int64 min + 1 + Number const n{std::numeric_limits::min() + 1, 0}; + + if (scale == MantissaRange::small) + testSuite(n, -922, 16, -922'337'203'685'477'600, 1, __LINE__); + else + testSuite(n, -922, 16, -9'223'372'036'854'775'807, 0, __LINE__); + } + { + // int64 min - 1 + // Need to cast to uint, even though we're dealing with a negative + // number to avoid overflow and UB + Number const n{ + true, + -static_cast( + std::numeric_limits::min()) + + 1, + 0, + Number::normalized{}}; + + if (scale == MantissaRange::small) + testSuite(n, -922, 16, -922'337'203'685'477'600, 1, __LINE__); + else + testSuite(n, -922, 16, -922'337'203'685'477'581, 1, __LINE__); + } + } + void run() override { @@ -1911,6 +2111,7 @@ class Number_test : public beast::unit_test::suite test_truncate(); testRounding(); testInt64(); + testNormalizeToRange(); } } };