From e22938d69f46308b0eb2c28e919077799ddf807d Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 12 May 2026 18:44:05 -0400 Subject: [PATCH] AI review feedback: createGuards - Refactor the Guard decision in withTxnType into createGuards, which lives in Rules.cpp. It is physically located near setCurrentTransactionRules, and documented to explain that changes need to be synchronized. --- include/xrpl/basics/Number.h | 4 ++-- include/xrpl/protocol/Rules.h | 9 +++++++++ src/libxrpl/protocol/Rules.cpp | 32 ++++++++++++++++++++++++++++++++ src/libxrpl/tx/applySteps.cpp | 17 +++-------------- 4 files changed, 46 insertions(+), 16 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 5026dd112a..30cb2b73cb 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -73,7 +73,7 @@ isPowerOfTen(T value) * 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 { @@ -250,7 +250,7 @@ concept Integral64 = std::is_same_v || std::is_same_v saved_; }; +class NumberSO; + +void +createGuards( + Rules const& rules, + std::optional& stNumberSO, + std::optional& rulesGuard, + std::optional& mantissaScaleGuard); + } // namespace xrpl diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index bffe3602d0..483b0c282e 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -46,6 +47,9 @@ setCurrentTransactionRules(std::optional r) // Because fixCleanup3_2_0 fixes a separate bug related to the large // mantissas, that can take precedence and activate the large // mantissas even in the absence of the other two amendments. + // + // If any new conditions with new amendments are added, those amendments must also be added to + // createGuards. bool const enableCuspRoundingFix = !r || r->enabled(fixCleanup3_2_0); bool const enableVaultNumbers = enableCuspRoundingFix || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); @@ -57,6 +61,34 @@ setCurrentTransactionRules(std::optional r) *getCurrentTransactionRulesRef() = std::move(r); } +void +createGuards( + Rules const& rules, + std::optional& stNumberSO, + std::optional& rulesGuard, + std::optional& mantissaScaleGuard) +{ + // The list amendments used to decide which guard(s) to create 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, and + // the first set of guards can be created directly at the call site, without using optional. + if (rules.enabled(fixCleanup3_2_0) || 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); + } +} + class Rules::Impl { private: diff --git a/src/libxrpl/tx/applySteps.cpp b/src/libxrpl/tx/applySteps.cpp index 99b3425275..4ac6f49f13 100644 --- a/src/libxrpl/tx/applySteps.cpp +++ b/src/libxrpl/tx/applySteps.cpp @@ -66,26 +66,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) {