fix: Unblock VaultSet and cash-basis LoanSet at AssetsMaximum (#8143)

This commit is contained in:
Vito Tumas
2026-09-01 14:06:17 +00:00
committed by GitHub
parent de6e5d3a94
commit b2453b626e
5 changed files with 327 additions and 18 deletions

View File

@@ -380,7 +380,7 @@ ValidVault::finalize(
beast::Journal const& j)
{
bool const enforce = view.rules().enabled(featureSingleAssetVault);
bool const fixEnabled = view.rules().enabled(fixCleanup3_4_0);
bool const fix340Enabled = view.rules().enabled(fixCleanup3_4_0);
if (!isTesSuccess(ret))
return true; // Do not perform checks
@@ -572,7 +572,7 @@ ValidVault::finalize(
else
{
bool const gapExceeded = [&] {
if (!fixEnabled)
if (!fix340Enabled)
{
return afterVault.lossUnrealized >
afterVault.assetsTotal - afterVault.assetsAvailable;
@@ -594,7 +594,7 @@ ValidVault::finalize(
}
}
if (fixEnabled && afterVault.lossUnrealized < kZero)
if (fix340Enabled && afterVault.lossUnrealized < kZero)
{
JLOG(j.fatal()) << "Invariant failed: loss unrealized must not be negative";
result = false;
@@ -765,8 +765,13 @@ ValidVault::finalize(
result = false;
}
// AssetsTotal may exceed AssetsMaximum when the excess is interest. After
// fixCleanup3_4_0, only reject a VaultSet that supplies sfAssetsMaximum or
// otherwise changes the cap to a nonzero value still below AssetsTotal.
if (afterVault.assetsMaximum > kZero &&
afterVault.assetsTotal > afterVault.assetsMaximum)
afterVault.assetsTotal > afterVault.assetsMaximum &&
(!fix340Enabled || tx.isFieldPresent(sfAssetsMaximum) ||
beforeVault.assetsMaximum != afterVault.assetsMaximum))
{
JLOG(j.fatal()) << //
"Invariant failed: set assets outstanding must not "
@@ -880,7 +885,7 @@ ValidVault::finalize(
result = false;
}
bool const acctVaultAddsUp = fixEnabled
bool const acctVaultAddsUp = fix340Enabled
? agreesWithinOneUnit(
localVaultDeltaAssets * -1,
accountDeltaAssets,
@@ -935,7 +940,7 @@ ValidVault::finalize(
auto const assetTotalDelta = roundToAsset(
vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale);
bool const totalAddsUp = fixEnabled
bool const totalAddsUp = fix340Enabled
? agreesWithinOneUnit(assetTotalDelta, vaultDeltaAssets, vaultAsset, minScale)
: assetTotalDelta == vaultDeltaAssets;
if (!totalAddsUp)
@@ -947,7 +952,7 @@ ValidVault::finalize(
auto const assetAvailableDelta = roundToAsset(
vaultAsset, afterVault.assetsAvailable - beforeVault.assetsAvailable, minScale);
bool const availableAddsUp = fixEnabled
bool const availableAddsUp = fix340Enabled
? agreesWithinOneUnit(
assetAvailableDelta, vaultDeltaAssets, vaultAsset, minScale)
: assetAvailableDelta == vaultDeltaAssets;
@@ -993,7 +998,7 @@ ValidVault::finalize(
// value merely rounds down to zero, so a missing delta while
// the pool still held positive effective value indicates a
// real accounting bug, not this exception.
bool const zeroDeltaIsLegitimate = fixEnabled && !maybeVaultDeltaAssets &&
bool const zeroDeltaIsLegitimate = fix340Enabled && !maybeVaultDeltaAssets &&
beforeVault.assetsTotal == beforeVault.lossUnrealized;
if (!maybeVaultDeltaAssets && !zeroDeltaIsLegitimate)
@@ -1100,7 +1105,7 @@ ValidVault::finalize(
vaultDeltaAssets.delta * -1 - destinationDelta.delta,
destinationScale,
Number::RoundingMode::Downward) == kZero;
bool const withdrawAddsUp = fixEnabled
bool const withdrawAddsUp = fix340Enabled
? agreesWithinOneUnit(
localPseudoDeltaAssets * -1,
roundedDestinationDelta,
@@ -1150,7 +1155,7 @@ ValidVault::finalize(
auto const assetTotalDelta = roundToAsset(
vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale);
// Note, vaultBalance is negative (see check above)
bool const totalAddsUp = fixEnabled
bool const totalAddsUp = fix340Enabled
? agreesWithinOneUnit(
assetTotalDelta, vaultPseudoDeltaAssets, vaultAsset, minScale)
: assetTotalDelta == vaultPseudoDeltaAssets;
@@ -1164,7 +1169,7 @@ ValidVault::finalize(
auto const assetAvailableDelta = roundToAsset(
vaultAsset, afterVault.assetsAvailable - beforeVault.assetsAvailable, minScale);
bool const availableAddsUp = fixEnabled
bool const availableAddsUp = fix340Enabled
? agreesWithinOneUnit(
assetAvailableDelta, vaultPseudoDeltaAssets, vaultAsset, minScale)
: assetAvailableDelta == vaultPseudoDeltaAssets;
@@ -1213,7 +1218,7 @@ ValidVault::finalize(
auto const assetsTotalDelta = roundToAsset(
vaultAsset, afterVault.assetsTotal - beforeVault.assetsTotal, minScale);
bool const totalAddsUp = fixEnabled
bool const totalAddsUp = fix340Enabled
? agreesWithinOneUnit(
assetsTotalDelta, vaultDeltaAssets, vaultAsset, minScale)
: assetsTotalDelta == vaultDeltaAssets;
@@ -1228,7 +1233,7 @@ ValidVault::finalize(
vaultAsset,
afterVault.assetsAvailable - beforeVault.assetsAvailable,
minScale);
bool const availableAddsUp = fixEnabled
bool const availableAddsUp = fix340Enabled
? agreesWithinOneUnit(
assetAvailableDelta, vaultDeltaAssets, vaultAsset, minScale)
: assetAvailableDelta == vaultDeltaAssets;

View File

@@ -336,7 +336,12 @@ LoanSet::preclaim(PreclaimContext const& ctx)
}
}
if (vault->at(sfAssetsMaximum) != 0 && vault->at(sfAssetsTotal) >= vault->at(sfAssetsMaximum))
// Accrual origination credits interestDue into AssetsTotal, so a vault
// already at AssetsMaximum cannot take another loan. Cash-basis origination
// does not change AssetsTotal (see cash_basis::loanOriginationDeltas), so
// this leftover accrual gate must not apply there.
if (getVaultVersion(vault) != VaultVersion::CashBasis && vault->at(sfAssetsMaximum) != 0 &&
vault->at(sfAssetsTotal) >= vault->at(sfAssetsMaximum))
{
JLOG(ctx.j.warn()) << "Vault at maximum assets limit. Can't add another loan.";
return tecLIMIT_EXCEEDED;
@@ -467,9 +472,11 @@ LoanSet::doApply()
properties.loanState.managementFeeDue);
XRPL_ASSERT_PARTS(
*vaultSle->at(sfAssetsMaximum) == 0 || *vaultSle->at(sfAssetsMaximum) > *vaultTotalProxy,
*vaultSle->at(sfAssetsMaximum) == 0 ||
getVaultVersion(vaultSle) == VaultVersion::CashBasis ||
*vaultSle->at(sfAssetsMaximum) > *vaultTotalProxy,
"xrpl::LoanSet::doApply",
"Vault is below maximum limit");
"accrual vault is below maximum limit");
if (loanOriginationExceedsVaultMaximum(vaultSle, vaultTotalProxy, state.interestDue))
{

View File

@@ -865,6 +865,42 @@ class InvariantsVault_test : public InvariantsBase
precloseXrp,
TxAccount::A2);
// The cap check has two post-fixCleanup3_4_0 triggers: the transaction
// supplied sfAssetsMaximum, or the cap changed. The case above covers
// the cap-changed one (its ttVAULT_SET carries no fields). This covers
// the other: the cap is left alone at 30 XRP and the transaction
// carries sfAssetsMaximum, so only the isFieldPresent disjunct can
// fire. AssetsTotal is pushed past the cap here rather than in
// preclose because VaultSet::doApply refuses to set a cap below
// AssetsTotal, so the over-cap state is only reachable by fabrication.
// Raising AssetsTotal also trips the "must not change assets
// outstanding" check, hence two expected messages.
Number const vaultCap = XRP(30).number();
doInvariantCheck(
{"set must not change assets outstanding",
"set assets outstanding must not exceed assets maximum"},
[&](Account const& a1, Account const& a2, ApplyContext& ac) {
auto const keylet = keylet::vault(a1.id(), SeqProxy::rawSequence(ac.view().seq()));
return kAdjust(ac.view(), keylet, kArgs(a2.id(), 0, [&](Adjustments& sample) {
sample.assetsTotal = XRP(1).value().xrp().drops();
}));
},
XRPAmount{},
STTx{ttVAULT_SET, [&](STObject& tx) { tx[sfAssetsMaximum] = vaultCap; }},
{tecINVARIANT_FAILED, tecINVARIANT_FAILED},
[&](Account const& a1, Account const& a2, Env& env) -> bool {
env.fund(XRP(1000), a3, a4);
Vault const vault{env};
auto [tx, keylet] = vault.create({.owner = a1, .asset = xrpIssue()});
tx[sfAssetsMaximum] = vaultCap;
env(tx);
env(vault.deposit({.depositor = a1, .id = keylet.key, .amount = XRP(10)}));
env(vault.deposit({.depositor = a2, .id = keylet.key, .amount = XRP(10)}));
env(vault.deposit({.depositor = a3, .id = keylet.key, .amount = XRP(10)}));
return true;
},
TxAccount::A2);
doInvariantCheck(
{"assets maximum must not be negative"},
[&](Account const& a1, Account const& a2, ApplyContext& ac) {

View File

@@ -4,10 +4,12 @@
#include <test/jtx/TestHelpers.h>
#include <test/jtx/amount.h>
#include <test/jtx/fee.h>
#include <test/jtx/permissioned_domains.h>
#include <test/jtx/ter.h>
#include <test/jtx/vault.h>
#include <xrpl/basics/Number.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/basics/chrono.h>
#include <xrpl/beast/unit_test/suite.h>
#include <xrpl/beast/utility/Zero.h>
@@ -25,6 +27,7 @@
#include <chrono>
#include <cstdint>
#include <functional>
#include <string>
#include <tuple>
namespace xrpl::test {
@@ -42,8 +45,9 @@ class LoanCashBasis_test : public LoanTestBase
{
private:
// 1. LoanSet origination: Vault.AssetsTotal/LoanBroker.DebtTotal deltas,
// and the AssetsMaximum/DebtMaximum guards (which always check against
// principal + interestDue, regardless of the amendment).
// and the AssetsMaximum/DebtMaximum guards. Accrual AssetsMaximum still
// requires headroom for interestDue; cash-basis AssetsMaximum does not,
// because origination does not credit interest into AssetsTotal.
void
testCashBasisLoanSetOrigination()
{
@@ -226,6 +230,10 @@ private:
// Even far less headroom than interestDue still succeeds, since
// cash-basis origination never adds interest to AssetsTotal.
runVaultGuard(all_ | featureLendingProtocolV1_1, oneDrop, tesSUCCESS);
// Fully subscribed: AssetsTotal == AssetsMaximum. Accrual preclaim
// used to refuse this; origination must still succeed because it
// does not change AssetsTotal.
runVaultGuard(all_ | featureLendingProtocolV1_1, Number{0}, tesSUCCESS);
}
// DebtMaximum guard: cash-basis projects principal-only DebtTotal;
@@ -489,6 +497,252 @@ private:
}
}
// VaultSet must still succeed when cash-basis LoanPay has already pushed
// AssetsTotal above a nonzero AssetsMaximum. Before fixCleanup3_4_0,
// ValidVault rejects that with tecINVARIANT_FAILED even though
// VaultSet::doApply and the product rule allow the over-cap state when
// the excess is interest.
void
testVaultSetWhileAssetsTotalExceedsMaximum()
{
using namespace jtx;
using namespace loan;
PrettyAsset const xrpAsset{xrpIssue(), 1'000'000};
BrokerParameters const brokerParams{
.vaultDeposit = 1'000'000,
.debtMax = 0,
.coverRateMin = TenthBips32{0},
.coverDeposit = 0,
.managementFeeRate = TenthBips16{0},
.coverRateLiquidation = TenthBips32{0}};
auto run =
[&](FeatureBitset features, TER expectedOverCapSet, bool native, bool vaultPrivate) {
bool const fix340Enabled = features[fixCleanup3_4_0];
testcase(
std::string("cash-basis: VaultSet while AssetsTotal exceeds AssetsMaximum") +
(native ? " XRP" : " IOU") + (vaultPrivate ? " private" : "") +
(fix340Enabled ? " (fixCleanup3_4_0)" : " (pre-fix)"));
Account const issuer{"issuer"};
Account const lender{"lender"};
Account const borrower{"borrower"};
Env env(*this, features);
BrokerParameters params = brokerParams;
if (vaultPrivate)
params.vaultFlags = tfVaultPrivate;
PrettyAsset vaultAsset = xrpAsset;
if (native)
{
env.fund(XRP(10'000'000), lender, borrower);
env.close();
}
else
{
vaultAsset = createFundedIouAsset(env, issuer, lender, borrower);
}
BrokerInfo const broker{createVaultAndBroker(env, vaultAsset, lender, params)};
auto const vaultBefore = env.le(broker.vaultKeylet());
BEAST_EXPECT(vaultBefore);
// One unit at the vault's asset scale so the stored cap is
// strictly above AssetsTotal (a smaller ULP rounds away).
// Cash-basis origination does not credit interest, so LoanSet
// still succeeds.
Number const slack{1, -static_cast<int>(vaultBefore->at(sfScale))};
Number const assetsMaximum = Number(vaultBefore->at(sfAssetsTotal)) + slack;
Vault const vault{env};
{
auto tx = vault.set({.owner = lender, .id = broker.vaultID});
tx[sfAssetsMaximum] = assetsMaximum;
env(tx);
env.close();
}
{
auto tx = vault.set({.owner = lender, .id = broker.vaultID});
tx[sfData] = "AA";
env(tx, Ter(tesSUCCESS));
env.close();
}
auto const brokerBeforeLoan = env.le(broker.brokerKeylet());
BEAST_EXPECT(brokerBeforeLoan);
auto const loanKeylet = keylet::loan(
broker.brokerID, SeqProxy::rawSequence(brokerBeforeLoan->at(sfLoanSequence)));
LoanParameters const loanParams{
.account = borrower,
.counter = lender,
.principalRequest = 12'000,
.interest = TenthBips32{percentageToTenthBips(12)},
.payTotal = 4,
.payInterval = 600,
.gracePd = 300,
};
env(loanParams(env, broker));
env.close();
auto const vaultAfterLoan = env.le(broker.vaultKeylet());
BEAST_EXPECT(vaultAfterLoan);
BEAST_EXPECT(vaultAfterLoan->at(sfAssetsTotal) <= assetsMaximum);
LoanState const state = getCurrentState(env, broker, loanKeylet);
STAmount const payment{
vaultAsset,
roundPeriodicPayment(vaultAsset, state.periodicPayment, state.loanScale) *
Number{3, -1} * 5};
env(pay(borrower, loanKeylet.key, payment), Ter(tesSUCCESS));
env.close();
auto const vaultAboveMaximum = env.le(broker.vaultKeylet());
BEAST_EXPECT(vaultAboveMaximum);
BEAST_EXPECT(vaultAboveMaximum->at(sfAssetsTotal) > assetsMaximum);
BEAST_EXPECT(vaultAboveMaximum->at(sfAssetsMaximum) == assetsMaximum);
{
auto tx = vault.set({.owner = lender, .id = broker.vaultID});
tx[sfData] = "BB";
env(tx, Ter(expectedOverCapSet));
env.close();
}
if (vaultPrivate)
{
pdomain::Credentials const credentials{
{.issuer = lender, .credType = "credential"}};
env(pdomain::setTx(lender, credentials));
auto const domainId = pdomain::getNewDomain(env.meta());
auto tx = vault.set({.owner = lender, .id = broker.vaultID});
tx[sfDomainID] = to_string(domainId);
env(tx, Ter(expectedOverCapSet));
env.close();
}
if (!fix340Enabled)
return;
{
auto tx = vault.set({.owner = lender, .id = broker.vaultID});
tx[sfAssetsMaximum] = assetsMaximum;
env(tx, Ter(tecLIMIT_EXCEEDED));
env.close();
}
{
auto tx = vault.set({.owner = lender, .id = broker.vaultID});
tx[sfAssetsMaximum] = Number{0};
env(tx, Ter(tesSUCCESS));
env.close();
}
};
FeatureBitset const withFix = all_ | featureLendingProtocolV1_1;
FeatureBitset const withoutFix = withFix - fixCleanup3_4_0;
run(withFix, tesSUCCESS, true, true);
run(withoutFix, tecINVARIANT_FAILED, true, true);
run(withFix, tesSUCCESS, false, false);
run(withoutFix, tecINVARIANT_FAILED, false, false);
}
void
testCashBasisLoanSetAfterInterestExceedsCap()
{
testcase("cash-basis: LoanSet after interest pushes AssetsTotal past AssetsMaximum");
using namespace jtx;
using namespace loan;
PrettyAsset const xrpAsset{xrpIssue(), 1'000'000};
BrokerParameters const brokerParams{
.vaultDeposit = 1'000'000,
.debtMax = 0,
.coverRateMin = TenthBips32{0},
.coverDeposit = 0,
.managementFeeRate = TenthBips16{0},
.coverRateLiquidation = TenthBips32{0}};
Account const lender{"lender"};
Account const borrower{"borrower"};
Env env(*this, all_ | featureLendingProtocolV1_1);
env.fund(XRP(10'000'000), lender, borrower);
env.close();
BrokerInfo const broker{createVaultAndBroker(env, xrpAsset, lender, brokerParams)};
auto const vaultBefore = env.le(broker.vaultKeylet());
BEAST_EXPECT(vaultBefore);
Number const assetsMaximum = Number(vaultBefore->at(sfAssetsTotal));
Vault const vault{env};
{
auto tx = vault.set({.owner = lender, .id = broker.vaultID});
tx[sfAssetsMaximum] = assetsMaximum;
env(tx);
env.close();
}
auto const brokerBeforeLoan = env.le(broker.brokerKeylet());
BEAST_EXPECT(brokerBeforeLoan);
auto const firstLoanKeylet = keylet::loan(
broker.brokerID, SeqProxy::rawSequence(brokerBeforeLoan->at(sfLoanSequence)));
Number const firstPrincipal = xrpAsset(12'000).value();
env(set(borrower, broker.brokerID, firstPrincipal),
kCounterparty(lender),
kInterestRate(TenthBips32{percentageToTenthBips(12)}),
kPaymentTotal(4),
kPaymentInterval(600),
Sig(sfCounterpartySignature, lender),
Fee(env.current()->fees().base * 2),
Ter(tesSUCCESS));
env.close();
auto const vaultAfterFirst = env.le(broker.vaultKeylet());
BEAST_EXPECT(vaultAfterFirst);
BEAST_EXPECT(vaultAfterFirst->at(sfAssetsTotal) == assetsMaximum);
BEAST_EXPECT(vaultAfterFirst->at(sfAssetsAvailable) == assetsMaximum - firstPrincipal);
LoanState const state = getCurrentState(env, broker, firstLoanKeylet);
STAmount const payment{
xrpAsset,
roundPeriodicPayment(xrpAsset, state.periodicPayment, state.loanScale) * Number{3, -1} *
5};
env(pay(borrower, firstLoanKeylet.key, payment), Ter(tesSUCCESS));
env.close();
auto const vaultAfterPay = env.le(broker.vaultKeylet());
BEAST_EXPECT(vaultAfterPay);
BEAST_EXPECT(vaultAfterPay->at(sfAssetsTotal) > assetsMaximum);
BEAST_EXPECT(vaultAfterPay->at(sfAssetsAvailable) > beast::kZero);
auto const brokerAfterPay = env.le(broker.brokerKeylet());
BEAST_EXPECT(brokerAfterPay);
auto const secondLoanKeylet = keylet::loan(
broker.brokerID, SeqProxy::rawSequence(brokerAfterPay->at(sfLoanSequence)));
Number const secondPrincipal = xrpAsset(1'000).value();
env(set(borrower, broker.brokerID, secondPrincipal),
kCounterparty(lender),
kInterestRate(TenthBips32{percentageToTenthBips(12)}),
kPaymentTotal(4),
kPaymentInterval(600),
Sig(sfCounterpartySignature, lender),
Fee(env.current()->fees().base * 2),
Ter(tesSUCCESS));
env.close();
auto const vaultAfterSecond = env.le(broker.vaultKeylet());
auto const secondLoan = env.le(secondLoanKeylet);
BEAST_EXPECT(vaultAfterSecond && secondLoan);
BEAST_EXPECT(vaultAfterSecond->at(sfAssetsTotal) == vaultAfterPay->at(sfAssetsTotal));
BEAST_EXPECT(secondLoan->at(sfPrincipalOutstanding) == secondPrincipal);
}
// 3. LoanManage: impair, unimpair, and default.
void
testCashBasisLoanManage()
@@ -1013,6 +1267,8 @@ public:
{
testCashBasisLoanSetOrigination();
testCashBasisLoanPay();
testVaultSetWhileAssetsTotalExceedsMaximum();
testCashBasisLoanSetAfterInterestExceedsCap();
testCashBasisLoanManage();
testLegacyVaultKeepsAccrualAfterAmendmentEnabled();
testCashBasisEndToEndTrajectory();

View File

@@ -101,6 +101,10 @@ protected:
TenthBips32 coverRateLiquidation = percentageToTenthBips(25);
std::string data = {}; // NOLINT(readability-redundant-member-init)
std::uint32_t flags = 0;
// VaultCreate flags (e.g. tfVaultPrivate). Distinct from `flags`,
// which are passed to LoanBrokerSet.
std::optional<std::uint32_t> vaultFlags =
std::nullopt; // NOLINT(readability-redundant-member-init)
// If set, the vault is created with this sfScale value. Useful for
// tests that need finer loanScale to exercise rounding edge cases.
std::optional<std::uint8_t> vaultScale =
@@ -526,6 +530,7 @@ protected:
auto [tx, vaultKeylet] = vault.create(
{.owner = lender,
.asset = asset,
.flags = params.vaultFlags,
.vaultKind = effectiveVaultKind == VaultKind::OpenEnded
? std::optional<std::uint8_t>{}
: std::optional<std::uint8_t>{std::to_underlying(effectiveVaultKind)},