From 7085aa3d55ca0df509c51447080c4495e6746bfb Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 10 Feb 2026 06:41:55 -0400 Subject: [PATCH] Add canonical "scale" computation to Number (#6246) * Add canonical "scale" computation to Number - Requires a template for STAmount and Asset. - Update tests and computeMinScale from #6217 to use scale. - Convert a few other places to use "scale" correctly. * ValidVault tracks scale of original operands alongside deltas * Update src/xrpld/app/tx/detail/InvariantCheck.cpp Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> * Change ValidVault::DeltaInfo::scale to an optional * Change computeMinScale to use a vector instead of an initializer_list * Fix compile errors --------- Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> --- include/xrpl/basics/Number.h | 32 ++ include/xrpl/protocol/STAmount.h | 4 +- src/test/app/Invariants_test.cpp | 136 ++++++++ src/xrpld/app/misc/LendingHelpers.h | 3 +- src/xrpld/app/tx/detail/InvariantCheck.cpp | 295 +++++++++++++----- src/xrpld/app/tx/detail/InvariantCheck.h | 14 +- .../app/tx/detail/LoanBrokerCoverWithdraw.cpp | 2 +- 7 files changed, 404 insertions(+), 82 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 2f467fb036..118714460e 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -110,6 +110,10 @@ template concept Integral64 = std::is_same_v || std::is_same_v; +template +concept CanUseAsScale = requires(Asset a, Number n) { STAmount(a, n); } && + requires(STAmount s) { s.exponent(); }; + /** Number is a floating point type that can represent a wide range of values. * * It can represent all values that can be represented by an STAmount - @@ -269,6 +273,26 @@ public: constexpr int exponent() const noexcept; + /** Get the scale of this Number for the given asset. + * + * "scale" is similar to "exponent", but from the perspective of STAmount, + * which has different rules for determining the exponent than Number. + * + * Because Number does not have access to STAmount or Asset, this function + * is implemented as a template, with the expectation that it will only be + * used by those types. Any types that fit the requirements will work, + * though, if there's a need. + * + * @tparam STAmount The STAmount type. + * @tparam Asset The Asset type. + * @param asset The asset to use for determining the scale. + * @return The scale of this Number for the given asset. + */ + template + int + scale(Asset const& asset) const + requires CanUseAsScale; + constexpr Number operator+() const noexcept; constexpr Number @@ -603,6 +627,14 @@ Number::exponent() const noexcept return e; } +template +int +Number::scale(Asset const& asset) const + requires CanUseAsScale +{ + return STAmount{asset, *this}.exponent(); +} + inline constexpr Number Number::operator+() const noexcept { diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index 4d86aed2ec..4572801a45 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -43,8 +43,8 @@ private: public: using value_type = STAmount; - static int const cMinOffset = -96; - static int const cMaxOffset = 80; + static int constexpr cMinOffset = -96; + static int constexpr cMaxOffset = 80; // Maximum native value supported by the code constexpr static std::uint64_t cMinValue = 1'000'000'000'000'000ull; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 1e81959ae6..ee5d2cbf08 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -3891,6 +3892,140 @@ class Invariants_test : public beast::unit_test::suite precloseMpt); } + void + testVaultComputeMinScale() + { + using namespace jtx; + + Account const issuer{"issuer"}; + PrettyAsset const vaultAsset = issuer["IOU"]; + + struct TestCase + { + std::string name; + std::int32_t expectedMinScale; + std::vector values; + }; + + NumberMantissaScaleGuard g{MantissaRange::large}; + + auto makeDelta = + [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { + return {n, n.scale(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) + { + testcase("vault computeMinScale: " + tc.name); + + auto const actualScale = + ValidVault::computeMinScale(vaultAsset, tc.values); + + 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 computeMinScale: " + tc.name); + + auto const actualScale = + ValidVault::computeMinScale(vaultAsset, 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)); + } + } + } + public: void run() override @@ -3914,6 +4049,7 @@ public: testValidPseudoAccounts(); testValidLoanBroker(); testVault(); + testVaultComputeMinScale(); } }; diff --git a/src/xrpld/app/misc/LendingHelpers.h b/src/xrpld/app/misc/LendingHelpers.h index 79fc617569..5780f76428 100644 --- a/src/xrpld/app/misc/LendingHelpers.h +++ b/src/xrpld/app/misc/LendingHelpers.h @@ -176,8 +176,7 @@ getAssetsTotalScale(SLE::const_ref vaultSle) { if (!vaultSle) return Number::minExponent - 1; // LCOV_EXCL_LINE - return STAmount{vaultSle->at(sfAsset), vaultSle->at(sfAssetsTotal)} - .exponent(); + return vaultSle->at(sfAssetsTotal).scale(vaultSle->at(sfAsset)); } TER diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 6fb832a05e..2c5f62105f 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -2664,8 +2664,13 @@ ValidVault::visitEntry( // Number balanceDelta will capture the difference (delta) between "before" // state (zero if created) and "after" state (zero if destroyed), so the // invariants can validate that the change in account balances matches the - // change in vault balances, stored to deltas_ at the end of this function. - Number balanceDelta{}; + // balanceDelta captures the difference (delta) between "before" + // state (zero if created) and "after" state (zero if destroyed), and + // preserves value scale (exponent) to round values to the same scale during + // validation. It is used to validate that the change in account + // balances matches the change in vault balances, stored to deltas_ at the + // end of this function. + DeltaInfo balanceDelta{numZero, std::nullopt}; std::int8_t sign = 0; if (before) @@ -2679,20 +2684,35 @@ ValidVault::visitEntry( // At this moment we have no way of telling if this object holds // vault shares or something else. Save it for finalize. beforeMPTs_.push_back(Shares::make(*before)); - balanceDelta = static_cast( + balanceDelta.delta = static_cast( before->getFieldU64(sfOutstandingAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = 1; break; case ltMPTOKEN: - balanceDelta = + balanceDelta.delta = static_cast(before->getFieldU64(sfMPTAmount)); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = -1; break; case ltACCOUNT_ROOT: - case ltRIPPLE_STATE: - balanceDelta = before->getFieldAmount(sfBalance); + balanceDelta.delta = before->getFieldAmount(sfBalance); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; sign = -1; break; + case ltRIPPLE_STATE: { + auto const amount = before->getFieldAmount(sfBalance); + balanceDelta.delta = amount; + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + balanceDelta.scale = amount.exponent(); + sign = -1; + break; + } default:; } } @@ -2708,21 +2728,36 @@ ValidVault::visitEntry( // At this moment we have no way of telling if this object holds // vault shares or something else. Save it for finalize. afterMPTs_.push_back(Shares::make(*after)); - balanceDelta -= Number( - static_cast( - after->getFieldU64(sfOutstandingAmount))); + balanceDelta.delta -= Number(static_cast( + after->getFieldU64(sfOutstandingAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = 1; break; case ltMPTOKEN: - balanceDelta -= Number( + balanceDelta.delta -= Number( static_cast(after->getFieldU64(sfMPTAmount))); + // MPTs are ints, so the scale is always 0. + balanceDelta.scale = 0; sign = -1; break; case ltACCOUNT_ROOT: - case ltRIPPLE_STATE: - balanceDelta -= Number(after->getFieldAmount(sfBalance)); + balanceDelta.delta -= Number(after->getFieldAmount(sfBalance)); + // Account balance is XRP, which is an int, so the scale is + // always 0. + balanceDelta.scale = 0; sign = -1; break; + case ltRIPPLE_STATE: { + auto const amount = after->getFieldAmount(sfBalance); + balanceDelta.delta -= Number(amount); + // Trust Line balances are STAmounts, so we can use the exponent + // directly to get the scale. + if (amount.exponent() > balanceDelta.scale) + balanceDelta.scale = amount.exponent(); + sign = -1; + break; + } default:; } } @@ -2734,7 +2769,14 @@ ValidVault::visitEntry( // transferred to the account. We intentionally do not compare balanceDelta // against zero, to avoid missing such updates. if (sign != 0) - deltas_[key] = balanceDelta * sign; + { + XRPL_ASSERT_PARTS( + balanceDelta.scale, + "xrpl::ValidVault::visitEntry", + "scale initialized"); + balanceDelta.delta *= sign; + deltas_[key] = balanceDelta; + } } bool @@ -3014,13 +3056,15 @@ ValidVault::finalize( } auto const& vaultAsset = afterVault.asset; - auto const deltaAssets = [&](AccountID const& id) -> std::optional { + auto const deltaAssets = + [&](AccountID const& id) -> std::optional { auto const get = // - [&](auto const& it, std::int8_t sign = 1) -> std::optional { + [&](auto const& it, + std::int8_t sign = 1) -> std::optional { if (it == deltas_.end()) return std::nullopt; - return it->second * sign; + return DeltaInfo{it->second.delta * sign, it->second.scale}; }; return std::visit( @@ -3041,7 +3085,7 @@ ValidVault::finalize( }, vaultAsset.value()); }; - auto const deltaAssetsTxAccount = [&]() -> std::optional { + auto const deltaAssetsTxAccount = [&]() -> std::optional { auto ret = deltaAssets(tx[sfAccount]); // Nothing returned or not XRP transaction if (!ret.has_value() || !vaultAsset.native()) @@ -3052,13 +3096,14 @@ ValidVault::finalize( delegate.has_value() && *delegate != tx[sfAccount]) return ret; - *ret += fee.drops(); - if (*ret == zero) + ret->delta += fee.drops(); + if (ret->delta == zero) return std::nullopt; return ret; }; - auto const deltaShares = [&](AccountID const& id) -> std::optional { + auto const deltaShares = + [&](AccountID const& id) -> std::optional { auto const it = [&]() { if (id == afterVault.pseudoId) return deltas_.find( @@ -3066,7 +3111,7 @@ ValidVault::finalize( return deltas_.find(keylet::mptoken(afterVault.shareMPTID, id).key); }(); - return it != deltas_.end() ? std::optional(it->second) + return it != deltas_.end() ? std::optional(it->second) : std::nullopt; }; @@ -3074,15 +3119,6 @@ ValidVault::finalize( return vault.assetsAvailable == 0 && vault.assetsTotal == 0; }; - auto const withinDistance = [&](Number const& num1, - Number const& num2) -> bool { - if (vaultAsset.integral()) - return num1 == num2; - // The exponent was set experimentally, based on whether - // Loan_test::testWithdrawReflectsUnrealizedLoss unit test fail - return withinRelativeDistance(num1, num2, Number{1, -13}); - }; - // Technically this does not need to be a lambda, but it's more // convenient thanks to early "return false"; the not-so-nice // alternatives are several layers of nested if/else or more complex @@ -3216,8 +3252,32 @@ ValidVault::finalize( return false; // That's all we can do } - auto const vaultDeltaAssets = *maybeVaultDeltaAssets; - if (vaultDeltaAssets > tx[sfAmount]) + // Get the coarsest scale to round calculations to + DeltaInfo totalDelta{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + afterVault.assetsTotal.scale(vaultAsset), + beforeVault.assetsTotal.scale(vaultAsset))}; + DeltaInfo availableDelta{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + afterVault.assetsAvailable.scale(vaultAsset), + beforeVault.assetsAvailable.scale( + vaultAsset))}; + auto const minScale = computeMinScale( + vaultAsset, + { + *maybeVaultDeltaAssets, + totalDelta, + availableDelta, + }); + + auto const vaultDeltaAssets = roundToAsset( + vaultAsset, maybeVaultDeltaAssets->delta, minScale); + auto const txAmount = + roundToAsset(vaultAsset, tx[sfAmount], minScale); + + if (vaultDeltaAssets > txAmount) { JLOG(j.fatal()) << // "Invariant failed: deposit must not change vault " @@ -3250,7 +3310,14 @@ ValidVault::finalize( "balance"; return false; } - auto const accountDeltaAssets = *maybeAccDeltaAssets; + auto const localMinScale = std::max( + minScale, + computeMinScale(vaultAsset, {*maybeAccDeltaAssets})); + + auto const accountDeltaAssets = roundToAsset( + vaultAsset, maybeAccDeltaAssets->delta, localMinScale); + auto const localVaultDeltaAssets = roundToAsset( + vaultAsset, vaultDeltaAssets, localMinScale); if (accountDeltaAssets >= zero) { @@ -3260,8 +3327,7 @@ ValidVault::finalize( result = false; } - if (!withinDistance( - vaultDeltaAssets * -1, accountDeltaAssets)) + if (localVaultDeltaAssets * -1 != accountDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault and " @@ -3289,7 +3355,7 @@ ValidVault::finalize( } // We don't need to round shares, they are integral MPT auto const& accountDeltaShares = *maybeAccDeltaShares; - if (accountDeltaShares <= zero) + if (accountDeltaShares.delta <= zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must increase depositor " @@ -3299,7 +3365,8 @@ ValidVault::finalize( auto const maybeVaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!maybeVaultDeltaShares || *maybeVaultDeltaShares == zero) + if (!maybeVaultDeltaShares || + maybeVaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: deposit must change vault shares"; @@ -3308,7 +3375,7 @@ ValidVault::finalize( // We don't need to round shares, they are integral MPT auto const& vaultDeltaShares = *maybeVaultDeltaShares; - if (vaultDeltaShares * -1 != accountDeltaShares) + if (vaultDeltaShares.delta * -1 != accountDeltaShares.delta) { JLOG(j.fatal()) << // "Invariant failed: deposit must change depositor and " @@ -3316,18 +3383,22 @@ ValidVault::finalize( result = false; } - auto const assetTotalDelta = - afterVault.assetsTotal - beforeVault.assetsTotal; - if (!withinDistance(assetTotalDelta, vaultDeltaAssets)) + auto const assetTotalDelta = roundToAsset( + vaultAsset, + afterVault.assetsTotal - beforeVault.assetsTotal, + minScale); + if (assetTotalDelta != vaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: deposit and assets " "outstanding must add up"; result = false; } - auto const assetAvailableDelta = - afterVault.assetsAvailable - beforeVault.assetsAvailable; - if (!withinDistance(assetAvailableDelta, vaultDeltaAssets)) + auto const assetAvailableDelta = roundToAsset( + vaultAsset, + afterVault.assetsAvailable - beforeVault.assetsAvailable, + minScale); + if (assetAvailableDelta != vaultDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: deposit and assets " "available must add up"; @@ -3355,7 +3426,24 @@ ValidVault::finalize( return false; // That's all we can do } - auto const vaultPseudoDeltaAssets = *maybeVaultDeltaAssets; + // Get the most coarse scale to round calculations to + auto const totalDelta = DeltaInfo{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + afterVault.assetsTotal.scale(vaultAsset), + beforeVault.assetsTotal.scale(vaultAsset))}; + auto const availableDelta = DeltaInfo{ + afterVault.assetsAvailable - beforeVault.assetsAvailable, + std::max( + afterVault.assetsAvailable.scale(vaultAsset), + beforeVault.assetsAvailable.scale( + vaultAsset))}; + auto const minScale = computeMinScale( + vaultAsset, + {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + + auto const vaultPseudoDeltaAssets = roundToAsset( + vaultAsset, maybeVaultDeltaAssets->delta, minScale); if (vaultPseudoDeltaAssets >= zero) { @@ -3377,7 +3465,7 @@ ValidVault::finalize( { auto const maybeAccDelta = deltaAssetsTxAccount(); auto const maybeOtherAccDelta = - [&]() -> std::optional { + [&]() -> std::optional { if (auto const destination = tx[~sfDestination]; destination && *destination != tx[sfAccount]) return deltaAssets(*destination); @@ -3396,7 +3484,16 @@ ValidVault::finalize( auto const destinationDelta = // maybeAccDelta ? *maybeAccDelta : *maybeOtherAccDelta; - if (destinationDelta <= zero) + // the scale of destinationDelta can be coarser than + // minScale, so we take that into account when rounding + auto const localMinScale = std::max( + minScale, + computeMinScale(vaultAsset, {destinationDelta})); + + auto const roundedDestinationDelta = roundToAsset( + vaultAsset, destinationDelta.delta, localMinScale); + + if (roundedDestinationDelta <= zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must increase " @@ -3404,8 +3501,9 @@ ValidVault::finalize( result = false; } - if (!withinDistance( - vaultPseudoDeltaAssets * -1, destinationDelta)) + auto const localPseudoDeltaAssets = roundToAsset( + vaultAsset, vaultPseudoDeltaAssets, localMinScale); + if (localPseudoDeltaAssets * -1 != roundedDestinationDelta) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change vault " @@ -3423,7 +3521,7 @@ ValidVault::finalize( return false; } - if (*accountDeltaShares >= zero) + if (accountDeltaShares->delta >= zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must decrease depositor " @@ -3432,14 +3530,14 @@ ValidVault::finalize( } // We don't need to round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *accountDeltaShares) + if (vaultDeltaShares->delta * -1 != accountDeltaShares->delta) { JLOG(j.fatal()) << // "Invariant failed: withdrawal must change depositor " @@ -3447,20 +3545,24 @@ ValidVault::finalize( result = false; } - auto const assetTotalDelta = - afterVault.assetsTotal - beforeVault.assetsTotal; + auto const assetTotalDelta = roundToAsset( + vaultAsset, + afterVault.assetsTotal - beforeVault.assetsTotal, + minScale); // Note, vaultBalance is negative (see check above) - if (!withinDistance(assetTotalDelta, vaultPseudoDeltaAssets)) + if (assetTotalDelta != vaultPseudoDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: withdrawal and " "assets outstanding must add up"; result = false; } - auto const assetAvailableDelta = - afterVault.assetsAvailable - beforeVault.assetsAvailable; - if (!withinDistance( - assetAvailableDelta, vaultPseudoDeltaAssets)) + auto const assetAvailableDelta = roundToAsset( + vaultAsset, + afterVault.assetsAvailable - beforeVault.assetsAvailable, + minScale); + + if (assetAvailableDelta != vaultPseudoDeltaAssets) { JLOG(j.fatal()) << "Invariant failed: withdrawal and " "assets available must add up"; @@ -3498,7 +3600,25 @@ ValidVault::finalize( deltaAssets(afterVault.pseudoId); if (maybeVaultDeltaAssets) { - auto const vaultDeltaAssets = *maybeVaultDeltaAssets; + auto const totalDelta = DeltaInfo{ + afterVault.assetsTotal - beforeVault.assetsTotal, + std::max( + afterVault.assetsTotal.scale(vaultAsset), + beforeVault.assetsTotal.scale( + vaultAsset))}; + auto const availableDelta = DeltaInfo{ + afterVault.assetsAvailable - + beforeVault.assetsAvailable, + std::max( + afterVault.assetsAvailable.scale( + vaultAsset), + beforeVault.assetsAvailable.scale( + vaultAsset))}; + auto const minScale = computeMinScale( + vaultAsset, + {*maybeVaultDeltaAssets, totalDelta, availableDelta}); + auto const vaultDeltaAssets = roundToAsset( + vaultAsset, maybeVaultDeltaAssets->delta, minScale); if (vaultDeltaAssets >= zero) { JLOG(j.fatal()) << // @@ -3507,22 +3627,24 @@ ValidVault::finalize( result = false; } - auto const assetsTotalDelta = - afterVault.assetsTotal - beforeVault.assetsTotal; + auto const assetsTotalDelta = roundToAsset( + vaultAsset, + afterVault.assetsTotal - beforeVault.assetsTotal, + minScale); if (assetsTotalDelta != vaultDeltaAssets) - if (!withinDistance(assetsTotalDelta, vaultDeltaAssets)) - { - JLOG(j.fatal()) << // - "Invariant failed: clawback and assets " - "outstanding " - "must add up"; - result = false; - } + { + JLOG(j.fatal()) << // + "Invariant failed: clawback and assets outstanding " + "must add up"; + result = false; + } - auto const assetAvailableDelta = + auto const assetAvailableDelta = roundToAsset( + vaultAsset, afterVault.assetsAvailable - - beforeVault.assetsAvailable; - if (!withinDistance(assetAvailableDelta, vaultDeltaAssets)) + beforeVault.assetsAvailable, + minScale); + if (assetAvailableDelta != vaultDeltaAssets) { JLOG(j.fatal()) << // "Invariant failed: clawback and assets available " @@ -3545,7 +3667,7 @@ ValidVault::finalize( "Invariant failed: clawback must change holder shares"; return false; // That's all we can do } - if (*maybeAccountDeltaShares >= zero) + if (maybeAccountDeltaShares->delta >= zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must decrease holder " @@ -3555,14 +3677,15 @@ ValidVault::finalize( // We don't need to round shares, they are integral MPT auto const vaultDeltaShares = deltaShares(afterVault.pseudoId); - if (!vaultDeltaShares || *vaultDeltaShares == zero) + if (!vaultDeltaShares || vaultDeltaShares->delta == zero) { JLOG(j.fatal()) << // "Invariant failed: clawback must change vault shares"; return false; // That's all we can do } - if (*vaultDeltaShares * -1 != *maybeAccountDeltaShares) + if (vaultDeltaShares->delta * -1 != + maybeAccountDeltaShares->delta) { JLOG(j.fatal()) << // "Invariant failed: clawback must change holder and " @@ -3599,4 +3722,24 @@ ValidVault::finalize( return true; } + +[[nodiscard]] std::int32_t +ValidVault::computeMinScale( + Asset const& asset, + std::vector const& numbers) +{ + if (numbers.size() == 0) + return 0; + + auto const max = std::max_element( + numbers.begin(), + numbers.end(), + [](auto const& a, auto const& b) -> bool { return a.scale < b.scale; }); + XRPL_ASSERT_PARTS( + max->scale, + "xrpl::ValidVault::computeMinScale", + "scale set for destinationDelta"); + return max->scale.value_or(STAmount::cMaxOffset); +} + } // namespace xrpl diff --git a/src/xrpld/app/tx/detail/InvariantCheck.h b/src/xrpld/app/tx/detail/InvariantCheck.h index 83152f7485..4bc24c1adc 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.h +++ b/src/xrpld/app/tx/detail/InvariantCheck.h @@ -879,11 +879,19 @@ class ValidVault Shares static make(SLE const&); }; +public: + struct DeltaInfo final + { + Number delta = numZero; + std::optional scale; + }; + +private: std::vector afterVault_ = {}; std::vector afterMPTs_ = {}; std::vector beforeVault_ = {}; std::vector beforeMPTs_ = {}; - std::unordered_map deltas_ = {}; + std::unordered_map deltas_ = {}; public: void @@ -899,6 +907,10 @@ public: XRPAmount const, ReadView const&, beast::Journal const&); + + // Compute the coarsest scale required to represent all numbers + [[nodiscard]] static std::int32_t + computeMinScale(Asset const& asset, std::vector const& numbers); }; // additional invariant checks can be declared above and then added to this diff --git a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp index 5d4d2053ed..baafc6f2c0 100644 --- a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp +++ b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp @@ -125,7 +125,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx) tenthBipsOfValue( currentDebtTotal, TenthBips32(sleBroker->at(sfCoverRateMinimum))), - currentDebtTotal.exponent()); + currentDebtTotal.scale(vaultAsset)); }(); if (coverAvail < amount) return tecINSUFFICIENT_FUNDS;