Merge branch 'ximinez/lending-refactoring-3' into ximinez/lending-refactoring-4

This commit is contained in:
Ed Hennis
2025-09-03 18:10:26 -04:00
committed by GitHub
9 changed files with 105 additions and 86 deletions

View File

@@ -28,9 +28,8 @@ namespace ripple {
// the destination can hold all values of the source. This is particularly
// handy when the source or destination is an enumeration type.
template <class Dest, class Src>
static constexpr bool is_safetocasttovalue_v =
(std::is_integral_v<Src> && std::is_integral_v<Dest>) &&
template <class Src, class Dest>
concept SafeToCast = (std::is_integral_v<Src> && std::is_integral_v<Dest>) &&
(std::is_signed<Src>::value || std::is_unsigned<Dest>::value) &&
(std::is_signed<Src>::value != std::is_signed<Dest>::value
? sizeof(Dest) > sizeof(Src)
@@ -78,7 +77,7 @@ inline constexpr std::
unsafe_cast(Src s) noexcept
{
static_assert(
!is_safetocasttovalue_v<Dest, Src>,
!SafeToCast<Src, Dest>,
"Only unsafe if casting signed to unsigned or "
"destination is too small");
return static_cast<Dest>(s);

View File

@@ -22,7 +22,6 @@
#include <xrpl/basics/ByteUtilities.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/basics/partitioned_unordered_map.h>
#include <cstdint>

View File

@@ -300,7 +300,7 @@ public:
static int
compare(SField const& f1, SField const& f2);
static std::map<int, SField const*> const&
static std::unordered_map<int, SField const*> const&
getKnownCodeToField()
{
return knownCodeToField;
@@ -308,8 +308,8 @@ public:
private:
static int num;
static std::map<int, SField const*> knownCodeToField;
static std::map<std::string, SField const*> knownNameToField;
static std::unordered_map<int, SField const*> knownCodeToField;
static std::unordered_map<std::string, SField const*> knownNameToField;
};
/** A field with a type known at compile time. */

View File

@@ -50,25 +50,32 @@ struct unitlessTag;
class BipsTag;
class TenthBipsTag;
template <class T>
using enable_if_unit_t = typename std::enable_if_t<
std::is_class_v<T> && std::is_object_v<typename T::unit_type> &&
std::is_object_v<typename T::value_type>>;
// These names don't have to be too descriptive, because we're in the "unit"
// namespace.
/** `is_usable_unit_v` is checked to ensure that only values with
template <class T>
concept Valid = std::is_class_v<T> && std::is_object_v<typename T::unit_type> &&
std::is_object_v<typename T::value_type>;
/** `Usable` is checked to ensure that only values with
known valid type tags can be used (sometimes transparently) in
non-unit contexts. At the time of implementation, this includes
all known tags, but more may be added in the future, and they
should not be added automatically unless determined to be
appropriate.
*/
template <class T, class = enable_if_unit_t<T>>
constexpr bool is_usable_unit_v =
std::is_same_v<typename T::unit_type, feelevelTag> ||
std::is_same_v<typename T::unit_type, unitlessTag> ||
std::is_same_v<typename T::unit_type, dropTag> ||
std::is_same_v<typename T::unit_type, BipsTag> ||
std::is_same_v<typename T::unit_type, TenthBipsTag>;
template <class T>
concept Usable = Valid<T> &&
(std::is_same_v<typename T::unit_type, feelevelTag> ||
std::is_same_v<typename T::unit_type, unitlessTag> ||
std::is_same_v<typename T::unit_type, dropTag> ||
std::is_same_v<typename T::unit_type, BipsTag> ||
std::is_same_v<typename T::unit_type, TenthBipsTag>);
template <class Other, class VU>
concept Compatible = Valid<VU> && std::is_arithmetic_v<Other> &&
std::is_arithmetic_v<typename VU::value_type> &&
std::is_convertible_v<Other, typename VU::value_type>;
template <class UnitTag, class T>
class ValueUnit : private boost::totally_ordered<ValueUnit<UnitTag, T>>,
@@ -85,25 +92,6 @@ public:
private:
value_type value_;
protected:
template <class Other>
static constexpr bool is_compatible_v =
std::is_arithmetic_v<Other> && std::is_arithmetic_v<value_type> &&
std::is_convertible_v<Other, value_type>;
template <class OtherValue, class = enable_if_unit_t<OtherValue>>
static constexpr bool is_compatiblevalue_v =
is_compatible_v<typename OtherValue::value_type> &&
std::is_same_v<UnitTag, typename OtherValue::unit_type>;
template <class Other>
using enable_if_compatible_t =
typename std::enable_if_t<is_compatible_v<Other>>;
template <class OtherValue>
using enable_if_compatiblevalue_t =
typename std::enable_if_t<is_compatiblevalue_v<OtherValue>>;
public:
ValueUnit() = default;
constexpr ValueUnit(ValueUnit const& other) = default;
@@ -135,12 +123,9 @@ public:
/** Instances with the same unit, and a type that is
"safe" to convert to this one can be converted
implicitly */
template <
class Other,
class = std::enable_if_t<
is_compatible_v<Other> &&
is_safetocasttovalue_v<value_type, Other>>>
template <Compatible<ValueUnit> Other>
constexpr ValueUnit(ValueUnit<unit_type, Other> const& value)
requires SafeToCast<Other, value_type>
: ValueUnit(safe_cast<value_type>(value.value()))
{
}
@@ -255,7 +240,7 @@ public:
return value_ == other.value_;
}
template <class Other, class = enable_if_compatible_t<Other>>
template <Compatible<ValueUnit> Other>
constexpr bool
operator==(ValueUnit<unit_type, Other> const& other) const
{
@@ -268,7 +253,7 @@ public:
return value_ == other;
}
template <class Other, class = enable_if_compatible_t<Other>>
template <Compatible<ValueUnit> Other>
constexpr bool
operator!=(ValueUnit<unit_type, Other> const& other) const
{
@@ -310,12 +295,13 @@ public:
return static_cast<double>(value_) / reference.value();
}
// `is_usable_unit_v` is checked to ensure that only values with
// `Usable` is checked to ensure that only values with
// known valid type tags can be converted to JSON. At the time
// of implementation, that includes all known tags, but more may
// be added in the future.
std::enable_if_t<is_usable_unit_v<ValueUnit>, Json::Value>
Json::Value
jsonClipped() const
requires Usable<ValueUnit>
{
if constexpr (std::is_integral_v<value_type>)
{
@@ -372,43 +358,27 @@ to_string(ValueUnit<UnitTag, T> const& amount)
return std::to_string(amount.value());
}
template <class Source, class = enable_if_unit_t<Source>>
template <Valid Source>
constexpr bool can_muldiv_source_v =
std::is_convertible_v<typename Source::value_type, std::uint64_t>;
template <class Dest, class = enable_if_unit_t<Dest>>
template <Valid Dest>
constexpr bool can_muldiv_dest_v =
can_muldiv_source_v<Dest> && // Dest is also a source
std::is_convertible_v<std::uint64_t, typename Dest::value_type> &&
sizeof(typename Dest::value_type) >= sizeof(std::uint64_t);
template <
class Source1,
class Source2,
class = enable_if_unit_t<Source1>,
class = enable_if_unit_t<Source2>>
template <Valid Source1, Valid Source2>
constexpr bool can_muldiv_sources_v =
can_muldiv_source_v<Source1> && can_muldiv_source_v<Source2> &&
std::is_same_v<typename Source1::unit_type, typename Source2::unit_type>;
template <
class Source1,
class Source2,
class Dest,
class = enable_if_unit_t<Source1>,
class = enable_if_unit_t<Source2>,
class = enable_if_unit_t<Dest>>
template <Valid Source1, Valid Source2, Valid Dest>
constexpr bool can_muldiv_v =
can_muldiv_sources_v<Source1, Source2> && can_muldiv_dest_v<Dest>;
// Source and Dest can be the same by default
template <
class Source1,
class Source2,
class Dest,
class = enable_if_unit_t<Source1>,
class = enable_if_unit_t<Source2>,
class = enable_if_unit_t<Dest>>
template <Valid Source1, Valid Source2, Valid Dest>
constexpr bool can_muldiv_commute_v = can_muldiv_v<Source1, Source2, Dest> &&
!std::is_same_v<typename Source1::unit_type, typename Dest::unit_type>;

View File

@@ -77,7 +77,7 @@ TRANSACTION(ttESCROW_CREATE, 1, EscrowCreate,
/** This transaction type completes an existing escrow. */
TRANSACTION(ttESCROW_FINISH, 2, EscrowFinish,
Delegation::delegatable,
mayAuthorizeMPT,
noPriv,
({
{sfOwner, soeREQUIRED},
{sfOfferSequence, soeREQUIRED},

View File

@@ -28,8 +28,8 @@ namespace ripple {
// Storage for static const members.
SField::IsSigning const SField::notSigning;
int SField::num = 0;
std::map<int, SField const*> SField::knownCodeToField;
std::map<std::string, SField const*> SField::knownNameToField;
std::unordered_map<int, SField const*> SField::knownCodeToField;
std::unordered_map<std::string, SField const*> SField::knownNameToField;
// Give only this translation unit permission to construct SFields
struct SField::private_access_tag_t

View File

@@ -31,9 +31,10 @@ fee::operator()(Env& env, JTx& jt) const
if (!manual_)
return;
jt.fill_fee = false;
assert(!increment_ || !amount_);
if (increment_)
jt[sfFee] = STAmount(env.current()->fees().increment).getJson();
if (amount_)
else if (amount_)
jt[sfFee] = amount_->getJson(JsonOptions::none);
}

View File

@@ -37,6 +37,25 @@
namespace ripple {
/*
assert(enforce)
There are several asserts (or XRPL_ASSERTs) in this file that check a variable
named `enforce` when an invariant fails. At first glace, those asserts may look
incorrect, but they are not.
Those asserts take advantage of two facts:
1. `asserts` are not (normally) executed in release builds.
2. Invariants should *never* fail, except in tests that specifically modify
the open ledger to break them.
To make it sort of a second-layer of invariant enforcement aimed at
_developers_. It's designed to fire if a developer writes code that violates an
invariant, and runs it in unit tests or a develop build that _does not have the
relevant amendments enabled_. It's intentionally a pain in the neck so that bad
code gets caught and fixed as early as possible.
*/
enum Privilege {
noPriv =
0x0000, // The transaction can not do any of the enumerated operations
@@ -75,7 +94,7 @@ operator|(Privilege lhs, Privilege rhs)
}
bool
checkMyPrivilege(STTx const& tx, Privilege priv)
hasPrivilege(STTx const& tx, Privilege priv)
{
switch (tx.getTxnType())
{
@@ -432,7 +451,7 @@ AccountRootsNotDeleted::finalize(
// transaction when the total AMM LP Tokens balance goes to 0.
// A successful AccountDelete or AMMDelete MUST delete exactly
// one account root.
if (checkMyPrivilege(tx, mustDeleteAcct) && result == tesSUCCESS)
if (hasPrivilege(tx, mustDeleteAcct) && result == tesSUCCESS)
{
if (accountsDeleted_ == 1)
return true;
@@ -449,7 +468,7 @@ AccountRootsNotDeleted::finalize(
// A successful AMMWithdraw/AMMClawback MAY delete one account root
// when the total AMM LP Tokens balance goes to 0. Not every AMM withdraw
// deletes the AMM account, accountsDeleted_ is set if it is deleted.
if (checkMyPrivilege(tx, mayDeleteAcct) && result == tesSUCCESS &&
if (hasPrivilege(tx, mayDeleteAcct) && result == tesSUCCESS &&
accountsDeleted_ == 1)
return true;
@@ -505,6 +524,8 @@ AccountRootsDeletedClean::finalize(
JLOG(j.fatal())
<< "Invariant failed: account deletion left behind a "
<< typeName << " object";
// The comment above starting with "assert(enforce)" explains this
// assert.
XRPL_ASSERT(
enforce,
"ripple::AccountRootsDeletedClean::finalize::objectExists : "
@@ -750,6 +771,8 @@ TransfersNotFrozen::finalize(
// just in case so rippled doesn't crash in release.
if (!issuerSle)
{
// The comment above starting with "assert(enforce)" explains this
// assert.
XRPL_ASSERT(
enforce,
"ripple::TransfersNotFrozen::finalize : enforce "
@@ -938,7 +961,7 @@ TransfersNotFrozen::validateFrozenState(
}
// AMMClawbacks are allowed to override some freeze rules
if ((!isAMMLine || globalFreeze) && checkMyPrivilege(tx, overrideFreeze))
if ((!isAMMLine || globalFreeze) && hasPrivilege(tx, overrideFreeze))
{
JLOG(j.debug()) << "Invariant check allowing funds to be moved "
<< (change.balanceChangeSign > 0 ? "to" : "from")
@@ -949,6 +972,7 @@ TransfersNotFrozen::validateFrozenState(
JLOG(j.fatal()) << "Invariant failed: Attempting to move frozen funds for "
<< tx.getTransactionID();
// The comment above starting with "assert(enforce)" explains this assert.
XRPL_ASSERT(
enforce,
"ripple::TransfersNotFrozen::validateFrozenState : enforce "
@@ -998,13 +1022,12 @@ ValidNewAccountRoot::finalize(
}
// From this point on we know exactly one account was created.
if (checkMyPrivilege(tx, createAcct | createPseudoAcct) &&
result == tesSUCCESS)
if (hasPrivilege(tx, createAcct | createPseudoAcct) && result == tesSUCCESS)
{
bool const pseudoAccount =
(pseudoAccount_ && view.rules().enabled(featureSingleAssetVault));
if (pseudoAccount && !checkMyPrivilege(tx, createPseudoAcct))
if (pseudoAccount && !hasPrivilege(tx, createPseudoAcct))
{
JLOG(j.fatal()) << "Invariant failed: pseudo-account created by a "
"wrong transaction type";
@@ -1238,7 +1261,7 @@ NFTokenCountTracking::finalize(
ReadView const& view,
beast::Journal const& j)
{
if (!checkMyPrivilege(tx, changeNFTCounts))
if (!hasPrivilege(tx, changeNFTCounts))
{
if (beforeMintedTotal != afterMintedTotal)
{
@@ -1423,12 +1446,12 @@ ValidMPTIssuance::finalize(
STTx const& tx,
TER const result,
XRPAmount const _fee,
ReadView const& _view,
ReadView const& view,
beast::Journal const& j)
{
if (result == tesSUCCESS)
{
if (checkMyPrivilege(tx, createMPTIssuance))
if (hasPrivilege(tx, createMPTIssuance))
{
if (mptIssuancesCreated_ == 0)
{
@@ -1449,7 +1472,7 @@ ValidMPTIssuance::finalize(
return mptIssuancesCreated_ == 1 && mptIssuancesDeleted_ == 0;
}
if (checkMyPrivilege(tx, destroyMPTIssuance))
if (hasPrivilege(tx, destroyMPTIssuance))
{
if (mptIssuancesDeleted_ == 0)
{
@@ -1470,7 +1493,17 @@ ValidMPTIssuance::finalize(
return mptIssuancesCreated_ == 0 && mptIssuancesDeleted_ == 1;
}
if (checkMyPrivilege(tx, mustAuthorizeMPT | mayAuthorizeMPT))
// ttESCROW_FINISH may authorize an MPT, but it can't have the
// mayAuthorizeMPT privilege, because that may cause
// non-amendment-gated side effects.
bool const enforceEscrowFinish = (tx.getTxnType() == ttESCROW_FINISH) &&
(view.rules().enabled(featureSingleAssetVault)
/*
TODO: Uncomment when LendingProtocol is defined
|| view.rules().enabled(featureLendingProtocol)*/
);
if (hasPrivilege(tx, mustAuthorizeMPT | mayAuthorizeMPT) ||
enforceEscrowFinish)
{
bool const submittedByIssuer = tx.isFieldPresent(sfHolder);
@@ -1496,7 +1529,7 @@ ValidMPTIssuance::finalize(
return false;
}
else if (
!submittedByIssuer && !checkMyPrivilege(tx, mayAuthorizeMPT) &&
!submittedByIssuer && hasPrivilege(tx, mustAuthorizeMPT) &&
(mptokensCreated_ + mptokensDeleted_ != 1))
{
// if the holder submitted this tx, then a mptoken must be
@@ -1509,6 +1542,17 @@ ValidMPTIssuance::finalize(
return true;
}
if (tx.getTxnType() == ttESCROW_FINISH)
{
// ttESCROW_FINISH may authorize an MPT, but it can't have the
// mayAuthorizeMPT privilege, because that may cause
// non-amendment-gated side effects.
XRPL_ASSERT_PARTS(
!enforceEscrowFinish,
"ripple::ValidMPTIssuance::finalize",
"not escrow finish tx");
return true;
}
}
if (mptIssuancesCreated_ != 0)
@@ -1711,6 +1755,8 @@ ValidPseudoAccounts::finalize(
beast::Journal const& j)
{
bool const enforce = view.rules().enabled(featureSingleAssetVault);
// The comment above starting with "assert(enforce)" explains this assert.
XRPL_ASSERT(
errors_.empty() || enforce,
"ripple::ValidPseudoAccounts::finalize : no bad "

View File

@@ -1092,9 +1092,13 @@ getPseudoAccountFields()
static std::vector<SField const*> const pseudoFields = []() {
auto const ar = LedgerFormats::getInstance().findByType(ltACCOUNT_ROOT);
if (!ar)
{
// LCOV_EXCL_START
LogicError(
"ripple::isPseudoAccount : unable to find account root ledger "
"format");
// LCOV_EXCL_STOP
}
auto const& soTemplate = ar->getSOTemplate();
std::vector<SField const*> pseudoFields;