diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 0604fb2d55..477ec039af 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -127,18 +127,24 @@ struct MantissaRange final using rep = std::uint64_t; enum class MantissaScale { + // Small can be removed when either featureSingleAssetVault or featureLendingProtocol are + // retired Small, // LargeLegacy can be removed when fixCleanup3_2_0 is retired LargeLegacy, // Large320 can be removed when fixCleanup3_3_0 is retired Large320, + // If Large330 is ever the only remaining "Large*" entry, it can be renamed to just "Large". Large330, }; - // This entire enum can be removed when fixCleanup3_2_0 is retired + // This entire enum can be removed when the last relevant amendment is retired enum class CuspRoundingFix : std::uint8_t { + // Disabled can be removed when fixCleanup3_2_0 is retired Disabled = 0, + // Enabled320 can be removed when fixCleanup3_3_0 is retired Enabled320 = 1, + // If we ever get to the point that there's only one entry, remove the entire enum Enabled330 = 2, }; diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 0de4daaf9a..0e27605020 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -47,7 +47,7 @@ to_string(MantissaRange::MantissaScale const& scale) case MantissaRange::MantissaScale::Large330: return "Large330"; default: - throw std::runtime_error("Bad scale"); + throw std::runtime_error("Bad scale"); // LCOV_EXCL_LINE } } @@ -65,7 +65,7 @@ to_string(Number::RoundingMode const& round) case Number::RoundingMode::Upward: return "Upward"; default: - throw std::runtime_error("Bad rounding mode"); + throw std::runtime_error("Bad rounding mode"); // LCOV_EXCL_LINE } } @@ -216,16 +216,17 @@ concept UnsignedMantissa = std::is_unsigned_v || std::is_same_v 1. The rounding mode 2. The last digit dropped from the mantissa (i.e. the first digit after the decimal point). (first byte of digits_) - 3. Whether any other non-zero digits were dropped from the mantissa. (xbit_) + 3. Whether any other non-zero digits were dropped from the mantissa. (remaining bytes of digits_ + and xbit_) Upward and Downward rounding modes round the unsigned mantissa toward or away from zero depending on whether the sign is negative (sbit_). For positive values, Upward is away, and Downward is toward. For negative values, that's reversed. For simplicity, I'm going to describe - the logic using "TowardZero" and "FromZero". + the logic using "TowardZero" and "AwayFromZero". * TowardZero is the easiest rounding mode. It always rounds down. digits_ and xbit_ are irrelevant. - * FromZero is almost as simple. If both "digits_" and "xbit_" are zero (0), it rounds down. + * AwayFromZero is almost as simple. If both "digits_" and "xbit_" are zero (0), it rounds down. Else it rounds up. * ToNearest is only a little more complicated. If the last dropped digit is < 5, then round down. If it is > 5, round up. If it is exactly 5, and there are _any_ other digits (the @@ -295,7 +296,8 @@ public: doDropDigit(T& mantissa, int& exponent) noexcept; enum class Round { - // The result is exact. No rounding is needed. Only used if cuspRoundingFix is Enabled. + // The result is exact. No rounding is needed. Only used if cuspRoundingFix is Enabled330 or + // higher. Exact = -2, // Round down. Since we use integer math, that usually means no change is needed. // Exceptions are for when the result is between kMaxRep and kMaxRepUp (round to kMaxRep), @@ -549,6 +551,8 @@ Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) } else { + // Need to preserve the incorrect behavior until the fix amendment can be retired, + // because otherwise would risk an unplanned ledger fork. if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { --mantissa; @@ -847,7 +851,7 @@ Number::operator+=(Number const& y) // 2. Then expand the mantissa of expandM/expandE, with a limit for expandM a few orders // of magnitude above the MantissaRange. This will leave a few extra digits for rounding - // later, but no excess. + // later, but nothing excessive. while (shrinkE < expandE && expandE > kMinExponent && expandM < upperLimit) { expandM *= 10; diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index 71e237a393..e14e505aac 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -41,17 +41,15 @@ setCurrentTransactionRules(std::optional r) // Declare the range this way to keep clang-tidy from complaining auto const range = [&r]() { - // If any new conditions with new amendments are added, those amendments must also be added - // to useRulesGuards. + // If any new conditions with new amendments are added to "enableVaultNumbers", those + // amendments must also be added to useRulesGuards. bool const enableVaultNumbers = !r || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); + XRPL_ASSERT( + !r || !enableVaultNumbers || useRulesGuards(*r), + "setCurrentTransactionRules : rule decisions match"); bool const enableCuspRounding320 = !r || r->enabled(fixCleanup3_2_0); bool const enableCuspRounding330 = !r || r->enabled(fixCleanup3_3_0); - XRPL_ASSERT( - !r || - useRulesGuards(*r) == - (enableVaultNumbers || enableCuspRounding320 || enableCuspRounding330), - "setCurrentTransactionRules : rule decisions match"); if (enableVaultNumbers) { @@ -76,8 +74,8 @@ bool useRulesGuards(Rules const& rules) { // The list of amendments used here - to decide whether to create a RulesGuard - must be a - // superset of the list used to figure out which mantissa scale to use in - // setCurrentTransactionRules. Additional amendments can be added if desired. + // superset of the list used to determine "enableVaultNumbers" in setCurrentTransactionRules. + // Additional amendments can be added if desired. // // As soon as any one of these amendments is retired, this whole function can be removed, along // with createGuards, and any other callers, and the first set of guards can be created directly