Merge remote-tracking branch 'origin/develop' into tapanito/lending-fix-amendment

Author: Tapanito

.clang-tidy

@@ -1,4 +1,6 @@
 ---
+# This entire group of checks was applied to all cpp files but not all header files.
+# ---
 Checks: "-*,
   bugprone-argument-comment,
   bugprone-assert-side-effect,
@@ -8,26 +10,26 @@ Checks: "-*,
   bugprone-chained-comparison,
   bugprone-compare-pointer-to-member-virtual-function,
   bugprone-copy-constructor-init,
-  bugprone-crtp-constructor-accessibility,
+  # bugprone-crtp-constructor-accessibility, # has issues
   bugprone-dangling-handle,
   bugprone-dynamic-static-initializers,
-  bugprone-empty-catch,
+  # bugprone-empty-catch, # has issues
   bugprone-fold-init-type,
-  bugprone-forward-declaration-namespace,
-  bugprone-inaccurate-erase,
-  bugprone-inc-dec-in-conditions,
-  bugprone-incorrect-enable-if,
-  bugprone-incorrect-roundings,
-  bugprone-infinite-loop,
-  bugprone-integer-division,
+  # bugprone-forward-declaration-namespace, # has issues
+  # bugprone-inaccurate-erase,
+  # bugprone-inc-dec-in-conditions,
+  # bugprone-incorrect-enable-if,
+  # bugprone-incorrect-roundings,
+  # bugprone-infinite-loop,
+  # bugprone-integer-division,
   bugprone-lambda-function-name,
-  bugprone-macro-parentheses,
+  # bugprone-macro-parentheses, # has issues
   bugprone-macro-repeated-side-effects,
   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-multi-level-implicit-pointer-conversion, # has issues
   bugprone-multiple-new-in-one-expression,
   bugprone-multiple-statement-macro,
   bugprone-no-escape,
@@ -37,13 +39,13 @@ Checks: "-*,
   bugprone-pointer-arithmetic-on-polymorphic-object,
   bugprone-posix-return,
   bugprone-redundant-branch-condition,
-  bugprone-reserved-identifier,
-  bugprone-return-const-ref-from-parameter,
+  # bugprone-reserved-identifier, # has issues
+  # bugprone-return-const-ref-from-parameter, # has issues
   bugprone-shared-ptr-array-mismatch,
   bugprone-signal-handler,
   bugprone-signed-char-misuse,
   bugprone-sizeof-container,
-  bugprone-sizeof-expression,
+  # bugprone-sizeof-expression, # has issues
   bugprone-spuriously-wake-up-functions,
   bugprone-standalone-empty,
   bugprone-string-constructor,
@@ -60,7 +62,7 @@ Checks: "-*,
   bugprone-suspicious-string-compare,
   bugprone-suspicious-stringview-data-usage,
   bugprone-swapped-arguments,
-  bugprone-switch-missing-default-case,
+  # bugprone-switch-missing-default-case, # has issues
   bugprone-terminating-continue,
   bugprone-throw-keyword-missing,
   bugprone-too-small-loop-variable,
@@ -71,61 +73,66 @@ Checks: "-*,
   bugprone-unhandled-self-assignment,
   bugprone-unique-ptr-array-mismatch,
   bugprone-unsafe-functions,
-  bugprone-use-after-move,
+  # bugprone-use-after-move, # has issues
   bugprone-unused-raii,
   bugprone-unused-return-value,
   bugprone-unused-local-non-trivial-variable,
   bugprone-virtual-near-miss,
-  cppcoreguidelines-init-variables,
-  cppcoreguidelines-misleading-capture-default-by-value,
+  # cppcoreguidelines-init-variables, # has issues
+  # cppcoreguidelines-misleading-capture-default-by-value, # has issues
   cppcoreguidelines-no-suspend-with-lock,
-  cppcoreguidelines-pro-type-member-init,
+  # cppcoreguidelines-pro-type-member-init, # has issues
   cppcoreguidelines-pro-type-static-cast-downcast,
-  cppcoreguidelines-rvalue-reference-param-not-moved,
-  cppcoreguidelines-use-default-member-init,
-  cppcoreguidelines-virtual-class-destructor,
+  # cppcoreguidelines-rvalue-reference-param-not-moved, # has issues
+  # cppcoreguidelines-use-default-member-init, # has issues
+  # cppcoreguidelines-virtual-class-destructor, # has issues
   hicpp-ignored-remove-result,
-  misc-definitions-in-headers,
+  # misc-definitions-in-headers, # has issues
   misc-header-include-cycle,
   misc-misplaced-const,
   misc-static-assert,
-  misc-throw-by-value-catch-by-reference,
+  # misc-throw-by-value-catch-by-reference, # has issues
   misc-unused-alias-decls,
   misc-unused-using-decls,
   modernize-deprecated-headers,
   modernize-make-shared,
   modernize-make-unique,
+  performance-faster-string-find,
+  performance-for-range-copy,
   performance-implicit-conversion-in-loop,
+  performance-inefficient-vector-operation,
+  performance-move-const-arg,
   performance-move-constructor-init,
+  performance-no-automatic-move,
   performance-trivially-destructible,
-  readability-avoid-nested-conditional-operator,
-  readability-avoid-return-with-void-value,
-  readability-braces-around-statements,
-  readability-const-return-type,
-  readability-container-contains,
-  readability-container-size-empty,
-  readability-convert-member-functions-to-static,
+  # readability-avoid-nested-conditional-operator, # has issues
+  # readability-avoid-return-with-void-value, # has issues
+  # readability-braces-around-statements, # has issues
+  # readability-const-return-type, # has issues
+  # readability-container-contains, # has issues
+  # readability-container-size-empty, # has issues
+  # readability-convert-member-functions-to-static, # has issues
   readability-duplicate-include,
-  readability-else-after-return,
-  readability-enum-initial-value,
-  readability-implicit-bool-conversion,
-  readability-make-member-function-const,
-  readability-math-missing-parentheses,
+  # readability-else-after-return, # has issues
+  # readability-enum-initial-value, # has issues
+  # readability-implicit-bool-conversion, # has issues
+  # readability-make-member-function-const, # has issues
+  # readability-math-missing-parentheses, # has issues
   readability-misleading-indentation,
   readability-non-const-parameter,
-  readability-redundant-casting,
-  readability-redundant-declaration,
-  readability-redundant-inline-specifier,
-  readability-redundant-member-init,
+  # readability-redundant-casting, # has issues
+  # readability-redundant-declaration, # has issues
+  # readability-redundant-inline-specifier, # has issues
+  # readability-redundant-member-init, # has issues
   readability-redundant-string-init,
   readability-reference-to-constructed-temporary,
-  readability-simplify-boolean-expr,
-  readability-static-definition-in-anonymous-namespace,
-  readability-suspicious-call-argument,
+  # readability-simplify-boolean-expr, # has issues
+  # readability-static-definition-in-anonymous-namespace, # has issues
+  # readability-suspicious-call-argument, # has issues
   readability-use-std-min-max
   "
 # ---
-# checks that have some issues that need to be resolved:
+# other checks that have issues that need to be resolved:
 #
 # llvm-namespace-comment,
 # misc-const-correctness,
@@ -134,7 +141,7 @@ Checks: "-*,
 #
 # readability-inconsistent-declaration-parameter-name, # in this codebase this check will break a lot of arg names
 # readability-static-accessed-through-instance,        # this check is probably unnecessary. it makes the code less readable
-# readability-identifier-naming,
+# readability-identifier-naming,                       # https://github.com/XRPLF/rippled/pull/6571
 #
 # modernize-concat-nested-namespaces,
 # modernize-pass-by-value,
@@ -148,12 +155,6 @@ Checks: "-*,
 # modernize-use-starts-ends-with,
 # modernize-use-std-numbers,
 # modernize-use-using,
-#
-# performance-faster-string-find,
-# performance-for-range-copy,
-# performance-inefficient-vector-operation,
-# performance-move-const-arg,
-# performance-no-automatic-move,
 # ---
 #
 CheckOptions:
@@ -195,5 +196,6 @@ CheckOptions:
   bugprone-unused-return-value.CheckedReturnTypes: ::std::error_code;::std::error_condition;::std::errc
 #   misc-include-cleaner.IgnoreHeaders: '.*/(detail|impl)/.*;.*(expected|unexpected).*;.*ranges_lower_bound\.h;time.h;stdlib.h;__chrono/.*;fmt/chrono.h;boost/uuid/uuid_hash.hpp'
 #
-# HeaderFilterRegex: '^.*/(src|tests)/.*\.(h|hpp)$'
+HeaderFilterRegex: '^.*/(test|xrpl|xrpld)/.*\.(h|hpp)$'
+ExcludeHeaderFilterRegex: '^.*/protocol_autogen/.*\.(h|hpp)$'
 WarningsAsErrors: "*"

.github/workflows/publish-docs.yml

@@ -81,13 +81,13 @@ jobs:
           cmake --build . --target docs --parallel ${BUILD_NPROC}
 
       - name: Create documentation artifact
-        if: ${{ github.event_name == 'push' }}
+        if: ${{ github.event.repository.visibility == 'public' && github.event_name == 'push' }}
         uses: actions/upload-pages-artifact@7b1f4a764d45c48632c6b24a0339c27f5614fb0b # v4.0.0
         with:
           path: ${{ env.BUILD_DIR }}/docs/html
 
   deploy:
-    if: ${{ github.event_name == 'push' }}
+    if: ${{ github.event.repository.visibility == 'public' && github.event_name == 'push' }}
     needs: build
     runs-on: ubuntu-latest
     permissions:
@@ -99,4 +99,4 @@ jobs:
     steps:
       - name: Deploy to GitHub Pages
         id: deploy
-        uses: actions/deploy-pages@d6db90164ac5ed86f2b6aed7e0febac5b3c0c03e # v4.0.5
+        uses: actions/deploy-pages@cd2ce8fcbc39b97be8ca5fce6e763baed58fa128 # v5.0.0

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

@@ -298,7 +298,7 @@ jobs:
 
       - name: Upload coverage report
         if: ${{ github.repository == 'XRPLF/rippled' && !inputs.build_only && env.COVERAGE_ENABLED == 'true' }}
-        uses: codecov/codecov-action@1af58845a975a7985b0beb0cbe6fbbb71a41dbad # v5.5.3
+        uses: codecov/codecov-action@57e3a136b779b570ffcdbf80b3bdc90e7fab3de2 # v6.0.0
         with:
           disable_search: true
           disable_telem: true

include/xrpl/core/Coro.ipp

@@ -70,14 +70,24 @@ JobQueue::Coro::resume()
         running_ = true;
     }
     {
-        std::lock_guard lock(jq_.m_mutex);
+        std::lock_guard lk(jq_.m_mutex);
         --jq_.nSuspend_;
     }
     auto saved = detail::getLocalValues().release();
     detail::getLocalValues().reset(&lvs_);
     std::lock_guard lock(mutex_);
-    XRPL_ASSERT(static_cast<bool>(coro_), "xrpl::JobQueue::Coro::resume : is runnable");
-    coro_();
+    // A late resume() can arrive after the coroutine has already completed.
+    // This is an expected (if rare) outcome of the race condition documented
+    // in JobQueue.h:354-377 where post() schedules a resume job before the
+    // coroutine yields — the mutex serializes access, but by the time this
+    // resume() acquires the lock the coroutine may have already run to
+    // completion. Calling operator() on a completed boost::coroutine2 is
+    // undefined behavior, so we must check and skip invoking the coroutine
+    // body if it has already completed.
+    if (coro_)
+    {
+        coro_();
+    }
     detail::getLocalValues().release();
     detail::getLocalValues().reset(saved);
     std::lock_guard lk(mutex_run_);

include/xrpl/core/JobQueue.h

@@ -99,8 +99,8 @@ class JobQueue : private Workers::Callback
             Effects:
                The coroutine continues execution from where it last left off
                  using this same thread.
-            Undefined behavior if called after the coroutine has completed
-              with a return (as opposed to a yield()).
+               If the coroutine has already completed, returns immediately
+                 (handles the documented post-before-yield race condition).
             Undefined behavior if resume() or post() called consecutively
               without a corresponding yield.
         */
@@ -357,8 +357,10 @@ class JobQueue : private Workers::Callback
     If the post() job were to be executed before yield(), undefined behavior
     would occur. The lock ensures that coro_ is not called again until we exit
     the coroutine. At which point a scheduled resume() job waiting on the lock
-    would gain entry, harmlessly call coro_ and immediately return as we have
-    already completed the coroutine.
+    would gain entry. resume() checks if the coroutine has already completed
+    (coro_ converts to false) and, if so, skips invoking operator() since
+    calling operator() on a completed boost::coroutine2 pull_type is undefined
+    behavior.
 
     The race condition occurs as follows:
 

include/xrpl/core/StartUpType.h

@@ -5,7 +5,7 @@
 
 namespace xrpl {
 
-enum class StartUpType { FRESH, NORMAL, LOAD, LOAD_FILE, REPLAY, NETWORK };
+enum class StartUpType { Fresh, Normal, Load, LoadFile, Replay, Network };
 
 inline std::ostream&
 operator<<(std::ostream& os, StartUpType const& type)

include/xrpl/ledger/OrderBookDB.h

@@ -76,16 +76,33 @@ class OrderBookDB
         @return true if a book from this issue to XRP exists
     */
     virtual bool
-    isBookToXRP(Issue const& issue, std::optional<Domain> domain = std::nullopt) = 0;
-
+    isBookToXRP(Issue const& issue, std::optional<Domain> const& domain = std::nullopt) = 0;
+
+    /**
+     * Process a transaction for order book tracking.
+     * @param ledger The ledger the transaction was applied to
+     * @param alTx The transaction to process
+     * @param jvObj The JSON object of the transaction
+     */
     virtual void
     processTxn(
         std::shared_ptr<ReadView const> const& ledger,
         AcceptedLedgerTx const& alTx,
         MultiApiJson const& jvObj) = 0;
 
+    /**
+     * Get the book listeners for a book.
+     * @param book The book to get the listeners for
+     * @return The book listeners for the book
+     */
     virtual BookListeners::pointer
     getBookListeners(Book const&) = 0;
+
+    /**
+     * Create a new book listeners for a book.
+     * @param book The book to create the listeners for
+     * @return The new book listeners for the book
+     */
     virtual BookListeners::pointer
     makeBookListeners(Book const&) = 0;
 };

include/xrpl/protocol/STVector256.h

@@ -69,9 +69,6 @@ class STVector256 : public STBase, public CountedObject<STVector256>
     std::vector<uint256>::iterator
     insert(std::vector<uint256>::const_iterator pos, uint256 const& value);
 
-    std::vector<uint256>::iterator
-    insert(std::vector<uint256>::const_iterator pos, uint256&& value);
-
     void
     push_back(uint256 const& v);
 
@@ -184,12 +181,6 @@ STVector256::insert(std::vector<uint256>::const_iterator pos, uint256 const& val
     return mValue.insert(pos, value);
 }
 
-inline std::vector<uint256>::iterator
-STVector256::insert(std::vector<uint256>::const_iterator pos, uint256&& value)
-{
-    return mValue.insert(pos, std::move(value));
-}
-
 inline void
 STVector256::push_back(uint256 const& v)
 {

include/xrpl/protocol/TxSearched.h

@@ -2,6 +2,6 @@
 
 namespace xrpl {
 
-enum class TxSearched { all, some, unknown };
+enum class TxSearched { All, Some, Unknown };
 
 }

include/xrpl/rdb/DatabaseCon.h

@@ -70,7 +70,7 @@ class DatabaseCon
     {
         explicit Setup() = default;
 
-        StartUpType startUp = StartUpType::NORMAL;
+        StartUpType startUp = StartUpType::Normal;
         bool standAlone = false;
         boost::filesystem::path dataDir;
         // Indicates whether or not to return the `globalPragma`
@@ -107,9 +107,8 @@ class DatabaseCon
         beast::Journal journal)
         // Use temporary files or regular DB files?
         : DatabaseCon(
-              setup.standAlone && setup.startUp != StartUpType::LOAD &&
-                      setup.startUp != StartUpType::LOAD_FILE &&
-                      setup.startUp != StartUpType::REPLAY
+              setup.standAlone && setup.startUp != StartUpType::Load &&
+                      setup.startUp != StartUpType::LoadFile && setup.startUp != StartUpType::Replay
                   ? ""
                   : (setup.dataDir / dbName),
               setup.commonPragma(),