From 52a04e68720f629477f1035e2fb3b2e2997ce8b6 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Wed, 12 Jun 2024 15:27:35 -0700 Subject: [PATCH] [FOLD] Address comments on the Quality implementation --- src/ripple/protocol/Quality.h | 175 ++++++++++++++++++++++------------ 1 file changed, 115 insertions(+), 60 deletions(-) diff --git a/src/ripple/protocol/Quality.h b/src/ripple/protocol/Quality.h index 2057e385de8..06b8254df5e 100644 --- a/src/ripple/protocol/Quality.h +++ b/src/ripple/protocol/Quality.h @@ -181,94 +181,71 @@ class Quality Math is avoided if the result is exact. The output is clamped to prevent money creation. */ - Amounts + [[nodiscard]] Amounts ceil_in(Amounts const& amount, STAmount const& limit) const; template - TAmounts - ceil_in(TAmounts const& amount, In const& limit) const - { - if (amount.in <= limit) - return amount; - - // Use the existing STAmount implementation for now, but consider - // replacing with code specific to IOUAMount and XRPAmount - Amounts stAmt(toSTAmount(amount.in), toSTAmount(amount.out)); - STAmount stLim(toSTAmount(limit)); - auto const stRes = ceil_in(stAmt, stLim); - return TAmounts( - toAmount(stRes.in), toAmount(stRes.out)); - } + [[nodiscard]] TAmounts + ceil_in(TAmounts const& amount, In const& limit) const; - Amounts + // Some of the underlying rounding functions called by ceil_in() ignored + // low order bits that could influence rounding decisions. This "strict" + // method uses underlying functions that pay attention to all the bits. + [[nodiscard]] Amounts ceil_in_strict(Amounts const& amount, STAmount const& limit, bool roundUp) const; template - TAmounts + [[nodiscard]] TAmounts ceil_in_strict( TAmounts const& amount, In const& limit, - bool roundUp) const - { - if (amount.in <= limit) - return amount; - - // Use the existing STAmount implementation for now, but consider - // replacing with code specific to IOUAMount and XRPAmount - Amounts stAmt(toSTAmount(amount.in), toSTAmount(amount.out)); - STAmount stLim(toSTAmount(limit)); - auto const stRes = ceil_in_strict(stAmt, stLim, roundUp); - return TAmounts( - toAmount(stRes.in), toAmount(stRes.out)); - } + bool roundUp) const; /** Returns the scaled amount with out capped. Math is avoided if the result is exact. The input is clamped to prevent money creation. */ - Amounts + [[nodiscard]] Amounts ceil_out(Amounts const& amount, STAmount const& limit) const; template - TAmounts - ceil_out(TAmounts const& amount, Out const& limit) const - { - if (amount.out <= limit) - return amount; - - // Use the existing STAmount implementation for now, but consider - // replacing with code specific to IOUAMount and XRPAmount - Amounts stAmt(toSTAmount(amount.in), toSTAmount(amount.out)); - STAmount stLim(toSTAmount(limit)); - auto const stRes = ceil_out(stAmt, stLim); - return TAmounts( - toAmount(stRes.in), toAmount(stRes.out)); - } + [[nodiscard]] TAmounts + ceil_out(TAmounts const& amount, Out const& limit) const; - Amounts + // Some of the underlying rounding functions called by ceil_out() ignored + // low order bits that could influence rounding decisions. This "strict" + // method uses underlying functions that pay attention to all the bits. + [[nodiscard]] Amounts ceil_out_strict(Amounts const& amount, STAmount const& limit, bool roundUp) const; template - TAmounts + [[nodiscard]] TAmounts ceil_out_strict( TAmounts const& amount, Out const& limit, - bool roundUp) const - { - if (amount.out <= limit) - return amount; - - // Use the existing STAmount implementation for now, but consider - // replacing with code specific to IOUAMount and XRPAmount - Amounts stAmt(toSTAmount(amount.in), toSTAmount(amount.out)); - STAmount stLim(toSTAmount(limit)); - auto const stRes = ceil_out_strict(stAmt, stLim, roundUp); - return TAmounts( - toAmount(stRes.in), toAmount(stRes.out)); - } + bool roundUp) const; + +private: + // The ceil_in and ceil_out methods that deal in TAmount all convert + // their arguments to STAoumout and convert the result back to TAmount. + // This helper function takes care of all the conversion operations. + template < + class In, + class Out, + class Lim, + typename FnPtr, + std::same_as... Round> + [[nodiscard]] TAmounts + ceil_TAmounts_helper( + TAmounts const& amount, + Lim const& limit, + Lim const& limit_cmp, + FnPtr ceil_function, + Round... round) const; +public: /** Returns `true` if lhs is lower quality than `rhs`. Lower quality means the taker receives a worse deal. Higher quality is better for the taker. @@ -350,6 +327,84 @@ class Quality } }; +template < + class In, + class Out, + class Lim, + typename FnPtr, + std::same_as... Round> +TAmounts +Quality::ceil_TAmounts_helper( + TAmounts const& amount, + Lim const& limit, + Lim const& limit_cmp, + FnPtr ceil_function, + Round... roundUp) const +{ + if (limit_cmp <= limit) + return amount; + + // Use the existing STAmount implementation for now, but consider + // replacing with code specific to IOUAMount and XRPAmount + Amounts stAmt(toSTAmount(amount.in), toSTAmount(amount.out)); + STAmount stLim(toSTAmount(limit)); + Amounts const stRes = ((*this).*ceil_function)(stAmt, stLim, roundUp...); + return TAmounts(toAmount(stRes.in), toAmount(stRes.out)); +} + +template +TAmounts +Quality::ceil_in(TAmounts const& amount, In const& limit) const +{ + // Construct a function pointer to the function we want to call. + static constexpr Amounts (Quality::*ceil_in_fn_ptr)( + Amounts const&, STAmount const&) const = &Quality::ceil_in; + + return ceil_TAmounts_helper(amount, limit, amount.in, ceil_in_fn_ptr); +} + +template +TAmounts +Quality::ceil_in_strict( + TAmounts const& amount, + In const& limit, + bool roundUp) const +{ + // Construct a function pointer to the function we want to call. + static constexpr Amounts (Quality::*ceil_in_fn_ptr)( + Amounts const&, STAmount const&, bool) const = &Quality::ceil_in_strict; + + return ceil_TAmounts_helper( + amount, limit, amount.in, ceil_in_fn_ptr, roundUp); +} + +template +TAmounts +Quality::ceil_out(TAmounts const& amount, Out const& limit) const +{ + // Construct a function pointer to the function we want to call. + static constexpr Amounts (Quality::*ceil_out_fn_ptr)( + Amounts const&, STAmount const&) const = &Quality::ceil_out; + + return ceil_TAmounts_helper(amount, limit, amount.out, ceil_out_fn_ptr); +} + +template +TAmounts +Quality::ceil_out_strict( + TAmounts const& amount, + Out const& limit, + bool roundUp) const +{ + // Construct a function pointer to the function we want to call. + static constexpr Amounts (Quality::*ceil_out_fn_ptr)( + Amounts const&, STAmount const&, bool) const = + &Quality::ceil_out_strict; + + return ceil_TAmounts_helper( + amount, limit, amount.out, ceil_out_fn_ptr, roundUp); +} + /** Calculate the quality of a two-hop path given the two hops. @param lhs The first leg of the path: input to intermediate. @param rhs The second leg of the path: intermediate to output.