Merge branch 'develop' into ximinez/emptydirectoryinvariant

Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>

.github/workflows/reusable-build-test-config.yml

@@ -210,6 +210,22 @@ jobs:
           retention-days: 3
           if-no-files-found: error
 
+      - name: Export server definitions
+        if: ${{ runner.os != 'Windows' && !inputs.build_only && env.VOIDSTAR_ENABLED != 'true' }}
+        working-directory: ${{ env.BUILD_DIR }}
+        run: |
+          set -o pipefail
+          ./xrpld --definitions | python3 -m json.tool > server_definitions.json
+
+      - name: Upload server definitions
+        if: ${{ github.event.repository.visibility == 'public' && inputs.config_name == 'debian-bookworm-gcc-13-amd64-release' }}
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        with:
+          name: server-definitions
+          path: ${{ env.BUILD_DIR }}/server_definitions.json
+          retention-days: 3
+          if-no-files-found: error
+
       - name: Check linking (Linux)
         if: ${{ runner.os == 'Linux' && env.SANITIZERS_ENABLED == 'false' }}
         working-directory: ${{ env.BUILD_DIR }}

.pre-commit-config.yaml

@@ -20,6 +20,22 @@ repos:
       - id: check-merge-conflict
         args: [--assume-in-merge]
 
+  - repo: local
+    hooks:
+      - id: clang-tidy
+        name: "clang-tidy (enable with: TIDY=1)"
+        entry: ./bin/pre-commit/clang_tidy_check.py
+        language: python
+        types_or: [c++, c]
+        exclude: ^include/xrpl/protocol_autogen
+        pass_filenames: false # script determines the staged files itself
+      - id: fix-include-style
+        name: fix include style
+        entry: ./bin/pre-commit/fix_include_style.py
+        language: python
+        types_or: [c++, c]
+        exclude: ^include/xrpl/protocol_autogen/(transactions|ledger_entries)/
+
   - repo: https://github.com/pre-commit/mirrors-clang-format
     rev: cd481d7b0bfb5c7b3090c21846317f9a8262e891 # frozen: v22.1.0
     hooks:
@@ -67,14 +83,6 @@ repos:
 
   - repo: local
     hooks:
-      - id: clang-tidy
-        name: "clang-tidy (enable with: TIDY=1)"
-        entry: ./bin/pre-commit/clang_tidy_check.py
-        language: python
-        types_or: [c++, c]
-        exclude: ^include/xrpl/protocol_autogen
-        pass_filenames: false # script determines the staged files itself
-
       - id: nix-fmt
         name: Format Nix files
         entry: |

bin/pre-commit/fix_include_style.py

@@ -0,0 +1,37 @@
+#!/usr/bin/env python3
+
+"""
+Converts quoted includes (#include "...") to angle-bracket includes
+(#include <...>), which is the required style in this project.
+
+Usage: ./bin/pre-commit/fix_include_style.py <file1> <file2> ...
+"""
+
+import re
+import sys
+from pathlib import Path
+
+PATTERN = re.compile(r'^(\s*#include\s*)"([^"]+)"', re.MULTILINE)
+
+
+def fix_includes(path: Path) -> bool:
+    original = path.read_text(encoding="utf-8")
+    fixed = PATTERN.sub(r"\1<\2>", original)
+    if fixed != original:
+        path.write_text(fixed, encoding="utf-8")
+        return False
+    return True
+
+
+def main() -> int:
+    files = [Path(f) for f in sys.argv[1:]]
+    success = True
+
+    for path in files:
+        success &= fix_includes(path)
+
+    return 0 if success else 1
+
+
+if __name__ == "__main__":
+    sys.exit(main())

include/xrpl/basics/SlabAllocator.h

@@ -284,7 +284,7 @@ class SlabAllocatorSet
 {
 private:
     // The list of allocators that belong to this set
-    boost::container::static_vector<SlabAllocator<Type>, 64> allocators_;
+    boost::container::static_vector<SlabAllocator<Type>, 64> allocators_{};
 
     std::size_t maxSize_ = 0;
 

include/xrpl/basics/hardened_hash.h

@@ -72,14 +72,12 @@ template <class HashAlgorithm = beast::xxhasher>
 class hardened_hash
 {
 private:
-    detail::seed_pair m_seeds;
+    detail::seed_pair m_seeds{detail::make_seed_pair<>()};
 
 public:
     using result_type = typename HashAlgorithm::result_type;
 
-    hardened_hash() : m_seeds(detail::make_seed_pair<>())
-    {
-    }
+    hardened_hash() = default;
 
     template <class T>
     result_type

include/xrpl/basics/partitioned_unordered_map.h

@@ -57,7 +57,7 @@ public:
     {
         using iterator_category = std::forward_iterator_tag;
         partition_map_type* map_{nullptr};
-        typename partition_map_type::iterator ait_;
+        typename partition_map_type::iterator ait_{};
         typename map_type::iterator mit_;
 
         iterator() = default;
@@ -126,7 +126,7 @@ public:
         using iterator_category = std::forward_iterator_tag;
 
         partition_map_type* map_{nullptr};
-        typename partition_map_type::iterator ait_;
+        typename partition_map_type::iterator ait_{};
         typename map_type::iterator mit_;
 
         const_iterator() = default;

include/xrpl/beast/core/LockFreeStack.h

@@ -24,9 +24,7 @@ public:
     using reference = std::
         conditional_t<IsConst, typename Container::const_reference, typename Container::reference>;
 
-    LockFreeStackIterator() : m_node()
-    {
-    }
+    LockFreeStackIterator() = default;
 
     LockFreeStackIterator(NodePtr node) : m_node(node)
     {
@@ -79,7 +77,7 @@ public:
     }
 
 private:
-    NodePtr m_node;
+    NodePtr m_node{};
 };
 
 //------------------------------------------------------------------------------

include/xrpl/core/ClosureCounter.h

@@ -72,7 +72,7 @@ private:
     {
     private:
         ClosureCounter& counter_;
-        std::remove_reference_t<Closure> closure_;
+        std::remove_reference_t<Closure> closure_{};
 
         static_assert(
             std::is_same_v<decltype(closure_(std::declval<Args_t>()...)), Ret_t>,

include/xrpl/ledger/detail/ReadViewFwdRange.h

@@ -102,7 +102,7 @@ public:
 
     private:
         ReadView const* view_ = nullptr;
-        std::unique_ptr<iter_base> impl_;
+        std::unique_ptr<iter_base> impl_{};
         std::optional<value_type> mutable cache_;
     };
 

include/xrpl/ledger/helpers/NFTokenHelpers.h

@@ -20,6 +20,10 @@ removeTokenOffersWithLimit(
     Keylet const& directory,
     std::size_t maxDeletableOffers);
 
+/** Returns tesSUCCESS if NFToken has few enough offers that it can be burned */
+TER
+notTooManyOffers(ReadView const& view, uint256 const& nftokenID);
+
 /** Finds the specified token in the owner's token directory. */
 std::optional<STObject>
 findToken(ReadView const& view, AccountID const& owner, uint256 const& nftokenID);

include/xrpl/protocol/KnownFormats.h

@@ -179,10 +179,10 @@ private:
     // One of the situations where a std::forward_list is useful.  We want to
     // store each Item in a place where its address won't change.  So a node-
     // based container is appropriate.  But we don't need searchability.
-    std::forward_list<Item> formats_;
+    std::forward_list<Item> formats_{};
 
-    boost::container::flat_map<std::string, Item const*> names_;
-    boost::container::flat_map<KeyType, Item const*> types_;
+    boost::container::flat_map<std::string, Item const*> names_{};
+    boost::container::flat_map<KeyType, Item const*> types_{};
     friend Derived;
 };
 

include/xrpl/protocol/STBitString.h

@@ -19,7 +19,7 @@ public:
     using value_type = base_uint<Bits>;
 
 private:
-    value_type value_;
+    value_type value_{};
 
 public:
     STBitString() = default;

include/xrpl/rdb/DatabaseCon.h

@@ -94,7 +94,7 @@ public:
 
     struct CheckpointerSetup
     {
-        JobQueue* jobQueue;
+        JobQueue* jobQueue{};
         std::reference_wrapper<ServiceRegistry> registry;
     };
 

include/xrpl/tx/SignerEntries.h

@@ -42,10 +42,10 @@ public:
         }
 
         // For sorting to look for duplicate accounts
-        friend bool
-        operator<(SignerEntry const& lhs, SignerEntry const& rhs)
+        friend auto
+        operator<=>(SignerEntry const& lhs, SignerEntry const& rhs)
         {
-            return lhs.account < rhs.account;
+            return lhs.account <=> rhs.account;
         }
 
         friend bool

include/xrpl/tx/applySteps.h

@@ -202,7 +202,7 @@ public:
 
     /// Success flag - whether the transaction is likely to
     /// claim a fee
-    bool const likelyToClaimFee;
+    bool const likelyToClaimFee{};
 
     /// Constructor
     template <class Context>

include/xrpl/tx/invariants/InvariantCheck.h

@@ -372,6 +372,22 @@ public:
     finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);
 };
 
+/**
+ * @brief Invariants: An account's directory should never be empty
+ *
+ */
+class NoEmptyDirectory
+{
+    bool bad_ = false;
+
+public:
+    void
+    visitEntry(bool, std::shared_ptr<SLE const> const&, std::shared_ptr<SLE const> const&);
+
+    bool
+    finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);
+};
+
 // additional invariant checks can be declared above and then added to this
 // tuple
 using InvariantChecks = std::tuple<
@@ -399,7 +415,8 @@ using InvariantChecks = std::tuple<
     ValidLoanBroker,
     ValidLoan,
     ValidVault,
-    ValidMPTPayment>;
+    ValidMPTPayment,
+    NoEmptyDirectory>;
 
 /**
  * @brief get a tuple of all invariant checks

src/libxrpl/ledger/ApplyView.cpp

@@ -253,6 +253,7 @@ ApplyView::emptyDirDelete(Keylet const& directory)
 bool
 ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const& key, bool keepRoot)
 {
+    keepRoot = false;
     auto node = peek(keylet::page(directory, page));
 
     if (!node)

src/libxrpl/ledger/PaymentSandbox.cpp

@@ -6,6 +6,7 @@
 #include <xrpl/ledger/RawView.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/Issue.h>
 #include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/MPTIssue.h>
 #include <xrpl/protocol/SField.h>

src/libxrpl/ledger/helpers/CredentialHelpers.cpp

@@ -15,6 +15,7 @@
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STArray.h>
 #include <xrpl/protocol/STLedgerEntry.h>
+#include <xrpl/protocol/STObject.h>
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/STVector256.h>
 #include <xrpl/protocol/TER.h>

src/libxrpl/ledger/helpers/NFTokenHelpers.cpp

@@ -621,6 +621,33 @@ removeTokenOffersWithLimit(ApplyView& view, Keylet const& directory, std::size_t
     return deletedOffersCount;
 }
 
+TER
+notTooManyOffers(ReadView const& view, uint256 const& nftokenID)
+{
+    std::size_t totalOffers = 0;
+
+    {
+        Dir const buys(view, keylet::nft_buys(nftokenID));
+        for (auto iter = buys.begin(); iter != buys.end(); iter.next_page())
+        {
+            totalOffers += iter.page_size();
+            if (totalOffers > maxDeletableTokenOfferEntries)
+                return tefTOO_BIG;
+        }
+    }
+
+    {
+        Dir const sells(view, keylet::nft_sells(nftokenID));
+        for (auto iter = sells.begin(); iter != sells.end(); iter.next_page())
+        {
+            totalOffers += iter.page_size();
+            if (totalOffers > maxDeletableTokenOfferEntries)
+                return tefTOO_BIG;
+        }
+    }
+    return tesSUCCESS;
+}
+
 bool
 deleteTokenOffer(ApplyView& view, std::shared_ptr<SLE> const& offer)
 {

src/libxrpl/tx/invariants/InvariantCheck.cpp

@@ -1043,4 +1043,42 @@ NoModifiedUnmodifiableFields::finalize(
     return true;
 }
 
+//------------------------------------------------------------------------------
+
+void
+NoEmptyDirectory::visitEntry(
+    bool isDelete,
+    std::shared_ptr<SLE const> const& before,
+    std::shared_ptr<SLE const> const& after)
+{
+    if (isDelete)
+        return;
+    if (before && before->getType() != ltDIR_NODE)
+        return;
+    if (after && after->getType() != ltDIR_NODE)
+        return;
+    if (!after->isFieldPresent(sfOwner))
+        // Not an account dir
+        return;
+
+    bad_ = after->at(sfIndexes).empty();
+}
+
+bool
+NoEmptyDirectory::finalize(
+    STTx const& tx,
+    TER const result,
+    XRPAmount const,
+    ReadView const& view,
+    beast::Journal const& j)
+{
+    if (bad_)
+    {
+        JLOG(j.fatal()) << "Invariant failed: empty owner directory.";
+        return false;
+    }
+
+    return true;
+}
+
 }  // namespace xrpl

src/libxrpl/tx/transactors/account/SignerListSet.cpp

@@ -257,11 +257,8 @@ SignerListSet::validateQuorumAndSignerEntries(
     }
 
     // Make sure there are no duplicate signers.
-    // SignerEntry only defines operator< and operator==, not the full
-    // std::totally_ordered set required by std::ranges::less, so the
-    // ranges version does not compile. NOLINTNEXTLINE(modernize-use-ranges)
     XRPL_ASSERT(
-        std::is_sorted(signers.begin(), signers.end()),
+        std::ranges::is_sorted(signers),
         "xrpl::SignerListSet::validateQuorumAndSignerEntries : sorted "
         "signers");
     if (std::ranges::adjacent_find(signers) != signers.end())

src/libxrpl/tx/transactors/delegate/DelegateSet.cpp

@@ -1,6 +1,7 @@
 #include <xrpl/tx/transactors/delegate/DelegateSet.h>
 
 #include <xrpl/basics/Log.h>
+#include <xrpl/beast/utility/Journal.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/helpers/AccountRootHelpers.h>
 #include <xrpl/ledger/helpers/DirectoryHelpers.h>

src/libxrpl/tx/transactors/nft/NFTokenMint.cpp

@@ -13,6 +13,7 @@
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/SOTemplate.h>
 #include <xrpl/protocol/STLedgerEntry.h>
+#include <xrpl/protocol/STObject.h>
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>

src/test/app/PayChan_test.cpp

@@ -41,7 +41,6 @@
 
 #include <algorithm>
 #include <cassert>
-#include <chrono>
 #include <cstddef>
 #include <cstdint>
 #include <iterator>

src/test/rpc/ServerDefinitions_test.cpp

@@ -1,6 +1,7 @@
-
 #include <test/jtx/Env.h>
 
+#include <xrpld/rpc/handlers/server_info/ServerDefinitions.h>
+
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/SOTemplate.h>
@@ -451,10 +452,40 @@ public:
         }
     }
 
+    void
+    testGetServerDefinitionsJson()
+    {
+        testcase("getServerDefinitionsJson");
+
+        auto const& defs = getServerDefinitionsJson();
+        for (auto const& field :
+             {jss::ACCOUNT_SET_FLAGS,
+              jss::FIELDS,
+              jss::LEDGER_ENTRY_FLAGS,
+              jss::LEDGER_ENTRY_FORMATS,
+              jss::LEDGER_ENTRY_TYPES,
+              jss::TRANSACTION_FLAGS,
+              jss::TRANSACTION_FORMATS,
+              jss::TRANSACTION_RESULTS,
+              jss::TRANSACTION_TYPES,
+              jss::TYPES,
+              jss::hash})
+        {
+            BEAST_EXPECT(defs.isMember(field));
+        }
+
+        // verify it returns the same hash as the RPC handler
+        using namespace test::jtx;
+        Env env(*this);
+        auto const rpcResult = env.rpc("server_definitions");
+        BEAST_EXPECT(defs[jss::hash] == rpcResult[jss::result][jss::hash]);
+    }
+
     void
     run() override
     {
         testServerDefinitions();
+        testGetServerDefinitionsJson();
     }
 };
 

src/xrpld/app/main/Main.cpp

@@ -3,6 +3,7 @@
 #include <xrpld/core/ConfigSections.h>
 #include <xrpld/core/TimeKeeper.h>
 #include <xrpld/rpc/RPCCall.h>
+#include <xrpld/rpc/handlers/server_info/ServerDefinitions.h>
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/basics/SlabAllocator.h>
@@ -13,6 +14,7 @@
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/core/StartUpType.h>
 #include <xrpl/git/Git.h>
+#include <xrpl/json/json_writer.h>
 #include <xrpl/protocol/BuildInfo.h>
 #include <xrpl/protocol/SystemParameters.h>
 #include <xrpl/server/Vacuum.h>
@@ -376,12 +378,12 @@ run(int argc, char** argv)
         "nodeid", po::value<std::string>(), "Specify the node identity for this server.")(
         "quorum", po::value<std::size_t>(), "Override the minimum validation quorum.")(
         "silent", "No output to the console after startup.")("standalone,a", "Run with no peers.")(
-        "verbose,v", "Verbose logging.")
-
-        ("force_ledger_present_range",
-         po::value<std::string>(),
-         "Specify the range of present ledgers for testing purposes. Min and "
-         "max values are comma separated.")("version", "Display the build version.");
+        "verbose,v", "Verbose logging.")(
+        "definitions", "Output server definitions as JSON and exit.")(
+        "force_ledger_present_range",
+        po::value<std::string>(),
+        "Specify the range of present ledgers for testing purposes. Min and "
+        "max values are comma separated.")("version", "Display the build version.");
 
     po::options_description data("Ledger/Data Options");
     data.add_options()("import", importText.c_str())(
@@ -503,10 +505,20 @@ run(int argc, char** argv)
 
     if (vm.contains("version"))
     {
+        // LCOV_EXCL_START
         std::cout << "xrpld version " << BuildInfo::getVersionString() << std::endl;
         std::cout << "Git commit hash: " << xrpl::git::getCommitHash() << std::endl;
         std::cout << "Git build branch: " << xrpl::git::getBuildBranch() << std::endl;
         return 0;
+        // LCOV_EXCL_STOP
+    }
+
+    if (vm.contains("definitions"))
+    {
+        // LCOV_EXCL_START
+        std::cout << Json::FastWriter().write(getServerDefinitionsJson());
+        return 0;
+        // LCOV_EXCL_STOP
     }
 
 #ifndef ENABLE_TESTS

src/xrpld/app/misc/TxQ.h

@@ -415,7 +415,7 @@ private:
             // Number of transactions expected per ledger.
             // One more than this value will be accepted
             // before escalation kicks in.
-            std::size_t const txnsExpected;
+            std::size_t const txnsExpected{};
             // Based on the median fee of the LCL. Used
             // when fee escalation kicks in.
             FeeLevel64 const escalationMultiplier;

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

@@ -1,6 +1,7 @@
 #include <xrpld/app/misc/ValidatorList.h>
 
 #include <xrpld/core/TimeKeeper.h>
+#include <xrpld/overlay/Message.h>
 #include <xrpld/overlay/Overlay.h>
 #include <xrpld/overlay/Peer.h>
 

src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp

@@ -1,3 +1,5 @@
+#include <xrpld/rpc/handlers/server_info/ServerDefinitions.h>
+
 #include <xrpld/rpc/Context.h>
 
 #include <xrpl/basics/base_uint.h>
@@ -369,8 +371,21 @@ ServerDefinitions::ServerDefinitions() : defs_{Json::objectValue}
     }
 }
 
+ServerDefinitions const&
+getDefinitions()
+{
+    static ServerDefinitions const defs{};
+    return defs;
+}
+
 }  // namespace detail
 
+Json::Value const&
+getServerDefinitionsJson()
+{
+    return detail::getDefinitions().get();
+}
+
 Json::Value
 doServerDefinitions(RPC::JsonContext& context)
 {
@@ -383,7 +398,7 @@ doServerDefinitions(RPC::JsonContext& context)
             return RPC::invalid_field_error(jss::hash);
     }
 
-    static detail::ServerDefinitions const defs{};
+    auto const& defs = detail::getDefinitions();
     if (defs.hashMatches(hash))
     {
         Json::Value jv = Json::objectValue;

src/xrpld/rpc/handlers/server_info/ServerDefinitions.h

@@ -0,0 +1,10 @@
+#pragma once
+
+#include <xrpl/json/json_value.h>
+
+namespace xrpl {
+
+Json::Value const&
+getServerDefinitionsJson();
+
+}  // namespace xrpl