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 GitHub
parent 8e05416211
commit c0ee813666
5 changed files with 133 additions and 49 deletions

View File

@@ -175,18 +175,32 @@ 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

@@ -386,21 +386,29 @@ 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

@@ -155,7 +155,7 @@ LoanPay::preclaim(PreclaimContext const& ctx)
if (tx.isFlag(tfLoanOverpayment) && !loanSle->isFlag(lsfLoanOverpayment))
{
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);

View File

@@ -2046,36 +2046,36 @@ class Invariants_test : public beast::unit_test::suite
{
// Initialize with a placeholder value because there's no default
// ctor
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) {
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 asset = setupAsset(alice, issuer, env);
loanBrokerKeylet = this->createLoanBroker(alice, env, asset);
return BEAST_EXPECT(env.le(loanBrokerKeylet));
};
@@ -2249,6 +2249,56 @@ 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

@@ -2071,7 +2071,19 @@ protected:
STAmount{broker.asset, state.periodicPayment * Number{15, -1}},
tfLoanOverpayment),
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