Polish the code

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

src/test/app/LendingHelpers_test.cpp

@@ -0,0 +1,642 @@
+#include <xrpl/beast/unit_test/suite.h>
+// DO NOT REMOVE
+#include <test/jtx.h>
+#include <test/jtx/Account.h>
+#include <test/jtx/amount.h>
+#include <test/jtx/mpt.h>
+
+#include <xrpld/app/misc/LendingHelpers.h>
+#include <xrpld/app/misc/LoadFeeTrack.h>
+#include <xrpld/app/tx/detail/Batch.h>
+#include <xrpld/app/tx/detail/LoanSet.h>
+
+#include <xrpl/beast/xor_shift_engine.h>
+#include <xrpl/protocol/SField.h>
+
+#include <string>
+#include <vector>
+
+namespace ripple {
+namespace test {
+
+class LendingHelpers_test : public beast::unit_test::suite
+{
+    void
+    testComputeRaisedRate()
+    {
+        using namespace jtx;
+        using namespace ripple::detail;
+        struct TestCase
+        {
+            std::string name;
+            Number periodicRate;
+            std::uint32_t paymentsRemaining;
+            Number expectedRaisedRate;
+        };
+
+        auto const testCases = std::vector<TestCase>{
+            {
+                .name = "Zero payments remaining",
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 0,
+                .expectedRaisedRate = Number{1},  // (1 + r)^0 = 1
+            },
+            {
+                .name = "One payment remaining",
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 1,
+                .expectedRaisedRate = Number{105, -2},
+            },  // 1.05^1
+            {
+                .name = "Multiple payments remaining",
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 3,
+                .expectedRaisedRate = Number{1157625, -6},
+            },  // 1.05^3
+            {
+                .name = "Zero periodic rate",
+                .periodicRate = Number{0},
+                .paymentsRemaining = 5,
+                .expectedRaisedRate = Number{1},  // (1 + 0)^5 = 1
+            }};
+
+        for (auto const& tc : testCases)
+        {
+            testcase("computeRaisedRate: " + tc.name);
+
+            auto const computedRaisedRate =
+                computeRaisedRate(tc.periodicRate, tc.paymentsRemaining);
+            BEAST_EXPECTS(
+                computedRaisedRate == tc.expectedRaisedRate,
+                "Raised rate mismatch: expected " +
+                    to_string(tc.expectedRaisedRate) + ", got " +
+                    to_string(computedRaisedRate));
+        }
+    }
+
+    void
+    testComputePaymentFactor()
+    {
+        using namespace jtx;
+        using namespace ripple::detail;
+        struct TestCase
+        {
+            std::string name;
+            Number periodicRate;
+            std::uint32_t paymentsRemaining;
+            Number expectedPaymentFactor;
+        };
+
+        auto const testCases = std::vector<TestCase>{
+            {
+                .name = "Zero periodic rate",
+                .periodicRate = Number{0},
+                .paymentsRemaining = 4,
+                .expectedPaymentFactor = Number{25, -2},
+            },  // 1/4 = 0.25
+            {
+                .name = "One payment remaining",
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 1,
+                .expectedPaymentFactor = Number{105, -2},
+            },  // 0.05/1 = 1.05
+            {
+                .name = "Multiple payments remaining",
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 3,
+                .expectedPaymentFactor = Number{367208564631245, -15},
+            },  // from calc
+            {
+                .name = "Zero payments remaining",
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 0,
+                .expectedPaymentFactor = Number{0},
+            }  // edge case
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("computePaymentFactor: " + tc.name);
+
+            auto const computedPaymentFactor =
+                computePaymentFactor(tc.periodicRate, tc.paymentsRemaining);
+            BEAST_EXPECTS(
+                computedPaymentFactor == tc.expectedPaymentFactor,
+                "Payment factor mismatch: expected " +
+                    to_string(tc.expectedPaymentFactor) + ", got " +
+                    to_string(computedPaymentFactor));
+        }
+    }
+
+    void
+    testLoanPeriodicPayment()
+    {
+        using namespace jtx;
+        using namespace ripple::detail;
+
+        struct TestCase
+        {
+            std::string name;
+            Number principalOutstanding;
+            Number periodicRate;
+            std::uint32_t paymentsRemaining;
+            Number expectedPeriodicPayment;
+        };
+
+        auto const testCases = std::vector<TestCase>{
+            {
+                .name = "Zero principal outstanding",
+                .principalOutstanding = Number{0},
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 5,
+                .expectedPeriodicPayment = Number{0},
+            },
+            {
+                .name = "Zero payments remaining",
+                .principalOutstanding = Number{1'000},
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 0,
+                .expectedPeriodicPayment = Number{0},
+            },
+            {
+                .name = "Zero periodic rate",
+                .principalOutstanding = Number{1'000},
+                .periodicRate = Number{0},
+                .paymentsRemaining = 4,
+                .expectedPeriodicPayment = Number{250},
+            },
+            {
+                .name = "Standard case",
+                .principalOutstanding = Number{1'000},
+                .periodicRate =
+                    loanPeriodicRate(TenthBips32(100'000), 30 * 24 * 60 * 60),
+                .paymentsRemaining = 3,
+                .expectedPeriodicPayment =
+                    Number{3895690663961231, -13},  // from calc
+            },
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("loanPeriodicPayment: " + tc.name);
+
+            auto const computedPeriodicPayment = loanPeriodicPayment(
+                tc.principalOutstanding, tc.periodicRate, tc.paymentsRemaining);
+            BEAST_EXPECTS(
+                computedPeriodicPayment == tc.expectedPeriodicPayment,
+                "Periodic payment mismatch: expected " +
+                    to_string(tc.expectedPeriodicPayment) + ", got " +
+                    to_string(computedPeriodicPayment));
+        }
+    }
+
+    void
+    testLoanPrincipalFromPeriodicPayment()
+    {
+        using namespace jtx;
+        using namespace ripple::detail;
+
+        struct TestCase
+        {
+            std::string name;
+            Number periodicPayment;
+            Number periodicRate;
+            std::uint32_t paymentsRemaining;
+            Number expectedPrincipalOutstanding;
+        };
+
+        auto const testCases = std::vector<TestCase>{
+            {
+                .name = "Zero periodic payment",
+                .periodicPayment = Number{0},
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 5,
+                .expectedPrincipalOutstanding = Number{0},
+            },
+            {
+                .name = "Zero payments remaining",
+                .periodicPayment = Number{1'000},
+                .periodicRate = Number{5, -2},
+                .paymentsRemaining = 0,
+                .expectedPrincipalOutstanding = Number{0},
+            },
+            {
+                .name = "Zero periodic rate",
+                .periodicPayment = Number{250},
+                .periodicRate = Number{0},
+                .paymentsRemaining = 4,
+                .expectedPrincipalOutstanding = Number{1'000},
+            },
+            {
+                .name = "Standard case",
+                .periodicPayment = Number{3895690663961231, -13},  // from calc
+                .periodicRate =
+                    loanPeriodicRate(TenthBips32(100'000), 30 * 24 * 60 * 60),
+                .paymentsRemaining = 3,
+                .expectedPrincipalOutstanding = Number{1'000},
+            },
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("loanPrincipalFromPeriodicPayment: " + tc.name);
+
+            auto const computedPrincipalOutstanding =
+                loanPrincipalFromPeriodicPayment(
+                    tc.periodicPayment, tc.periodicRate, tc.paymentsRemaining);
+            BEAST_EXPECTS(
+                computedPrincipalOutstanding == tc.expectedPrincipalOutstanding,
+                "Principal outstanding mismatch: expected " +
+                    to_string(tc.expectedPrincipalOutstanding) + ", got " +
+                    to_string(computedPrincipalOutstanding));
+        }
+    }
+
+    void
+    testComputeOverpaymentComponents()
+    {
+        testcase("computeOverpaymentComponents");
+        using namespace jtx;
+        using namespace ripple::detail;
+
+        Account const issuer{"issuer"};
+        PrettyAsset const IOU = issuer["IOU"];
+        int32_t const loanScale = 1;
+        auto const overpayment = Number{1'000};
+        auto const overpaymentInterestRate = TenthBips32{10'000};  // 10%
+        auto const overpaymentFeeRate = TenthBips32{50'000};       // 50%
+        auto const managementFeeRate = TenthBips16{10'000};        // 10%
+
+        auto const expectedOverpaymentFee = Number{500};  // 50% of 1,000
+        auto const expectedOverpaymentInterestGross =
+            Number{100};  // 10% of 1,000
+        auto const expectedOverpaymentInterestNet =
+            Number{90};  // 100 - 10% of 100
+        auto const expectedOverpaymentManagementFee = Number{10};  // 10% of 100
+        auto const expectedPrincipalPortion = Number{400};  // 1,000 - 100 - 500
+
+        auto const components = detail::computeOverpaymentComponents(
+            IOU,
+            loanScale,
+            overpayment,
+            overpaymentInterestRate,
+            overpaymentFeeRate,
+            managementFeeRate);
+
+        BEAST_EXPECT(
+            components.untrackedManagementFee == expectedOverpaymentFee);
+
+        BEAST_EXPECT(
+            components.untrackedInterest == expectedOverpaymentInterestNet);
+        BEAST_EXPECT(
+            components.trackedManagementFeeDelta ==
+            expectedOverpaymentManagementFee);
+        BEAST_EXPECT(
+            components.trackedPrincipalDelta == expectedPrincipalPortion);
+        BEAST_EXPECT(
+            components.trackedManagementFeeDelta +
+                components.untrackedInterest ==
+            expectedOverpaymentInterestGross);
+
+        BEAST_EXPECT(
+            components.trackedManagementFeeDelta +
+                components.untrackedInterest +
+                components.trackedPrincipalDelta +
+                components.untrackedManagementFee ==
+            overpayment);
+    }
+
+    void
+    testComputeInterestAndFeeParts()
+    {
+        using namespace jtx;
+        using namespace ripple::detail;
+
+        struct TestCase
+        {
+            std::string name;
+            Number interest;
+            TenthBips16 managementFeeRate;
+            Number expectedInterestPart;
+            Number expectedFeePart;
+        };
+
+        Account const issuer{"issuer"};
+        PrettyAsset const IOU = issuer["IOU"];
+        std::int32_t const loanScale = 1;
+
+        auto const testCases = std::vector<TestCase>{
+            {.name = "Zero interest",
+             .interest = Number{0},
+             .managementFeeRate = TenthBips16{10'000},
+             .expectedInterestPart = Number{0},
+             .expectedFeePart = Number{0}},
+            {.name = "Zero fee rate",
+             .interest = Number{1'000},
+             .managementFeeRate = TenthBips16{0},
+             .expectedInterestPart = Number{1'000},
+             .expectedFeePart = Number{0}},
+            {.name = "10% fee rate",
+             .interest = Number{1'000},
+             .managementFeeRate = TenthBips16{10'000},
+             .expectedInterestPart = Number{900},
+             .expectedFeePart = Number{100}},
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("computeInterestAndFeeParts: " + tc.name);
+
+            auto const [computedInterestPart, computedFeePart] =
+                computeInterestAndFeeParts(
+                    IOU, tc.interest, tc.managementFeeRate, loanScale);
+            BEAST_EXPECTS(
+                computedInterestPart == tc.expectedInterestPart,
+                "Interest part mismatch: expected " +
+                    to_string(tc.expectedInterestPart) + ", got " +
+                    to_string(computedInterestPart));
+            BEAST_EXPECTS(
+                computedFeePart == tc.expectedFeePart,
+                "Fee part mismatch: expected " + to_string(tc.expectedFeePart) +
+                    ", got " + to_string(computedFeePart));
+        }
+    }
+
+    void
+    testLoanLatePaymentInterest()
+    {
+        using namespace jtx;
+        using namespace ripple::detail;
+        struct TestCase
+        {
+            std::string name;
+            Number principalOutstanding;
+            TenthBips32 lateInterestRate;
+            NetClock::time_point parentCloseTime;
+            std::uint32_t nextPaymentDueDate;
+            Number expectedLateInterest;
+        };
+
+        auto const testCases = std::vector<TestCase>{
+            {
+                .name = "On-time payment",
+                .principalOutstanding = Number{1'000},
+                .lateInterestRate = TenthBips32{10'000},  // 10%
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .nextPaymentDueDate = 3'000,
+                .expectedLateInterest = Number{0},
+            },
+            {
+                .name = "Early payment",
+                .principalOutstanding = Number{1'000},
+                .lateInterestRate = TenthBips32{10'000},  // 10%
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .nextPaymentDueDate = 4'000,
+                .expectedLateInterest = Number{0},
+            },
+            {
+                .name = "No principal outstanding",
+                .principalOutstanding = Number{0},
+                .lateInterestRate = TenthBips32{10'000},  // 10%
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .nextPaymentDueDate = 2'000,
+                .expectedLateInterest = Number{0},
+            },
+            {
+                .name = "No late interest rate",
+                .principalOutstanding = Number{1'000},
+                .lateInterestRate = TenthBips32{0},  // 0%
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .nextPaymentDueDate = 2'000,
+                .expectedLateInterest = Number{0},
+            },
+            {
+                .name = "Late payment",
+                .principalOutstanding = Number{1'000},
+                .lateInterestRate = TenthBips32{100'000},  // 100%
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .nextPaymentDueDate = 2'000,
+                .expectedLateInterest =
+                    Number{3170979198376459, -17},  // from calc
+            },
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("loanLatePaymentInterest: " + tc.name);
+
+            auto const computedLateInterest = loanLatePaymentInterest(
+                tc.principalOutstanding,
+                tc.lateInterestRate,
+                tc.parentCloseTime,
+                tc.nextPaymentDueDate);
+            BEAST_EXPECTS(
+                computedLateInterest == tc.expectedLateInterest,
+                "Late interest mismatch: expected " +
+                    to_string(tc.expectedLateInterest) + ", got " +
+                    to_string(computedLateInterest));
+        }
+    }
+
+    void
+    testLoanAccruedInterest()
+    {
+        using namespace jtx;
+        using namespace ripple::detail;
+        struct TestCase
+        {
+            std::string name;
+            Number principalOutstanding;
+            Number periodicRate;
+            NetClock::time_point parentCloseTime;
+            std::uint32_t startDate;
+            std::uint32_t prevPaymentDate;
+            std::uint32_t paymentInterval;
+            Number expectedAccruedInterest;
+        };
+
+        auto const testCases = std::vector<TestCase>{
+            {
+                .name = "Zero principal outstanding",
+                .principalOutstanding = Number{0},
+                .periodicRate = Number{5, -2},
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .startDate = 2'000,
+                .prevPaymentDate = 2'500,
+                .paymentInterval = 30 * 24 * 60 * 60,
+                .expectedAccruedInterest = Number{0},
+            },
+            {
+                .name = "Before start date",
+                .principalOutstanding = Number{1'000},
+                .periodicRate = Number{5, -2},
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{1'000}},
+                .startDate = 2'000,
+                .prevPaymentDate = 1'500,
+                .paymentInterval = 30 * 24 * 60 * 60,
+                .expectedAccruedInterest = Number{0},
+            },
+            {
+                .name = "Zero periodic rate",
+                .principalOutstanding = Number{1'000},
+                .periodicRate = Number{0},
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .startDate = 2'000,
+                .prevPaymentDate = 2'500,
+                .paymentInterval = 30 * 24 * 60 * 60,
+                .expectedAccruedInterest = Number{0},
+            },
+            {
+                .name = "Zero payment interval",
+                .principalOutstanding = Number{1'000},
+                .periodicRate = Number{5, -2},
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .startDate = 2'000,
+                .prevPaymentDate = 2'500,
+                .paymentInterval = 0,
+                .expectedAccruedInterest = Number{0},
+            },
+            {
+                .name = "Standard case",
+                .principalOutstanding = Number{1'000},
+                .periodicRate = Number{5, -2},
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .startDate = 1'000,
+                .prevPaymentDate = 2'000,
+                .paymentInterval = 30 * 24 * 60 * 60,
+                .expectedAccruedInterest =
+                    Number{1929012345679012, -17},  // from calc
+            },
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("loanAccruedInterest: " + tc.name);
+
+            auto const computedAccruedInterest = loanAccruedInterest(
+                tc.principalOutstanding,
+                tc.periodicRate,
+                tc.parentCloseTime,
+                tc.startDate,
+                tc.prevPaymentDate,
+                tc.paymentInterval);
+            BEAST_EXPECTS(
+                computedAccruedInterest == tc.expectedAccruedInterest,
+                "Accrued interest mismatch: expected " +
+                    to_string(tc.expectedAccruedInterest) + ", got " +
+                    to_string(computedAccruedInterest));
+        }
+    }
+
+    // This test overlaps with testLoanAccruedInterest, the test cases only
+    // exercise the computeFullPaymentInterest parts unique to it.
+    void
+    testComputeFullPaymentInterest()
+    {
+        using namespace jtx;
+        using namespace ripple::detail;
+
+        struct TestCase
+        {
+            std::string name;
+            Number rawPrincipalOutstanding;
+            Number periodicRate;
+            NetClock::time_point parentCloseTime;
+            std::uint32_t paymentInterval;
+            std::uint32_t prevPaymentDate;
+            std::uint32_t startDate;
+            TenthBips32 closeInterestRate;
+            Number expectedFullPaymentInterest;
+        };
+
+        auto const testCases = std::vector<TestCase>{
+            {
+                .name = "Zero principal outstanding",
+                .rawPrincipalOutstanding = Number{0},
+                .periodicRate = Number{5, -2},
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .paymentInterval = 30 * 24 * 60 * 60,
+                .prevPaymentDate = 2'000,
+                .startDate = 1'000,
+                .closeInterestRate = TenthBips32{10'000},
+                .expectedFullPaymentInterest = Number{0},
+            },
+            {
+                .name = "Zero close interest rate",
+                .rawPrincipalOutstanding = Number{1'000},
+                .periodicRate = Number{5, -2},
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .paymentInterval = 30 * 24 * 60 * 60,
+                .prevPaymentDate = 2'000,
+                .startDate = 1'000,
+                .closeInterestRate = TenthBips32{0},
+                .expectedFullPaymentInterest =
+                    Number{1929012345679012, -17},  // from calc
+            },
+            {
+                .name = "Standard case",
+                .rawPrincipalOutstanding = Number{1'000},
+                .periodicRate = Number{5, -2},
+                .parentCloseTime =
+                    NetClock::time_point{NetClock::duration{3'000}},
+                .paymentInterval = 30 * 24 * 60 * 60,
+                .prevPaymentDate = 2'000,
+                .startDate = 1'000,
+                .closeInterestRate = TenthBips32{10'000},
+                .expectedFullPaymentInterest =
+                    Number{1000192901234568, -13},  // from calc
+            },
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("computeFullPaymentInterest: " + tc.name);
+
+            auto const computedFullPaymentInterest = computeFullPaymentInterest(
+                tc.rawPrincipalOutstanding,
+                tc.periodicRate,
+                tc.parentCloseTime,
+                tc.paymentInterval,
+                tc.prevPaymentDate,
+                tc.startDate,
+                tc.closeInterestRate);
+            BEAST_EXPECTS(
+                computedFullPaymentInterest == tc.expectedFullPaymentInterest,
+                "Full payment interest mismatch: expected " +
+                    to_string(tc.expectedFullPaymentInterest) + ", got " +
+                    to_string(computedFullPaymentInterest));
+        }
+    }
+
+public:
+    void
+    run() override
+    {
+        testComputeFullPaymentInterest();
+        testLoanAccruedInterest();
+        testLoanLatePaymentInterest();
+        testLoanPeriodicPayment();
+        testLoanPrincipalFromPeriodicPayment();
+        testComputeRaisedRate();
+        testComputePaymentFactor();
+        testComputeOverpaymentComponents();
+        testComputeInterestAndFeeParts();
+    }
+};
+
+BEAST_DEFINE_TESTSUITE(LendingHelpers, app, ripple);
+
+}  // namespace test
+}  // namespace ripple

src/test/app/Loan_test.cpp

@@ -6145,15 +6145,16 @@ protected:
         // Accrued + prepayment-penalty interest based on current periodic
         // schedule
         auto const fullPaymentInterest = computeFullPaymentInterest(
-            after.periodicPayment,
+            detail::loanPrincipalFromPeriodicPayment(
+                after.periodicPayment, periodicRate2, after.paymentRemaining),
             periodicRate2,
-            after.paymentRemaining,
             env.current()->parentCloseTime(),
             after.paymentInterval,
             after.previousPaymentDate,
             static_cast<std::uint32_t>(
                 after.startDate.time_since_epoch().count()),
             closeInterestRate);
+
         // Round to asset scale and split interest/fee parts
         auto const roundedInterest =
             roundToAsset(asset.raw(), fullPaymentInterest, after.loanScale);
@@ -6181,9 +6182,9 @@ protected:
         // window by clamping prevPaymentDate to 'now' for the full-pay path.
         auto const prevClamped = std::min(after.previousPaymentDate, nowSecs);
         auto const fullPaymentInterestClamped = computeFullPaymentInterest(
-            after.periodicPayment,
+            detail::loanPrincipalFromPeriodicPayment(
+                after.periodicPayment, periodicRate2, after.paymentRemaining),
             periodicRate2,
-            after.paymentRemaining,
             env.current()->parentCloseTime(),
             after.paymentInterval,
             prevClamped,
@@ -7027,6 +7028,140 @@ protected:
             paymentParams);
     }
 
+    void
+    testLoanPayBrokerOwnerMissingTrustline()
+    {
+        testcase << "LoanPay Broker Owner Missing Trustline (PoC)";
+        using namespace jtx;
+        using namespace loan;
+        Account const issuer("issuer");
+        Account const borrower("borrower");
+        Account const broker("broker");
+        auto const IOU = issuer["IOU"];
+        Env env(*this, all);
+        env.fund(XRP(20'000), issuer, broker, borrower);
+        env.close();
+        // Set up trustlines and fund accounts
+        env(trust(broker, IOU(20'000'000)));
+        env(trust(borrower, IOU(20'000'000)));
+        env(pay(issuer, broker, IOU(10'000'000)));
+        env(pay(issuer, borrower, IOU(1'000)));
+        env.close();
+        // Create vault and broker
+        auto const brokerInfo = createVaultAndBroker(env, IOU, broker);
+        // Create a loan first (this creates debt)
+        auto const keylet = keylet::loan(brokerInfo.brokerID, 1);
+        env(set(borrower, brokerInfo.brokerID, 10'000),
+            sig(sfCounterpartySignature, broker),
+            loanServiceFee(IOU(100).value()),
+            paymentInterval(100),
+            fee(XRP(100)));
+        env.close();
+        // Ensure broker has sufficient cover so brokerPayee == brokerOwner
+        // We need coverAvailable >= (debtTotal * coverRateMinimum)
+        // Deposit enough cover to ensure the fee goes to broker owner
+        // The default coverRateMinimum is 10%, so for a 10,000 loan we need
+        // at least 1,000 cover. Default cover is 1,000, so we add more to be
+        // safe.
+        auto const additionalCover = IOU(50'000).value();
+        env(loanBroker::coverDeposit(
+            broker, brokerInfo.brokerID, STAmount{IOU, additionalCover}));
+        env.close();
+        // Verify broker owner has a trustline
+        auto const brokerTrustline = keylet::line(broker, IOU);
+        BEAST_EXPECT(env.le(brokerTrustline) != nullptr);
+        // Broker owner deletes their trustline
+        // First, pay any positive balance to issuer to zero it out
+        auto const brokerBalance = env.balance(broker, IOU);
+        env(pay(broker, issuer, brokerBalance));
+        env.close();
+        // Remove the trustline by setting limit to 0
+        env(trust(broker, IOU(0)));
+        env.close();
+        // Verify trustline is deleted
+        BEAST_EXPECT(env.le(brokerTrustline) == nullptr);
+        // Now borrower tries to make a payment
+        // We should get a tesSUCCESS instead of a tecNO_LINE.
+        env(pay(borrower, keylet.key, IOU(10'100)),
+            fee(XRP(100)),
+            ter(tesSUCCESS));
+        env.close();
+    }
+
+    void
+    testLoanPayBrokerOwnerUnauthorizedMPT()
+    {
+        testcase << "LoanPay Broker Owner MPT unauthorized";
+        using namespace jtx;
+        using namespace loan;
+
+        Account const issuer("issuer");
+        Account const borrower("borrower");
+        Account const broker("broker");
+
+        Env env(*this, all);
+        env.fund(XRP(20'000), issuer, broker, borrower);
+        env.close();
+
+        MPTTester mptt{env, issuer, mptInitNoFund};
+        mptt.create(
+            {.flags = tfMPTCanClawback | tfMPTCanTransfer | tfMPTCanLock});
+
+        PrettyAsset const MPT{mptt.issuanceID()};
+
+        // Authorize broker and borrower
+        mptt.authorize({.account = broker});
+        mptt.authorize({.account = borrower});
+
+        env.close();
+
+        // Fund accounts
+        env(pay(issuer, broker, MPT(10'000'000)));
+        env(pay(issuer, borrower, MPT(1'000)));
+        env.close();
+
+        // Create vault and broker
+        auto const brokerInfo = createVaultAndBroker(env, MPT, broker);
+        // Create a loan first (this creates debt)
+        auto const keylet = keylet::loan(brokerInfo.brokerID, 1);
+        env(set(borrower, brokerInfo.brokerID, 10'000),
+            sig(sfCounterpartySignature, broker),
+            loanServiceFee(MPT(100).value()),
+            paymentInterval(100),
+            fee(XRP(100)));
+        env.close();
+        // Ensure broker has sufficient cover so brokerPayee == brokerOwner
+        // We need coverAvailable >= (debtTotal * coverRateMinimum)
+        // Deposit enough cover to ensure the fee goes to broker owner
+        // The default coverRateMinimum is 10%, so for a 10,000 loan we need
+        // at least 1,000 cover. Default cover is 1,000, so we add more to be
+        // safe.
+        auto const additionalCover = MPT(50'000).value();
+        env(loanBroker::coverDeposit(
+            broker, brokerInfo.brokerID, STAmount{MPT, additionalCover}));
+        env.close();
+        // Verify broker owner is authorized
+        auto const brokerMpt = keylet::mptoken(mptt.issuanceID(), broker);
+        BEAST_EXPECT(env.le(brokerMpt) != nullptr);
+        // Broker owner unauthorizes.
+        // First, pay any positive balance to issuer to zero it out
+        auto const brokerBalance = env.balance(broker, MPT);
+        env(pay(broker, issuer, brokerBalance));
+        env.close();
+        // Then, unauthorize the MPT.
+        mptt.authorize({.account = broker, .flags = tfMPTUnauthorize});
+        env.close();
+        // Verify the MPT is unauthorized.
+        BEAST_EXPECT(env.le(brokerMpt) == nullptr);
+        // Now borrower tries to make a payment
+        // We should get a tesSUCCESS instead of a tecNO_AUTH.
+        auto const borrowerBalance = env.balance(borrower, MPT);
+        env(pay(borrower, keylet.key, MPT(10'100)),
+            fee(XRP(100)),
+            ter(tesSUCCESS));
+        env.close();
+    }
+
 public:
     void
     run() override
@@ -7075,6 +7210,8 @@ public:
         testBorrowerIsBroker();
         testIssuerIsBorrower();
         testLimitExceeded();
+        testLoanPayBrokerOwnerMissingTrustline();
+        testLoanPayBrokerOwnerUnauthorizedMPT();
     }
 };
 

src/xrpld/app/misc/LendingHelpers.h

@@ -202,14 +202,6 @@ computeRawLoanState(
     std::uint32_t const paymentRemaining,
     TenthBips32 const managementFeeRate);
 
-LoanState
-computeRawLoanState(
-    Number const& periodicPayment,
-    TenthBips32 interestRate,
-    std::uint32_t paymentInterval,
-    std::uint32_t const paymentRemaining,
-    TenthBips32 const managementFeeRate);
-
 // Constructs a valid LoanState object from arbitrary inputs
 LoanState
 constructLoanState(
@@ -239,17 +231,6 @@ computeFullPaymentInterest(
     std::uint32_t startDate,
     TenthBips32 closeInterestRate);
 
-Number
-computeFullPaymentInterest(
-    Number const& periodicPayment,
-    Number const& periodicRate,
-    std::uint32_t paymentRemaining,
-    NetClock::time_point parentCloseTime,
-    std::uint32_t paymentInterval,
-    std::uint32_t prevPaymentDate,
-    std::uint32_t startDate,
-    TenthBips32 closeInterestRate);
-
 namespace detail {
 // These classes and functions should only be accessed by LendingHelper
 // functions and unit tests
@@ -387,6 +368,58 @@ struct LoanStateDeltas
     nonNegative();
 };
 
+Number
+computeRaisedRate(Number const& periodicRate, std::uint32_t paymentsRemaining);
+
+Number
+computePaymentFactor(
+    Number const& periodicRate,
+    std::uint32_t paymentsRemaining);
+
+std::pair<Number, Number>
+computeInterestAndFeeParts(
+    Asset const& asset,
+    Number const& interest,
+    TenthBips16 managementFeeRate,
+    std::int32_t loanScale);
+
+Number
+loanPeriodicPayment(
+    Number const& principalOutstanding,
+    Number const& periodicRate,
+    std::uint32_t paymentsRemaining);
+
+Number
+loanPrincipalFromPeriodicPayment(
+    Number const& periodicPayment,
+    Number const& periodicRate,
+    std::uint32_t paymentsRemaining);
+
+Number
+loanLatePaymentInterest(
+    Number const& principalOutstanding,
+    TenthBips32 lateInterestRate,
+    NetClock::time_point parentCloseTime,
+    std::uint32_t nextPaymentDueDate);
+
+Number
+loanAccruedInterest(
+    Number const& principalOutstanding,
+    Number const& periodicRate,
+    NetClock::time_point parentCloseTime,
+    std::uint32_t startDate,
+    std::uint32_t prevPaymentDate,
+    std::uint32_t paymentInterval);
+
+ExtendedPaymentComponents
+computeOverpaymentComponents(
+    Asset const& asset,
+    int32_t const loanScale,
+    Number const& overpayment,
+    TenthBips32 const overpaymentInterestRate,
+    TenthBips32 const overpaymentFeeRate,
+    TenthBips16 const managementFeeRate);
+
 PaymentComponents
 computePaymentComponents(
     Asset const& asset,

src/xrpld/app/misc/detail/LendingHelpers.cpp

@@ -100,6 +100,9 @@ computePaymentFactor(
     Number const& periodicRate,
     std::uint32_t paymentsRemaining)
 {
+    if (paymentsRemaining == 0)
+        return numZero;
+
     // For zero interest, payment factor is simply 1/paymentsRemaining
     if (periodicRate == beast::zero)
         return Number{1} / paymentsRemaining;
@@ -132,27 +135,6 @@ loanPeriodicPayment(
         computePaymentFactor(periodicRate, paymentsRemaining);
 }
 
-/* Calculates the periodic payment amount from annualized interest rate.
- * Converts the annual rate to periodic rate before computing payment.
- *
- * Equation (7) from XLS-66 spec, Section A-2 Equation Glossary
- */
-Number
-loanPeriodicPayment(
-    Number const& principalOutstanding,
-    TenthBips32 interestRate,
-    std::uint32_t paymentInterval,
-    std::uint32_t paymentsRemaining)
-{
-    if (principalOutstanding == 0 || paymentsRemaining == 0)
-        return 0;
-
-    Number const periodicRate = loanPeriodicRate(interestRate, paymentInterval);
-
-    return loanPeriodicPayment(
-        principalOutstanding, periodicRate, paymentsRemaining);
-}
-
 /* Reverse-calculates principal from periodic payment amount.
  * Used to determine theoretical principal at any point in the schedule.
  *
@@ -164,6 +146,9 @@ loanPrincipalFromPeriodicPayment(
     Number const& periodicRate,
     std::uint32_t paymentsRemaining)
 {
+    if (paymentsRemaining == 0)
+        return numZero;
+
     if (periodicRate == 0)
         return periodicPayment * paymentsRemaining;
 
@@ -171,21 +156,6 @@ loanPrincipalFromPeriodicPayment(
         computePaymentFactor(periodicRate, paymentsRemaining);
 }
 
-/* Splits gross interest into net interest (to vault) and management fee (to
- * broker). Returns pair of (net interest, management fee).
- *
- * Equation (33) from XLS-66 spec, Section A-2 Equation Glossary
- */
-std::pair<Number, Number>
-computeInterestAndFeeParts(
-    Number const& interest,
-    TenthBips16 managementFeeRate)
-{
-    auto const fee = tenthBipsOfValue(interest, managementFeeRate);
-
-    return std::make_pair(interest - fee, fee);
-}
-
 /*
  * Computes the interest and management fee parts from interest amount.
  *
@@ -216,6 +186,12 @@ loanLatePaymentInterest(
     NetClock::time_point parentCloseTime,
     std::uint32_t nextPaymentDueDate)
 {
+    if (principalOutstanding == beast::zero)
+        return numZero;
+
+    if (lateInterestRate == TenthBips32{0})
+        return numZero;
+
     auto const now = parentCloseTime.time_since_epoch().count();
 
     // If the payment is not late by any amount of time, then there's no late
@@ -248,6 +224,9 @@ loanAccruedInterest(
     if (periodicRate == beast::zero)
         return numZero;
 
+    if (paymentInterval == 0)
+        return numZero;
+
     auto const lastPaymentDate = std::max(prevPaymentDate, startDate);
     auto const now = parentCloseTime.time_since_epoch().count();
 
@@ -1225,17 +1204,12 @@ computeOverpaymentComponents(
     // This interest doesn't follow the normal amortization schedule - it's
     // a one-time charge for paying early.
     // Equation (20) and (21) from XLS-66 spec, Section A-2 Equation Glossary
-    auto const [rawOverpaymentInterest, _] = [&]() {
-        Number const interest =
-            tenthBipsOfValue(overpayment, overpaymentInterestRate);
-        return detail::computeInterestAndFeeParts(interest, managementFeeRate);
-    }();
-
-    // Round the penalty interest components to the loan scale
     auto const [roundedOverpaymentInterest, roundedOverpaymentManagementFee] =
         [&]() {
-            Number const interest =
-                roundToAsset(asset, rawOverpaymentInterest, loanScale);
+            auto const interest = roundToAsset(
+                asset,
+                tenthBipsOfValue(overpayment, overpaymentInterestRate),
+                loanScale);
             return detail::computeInterestAndFeeParts(
                 asset, interest, managementFeeRate, loanScale);
         }();
@@ -1429,31 +1403,6 @@ computeFullPaymentInterest(
     return accruedInterest + prepaymentPenalty;
 }
 
-Number
-computeFullPaymentInterest(
-    Number const& periodicPayment,
-    Number const& periodicRate,
-    std::uint32_t paymentRemaining,
-    NetClock::time_point parentCloseTime,
-    std::uint32_t paymentInterval,
-    std::uint32_t prevPaymentDate,
-    std::uint32_t startDate,
-    TenthBips32 closeInterestRate)
-{
-    Number const rawPrincipalOutstanding =
-        detail::loanPrincipalFromPeriodicPayment(
-            periodicPayment, periodicRate, paymentRemaining);
-
-    return computeFullPaymentInterest(
-        rawPrincipalOutstanding,
-        periodicRate,
-        parentCloseTime,
-        paymentInterval,
-        prevPaymentDate,
-        startDate,
-        closeInterestRate);
-}
-
 /* Calculates the theoretical loan state at maximum precision for a given point
  * in the amortization schedule.
  *
@@ -1515,21 +1464,6 @@ computeRawLoanState(
         .managementFeeDue = rawManagementFeeOutstanding};
 };
 
-LoanState
-computeRawLoanState(
-    Number const& periodicPayment,
-    TenthBips32 interestRate,
-    std::uint32_t paymentInterval,
-    std::uint32_t const paymentRemaining,
-    TenthBips32 const managementFeeRate)
-{
-    return computeRawLoanState(
-        periodicPayment,
-        loanPeriodicRate(interestRate, paymentInterval),
-        paymentRemaining,
-        managementFeeRate);
-}
-
 /* Constructs a LoanState from rounded Loan ledger object values.
  *
  * This function creates a LoanState structure from the three tracked values

src/xrpld/app/tx/detail/LoanBrokerDelete.cpp

@@ -67,7 +67,7 @@ LoanBrokerDelete::preclaim(PreclaimContext const& ctx)
             JLOG(ctx.j.warn()) << "LoanBrokerDelete: Debt total is "
                                << debtTotal << ", which rounds to " << rounded;
             return tecHAS_OBLIGATIONS;
-            // LCOV_EXCL_START
+            // LCOV_EXCL_STOP
         }
     }
 

src/xrpld/app/tx/detail/LoanPay.cpp

@@ -262,9 +262,10 @@ LoanPay::doApply()
     auto debtTotalProxy = brokerSle->at(sfDebtTotal);
 
     // Send the broker fee to the owner if they have sufficient cover available,
-    // _and_ if the owner can receive funds. If not, so as not to block the
-    // payment, add it to the cover balance (send it to the broker pseudo
-    // account).
+    // _and_ if the owner can receive funds
+    // _and_ if the broker is authorized to hold funds. If not, so as not to
+    // block the payment, add it to the cover balance (send it to the broker
+    // pseudo account).
     //
     // Normally freeze status is checked in preflight, but we do it here to
     // avoid duplicating the check. It'll claim a fee either way.
@@ -278,7 +279,9 @@ LoanPay::doApply()
                    asset,
                    tenthBipsOfValue(debtTotalProxy.value(), coverRateMinimum),
                    loanScale) &&
-            !isDeepFrozen(view, brokerOwner, asset);
+            !isDeepFrozen(view, brokerOwner, asset) &&
+            requireAuth(view, asset, brokerOwner, AuthType::StrongAuth) ==
+            tesSUCCESS;
     }();
 
     auto const brokerPayee =
@@ -305,13 +308,7 @@ LoanPay::doApply()
     // change will be discarded.
     if (loanSle->isFlag(lsfLoanImpaired))
     {
-        if (auto const ret =
-                LoanManage::unimpairLoan(view, loanSle, vaultSle, j_);
-            ret != tesSUCCESS)
-        {
-            JLOG(j_.fatal()) << "Failed to unimpair loan before payment.";
-            return ret;  // LCOV_EXCL_LINE
-        }
+        LoanManage::unimpairLoan(view, loanSle, vaultSle, j_);
     }
 
     LoanPaymentType const paymentType = [&tx]() {