ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-24 23:50:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Conserve funds correctly when LoanPay fee payee is below reserve

Browse Source 
LoanPay's fund-conservation check read XRP balances via
accountHolds(SpendableHandling::FullBalance), which for XRP always
defers to xrpLiquid (balance minus reserve, clamped at zero). When
the broker fee landed on a payee sitting below its own reserve, the
payee's clamped balance stayed zero and the fee vanished from the
conservation sum, tripping the "funds are conserved" assert on an
otherwise valid payment.
This commit is contained in:
Vito
2026-07-22 17:52:38 +02:00
parent 042c9660cd
commit bad77604ed
2 changed files with 139 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

85
src/libxrpl/tx/transactors/lending/LoanPay.cpp
View File

@@ -33,6 +33,34 @@
namespace xrpl {
namespace {
// Returns the account's true, unclamped balance in `asset`, for use only in
// fund-conservation checks. accountHolds(..., SpendableHandling::FullBalance)
// cannot be used for this: for XRP it always defers to xrpLiquid, which
// subtracts the account's reserve, so a payee sitting below its own reserve
// would appear to receive nothing even though its raw ledger balance grew.
// That mismatch is exactly what a conservation check must not see.
STAmount
conservationBalance(ReadView const& view, AccountID const& id, Asset const& asset, beast::Journal j)
{
if (isXRP(asset))
{
auto const sle = view.read(keylet::account(id));
if (!sle)
return STAmount{};
return view.balanceHookIOU(id, xrpAccount(), sle->getFieldAmount(sfBalance));
}
return accountHolds(
view,
id,
asset,
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j,
SpendableHandling::FullBalance);
}
} // namespace
bool
LoanPay::checkExtraFeatures(PreflightContext const& ctx)
{
@@ -584,34 +612,13 @@ LoanPay::doApply()
}
// These three values are used to check that funds are conserved after the transfers
auto const accountBalanceBefore = accountHolds(
view,
accountID_,
asset,
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j_,
SpendableHandling::FullBalance);
auto const accountBalanceBefore = conservationBalance(view, accountID_, asset, j_);
auto const vaultBalanceBefore = accountID_ == vaultPseudoAccount
? STAmount{asset, 0}
: accountHolds(
view,
vaultPseudoAccount,
asset,
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j_,
SpendableHandling::FullBalance);
: conservationBalance(view, vaultPseudoAccount, asset, j_);
auto const brokerBalanceBefore = accountID_ == brokerPayee
? STAmount{asset, 0}
: accountHolds(
view,
brokerPayee,
asset,
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j_,
SpendableHandling::FullBalance);
: conservationBalance(view, brokerPayee, asset, j_);
if (totalPaidToVaultRounded != beast::kZero)
{
@@ -667,33 +674,13 @@ LoanPay::doApply()
#endif
// Check that funds are conserved
auto const accountBalanceAfter = accountHolds(
view,
accountID_,
asset,
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j_,
SpendableHandling::FullBalance);
auto const accountBalanceAfter = conservationBalance(view, accountID_, asset, j_);
auto const vaultBalanceAfter = accountID_ == vaultPseudoAccount
? STAmount{asset, 0}
: accountHolds(
view,
vaultPseudoAccount,
asset,
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j_,
SpendableHandling::FullBalance);
auto const brokerBalanceAfter = accountID_ == brokerPayee ? STAmount{asset, 0}
: accountHolds(
view,
brokerPayee,
asset,
FreezeHandling::IgnoreFreeze,
AuthHandling::IgnoreAuth,
j_,
SpendableHandling::FullBalance);
: conservationBalance(view, vaultPseudoAccount, asset, j_);
auto const brokerBalanceAfter = accountID_ == brokerPayee
? STAmount{asset, 0}
: conservationBalance(view, brokerPayee, asset, j_);
auto const balanceScale = [&]() {
// Find a reasonable scale to use for the balance comparisons.
//

103
src/test/app/Loan_test.cpp
View File

@@ -8547,6 +8547,107 @@ protected:
});
}
void
testLoanPayFundsConservedPayeeBelowReserve(FeatureBitset features)
{
// Regression test: LoanPay::doApply's fund-conservation check used to
// read XRP balances via accountHolds(..., SpendableHandling::
// FullBalance), which for XRP always defers to xrpLiquid (balance
// minus reserve, clamped at zero). When the broker fee landed on a
// payee sitting below its own reserve, that payee's clamped balance
// stayed zero and the fee vanished from the conservation sum,
// tripping "funds are conserved (with rounding)".
testcase << "LoanPay funds conserved: broker fee payee below reserve";
using namespace jtx;
Env env(*this, features);
Account const issuer{"issuer"};
Account const lender{"lender"};
Account const borrower{"borrower"};
// Broker defaults match the fuzz workload: ManagementFeeRate = 100
// tenth-bips. The service fee guarantees feePaid > 0 on the first
// regular payment.
BrokerParameters const brokerParams;
Number const serviceFeeValue{2};
LoanParameters const loanParams{
.account = borrower,
.counter = lender,
.principalRequest = 1000,
.serviceFee = serviceFeeValue,
.interest = TenthBips32{percentageToTenthBips(12)},
.payTotal = 12,
.payInterval = 3600};
auto const loanOpt =
createLoan(env, AssetType::XRP, brokerParams, loanParams, issuer, lender, borrower);
if (!BEAST_EXPECT(loanOpt))
return;
auto const& [broker, loanKeylet, brokerPseudo] = *loanOpt;
auto const vaultPseudo = [&]() {
auto const vaultSle = env.le(keylet::vault(broker.vaultID));
BEAST_EXPECT(vaultSle);
return vaultSle ? vaultSle->at(sfAccount) : AccountID{};
}();
// Raw AccountRoot balance, matching LoanPay::doApply's conservation
// check (not the reserve-clamped accountHolds()/xrpLiquid() value).
auto rawBalance = [&](AccountID const& id) -> STAmount {
auto const sle = env.le(keylet::account(id));
return sle ? sle->getFieldAmount(sfBalance) : STAmount{};
};
auto lenderReserve = [&] {
return env.current()->fees().accountReserve(ownerCount(env, lender), 1);
};
STAmount const baseFee{env.current()->fees().base};
// Park the lender (broker owner, fee payee) exactly at its reserve,
// then burn part of the reserve with an oversized transaction fee.
// Fees are exempt from the reserve check, so the balance ends up
// below the reserve.
env(pay(lender, issuer, rawBalance(lender.id()) - lenderReserve() - baseFee));
env(noop(lender), Fee(XRP(100)));
env.close();
BEAST_EXPECT(env.balance(lender) < lenderReserve());
// First regular payment, exactly the amount due.
auto const state = getCurrentState(env, broker, loanKeylet);
STAmount const serviceFee = broker.asset(serviceFeeValue);
STAmount const roundedPeriodicPayment{
broker.asset,
roundPeriodicPayment(broker.asset, state.periodicPayment, state.loanScale)};
STAmount const totalDue = roundToScale(
roundedPeriodicPayment + serviceFee, state.loanScale, Number::RoundingMode::Upward);
auto const borrowerBefore = rawBalance(borrower.id());
auto const vaultBefore = rawBalance(vaultPseudo);
auto const lenderBefore = rawBalance(lender.id());
// Before the fix, this aborted inside LoanPay::doApply on
// XRPL_ASSERT_PARTS(goodRounding, "xrpl::LoanPay::doApply", "funds
// are conserved (with rounding)").
env(loan::pay(borrower, loanKeylet.key, totalDue));
env.close();
auto const borrowerAfter = rawBalance(borrower.id());
auto const vaultAfter = rawBalance(vaultPseudo);
auto const lenderAfter = rawBalance(lender.id());
// The broker fee reached the lender's AccountRoot, even though the
// lender's balance remains below its reserve.
BEAST_EXPECT(lenderAfter > lenderBefore);
BEAST_EXPECT(lenderAfter < lenderReserve());
// Total funds conserved across the payer, vault, and fee payee.
BEAST_EXPECT(
borrowerBefore - baseFee + vaultBefore + lenderBefore ==
borrowerAfter + vaultAfter + lenderAfter);
}
void
runAmendmentIndependent()
{
@@ -8580,6 +8681,8 @@ protected:
testLoanPayLateFullPaymentBypassesPenalties(features);
testLoanCoverMinimumRoundingExploit(features);
#endif
testLoanPayFundsConservedPayeeBelowReserve(features);
// Lifecycle
testLifecycle(features);
testLoanSet(features);