Compare commits

...

16 Commits

Author SHA1 Message Date
Ed Hennis
2f0690f3c5 Merge remote-tracking branch 'XRPLF/tapanito/lending-vault-invariant' into ximinez/number-scale
* XRPLF/tapanito/lending-vault-invariant:
  refactors vault invariant to use relative distance
  Limit reply size on `TMGetObjectByHash` queries (6110)
  ci: remove 'master' branch as a trigger (6234)
  Improve ledger_entry lookups for fee, amendments, NUNL, and hashes (5644)
2026-01-21 15:14:38 -05:00
Ed Hennis
040bf34257 ValidVault tracks scale of original operands alongside deltas 2026-01-21 14:21:58 -05:00
Vito Tumas
5c87c4ffb0 Merge branch 'develop' into tapanito/lending-vault-invariant 2026-01-21 18:58:09 +01:00
Vito
0a9436def4 refactors vault invariant to use relative distance 2026-01-21 18:57:33 +01:00
Ed Hennis
e5646e4ebe Merge remote-tracking branch 'XRPLF/tapanito/lending-vault-invariant' into ximinez/number-scale
* XRPLF/tapanito/lending-vault-invariant:
  flyby change removing unused includes
  addreses review comments
  adds invariant test
2026-01-21 12:48:48 -05:00
Vito
f76bf5340c flyby change removing unused includes 2026-01-21 11:50:38 +01:00
Vito
1af0f4bd43 addreses review comments 2026-01-21 11:50:18 +01:00
Vito
c6821ab842 adds invariant test 2026-01-21 11:42:03 +01:00
Ed Hennis
7ab9709373 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.
2026-01-20 20:06:45 -05:00
Vito
aa12210fcd fixes a minor min bug 2026-01-20 18:01:14 +01:00
Vito
9235ec483a adds missing incldues 2026-01-20 17:06:23 +01:00
Vito Tumas
ffe0a3cc61 Merge branch 'develop' into tapanito/lending-vault-invariant 2026-01-16 11:26:28 +01:00
Vito
add9071b20 fixes formatting 2026-01-16 11:26:12 +01:00
Vito Tumas
465e7b6d91 Merge branch 'develop' into tapanito/lending-vault-invariant 2026-01-15 16:10:25 +01:00
Vito
6223ebe05e improves VaultWithdraw invariant rounding 2026-01-15 16:09:13 +01:00
Vito
4fe50c2d31 attempt to fix rounding issues 2026-01-14 20:58:04 +01:00
8 changed files with 573 additions and 85 deletions

View File

@@ -109,6 +109,10 @@ template <class T>
concept Integral64 =
std::is_same_v<T, std::int64_t> || std::is_same_v<T, std::uint64_t>;
template <class STAmount, class Asset>
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 -
@@ -268,6 +272,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 <class STAmount, class Asset>
int
scale(Asset const& asset) const
requires CanUseAsScale<STAmount, Asset>;
constexpr Number
operator+() const noexcept;
constexpr Number
@@ -602,6 +626,14 @@ Number::exponent() const noexcept
return e;
}
template <class STAmount, class Asset>
int
Number::scale(Asset const& asset) const
requires CanUseAsScale<STAmount, Asset>
{
return STAmount{asset, *this}.exponent();
}
inline constexpr Number
Number::operator+() const noexcept
{

View File

@@ -4,6 +4,7 @@
#include <xrpld/app/tx/apply.h>
#include <xrpld/app/tx/detail/ApplyContext.h>
#include <xrpld/app/tx/detail/InvariantCheck.h>
#include <xrpl/beast/unit_test/suite.h>
#include <xrpl/beast/utility/Journal.h>
@@ -20,6 +21,9 @@
#include <boost/algorithm/string/predicate.hpp>
#include <initializer_list>
#include <string>
namespace xrpl {
namespace test {
@@ -3888,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::initializer_list<ValidVault::DeltaInfo const> values;
};
NumberMantissaScaleGuard g{MantissaRange::large};
auto makeDelta =
[&vaultAsset](Number const& n) -> ValidVault::DeltaInfo {
return {n, n.scale<STAmount>(vaultAsset.raw())};
};
auto const testCases = std::vector<TestCase>{
{
.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<TestCase>{
{
.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<Number> 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
@@ -3911,6 +4049,7 @@ public:
testValidPseudoAccounts();
testValidLoanBroker();
testVault();
testVaultComputeMinScale();
}
};

View File

@@ -3,16 +3,11 @@
#include <test/jtx.h>
#include <test/jtx/Account.h>
#include <test/jtx/amount.h>
#include <test/jtx/mpt.h>
#include <xrpld/app/misc/LendingHelpers.h>
#include <xrpld/app/misc/LoadFeeTrack.h>
#include <xrpld/app/tx/detail/Batch.h>
#include <xrpld/app/tx/detail/LoanSet.h>
#include <xrpl/beast/xor_shift_engine.h>
#include <xrpl/protocol/SField.h>
#include <string>
#include <vector>

View File

@@ -7641,6 +7641,149 @@ protected:
BEAST_EXPECT(afterSecondCoverAvailable == 0);
}
// Tests that vault withdrawals work correctly when the vault has unrealized
// loss from an impaired loan, ensuring the invariant check properly
// accounts for the loss.
void
testWithdrawReflectsUnrealizedLoss()
{
using namespace jtx;
using namespace loan;
using namespace std::chrono_literals;
testcase("Vault withdraw reflects sfLossUnrealized");
// Test constants
static constexpr std::int64_t INITIAL_FUNDING = 1'000'000;
static constexpr std::int64_t LENDER_INITIAL_IOU = 5'000'000;
static constexpr std::int64_t DEPOSITOR_INITIAL_IOU = 1'000'000;
static constexpr std::int64_t BORROWER_INITIAL_IOU = 100'000;
static constexpr std::int64_t DEPOSIT_AMOUNT = 5'000;
static constexpr std::int64_t PRINCIPAL_AMOUNT = 99;
static constexpr std::uint64_t EXPECTED_SHARES_PER_DEPOSITOR =
5'000'000'000;
static constexpr std::uint32_t PAYMENT_INTERVAL = 600;
static constexpr std::uint32_t PAYMENT_TOTAL = 2;
Env env(*this, all);
// Setup accounts
Account const issuer{"issuer"};
Account const lender{"lender"};
Account const depositorA{"lpA"};
Account const depositorB{"lpB"};
Account const borrower{"borrowerA"};
env.fund(
XRP(INITIAL_FUNDING),
issuer,
lender,
depositorA,
depositorB,
borrower);
env.close();
// Setup trust lines
PrettyAsset const iouAsset = issuer[iouCurrency];
env(trust(lender, iouAsset(10'000'000)));
env(trust(depositorA, iouAsset(10'000'000)));
env(trust(depositorB, iouAsset(10'000'000)));
env(trust(borrower, iouAsset(10'000'000)));
env.close();
// Fund accounts with IOUs
env(pay(issuer, lender, iouAsset(LENDER_INITIAL_IOU)));
env(pay(issuer, depositorA, iouAsset(DEPOSITOR_INITIAL_IOU)));
env(pay(issuer, depositorB, iouAsset(DEPOSITOR_INITIAL_IOU)));
env(pay(issuer, borrower, iouAsset(BORROWER_INITIAL_IOU)));
env.close();
// Create vault and broker, then add deposits from two depositors
auto const broker = createVaultAndBroker(env, iouAsset, lender);
Vault v{env};
env(v.deposit({
.depositor = depositorA,
.id = broker.vaultKeylet().key,
.amount = iouAsset(DEPOSIT_AMOUNT),
}),
ter(tesSUCCESS));
env(v.deposit({
.depositor = depositorB,
.id = broker.vaultKeylet().key,
.amount = iouAsset(DEPOSIT_AMOUNT),
}),
ter(tesSUCCESS));
env.close();
// Create a loan
auto const sleBroker = env.le(keylet::loanbroker(broker.brokerID));
if (!BEAST_EXPECT(sleBroker))
return;
auto const loanKeylet =
keylet::loan(broker.brokerID, sleBroker->at(sfLoanSequence));
env(set(borrower, broker.brokerID, PRINCIPAL_AMOUNT),
sig(sfCounterpartySignature, lender),
paymentTotal(PAYMENT_TOTAL),
paymentInterval(PAYMENT_INTERVAL),
fee(env.current()->fees().base * 2),
ter(tesSUCCESS));
env.close();
// Impair the loan to create unrealized loss
env(manage(lender, loanKeylet.key, tfLoanImpair), ter(tesSUCCESS));
env.close();
// Verify unrealized loss is recorded in the vault
auto const vaultAfterImpair = env.le(broker.vaultKeylet());
if (!BEAST_EXPECT(vaultAfterImpair))
return;
BEAST_EXPECT(
vaultAfterImpair->at(sfLossUnrealized) ==
broker.asset(PRINCIPAL_AMOUNT).value());
// Helper to get share balance for a depositor
auto const shareAsset = vaultAfterImpair->at(sfShareMPTID);
auto const getShareBalance =
[&](Account const& depositor) -> std::uint64_t {
auto const token =
env.le(keylet::mptoken(shareAsset, depositor.id()));
return token ? token->getFieldU64(sfMPTAmount) : 0;
};
// Verify both depositors have equal shares
auto const sharesLpA = getShareBalance(depositorA);
auto const sharesLpB = getShareBalance(depositorB);
BEAST_EXPECT(sharesLpA == EXPECTED_SHARES_PER_DEPOSITOR);
BEAST_EXPECT(sharesLpB == EXPECTED_SHARES_PER_DEPOSITOR);
BEAST_EXPECT(sharesLpA == sharesLpB);
// Helper to attempt withdrawal
auto const attemptWithdrawShares = [&](Account const& depositor,
std::uint64_t shareAmount,
TER expected) {
STAmount const shareAmt{MPTIssue{shareAsset}, Number(shareAmount)};
env(v.withdraw(
{.depositor = depositor,
.id = broker.vaultKeylet().key,
.amount = shareAmt}),
ter(expected));
env.close();
};
// Regression test: Both depositors should successfully withdraw despite
// unrealized loss. Previously failed with invariant violation:
// "withdrawal must change vault and destination balance by equal
// amount". This was caused by sharesToAssetsWithdraw rounding down,
// creating a mismatch where vaultDeltaAssets * -1 != destinationDelta
// when unrealized loss exists.
attemptWithdrawShares(depositorA, sharesLpA, tesSUCCESS);
attemptWithdrawShares(depositorB, sharesLpB, tesSUCCESS);
}
public:
void
run() override
@@ -7649,6 +7792,7 @@ public:
testLoanPayLateFullPaymentBypassesPenalties();
testLoanCoverMinimumRoundingExploit();
#endif
testWithdrawReflectsUnrealizedLoss();
testInvalidLoanSet();
testCoverDepositWithdrawNonTransferableMPT();

View File

@@ -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<STAmount>(vaultSle->at(sfAsset));
}
TER

View File

@@ -19,10 +19,11 @@
#include <xrpl/protocol/SystemParameters.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFormats.h>
#include <xrpl/protocol/Units.h>
#include <xrpl/protocol/nftPageMask.h>
#include <cstdint>
#include <algorithm>
#include <cstddef>
#include <initializer_list>
#include <optional>
namespace xrpl {
@@ -2664,7 +2665,7 @@ ValidVault::visitEntry(
// 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{};
DeltaInfo balanceDelta{numZero, STAmount::cMinOffset - 1};
std::int8_t sign = 0;
if (before)
@@ -2678,20 +2679,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<std::int64_t>(
balanceDelta.delta = static_cast<std::int64_t>(
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<std::int64_t>(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:;
}
}
@@ -2707,20 +2723,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<std::int64_t>(
balanceDelta.delta -= Number(static_cast<std::int64_t>(
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<std::int64_t>(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.
balanceDelta.scale =
std::max(balanceDelta.scale, amount.exponent());
sign = -1;
break;
}
default:;
}
}
@@ -2732,7 +2764,10 @@ 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;
{
balanceDelta.delta *= sign;
deltas_[key] = balanceDelta;
}
}
bool
@@ -3012,13 +3047,15 @@ ValidVault::finalize(
}
auto const& vaultAsset = afterVault.asset;
auto const deltaAssets = [&](AccountID const& id) -> std::optional<Number> {
auto const deltaAssets =
[&](AccountID const& id) -> std::optional<DeltaInfo> {
auto const get = //
[&](auto const& it, std::int8_t sign = 1) -> std::optional<Number> {
[&](auto const& it,
std::int8_t sign = 1) -> std::optional<DeltaInfo> {
if (it == deltas_.end())
return std::nullopt;
return it->second * sign;
return DeltaInfo{it->second.delta * sign, it->second.scale};
};
return std::visit(
@@ -3039,7 +3076,7 @@ ValidVault::finalize(
},
vaultAsset.value());
};
auto const deltaAssetsTxAccount = [&]() -> std::optional<Number> {
auto const deltaAssetsTxAccount = [&]() -> std::optional<DeltaInfo> {
auto ret = deltaAssets(tx[sfAccount]);
// Nothing returned or not XRP transaction
if (!ret.has_value() || !vaultAsset.native())
@@ -3050,13 +3087,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<Number> {
auto const deltaShares =
[&](AccountID const& id) -> std::optional<DeltaInfo> {
auto const it = [&]() {
if (id == afterVault.pseudoId)
return deltas_.find(
@@ -3064,7 +3102,7 @@ ValidVault::finalize(
return deltas_.find(keylet::mptoken(afterVault.shareMPTID, id).key);
}();
return it != deltas_.end() ? std::optional<Number>(it->second)
return it != deltas_.end() ? std::optional<DeltaInfo>(it->second)
: std::nullopt;
};
@@ -3196,16 +3234,41 @@ ValidVault::finalize(
"xrpl::ValidVault::finalize : deposit updated a vault");
auto const& beforeVault = beforeVault_[0];
auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId);
if (!vaultDeltaAssets)
auto const maybeVaultDeltaAssets =
deltaAssets(afterVault.pseudoId);
if (!maybeVaultDeltaAssets)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change vault balance";
return false; // That's all we can do
}
if (*vaultDeltaAssets > tx[sfAmount])
// Get the coarsest scale to round calculations to
DeltaInfo totalDelta{
afterVault.assetsTotal - beforeVault.assetsTotal,
std::max(
afterVault.assetsTotal.scale<STAmount>(vaultAsset),
beforeVault.assetsTotal.scale<STAmount>(vaultAsset))};
DeltaInfo availableDelta{
afterVault.assetsAvailable - beforeVault.assetsAvailable,
std::max(
afterVault.assetsAvailable.scale<STAmount>(vaultAsset),
beforeVault.assetsAvailable.scale<STAmount>(
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 "
@@ -3213,7 +3276,7 @@ ValidVault::finalize(
result = false;
}
if (*vaultDeltaAssets <= zero)
if (vaultDeltaAssets <= zero)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must increase vault balance";
@@ -3230,16 +3293,24 @@ ValidVault::finalize(
if (!issuerDeposit)
{
auto const accountDeltaAssets = deltaAssetsTxAccount();
if (!accountDeltaAssets)
auto const maybeAccDeltaAssets = deltaAssetsTxAccount();
if (!maybeAccDeltaAssets)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change depositor "
"balance";
return false;
}
auto const localMinScale = std::max(
minScale,
computeMinScale(vaultAsset, {*maybeAccDeltaAssets}));
if (*accountDeltaAssets >= zero)
auto const accountDeltaAssets = roundToAsset(
vaultAsset, maybeAccDeltaAssets->delta, localMinScale);
auto const localVaultDeltaAssets = roundToAsset(
vaultAsset, vaultDeltaAssets, localMinScale);
if (accountDeltaAssets >= zero)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must decrease depositor "
@@ -3247,7 +3318,7 @@ ValidVault::finalize(
result = false;
}
if (*accountDeltaAssets * -1 != *vaultDeltaAssets)
if (localVaultDeltaAssets * -1 != accountDeltaAssets)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change vault and "
@@ -3265,16 +3336,17 @@ ValidVault::finalize(
result = false;
}
auto const accountDeltaShares = deltaShares(tx[sfAccount]);
if (!accountDeltaShares)
auto const maybeAccDeltaShares = deltaShares(tx[sfAccount]);
if (!maybeAccDeltaShares)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change depositor "
"shares";
return false; // That's all we can do
}
if (*accountDeltaShares <= zero)
// We don't need to round shares, they are integral MPT
auto const& accountDeltaShares = *maybeAccDeltaShares;
if (accountDeltaShares.delta <= zero)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must increase depositor "
@@ -3282,15 +3354,19 @@ ValidVault::finalize(
result = false;
}
auto const vaultDeltaShares = deltaShares(afterVault.pseudoId);
if (!vaultDeltaShares || *vaultDeltaShares == zero)
auto const maybeVaultDeltaShares =
deltaShares(afterVault.pseudoId);
if (!maybeVaultDeltaShares ||
maybeVaultDeltaShares->delta == zero)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change vault shares";
return false; // That's all we can do
}
if (*vaultDeltaShares * -1 != *accountDeltaShares)
// We don't need to round shares, they are integral MPT
auto const& vaultDeltaShares = *maybeVaultDeltaShares;
if (vaultDeltaShares.delta * -1 != accountDeltaShares.delta)
{
JLOG(j.fatal()) << //
"Invariant failed: deposit must change depositor and "
@@ -3298,15 +3374,22 @@ ValidVault::finalize(
result = false;
}
if (beforeVault.assetsTotal + *vaultDeltaAssets !=
afterVault.assetsTotal)
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;
}
if (beforeVault.assetsAvailable + *vaultDeltaAssets !=
afterVault.assetsAvailable)
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";
@@ -3324,22 +3407,41 @@ ValidVault::finalize(
"vault");
auto const& beforeVault = beforeVault_[0];
auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId);
auto const maybeVaultDeltaAssets =
deltaAssets(afterVault.pseudoId);
if (!vaultDeltaAssets)
if (!maybeVaultDeltaAssets)
{
JLOG(j.fatal()) << "Invariant failed: withdrawal must "
"change vault balance";
return false; // That's all we can do
}
if (*vaultDeltaAssets >= zero)
// Get the most coarse scale to round calculations to
auto const totalDelta = DeltaInfo{
afterVault.assetsTotal - beforeVault.assetsTotal,
std::max(
afterVault.assetsTotal.scale<STAmount>(vaultAsset),
beforeVault.assetsTotal.scale<STAmount>(vaultAsset))};
auto const availableDelta = DeltaInfo{
afterVault.assetsAvailable - beforeVault.assetsAvailable,
std::max(
afterVault.assetsAvailable.scale<STAmount>(vaultAsset),
beforeVault.assetsAvailable.scale<STAmount>(
vaultAsset))};
auto const minScale = computeMinScale(
vaultAsset,
{*maybeVaultDeltaAssets, totalDelta, availableDelta});
auto const vaultPseudoDeltaAssets = roundToAsset(
vaultAsset, maybeVaultDeltaAssets->delta, minScale);
if (vaultPseudoDeltaAssets >= zero)
{
JLOG(j.fatal()) << "Invariant failed: withdrawal must "
"decrease vault balance";
result = false;
}
// Any payments (including withdrawal) going to the issuer
// do not change their balance, but destroy funds instead.
bool const issuerWithdrawal = [&]() -> bool {
@@ -3352,17 +3454,17 @@ ValidVault::finalize(
if (!issuerWithdrawal)
{
auto const accountDeltaAssets = deltaAssetsTxAccount();
auto const otherAccountDelta =
[&]() -> std::optional<Number> {
auto const maybeAccDelta = deltaAssetsTxAccount();
auto const maybeOtherAccDelta =
[&]() -> std::optional<DeltaInfo> {
if (auto const destination = tx[~sfDestination];
destination && *destination != tx[sfAccount])
return deltaAssets(*destination);
return std::nullopt;
}();
if (accountDeltaAssets.has_value() ==
otherAccountDelta.has_value())
if (maybeAccDelta.has_value() ==
maybeOtherAccDelta.has_value())
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must change one "
@@ -3371,10 +3473,18 @@ ValidVault::finalize(
}
auto const destinationDelta = //
accountDeltaAssets ? *accountDeltaAssets
: *otherAccountDelta;
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 "
@@ -3382,7 +3492,9 @@ ValidVault::finalize(
result = false;
}
if (*vaultDeltaAssets * -1 != destinationDelta)
auto const localPseudoDeltaAssets = roundToAsset(
vaultAsset, vaultPseudoDeltaAssets, localMinScale);
if (localPseudoDeltaAssets * -1 != roundedDestinationDelta)
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must change vault "
@@ -3390,7 +3502,7 @@ ValidVault::finalize(
result = false;
}
}
// We don't need to round shares, they are integral MPT
auto const accountDeltaShares = deltaShares(tx[sfAccount]);
if (!accountDeltaShares)
{
@@ -3400,23 +3512,23 @@ ValidVault::finalize(
return false;
}
if (*accountDeltaShares >= zero)
if (accountDeltaShares->delta >= zero)
{
JLOG(j.fatal()) << //
"Invariant failed: withdrawal must decrease depositor "
"shares";
result = false;
}
// 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 "
@@ -3424,17 +3536,24 @@ ValidVault::finalize(
result = false;
}
auto const assetTotalDelta = roundToAsset(
vaultAsset,
afterVault.assetsTotal - beforeVault.assetsTotal,
minScale);
// Note, vaultBalance is negative (see check above)
if (beforeVault.assetsTotal + *vaultDeltaAssets !=
afterVault.assetsTotal)
if (assetTotalDelta != vaultPseudoDeltaAssets)
{
JLOG(j.fatal()) << "Invariant failed: withdrawal and "
"assets outstanding must add up";
result = false;
}
if (beforeVault.assetsAvailable + *vaultDeltaAssets !=
afterVault.assetsAvailable)
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";
@@ -3468,10 +3587,30 @@ ValidVault::finalize(
}
}
auto const vaultDeltaAssets = deltaAssets(afterVault.pseudoId);
if (vaultDeltaAssets)
auto const maybeVaultDeltaAssets =
deltaAssets(afterVault.pseudoId);
if (maybeVaultDeltaAssets)
{
if (*vaultDeltaAssets >= zero)
auto const totalDelta = DeltaInfo{
afterVault.assetsTotal - beforeVault.assetsTotal,
std::max(
afterVault.assetsTotal.scale<STAmount>(vaultAsset),
beforeVault.assetsTotal.scale<STAmount>(
vaultAsset))};
auto const availableDelta = DeltaInfo{
afterVault.assetsAvailable -
beforeVault.assetsAvailable,
std::max(
afterVault.assetsAvailable.scale<STAmount>(
vaultAsset),
beforeVault.assetsAvailable.scale<STAmount>(
vaultAsset))};
auto const minScale = computeMinScale(
vaultAsset,
{*maybeVaultDeltaAssets, totalDelta, availableDelta});
auto const vaultDeltaAssets = roundToAsset(
vaultAsset, maybeVaultDeltaAssets->delta, minScale);
if (vaultDeltaAssets >= zero)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback must decrease vault "
@@ -3479,8 +3618,11 @@ ValidVault::finalize(
result = false;
}
if (beforeVault.assetsTotal + *vaultDeltaAssets !=
afterVault.assetsTotal)
auto const assetsTotalDelta = roundToAsset(
vaultAsset,
afterVault.assetsTotal - beforeVault.assetsTotal,
minScale);
if (assetsTotalDelta != vaultDeltaAssets)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback and assets outstanding "
@@ -3488,8 +3630,12 @@ ValidVault::finalize(
result = false;
}
if (beforeVault.assetsAvailable + *vaultDeltaAssets !=
afterVault.assetsAvailable)
auto const assetAvailableDelta = roundToAsset(
vaultAsset,
afterVault.assetsAvailable -
beforeVault.assetsAvailable,
minScale);
if (assetAvailableDelta != vaultDeltaAssets)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback and assets available "
@@ -3504,15 +3650,15 @@ ValidVault::finalize(
return false; // That's all we can do
}
auto const accountDeltaShares = deltaShares(tx[sfHolder]);
if (!accountDeltaShares)
// We don't need to round shares, they are integral MPT
auto const maybeAccountDeltaShares = deltaShares(tx[sfHolder]);
if (!maybeAccountDeltaShares)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback must change holder shares";
return false; // That's all we can do
}
if (*accountDeltaShares >= zero)
if (maybeAccountDeltaShares->delta >= zero)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback must decrease holder "
@@ -3520,15 +3666,17 @@ ValidVault::finalize(
result = false;
}
// 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 != *accountDeltaShares)
if (vaultDeltaShares->delta * -1 !=
maybeAccountDeltaShares->delta)
{
JLOG(j.fatal()) << //
"Invariant failed: clawback must change holder and "
@@ -3566,4 +3714,22 @@ ValidVault::finalize(
return true;
}
[[nodiscard]] std::int32_t
ValidVault::computeMinScale(
Asset const& asset,
std::initializer_list<DeltaInfo const> numbers)
{
if (numbers.size() == 0)
return 0;
std::vector<std::int32_t> natScales;
std::transform(
numbers.begin(),
numbers.end(),
std::back_inserter(natScales),
[&](DeltaInfo const& n) { return n.scale; });
return *std::max_element(natScales.begin(), natScales.end());
}
} // namespace xrpl

View File

@@ -9,7 +9,6 @@
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>
#include <cstdint>
#include <tuple>
#include <unordered_set>
@@ -880,11 +879,19 @@ class ValidVault
Shares static make(SLE const&);
};
public:
struct DeltaInfo final
{
Number delta = numZero;
int scale = 0;
};
private:
std::vector<Vault> afterVault_ = {};
std::vector<Shares> afterMPTs_ = {};
std::vector<Vault> beforeVault_ = {};
std::vector<Shares> beforeMPTs_ = {};
std::unordered_map<uint256, Number> deltas_ = {};
std::unordered_map<uint256, DeltaInfo> deltas_ = {};
public:
void
@@ -900,6 +907,12 @@ 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::initializer_list<DeltaInfo const> numbers);
};
// additional invariant checks can be declared above and then added to this

View File

@@ -125,7 +125,7 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx)
tenthBipsOfValue(
currentDebtTotal,
TenthBips32(sleBroker->at(sfCoverRateMinimum))),
currentDebtTotal.exponent());
currentDebtTotal.scale<STAmount>(vaultAsset));
}();
if (coverAvail < amount)
return tecINSUFFICIENT_FUNDS;