From 7da643d8648959f83d5d6a7d5d3a869171dbf1ca Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 27 May 2026 11:19:20 -0400 Subject: [PATCH 1/6] fix: Fix a rounding error at the `Number::maxRep` cusp (#7051) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> --- include/xrpl/basics/Number.h | 137 +++++---- include/xrpl/protocol/Rules.h | 13 + include/xrpl/protocol/STAmount.h | 2 +- src/libxrpl/basics/Number.cpp | 351 ++++++++++++++++-------- src/libxrpl/protocol/IOUAmount.cpp | 2 +- src/libxrpl/protocol/Rules.cpp | 62 ++++- src/libxrpl/protocol/STNumber.cpp | 3 +- src/libxrpl/tx/applySteps.cpp | 18 +- src/test/app/AMMExtended_test.cpp | 109 +++----- src/test/app/AMM_test.cpp | 9 - src/test/app/Invariants_test.cpp | 206 +++++++------- src/test/basics/IOUAmount_test.cpp | 3 +- src/test/basics/Number_test.cpp | 86 +++++- src/test/protocol/STNumber_test.cpp | 3 +- src/test/rpc/GetAggregatePrice_test.cpp | 3 +- 15 files changed, 646 insertions(+), 361 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index e67f1f534d..3aeb4b5bcd 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -7,7 +7,9 @@ #include #include #include +#include #include +#include namespace xrpl { @@ -44,11 +46,11 @@ isPowerOfTen(T value) * * 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. + * The MantissaScale enum indicates properties of the range: size, and some behavioral + * options. This intentionally restricts the number of unique MantissaRanges that can + * be instantiated: one for each scale. * - * The "small" scale is based on the behavior of STAmount for IOUs. It has a min + * 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 * uses before Lending Protocol was implemented, mostly related to AMM. * @@ -59,29 +61,54 @@ isPowerOfTen(T value) * STNumber field type, and for internal calculations. That necessitated the * "large" scale. * - * The "large" scale is intended to represent all values that can be represented + * The "Large" scales are 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. + * value of 10^19-1. "LargeLegacy" is like "Large", but preserves + * a rounding error when a computation results in a mantissa of + * Number::kMaxRep that needs to be rounded up, but rounds down + * instead. It will maintain consistent behavior until the fixCleanup3_2_0 + * amendment is enabled. * * 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 + * should be left in place, but the "Small" scale option should be removed. This * will allow for future expansion beyond 64-bits if it is ever needed. */ -struct MantissaRange +struct MantissaRange final { using rep = std::uint64_t; - enum class MantissaScale { Small, Large }; + enum class MantissaScale { + Small, + // LargeLegacy can be removed when fixCleanup3_2_0 is retired + LargeLegacy, + Large, + }; + + // This entire enum can be removed when fixCleanup3_2_0 is retired + enum class CuspRoundingFix : bool { + Disabled = false, + Enabled = true, + }; explicit constexpr MantissaRange(MantissaScale scale) - : min(getMin(scale)), log(logTen(min).value_or(-1)), scale(scale) + : min(getMin(scale)) + , cuspRoundingFixEnabled(isCuspFixEnabled(scale)) + , log(logTen(min).value_or(-1)) + , scale(scale) { } rep min; rep max{(min * 10) - 1}; + CuspRoundingFix cuspRoundingFixEnabled; int log; MantissaScale scale; + static MantissaRange const& + getMantissaRange(MantissaScale scale); + + static std::set const& + getAllScales(); + private: static constexpr rep getMin(MantissaScale scale) @@ -90,15 +117,35 @@ private: { case MantissaScale::Small: return 1'000'000'000'000'000ULL; + case MantissaScale::LargeLegacy: case MantissaScale::Large: return 1'000'000'000'000'000'000ULL; default: - // Since this can never be called outside a non-constexpr - // context, this throw assures that the build fails if an + // If called in a constexpr context, this throw assures that the build fails if an // invalid scale is used. - throw std::runtime_error("Unknown mantissa scale"); + throw std::runtime_error("Unknown mantissa scale"); // LCOV_EXCL_LINE } } + + static constexpr CuspRoundingFix + isCuspFixEnabled(MantissaScale scale) + { + switch (scale) + { + case MantissaScale::Small: + case MantissaScale::LargeLegacy: + return CuspRoundingFix::Disabled; + case MantissaScale::Large: + return CuspRoundingFix::Enabled; + default: + // If called in a constexpr context, this throw assures that the build fails if an + // invalid scale is used. + throw std::runtime_error("Unknown mantissa scale"); // LCOV_EXCL_LINE + } + } + + static std::unordered_map const& + getRanges(); }; // Like std::integral, but only 64-bit integral types. @@ -203,7 +250,7 @@ concept Integral64 = std::is_same_v || std::is_same_v + template < + auto MinMantissa, + auto MaxMantissa, + Integral64 T = std::decay_t, + Integral64 TMax = std::decay_t> [[nodiscard]] std::pair - normalizeToRange(T minMantissa, T maxMantissa) const; + normalizeToRange() const; private: static thread_local RoundingMode mode; // The available ranges for mantissa - static constexpr MantissaRange kSmallRange{MantissaRange::MantissaScale::Small}; - static_assert(isPowerOfTen(kSmallRange.min)); - static_assert(kSmallRange.min == 1'000'000'000'000'000LL); - static_assert(kSmallRange.max == 9'999'999'999'999'999LL); - static_assert(kSmallRange.log == 15); - static_assert(kSmallRange.min < kMaxRep); - static_assert(kSmallRange.max < kMaxRep); - static constexpr MantissaRange kLargeRange{MantissaRange::MantissaScale::Large}; - static_assert(isPowerOfTen(kLargeRange.min)); - static_assert(kLargeRange.min == 1'000'000'000'000'000'000ULL); - static_assert(kLargeRange.max == internalrep(9'999'999'999'999'999'999ULL)); - static_assert(kLargeRange.log == 18); - static_assert(kLargeRange.min < kMaxRep); - static_assert(kLargeRange.max > kMaxRep); - // The range for the mantissa when normalized. // Use reference_wrapper to avoid making copies, and prevent accidentally // changing the values inside the range. static thread_local std::reference_wrapper kRange; void - normalize(); + normalize(MantissaRange const& range); /** Normalize Number components to an arbitrary range. * @@ -481,7 +508,8 @@ private: T& mantissa, int& exponent, internalrep const& minMantissa, - internalrep const& maxMantissa); + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); template friend void @@ -490,7 +518,8 @@ private: T& mantissa, int& exponent, MantissaRange::rep const& minMantissa, - MantissaRange::rep const& maxMantissa); + MantissaRange::rep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); [[nodiscard]] bool isnormal() const noexcept; @@ -526,7 +555,7 @@ static constexpr Number kNumZero{}; inline Number::Number(bool negative, internalrep mantissa, int exponent, Normalized) : Number(negative, mantissa, exponent, Unchecked{}) { - normalize(); + normalize(kRange); } inline Number::Number(internalrep mantissa, int exponent, Normalized) @@ -696,10 +725,19 @@ Number::isnormal() const noexcept kMinExponent <= exponent_ && exponent_ <= kMaxExponent); } -template +template std::pair -Number::normalizeToRange(T minMantissa, T maxMantissa) const +Number::normalizeToRange() const { + static_assert(std::is_same_v || std::is_same_v); + static_assert(std::is_same_v); + auto constexpr kMIN = static_cast(MinMantissa); + auto constexpr kMAX = static_cast(MaxMantissa); + static_assert(kMIN > 0); + static_assert(kMIN % 10 == 0); + static_assert(kMAX % 10 == 9); + static_assert((kMAX + 1) / 10 == kMIN); + bool negative = negative_; internalrep mantissa = mantissa_; int exponent = exponent_; @@ -711,7 +749,10 @@ Number::normalizeToRange(T minMantissa, T maxMantissa) const "xrpl::Number::normalizeToRange", "Number is non-negative for unsigned range."); } - Number::normalize(negative, mantissa, exponent, minMantissa, maxMantissa); + // Don't need to worry about the cuspRounding fix because rounding up will never take the + // mantissa over maxMantissa with a ones digit value other than 0. 0 can safely be truncated. + Number::normalize( + negative, mantissa, exponent, kMIN, kMAX, MantissaRange::CuspRoundingFix::Disabled); auto const sign = negative ? -1 : 1; return std::make_pair(static_cast(sign * mantissa), exponent); @@ -763,6 +804,8 @@ to_string(MantissaRange::MantissaScale const& scale) { case MantissaRange::MantissaScale::Small: return "small"; + case MantissaRange::MantissaScale::LargeLegacy: + return "largeLegacy"; case MantissaRange::MantissaScale::Large: return "large"; default: diff --git a/include/xrpl/protocol/Rules.h b/include/xrpl/protocol/Rules.h index fbbd3d8805..9c17ff2391 100644 --- a/include/xrpl/protocol/Rules.h +++ b/include/xrpl/protocol/Rules.h @@ -122,4 +122,17 @@ private: std::optional saved_; }; +class NumberSO; +class NumberMantissaScaleGuard; + +bool +useRulesGuards(Rules const& rules); + +void +createGuards( + Rules const& rules, + std::optional& stNumberSO, + std::optional& rulesGuard, + std::optional& mantissaScaleGuard); + } // namespace xrpl diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index a4fffad40c..1a5b442d8b 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -559,7 +559,7 @@ STAmount::fromNumber(A const& a, Number const& number) return STAmount{asset, intValue, 0, negative}; } - auto const [mantissa, exponent] = working.normalizeToRange(kMinValue, kMaxValue); + auto const [mantissa, exponent] = working.normalizeToRange(); return STAmount{asset, mantissa, exponent, negative}; } diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 06bd78d8b0..11f5934b04 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -10,9 +10,11 @@ #include #include #include +#include #include #include #include +#include #include #ifdef _MSC_VER @@ -28,7 +30,76 @@ using int128_t = __int128_t; namespace xrpl { thread_local Number::RoundingMode Number::mode = Number::RoundingMode::ToNearest; -thread_local std::reference_wrapper Number::kRange = kLargeRange; +thread_local std::reference_wrapper Number::kRange = + MantissaRange::getMantissaRange(MantissaRange::MantissaScale::Large); + +std::set const& +MantissaRange::getAllScales() +{ + static std::set const kScales = { + MantissaRange::MantissaScale::Small, + MantissaRange::MantissaScale::LargeLegacy, + MantissaRange::MantissaScale::Large, + }; + return kScales; +} + +std::unordered_map const& +MantissaRange::getRanges() +{ + static auto const kMap = []() { + std::unordered_map map; + for (auto const scale : getAllScales()) + { + map.emplace(scale, scale); + } + + // Use these constexpr declarations to do static_asserts to verify the MantissaRanges are + // created correctly, but nothing else. + { + [[maybe_unused]] + constexpr static MantissaRange kRange{MantissaRange::MantissaScale::Small}; + static_assert(isPowerOfTen(kRange.min)); + static_assert(kRange.min == 1'000'000'000'000'000LL); + static_assert(kRange.max == 9'999'999'999'999'999LL); + static_assert(kRange.log == 15); + static_assert(kRange.min < Number::kMaxRep); + static_assert(kRange.max < Number::kMaxRep); + static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + } + { + [[maybe_unused]] + constexpr static MantissaRange kRange{MantissaRange::MantissaScale::LargeLegacy}; + static_assert(isPowerOfTen(kRange.min)); + static_assert(kRange.min == 1'000'000'000'000'000'000ULL); + static_assert(kRange.max == rep(9'999'999'999'999'999'999ULL)); + static_assert(kRange.log == 18); + static_assert(kRange.min < Number::kMaxRep); + static_assert(kRange.max > Number::kMaxRep); + static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + } + { + [[maybe_unused]] + constexpr static MantissaRange kRange{MantissaRange::MantissaScale::Large}; + static_assert(isPowerOfTen(kRange.min)); + static_assert(kRange.min == 1'000'000'000'000'000'000ULL); + static_assert(kRange.max == rep(9'999'999'999'999'999'999ULL)); + static_assert(kRange.log == 18); + static_assert(kRange.min < Number::kMaxRep); + static_assert(kRange.max > Number::kMaxRep); + static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Enabled); + } + return map; + }(); + + return kMap; +} + +MantissaRange const& +MantissaRange::getMantissaRange(MantissaScale scale) +{ + return getRanges().at(scale); +} Number::RoundingMode Number::getround() @@ -51,10 +122,37 @@ Number::getMantissaScale() void Number::setMantissaScale(MantissaRange::MantissaScale scale) { - if (scale != MantissaRange::MantissaScale::Small && - scale != MantissaRange::MantissaScale::Large) + if (!MantissaRange::getAllScales().contains(scale)) logicError("Unknown mantissa scale"); - kRange = scale == MantissaRange::MantissaScale::Small ? kSmallRange : kLargeRange; + kRange = MantissaRange::getMantissaRange(scale); +} + +// Optimization equivalent to: +// auto r = static_cast(u % 10); +// u /= 10; +// return r; +// Derived from Hacker's Delight Second Edition Chapter 10 +// by Henry S. Warren, Jr. +static inline unsigned +divu10(uint128_t& u) +{ + // q = u * 0.75 + auto q = (u >> 1) + (u >> 2); + // iterate towards q = u * 0.8 + q += q >> 4; + q += q >> 8; + q += q >> 16; + q += q >> 32; + q += q >> 64; + // q /= 8 approximately == u / 10 + q >>= 3; + // r = u - q * 10 approximately == u % 10 + auto r = static_cast(u - ((q << 3) + (q << 1))); + // correction c is 1 if r >= 10 else 0 + auto c = (r + 6) >> 4; + u = q + c; + r -= c * 10; + return r; } // Guard @@ -92,6 +190,18 @@ public: unsigned pop() noexcept; + /** Drop a digit from the mantissa, and increment the exponent, storing the dropped digit in + * this Guard. + * + * Substitute for: + push(mantissa % 10); + mantissa /= 10; + ++exponent; + */ + template + void + doDropDigit(T& mantissa, int& exponent) noexcept; + // Indicate round direction: 1 is up, -1 is down, 0 is even // This enables the client to round towards nearest, and on // tie, round towards even. @@ -107,6 +217,7 @@ public: int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, std::string location); // Modify the result to the correctly rounded value @@ -168,6 +279,27 @@ Number::Guard::pop() noexcept return d; } +template +void +Number::Guard::doDropDigit(T& mantissa, int& exponent) noexcept +{ + push(mantissa % 10); + mantissa /= 10; + ++exponent; +} + +// Use the divu10 optimization for uint128s +template <> +void +Number::Guard::doDropDigit(uint128_t& mantissa, int& exponent) noexcept +{ + // The following is optimization for: + // push(static_cast(mantissa % 10)); + // mantissa /= 10; + push(divu10(mantissa)); + ++exponent; +} + // Returns: // -1 if Guard is less than half // 0 if Guard is exactly half @@ -242,18 +374,60 @@ Number::Guard::doRoundUp( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, std::string location) { auto r = round(); if (r == 1 || (r == 0 && (mantissa & 1) == 1)) { - ++mantissa; - // Ensure mantissa after incrementing fits within both the - // min/maxMantissa range and is a valid "rep". - if (mantissa > maxMantissa || mantissa > kMaxRep) + auto const safeToIncrement = [&maxMantissa](auto const& mantissa) { + return mantissa < maxMantissa && mantissa < kMaxRep; + }; + if (cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) { - mantissa /= 10; - ++exponent; + // Ensure mantissa after incrementing fits within both the + // min/maxMantissa range and is a valid "rep". + if (safeToIncrement(mantissa)) + { + // Nothing unusual here, just increment the mantissa + ++mantissa; + } + else + { + // Incrementing the mantissa will require dividing, which will require rounding. So + // _don't_ increment the mantissa. Instead, divide and round recursively. It should + // be impossible to recurse more than once, because once the mantissa is divided by + // 10, it will be _well_ under maxMantissa and kMaxRep, so adding 1 will have no + // change of bringing it back over. + doDropDigit(mantissa, exponent); + XRPL_ASSERT_PARTS( + safeToIncrement(mantissa), + "xrpl::Number::Guard::doRoundUp", + "can't recurse more than once"); + doRoundUp( + negative, + mantissa, + exponent, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + location); + return; + } + } + else + { + // Need to preserve the incorrect behavior until the fix amendment can be retired, + // because otherwise would risk an unplanned ledger fork. + ++mantissa; + // Ensure mantissa after incrementing fits within both the + // min/maxMantissa range and is a valid "rep". + if (mantissa > maxMantissa || mantissa > kMaxRep) + { + // Don't use doDropDigit here + mantissa /= 10; + ++exponent; + } } } bringIntoRange(negative, mantissa, exponent, minMantissa); @@ -293,9 +467,9 @@ Number::Guard::doRound(rep& drops, std::string location) const { static_assert(sizeof(internalrep) == sizeof(rep)); // This should be impossible, because it's impossible to represent - // "maxRep + 0.6" in Number, regardless of the scale. There aren't - // enough digits available. You'd either get a mantissa of "maxRep" - // or "(maxRep + 1) / 10", neither of which will round up when + // "kMaxRep + 0.6" in Number, regardless of the scale. There aren't + // enough digits available. You'd either get a mantissa of "kMaxRep" + // or "(kMaxRep + 1) / 10", neither of which will round up when // converting to rep, though the latter might overflow _before_ // rounding. Throw(std::string(location)); // LCOV_EXCL_LINE @@ -331,29 +505,11 @@ Number::externalToInternal(rep mantissa) return static_cast(-temp); } -constexpr Number -Number::oneSmall() -{ - return Number{false, Number::kSmallRange.min, -Number::kSmallRange.log, Number::Unchecked{}}; -}; - -constexpr Number kOneSml = Number::oneSmall(); - -constexpr Number -Number::oneLarge() -{ - return Number{false, Number::kLargeRange.min, -Number::kLargeRange.log, Number::Unchecked{}}; -}; - -constexpr Number kOneLrg = Number::oneLarge(); - Number Number::one() { - if (&kRange.get() == &kSmallRange) - return kOneSml; - XRPL_ASSERT(&kRange.get() == &kLargeRange, "Number::one() : valid range"); - return kOneLrg; + auto const& range = kRange.get(); + return Number{false, range.min, -range.log, Number::Unchecked{}}; } // Use the member names in this static function for now so the diff is cleaner @@ -365,7 +521,8 @@ doNormalize( T& mantissa, int& exponent, MantissaRange::rep const& minMantissa, - MantissaRange::rep const& maxMantissa) + MantissaRange::rep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { static constexpr auto kMinExponent = Number::kMinExponent; static constexpr auto kMaxExponent = Number::kMaxExponent; @@ -394,9 +551,7 @@ doNormalize( { if (exponent >= kMaxExponent) throw std::overflow_error("Number::normalize 1"); - g.push(m % 10); - m /= 10; - ++exponent; + g.doDropDigit(m, exponent); } if ((exponent < kMinExponent) || (m < minMantissa)) { @@ -407,7 +562,7 @@ doNormalize( } // 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 + // 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. // @@ -415,26 +570,31 @@ doNormalize( // 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 + // 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. + // mantissa() will return mantissa / 10, and exponent() will return + // exponent + 1. if (m > kMaxRep) { if (exponent >= kMaxExponent) throw std::overflow_error("Number::normalize 1.5"); - g.push(m % 10); - m /= 10; - ++exponent; + g.doDropDigit(m, 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) + // modification, it must be less than kMaxRep. In other words, the original + // value should have been no more than kMaxRep * 10. + // (kMaxRep * 10 > maxMantissa) XRPL_ASSERT_PARTS(m <= kMaxRep, "xrpl::doNormalize", "intermediate mantissa fits in int64"); mantissa = m; - g.doRoundUp(negative, mantissa, exponent, minMantissa, maxMantissa, "Number::normalize 2"); + g.doRoundUp( + negative, + mantissa, + exponent, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + "Number::normalize 2"); XRPL_ASSERT_PARTS( mantissa >= minMantissa && mantissa <= maxMantissa, "xrpl::doNormalize", @@ -448,9 +608,10 @@ Number::normalize( uint128_t& mantissa, int& exponent, internalrep const& minMantissa, - internalrep const& maxMantissa) + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { - doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled); } template <> @@ -460,9 +621,10 @@ Number::normalize( unsigned long long& mantissa, int& exponent, internalrep const& minMantissa, - internalrep const& maxMantissa) + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { - doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled); } template <> @@ -472,16 +634,16 @@ Number::normalize( unsigned long& mantissa, int& exponent, internalrep const& minMantissa, - internalrep const& maxMantissa) + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { - doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled); } void -Number::normalize() +Number::normalize(MantissaRange const& range) { - auto const& range = kRange.get(); - normalize(negative_, mantissa_, exponent_, range.min, range.max); + normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFixEnabled); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -542,9 +704,7 @@ Number::operator+=(Number const& y) g.setNegative(); do { - g.push(xm % 10); - xm /= 10; - ++xe; + g.doDropDigit(xm, xe); } while (xe < ye); } else if (xe > ye) @@ -553,26 +713,30 @@ Number::operator+=(Number const& y) g.setNegative(); do { - g.push(ym % 10); - ym /= 10; - ++ye; + g.doDropDigit(ym, ye); } while (xe > ye); } auto const& range = kRange.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; + auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; if (xn == yn) { xm += ym; if (xm > maxMantissa || xm > kMaxRep) { - g.push(xm % 10); - xm /= 10; - ++xe; + g.doDropDigit(xm, xe); } - g.doRoundUp(xn, xm, xe, minMantissa, maxMantissa, "Number::addition overflow"); + g.doRoundUp( + xn, + xm, + xe, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + "Number::addition overflow"); } else { @@ -598,38 +762,10 @@ Number::operator+=(Number const& y) negative_ = xn; mantissa_ = static_cast(xm); exponent_ = xe; - normalize(); + normalize(range); return *this; } -// Optimization equivalent to: -// auto r = static_cast(u % 10); -// u /= 10; -// return r; -// Derived from Hacker's Delight Second Edition Chapter 10 -// by Henry S. Warren, Jr. -static inline unsigned -divu10(uint128_t& u) -{ - // q = u * 0.75 - auto q = (u >> 1) + (u >> 2); - // iterate towards q = u * 0.8 - q += q >> 4; - q += q >> 8; - q += q >> 16; - q += q >> 32; - q += q >> 64; - // q /= 8 approximately == u / 10 - q >>= 3; - // r = u - q * 10 approximately == u % 10 - auto r = static_cast(u - ((q << 3) + (q << 1))); - // correction c is 1 if r >= 10 else 0 - auto c = (r + 6) >> 4; - u = q + c; - r -= c * 10; - return r; -} - Number& Number::operator*=(Number const& y) { @@ -667,15 +803,13 @@ Number::operator*=(Number const& y) auto const& range = kRange.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; + auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; while (zm > maxMantissa || zm > kMaxRep) { - // The following is optimization for: - // g.push(static_cast(zm % 10)); - // zm /= 10; - g.push(divu10(zm)); - ++ze; + g.doDropDigit(zm, ze); } + xm = static_cast(zm); xe = ze; g.doRoundUp( @@ -684,12 +818,13 @@ Number::operator*=(Number const& y) xe, minMantissa, maxMantissa, + cuspRoundingFixEnabled, "Number::multiplication overflow : exponent is " + std::to_string(xe)); negative_ = zn; mantissa_ = xm; exponent_ = xe; - normalize(); + normalize(range); return *this; } @@ -721,6 +856,7 @@ Number::operator/=(Number const& y) auto const& range = kRange.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; + auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; // Shift by 10^17 gives greatest precision while not overflowing // uint128_t or the cast back to int64_t @@ -728,8 +864,6 @@ Number::operator/=(Number const& y) // log(2^128,10) ~ 38.5 // largeRange.log = 18, fits in 10^19 // f can be up to 10^(38-19) = 10^19 safely - static_assert(kSmallRange.log == 15); - static_assert(kLargeRange.log == 18); bool const small = Number::getMantissaScale() == MantissaRange::MantissaScale::Small; uint128_t const f = small ? 100'000'000'000'000'000 : 10'000'000'000'000'000'000ULL; XRPL_ASSERT_PARTS(f >= minMantissa * 10, "Number::operator/=", "factor expected size"); @@ -779,7 +913,7 @@ Number::operator/=(Number const& y) ze -= 3; } } - normalize(zn, zm, ze, minMantissa, maxMantissa); + normalize(zn, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled); negative_ = zn; mantissa_ = static_cast(zm); exponent_ = ze; @@ -801,10 +935,9 @@ operator rep() const g.setNegative(); drops = -drops; } - for (; offset < 0; ++offset) + while (offset < 0) { - g.push(drops % 10); - drops /= 10; + g.doDropDigit(drops, offset); } for (; offset > 0; --offset) { @@ -831,7 +964,7 @@ Number::truncate() const noexcept } // We are guaranteed that normalize() will never throw an exception // because exponent is either negative or zero at this point. - ret.normalize(); + ret.normalize(kRange); return ret; } diff --git a/src/libxrpl/protocol/IOUAmount.cpp b/src/libxrpl/protocol/IOUAmount.cpp index d65ba41a01..d214995809 100644 --- a/src/libxrpl/protocol/IOUAmount.cpp +++ b/src/libxrpl/protocol/IOUAmount.cpp @@ -56,7 +56,7 @@ IOUAmount::fromNumber(Number const& number) // to normalize, which calls fromNumber IOUAmount result{}; std::tie(result.mantissa_, result.exponent_) = - number.normalizeToRange(kMinMantissa, kMaxMantissa); + number.normalizeToRange(); return result; } diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index 2c971749b6..08a95145eb 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -38,15 +39,68 @@ setCurrentTransactionRules(std::optional r) // Make global changes associated with the rules before the value is moved. // Push the appropriate setting, instead of having the class pull every time // the value is needed. That could get expensive fast. - bool const enableLargeNumbers = + + // If any new conditions with new amendments are added, those amendments must also be added to + // useRulesGuards. + bool const enableVaultNumbers = !r || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); - Number::setMantissaScale( - enableLargeNumbers ? MantissaRange::MantissaScale::Large - : MantissaRange::MantissaScale::Small); + bool const enableCuspRoundingFix = !r || r->enabled(fixCleanup3_2_0); + XRPL_ASSERT( + !r || useRulesGuards(*r) == (enableCuspRoundingFix || enableVaultNumbers), + "setCurrentTransactionRules : rule decisions match"); + + // Declare the range this way to keep clang-tidy from complaining + auto const range = [enableCuspRoundingFix, enableVaultNumbers]() { + if (enableVaultNumbers) + { + if (enableCuspRoundingFix) + { + return MantissaRange::MantissaScale::Large; + } + return MantissaRange::MantissaScale::LargeLegacy; + } + return MantissaRange::MantissaScale::Small; + }(); + Number::setMantissaScale(range); *getCurrentTransactionRulesRef() = std::move(r); } +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. + // + // 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 + // at the call site, without using optional. + return rules.enabled(fixCleanup3_2_0) || rules.enabled(featureSingleAssetVault) || + rules.enabled(featureLendingProtocol); +} + +void +createGuards( + Rules const& rules, + std::optional& stNumberSO, + std::optional& rulesGuard, + std::optional& mantissaScaleGuard) +{ + if (useRulesGuards(rules)) + { + // raii classes for the current ledger rules. + // fixUniversalNumber predates the rulesGuard and should be replaced. + stNumberSO.emplace(rules.enabled(fixUniversalNumber)); + rulesGuard.emplace(rules); + } + else + { + // Without those features enabled, always use the old number rules. + mantissaScaleGuard.emplace(MantissaRange::MantissaScale::Small); + } +} + class Rules::Impl { private: diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index aa3e83515c..8ef7b9760f 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -96,7 +96,8 @@ STNumber::add(Serializer& s) const // Json. Regardless, the only time we should be serializing an // STNumber is when the scale is large. XRPL_ASSERT_PARTS( - Number::getMantissaScale() == MantissaRange::MantissaScale::Large, + Number::getMantissaScale() == MantissaRange::MantissaScale::LargeLegacy || + Number::getMantissaScale() == MantissaRange::MantissaScale::Large, "xrpl::STNumber::add", "STNumber only used with large mantissa scale"); #endif diff --git a/src/libxrpl/tx/applySteps.cpp b/src/libxrpl/tx/applySteps.cpp index e0b1af80e2..217fdd717f 100644 --- a/src/libxrpl/tx/applySteps.cpp +++ b/src/libxrpl/tx/applySteps.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -66,26 +65,15 @@ withTxnType(Rules const& rules, TxType txnType, F&& f) // so these need to be more global. // // To prevent unintentional side effects on existing checks, they will be - // set for every operation only once SingleAssetVault (or later - // LendingProtocol) are enabled. + // set for every operation only once at least one of the relevant amendments + // are enabled. // // See also Transactor::operator(). // std::optional stNumberSO; std::optional rulesGuard; std::optional mantissaScaleGuard; - if (rules.enabled(featureSingleAssetVault) || rules.enabled(featureLendingProtocol)) - { - // raii classes for the current ledger rules. - // fixUniversalNumber predates the rulesGuard and should be replaced. - stNumberSO.emplace(rules.enabled(fixUniversalNumber)); - rulesGuard.emplace(rules); - } - else - { - // Without those features enabled, always use the old number rules. - mantissaScaleGuard.emplace(MantissaRange::MantissaScale::Small); - } + createGuards(rules, stNumberSO, rulesGuard, mantissaScaleGuard); switch (txnType) { diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 18f2d6df2f..a0a7d0fb15 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -65,10 +65,15 @@ namespace xrpl::test { /** * Tests of AMM that use offers too. */ -struct AMMExtended_test : public jtx::AMMTest +class AMMExtended_test : public jtx::AMMTest { // Use small Number mantissas for the life of this test. - NumberMantissaScaleGuard const sg{xrpl::MantissaRange::MantissaScale::Small}; + NumberMantissaScaleGuard const sg_{xrpl::MantissaRange::MantissaScale::Small}; + + // For now, just disable SAV entirely, which locks in the small Number + // mantissas + FeatureBitset const all_{ + testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; private: void @@ -1349,37 +1354,33 @@ private: testOffers() { using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas - FeatureBitset const all{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testRmFundedOffer(all); - testRmFundedOffer(all - fixAMMv1_1 - fixAMMv1_3); - testEnforceNoRipple(all); - testFillModes(all); - testOfferCrossWithXRP(all); - testOfferCrossWithLimitOverride(all); - testCurrencyConversionEntire(all); - testCurrencyConversionInParts(all); - testCrossCurrencyStartXRP(all); - testCrossCurrencyEndXRP(all); - testCrossCurrencyBridged(all); - testOfferFeesConsumeFunds(all); - testOfferCreateThenCross(all); - testSellFlagExceedLimit(all); - testGatewayCrossCurrency(all); - testGatewayCrossCurrency(all - fixAMMv1_1 - fixAMMv1_3); - testBridgedCross(all); - testSellWithFillOrKill(all); - testTransferRateOffer(all); - testSelfIssueOffer(all); - testBadPathAssert(all); - testSellFlagBasic(all); - testDirectToDirectPath(all); - testDirectToDirectPath(all - fixAMMv1_1 - fixAMMv1_3); - testRequireAuth(all); - testMissingAuth(all); + testRmFundedOffer(all_); + testRmFundedOffer(all_ - fixAMMv1_1 - fixAMMv1_3); + testEnforceNoRipple(all_); + testFillModes(all_); + testOfferCrossWithXRP(all_); + testOfferCrossWithLimitOverride(all_); + testCurrencyConversionEntire(all_); + testCurrencyConversionInParts(all_); + testCrossCurrencyStartXRP(all_); + testCrossCurrencyEndXRP(all_); + testCrossCurrencyBridged(all_); + testOfferFeesConsumeFunds(all_); + testOfferCreateThenCross(all_); + testSellFlagExceedLimit(all_); + testGatewayCrossCurrency(all_); + testGatewayCrossCurrency(all_ - fixAMMv1_1 - fixAMMv1_3); + testBridgedCross(all_); + testSellWithFillOrKill(all_); + testTransferRateOffer(all_); + testSelfIssueOffer(all_); + testBadPathAssert(all_); + testSellFlagBasic(all_); + testDirectToDirectPath(all_); + testDirectToDirectPath(all_ - fixAMMv1_1 - fixAMMv1_3); + testRequireAuth(all_); + testMissingAuth(all_); } void @@ -3516,15 +3517,11 @@ private: testFlow() { using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const all{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testFalseDry(all); - testBookStep(all); - testTransferRateNoOwnerFee(all); - testTransferRateNoOwnerFee(all - fixAMMv1_1 - fixAMMv1_3); + testFalseDry(all_); + testBookStep(all_); + testTransferRateNoOwnerFee(all_); + testTransferRateNoOwnerFee(all_ - fixAMMv1_1 - fixAMMv1_3); testLimitQuality(); testXRPPathLoop(); } @@ -3533,34 +3530,22 @@ private: testCrossingLimits() { using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const all{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testStepLimit(all); - testStepLimit(all - fixAMMv1_1 - fixAMMv1_3); + testStepLimit(all_); + testStepLimit(all_ - fixAMMv1_1 - fixAMMv1_3); } void testDeliverMin() { using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const all{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testConvertAllOfAnAsset(all); - testConvertAllOfAnAsset(all - fixAMMv1_1 - fixAMMv1_3); + testConvertAllOfAnAsset(all_); + testConvertAllOfAnAsset(all_ - fixAMMv1_1 - fixAMMv1_3); } void testDepositAuth() { - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const all{ - jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testPayment(all); + testPayment(all_); testPayIOU(); } @@ -3568,13 +3553,9 @@ private: testFreeze() { using namespace test::jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const sa{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testRippleState(sa); - testGlobalFreeze(sa); - testOffersWhenFrozen(sa); + testRippleState(all_); + testGlobalFreeze(all_); + testOffersWhenFrozen(all_); } void diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 17f959ae3c..64972c24ab 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -2625,10 +2625,6 @@ private: using namespace jtx; using namespace std::chrono; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas - features = features - featureSingleAssetVault - featureLendingProtocol; - // Auction slot initially is owned by AMM creator, who pays 0 price. // Bid 110 tokens. Pay bidMin. @@ -3337,11 +3333,6 @@ private: testcase("Basic Payment"); using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas - features = - features - featureSingleAssetVault - featureLendingProtocol - featureLendingProtocol; - // Payment 100USD for 100XRP. // Force one path with tfNoRippleDirect. testAMM( diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index c8a6e813de..432dccce61 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -4767,109 +4767,119 @@ class Invariants_test : public beast::unit_test::Suite std::vector values; }; - NumberMantissaScaleGuard const g{MantissaRange::MantissaScale::Large}; - - auto makeDelta = [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { - return {.delta = n, .scale = scale(n, vaultAsset.raw())}; - }; - - auto const testCases = std::vector{ - { - .name = "No values", - .expectedMinScale = 0, - .values = {}, - }, - { - .name = "Mixed integer and Number values", - .expectedMinScale = -15, - .values = {makeDelta(1), makeDelta(-1), makeDelta(Number{10, -1})}, - }, - { - .name = "Mixed scales", - .expectedMinScale = -17, - .values = - {makeDelta(Number{1, -2}), makeDelta(Number{5, -3}), makeDelta(Number{3, -2})}, - }, - { - .name = "Equal scales", - .expectedMinScale = -16, - .values = - {makeDelta(Number{1, -1}), makeDelta(Number{5, -1}), makeDelta(Number{1, -1})}, - }, - { - .name = "Mixed mantissa sizes", - .expectedMinScale = -12, - .values = - {makeDelta(Number{1}), - makeDelta(Number{1234, -3}), - makeDelta(Number{12345, -6}), - makeDelta(Number{123, 1})}, - }, - }; - - for (auto const& tc : testCases) + for (auto const mantissaScale : { + MantissaRange::MantissaScale::LargeLegacy, + MantissaRange::MantissaScale::Large, + }) { - testcase("vault computeCoarsestScale: " + tc.name); + NumberMantissaScaleGuard const g{mantissaScale}; - auto const actualScale = ValidVault::computeCoarsestScale(tc.values); + auto makeDelta = [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { + return {.delta = n, .scale = scale(n, vaultAsset.raw())}; + }; - BEAST_EXPECTS( - 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.delta, actualScale); - BEAST_EXPECTS( - actualRounded == num.delta, - "number " + to_string(num.delta) + " rounded to scale " + - std::to_string(actualScale) + " is " + to_string(actualRounded)); - } - } - - auto const testCases2 = std::vector{ - { - .name = "False equivalence", - .expectedMinScale = -15, - .values = - { - makeDelta(Number{1234567890123456789, -18}), - makeDelta(Number{12345, -4}), - makeDelta(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 computeCoarsestScale: " + tc.name); - - auto const actualScale = ValidVault::computeCoarsestScale(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) + auto const testCases = std::vector{ { - first = num.delta; - firstRounded = roundToAsset(vaultAsset, num.delta, actualScale); - continue; - } - auto const numRounded = roundToAsset(vaultAsset, num.delta, actualScale); + .name = "No values", + .expectedMinScale = 0, + .values = {}, + }, + { + .name = "Mixed integer and Number values", + .expectedMinScale = -15, + .values = {makeDelta(1), makeDelta(-1), makeDelta(Number{10, -1})}, + }, + { + .name = "Mixed scales", + .expectedMinScale = -17, + .values = + {makeDelta(Number{1, -2}), + makeDelta(Number{5, -3}), + makeDelta(Number{3, -2})}, + }, + { + .name = "Equal scales", + .expectedMinScale = -16, + .values = + {makeDelta(Number{1, -1}), + makeDelta(Number{5, -1}), + makeDelta(Number{1, -1})}, + }, + { + .name = "Mixed mantissa sizes", + .expectedMinScale = -12, + .values = + {makeDelta(Number{1}), + makeDelta(Number{1234, -3}), + makeDelta(Number{12345, -6}), + makeDelta(Number{123, 1})}, + }, + }; + + for (auto const& tc : testCases) + { + testcase("vault computeCoarsestScale: " + tc.name); + + auto const actualScale = ValidVault::computeCoarsestScale(tc.values); + BEAST_EXPECTS( - numRounded != firstRounded, - "at a scale of " + std::to_string(actualScale) + " " + to_string(num.delta) + - " == " + to_string(*first)); + 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.delta, actualScale); + BEAST_EXPECTS( + actualRounded == num.delta, + "number " + to_string(num.delta) + " rounded to scale " + + std::to_string(actualScale) + " is " + to_string(actualRounded)); + } + } + + auto const testCases2 = std::vector{ + { + .name = "False equivalence", + .expectedMinScale = -15, + .values = + { + makeDelta(Number{1234567890123456789, -18}), + makeDelta(Number{12345, -4}), + makeDelta(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 computeCoarsestScale: " + tc.name); + + auto const actualScale = ValidVault::computeCoarsestScale(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.delta; + firstRounded = roundToAsset(vaultAsset, num.delta, actualScale); + continue; + } + auto const numRounded = roundToAsset(vaultAsset, num.delta, actualScale); + BEAST_EXPECTS( + numRounded != firstRounded, + "at a scale of " + std::to_string(actualScale) + " " + + to_string(num.delta) + " == " + to_string(*first)); + } } } } diff --git a/src/test/basics/IOUAmount_test.cpp b/src/test/basics/IOUAmount_test.cpp index ef053449a5..b652d27625 100644 --- a/src/test/basics/IOUAmount_test.cpp +++ b/src/test/basics/IOUAmount_test.cpp @@ -156,8 +156,7 @@ public: BEAST_EXPECTS(result == expected, ss.str()); }; - for (auto const mantissaSize : - {MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::Large}) + for (auto const mantissaSize : MantissaRange::getAllScales()) { NumberMantissaScaleGuard const mg(mantissaSize); diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 2a4e176ae5..26060c70e9 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -6,7 +6,10 @@ #include #include +#include + #include +#include #include #include #include @@ -19,6 +22,24 @@ namespace xrpl { class Number_test : public beast::unit_test::Suite { + using BigInt = boost::multiprecision::cpp_int; + + static std::string + fmt(BigInt const& value) + { + auto s = to_string(value); + std::string out; + int count = 0; + for (auto it = s.rbegin(); it != s.rend(); ++it) + { + if (count != 0 && count % 3 == 0 && (isdigit(*it) != 0)) + out.insert(out.begin(), '_'); + out.insert(out.begin(), *it); + ++count; + } + return out; + } + public: void testZero() @@ -178,7 +199,6 @@ public: {Number{true, 9'999'999'999'999'999'999ULL, -37, Number::Normalized{}}, Number{1'000'000'000'000'000'000, -18}, Number{false, 9'999'999'999'999'999'990ULL, -19, Number::Normalized{}}}, - {Number{Number::kMaxRep}, Number{6, -1}, Number{Number::kMaxRep / 10, 1}}, {Number{Number::kMaxRep - 1}, Number{1, 0}, Number{Number::kMaxRep}}, // Test extremes { @@ -189,16 +209,22 @@ public: Number{2, 19}, }, { - // Does not round. Mantissas are going to be > maxRep, so if + // Does not round. Mantissas are going to be > kMaxRep, 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. + // kMaxRep. Number{false, 9'999'999'999'999'999'990ULL, 0, Number::Normalized{}}, Number{false, 9'999'999'999'999'999'990ULL, 0, Number::Normalized{}}, Number{false, 1'999'999'999'999'999'998ULL, 1, Number::Normalized{}}, }, }); + auto const cLargeLegacy = std::to_array({ + {Number{Number::kMaxRep}, Number{6, -1}, Number{Number::kMaxRep / 10, 1}}, + }); + auto const cLargeCorrected = std::to_array({ + {Number{Number::kMaxRep}, Number{6, -1}, Number{(Number::kMaxRep / 10) + 1, 1}}, + }); auto test = [this](auto const& c) { for (auto const& [x, y, z] : c) { @@ -215,6 +241,14 @@ public: else { test(cLarge); + if (scale == MantissaRange::MantissaScale::LargeLegacy) + { + test(cLargeLegacy); + } + else + { + test(cLargeCorrected); + } } { bool caught = false; @@ -835,7 +869,7 @@ public: /* auto tests = [&](auto const& cSmall, auto const& cLarge) { test(cSmall); - if (scale != MantissaRange::mantissa_scale::small) + if (scale != MantissaRange::MantissaScale::Small) test(cLarge); }; */ @@ -1266,6 +1300,7 @@ public: "9223372036854775e3"); } break; + case MantissaRange::MantissaScale::LargeLegacy: case MantissaRange::MantissaScale::Large: // Test the edges // ((exponent < -(28)) || (exponent > -(8))))) @@ -1551,11 +1586,48 @@ public: } } + void + testUpwardRoundsDown() + { + testcase << "upward rounding produces a value below exact at kMaxRep cusp"; + + NumberMantissaScaleGuard const mg{MantissaRange::MantissaScale::Large}; + NumberRoundModeGuard const rg{Number::RoundingMode::Upward}; + + constexpr std::int64_t kAValue = 1'000'000'000'000'049'863LL; + constexpr std::int64_t kBValue = 9'223'372'036'854'315'903LL; + + Number const a = kAValue; + Number const b = kBValue; + Number const product = a * b; + + // Exact reference in BigInt. + BigInt const exactProduct = BigInt(kAValue) * BigInt(kBValue); + + // What Number actually stored. + BigInt storedValue = BigInt(product.mantissa()); + for (int i = 0; i < product.exponent(); ++i) + storedValue *= 10; + + BigInt const signedDifference = storedValue - exactProduct; + + log << "\n" + << " a = " << fmt(BigInt(kAValue)) << "\n" + << " b = " << fmt(BigInt(kBValue)) << "\n" + << " exact a*b = " << fmt(exactProduct) << "\n" + << " stored = " << fmt(storedValue) << "\n" + << " stored - exact = " << fmt(signedDifference) << "\n" + << " upward = " << (signedDifference >= 0 ? "held" : "VIOLATED") << "\n"; + + BEAST_EXPECT(signedDifference >= 0); + BEAST_EXPECT(product.mantissa() == (std::numeric_limits::max() / 10) + 1); + BEAST_EXPECT(product.exponent() == 19); + } + void run() override { - for (auto const scale : - {MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::Large}) + for (auto const scale : MantissaRange::getAllScales()) { NumberMantissaScaleGuard const sg(scale); testZero(); @@ -1580,6 +1652,8 @@ public: testRounding(); testInt64(); } + // This test sets its own number range + testUpwardRoundsDown(); } }; diff --git a/src/test/protocol/STNumber_test.cpp b/src/test/protocol/STNumber_test.cpp index 4d95bc1ef7..5c9c3fd83c 100644 --- a/src/test/protocol/STNumber_test.cpp +++ b/src/test/protocol/STNumber_test.cpp @@ -280,8 +280,7 @@ struct STNumber_test : public beast::unit_test::Suite { static_assert(!std::is_convertible_v); - for (auto const scale : - {MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::Large}) + for (auto const scale : MantissaRange::getAllScales()) { NumberMantissaScaleGuard const sg(scale); testcase << to_string(Number::getMantissaScale()); diff --git a/src/test/rpc/GetAggregatePrice_test.cpp b/src/test/rpc/GetAggregatePrice_test.cpp index 1de08da205..37ecc54172 100644 --- a/src/test/rpc/GetAggregatePrice_test.cpp +++ b/src/test/rpc/GetAggregatePrice_test.cpp @@ -177,8 +177,7 @@ public: auto const all = testableAmendments(); for (auto const& feats : {all - featureSingleAssetVault - featureLendingProtocol, all}) { - for (auto const mantissaSize : - {MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::Large}) + for (auto const mantissaSize : MantissaRange::getAllScales()) { // Regardless of the features enabled, RPC is controlled by // the global mantissa size. And since it's a thread-local, From 1438bf1c6764056216bfeab5027e0eccc12fd870 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Wed, 27 May 2026 18:20:57 +0100 Subject: [PATCH 2/6] release: Bump version to 3.2.0-rc1 (#7335) --- src/libxrpl/protocol/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index 5613a9366e..4e95683308 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/src/libxrpl/protocol/BuildInfo.cpp @@ -23,7 +23,7 @@ namespace { //------------------------------------------------------------------------------ // clang-format off // NOLINTNEXTLINE(readability-identifier-naming) -char const* const versionString = "3.2.0-b7" +char const* const versionString = "3.2.0-rc1" // clang-format on ; From 396d772a15a634d4a50e3f8e22e7378d1eed914d Mon Sep 17 00:00:00 2001 From: Bart Date: Wed, 27 May 2026 15:10:33 -0400 Subject: [PATCH 3/6] refactor: Enable support for `fixCleanup3_2_0` amendment (#7347) --- include/xrpl/protocol/detail/features.macro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index fd62b74d59..c99c1e5ce8 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,7 +15,7 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. -XRPL_FIX (Cleanup3_2_0, Supported::No, VoteBehavior::DefaultNo) +XRPL_FIX (Cleanup3_2_0, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_1_3, Supported::Yes, VoteBehavior::DefaultYes) XRPL_FIX (BatchInnerSigs, Supported::No, VoteBehavior::DefaultNo) From 1acc42313c01e7e114333390b4485e3e176ba66f Mon Sep 17 00:00:00 2001 From: Bart Date: Wed, 27 May 2026 15:11:38 -0400 Subject: [PATCH 4/6] release: Bump version to 3.2.0-rc2 (#7348) --- src/libxrpl/protocol/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index 4e95683308..2e33d03088 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/src/libxrpl/protocol/BuildInfo.cpp @@ -23,7 +23,7 @@ namespace { //------------------------------------------------------------------------------ // clang-format off // NOLINTNEXTLINE(readability-identifier-naming) -char const* const versionString = "3.2.0-rc1" +char const* const versionString = "3.2.0-rc2" // clang-format on ; From f9551ac5ca08bb851b2a17d2f5259de13c105876 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Wed, 27 May 2026 20:24:18 +0100 Subject: [PATCH 5/6] style: Run shfmt on workflows, actions and markdown bash code (#7333) --- .github/actions/build-deps/action.yml | 18 +- .github/actions/generate-version/action.yml | 10 +- .github/scripts/format-inline-bash.py | 403 ++++++++++++++++++ .github/workflows/build-nix-image.yml | 4 +- .github/workflows/check-pr-description.yml | 10 +- .github/workflows/check-pr-title.yml | 2 +- .github/workflows/on-pr.yml | 8 +- .github/workflows/pre-commit.yml | 2 +- .../workflows/reusable-build-docker-image.yml | 2 +- .../workflows/reusable-build-test-config.yml | 76 ++-- .../workflows/reusable-check-levelization.yml | 10 +- .github/workflows/reusable-check-rename.yml | 10 +- .github/workflows/reusable-clang-tidy.yml | 48 +-- .github/workflows/reusable-package.yml | 2 +- .../workflows/reusable-strategy-matrix.yml | 2 +- .pre-commit-config.yaml | 13 + BUILD.md | 6 +- cspell.config.yaml | 1 + include/xrpl/protocol_autogen/README.md | 4 +- package/README.md | 18 +- 20 files changed, 533 insertions(+), 116 deletions(-) create mode 100755 .github/scripts/format-inline-bash.py diff --git a/.github/actions/build-deps/action.yml b/.github/actions/build-deps/action.yml index 9d52be1998..0891d56dfa 100644 --- a/.github/actions/build-deps/action.yml +++ b/.github/actions/build-deps/action.yml @@ -37,12 +37,12 @@ runs: run: | echo 'Installing dependencies.' conan install \ - --profile ci \ - --build="${BUILD_OPTION}" \ - --options:host='&:tests=True' \ - --options:host='&:xrpld=True' \ - --settings:all build_type="${BUILD_TYPE}" \ - --conf:all tools.build:jobs=${BUILD_NPROC} \ - --conf:all tools.build:verbosity="${LOG_VERBOSITY}" \ - --conf:all tools.compilation:verbosity="${LOG_VERBOSITY}" \ - . + --profile ci \ + --build="${BUILD_OPTION}" \ + --options:host='&:tests=True' \ + --options:host='&:xrpld=True' \ + --settings:all build_type="${BUILD_TYPE}" \ + --conf:all tools.build:jobs=${BUILD_NPROC} \ + --conf:all tools.build:verbosity="${LOG_VERBOSITY}" \ + --conf:all tools.compilation:verbosity="${LOG_VERBOSITY}" \ + . diff --git a/.github/actions/generate-version/action.yml b/.github/actions/generate-version/action.yml index 8edb7920c6..50b3166596 100644 --- a/.github/actions/generate-version/action.yml +++ b/.github/actions/generate-version/action.yml @@ -15,7 +15,7 @@ runs: shell: bash env: VERSION: ${{ github.ref_name }} - run: echo "VERSION=${VERSION}" >> "${GITHUB_ENV}" + run: echo "VERSION=${VERSION}" >>"${GITHUB_ENV}" # When a tag is not pushed, then the version (e.g. 1.2.3-b0) is extracted # from the BuildInfo.cpp file and the shortened commit hash appended to it. @@ -28,17 +28,17 @@ runs: echo 'Extracting version from BuildInfo.cpp.' VERSION="$(cat src/libxrpl/protocol/BuildInfo.cpp | grep "versionString =" | awk -F '"' '{print $2}')" if [[ -z "${VERSION}" ]]; then - echo 'Unable to extract version from BuildInfo.cpp.' - exit 1 + echo 'Unable to extract version from BuildInfo.cpp.' + exit 1 fi echo 'Appending shortened commit hash to version.' SHA='${{ github.sha }}' VERSION="${VERSION}+${SHA:0:7}" - echo "VERSION=${VERSION}" >> "${GITHUB_ENV}" + echo "VERSION=${VERSION}" >>"${GITHUB_ENV}" - name: Output version id: version shell: bash - run: echo "version=${VERSION}" >> "${GITHUB_OUTPUT}" + run: echo "version=${VERSION}" >>"${GITHUB_OUTPUT}" diff --git a/.github/scripts/format-inline-bash.py b/.github/scripts/format-inline-bash.py new file mode 100755 index 0000000000..423c78109c --- /dev/null +++ b/.github/scripts/format-inline-bash.py @@ -0,0 +1,403 @@ +#!/usr/bin/env python3 + +""" +Format embedded shell snippets using the shfmt hook configured in +.pre-commit-config.yaml. + +Two shapes are recognised: + +* YAML workflow/action files: literal block-scalar runs (`run: |`) and + single-line runs (`run: some command`). A single-line run is upgraded to + a `run: |` block scalar if shfmt's output spans multiple lines. + +* Markdown files: ``` ```bash ``` fenced code blocks. + +Any block that shfmt cannot parse is skipped with a warning on stderr, so +the file is left untouched and surrounding blocks still get formatted. + +For each occurrence the body is dedented, written to a temp .sh file, +formatted via `pre-commit run shfmt --files ` (falling back to +`prek`), then re-indented and written back in place. + +When invoked without arguments, every .yml/.yaml under .github/ plus every +.md file in the repo is scanned. When invoked with file arguments (the +pre-commit case), only those files are processed. +""" + +from __future__ import annotations + +import re +import shutil +import subprocess +import sys +import tempfile +from dataclasses import dataclass +from pathlib import Path +from typing import Union + +REPO = Path(__file__).resolve().parents[2] + +_HOOK_RUNNER = next((cmd for cmd in ("pre-commit", "prek") if shutil.which(cmd)), None) +if _HOOK_RUNNER is None: + sys.exit("error: neither `pre-commit` nor `prek` found on PATH") + +RUN_BLOCK_RE = re.compile(r"^(?P[ \t]*(?:- )?)run:[ \t]*\|[+-]?[ \t]*$") +RUN_INLINE_RE = re.compile( + r"^(?P[ \t]*(?:- )?)run:[ \t]+" r"(?P(?!\|[+-]?[ \t]*$)\S.*?)[ \t]*$" +) +MD_BASH_OPEN_RE = re.compile(r"^(?P[ ]{0,3})`{3}bash[ \t]*$") +MD_FENCE_CLOSE_RE = re.compile(r"^[ ]{0,3}`{3,}[ \t]*$") + + +@dataclass(frozen=True) +class BlockRun: + """A `run: |` block scalar; `body_start:body_end` slices into `lines`.""" + + body_start: int + body_end: int + body_indent: int + + +@dataclass(frozen=True) +class InlineRun: + """A single-line `run: value` at `line_idx`.""" + + line_idx: int + prefix: str + value: str + + +@dataclass(frozen=True) +class MdBashBlock: + """A markdown ``` ```bash ``` fenced code block. + + `body_start:body_end` slices into the file's lines; `open_line_idx` + points at the opening fence line. + """ + + open_line_idx: int + body_start: int + body_end: int + body_indent: int + + +RunItem = Union[BlockRun, InlineRun] + + +def _scan_block_body( + lines: list[str], body_start: int, run_col: int +) -> tuple[int | None, int]: + """Locate the body of a `run: |` block scalar starting at `body_start`. + + Returns `(body_indent, scan_end)`. `scan_end` is the line index where the + outer scanner should resume. `body_indent` is `None` when no body is + present (the scalar is empty, or the next non-blank line has indent + `<= run_col`). + """ + body_indent: int | None = None + scan_end = len(lines) + for idx in range(body_start, len(lines)): + line = lines[idx] + if line.strip() == "": + continue + indent = len(line) - len(line.lstrip(" ")) + if body_indent is None: + if indent > run_col: + body_indent = indent + else: + scan_end = idx + break + elif indent < body_indent: + scan_end = idx + break + if body_indent is not None: + while scan_end > body_start and lines[scan_end - 1].strip() == "": + scan_end -= 1 + if scan_end <= body_start: + body_indent = None + return body_indent, scan_end + + +def find_run_blocks(lines: list[str]) -> list[RunItem]: + """Return run items in document order.""" + items: list[RunItem] = [] + line_idx = 0 + while line_idx < len(lines): + line = lines[line_idx] + if block_match := RUN_BLOCK_RE.match(line): + run_col = len(block_match.group("prefix")) + body_start = line_idx + 1 + body_indent, scan_end = _scan_block_body(lines, body_start, run_col) + if body_indent is not None: + items.append( + BlockRun( + body_start=body_start, + body_end=scan_end, + body_indent=body_indent, + ) + ) + line_idx = scan_end + continue + if inline_match := RUN_INLINE_RE.match(line): + items.append( + InlineRun( + line_idx=line_idx, + prefix=inline_match.group("prefix"), + value=inline_match.group("value"), + ) + ) + line_idx += 1 + return items + + +def find_md_bash_blocks(lines: list[str]) -> list[MdBashBlock]: + """Return ``` ```bash ``` fenced code blocks in document order.""" + blocks: list[MdBashBlock] = [] + line_idx = 0 + while line_idx < len(lines): + open_match = MD_BASH_OPEN_RE.match(lines[line_idx]) + if not open_match: + line_idx += 1 + continue + body_start = line_idx + 1 + close_idx = next( + ( + j + for j in range(body_start, len(lines)) + if MD_FENCE_CLOSE_RE.match(lines[j]) + ), + None, + ) + if close_idx is None: + line_idx = body_start + continue + body = lines[body_start:close_idx] + non_blank = [b for b in body if b.strip()] + body_indent = ( + min(len(b) - len(b.lstrip(" ")) for b in non_blank) + if non_blank + else len(open_match.group("indent")) + ) + blocks.append( + MdBashBlock( + open_line_idx=line_idx, + body_start=body_start, + body_end=close_idx, + body_indent=body_indent, + ) + ) + line_idx = close_idx + 1 + return blocks + + +def dedent(lines: list[str], n: int) -> list[str]: + pad = " " * n + return [ + ( + "" + if line.strip() == "" + else (line[n:] if line.startswith(pad) else line.lstrip(" ")) + ) + for line in lines + ] + + +def reindent(lines: list[str], n: int) -> list[str]: + pad = " " * n + return [pad + line if line else "" for line in lines] + + +_SHFMT_ERR_RE = re.compile(r"\.sh:\d+:\d+:\s") +_GHA_EXPR_RE = re.compile(r"\$\{\{.*?\}\}", re.DOTALL) +_GHA_PLACEHOLDER_RE = re.compile(r"__GHA_EXPR_(\d+)__") + + +def _encode_gha_exprs(text: str) -> tuple[str, list[str]]: + """Replace `${{ ... }}` expressions with bash-safe placeholder identifiers.""" + exprs: list[str] = [] + + def repl(match: re.Match[str]) -> str: + exprs.append(match.group(0)) + return f"__GHA_EXPR_{len(exprs) - 1}__" + + return _GHA_EXPR_RE.sub(repl, text), exprs + + +def _decode_gha_exprs(text: str, exprs: list[str]) -> str: + """Restore `${{ ... }}` expressions from placeholder identifiers.""" + return _GHA_PLACEHOLDER_RE.sub(lambda m: exprs[int(m.group(1))], text) + + +def shfmt_via_hook(tmp_path: Path) -> tuple[bool, str]: + # `${{ ... }}` is not valid shell, so swap it for a placeholder identifier + # that shfmt can parse, then restore it after formatting. + encoded, exprs = _encode_gha_exprs(tmp_path.read_text()) + if exprs: + tmp_path.write_text(encoded) + res = subprocess.run( + [_HOOK_RUNNER, "run", "shfmt", "--files", str(tmp_path)], + cwd=REPO, + capture_output=True, + text=True, + ) + output = res.stdout + res.stderr + # shfmt emits parse errors as "::: ". + parse_err = bool(_SHFMT_ERR_RE.search(output)) + # A non-zero exit that is neither a parse error nor pre-commit's "I had + # to modify files" signal means the hook itself failed to run (missing + # binary, install failure, bad config, ...). Surface that loudly rather + # than silently treating it as a no-op. + if ( + res.returncode != 0 + and not parse_err + and "files were modified by this hook" not in output + ): + sys.exit( + f"error: `{_HOOK_RUNNER} run shfmt` failed with exit {res.returncode}:\n{output}" + ) + if exprs and not parse_err: + tmp_path.write_text(_decode_gha_exprs(tmp_path.read_text(), exprs)) + return not parse_err, output + + +def _skip(path: Path, where: int, kind: str, output: str) -> None: + print( + f" shfmt could not parse {kind} at {path}:{where + 1} — skipped", + file=sys.stderr, + ) + print(f" {output.strip()}", file=sys.stderr) + + +def process_yaml_file(path: Path, tmp_path: Path) -> int: + text = path.read_text() + had_nl = text.endswith("\n") + lines = text.split("\n") + if had_nl: + lines = lines[:-1] + items = find_run_blocks(lines) + if not items: + return 0 + changed = 0 + # Process in reverse so earlier indices remain valid as we splice. + for item in reversed(items): + if isinstance(item, BlockRun): + body = lines[item.body_start : item.body_end] + tmp_path.write_text("\n".join(dedent(body, item.body_indent)) + "\n") + ok, output = shfmt_via_hook(tmp_path) + if not ok: + _skip(path, item.body_start, "block", output) + continue + formatted = tmp_path.read_text().rstrip("\n") + new_body = reindent(formatted.split("\n"), item.body_indent) + if new_body != body: + lines[item.body_start : item.body_end] = new_body + changed += 1 + else: + tmp_path.write_text(item.value + "\n") + ok, output = shfmt_via_hook(tmp_path) + if not ok: + _skip(path, item.line_idx, "inline run", output) + continue + formatted = tmp_path.read_text().rstrip("\n") + if formatted == item.value: + continue + formatted_lines = formatted.split("\n") + if len(formatted_lines) == 1: + lines[item.line_idx] = f"{item.prefix}run: {formatted}" + else: + body_indent = len(item.prefix) + 2 + lines[item.line_idx : item.line_idx + 1] = [ + f"{item.prefix}run: |", + *reindent(formatted_lines, body_indent), + ] + changed += 1 + new_text = "\n".join(lines) + ("\n" if had_nl else "") + if new_text != text: + path.write_text(new_text) + return changed + + +def process_md_file(path: Path, tmp_path: Path) -> int: + text = path.read_text() + had_nl = text.endswith("\n") + lines = text.split("\n") + if had_nl: + lines = lines[:-1] + blocks = find_md_bash_blocks(lines) + if not blocks: + return 0 + changed = 0 + for block in reversed(blocks): + body = lines[block.body_start : block.body_end] + tmp_path.write_text("\n".join(dedent(body, block.body_indent)) + "\n") + ok, output = shfmt_via_hook(tmp_path) + if not ok: + _skip(path, block.open_line_idx, "```bash block", output) + continue + formatted = tmp_path.read_text().rstrip("\n") + formatted_lines = formatted.split("\n") if formatted else [] + new_body = reindent(formatted_lines, block.body_indent) + if new_body != body: + lines[block.body_start : block.body_end] = new_body + changed += 1 + new_text = "\n".join(lines) + ("\n" if had_nl else "") + if new_text != text: + path.write_text(new_text) + return changed + + +def process_file(path: Path, tmp_path: Path) -> int: + if path.suffix in (".yml", ".yaml"): + return process_yaml_file(path, tmp_path) + if path.suffix == ".md": + return process_md_file(path, tmp_path) + return 0 + + +def gather_files(argv: list[str]) -> list[Path]: + """Return YAML workflow/action files and markdown files that we should + process — either the paths in `argv` or, when `argv` is empty, every + such file in the repo (skipping `external/`).""" + if argv: + candidates: list[Path] = [ + (REPO / a).resolve() if not Path(a).is_absolute() else Path(a) for a in argv + ] + else: + gh = REPO / ".github" + candidates = [ + *gh.rglob("*.yml"), + *gh.rglob("*.yaml"), + *( + p + for p in REPO.rglob("*.md") + if "external" not in p.relative_to(REPO).parts + ), + ] + return sorted( + p + for p in candidates + if p.exists() + and ( + (p.suffix in (".yml", ".yaml") and ".github" in p.parts) + or p.suffix == ".md" + ) + ) + + +def main(argv: list[str]) -> int: + files = gather_files(argv) + if not files: + return 0 + with tempfile.TemporaryDirectory(prefix="format-inline-bash-") as tmpdir: + tmp_path = Path(tmpdir) / "shfmt.sh" + total = 0 + for f in files: + n = process_file(f, tmp_path) + if n: + print(f"{f.relative_to(REPO)}: reformatted {n} block(s)") + total += n + return 1 if total else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/.github/workflows/build-nix-image.yml b/.github/workflows/build-nix-image.yml index edd35132fa..bae4cfd437 100644 --- a/.github/workflows/build-nix-image.yml +++ b/.github/workflows/build-nix-image.yml @@ -100,8 +100,8 @@ jobs: - name: Create multi-arch manifests run: | - for tag in $(jq -cr '.tags[]' <<< "$DOCKER_METADATA_OUTPUT_JSON"); do - docker buildx imagetools create -t "$tag" "${tag}-amd64" "${tag}-arm64" + for tag in $(jq -cr '.tags[]' <<<"$DOCKER_METADATA_OUTPUT_JSON"); do + docker buildx imagetools create -t "$tag" "${tag}-amd64" "${tag}-arm64" done - name: Inspect image diff --git a/.github/workflows/check-pr-description.yml b/.github/workflows/check-pr-description.yml index f6eee50291..3f345a9dcb 100644 --- a/.github/workflows/check-pr-description.yml +++ b/.github/workflows/check-pr-description.yml @@ -20,11 +20,11 @@ jobs: env: PR_BODY: ${{ github.event.pull_request.body }} if: ${{ github.event_name == 'pull_request' }} - run: printenv PR_BODY > pr_body.md + run: printenv PR_BODY >pr_body.md - name: Check PR description differs from template if: ${{ github.event_name == 'pull_request' }} - run: > - python .github/scripts/check-pr-description.py - --template-file .github/pull_request_template.md - --pr-body-file pr_body.md + run: | + python .github/scripts/check-pr-description.py \ + --template-file .github/pull_request_template.md \ + --pr-body-file pr_body.md diff --git a/.github/workflows/check-pr-title.yml b/.github/workflows/check-pr-title.yml index 5631950df6..79962e0620 100644 --- a/.github/workflows/check-pr-title.yml +++ b/.github/workflows/check-pr-title.yml @@ -11,4 +11,4 @@ on: jobs: check_title: if: ${{ github.event.pull_request.draft != true }} - uses: XRPLF/actions/.github/workflows/check-pr-title.yml@291206777251b4d493641b5afbdf7c23009d2988 + uses: XRPLF/actions/.github/workflows/check-pr-title.yml@cba1f0891650baf1a9c88624dc2d72573be2eb81 diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index ca715e0376..db3c8667e5 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -98,7 +98,7 @@ jobs: READY: ${{ contains(github.event.pull_request.labels.*.name, 'Ready to merge') }} MERGE: ${{ github.event_name == 'merge_group' }} run: | - echo "go=${{ (env.DRAFT != 'true' && env.READY == 'true') || env.FILES == 'true' || env.MERGE == 'true' }}" >> "${GITHUB_OUTPUT}" + echo "go=${{ (env.DRAFT != 'true' && env.READY == 'true') || env.FILES == 'true' || env.MERGE == 'true' }}" >>"${GITHUB_OUTPUT}" cat "${GITHUB_OUTPUT}" outputs: go: ${{ steps.go.outputs.go == 'true' }} @@ -168,9 +168,9 @@ jobs: PR_URL: ${{ github.event.pull_request.html_url }} run: | gh api --method POST -H "Accept: application/vnd.github+json" -H "X-GitHub-Api-Version: 2022-11-28" \ - /repos/xrplf/clio/dispatches -f "event_type=check_libxrpl" \ - -F "client_payload[ref]=${{ needs.upload-recipe.outputs.recipe_ref }}" \ - -F "client_payload[pr_url]=${PR_URL}" + /repos/xrplf/clio/dispatches -f "event_type=check_libxrpl" \ + -F "client_payload[ref]=${{ needs.upload-recipe.outputs.recipe_ref }}" \ + -F "client_payload[pr_url]=${PR_URL}" passed: if: failure() || cancelled() diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 2f15b82266..de6a4f40b4 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -14,7 +14,7 @@ on: jobs: # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks. run-hooks: - uses: XRPLF/actions/.github/workflows/pre-commit.yml@5e942d61bf32f7557a7c159cfac4712a687b3e3a + uses: XRPLF/actions/.github/workflows/pre-commit.yml@cba1f0891650baf1a9c88624dc2d72573be2eb81 with: runs_on: ubuntu-latest container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-41ec7c1" }' diff --git a/.github/workflows/reusable-build-docker-image.yml b/.github/workflows/reusable-build-docker-image.yml index 5b555b713f..c3795e56fa 100644 --- a/.github/workflows/reusable-build-docker-image.yml +++ b/.github/workflows/reusable-build-docker-image.yml @@ -53,7 +53,7 @@ jobs: env: PLATFORM: ${{ inputs.platform }} run: | - echo "arch=${PLATFORM##*/}" >> $GITHUB_OUTPUT + echo "arch=${PLATFORM##*/}" >>$GITHUB_OUTPUT - name: Set up Docker Buildx uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0 diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 4f9b926e98..31457bb892 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -113,7 +113,7 @@ jobs: - name: Set ccache log file if: ${{ inputs.ccache_enabled && runner.debug == '1' }} - run: echo "CCACHE_LOGFILE=${{ runner.temp }}/ccache.log" >> "${GITHUB_ENV}" + run: echo "CCACHE_LOGFILE=${{ runner.temp }}/ccache.log" >>"${GITHUB_ENV}" - name: Print build environment uses: XRPLF/actions/print-build-env@59dec886e4afb05a1724443af08baccbc045b574 @@ -146,11 +146,11 @@ jobs: CMAKE_ARGS: ${{ inputs.cmake_args }} run: | cmake \ - -G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \ - -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ - -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \ - ${CMAKE_ARGS} \ - .. + -G '${{ runner.os == 'Windows' && 'Visual Studio 17 2022' || 'Ninja' }}' \ + -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ + -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \ + ${CMAKE_ARGS} \ + .. - name: Check protocol autogen files are up-to-date working-directory: ${{ env.BUILD_DIR }} @@ -172,10 +172,10 @@ jobs: cmake --build . --target code_gen DIFF=$(git -C .. status --porcelain -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen) if [ -n "${DIFF}" ]; then - echo "::error::Generated protocol files are out of date" - git -C .. diff -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen - echo "${MESSAGE}" - exit 1 + echo "::error::Generated protocol files are out of date" + git -C .. diff -- include/xrpl/protocol_autogen src/tests/libxrpl/protocol_autogen + echo "${MESSAGE}" + exit 1 fi - name: Build the binary @@ -186,18 +186,18 @@ jobs: CMAKE_TARGET: ${{ inputs.cmake_target }} run: | cmake \ - --build . \ - --config "${BUILD_TYPE}" \ - --parallel "${BUILD_NPROC}" \ - --target "${CMAKE_TARGET}" + --build . \ + --config "${BUILD_TYPE}" \ + --parallel "${BUILD_NPROC}" \ + --target "${CMAKE_TARGET}" - name: Show ccache statistics if: ${{ inputs.ccache_enabled }} run: | ccache --show-stats -vv if [ '${{ runner.debug }}' = '1' ]; then - cat "${CCACHE_LOGFILE}" - curl ${CCACHE_REMOTE_STORAGE%|*}/status || true + cat "${CCACHE_LOGFILE}" + curl ${CCACHE_REMOTE_STORAGE%|*}/status || true fi - name: Upload the binary (Linux) @@ -214,7 +214,7 @@ jobs: working-directory: ${{ env.BUILD_DIR }} run: | set -o pipefail - ./xrpld --definitions | python3 -m json.tool > server_definitions.json + ./xrpld --definitions | python3 -m json.tool >server_definitions.json - name: Upload server definitions if: ${{ github.event.repository.visibility == 'public' && inputs.config_name == 'debian-bookworm-gcc-13-amd64-release' }} @@ -231,10 +231,10 @@ jobs: run: | ldd ./xrpld if [ "$(ldd ./xrpld | grep -E '(libstdc\+\+|libgcc)' | wc -l)" -eq 0 ]; then - echo 'The binary is statically linked.' + echo 'The binary is statically linked.' else - echo 'The binary is dynamically linked.' - exit 1 + echo 'The binary is dynamically linked.' + exit 1 fi - name: Verify presence of instrumentation (Linux) @@ -250,12 +250,12 @@ jobs: run: | ASAN_OPTS="include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-asan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp" if [[ "${CONFIG_NAME}" == *gcc* ]]; then - ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0" + ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0" fi - echo "ASAN_OPTIONS=${ASAN_OPTS}" >> ${GITHUB_ENV} - echo "TSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-tsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >> ${GITHUB_ENV} - echo "UBSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-ubsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV} - echo "LSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-lsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${GITHUB_ENV} + echo "ASAN_OPTIONS=${ASAN_OPTS}" >>${GITHUB_ENV} + echo "TSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-tsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >>${GITHUB_ENV} + echo "UBSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-ubsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >>${GITHUB_ENV} + echo "LSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-lsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >>${GITHUB_ENV} - name: Run the separate tests if: ${{ !inputs.build_only }} @@ -266,9 +266,9 @@ jobs: PARALLELISM: ${{ runner.os == 'Windows' && '1' || steps.nproc.outputs.nproc }} run: | ctest \ - --output-on-failure \ - -C "${BUILD_TYPE}" \ - -j "${PARALLELISM}" + --output-on-failure \ + -C "${BUILD_TYPE}" \ + -j "${PARALLELISM}" - name: Run the embedded tests if: ${{ !inputs.build_only }} @@ -278,7 +278,7 @@ jobs: run: | set -o pipefail # Coverage builds are slower due to instrumentation; use fewer parallel jobs to avoid flakiness - [ "$COVERAGE_ENABLED" = "true" ] && BUILD_NPROC=$(( BUILD_NPROC - 2 )) + [ "$COVERAGE_ENABLED" = "true" ] && BUILD_NPROC=$((BUILD_NPROC - 2)) ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log - name: Show test failure summary @@ -287,19 +287,19 @@ jobs: WORKING_DIR: ${{ runner.os == 'Windows' && format('{0}\{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }} run: | if [ ! -d "${WORKING_DIR}" ]; then - echo "Working directory '${WORKING_DIR}' does not exist." - exit 0 + echo "Working directory '${WORKING_DIR}' does not exist." + exit 0 fi cd "${WORKING_DIR}" if [ ! -f unittest.log ]; then - echo "unittest.log not found; embedded tests may not have run." - exit 0 + echo "unittest.log not found; embedded tests may not have run." + exit 0 fi if ! grep -E "failed" unittest.log; then - echo "Log present but no failure lines found in unittest.log." + echo "Log present but no failure lines found in unittest.log." fi - name: Debug failure (Linux) if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }} @@ -317,10 +317,10 @@ jobs: BUILD_TYPE: ${{ inputs.build_type }} run: | cmake \ - --build . \ - --config "${BUILD_TYPE}" \ - --parallel "${BUILD_NPROC}" \ - --target coverage + --build . \ + --config "${BUILD_TYPE}" \ + --parallel "${BUILD_NPROC}" \ + --target coverage - name: Upload coverage report if: ${{ github.repository == 'XRPLF/rippled' && !inputs.build_only && env.COVERAGE_ENABLED == 'true' }} diff --git a/.github/workflows/reusable-check-levelization.yml b/.github/workflows/reusable-check-levelization.yml index 4efe3e1138..b5d57a177a 100644 --- a/.github/workflows/reusable-check-levelization.yml +++ b/.github/workflows/reusable-check-levelization.yml @@ -38,9 +38,9 @@ jobs: run: | DIFF=$(git status --porcelain) if [ -n "${DIFF}" ]; then - # Print the differences to give the contributor a hint about what to - # expect when running levelization on their own machine. - git diff - echo "${MESSAGE}" - exit 1 + # Print the differences to give the contributor a hint about what to + # expect when running levelization on their own machine. + git diff + echo "${MESSAGE}" + exit 1 fi diff --git a/.github/workflows/reusable-check-rename.yml b/.github/workflows/reusable-check-rename.yml index 56a1a3e637..7aa5b80594 100644 --- a/.github/workflows/reusable-check-rename.yml +++ b/.github/workflows/reusable-check-rename.yml @@ -48,9 +48,9 @@ jobs: run: | DIFF=$(git status --porcelain) if [ -n "${DIFF}" ]; then - # Print the differences to give the contributor a hint about what to - # expect when running the renaming scripts on their own machine. - git diff - echo "${MESSAGE}" - exit 1 + # Print the differences to give the contributor a hint about what to + # expect when running the renaming scripts on their own machine. + git diff + echo "${MESSAGE}" + exit 1 fi diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index e01a50cf6d..8be1db5fb2 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -70,13 +70,13 @@ jobs: working-directory: ${{ env.BUILD_DIR }} run: | cmake \ - -G 'Ninja' \ - -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ - -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \ - -Dtests=ON \ - -Dwerr=ON \ - -Dxrpld=ON \ - .. + -G 'Ninja' \ + -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ + -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" \ + -Dtests=ON \ + -Dwerr=ON \ + -Dxrpld=ON \ + .. # clang-tidy needs headers generated from proto files - name: Build libxrpl.libpb @@ -133,7 +133,7 @@ jobs: - name: Write issue header if: ${{ steps.run_clang_tidy.outcome != 'success' }} run: | - cat > "${ISSUE_FILE}" <"${ISSUE_FILE}" < filtered-output.txt || true + # Extract lines containing 'error:', 'warning:', or 'note:' + grep -E '(error:|warning:|note:)' "${OUTPUT_FILE}" >filtered-output.txt || true - # If filtered output is empty, use original (might be a different error format) - if [ ! -s filtered-output.txt ]; then - cp "${OUTPUT_FILE}" filtered-output.txt - fi + # If filtered output is empty, use original (might be a different error format) + if [ ! -s filtered-output.txt ]; then + cp "${OUTPUT_FILE}" filtered-output.txt + fi - # Truncate if too large - head -c 60000 filtered-output.txt >> "${ISSUE_FILE}" - if [ "$(wc -c < filtered-output.txt)" -gt 60000 ]; then - echo "" >> "${ISSUE_FILE}" - echo "... (output truncated, see artifacts for full output)" >> "${ISSUE_FILE}" - fi + # Truncate if too large + head -c 60000 filtered-output.txt >>"${ISSUE_FILE}" + if [ "$(wc -c >"${ISSUE_FILE}" + echo "... (output truncated, see artifacts for full output)" >>"${ISSUE_FILE}" + fi - rm filtered-output.txt + rm filtered-output.txt else - echo "No output file found" >> "${ISSUE_FILE}" + echo "No output file found" >>"${ISSUE_FILE}" fi - name: Append issue footer if: ${{ steps.run_clang_tidy.outcome != 'success' }} run: | - cat >> "${ISSUE_FILE}" <>"${ISSUE_FILE}" <> "${GITHUB_OUTPUT}" + ./generate.py --packaging --config=linux.json >>"${GITHUB_OUTPUT}" generate-version: runs-on: ubuntu-latest diff --git a/.github/workflows/reusable-strategy-matrix.yml b/.github/workflows/reusable-strategy-matrix.yml index b1232a138f..62d65ad3fa 100644 --- a/.github/workflows/reusable-strategy-matrix.yml +++ b/.github/workflows/reusable-strategy-matrix.yml @@ -42,4 +42,4 @@ jobs: env: GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}.json', inputs.os) || '' }} GENERATE_OPTION: ${{ inputs.strategy_matrix == 'all' && '--all' || '' }} - run: ./generate.py ${GENERATE_OPTION} ${GENERATE_CONFIG} >> "${GITHUB_OUTPUT}" + run: ./generate.py ${GENERATE_OPTION} ${GENERATE_CONFIG} >>"${GITHUB_OUTPUT}" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 23441c9dde..c9dec89435 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -66,6 +66,19 @@ repos: - id: shfmt args: [--write, --indent=4, --case-indent=true] + - repo: local + hooks: + - id: format-inline-bash-workflows + name: "format `run:` blocks in workflows/actions" + entry: ./.github/scripts/format-inline-bash.py + language: python + files: ^\.github/(workflows|actions)/.*\.ya?ml$ + - id: format-inline-bash-markdown + name: "format ```bash blocks in markdown" + entry: ./.github/scripts/format-inline-bash.py + language: python + files: \.md$ + - repo: https://github.com/streetsidesoftware/cspell-cli rev: 4643f154907327ee0a2c7038f0296e0dd77d9776 # frozen: v10.0.0 hooks: diff --git a/BUILD.md b/BUILD.md index a1163dbfcc..1d3fc8f774 100644 --- a/BUILD.md +++ b/BUILD.md @@ -151,8 +151,8 @@ git init git remote add origin git@github.com:XRPLF/conan-center-index.git git sparse-checkout init for recipe in "${recipes[@]}"; do - echo "Checking out recipe '${recipe}'..." - git sparse-checkout add recipes/${recipe} + echo "Checking out recipe '${recipe}'..." + git sparse-checkout add recipes/${recipe} done git fetch origin master git checkout master @@ -180,7 +180,7 @@ the new recipe will be automatically pulled from the official Conan Center. If you see an error similar to the following after running `conan profile show`: -```bash +```text ERROR: Invalid setting '17' is not a valid 'settings.compiler.version' value. Possible values are ['5.0', '5.1', '6.0', '6.1', '7.0', '7.3', '8.0', '8.1', '9.0', '9.1', '10.0', '11.0', '12.0', '13', '13.0', '13.1', '14', '14.0', '15', diff --git a/cspell.config.yaml b/cspell.config.yaml index f26268c804..ed936941e4 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -93,6 +93,7 @@ words: - daria - dcmake - dearmor + - dedented - deleteme - demultiplexer - deserializaton diff --git a/include/xrpl/protocol_autogen/README.md b/include/xrpl/protocol_autogen/README.md index 860ed1a242..608ffed085 100644 --- a/include/xrpl/protocol_autogen/README.md +++ b/include/xrpl/protocol_autogen/README.md @@ -15,8 +15,8 @@ Generation requires a one-time setup step to create a virtual environment and install Python dependencies, followed by running the generation target: ```bash -cmake --build . --target setup_code_gen # create venv and install dependencies (once) -cmake --build . --target code_gen # generate code +cmake --build . --target setup_code_gen # create venv and install dependencies (once) +cmake --build . --target code_gen # generate code ``` By default, `CODEGEN_VENV_DIR` points to `.venv` in the project root. The diff --git a/package/README.md b/package/README.md index 2089e32e64..867ca273b4 100644 --- a/package/README.md +++ b/package/README.md @@ -74,10 +74,10 @@ VERSION=2.4.0-local PKG_RELEASE=1 docker run --rm \ - -v "$(pwd):/src" \ - -w /src \ - "$IMAGE" \ - ./package/build_pkg.sh --pkg-version "$VERSION" --pkg-release "$PKG_RELEASE" + -v "$(pwd):/src" \ + -w /src \ + "$IMAGE" \ + ./package/build_pkg.sh --pkg-version "$VERSION" --pkg-release "$PKG_RELEASE" # Output: # build/debbuild/*.deb (DEB + dbgsym .ddeb) @@ -92,12 +92,12 @@ needed, but the host toolchain replaces the pinned CI image: ```bash cmake \ - -Dxrpld=ON \ - -Dxrpld_version=2.4.0-local \ - -Dtests=OFF \ - .. + -Dxrpld=ON \ + -Dxrpld_version=2.4.0-local \ + -Dtests=OFF \ + .. -cmake --build . --target package # deb on Debian/Ubuntu, rpm on RHEL +cmake --build . --target package # deb on Debian/Ubuntu, rpm on RHEL ``` The `cmake/XrplPackaging.cmake` module defines the target only if at least one From 2f3558c610ee3d446b1c68a003442e63d4abbdf5 Mon Sep 17 00:00:00 2001 From: Bart Date: Thu, 28 May 2026 10:57:29 -0400 Subject: [PATCH 6/6] ci: Run PR title and description checks on staging and release branches (#7331) Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com> --- .github/workflows/check-pr-description.yml | 13 +++++++++++-- .github/workflows/check-pr-title.yml | 13 +++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check-pr-description.yml b/.github/workflows/check-pr-description.yml index 3f345a9dcb..ff28220171 100644 --- a/.github/workflows/check-pr-description.yml +++ b/.github/workflows/check-pr-description.yml @@ -5,8 +5,17 @@ on: types: - checks_requested pull_request: - types: [opened, edited, reopened, synchronize, ready_for_review] - branches: [develop] + types: + - opened + - edited + - reopened + - synchronize + - ready_for_review + branches: + - develop + - "release-*" + - "release/*" + - "staging/*" jobs: check_description: diff --git a/.github/workflows/check-pr-title.yml b/.github/workflows/check-pr-title.yml index 79962e0620..4b5f679df1 100644 --- a/.github/workflows/check-pr-title.yml +++ b/.github/workflows/check-pr-title.yml @@ -5,8 +5,17 @@ on: types: - checks_requested pull_request: - types: [opened, edited, reopened, synchronize, ready_for_review] - branches: [develop] + types: + - opened + - edited + - reopened + - synchronize + - ready_for_review + branches: + - develop + - "release-*" + - "release/*" + - "staging/*" jobs: check_title: