Review feedback from @gregtatcam

- Remove a redundant struct LoanPaymentParts declaration.
- Add pointers to the XLS-66 spec for all of the LendingHeler formulas.
- Changed if/else if in LoanManage.

src/xrpld/app/misc/LendingHelpers.h

@@ -36,14 +36,6 @@ namespace detail {
 // These functions should rarely be used directly. More often, the ultimate
 // result needs to be roundToAsset'd.
 
-struct LoanPaymentParts
-{
-    Number principalPaid;
-    Number interestPaid;
-    Number valueChange;
-    Number feePaid;
-};
-
 Number
 loanPeriodicRate(TenthBips32 interestRate, std::uint32_t paymentInterval);
 
@@ -70,6 +62,10 @@ loanTotalValueOutstanding(
 {
     return roundToAsset(
         asset,
+        /*
+         * This formula is from the XLS-66 spec, section 3.2.4.2 (Total Loan
+         * Value Calculation), specifically "totalValueOutstanding = ..."
+         */
         periodicPayment * paymentsRemaining,
         originalPrincipal,
         Number::upward);
@@ -85,6 +81,10 @@ loanTotalValueOutstanding(
     std::uint32_t paymentInterval,
     std::uint32_t paymentsRemaining)
 {
+    /*
+     * This function is derived from the XLS-66 spec, section 3.2.4.2 (Total
+     * Loan Value Calculation)
+     */
     return loanTotalValueOutstanding(
         asset,
         originalPrincipal,
@@ -101,6 +101,10 @@ loanTotalInterestOutstanding(
     Number principalOutstanding,
     Number totalValueOutstanding)
 {
+    /*
+     * This formula is from the XLS-66 spec, section 3.2.4.2 (Total Loan
+     * Value Calculation), specifically "totalInterestOutstanding = ..."
+     */
     return totalValueOutstanding - principalOutstanding;
 }
 
@@ -114,6 +118,10 @@ loanTotalInterestOutstanding(
     std::uint32_t paymentInterval,
     std::uint32_t paymentsRemaining)
 {
+    /*
+     * This formula is derived from the XLS-66 spec, section 3.2.4.2 (Total Loan
+     * Value Calculation)
+     */
     return loanTotalInterestOutstanding(
         principalOutstanding,
         loanTotalValueOutstanding(
@@ -246,6 +254,10 @@ computePeriodicPaymentParts(
     Number const& periodicRate,
     std::uint32_t paymentRemaining)
 {
+    /*
+     * This function is derived from the XLS-66 spec, section 3.2.4.1.1 (Regular
+     * Payment)
+     */
     if (paymentRemaining == 1)
     {
         // If there's only one payment left, we need to pay off the principal.
@@ -313,6 +325,10 @@ loanComputePaymentParts(
     STAmount const& amount,
     beast::Journal j)
 {
+    /*
+     * This function is an implementation of the XLS-66 spec, section 3.2.4.3
+     * (Transaction Pseudo-code)
+     */
     Number const originalPrincipalRequested = loan->at(sfPrincipalRequested);
     auto principalOutstandingField = loan->at(sfPrincipalOutstanding);
     bool const allowOverpayment = loan->isFlag(lsfLoanOverpayment);

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

@@ -37,6 +37,11 @@ loanPeriodicRate(TenthBips32 interestRate, std::uint32_t paymentInterval)
 {
     // Need floating point math for this one, since we're dividing by some
     // large numbers
+    /*
+     * This formula is from the XLS-66 spec, section 3.2.4.1.1 (Regular
+     * Payment), specifically "periodicRate = ...", though it is duplicated in
+     * other places.
+     */
     return tenthBipsOfValue(Number(paymentInterval), interestRate) /
         (365 * 24 * 60 * 60);
 }
@@ -54,6 +59,11 @@ loanPeriodicPayment(
     if (periodicRate == beast::zero)
         return principalOutstanding / paymentsRemaining;
 
+    /*
+     * This formula is from the XLS-66 spec, section 3.2.4.1.1 (Regular
+     * Payment), though the awkwardly-named "timeFactor" is computed only once
+     * and used twice.
+     */
     // TODO: Need a better name
     Number const timeFactor = power(1 + periodicRate, paymentsRemaining);
 
@@ -69,6 +79,10 @@ loanPeriodicPayment(
 {
     if (principalOutstanding == 0 || paymentsRemaining == 0)
         return 0;
+    /*
+     * This function is derived from the XLS-66 spec, section 3.2.4.1.1 (Regular
+     * payment), though it is duplicated in other places.
+     */
     Number const periodicRate = loanPeriodicRate(interestRate, paymentInterval);
 
     return loanPeriodicPayment(
@@ -83,6 +97,10 @@ loanLatePaymentInterest(
     std::uint32_t startDate,
     std::uint32_t prevPaymentDate)
 {
+    /*
+     * This formula is from the XLS-66 spec, section 3.2.4.1.2 (Late payment),
+     * specifically "latePaymentInterest = ..."
+     */
     auto const lastPaymentDate = std::max(prevPaymentDate, startDate);
 
     auto const secondsSinceLastPayment =
@@ -103,6 +121,10 @@ loanAccruedInterest(
     std::uint32_t prevPaymentDate,
     std::uint32_t paymentInterval)
 {
+    /*
+     * This formula is from the XLS-66 spec, section 3.2.4.1.4 (Early Full
+     * Repayment), specifically "accruedInterest = ...".
+     */
     auto const lastPaymentDate = std::max(prevPaymentDate, startDate);
 
     auto const secondsSinceLastPayment =

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

@@ -399,7 +399,7 @@ LoanManage::doApply()
                 j_))
             return ter;
     }
-    if (tx.isFlag(tfLoanImpair))
+    else if (tx.isFlag(tfLoanImpair))
     {
         if (auto const ter = impairLoan(
                 view,
@@ -413,7 +413,7 @@ LoanManage::doApply()
                 j_))
             return ter;
     }
-    if (tx.isFlag(tfLoanUnimpair))
+    else if (tx.isFlag(tfLoanUnimpair))
     {
         if (auto const ter = unimpairLoan(
                 view,