Merge branch 'ximinez/online-delete-gaps' into ximinez/online-delete-lastrotated

- First approximation. Good enough and consistent enough for unit tests.
  Not sure about real world, but it'll be an improvement either way.

cfg/rippled-example.cfg

@@ -1051,10 +1051,11 @@
 #                           The online delete process checks periodically
 #                           that rippled is still in sync with the network,
 #                           and that the validated ledger is less than
-#                           'age_threshold_seconds' old. If not, then continue
+#                           'age_threshold_seconds' old, and that all
+#                           recent ledgers are available. If not, then continue
 #                           sleeping for this number of seconds and
 #                           checking until healthy.
-#                           Default is 5.
+#                           Default is 1.
 #
 #   Notes:
 #       The 'node_db' entry configures the primary, persistent storage.

src/libxrpl/ledger/View.cpp

@@ -3176,24 +3176,6 @@ requireAuth(
                 !isTesSuccess(err))
                 return err;
         }
-
-        // requireAuth is also recursive if the _account_ is a vault
-        auto const sleAccount = view.read(keylet::account(account));
-        if (!sleAccount)
-            return tefINTERNAL;  // LCOV_EXCL_LINE
-
-        if (sleAccount->isFieldPresent(sfVaultID))
-        {
-            auto const sleVault =
-                view.read(keylet::vault(sleAccount->getFieldH256(sfVaultID)));
-            if (!sleVault)
-                return tefINTERNAL;  // LCOV_EXCL_LINE
-            auto const ownerAcct = sleVault->getAccountID(sfOwner);
-            if (auto const err =
-                    requireAuth(view, mptIssue, ownerAcct, authType, depth + 1);
-                !isTesSuccess(err))
-                return err;
-        }
     }
 
     auto const mptokenID = keylet::mptoken(mptID.key, account);

src/test/app/LedgerMaster_test.cpp

@@ -2,6 +2,7 @@
 #include <test/jtx/Env.h>
 
 #include <xrpld/app/ledger/LedgerMaster.h>
+#include <xrpld/app/misc/SHAMapStore.h>
 
 namespace ripple {
 namespace test {
@@ -100,6 +101,89 @@ class LedgerMaster_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testCompleteLedgerRange(FeatureBitset features)
+    {
+        // Note that this test is intentionally very similar to
+        // SHAMapStore_test::testLedgerGaps, but has a different
+        // focus.
+
+        testcase("Complete Ledger operations");
+
+        using namespace test::jtx;
+
+        auto const deleteInterval = 8;
+
+        Env env{*this, envconfig([](auto cfg) {
+                    return online_delete(std::move(cfg), deleteInterval);
+                })};
+
+        auto const alice = Account("alice");
+        env.fund(XRP(1000), alice);
+        env.close();
+
+        auto& lm = env.app().getLedgerMaster();
+        LedgerIndex minSeq = 2;
+        LedgerIndex maxSeq = env.closed()->info().seq;
+        auto& store = env.app().getSHAMapStore();
+        store.rendezvous();
+        LedgerIndex lastRotated = store.getLastRotated();
+        BEAST_EXPECTS(maxSeq == 3, to_string(maxSeq));
+        BEAST_EXPECTS(
+            lm.getCompleteLedgers() == "2-3", lm.getCompleteLedgers());
+        BEAST_EXPECTS(lastRotated == 3, to_string(lastRotated));
+        BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0);
+        BEAST_EXPECT(
+            lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0);
+        BEAST_EXPECT(
+            lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2);
+        BEAST_EXPECT(
+            lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2);
+        BEAST_EXPECT(
+            lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2);
+
+        // Close enough ledgers to rotate a few times
+        for (int i = 0; i < 24; ++i)
+        {
+            for (int t = 0; t < 3; ++t)
+            {
+                env(noop(alice));
+            }
+            env.close();
+            store.rendezvous();
+
+            ++maxSeq;
+
+            if (maxSeq == lastRotated + deleteInterval)
+            {
+                minSeq = lastRotated;
+                lastRotated = store.getLastRotated();
+                BEAST_EXPECT(lastRotated = maxSeq + 2);
+            }
+            BEAST_EXPECTS(
+                env.closed()->info().seq == maxSeq,
+                to_string(env.closed()->info().seq));
+            BEAST_EXPECTS(
+                store.getLastRotated() == lastRotated,
+                to_string(store.getLastRotated()));
+            std::stringstream expectedRange;
+            expectedRange << minSeq << "-" << maxSeq;
+            BEAST_EXPECTS(
+                lm.getCompleteLedgers() == expectedRange.str(),
+                lm.getCompleteLedgers());
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0);
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0);
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2);
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2);
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2);
+        }
+    }
+
 public:
     void
     run() override
@@ -113,6 +197,7 @@ public:
     testWithFeats(FeatureBitset features)
     {
         testTxnIdFromIndex(features);
+        testCompleteLedgerRange(features);
     }
 };
 

src/test/app/LendingHelpers_test.cpp

@@ -1,642 +0,0 @@
-#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

@@ -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.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
@@ -1275,8 +1271,7 @@ protected:
             verifyLoanStatus,
             issuer,
             lender,
-            borrower,
-            PaymentParameters{.showStepBalances = true});
+            borrower);
     }
 
     /** Runs through the complete lifecycle of a loan
@@ -1482,8 +1477,7 @@ protected:
             state.paymentInterval,
             state.paymentRemaining,
             broker.params.managementFeeRate,
-            state.loanScale,
-            env.journal);
+            state.loanScale);
 
         verifyLoanStatus(
             0,
@@ -2454,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(
-                        payoffAmount ==
-                            broker.asset(Number(1024014840139457, -12)),
-                        to_string(payoffAmount));
-                    BEAST_EXPECT(payoffAmount > state.principalOutstanding);
-                    return payoffAmount;
-                }();
+                auto const rawPayoff = startingPayments *
+                    (state.periodicPayment + broker.asset(2).value());
+                STAmount const payoffAmount{broker.asset, rawPayoff};
+                BEAST_EXPECT(
+                    payoffAmount ==
+                    broker.asset(Number(1024014840139457, -12)));
+                BEAST_EXPECT(payoffAmount > state.principalOutstanding);
 
                 singlePayment(
                     loanKeylet,
@@ -4020,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));
@@ -4032,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();
     }
 
@@ -5007,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(
@@ -6155,16 +6144,15 @@ protected:
         // Accrued + prepayment-penalty interest based on current periodic
         // schedule
         auto const fullPaymentInterest = computeFullPaymentInterest(
-            detail::loanPrincipalFromPeriodicPayment(
-                after.periodicPayment, periodicRate2, after.paymentRemaining),
+            after.periodicPayment,
             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);
@@ -6192,9 +6180,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(
-            detail::loanPrincipalFromPeriodicPayment(
-                after.periodicPayment, periodicRate2, after.paymentRemaining),
+            after.periodicPayment,
             periodicRate2,
+            after.paymentRemaining,
             env.current()->parentCloseTime(),
             after.paymentInterval,
             prevClamped,
@@ -7205,15 +7193,15 @@ class LoanArbitrary_test : public LoanBatch_test
             .vaultDeposit = 10000,
             .debtMax = 0,
             .coverRateMin = TenthBips32{0},
-            .managementFeeRate = TenthBips16{0},
+            // .managementFeeRate = TenthBips16{5919},
             .coverRateLiquidation = TenthBips32{0}};
         LoanParameters const loanParams{
             .account = Account("lender"),
             .counter = Account("borrower"),
-            .principalRequest = Number{200000, -6},
-            .interest = TenthBips32{50000},
-            .payTotal = 2,
-            .payInterval = 200};
+            .principalRequest = Number{10000, 0},
+            // .interest = TenthBips32{0},
+            // .payTotal = 5816,
+            .payInterval = 150};
 
         runLoan(AssetType::XRP, brokerParams, loanParams);
     }

src/test/app/SHAMapStore_test.cpp

@@ -1,6 +1,7 @@
 #include <test/jtx.h>
 #include <test/jtx/envconfig.h>
 
+#include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/main/Application.h>
 #include <xrpld/app/main/NodeStoreScheduler.h>
 #include <xrpld/app/misc/SHAMapStore.h>
@@ -10,6 +11,8 @@
 #include <xrpl/nodestore/detail/DatabaseRotatingImp.h>
 #include <xrpl/protocol/jss.h>
 
+#include <thread>
+
 namespace ripple {
 namespace test {
 
@@ -20,10 +23,8 @@ class SHAMapStore_test : public beast::unit_test::suite
     static auto
     onlineDelete(std::unique_ptr<Config> cfg)
     {
-        cfg->LEDGER_HISTORY = deleteInterval;
-        auto& section = cfg->section(ConfigSection::nodeDatabase());
-        section.set("online_delete", std::to_string(deleteInterval));
-        return cfg;
+        using namespace jtx;
+        return online_delete(std::move(cfg), deleteInterval);
     }
 
     static auto
@@ -226,9 +227,10 @@ public:
 
         store.rendezvous();
 
-        BEAST_EXPECT(store.getLastRotated() == deleteInterval + 3);
+        BEAST_EXPECT(env.closed()->info().seq == deleteInterval + 3);
         lastRotated = store.getLastRotated();
-        BEAST_EXPECT(lastRotated == 11);
+        BEAST_EXPECT(lastRotated == deleteInterval + 5);
+        BEAST_EXPECT(lastRotated == 13);
 
         // That took care of the fake hashes
         ledgerCheck(env, deleteInterval + 1, 3);
@@ -236,9 +238,11 @@ public:
         accountTransactionCheck(env, 2 * deleteInterval);
 
         // The last iteration of this loop should trigger a rotate
-        for (auto i = lastRotated - 1; i < lastRotated + deleteInterval - 1;
+        for (auto i = lastRotated - 3; i < lastRotated + deleteInterval - 1;
              ++i)
         {
+            BEAST_EXPECT(store.getLastRotated() == lastRotated);
+
             env.close();
 
             ledgerTmp = env.rpc("ledger", "current");
@@ -256,7 +260,8 @@ public:
 
         store.rendezvous();
 
-        BEAST_EXPECT(store.getLastRotated() == deleteInterval + lastRotated);
+        BEAST_EXPECT(
+            store.getLastRotated() == deleteInterval + lastRotated + 2);
 
         ledgerCheck(env, deleteInterval + 1, lastRotated);
         transactionCheck(env, 0);
@@ -397,8 +402,8 @@ public:
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
 
-        BEAST_EXPECT(store.getLastRotated() == ledgerSeq - 1);
-        lastRotated = ledgerSeq - 1;
+        lastRotated = store.getLastRotated();
+        BEAST_EXPECT(lastRotated == ledgerSeq + 1);
 
         for (; ledgerSeq < lastRotated + deleteInterval; ++ledgerSeq)
         {
@@ -425,10 +430,10 @@ public:
 
         store.rendezvous();
 
-        ledgerCheck(env, ledgerSeq - firstBatch, firstBatch);
+        ledgerCheck(env, ledgerSeq - firstBatch - 2, firstBatch + 2);
 
-        BEAST_EXPECT(store.getLastRotated() == ledgerSeq - 1);
-        lastRotated = ledgerSeq - 1;
+        lastRotated = store.getLastRotated();
+        BEAST_EXPECT(lastRotated == ledgerSeq + 1);
 
         // This does not kick off a cleanup
         canDelete = env.rpc("can_delete", "always");
@@ -464,13 +469,13 @@ public:
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
 
-        BEAST_EXPECT(store.getLastRotated() == ledgerSeq - 1);
-        lastRotated = ledgerSeq - 1;
+        lastRotated = store.getLastRotated();
+        BEAST_EXPECT(lastRotated == ledgerSeq + 1);
 
         // This does not kick off a cleanup
         canDelete = env.rpc("can_delete", "now");
         BEAST_EXPECT(!RPC::contains_error(canDelete[jss::result]));
-        BEAST_EXPECT(canDelete[jss::result][jss::can_delete] == ledgerSeq - 1);
+        BEAST_EXPECT(canDelete[jss::result][jss::can_delete] == lastRotated);
 
         for (; ledgerSeq < lastRotated + deleteInterval; ++ledgerSeq)
         {
@@ -499,8 +504,8 @@ public:
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
 
-        BEAST_EXPECT(store.getLastRotated() == ledgerSeq - 1);
-        lastRotated = ledgerSeq - 1;
+        lastRotated = store.getLastRotated();
+        BEAST_EXPECT(lastRotated == ledgerSeq + 1);
     }
 
     std::unique_ptr<NodeStore::Backend>
@@ -626,6 +631,212 @@ public:
         BEAST_EXPECT(dbr->getName() == "3");
     }
 
+    void
+    testLedgerGaps()
+    {
+        // Note that this test is intentionally very similar to
+        // LedgerMaster_test::testCompleteLedgerRange, but has a different
+        // focus.
+
+        testcase("Wait for ledger gaps to fill in");
+
+        using namespace test::jtx;
+
+        Env env{*this, envconfig(onlineDelete)};
+
+        auto& ns = env.app().getNodeStore();
+        std::map<LedgerIndex, uint256> hashes;
+        auto storeHash = [&]() {
+            hashes.emplace(
+                env.current()->info().seq, env.current()->info().hash);
+
+            auto const& root =
+                ns.fetchNodeObject(hashes[env.current()->info().seq]);
+            BEAST_EXPECT(root);
+        };
+
+        auto failureMessage = [&](char const* label,
+                                  auto expected,
+                                  auto actual) {
+            std::stringstream ss;
+            ss << label << ": Expected: " << expected << ", Got: " << actual;
+            return ss.str();
+        };
+
+        auto const alice = Account("alice");
+        env.fund(XRP(1000), alice);
+        env.close();
+        storeHash();
+
+        auto& lm = env.app().getLedgerMaster();
+        LedgerIndex minSeq = 2;
+        LedgerIndex maxSeq = env.closed()->info().seq;
+        auto& store = env.app().getSHAMapStore();
+        store.rendezvous();
+        LedgerIndex lastRotated = store.getLastRotated();
+        BEAST_EXPECTS(maxSeq == 3, to_string(maxSeq));
+        BEAST_EXPECTS(
+            lm.getCompleteLedgers() == "2-3", lm.getCompleteLedgers());
+        BEAST_EXPECTS(lastRotated == 3, to_string(lastRotated));
+        BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0);
+        BEAST_EXPECT(
+            lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0);
+        BEAST_EXPECT(
+            lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2);
+        BEAST_EXPECT(
+            lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2);
+        BEAST_EXPECT(
+            lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2);
+
+        // Close enough ledgers to rotate a few times
+        while (maxSeq < 28)
+        {
+            for (int t = 0; t < 3; ++t)
+            {
+                env(noop(alice));
+            }
+            env.close();
+            storeHash();
+            store.rendezvous();
+
+            ++maxSeq;
+
+            if (maxSeq + 1 == lastRotated + deleteInterval)
+            {
+                using namespace std::chrono_literals;
+
+                // The next ledger will trigger a rotation. Delete the
+                // current ledger from LedgerMaster.
+                std::this_thread::sleep_for(100ms);
+                LedgerIndex const deleteSeq = maxSeq;
+                while (!lm.haveLedger(deleteSeq))
+                {
+                    std::this_thread::sleep_for(100ms);
+                }
+                lm.clearLedger(deleteSeq);
+
+                auto expectedRange =
+                    [](auto minSeq, auto deleteSeq, auto maxSeq) {
+                        std::stringstream expectedRange;
+                        expectedRange << minSeq << "-" << (deleteSeq - 1);
+                        if (deleteSeq + 1 == maxSeq)
+                            expectedRange << "," << maxSeq;
+                        else if (deleteSeq < maxSeq)
+                            expectedRange << "," << (deleteSeq + 1) << "-"
+                                          << maxSeq;
+                        return expectedRange.str();
+                    };
+                BEAST_EXPECTS(
+                    lm.getCompleteLedgers() ==
+                        expectedRange(minSeq, deleteSeq, maxSeq),
+                    failureMessage(
+                        "Complete ledgers",
+                        expectedRange(minSeq, deleteSeq, maxSeq),
+                        lm.getCompleteLedgers()));
+                BEAST_EXPECT(
+                    lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 1);
+
+                // Close another ledger, which will trigger a rotation, but the
+                // rotation will be stuck until the missing ledger is filled in.
+                env.close();
+                storeHash();
+                // DO NOT CALL rendezvous()! You'll end up with a deadlock.
+                ++maxSeq;
+
+                // Nothing has changed
+                BEAST_EXPECTS(
+                    store.getLastRotated() == lastRotated,
+                    failureMessage(
+                        "lastRotated", lastRotated, store.getLastRotated()));
+                BEAST_EXPECTS(
+                    lm.getCompleteLedgers() ==
+                        expectedRange(minSeq, deleteSeq, maxSeq),
+                    failureMessage(
+                        "Complete ledgers",
+                        expectedRange(minSeq, deleteSeq, maxSeq),
+                        lm.getCompleteLedgers()));
+
+                // Close 5 more ledgers, waiting one second in between to
+                // simulate the ledger making progress while online delete waits
+                // for the missing ledger to be filled in.
+                // This ensures the healthWait check has time to run and
+                // detect the gap.
+                for (int l = 0; l < 5; ++l)
+                {
+                    env.close();
+                    storeHash();
+                    // DO NOT CALL rendezvous()! You'll end up with a deadlock.
+                    ++maxSeq;
+                    // Nothing has changed
+                    BEAST_EXPECTS(
+                        store.getLastRotated() == lastRotated,
+                        failureMessage(
+                            "lastRotated",
+                            lastRotated,
+                            store.getLastRotated()));
+                    BEAST_EXPECTS(
+                        lm.getCompleteLedgers() ==
+                            expectedRange(minSeq, deleteSeq, maxSeq),
+                        failureMessage(
+                            "Complete Ledgers",
+                            expectedRange(minSeq, deleteSeq, maxSeq),
+                            lm.getCompleteLedgers()));
+                    std::this_thread::sleep_for(1s);
+                }
+
+                // Put the missing ledger back in LedgerMaster
+                lm.setLedgerRangePresent(deleteSeq, deleteSeq);
+
+                // Wait for the rotation to finish
+                store.rendezvous();
+
+                minSeq = lastRotated;
+                lastRotated = maxSeq + 2;
+
+                // Bypass caching and try to load the ledger roots from the node
+                // store
+                for (auto const& [seq, hash] : hashes)
+                {
+                    auto const nodeObject = ns.fetchNodeObject(hash);
+                    std::stringstream ss;
+                    ss << "minSeq: " << minSeq << ", maxSeq: " << maxSeq
+                       << ", search: " << seq << ". Should "
+                       << (seq < minSeq ? "NOT " : "") << "be found";
+                    if (seq < minSeq)
+                        BEAST_EXPECTS(!nodeObject, ss.str());
+                    else
+                        BEAST_EXPECTS(nodeObject, ss.str());
+                }
+            }
+            BEAST_EXPECT(maxSeq != lastRotated + deleteInterval);
+            BEAST_EXPECTS(
+                env.closed()->info().seq == maxSeq,
+                failureMessage("maxSeq", maxSeq, env.closed()->info().seq));
+            BEAST_EXPECTS(
+                store.getLastRotated() == lastRotated,
+                failureMessage(
+                    "lastRotated", lastRotated, store.getLastRotated()));
+            std::stringstream expectedRange;
+            expectedRange << minSeq << "-" << maxSeq;
+            BEAST_EXPECTS(
+                lm.getCompleteLedgers() == expectedRange.str(),
+                failureMessage(
+                    "CompleteLedgers",
+                    expectedRange.str(),
+                    lm.getCompleteLedgers()));
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0);
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0);
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2);
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2);
+            BEAST_EXPECT(
+                lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2);
+        }
+    }
+
     void
     run() override
     {
@@ -633,6 +844,7 @@ public:
         testAutomatic();
         testCanDelete();
         testRotate();
+        testLedgerGaps();
     }
 };
 

src/test/app/Vault_test.cpp

@@ -2358,18 +2358,6 @@ class Vault_test : public beast::unit_test::suite
                      .amount = asset(100)});
                 env(tx, ter(tecNO_AUTH));
 
-                // Withdrawal to other (authorized) accounts doesn't work.
-                // Issuer would have to VaultClawback
-                tx[sfDestination] = issuer.human();
-                env(tx, ter(tecNO_AUTH));
-                tx[sfDestination] = owner.human();
-                env(tx, ter(tecNO_AUTH));
-                env.close();
-
-                // Issuer reauthorizes
-                mptt.authorize({.account = issuer, .holder = depositor});
-                env.close();
-
                 // Withdrawal to other (authorized) accounts works
                 tx[sfDestination] = issuer.human();
                 env(tx);
@@ -2380,13 +2368,6 @@ class Vault_test : public beast::unit_test::suite
                 env.close();
             }
 
-            // Re-unauthorize
-            mptt.authorize(
-                {.account = issuer,
-                 .holder = depositor,
-                 .flags = tfMPTUnauthorize});
-            env.close();
-
             {
                 // Cannot deposit some more
                 auto tx = vault.deposit(

src/test/jtx/envconfig.h

@@ -58,6 +58,17 @@ envconfig(F&& modfunc, Args&&... args)
     return modfunc(envconfig(), std::forward<Args>(args)...);
 }
 
+/// @brief adjust config to enable online_delete
+///
+/// @param cfg config instance to be modified
+///
+/// @param deleteInterval how many new ledgers should be available before
+/// rotating. Defaults to 8, because the standalone minimum is 8.
+///
+/// @return unique_ptr to Config instance
+std::unique_ptr<Config>
+online_delete(std::unique_ptr<Config> cfg, std::uint32_t deleteInterval = 8);
+
 /// @brief adjust config so no admin ports are enabled
 ///
 /// this is intended for use with envconfig, as in

src/test/jtx/impl/envconfig.cpp

@@ -53,6 +53,15 @@ setupConfigForUnitTests(Config& cfg)
 
 namespace jtx {
 
+std::unique_ptr<Config>
+online_delete(std::unique_ptr<Config> cfg, std::uint32_t deleteInterval)
+{
+    cfg->LEDGER_HISTORY = deleteInterval;
+    auto& section = cfg->section(ConfigSection::nodeDatabase());
+    section.set("online_delete", std::to_string(deleteInterval));
+    return cfg;
+}
+
 std::unique_ptr<Config>
 no_admin(std::unique_ptr<Config> cfg)
 {

src/test/jtx/impl/mpt.cpp

@@ -644,7 +644,7 @@ MPTTester::operator[](std::string const& name) const
 }
 
 PrettyAmount
-MPTTester::operator()(std::int64_t amount) const
+MPTTester::operator()(std::uint64_t amount) const
 {
     return MPT("", issuanceID())(amount);
 }

src/test/jtx/mpt.h

@@ -272,7 +272,7 @@ public:
     operator[](std::string const& name) const;
 
     PrettyAmount
-    operator()(std::int64_t amount) const;
+    operator()(std::uint64_t amount) const;
 
     operator Asset() const;
 

src/test/rpc/Feature_test.cpp

@@ -187,9 +187,9 @@ class Feature_test : public beast::unit_test::suite
         BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
         jrr.removeMember(jss::status);
         BEAST_EXPECT(jrr.size() == 1);
-        BEAST_EXPECT(jrr.isMember(
-            "740352F2412A9909880C23A559FCECEDA3BE2126FED62FC7660D6"
-            "28A06927F11"));
+        BEAST_EXPECT(
+            jrr.isMember("12523DF04B553A0B1AD74F42DDB741DE8DC06A03FC089A0EF197E"
+                         "2A87F1D8107"));
         auto feature = *(jrr.begin());
 
         BEAST_EXPECTS(feature[jss::name] == "fixAMMOverflowOffer", "name");

src/xrpld/app/ledger/LedgerMaster.h

@@ -108,7 +108,10 @@ public:
     failedSave(std::uint32_t seq, uint256 const& hash);
 
     std::string
-    getCompleteLedgers();
+    getCompleteLedgers() const;
+
+    std::size_t
+    missingFromCompleteLedgerRange(LedgerIndex first, LedgerIndex last) const;
 
     /** Apply held transactions to the open ledger
         This is normally called as we close the ledger.
@@ -325,7 +328,7 @@ private:
     // A set of transactions to replay during the next close
     std::unique_ptr<LedgerReplay> replayData;
 
-    std::recursive_mutex mCompleteLock;
+    std::recursive_mutex mutable mCompleteLock;
     RangeSet<std::uint32_t> mCompleteLedgers;
 
     // Publish thread is running.

src/xrpld/app/ledger/detail/LedgerMaster.cpp

@@ -1571,12 +1571,36 @@ LedgerMaster::getPublishedLedger()
 }
 
 std::string
-LedgerMaster::getCompleteLedgers()
+LedgerMaster::getCompleteLedgers() const
 {
     std::lock_guard sl(mCompleteLock);
     return to_string(mCompleteLedgers);
 }
 
+std::size_t
+LedgerMaster::missingFromCompleteLedgerRange(
+    LedgerIndex first,
+    LedgerIndex last) const
+{
+    // Make a copy of the range to avoid holding the lock
+    auto const range = [&] {
+        std::lock_guard sl(mCompleteLock);
+        return mCompleteLedgers;
+    }();
+
+    std::size_t missing = 0;
+
+    for (LedgerIndex idx = first; idx <= last; ++idx)
+    {
+        if (!boost::icl::contains(range, idx))
+        {
+            ++missing;
+        }
+    }
+
+    return missing;
+}
+
 std::optional<NetClock::time_point>
 LedgerMaster::getCloseTimeBySeq(LedgerIndex ledgerIndex)
 {

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
@@ -203,6 +202,14 @@ 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(
@@ -232,6 +239,17 @@ 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
@@ -369,58 +387,6 @@ 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,
@@ -452,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/SHAMapStoreImp.cpp

@@ -289,6 +289,18 @@ SHAMapStoreImp::run()
             validatedSeq >= lastRotated + deleteInterval_ &&
             canDelete_ >= lastRotated - 1 && healthWait() == keepGoing;
 
+        JLOG(journal_.debug())
+            << "run: Setting lastGoodValidatedLedger_ to " << validatedSeq;
+
+        {
+            // Note that this is set after the healthWait() check, so that we
+            // don't start the rotation until the validated ledger is fully
+            // processed. It is not guaranteed to be done at this point. It also
+            // allows the testLedgerGaps unit test to work.
+            std::unique_lock<std::mutex> lock(mutex_);
+            lastGoodValidatedLedger_ = validatedSeq;
+        }
+
         // will delete up to (not including) lastRotated
         if (readyToRotate)
         {
@@ -297,7 +309,9 @@ SHAMapStoreImp::run()
                 << lastRotated << " deleteInterval " << deleteInterval_
                 << " canDelete_ " << canDelete_ << " state "
                 << app_.getOPs().strOperatingMode(false) << " age "
-                << ledgerMaster_->getValidatedLedgerAge().count() << 's';
+                << ledgerMaster_->getValidatedLedgerAge().count()
+                << "s. Complete ledgers: "
+                << ledgerMaster_->getCompleteLedgers();
 
             clearPrior(lastRotated);
             if (healthWait() == stopping)
@@ -345,7 +359,13 @@ SHAMapStoreImp::run()
             if (healthWait() == stopping)
                 return;
 
-            lastRotated = validatedSeq;
+            // The rotation process takes time, and more ledgers - possibly many
+            // more - may have been written to the now-archive database before
+            // the writable database was created. Update the validatedSeq to the
+            // current validated ledger sequence plus a buffer to ensure that
+            // all modified nodes for that ledger forward are all written to the
+            // new writable database.
+            lastRotated = ledgerMaster_->getValidLedgerIndex() + 2;
 
             dbRotating_->rotate(
                 std::move(newBackend),
@@ -360,7 +380,10 @@ SHAMapStoreImp::run()
                     clearCaches(validatedSeq);
                 });
 
-            JLOG(journal_.warn()) << "finished rotation " << validatedSeq;
+            JLOG(journal_.warn())
+                << "finished rotation. validatedSeq: " << validatedSeq
+                << ", lastRotated: " << lastRotated << ". Complete ledgers: "
+                << ledgerMaster_->getCompleteLedgers();
         }
     }
 }
@@ -615,22 +638,47 @@ SHAMapStoreImp::clearPrior(LedgerIndex lastRotated)
 SHAMapStoreImp::HealthResult
 SHAMapStoreImp::healthWait()
 {
+    auto index = ledgerMaster_->getValidLedgerIndex();
     auto age = ledgerMaster_->getValidatedLedgerAge();
     OperatingMode mode = netOPs_->getOperatingMode();
     std::unique_lock lock(mutex_);
-    while (!stop_ && (mode != OperatingMode::FULL || age > ageThreshold_))
+
+    auto numMissing = ledgerMaster_->missingFromCompleteLedgerRange(
+        lastGoodValidatedLedger_, index);
+    while (
+        !stop_ &&
+        (mode != OperatingMode::FULL || age > ageThreshold_ || numMissing > 0))
     {
+        // this value shouldn't change, so grab it while we have the
+        // lock
+        auto const lastGood = lastGoodValidatedLedger_;
+
         lock.unlock();
-        JLOG(journal_.warn()) << "Waiting " << recoveryWaitTime_.count()
-                              << "s for node to stabilize. state: "
-                              << app_.getOPs().strOperatingMode(mode, false)
-                              << ". age " << age.count() << 's';
+
+        auto const stream = mode != OperatingMode::FULL || age > ageThreshold_
+            ? journal_.warn()
+            : journal_.info();
+        JLOG(stream) << "Waiting " << recoveryWaitTime_.count()
+                     << "s for node to stabilize. state: "
+                     << app_.getOPs().strOperatingMode(mode, false) << ". age "
+                     << age.count() << "s. Missing ledgers: " << numMissing
+                     << ". Expect: " << lastGood << "-" << index
+                     << ". Complete ledgers: "
+                     << ledgerMaster_->getCompleteLedgers();
         std::this_thread::sleep_for(recoveryWaitTime_);
+        index = ledgerMaster_->getValidLedgerIndex();
         age = ledgerMaster_->getValidatedLedgerAge();
         mode = netOPs_->getOperatingMode();
+        numMissing =
+            ledgerMaster_->missingFromCompleteLedgerRange(lastGood, index);
+
         lock.lock();
     }
 
+    JLOG(journal_.debug()) << "healthWait: Setting lastGoodValidatedLedger_ to "
+                           << index;
+    lastGoodValidatedLedger_ = index;
+
     return stop_ ? stopping : keepGoing;
 }
 

src/xrpld/app/misc/SHAMapStoreImp.h

@@ -71,6 +71,11 @@ private:
     std::thread thread_;
     bool stop_ = false;
     bool healthy_ = true;
+    // Used to prevent ledger gaps from forming during online deletion. Keeps
+    // track of the last validated ledger that was processed without gaps. There
+    // are no guarantees about gaps while online delete is not running. For
+    // that, use advisory_delete and check for gaps externally.
+    LedgerIndex lastGoodValidatedLedger_ = 0;
     mutable std::condition_variable cond_;
     mutable std::condition_variable rendezvous_;
     mutable std::mutex mutex_;
@@ -84,11 +89,11 @@ private:
     std::uint32_t deleteBatch_ = 100;
     std::chrono::milliseconds backOff_{100};
     std::chrono::seconds ageThreshold_{60};
-    /// If  the node is out of sync during an online_delete healthWait()
-    /// call, sleep the thread for this time, and continue checking until
-    /// recovery.
+    /// If  the node is out of sync, or any recent ledgers are not
+    /// available during an online_delete healthWait() call, sleep
+    /// the thread for this time, and continue checking until recovery.
     /// See also: "recovery_wait_seconds" in rippled-example.cfg
-    std::chrono::seconds recoveryWaitTime_{5};
+    std::chrono::seconds recoveryWaitTime_{1};
 
     // these do not exist upon SHAMapStore creation, but do exist
     // as of run() or before
@@ -212,6 +217,8 @@ private:
     enum HealthResult { stopping, keepGoing };
     [[nodiscard]] HealthResult
     healthWait();
+    bool
+    hasCompleteRange(LedgerIndex first, LedgerIndex last);
 
 public:
     void

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

@@ -100,9 +100,6 @@ 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;
@@ -135,6 +132,27 @@ 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.
  *
@@ -146,9 +164,6 @@ loanPrincipalFromPeriodicPayment(
     Number const& periodicRate,
     std::uint32_t paymentsRemaining)
 {
-    if (paymentsRemaining == 0)
-        return numZero;
-
     if (periodicRate == 0)
         return periodicPayment * paymentsRemaining;
 
@@ -156,6 +171,21 @@ 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.
  *
@@ -186,12 +216,6 @@ 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
@@ -224,9 +248,6 @@ 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();
 
@@ -430,8 +451,7 @@ tryOverpayment(
         paymentInterval,
         paymentRemaining,
         managementFeeRate,
-        loanScale,
-        j);
+        loanScale);
 
     JLOG(j.debug()) << "new periodic payment: "
                     << newLoanProperties.periodicPayment
@@ -527,28 +547,21 @@ tryOverpayment(
 
     auto const deltas = rounded - newRounded;
 
-    // The change in loan management fee is equal to the change between the old
-    // and the new outstanding management fees
-    XRPL_ASSERT_PARTS(
-        deltas.managementFee ==
-            rounded.managementFeeDue - managementFeeOutstanding,
-        "ripple::detail::tryOverpayment",
-        "no fee change");
-
     auto const hypotheticalValueOutstanding =
         rounded.valueOutstanding - deltas.principal;
 
     // 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 "
                           "the loan. Ignore the overpayment";
         return Unexpected(tesSUCCESS);
     }
+
     return LoanPaymentParts{
         // Principal paid is the reduction in principal outstanding
         .principalPaid = deltas.principal,
@@ -663,6 +676,12 @@ doOverpayment(
         "ripple::detail::doOverpayment",
         "principal change agrees");
 
+    XRPL_ASSERT_PARTS(
+        overpaymentComponents.trackedManagementFeeDelta ==
+            managementFeeOutstandingProxy - managementFeeOutstanding,
+        "ripple::detail::doOverpayment",
+        "no fee change");
+
     // I'm not 100% sure the following asserts are correct. If in doubt, and
     // everything else works, remove any that cause trouble.
 
@@ -693,6 +712,13 @@ doOverpayment(
         "ripple::detail::doOverpayment",
         "principal payment matches");
 
+    XRPL_ASSERT_PARTS(
+        loanPaymentParts.feePaid ==
+            overpaymentComponents.untrackedManagementFee +
+                overpaymentComponents.trackedManagementFeeDelta,
+        "ripple::detail::doOverpayment",
+        "fee payment matches");
+
     // All validations passed, so update the proxy objects (which will
     // modify the actual Loan ledger object)
     totalValueOutstandingProxy = totalValueOutstanding;
@@ -1205,12 +1231,17 @@ 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] =
         [&]() {
-            auto const interest = roundToAsset(
-                asset,
-                tenthBipsOfValue(overpayment, overpaymentInterestRate),
-                loanScale);
+            Number const interest =
+                roundToAsset(asset, rawOverpaymentInterest, loanScale);
             return detail::computeInterestAndFeeParts(
                 asset, interest, managementFeeRate, loanScale);
         }();
@@ -1404,6 +1435,31 @@ 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.
  *
@@ -1465,6 +1521,21 @@ 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
@@ -1540,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(
@@ -1552,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,
@@ -1582,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);
@@ -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_STOP
+            // LCOV_EXCL_START
         }
     }
 

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
     }

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

@@ -1,6 +1,4 @@
 #include <xrpld/app/tx/detail/VaultWithdraw.h>
-//
-#include <xrpld/app/tx/detail/Payment.h>
 
 #include <xrpl/ledger/CredentialHelpers.h>
 #include <xrpl/ledger/View.h>
@@ -199,7 +197,7 @@ VaultWithdraw::doApply()
     assetsAvailable -= assetsWithdrawn;
     view().update(vault);
 
-    AccountID const& vaultAccount = vault->at(sfAccount);
+    auto const& vaultAccount = vault->at(sfAccount);
     // Transfer shares from depositor to vault.
     if (auto const ter = accountSend(
             view(),