Merge branch 'develop' into ximinez/directory

This change mainly includes XRPLF/actions#51.

.clang-tidy

@@ -30,10 +30,12 @@ Checks: "-*,
   bugprone-multiple-statement-macro,
   bugprone-no-escape,
   bugprone-non-zero-enum-to-bool-conversion,
+  bugprone-optional-value-conversion,
   bugprone-parent-virtual-call,
   bugprone-pointer-arithmetic-on-polymorphic-object,
   bugprone-posix-return,
   bugprone-redundant-branch-condition,
+  bugprone-reserved-identifier,
   bugprone-return-const-ref-from-parameter,
   bugprone-shared-ptr-array-mismatch,
   bugprone-signal-handler,
@@ -54,9 +56,11 @@ Checks: "-*,
   bugprone-suspicious-realloc-usage,
   bugprone-suspicious-semicolon,
   bugprone-suspicious-string-compare,
+  bugprone-suspicious-stringview-data-usage,
   bugprone-swapped-arguments,
   bugprone-terminating-continue,
   bugprone-throw-keyword-missing,
+  bugprone-too-small-loop-variable,
   bugprone-undefined-memory-manipulation,
   bugprone-undelegated-constructor,
   bugprone-unhandled-exception-at-new,
@@ -92,12 +96,8 @@ Checks: "-*,
 #
 # bugprone-crtp-constructor-accessibility,
 # bugprone-inc-dec-in-conditions,
-# bugprone-reserved-identifier,
 # bugprone-move-forwarding-reference,
 # bugprone-switch-missing-default-case,
-# bugprone-suspicious-stringview-data-usage,
-# bugprone-optional-value-conversion,
-# bugprone-too-small-loop-variable,
 # bugprone-unused-return-value,
 # bugprone-use-after-move,
 # bugprone-unhandled-self-assignment,

.github/workflows/pre-commit.yml

@@ -11,7 +11,7 @@ on:
 jobs:
   # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks.
   run-hooks:
-    uses: XRPLF/actions/.github/workflows/pre-commit.yml@56de1bdf19639e009639a50b8d17c28ca954f267
+    uses: XRPLF/actions/.github/workflows/pre-commit.yml@44856eb0d6ecb7d376370244324ab3dc8b863bad
     with:
       runs_on: ubuntu-latest
       container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-41ec7c1" }'

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_FIX   (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (BatchInnerSigs,             Supported::no, VoteBehavior::DefaultNo)

src/libxrpl/beast/core/CurrentThreadName.cpp

@@ -62,7 +62,8 @@ namespace beast::detail {
 inline void
 setCurrentThreadNameImpl(std::string_view name)
 {
-    pthread_setname_np(name.data());
+    // The string is assumed to be null terminated
+    pthread_setname_np(name.data());  // NOLINT(bugprone-suspicious-stringview-data-usage)
 }
 
 }  // namespace beast::detail
@@ -85,7 +86,7 @@ setCurrentThreadNameImpl(std::string_view name)
         sizeof(boundedName),
         "%.*s",
         static_cast<int>(maxThreadNameLength),
-        name.data());
+        name.data());  // NOLINT(bugprone-suspicious-stringview-data-usage)
 
     pthread_setname_np(pthread_self(), boundedName);
 

src/libxrpl/ledger/ApplyView.cpp

@@ -10,6 +10,14 @@ namespace xrpl {
 
 namespace directory {
 
+struct Gap
+{
+    uint64_t const page;
+    SLE::pointer node;
+    uint64_t const nextPage;
+    SLE::pointer next;
+};
+
 std::uint64_t
 createRoot(
     ApplyView& view,
@@ -111,7 +119,9 @@ insertPage(
     if (page == 0)
         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:
@@ -133,12 +143,8 @@ insertPage(
     // it's the default.
     if (page != 1)
         node->setFieldU64(sfIndexPrevious, page - 1);
-    XRPL_ASSERT_PARTS(!nextPage, "xrpl::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);
 
@@ -154,7 +160,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)
     {
@@ -164,6 +170,43 @@ 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/libxrpl/protocol/Permissions.cpp

@@ -86,7 +86,7 @@ Permission::getPermissionName(std::uint32_t const value) const
 {
     auto const permissionValue = static_cast<GranularPermissionType>(value);
     if (auto const granular = getGranularName(permissionValue))
-        return *granular;
+        return granular;
 
     // not a granular permission, check if it maps to a transaction type
     auto const txType = permissionToTxType(value);

src/libxrpl/tx/invariants/InvariantCheck.cpp

@@ -488,8 +488,9 @@ AccountRootsDeletedClean::finalize(
                 return false;
         }
         // Simple types
-        for (auto const& [keyletfunc, _, __] : directAccountKeylets)
+        for (auto const& [keyletfunc, _1, _2] : directAccountKeylets)
         {
+            // TODO: use '_' for both unused variables above once we are in C++26
             if (objectExists(std::invoke(keyletfunc, accountID)) && enforce)
                 return false;
         }

src/test/app/AMM_test.cpp

@@ -2342,7 +2342,7 @@ private:
             // The vote is not added to the slots
             ammAlice.vote(carol, 1'000);
             auto const info = ammAlice.ammRpcInfo()[jss::amm][jss::vote_slots];
-            for (std::uint16_t i = 0; i < info.size(); ++i)
+            for (std::uint32_t i = 0; i < info.size(); ++i)
                 BEAST_EXPECT(info[i][jss::account] != carol.human());
             // But the slots are refreshed and the fee is changed
             BEAST_EXPECT(ammAlice.expectTradingFee(82));

src/test/app/PayStrand_test.cpp

@@ -262,7 +262,7 @@ public:
             if (has(SB::sameAccIss))
                 return acc;
             if (has(SB::existingIss) && existingIss)
-                return *existingIss;
+                return existingIss;
             return issF().id();
         }();
         auto const cur = [&]() -> std::optional<Currency> {
@@ -271,7 +271,7 @@ public:
             if (has(SB::xrp))
                 return xrpCurrency();
             if (has(SB::existingCur) && existingCur)
-                return *existingCur;
+                return existingCur;
             return currencyF();
         }();
         if (!has(SB::boundary))

src/test/app/ValidatorList_test.cpp

@@ -297,7 +297,7 @@ private:
             auto const localSigningPublic =
                 parseBase58<PublicKey>(TokenType::NodePublic, cfgKeys.front());
 
-            BEAST_EXPECT(trustedKeys->load(*localSigningPublic, cfgKeys, emptyCfgPublishers));
+            BEAST_EXPECT(trustedKeys->load(localSigningPublic, cfgKeys, emptyCfgPublishers));
 
             BEAST_EXPECT(trustedKeys->localPublicKey() == localSigningPublic);
             BEAST_EXPECT(trustedKeys->listed(*localSigningPublic));

src/test/core/Config_test.cpp

@@ -1280,7 +1280,7 @@ r.ripple.com:51235
         for (auto const& t : tests)
         {
             Section s;
-            s.append(t.line.data());
+            s.append(std::string(t.line));
             BEAST_EXPECT(s.had_trailing_comments() == t.had_comment);
             if (t.field.empty())
             {
@@ -1289,7 +1289,7 @@ r.ripple.com:51235
             else
             {
                 std::string field;
-                BEAST_EXPECTS(set(field, t.field.data(), s), t.line);
+                BEAST_EXPECTS(set(field, std::string(t.field), s), t.line);
                 BEAST_EXPECTS(field == t.expect, t.line);
             }
         }

src/test/rpc/AccountInfo_test.cpp

@@ -502,11 +502,12 @@ public:
             Json::Value params;
             params[jss::account] = account.human();
             auto const info = env.rpc("json", "account_info", to_string(params));
+            auto const name = std::string(fName);
 
             std::optional<bool> res;
             if (info[jss::result][jss::status] == "success" &&
-                info[jss::result][jss::account_flags].isMember(fName.data()))
-                res.emplace(info[jss::result][jss::account_flags][fName.data()].asBool());
+                info[jss::result][jss::account_flags].isMember(name))
+                res.emplace(info[jss::result][jss::account_flags][name].asBool());
 
             return res;
         };

src/xrpld/overlay/detail/OverlayImpl.cpp

@@ -825,7 +825,7 @@ OverlayImpl::processValidatorList(http_request_type const& req, Handoff& handoff
     // return the most recent validator list for that key.
     constexpr std::string_view prefix("/vl/");
 
-    if (!req.target().starts_with(prefix.data()) || !setup_.vlEnabled)
+    if (!req.target().starts_with(prefix) || !setup_.vlEnabled)
         return false;
 
     std::uint32_t version = 1;

src/xrpld/rpc/detail/DeliveredAmount.cpp

@@ -34,7 +34,7 @@ getDeliveredAmount(
     if (auto const& deliveredAmount = transactionMeta.getDeliveredAmount();
         deliveredAmount.has_value())
     {
-        return *deliveredAmount;
+        return deliveredAmount;
     }
 
     if (serializedTx->isFieldPresent(sfAmount))

src/xrpld/rpc/handlers/Simulate.cpp

@@ -126,7 +126,7 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context)
     }
 
     if (auto error = autofillSignature(tx_json))
-        return *error;
+        return error;
 
     if (!tx_json.isMember(jss::Sequence))
     {