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.
This commit is contained in:
Ed Hennis
2026-05-12 18:44:05 -04:00
parent 7c9a56ff24
commit e22938d69f
4 changed files with 46 additions and 16 deletions

View File

@@ -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<T, std::int64_t> || std::is_same_v<T, std::u
* amendments are enabled to determine which result to expect.
*
*/
class Number
class Number final
{
using rep = std::int64_t;
using internalrep = MantissaRange::rep;

View File

@@ -122,4 +122,13 @@ private:
std::optional<Rules> saved_;
};
class NumberSO;
void
createGuards(
Rules const& rules,
std::optional<NumberSO>& stNumberSO,
std::optional<CurrentTransactionRulesGuard>& rulesGuard,
std::optional<NumberMantissaScaleGuard>& mantissaScaleGuard);
} // namespace xrpl

View File

@@ -7,6 +7,7 @@
#include <xrpl/beast/hash/uhash.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/IOUAmount.h>
#include <xrpl/protocol/STVector256.h>
#include <memory>
@@ -46,6 +47,9 @@ setCurrentTransactionRules(std::optional<Rules> 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<Rules> r)
*getCurrentTransactionRulesRef() = std::move(r);
}
void
createGuards(
Rules const& rules,
std::optional<NumberSO>& stNumberSO,
std::optional<CurrentTransactionRulesGuard>& rulesGuard,
std::optional<NumberMantissaScaleGuard>& 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:

View File

@@ -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<NumberSO> stNumberSO;
std::optional<CurrentTransactionRulesGuard> rulesGuard;
std::optional<NumberMantissaScaleGuard> 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)
{