ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-06-03 00:36:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Cap the base fee for LoanPay (#6969) (#6970)

Browse Source 
Co-authored-by: Bart <bthomee@users.noreply.github.com>
This commit is contained in:
Ed Hennis
2026-05-07 08:48:55 -04:00
committed by GitHub
parent d67e06102a
commit d6c4e6cb93
2 changed files with 132 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -141,6 +141,23 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx)
NumberRoundModeGuard const mg(
tx.isFlag(tfLoanOverpayment) ? Number::RoundingMode::Upward
: Number::RoundingMode::Downward);
static_assert(kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION % kLOAN_PAYMENTS_PER_FEE_INCREMENT == 0);
std::int64_t constexpr kMAX_FEE_INCREMENTS =
kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION / kLOAN_PAYMENTS_PER_FEE_INCREMENT;
if (view.rules().enabled(fixSecurity3_1_3) &&
amount >= regularPayment * kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION)
{
// The payment handler will never process more than
// loanMaximumPaymentsPerTransaction payments (including overpayments),
// and one fee increment is charged for every
// loanPaymentsPerFeeIncrement, so don't charge more than
// loanMaximumPaymentsPerTransaction / loanPaymentsPerFeeIncrement fee
// increments.
return kMAX_FEE_INCREMENTS * normalCost;
}
// Estimate how many payments will be made
Number const numPaymentEstimate = static_cast<std::int64_t>(amount / regularPayment);
@@ -149,6 +166,10 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx)
auto const feeIncrements = std::max(
std::int64_t(1),
static_cast<std::int64_t>(numPaymentEstimate / kLOAN_PAYMENTS_PER_FEE_INCREMENT));
XRPL_ASSERT(
!view.rules().enabled(fixSecurity3_1_3) || feeIncrements <= kMAX_FEE_INCREMENTS,
"xrpl::LoanPay::calculateBaseFee : number of fee increments is in "
"range");
return feeIncrements * normalCost;
}

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

@@ -2797,8 +2797,8 @@ protected:
pseudoAcct,
tfLoanOverpayment,
[&](Keylet const& loanKeylet, VerifyLoanStatus const& verifyLoanStatus) {
// Estimate optimal values for loanPaymentsPerFeeIncrement and
// loanMaximumPaymentsPerTransaction.
// Estimate optimal values for kLOAN_PAYMENTS_PER_FEE_INCREMENT and
// kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION.
using namespace loan;
auto const state = getCurrentState(env, broker, verifyLoanStatus.keylet);
@@ -2816,7 +2816,8 @@ protected:
// Make all but the final payment
auto const numPayments = (state.paymentRemaining - 2);
STAmount const bigPayment{broker.asset, totalDue * numPayments};
XRPAmount const bigFee{baseFee * (numPayments / loanPaymentsPerFeeIncrement + 1)};
XRPAmount const bigFee{
baseFee * (numPayments / kLOAN_PAYMENTS_PER_FEE_INCREMENT + 1)};
time("ten payments", [&]() {
env(pay(borrower, loanKeylet.key, bigPayment), Fee(bigFee));
});
@@ -4753,15 +4754,17 @@ protected:
}
void
testDosLoanPay()
testDosLoanPay(FeatureBitset features)
{
bool const feeCapped = features[fixSecurity3_1_3];
// From FIND-005
testcase << "DoS LoanPay";
testcase << "DoS LoanPay: fee calculation " << (feeCapped ? "capped" : "uncapped");
using namespace jtx;
using namespace std::chrono_literals;
using namespace Lending;
Env env(*this, all_);
Env env(*this, features);
Account const issuer{"issuer"};
Account const lender{"lender"};
@@ -4770,6 +4773,8 @@ protected:
env.fund(XRP(1'000'000), issuer, lender, borrower);
env.close();
BEAST_EXPECT(feeCapped == env.current()->rules().enabled(fixSecurity3_1_3));
PrettyAsset const iouAsset = issuer[iouCurrency_];
env(trust(lender, iouAsset(100'000'000)));
env(trust(borrower, iouAsset(100'000'000)));
@@ -4782,52 +4787,117 @@ protected:
using namespace loan;
auto const loanSetFee = Fee(env.current()->fees().base * 2);
Number const principalRequest{1, 3};
Number const principalRequest{3959'37, -2};
auto const baseFee = env.current()->fees().base;
auto createJson = env.json(
auto const createJson = env.json(
set(borrower, broker.brokerID, principalRequest),
Fee(loanSetFee),
Json(sfCounterpartySignature, json::ObjectValue));
createJson["ClosePaymentFee"] = "0";
createJson["GracePeriod"] = 60;
createJson["InterestRate"] = 20930;
createJson["LateInterestRate"] = 77049;
createJson["LatePaymentFee"] = "0";
createJson["LoanServiceFee"] = "0";
createJson["OverpaymentFee"] = 7;
createJson["OverpaymentInterestRate"] = 66653;
createJson["PaymentInterval"] = 60;
createJson["PaymentTotal"] = 3239184;
createJson["PrincipalRequested"] = "3959.37";
Json(sfCounterpartySignature, json::ObjectValue),
kCLOSE_PAYMENT_FEE(0),
kGRACE_PERIOD(60),
kINTEREST_RATE(TenthBips32(20930)),
kLATE_INTEREST_RATE(TenthBips32(77049)),
kLATE_PAYMENT_FEE(0),
kLOAN_SERVICE_FEE(0),
kOVERPAYMENT_FEE(TenthBips32(7)),
kOVERPAYMENT_INTEREST_RATE(TenthBips32(66653)),
kPAYMENT_INTERVAL(60),
kPAYMENT_TOTAL(3239184));
// There are enough payments due on this loan that it only needs to be
// created once, and can be paid on multiple times. Just don't create a
// gazillion test cases.
auto const brokerStateBefore = env.le(keylet::loanbroker(broker.brokerID));
auto const loanSequence = brokerStateBefore->at(sfLoanSequence);
auto const keylet = keylet::loan(broker.brokerID, loanSequence);
createJson = env.json(createJson, Sig(sfCounterpartySignature, lender));
env(createJson, Ter(tesSUCCESS));
env(createJson, Sig(sfCounterpartySignature, lender));
env.close();
auto const stateBefore = getCurrentState(env, broker, keylet);
BEAST_EXPECT(stateBefore.paymentRemaining == 3239184);
BEAST_EXPECT(stateBefore.paymentRemaining > kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION);
auto const roundedPayment = [&]() {
auto const stateBefore = getCurrentState(env, broker, keylet);
BEAST_EXPECT(stateBefore.paymentRemaining == 3239184);
BEAST_EXPECT(stateBefore.paymentRemaining > kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION);
auto loanPayTx = env.json(pay(borrower, keylet.key, STAmount{broker.asset, Number{}}));
Number const amount{395937, -2};
loanPayTx["Amount"]["value"] = to_string(amount);
XRPAmount const payFee{
baseFee *
std::int64_t(
amount / stateBefore.periodicPayment / kLOAN_PAYMENTS_PER_FEE_INCREMENT + 1)};
env(loanPayTx, Ter(tesSUCCESS), Fee(payFee));
env.close();
return roundToAsset(
iouAsset,
stateBefore.periodicPayment,
stateBefore.loanScale,
Number::RoundingMode::Upward);
}();
auto const stateAfter = getCurrentState(env, broker, keylet);
BEAST_EXPECT(
stateAfter.paymentRemaining ==
stateBefore.paymentRemaining - kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION);
auto test = [&](int const payFactor,
int const feeFactor,
TER const expectedTer = tesSUCCESS) {
auto const stateBefore = getCurrentState(env, broker, keylet);
BEAST_EXPECT(stateBefore.paymentRemaining <= 3239184);
BEAST_EXPECT(stateBefore.paymentRemaining > kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION);
Number const amount = roundedPayment * payFactor;
auto loanPayTx = env.json(pay(borrower, keylet.key, STAmount{broker.asset, amount}));
XRPAmount const payFee{baseFee * feeFactor};
env(loanPayTx, Ter(expectedTer), Fee(payFee));
env.close();
auto const expectedChange = isTesSuccess(expectedTer)
? std::min(kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION, payFactor)
: 0;
auto const stateAfter = getCurrentState(env, broker, keylet);
BEAST_EXPECT(
stateAfter.paymentRemaining == stateBefore.paymentRemaining - expectedChange);
};
std::int64_t constexpr kMAX_FEE_INCREMENTS =
kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION / kLOAN_PAYMENTS_PER_FEE_INCREMENT;
TER const failWithoutFix = feeCapped ? (TER)tesSUCCESS : (TER)telINSUF_FEE_P;
// * Amount well above threshold -> capped fee
// The original test case - way over the limit - more fee is always ok
test(1819878, 363976);
// The capped fee is only sufficient if the amendment is enabled.
test(1819878, kMAX_FEE_INCREMENTS, failWithoutFix);
// * Amount exactly at threshold -> capped fee
test(kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION, kMAX_FEE_INCREMENTS);
// More fee is always ok
test(kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION, kMAX_FEE_INCREMENTS + 10);
// * Amount below threshold -> normal calculation
test(1, 1);
test(kLOAN_PAYMENTS_PER_FEE_INCREMENT * 2, 2);
test(0, 0, temBAD_AMOUNT);
test(0, 1, temBAD_AMOUNT);
// Fee difference rounds evenly
test(
kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - 10,
((kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - 10) / kLOAN_PAYMENTS_PER_FEE_INCREMENT) - 1,
telINSUF_FEE_P);
test(
kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - 10,
((kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - 10) / kLOAN_PAYMENTS_PER_FEE_INCREMENT));
// More fee is always ok
test(
kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - 10,
((kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - 10) / kLOAN_PAYMENTS_PER_FEE_INCREMENT) + 3);
// Fee rounds up
for (int under = 1; under < kLOAN_PAYMENTS_PER_FEE_INCREMENT; ++under)
{
test(
kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - under,
kMAX_FEE_INCREMENTS - 1,
telINSUF_FEE_P);
test(kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - under, kMAX_FEE_INCREMENTS);
}
// Only when you get one less fee increment can you pay less
test(
kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - kLOAN_PAYMENTS_PER_FEE_INCREMENT,
kMAX_FEE_INCREMENTS - 1);
// And again, more fee is always ok.
test(
kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION - kLOAN_PAYMENTS_PER_FEE_INCREMENT,
kMAX_FEE_INCREMENTS);
}
void
@@ -7248,7 +7318,8 @@ public:
testLoanPayDebtDecreaseInvariant();
testWrongMaxDebtBehavior();
testLoanPayComputePeriodicPaymentValidTotalInterestInvariant();
testDosLoanPay();
testDosLoanPay(all | fixSecurity3_1_3);
testDosLoanPay(all - fixSecurity3_1_3);
testLoanPayComputePeriodicPaymentValidTotalPrincipalPaidInvariant();
testLoanPayComputePeriodicPaymentValidTotalInterestPaidInvariant();
testLoanNextPaymentDueDateOverflow();