fix: Add assorted Lending Protocol fixes (#6678)

Co-authored-by: Shawn Xie <35279399+shawnxie999@users.noreply.github.com>
This commit is contained in:
Vito Tumas
2026-04-03 19:41:45 +02:00
committed by Ed Hennis
parent dd99ecc656
commit c979643d01
5 changed files with 153 additions and 54 deletions

View File

@@ -1939,39 +1939,41 @@ class Invariants_test : public beast::unit_test::suite
{
// Initialize with a placeholder value because there's no default
// ctor
Keylet loanBrokerKeylet = keylet::amendments();
Preclose createLoanBroker = [&, this](
Account const& alice,
Account const& issuer,
Env& env) {
PrettyAsset const asset = [&]() {
switch (assetType)
{
case Asset::IOU: {
PrettyAsset const iouAsset = issuer["IOU"];
env(trust(alice, iouAsset(1000)));
env(pay(issuer, alice, iouAsset(1000)));
env.close();
return iouAsset;
}
case Asset::MPT: {
MPTTester mptt{env, issuer, mptInitNoFund};
mptt.create(
{.flags = tfMPTCanClawback | tfMPTCanTransfer |
tfMPTCanLock});
PrettyAsset const mptAsset = mptt.issuanceID();
mptt.authorize({.account = alice});
env(pay(issuer, alice, mptAsset(1000)));
env.close();
return mptAsset;
}
case Asset::XRP:
default:
return PrettyAsset{xrpIssue(), 1'000'000};
auto const setupAsset = [&](Account const& alice,
Account const& issuer,
Env& env) -> PrettyAsset {
switch (assetType)
{
case Asset::IOU: {
PrettyAsset const iouAsset = issuer["IOU"];
env(trust(alice, iouAsset(1000)));
env(pay(issuer, alice, iouAsset(1000)));
env.close();
return iouAsset;
}
}();
case Asset::MPT: {
MPTTester mptt{env, issuer, mptInitNoFund};
mptt.create(
{.flags = tfMPTCanClawback | tfMPTCanTransfer |
tfMPTCanLock});
PrettyAsset const mptAsset = mptt.issuanceID();
mptt.authorize({.account = alice});
env(pay(issuer, alice, mptAsset(1000)));
env.close();
return mptAsset;
}
case Asset::XRP:
default:
return PrettyAsset{xrpIssue(), 1'000'000};
}
};
Keylet loanBrokerKeylet = keylet::amendments();
Preclose const createLoanBroker = [&, this](
Account const& alice,
Account const& issuer,
Env& env) {
auto const asset = setupAsset(alice, issuer, env);
loanBrokerKeylet = this->createLoanBroker(alice, env, asset);
return BEAST_EXPECT(env.le(loanBrokerKeylet));
};
@@ -2159,6 +2161,61 @@ class Invariants_test : public beast::unit_test::suite
STTx{ttLOAN_BROKER_SET, [](STObject& tx) {}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
createLoanBroker);
// Test: cover available less than pseudo-account asset balance
{
Keylet brokerKeylet = keylet::amendments();
Preclose const createBrokerWithCover =
[&, this](
Account const& alice, Account const& issuer, Env& env) {
auto const asset = setupAsset(alice, issuer, env);
brokerKeylet =
this->createLoanBroker(alice, env, asset);
if (!BEAST_EXPECT(env.le(brokerKeylet)))
return false;
env(loanBroker::coverDeposit(
alice, brokerKeylet.key, asset(10)));
env.close();
return BEAST_EXPECT(env.le(brokerKeylet));
};
doInvariantCheck(
{{"Loan Broker cover available is less than pseudo-account "
"asset balance"}},
[&](Account const&, Account const&, ApplyContext& ac) {
auto sle = ac.view().peek(brokerKeylet);
if (!BEAST_EXPECT(sle))
return false;
// Pseudo-account holds 10 units, set cover to 5
sle->at(sfCoverAvailable) = Number(5);
ac.view().update(sle);
return true;
},
XRPAmount{},
STTx{ttLOAN_BROKER_SET, [](STObject& tx) {}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
createBrokerWithCover);
}
// Test: cover available greater than pseudo-account asset balance
// (requires fixSecurity3_1_3)
doInvariantCheck(
{{"Loan Broker cover available is greater than pseudo-account "
"asset balance"}},
[&](Account const&, Account const&, ApplyContext& ac) {
auto sle = ac.view().peek(loanBrokerKeylet);
if (!BEAST_EXPECT(sle))
return false;
// Pseudo-account has no cover deposited; set cover
// higher than any incidental balance
sle->at(sfCoverAvailable) = Number(1'000'000);
ac.view().update(sle);
return true;
},
XRPAmount{},
STTx{ttLOAN_BROKER_SET, [](STObject& tx) {}},
{tecINVARIANT_FAILED, tefINVARIANT_FAILED},
createLoanBroker);
}
}

View File

@@ -2273,7 +2273,23 @@ protected:
fee(XRPAmount{
baseFee *
(Number{15, -1} / loanPaymentsPerFeeIncrement + 1)}),
ter(temINVALID_FLAG));
ter(tecNO_PERMISSION));
{
env.disableFeature(fixSecurity3_1_3);
env(pay(borrower,
loanKeylet.key,
STAmount{
broker.asset,
state.periodicPayment * Number{15, -1}},
tfLoanOverpayment),
fee(XRPAmount{
baseFee *
(Number{15, -1} / loanPaymentsPerFeeIncrement +
1)}),
ter(temINVALID_FLAG));
env.enableFeature(fixSecurity3_1_3);
}
}
// Try to send a payment marked as multiple mutually exclusive
// payment types. Do not include `txFlags`, so we don't duplicate

View File

@@ -2532,18 +2532,33 @@ ValidLoanBroker::finalize(
return false;
}
auto const& vaultAsset = vault->at(sfAsset);
if (after->at(sfCoverAvailable) < accountHolds(
view,
after->at(sfAccount),
vaultAsset,
FreezeHandling::fhIGNORE_FREEZE,
AuthHandling::ahIGNORE_AUTH,
j))
auto const pseudoBalance = accountHolds(
view,
after->at(sfAccount),
vaultAsset,
FreezeHandling::fhIGNORE_FREEZE,
AuthHandling::ahIGNORE_AUTH,
j);
if (after->at(sfCoverAvailable) < pseudoBalance)
{
JLOG(j.fatal()) << "Invariant failed: Loan Broker cover available "
"is less than pseudo-account asset balance";
return false;
}
if (view.rules().enabled(fixSecurity3_1_3))
{
// Don't check the balance when LoanBroker is deleted,
// sfCoverAvailable is not zeroed
if (tx.getTxnType() != ttLOAN_BROKER_DELETE &&
after->at(sfCoverAvailable) > pseudoBalance)
{
JLOG(j.fatal()) << "Invariant failed: Loan Broker cover "
"available is greater "
"than pseudo-account asset balance";
return false;
}
}
}
return true;
}

View File

@@ -418,21 +418,30 @@ LoanManage::doApply()
return tefBAD_LEDGER; // LCOV_EXCL_LINE
auto const vaultAsset = vaultSle->at(sfAsset);
// Valid flag combinations are checked in preflight. No flags is valid -
// just a noop.
if (tx.isFlag(tfLoanDefault))
return defaultLoan(view, loanSle, brokerSle, vaultSle, vaultAsset, j_);
if (tx.isFlag(tfLoanImpair))
return impairLoan(view, loanSle, vaultSle, vaultAsset, j_);
if (tx.isFlag(tfLoanUnimpair))
return unimpairLoan(view, loanSle, vaultSle, vaultAsset, j_);
// Noop, as described above.
auto const result = [&]() -> TER {
// Valid flag combinations are checked in preflight. No flags is valid -
// just a noop.
if (tx.isFlag(tfLoanDefault))
return defaultLoan(
view, loanSle, brokerSle, vaultSle, vaultAsset, j_);
if (tx.isFlag(tfLoanImpair))
return impairLoan(view, loanSle, vaultSle, vaultAsset, j_);
if (tx.isFlag(tfLoanUnimpair))
return unimpairLoan(view, loanSle, vaultSle, vaultAsset, j_);
// Noop, as described above.
return tesSUCCESS;
}();
associateAsset(*loanSle, vaultAsset);
associateAsset(*brokerSle, vaultAsset);
associateAsset(*vaultSle, vaultAsset);
// Pre-amendment, associateAsset was only called on the noop (no flags)
// path. Post-amendment, we call associateAsset on all successful paths.
if (view.rules().enabled(fixSecurity3_1_3) && isTesSuccess(result))
{
associateAsset(*loanSle, vaultAsset);
associateAsset(*brokerSle, vaultAsset);
associateAsset(*vaultSle, vaultAsset);
}
return tesSUCCESS;
return result;
}
//------------------------------------------------------------------------------

View File

@@ -149,7 +149,9 @@ LoanPay::preclaim(PreclaimContext const& ctx)
{
JLOG(ctx.j.warn())
<< "Requested overpayment on a loan that doesn't allow it";
return temINVALID_FLAG;
return ctx.view.rules().enabled(fixSecurity3_1_3)
? TER{tecNO_PERMISSION}
: temINVALID_FLAG;
}
auto const principalOutstanding = loanSle->at(sfPrincipalOutstanding);