Review feedback from @gregtatcam:

- In progress
- Add explanatory comments, and start refactoring
  loanComputePaymentParts into functions for readability.

include/xrpl/protocol/STAmount.h

@@ -695,6 +695,17 @@ divRoundStrict(
 std::uint64_t
 getRate(STAmount const& offerOut, STAmount const& offerIn);
 
+/** Round an arbitrary precision Amount to the precision of a reference Amount.
+ *
+ * This is used to ensure that calculations involving IOU amounts do not collect
+ * dust beyond the precision of the reference value.
+ *
+ * @param value The value to be rounded
+ * @param referenceValue A reference value to establish the precision limit of
+ *     `value`. Should be larger than `value`.
+ * @param rounding Optional Number rounding mode
+ *
+ */
 STAmount
 roundToReference(
     STAmount const value,
@@ -702,6 +713,10 @@ roundToReference(
     Number::rounding_mode rounding = Number::getround());
 
 /** Round an arbitrary precision Number to the precision of a given Asset.
+ *
+ * This is used to ensure that calculations do not collect dust beyond the
+ * precision of the reference value for IOUs, or fractional amounts for the
+ * integral types XRP and MPT.
  *
  * @param asset The relevant asset
  * @param value The value to be rounded
@@ -722,6 +737,8 @@ roundToAsset(
     STAmount const ret{asset, value};
     if (ret.asset().native() || !ret.asset().holds<Issue>())
         return ret;
+    // Not that the ctor will round integral types (XRP, MPT) via canonicalize,
+    // so no extra work is needed for those.
     return roundToReference(ret, STAmount{asset, referenceValue});
 }
 

src/libxrpl/basics/Number.cpp

@@ -20,8 +20,6 @@
 #include <xrpl/basics/Number.h>
 #include <xrpl/beast/utility/instrumentation.h>
 
-#include <boost/predef.h>
-
 #include <algorithm>
 #include <cstddef>
 #include <cstdint>

src/libxrpl/protocol/STAmount.cpp

@@ -1515,20 +1515,25 @@ roundToReference(
     STAmount referenceValue,
     Number::rounding_mode rounding)
 {
+    // Nothing to do for intgral types.
     if (value.asset().native() || !value.asset().holds<Issue>())
         return value;
 
-    NumberRoundModeGuard mg(rounding);
-    if (referenceValue.negative() != value.negative())
-        referenceValue.negate();
-
-    if (value.exponent() > referenceValue.exponent() &&
+    // If the value is greater than or equal to the referenceValue (ignoring
+    // sign), then rounding will do nothing, so just return the value.
+    if (value.exponent() > referenceValue.exponent() ||
         (value.exponent() == referenceValue.exponent() &&
          value.mantissa() >= referenceValue.mantissa()))
         return value;
+
+    if (referenceValue.negative() != value.negative())
+        referenceValue.negate();
+
+    NumberRoundModeGuard mg(rounding);
     // With an IOU, the total will be truncated to the precision of the
     // larger value: referenceValue
     STAmount const total = referenceValue + value;
+    // Remove the reference value, and we're left with the rounded value.
     STAmount const result = total - referenceValue;
     return result;
 }

src/xrpld/app/misc/LendingHelpers.h

@@ -316,6 +316,72 @@ struct LoanPaymentParts
     Number feePaid;
 };
 
+struct LatePaymentParams
+{
+};
+
+template <AssetType A>
+Expected<LoanPaymentParts, TER>
+handleLatePayment(
+    A const& asset,
+    ApplyView& view,
+    STObject::ValueProxy<STNumber>& principalOutstandingField,
+    STObject::ValueProxy<STUInt32>& paymentRemainingField,
+    STObject::ValueProxy<STUInt32>& prevPaymentDateField,
+    STObject::ValueProxy<STUInt32>& nextDueDateField,
+    PeriodicPaymentParts const& periodic,
+    std::uint32_t const startDate,
+    std::uint32_t const paymentInterval,
+    TenthBips32 const lateInterestRate,
+    Number const& originalPrincipalRequested,
+    Number const& periodicPaymentAmount,
+    Number const& latePaymentFee,
+    STAmount const& amount,
+    beast::Journal j)
+{
+    if (!hasExpired(view, nextDueDateField))
+        return Unexpected(tesSUCCESS);
+
+    // the payment is late
+    auto const latePaymentInterest = loanLatePaymentInterest(
+        asset,
+        principalOutstandingField,
+        lateInterestRate,
+        view.parentCloseTime(),
+        startDate,
+        prevPaymentDateField,
+        originalPrincipalRequested);
+    XRPL_ASSERT(
+        latePaymentInterest >= 0,
+        "ripple::loanComputePaymentParts : valid late interest");
+    auto const latePaymentAmount =
+        periodicPaymentAmount + latePaymentInterest + latePaymentFee;
+
+    if (amount < latePaymentAmount)
+    {
+        JLOG(j.warn()) << "Late loan payment amount is insufficient. Due: "
+                       << latePaymentAmount << ", paid: " << amount;
+        return Unexpected(tecINSUFFICIENT_PAYMENT);
+    }
+
+    paymentRemainingField -= 1;
+    // A single payment always pays the same amount of principal. Only the
+    // interest and fees are extra for a late payment
+    principalOutstandingField -= periodic.principal;
+
+    // Make sure this does an assignment
+    prevPaymentDateField = nextDueDateField;
+    nextDueDateField += paymentInterval;
+
+    // A late payment increases the value of the loan by the difference
+    // between periodic and late payment interest
+    return LoanPaymentParts{
+        periodic.principal,
+        latePaymentInterest + periodic.interest,
+        latePaymentInterest,
+        latePaymentFee};
+}
+
 template <AssetType A>
 Expected<LoanPaymentParts, TER>
 loanComputePaymentParts(
@@ -408,47 +474,25 @@ loanComputePaymentParts(
 
     // -------------------------------------------------------------
     // late payment handling
-    if (hasExpired(view, nextDueDateField))
-    {
-        // the payment is late
-        auto const latePaymentInterest = loanLatePaymentInterest(
+    if (auto const latePaymentParts = handleLatePayment(
             asset,
+            view,
             principalOutstandingField,
-            lateInterestRate,
-            view.parentCloseTime(),
-            startDate,
+            paymentRemainingField,
             prevPaymentDateField,
-            originalPrincipalRequested);
-        XRPL_ASSERT(
-            latePaymentInterest >= 0,
-            "ripple::loanComputePaymentParts : valid late interest");
-        auto const latePaymentAmount =
-            periodicPaymentAmount + latePaymentInterest + latePaymentFee;
-
-        if (amount < latePaymentAmount)
-        {
-            JLOG(j.warn()) << "Late loan payment amount is insufficient. Due: "
-                           << latePaymentAmount << ", paid: " << amount;
-            return Unexpected(tecINSUFFICIENT_PAYMENT);
-        }
-
-        paymentRemainingField -= 1;
-        // A single payment always pays the same amount of principal. Only the
-        // interest and fees are extra for a late payment
-        principalOutstandingField -= periodic.principal;
-
-        // Make sure this does an assignment
-        prevPaymentDateField = nextDueDateField;
-        nextDueDateField += paymentInterval;
-
-        // A late payment increases the value of the loan by the difference
-        // between periodic and late payment interest
-        return LoanPaymentParts{
-            periodic.principal,
-            latePaymentInterest + periodic.interest,
-            latePaymentInterest,
-            latePaymentFee};
-    }
+            nextDueDateField,
+            periodic,
+            startDate,
+            paymentInterval,
+            lateInterestRate,
+            originalPrincipalRequested,
+            periodicPaymentAmount,
+            latePaymentFee,
+            amount,
+            j))
+        return *latePaymentParts;
+    else if (latePaymentParts.error())
+        return latePaymentParts;
 
     // -------------------------------------------------------------
     // full payment handling
@@ -505,7 +549,7 @@ loanComputePaymentParts(
     }
 
     // -------------------------------------------------------------
-    // normal payment handling
+    // regular periodic payment handling
 
     // if the payment is not late nor if it's a full payment, then it must be a
     // periodic one, with possible overpayments
@@ -582,6 +626,8 @@ loanComputePaymentParts(
                                    paymentRemainingField) +
         totalInterestPaid;
 
+    // -------------------------------------------------------------
+    // overpayment handling
     Number overpaymentInterestPortion = 0;
     if (allowOverpayment)
     {

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

@@ -187,6 +187,13 @@ LoanPay::doApply()
 
     auto const totalPaidToBroker = paymentParts->feePaid + managementFee;
 
+    XRPL_ASSERT_PARTS(
+        (totalPaidToVault + totalPaidToBroker) ==
+            (paymentParts->principalPaid + paymentParts->interestPaid +
+             paymentParts->feePaid),
+        "ripple::LoanPay::doApply",
+        "payments add up");
+
     // If there is not enough first-loss capital
     auto coverAvailableField = brokerSle->at(sfCoverAvailable);
     auto debtTotalField = brokerSle->at(sfDebtTotal);
@@ -201,6 +208,8 @@ LoanPay::doApply()
         // Add the fee to to First Loss Cover Pool
         coverAvailableField += totalPaidToBroker;
     }
+    auto const brokerPayee =
+        sufficientCover ? brokerOwner : brokerPseudoAccount;
 
     // Decrease LoanBroker Debt by the amount paid, add the Loan value change,
     // and subtract the change in the management fee
@@ -241,6 +250,13 @@ LoanPay::doApply()
         "ripple::LoanPay::doApply",
         "payment agreement");
 
+    auto const accountBalanceBefore =
+        accountHolds(view, account_, asset, fhIGNORE_FREEZE, ahIGNORE_AUTH, j_);
+    auto const vaultBalanceBefore = accountHolds(
+        view, vaultPseudoAccount, asset, fhIGNORE_FREEZE, ahIGNORE_AUTH, j_);
+    auto const brokerBalanceBefore = accountHolds(
+        view, brokerPayee, asset, fhIGNORE_FREEZE, ahIGNORE_AUTH, j_);
+
     if (auto const ter = accountSend(
             view,
             account_,
@@ -252,7 +268,7 @@ LoanPay::doApply()
     if (auto const ter = accountSend(
             view,
             account_,
-            sufficientCover ? brokerOwner : brokerPseudoAccount,
+            brokerPayee,
             paidToBroker,
             j_,
             WaiveTransferFee::Yes))