Merge branch 'develop' into ximinez/directory

* XRPLF/develop:
  Implement Lending Protocol (unsupported) (5270)

include/xrpl/protocol/detail/features.macro

@@ -16,6 +16,7 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
+XRPL_FEATURE(DefragDirectories,          Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(LendingProtocol,            Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionDelegationV1_1,   Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FIX    (DirectoryLimit,             Supported::yes, VoteBehavior::DefaultNo)

src/libxrpl/ledger/ApplyView.cpp

@@ -10,6 +10,14 @@ namespace ripple {
 
 namespace directory {
 
+struct Gap
+{
+    uint64_t const page;
+    SLE::pointer node;
+    uint64_t const nextPage;
+    SLE::pointer next;
+};
+
 std::uint64_t
 createRoot(
     ApplyView& view,
@@ -112,7 +120,9 @@ insertPage(
         return std::nullopt;
     if (!view.rules().enabled(fixDirectoryLimit) &&
         page >= dirNodeMaxPages)  // Old pages limit
+    {
         return std::nullopt;
+    }
 
     // We are about to create a new node; we'll link it to
     // the chain first:
@@ -134,15 +144,8 @@ insertPage(
     // it's the default.
     if (page != 1)
         node->setFieldU64(sfIndexPrevious, page - 1);
-    XRPL_ASSERT_PARTS(
-        !nextPage,
-        "ripple::directory::insertPage",
-        "nextPage has default value");
-    /* Reserved for future use when directory pages may be inserted in
-     * between two other pages instead of only at the end of the chain.
     if (nextPage)
         node->setFieldU64(sfIndexNext, nextPage);
-    */
     describe(node);
     view.insert(node);
 
@@ -158,7 +161,7 @@ ApplyView::dirAdd(
     uint256 const& key,
     std::function<void(std::shared_ptr<SLE> const&)> const& describe)
 {
-    auto root = peek(directory);
+    auto const root = peek(directory);
 
     if (!root)
     {
@@ -169,6 +172,44 @@ ApplyView::dirAdd(
     auto [page, node, indexes] =
         directory::findPreviousPage(*this, directory, root);
 
+    if (rules().enabled(featureDefragDirectories))
+    {
+        // If there are more nodes than just the root, and there's no space in
+        // the last one, walk backwards to find one with space, or to find one
+        // missing.
+        std::optional<directory::Gap> gapPages;
+        while (page && indexes.size() >= dirNodeMaxEntries)
+        {
+            // Find a page with space, or a gap in pages.
+            auto [prevPage, prevNode, prevIndexes] =
+                directory::findPreviousPage(*this, directory, node);
+            if (!gapPages && prevPage != page - 1)
+                gapPages.emplace(prevPage, prevNode, page, node);
+            page = prevPage;
+            node = prevNode;
+            indexes = prevIndexes;
+        }
+        // We looped through all the pages back to the root.
+        if (!page)
+        {
+            // If we found a gap, use it.
+            if (gapPages)
+            {
+                return directory::insertPage(
+                    *this,
+                    gapPages->page,
+                    gapPages->node,
+                    gapPages->nextPage,
+                    gapPages->next,
+                    key,
+                    directory,
+                    describe);
+            }
+            std::tie(page, node, indexes) =
+                directory::findPreviousPage(*this, directory, root);
+        }
+    }
+
     // If there's space, we use it:
     if (indexes.size() < dirNodeMaxEntries)
     {

src/test/app/Loan_test.cpp

@@ -141,7 +141,7 @@ protected:
             using namespace jtx;
 
             auto const vaultSle = env.le(keylet::vault(vaultID));
-            return getAssetsTotalScale(vaultSle);
+            return getVaultScale(vaultSle);
         }
     };
 
@@ -551,15 +551,12 @@ protected:
                        broker.vaultScale(env),
                        state.principalOutstanding.exponent())));
         BEAST_EXPECT(state.paymentInterval == 600);
-        {
-            NumberRoundModeGuard mg(Number::upward);
         BEAST_EXPECT(
             state.totalValue ==
             roundToAsset(
                 broker.asset,
                 state.periodicPayment * state.paymentRemaining,
                 state.loanScale));
-        }
         BEAST_EXPECT(
             state.managementFeeOutstanding ==
             computeManagementFee(
@@ -700,8 +697,7 @@ protected:
             interval,
             total,
             feeRate,
-            asset(brokerParams.vaultDeposit).number().exponent(),
-            env.journal);
+            asset(brokerParams.vaultDeposit).number().exponent());
         log << "Loan properties:\n"
             << "\tPrincipal: " << principal << std::endl
             << "\tInterest rate: " << interest << std::endl
@@ -1481,8 +1477,7 @@ protected:
             state.paymentInterval,
             state.paymentRemaining,
             broker.params.managementFeeRate,
-            state.loanScale,
-            env.journal);
+            state.loanScale);
 
         verifyLoanStatus(
             0,
@@ -2453,18 +2448,13 @@ protected:
                 // Make all the payments in one transaction
                 // service fee is 2
                 auto const startingPayments = state.paymentRemaining;
-                STAmount const payoffAmount = [&]() {
-                    NumberRoundModeGuard mg(Number::upward);
                 auto const rawPayoff = startingPayments *
                     (state.periodicPayment + broker.asset(2).value());
                 STAmount const payoffAmount{broker.asset, rawPayoff};
-                    BEAST_EXPECTS(
+                BEAST_EXPECT(
                     payoffAmount ==
-                            broker.asset(Number(1024014840139457, -12)),
-                        to_string(payoffAmount));
+                    broker.asset(Number(1024014840139457, -12)));
                 BEAST_EXPECT(payoffAmount > state.principalOutstanding);
-                    return payoffAmount;
-                }();
 
                 singlePayment(
                     loanKeylet,
@@ -4019,7 +4009,7 @@ protected:
         createJson = env.json(createJson, sig(sfCounterpartySignature, lender));
         // Fails in preclaim because principal requested can't be
         // represented as XRP
-        env(createJson, ter(tecPRECISION_LOSS), THISLINE);
+        env(createJson, ter(tecPRECISION_LOSS));
         env.close();
 
         BEAST_EXPECT(!env.le(keylet));
@@ -4031,7 +4021,7 @@ protected:
         createJson = env.json(createJson, sig(sfCounterpartySignature, lender));
         // Fails in doApply because the payment is too small to be
         // represented as XRP.
-        env(createJson, ter(tecPRECISION_LOSS), THISLINE);
+        env(createJson, ter(tecPRECISION_LOSS));
         env.close();
     }
 
@@ -5006,7 +4996,7 @@ protected:
         auto const keylet = keylet::loan(broker.brokerID, loanSequence);
 
         createJson = env.json(createJson, sig(sfCounterpartySignature, lender));
-        env(createJson, ter(tecPRECISION_LOSS), THISLINE);
+        env(createJson, ter(tecPRECISION_LOSS));
         env.close(startDate);
 
         auto loanPayTx = env.json(

src/xrpld/app/misc/LendingHelpers.h

@@ -179,12 +179,11 @@ adjustImpreciseNumber(
 }
 
 inline int
-getAssetsTotalScale(SLE::const_ref vaultSle)
+getVaultScale(SLE::const_ref vaultSle)
 {
     if (!vaultSle)
         return Number::minExponent - 1;  // LCOV_EXCL_LINE
-    return STAmount{vaultSle->at(sfAsset), vaultSle->at(sfAssetsTotal)}
-        .exponent();
+    return vaultSle->at(sfAssetsTotal).exponent();
 }
 
 TER
@@ -419,8 +418,7 @@ computeLoanProperties(
     std::uint32_t paymentInterval,
     std::uint32_t paymentsRemaining,
     TenthBips32 managementFeeRate,
-    std::int32_t minimumScale,
-    beast::Journal j);
+    std::int32_t minimumScale);
 
 bool
 isRounded(Asset const& asset, Number const& value, std::int32_t scale);

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

@@ -451,8 +451,7 @@ tryOverpayment(
         paymentInterval,
         paymentRemaining,
         managementFeeRate,
-        loanScale,
-        j);
+        loanScale);
 
     JLOG(j.debug()) << "new periodic payment: "
                     << newLoanProperties.periodicPayment
@@ -554,8 +553,8 @@ tryOverpayment(
     // Calculate how the loan's value changed due to the overpayment.
     // This should be negative (value decreased) or zero. A principal
     // overpayment should never increase the loan's value.
-    auto const valueChange = newRounded.valueOutstanding -
-        hypotheticalValueOutstanding - deltas.managementFee;
+    auto const valueChange =
+        newRounded.valueOutstanding - hypotheticalValueOutstanding;
     if (valueChange > 0)
     {
         JLOG(j.warn()) << "Principal overpayment would increase the value of "
@@ -1612,8 +1611,7 @@ computeLoanProperties(
     std::uint32_t paymentInterval,
     std::uint32_t paymentsRemaining,
     TenthBips32 managementFeeRate,
-    std::int32_t minimumScale,
-    beast::Journal j)
+    std::int32_t minimumScale)
 {
     auto const periodicRate = loanPeriodicRate(interestRate, paymentInterval);
     XRPL_ASSERT(
@@ -1624,22 +1622,13 @@ computeLoanProperties(
         principalOutstanding, periodicRate, paymentsRemaining);
 
     auto const [totalValueOutstanding, loanScale] = [&]() {
-        // only round up if there should be interest
-        NumberRoundModeGuard mg(
-            periodicRate == 0 ? Number::to_nearest : Number::upward);
+        NumberRoundModeGuard mg(Number::to_nearest);
         // Use STAmount's internal rounding instead of roundToAsset, because
         // we're going to use this result to determine the scale for all the
         // other rounding.
 
         // Equation (30) from XLS-66 spec, Section A-2 Equation Glossary
         STAmount amount{asset, periodicPayment * paymentsRemaining};
-        JLOG(j.debug()) << "computeLoanProperties:" << " Principal requested: "
-                        << principalOutstanding
-                        << ". Periodic payment: " << periodicPayment
-                        << ". Payments remaining: " << paymentsRemaining
-                        << ". Raw total value: "
-                        << periodicPayment * paymentsRemaining
-                        << ". Candidate total value: " << amount << std::endl;
 
         // Base the loan scale on the total value, since that's going to be
         // the biggest number involved (barring unusual parameters for late,
@@ -1654,10 +1643,7 @@ computeLoanProperties(
 
         // We may need to truncate the total value because of the minimum
         // scale
-        amount = roundToAsset(asset, amount, loanScale);
-
-        JLOG(j.debug()) << "computeLoanProperties: Loan scale:" << loanScale
-                        << ". Actual total value: " << amount << std::endl;
+        amount = roundToAsset(asset, amount, loanScale, Number::to_nearest);
 
         return std::make_pair(amount, loanScale);
     }();

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

@@ -56,7 +56,7 @@ LoanBrokerDelete::preclaim(PreclaimContext const& ctx)
         if (!vault)
             return tefINTERNAL;  // LCOV_EXCL_LINE
         auto const asset = vault->at(sfAsset);
-        auto const scale = getAssetsTotalScale(vault);
+        auto const scale = getVaultScale(vault);
 
         auto const rounded =
             roundToAsset(asset, debtTotal, scale, Number::towards_zero);

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

@@ -115,7 +115,7 @@ LoanDelete::doApply()
                 roundToAsset(
                     vaultSle->at(sfAsset),
                     debtTotalProxy,
-                    getAssetsTotalScale(vaultSle),
+                    getVaultScale(vaultSle),
                     Number::towards_zero) == beast::zero,
                 "ripple::LoanDelete::doApply",
                 "last loan, remaining debt rounds to zero");

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

@@ -106,7 +106,7 @@ LoanManage::preclaim(PreclaimContext const& ctx)
     if (loanBrokerSle->at(sfOwner) != account)
     {
         JLOG(ctx.j.warn())
-            << "LoanBroker for Loan does not belong to the account. LoanManage "
+            << "LoanBroker for Loan does not belong to the account. LoanModify "
                "can only be submitted by the Loan Broker.";
         return tecNO_PERMISSION;
     }
@@ -178,7 +178,7 @@ LoanManage::defaultLoan(
     // The vault may be at a different scale than the loan. Reduce rounding
     // errors during the accounting by rounding some of the values to that
     // scale.
-    auto const vaultScale = getAssetsTotalScale(vaultSle);
+    auto const vaultScale = getVaultScale(vaultSle);
 
     {
         // Decrease the Total Value of the Vault:
@@ -242,11 +242,7 @@ LoanManage::defaultLoan(
                 return tefBAD_LEDGER;
                 // LCOV_EXCL_STOP
             }
-            adjustImpreciseNumber(
-                vaultLossUnrealizedProxy,
-                -totalDefaultAmount,
-                vaultAsset,
-                vaultScale);
+            vaultLossUnrealizedProxy -= totalDefaultAmount;
         }
         view.update(vaultSle);
     }
@@ -254,9 +250,11 @@ LoanManage::defaultLoan(
     // Update the LoanBroker object:
 
     {
+        auto const asset = *vaultSle->at(sfAsset);
+
         // Decrease the Debt of the LoanBroker:
         adjustImpreciseNumber(
-            brokerDebtTotalProxy, -totalDefaultAmount, vaultAsset, vaultScale);
+            brokerDebtTotalProxy, -totalDefaultAmount, asset, vaultScale);
         // Decrease the First-Loss Capital Cover Available:
         auto coverAvailableProxy = brokerSle->at(sfCoverAvailable);
         if (coverAvailableProxy < defaultCovered)
@@ -299,20 +297,13 @@ LoanManage::impairLoan(
     ApplyView& view,
     SLE::ref loanSle,
     SLE::ref vaultSle,
-    Asset const& vaultAsset,
     beast::Journal j)
 {
     Number const lossUnrealized = owedToVault(loanSle);
 
-    // The vault may be at a different scale than the loan. Reduce rounding
-    // errors during the accounting by rounding some of the values to that
-    // scale.
-    auto const vaultScale = getAssetsTotalScale(vaultSle);
-
     // Update the Vault object(set "paper loss")
     auto vaultLossUnrealizedProxy = vaultSle->at(sfLossUnrealized);
-    adjustImpreciseNumber(
-        vaultLossUnrealizedProxy, lossUnrealized, vaultAsset, vaultScale);
+    vaultLossUnrealizedProxy += lossUnrealized;
     if (vaultLossUnrealizedProxy >
         vaultSle->at(sfAssetsTotal) - vaultSle->at(sfAssetsAvailable))
     {
@@ -343,14 +334,8 @@ LoanManage::unimpairLoan(
     ApplyView& view,
     SLE::ref loanSle,
     SLE::ref vaultSle,
-    Asset const& vaultAsset,
     beast::Journal j)
 {
-    // The vault may be at a different scale than the loan. Reduce rounding
-    // errors during the accounting by rounding some of the values to that
-    // scale.
-    auto const vaultScale = getAssetsTotalScale(vaultSle);
-
     // Update the Vault object(clear "paper loss")
     auto vaultLossUnrealizedProxy = vaultSle->at(sfLossUnrealized);
     Number const lossReversed = owedToVault(loanSle);
@@ -362,10 +347,7 @@ LoanManage::unimpairLoan(
         return tefBAD_LEDGER;
         // LCOV_EXCL_STOP
     }
-    // Reverse the "paper loss"
-    adjustImpreciseNumber(
-        vaultLossUnrealizedProxy, -lossReversed, vaultAsset, vaultScale);
-
+    vaultLossUnrealizedProxy -= lossReversed;
     view.update(vaultSle);
 
     // Update the Loan object
@@ -421,14 +403,12 @@ LoanManage::doApply()
     }
     else if (tx.isFlag(tfLoanImpair))
     {
-        if (auto const ter =
-                impairLoan(view, loanSle, vaultSle, vaultAsset, j_))
+        if (auto const ter = impairLoan(view, loanSle, vaultSle, j_))
             return ter;
     }
     else if (tx.isFlag(tfLoanUnimpair))
     {
-        if (auto const ter =
-                unimpairLoan(view, loanSle, vaultSle, vaultAsset, j_))
+        if (auto const ter = unimpairLoan(view, loanSle, vaultSle, j_))
             return ter;
     }
 

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

@@ -44,7 +44,6 @@ public:
         ApplyView& view,
         SLE::ref loanSle,
         SLE::ref vaultSle,
-        Asset const& vaultAsset,
         beast::Journal j);
 
     /** Helper function that might be needed by other transactors
@@ -54,7 +53,6 @@ public:
         ApplyView& view,
         SLE::ref loanSle,
         SLE::ref vaultSle,
-        Asset const& vaultAsset,
         beast::Journal j);
 
     TER

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

@@ -305,7 +305,7 @@ LoanPay::doApply()
     // change will be discarded.
     if (loanSle->isFlag(lsfLoanImpaired))
     {
-        LoanManage::unimpairLoan(view, loanSle, vaultSle, asset, j_);
+        LoanManage::unimpairLoan(view, loanSle, vaultSle, j_);
     }
 
     LoanPaymentType const paymentType = [&tx]() {
@@ -379,7 +379,7 @@ LoanPay::doApply()
 
     // The vault may be at a different scale than the loan. Reduce rounding
     // errors during the payment by rounding some of the values to that scale.
-    auto const vaultScale = getAssetsTotalScale(vaultSle);
+    auto const vaultScale = assetsTotalProxy.value().exponent();
 
     auto const totalPaidToVaultRaw =
         paymentParts->principalPaid + paymentParts->interestPaid;

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

@@ -383,7 +383,7 @@ LoanSet::doApply()
 
     auto vaultAvailableProxy = vaultSle->at(sfAssetsAvailable);
     auto vaultTotalProxy = vaultSle->at(sfAssetsTotal);
-    auto const vaultScale = getAssetsTotalScale(vaultSle);
+    auto const vaultScale = getVaultScale(vaultSle);
     if (vaultAvailableProxy < principalRequested)
     {
         JLOG(j_.warn())
@@ -404,8 +404,7 @@ LoanSet::doApply()
         paymentInterval,
         paymentTotal,
         TenthBips16{brokerSle->at(sfManagementFeeRate)},
-        vaultScale,
-        j_);
+        vaultScale);
 
     // Check that relevant values won't lose precision. This is mostly only
     // relevant for IOU assets.
@@ -441,10 +440,7 @@ LoanSet::doApply()
     {
         // LCOV_EXCL_START
         JLOG(j_.warn())
-            << "Computed loan properties are invalid. Does not compute."
-            << " Management fee: " << properties.managementFeeOwedToBroker
-            << ". Total Value: " << properties.totalValueOutstanding
-            << ". PeriodicPayment: " << properties.periodicPayment;
+            << "Computed loan properties are invalid. Does not compute.";
         return tecINTERNAL;
         // LCOV_EXCL_STOP
     }