Merge remote-tracking branch 'XRPLF/develop' into ximinez/emptydirectoryinvariant

* XRPLF/develop:
  chore: Update pre-commit hooks (6460)
  ci: [DEPENDABOT] bump actions/upload-artifact from 6.0.0 to 7.0.0 (6450)
  tests: Improve stability of Subscribe tests (6420)
  refactor: Fix clang-tidy `bugprone-empty-catch` check (6419)
  refactor: Splits invariant checks into multiple classes (6440)
  chore: Make nix hook optional (6431)
  test: Grep for failures in CI (6339)
  chore: Enable clang-tidy checks without issues (6414)
  refactor: Use uint256 directly as key instead of void pointer (6313)
  chore: Update cleanup-workspace to delete old .conan2 dir on macOS (6412)

.clang-tidy

@@ -24,7 +24,6 @@ Checks: "-*,
   bugprone-misplaced-operator-in-strlen-in-alloc,
   bugprone-misplaced-pointer-arithmetic-in-alloc,
   bugprone-misplaced-widening-cast,
-  bugprone-move-forwarding-reference,
   bugprone-multi-level-implicit-pointer-conversion,
   bugprone-multiple-new-in-one-expression,
   bugprone-multiple-statement-macro,
@@ -33,12 +32,10 @@ Checks: "-*,
   bugprone-parent-virtual-call,
   bugprone-posix-return,
   bugprone-redundant-branch-condition,
-  bugprone-return-const-ref-from-parameter,
   bugprone-shared-ptr-array-mismatch,
   bugprone-signal-handler,
   bugprone-signed-char-misuse,
   bugprone-sizeof-container,
-  bugprone-sizeof-expression,
   bugprone-spuriously-wake-up-functions,
   bugprone-standalone-empty,
   bugprone-string-constructor,
@@ -85,14 +82,16 @@ Checks: "-*,
   performance-trivially-destructible
   "
 # ---
-# more checks that have some issues that need to be resolved:
+# checks that have some issues that need to be resolved:
 #
 # bugprone-crtp-constructor-accessibility,
 # bugprone-inc-dec-in-conditions,
 # bugprone-reserved-identifier,
 # bugprone-move-forwarding-reference,
 # bugprone-unused-local-non-trivial-variable,
+# bugprone-return-const-ref-from-parameter,
 # bugprone-switch-missing-default-case,
+# bugprone-sizeof-expression,
 # bugprone-suspicious-stringview-data-usage,
 # bugprone-suspicious-missing-comma,
 # bugprone-pointer-arithmetic-on-polymorphic-object,

.github/workflows/publish-docs.yml

@@ -49,11 +49,6 @@ jobs:
       - name: Checkout repository
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
 
-      - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@2cbf481018d930656e9276fcc20dc0e3a0be5b6d
-        with:
-          enable_ccache: false
-
       - name: Get number of processors
         uses: XRPLF/actions/get-nproc@cf0433aa74563aead044a1e395610c96d65a37cf
         id: nproc

CMakeLists.txt

@@ -36,6 +36,26 @@ endif ()
 # Enable ccache to speed up builds.
 include(Ccache)
 
+# make GIT_COMMIT_HASH define available to all sources
+find_package(Git)
+if (Git_FOUND)
+    execute_process(COMMAND ${GIT_EXECUTABLE} --git-dir=${CMAKE_CURRENT_SOURCE_DIR}/.git rev-parse
+                            HEAD OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_VARIABLE gch)
+    if (gch)
+        set(GIT_COMMIT_HASH "${gch}")
+        message(STATUS gch: ${GIT_COMMIT_HASH})
+        add_definitions(-DGIT_COMMIT_HASH="${GIT_COMMIT_HASH}")
+    endif ()
+
+    execute_process(COMMAND ${GIT_EXECUTABLE} --git-dir=${CMAKE_CURRENT_SOURCE_DIR}/.git rev-parse
+                            --abbrev-ref HEAD OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_VARIABLE gb)
+    if (gb)
+        set(GIT_BRANCH "${gb}")
+        message(STATUS gb: ${GIT_BRANCH})
+        add_definitions(-DGIT_BRANCH="${GIT_BRANCH}")
+    endif ()
+endif () # git
+
 if (thread_safety_analysis)
     add_compile_options(-Wthread-safety -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
                         -DXRPL_ENABLE_THREAD_SAFETY_ANNOTATIONS)

cmake/GitInfo.cmake

@@ -1,21 +0,0 @@
-include_guard()
-
-set(GIT_BUILD_BRANCH "")
-set(GIT_COMMIT_HASH "")
-
-find_package(Git)
-if (NOT Git_FOUND)
-    message(WARNING "Git not found. Git branch and commit hash will be empty.")
-    return()
-endif ()
-
-set(GIT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/.git)
-
-execute_process(COMMAND ${GIT_EXECUTABLE} --git-dir=${GIT_DIRECTORY} rev-parse --abbrev-ref HEAD
-                OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_VARIABLE GIT_BUILD_BRANCH)
-
-execute_process(COMMAND ${GIT_EXECUTABLE} --git-dir=${GIT_DIRECTORY} rev-parse HEAD
-                OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_VARIABLE GIT_COMMIT_HASH)
-
-message(STATUS "Git branch: ${GIT_BUILD_BRANCH}")
-message(STATUS "Git commit hash: ${GIT_COMMIT_HASH}")

cmake/XrplCore.cmake

@@ -58,12 +58,6 @@ include(target_link_modules)
 add_module(xrpl beast)
 target_link_libraries(xrpl.libxrpl.beast PUBLIC xrpl.imports.main)
 
-include(GitInfo)
-add_module(xrpl git)
-target_compile_definitions(xrpl.libxrpl.git PRIVATE GIT_COMMIT_HASH="${GIT_COMMIT_HASH}"
-                                                    GIT_BUILD_BRANCH="${GIT_BUILD_BRANCH}")
-target_link_libraries(xrpl.libxrpl.git PUBLIC xrpl.imports.main)
-
 # Level 02
 add_module(xrpl basics)
 target_link_libraries(xrpl.libxrpl.basics PUBLIC xrpl.libxrpl.beast)
@@ -77,8 +71,7 @@ target_link_libraries(xrpl.libxrpl.crypto PUBLIC xrpl.libxrpl.basics)
 
 # Level 04
 add_module(xrpl protocol)
-target_link_libraries(xrpl.libxrpl.protocol PUBLIC xrpl.libxrpl.crypto xrpl.libxrpl.git
-                                                   xrpl.libxrpl.json)
+target_link_libraries(xrpl.libxrpl.protocol PUBLIC xrpl.libxrpl.crypto xrpl.libxrpl.json)
 
 # Level 05
 add_module(xrpl core)
@@ -142,7 +135,6 @@ target_link_modules(
     conditions
     core
     crypto
-    git
     json
     ledger
     net

cmake/XrplInstall.cmake

@@ -23,7 +23,6 @@ install(TARGETS common
                 xrpl.libxrpl.conditions
                 xrpl.libxrpl.core
                 xrpl.libxrpl.crypto
-                xrpl.libxrpl.git
                 xrpl.libxrpl.json
                 xrpl.libxrpl.rdb
                 xrpl.libxrpl.ledger

include/xrpl/beast/core/SemanticVersion.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include <string>
-#include <string_view>
 #include <vector>
 
 namespace beast {
@@ -27,14 +26,14 @@ public:
 
     SemanticVersion();
 
-    SemanticVersion(std::string_view version);
+    SemanticVersion(std::string const& version);
 
     /** Parse a semantic version string.
         The parsing is as strict as possible.
         @return `true` if the string was parsed.
     */
     bool
-    parse(std::string_view input);
+    parse(std::string const& input);
 
     /** Produce a string from semantic version components. */
     std::string

include/xrpl/git/Git.h

@@ -1,13 +0,0 @@
-#pragma once
-
-#include <string>
-
-namespace xrpl::git {
-
-std::string const&
-getCommitHash();
-
-std::string const&
-getBuildBranch();
-
-}  // namespace xrpl::git

include/xrpl/protocol/BuildInfo.h

@@ -49,7 +49,7 @@ getFullVersionString();
     @return the encoded version in a 64-bit integer
 */
 std::uint64_t
-encodeSoftwareVersion(std::string_view versionStr);
+encodeSoftwareVersion(char const* const versionStr);
 
 /** Returns this server's version packed in a 64-bit integer. */
 std::uint64_t

include/xrpl/tx/invariants/InvariantCheck.h

@@ -340,6 +340,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<
@@ -366,7 +382,8 @@ using InvariantChecks = std::tuple<
     ValidPseudoAccounts,
     ValidLoanBroker,
     ValidLoan,
-    ValidVault>;
+    ValidVault,
+    NoEmptyDirectory>;
 
 /**
  * @brief get a tuple of all invariant checks

src/libxrpl/beast/core/SemanticVersion.cpp

@@ -138,14 +138,14 @@ SemanticVersion::SemanticVersion() : majorVersion(0), minorVersion(0), patchVers
 {
 }
 
-SemanticVersion::SemanticVersion(std::string_view version) : SemanticVersion()
+SemanticVersion::SemanticVersion(std::string const& version) : SemanticVersion()
 {
     if (!parse(version))
         throw std::invalid_argument("invalid version string");
 }
 
 bool
-SemanticVersion::parse(std::string_view input)
+SemanticVersion::parse(std::string const& input)
 {
     // May not have leading or trailing whitespace
     auto left_iter = std::find_if_not(input.begin(), input.end(), [](std::string::value_type c) {

src/libxrpl/git/Git.cpp

@@ -1,31 +0,0 @@
-#include "xrpl/git/Git.h"
-
-#include <string>
-
-#ifndef GIT_COMMIT_HASH
-#error "GIT_COMMIT_HASH must be defined"
-#endif
-#ifndef GIT_BUILD_BRANCH
-#error "GIT_BUILD_BRANCH must be defined"
-#endif
-
-namespace xrpl::git {
-
-static constexpr char kGIT_COMMIT_HASH[] = GIT_COMMIT_HASH;
-static constexpr char kGIT_BUILD_BRANCH[] = GIT_BUILD_BRANCH;
-
-std::string const&
-getCommitHash()
-{
-    static std::string const kVALUE = kGIT_COMMIT_HASH;
-    return kVALUE;
-}
-
-std::string const&
-getBuildBranch()
-{
-    static std::string const kVALUE = kGIT_BUILD_BRANCH;
-    return kVALUE;
-}
-
-}  // namespace xrpl::git

src/libxrpl/ledger/ApplyView.cpp

@@ -244,6 +244,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/protocol/BuildInfo.cpp

@@ -1,7 +1,6 @@
 #include <xrpl/basics/contract.h>
 #include <xrpl/beast/core/LexicalCast.h>
 #include <xrpl/beast/core/SemanticVersion.h>
-#include <xrpl/git/Git.h>
 #include <xrpl/protocol/BuildInfo.h>
 
 #include <boost/preprocessor/stringize.hpp>
@@ -15,60 +14,44 @@ namespace xrpl {
 
 namespace BuildInfo {
 
-namespace {
-
 //--------------------------------------------------------------------------
 //  The build version number. You must edit this for each release
 //  and follow the format described at http://semver.org/
 //------------------------------------------------------------------------------
 // clang-format off
 char const* const versionString = "3.2.0-b0"
-    // clang-format on
+// clang-format on
+
+#if defined(DEBUG) || defined(SANITIZERS)
+    "+"
+#ifdef GIT_COMMIT_HASH
+    GIT_COMMIT_HASH
+    "."
+#endif
+#ifdef DEBUG
+    "DEBUG"
+#ifdef SANITIZERS
+    "."
+#endif
+#endif
+
+#ifdef SANITIZERS
+    BOOST_PP_STRINGIZE(SANITIZERS)  // cspell: disable-line
+#endif
+#endif
+
+    //--------------------------------------------------------------------------
     ;
 
 //
 // Don't touch anything below this line
 //
 
-std::string
-buildVersionString()
-{
-    std::string version = versionString;
-
-#if defined(DEBUG) || defined(SANITIZERS)
-    std::string metadata;
-
-    std::string const& commitHash = xrpl::git::getCommitHash();
-    if (!commitHash.empty())
-        metadata += commitHash + ".";
-
-#ifdef DEBUG
-    metadata += "DEBUG";
-#endif
-
-#if defined(DEBUG) && defined(SANITIZERS)
-    metadata += ".";
-#endif
-
-#ifdef SANITIZERS
-    metadata += BOOST_PP_STRINGIZE(SANITIZERS);  // cspell: disable-line
-#endif
-
-    if (!metadata.empty())
-        version += "+" + metadata;
-#endif
-
-    return version;
-}
-
-}  // namespace
-
 std::string const&
 getVersionString()
 {
     static std::string const value = [] {
-        std::string const s = buildVersionString();
-
+        std::string const s = versionString;
         beast::SemanticVersion v;
         if (!v.parse(s) || v.print() != s)
             LogicError(s + ": Bad server version string");
@@ -88,13 +71,13 @@ static constexpr std::uint64_t implementationVersionIdentifier = 0x183B'0000'000
 static constexpr std::uint64_t implementationVersionIdentifierMask = 0xFFFF'0000'0000'0000LLU;
 
 std::uint64_t
-encodeSoftwareVersion(std::string_view versionStr)
+encodeSoftwareVersion(char const* const versionStr)
 {
     std::uint64_t c = implementationVersionIdentifier;
 
     beast::SemanticVersion v;
 
-    if (v.parse(versionStr))
+    if (v.parse(std::string(versionStr)))
     {
         if (v.majorVersion >= 0 && v.majorVersion <= 255)
             c |= static_cast<std::uint64_t>(v.majorVersion) << 40;
@@ -154,7 +137,7 @@ encodeSoftwareVersion(std::string_view versionStr)
 std::uint64_t
 getEncodedVersion()
 {
-    static std::uint64_t const cookie = {encodeSoftwareVersion(getVersionString())};
+    static std::uint64_t const cookie = {encodeSoftwareVersion(versionString)};
     return cookie;
 }
 

src/libxrpl/tx/invariants/InvariantCheck.cpp

@@ -1006,4 +1006,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/PermissionedDomain/PermissionedDomainDelete.cpp

@@ -18,7 +18,7 @@ TER
 PermissionedDomainDelete::preclaim(PreclaimContext const& ctx)
 {
     auto const domain = ctx.tx.getFieldH256(sfDomainID);
-    auto const sleDomain = ctx.view.read(keylet::permissionedDomain(domain));
+    auto const sleDomain = ctx.view.read({ltPERMISSIONED_DOMAIN, domain});
 
     if (!sleDomain)
         return tecNO_ENTRY;
@@ -40,7 +40,7 @@ PermissionedDomainDelete::doApply()
         ctx_.tx.isFieldPresent(sfDomainID),
         "xrpl::PermissionedDomainDelete::doApply : required field present");
 
-    auto const slePd = view().peek(keylet::permissionedDomain(ctx_.tx.at(sfDomainID)));
+    auto const slePd = view().peek({ltPERMISSIONED_DOMAIN, ctx_.tx.at(sfDomainID)});
     auto const page = (*slePd)[sfOwnerNode];
 
     if (!view().dirRemove(keylet::ownerDir(account_), page, slePd->key(), true))

src/test/beast/aged_associative_container_test.cpp

@@ -227,7 +227,7 @@ public:
         static typename Base::Key const&
         extract(Value const& value)
         {
-            return value;  // NOLINT(bugprone-return-const-ref-from-parameter)
+            return value;
         }
 
         static Values

src/test/protocol/MultiApiJson_test.cpp

@@ -556,7 +556,7 @@ struct MultiApiJson_test : beast::unit_test::suite
             static_assert([](auto&& v) {
                 return !requires {
                     v.visitor(
-                        decltype(v){},  // cannot bind rvalue
+                        std::move(v),  // cannot bind rvalue
                         1,
                         [](Json::Value&, auto) {});
                 };

src/test/rpc/GatewayBalances_test.cpp

@@ -223,45 +223,6 @@ public:
         expect(jv[jss::result][jss::obligations]["USD"] == maxUSD.getText());
     }
 
-    void
-    testGWBWithMPT()
-    {
-        testcase("Gateway Balances with MPT Escrow");
-        using namespace std::chrono_literals;
-        using namespace jtx;
-
-        // Ensure MPT is enabled
-        FeatureBitset features = testable_amendments() | featureMPTokensV1;
-        Env env(*this, features);
-
-        Account const alice{"alice"};
-        Account const bob{"bob"};
-
-        env.fund(XRP(10000), alice, bob);
-        env.close();
-
-        // Create MPT issuance (Alice) with Escrow capability
-        MPTTester mpt(env, alice, {.holders = {bob}, .fund = false});
-        mpt.create({.flags = tfMPTCanEscrow});
-
-        // Authorize Bob and fund him
-        mpt.authorize({.account = bob, .holderCount = 1});
-        mpt.pay(alice, bob, 1000);
-
-        // Bob creates an escrow of MPT to Alice.
-        auto const MPT = mpt["MPT"];
-        env(escrow::create(bob, alice, MPT(100)), escrow::finish_time(env.now() + 10s));
-        env.close();
-
-        // Query gateway_balances for Bob.
-        auto wsc = makeWSClient(env.app().config());
-        Json::Value qry;
-        qry[jss::account] = bob.human();
-
-        auto jv = wsc->invoke("gateway_balances", qry);
-        expect(jv[jss::status] == "success");
-    }
-
     void
     run() override
     {
@@ -272,7 +233,7 @@ public:
             testGWB(feature);
             testGWBApiVersions(feature);
         }
-        testGWBWithMPT();
+
         testGWBOverflow();
     }
 };

src/xrpld/app/main/Main.cpp

@@ -6,7 +6,6 @@
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/beast/core/CurrentThreadName.h>
-#include <xrpl/git/Git.h>
 #include <xrpl/protocol/BuildInfo.h>
 #include <xrpl/server/Vacuum.h>
 
@@ -477,8 +476,12 @@ run(int argc, char** argv)
     if (vm.count("version"))
     {
         std::cout << "rippled 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;
+#ifdef GIT_COMMIT_HASH
+        std::cout << "Git commit hash: " << GIT_COMMIT_HASH << std::endl;
+#endif
+#ifdef GIT_BRANCH
+        std::cout << "Git build branch: " << GIT_BRANCH << std::endl;
+#endif
         return 0;
     }
 

src/xrpld/app/misc/NetworkOPs.cpp

@@ -40,7 +40,6 @@
 #include <xrpl/core/PerfLog.h>
 #include <xrpl/crypto/RFC1751.h>
 #include <xrpl/crypto/csprng.h>
-#include <xrpl/git/Git.h>
 #include <xrpl/ledger/AmendmentTable.h>
 #include <xrpl/ledger/OrderBookDB.h>
 #include <xrpl/protocol/BuildInfo.h>
@@ -2594,14 +2593,17 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters)
             }
         }
 
-        if (!xrpl::git::getCommitHash().empty() || !xrpl::git::getBuildBranch().empty())
+#if defined(GIT_COMMIT_HASH) || defined(GIT_BRANCH)
         {
             auto& x = (info[jss::git] = Json::objectValue);
-            if (!xrpl::git::getCommitHash().empty())
-                x[jss::hash] = xrpl::git::getCommitHash();
-            if (!xrpl::git::getBuildBranch().empty())
-                x[jss::branch] = xrpl::git::getBuildBranch();
+#ifdef GIT_COMMIT_HASH
+            x[jss::hash] = GIT_COMMIT_HASH;
+#endif
+#ifdef GIT_BRANCH
+            x[jss::branch] = GIT_BRANCH;
+#endif
         }
+#endif
     }
     info[jss::io_latency_ms] = static_cast<Json::UInt>(registry_.app().getIOLatency().count());
 

src/xrpld/rpc/handlers/GatewayBalances.cpp

@@ -131,10 +131,6 @@ doGatewayBalances(RPC::JsonContext& context)
             if (sle->getType() == ltESCROW)
             {
                 auto const& escrow = sle->getFieldAmount(sfAmount);
-                // Gateway Balance should not include MPTs
-                if (escrow.holds<MPTIssue>())
-                    return;
-
                 auto& bal = locked[escrow.getCurrency()];
                 if (bal == beast::zero)
                 {