Merge branch 'develop' into pratik/Fix_asan_boost_coroutine_issues

.clang-tidy

@@ -1,105 +1,143 @@
 ---
 Checks: "-*,
-  bugprone-argument-comment
+  bugprone-argument-comment,
+  bugprone-assert-side-effect,
+  bugprone-bad-signal-to-kill-thread,
+  bugprone-bool-pointer-implicit-conversion,
+  bugprone-casting-through-void,
+  bugprone-chained-comparison,
+  bugprone-compare-pointer-to-member-virtual-function,
+  bugprone-copy-constructor-init,
+  bugprone-dangling-handle,
+  bugprone-dynamic-static-initializers,
+  bugprone-fold-init-type,
+  bugprone-forward-declaration-namespace,
+  bugprone-inaccurate-erase,
+  bugprone-incorrect-enable-if,
+  bugprone-incorrect-roundings,
+  bugprone-infinite-loop,
+  bugprone-integer-division,
+  bugprone-lambda-function-name,
+  bugprone-macro-parentheses,
+  bugprone-macro-repeated-side-effects,
+  bugprone-misplaced-operator-in-strlen-in-alloc,
+  bugprone-misplaced-pointer-arithmetic-in-alloc,
+  bugprone-misplaced-widening-cast,
+  bugprone-multi-level-implicit-pointer-conversion,
+  bugprone-multiple-new-in-one-expression,
+  bugprone-multiple-statement-macro,
+  bugprone-no-escape,
+  bugprone-non-zero-enum-to-bool-conversion,
+  bugprone-parent-virtual-call,
+  bugprone-posix-return,
+  bugprone-redundant-branch-condition,
+  bugprone-shared-ptr-array-mismatch,
+  bugprone-signal-handler,
+  bugprone-signed-char-misuse,
+  bugprone-sizeof-container,
+  bugprone-spuriously-wake-up-functions,
+  bugprone-standalone-empty,
+  bugprone-string-constructor,
+  bugprone-string-integer-assignment,
+  bugprone-string-literal-with-embedded-nul,
+  bugprone-stringview-nullptr,
+  bugprone-suspicious-enum-usage,
+  bugprone-suspicious-include,
+  bugprone-suspicious-memory-comparison,
+  bugprone-suspicious-memset-usage,
+  bugprone-suspicious-realloc-usage,
+  bugprone-suspicious-semicolon,
+  bugprone-suspicious-string-compare,
+  bugprone-swapped-arguments,
+  bugprone-terminating-continue,
+  bugprone-throw-keyword-missing,
+  bugprone-undefined-memory-manipulation,
+  bugprone-undelegated-constructor,
+  bugprone-unhandled-exception-at-new,
+  bugprone-unique-ptr-array-mismatch,
+  bugprone-unsafe-functions,
+  bugprone-virtual-near-miss,
+  cppcoreguidelines-no-suspend-with-lock,
+  cppcoreguidelines-virtual-class-destructor,
+  hicpp-ignored-remove-result,
+  misc-definitions-in-headers,
+  misc-header-include-cycle,
+  misc-misplaced-const,
+  misc-static-assert,
+  misc-throw-by-value-catch-by-reference,
+  misc-unused-alias-decls,
+  misc-unused-using-decls,
+  readability-duplicate-include,
+  readability-enum-initial-value,
+  readability-misleading-indentation,
+  readability-non-const-parameter,
+  readability-redundant-declaration,
+  readability-reference-to-constructed-temporary,
+  modernize-deprecated-headers,
+  modernize-make-shared,
+  modernize-make-unique,
+  performance-implicit-conversion-in-loop,
+  performance-move-constructor-init,
+  performance-trivially-destructible
   "
-# bugprone-assert-side-effect,
-# bugprone-bad-signal-to-kill-thread,
-# bugprone-bool-pointer-implicit-conversion,
-# bugprone-casting-through-void,
-# bugprone-chained-comparison,
-# bugprone-compare-pointer-to-member-virtual-function,
-# bugprone-copy-constructor-init,
-# bugprone-crtp-constructor-accessibility,
-# bugprone-dangling-handle,
-# bugprone-dynamic-static-initializers,
+# ---
+# checks that have some issues that need to be resolved:
+#
 # bugprone-empty-catch,
-# bugprone-fold-init-type,
-# bugprone-forward-declaration-namespace,
-# bugprone-inaccurate-erase,
+# bugprone-crtp-constructor-accessibility,
 # 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-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-multiple-new-in-one-expression,
-# 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,
-# bugprone-signed-char-misuse,
-# bugprone-sizeof-container,
-# bugprone-sizeof-expression,
-# bugprone-spuriously-wake-up-functions,
-# bugprone-standalone-empty,
-# bugprone-string-constructor,
-# bugprone-string-integer-assignment,
-# bugprone-string-literal-with-embedded-nul,
-# bugprone-stringview-nullptr,
-# bugprone-suspicious-enum-usage,
-# bugprone-suspicious-include,
-# bugprone-suspicious-memory-comparison,
-# bugprone-suspicious-memset-usage,
-# bugprone-suspicious-missing-comma,
-# bugprone-suspicious-realloc-usage,
-# bugprone-suspicious-semicolon,
-# bugprone-suspicious-string-compare,
-# bugprone-suspicious-stringview-data-usage,
-# bugprone-swapped-arguments,
-# bugprone-switch-missing-default-case,
-# bugprone-terminating-continue,
-# bugprone-throw-keyword-missing,
-# bugprone-too-small-loop-variable,
-# bugprone-undefined-memory-manipulation,
-# bugprone-undelegated-constructor,
-# bugprone-unhandled-exception-at-new,
-# bugprone-unhandled-self-assignment,
-# bugprone-unique-ptr-array-mismatch,
-# bugprone-unsafe-functions,
+# bugprone-move-forwarding-reference,
 # bugprone-unused-local-non-trivial-variable,
-# bugprone-unused-raii,
+# 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,
+# bugprone-optional-value-conversion,
+# bugprone-too-small-loop-variable,
 # bugprone-unused-return-value,
 # bugprone-use-after-move,
-# bugprone-virtual-near-miss,
-# cppcoreguidelines-init-variables,
+# bugprone-unhandled-self-assignment,
+# bugprone-unused-raii,
+#
 # cppcoreguidelines-misleading-capture-default-by-value,
-# cppcoreguidelines-no-suspend-with-lock,
+# cppcoreguidelines-init-variables,
 # cppcoreguidelines-pro-type-member-init,
 # cppcoreguidelines-pro-type-static-cast-downcast,
-# cppcoreguidelines-rvalue-reference-param-not-moved,
 # cppcoreguidelines-use-default-member-init,
-# cppcoreguidelines-virtual-class-destructor,
-# hicpp-ignored-remove-result,
+# cppcoreguidelines-rvalue-reference-param-not-moved,
+#
 # llvm-namespace-comment,
 # misc-const-correctness,
-# misc-definitions-in-headers,
-# misc-header-include-cycle,
 # misc-include-cleaner,
-# misc-misplaced-const,
 # misc-redundant-expression,
-# misc-static-assert,
-# misc-throw-by-value-catch-by-reference,
-# misc-unused-alias-decls,
-# misc-unused-using-decls,
+#
+# readability-avoid-nested-conditional-operator,
+# readability-avoid-return-with-void-value,
+# readability-braces-around-statements,
+# readability-container-contains,
+# readability-container-size-empty,
+# readability-convert-member-functions-to-static,
+# readability-const-return-type,
+# readability-else-after-return,
+# readability-implicit-bool-conversion,
+# readability-inconsistent-declaration-parameter-name,
+# readability-identifier-naming,
+# readability-make-member-function-const,
+# readability-math-missing-parentheses,
+# readability-redundant-inline-specifier,
+# readability-redundant-member-init,
+# readability-redundant-casting,
+# readability-redundant-string-init,
+# readability-simplify-boolean-expr,
+# readability-static-definition-in-anonymous-namespace,
+# readability-suspicious-call-argument,
+# readability-use-std-min-max,
+# readability-static-accessed-through-instance,
+#
 # modernize-concat-nested-namespaces,
-# modernize-deprecated-headers,
-# modernize-make-shared,
-# modernize-make-unique,
 # modernize-pass-by-value,
 # modernize-type-traits,
 # modernize-use-designated-initializers,
@@ -111,79 +149,50 @@ Checks: "-*,
 # modernize-use-starts-ends-with,
 # modernize-use-std-numbers,
 # modernize-use-using,
+#
 # 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-duplicate-include,
-# readability-else-after-return,
-# readability-enum-initial-value,
-# readability-implicit-bool-conversion,
-# readability-inconsistent-declaration-parameter-name,
-# readability-identifier-naming,
-# readability-make-member-function-const,
-# readability-math-missing-parentheses,
-# readability-misleading-indentation,
-# readability-non-const-parameter,
-# readability-redundant-casting,
-# readability-redundant-declaration,
-# readability-redundant-inline-specifier,
-# readability-redundant-member-init,
-# readability-redundant-string-init,
-# readability-reference-to-constructed-temporary,
-# readability-simplify-boolean-expr,
-# readability-static-accessed-through-instance,
-# readability-static-definition-in-anonymous-namespace,
-# readability-suspicious-call-argument,
-# readability-use-std-min-max
+# ---
 #
-# CheckOptions:
-#   readability-braces-around-statements.ShortStatementLines: 2
-#   readability-identifier-naming.MacroDefinitionCase: UPPER_CASE
-#   readability-identifier-naming.ClassCase: CamelCase
-#   readability-identifier-naming.StructCase: CamelCase
-#   readability-identifier-naming.UnionCase: CamelCase
-#   readability-identifier-naming.EnumCase: CamelCase
-#   readability-identifier-naming.EnumConstantCase: CamelCase
-#   readability-identifier-naming.ScopedEnumConstantCase: CamelCase
-#   readability-identifier-naming.GlobalConstantCase: UPPER_CASE
-#   readability-identifier-naming.GlobalConstantPrefix: "k"
-#   readability-identifier-naming.GlobalVariableCase: CamelCase
-#   readability-identifier-naming.GlobalVariablePrefix: "g"
-#   readability-identifier-naming.ConstexprFunctionCase: camelBack
-#   readability-identifier-naming.ConstexprMethodCase: camelBack
-#   readability-identifier-naming.ClassMethodCase: camelBack
-#   readability-identifier-naming.ClassMemberCase: camelBack
-#   readability-identifier-naming.ClassConstantCase: UPPER_CASE
-#   readability-identifier-naming.ClassConstantPrefix: "k"
-#   readability-identifier-naming.StaticConstantCase: UPPER_CASE
-#   readability-identifier-naming.StaticConstantPrefix: "k"
-#   readability-identifier-naming.StaticVariableCase: UPPER_CASE
-#   readability-identifier-naming.StaticVariablePrefix: "k"
-#   readability-identifier-naming.ConstexprVariableCase: UPPER_CASE
-#   readability-identifier-naming.ConstexprVariablePrefix: "k"
-#   readability-identifier-naming.LocalConstantCase: camelBack
-#   readability-identifier-naming.LocalVariableCase: camelBack
-#   readability-identifier-naming.TemplateParameterCase: CamelCase
-#   readability-identifier-naming.ParameterCase: camelBack
-#   readability-identifier-naming.FunctionCase: camelBack
-#   readability-identifier-naming.MemberCase: camelBack
-#   readability-identifier-naming.PrivateMemberSuffix: _
-#   readability-identifier-naming.ProtectedMemberSuffix: _
-#   readability-identifier-naming.PublicMemberSuffix: ""
-#   readability-identifier-naming.FunctionIgnoredRegexp: ".*tag_invoke.*"
-#   bugprone-unsafe-functions.ReportMoreUnsafeFunctions: true
+CheckOptions:
+  #   readability-braces-around-statements.ShortStatementLines: 2
+  #   readability-identifier-naming.MacroDefinitionCase: UPPER_CASE
+  #   readability-identifier-naming.ClassCase: CamelCase
+  #   readability-identifier-naming.StructCase: CamelCase
+  #   readability-identifier-naming.UnionCase: CamelCase
+  #   readability-identifier-naming.EnumCase: CamelCase
+  #   readability-identifier-naming.EnumConstantCase: CamelCase
+  #   readability-identifier-naming.ScopedEnumConstantCase: CamelCase
+  #   readability-identifier-naming.GlobalConstantCase: UPPER_CASE
+  #   readability-identifier-naming.GlobalConstantPrefix: "k"
+  #   readability-identifier-naming.GlobalVariableCase: CamelCase
+  #   readability-identifier-naming.GlobalVariablePrefix: "g"
+  #   readability-identifier-naming.ConstexprFunctionCase: camelBack
+  #   readability-identifier-naming.ConstexprMethodCase: camelBack
+  #   readability-identifier-naming.ClassMethodCase: camelBack
+  #   readability-identifier-naming.ClassMemberCase: camelBack
+  #   readability-identifier-naming.ClassConstantCase: UPPER_CASE
+  #   readability-identifier-naming.ClassConstantPrefix: "k"
+  #   readability-identifier-naming.StaticConstantCase: UPPER_CASE
+  #   readability-identifier-naming.StaticConstantPrefix: "k"
+  #   readability-identifier-naming.StaticVariableCase: UPPER_CASE
+  #   readability-identifier-naming.StaticVariablePrefix: "k"
+  #   readability-identifier-naming.ConstexprVariableCase: UPPER_CASE
+  #   readability-identifier-naming.ConstexprVariablePrefix: "k"
+  #   readability-identifier-naming.LocalConstantCase: camelBack
+  #   readability-identifier-naming.LocalVariableCase: camelBack
+  #   readability-identifier-naming.TemplateParameterCase: CamelCase
+  #   readability-identifier-naming.ParameterCase: camelBack
+  #   readability-identifier-naming.FunctionCase: camelBack
+  #   readability-identifier-naming.MemberCase: camelBack
+  #   readability-identifier-naming.PrivateMemberSuffix: _
+  #   readability-identifier-naming.ProtectedMemberSuffix: _
+  #   readability-identifier-naming.PublicMemberSuffix: ""
+  #   readability-identifier-naming.FunctionIgnoredRegexp: ".*tag_invoke.*"
+  bugprone-unsafe-functions.ReportMoreUnsafeFunctions: true
 #   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'
 #

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

@@ -235,8 +235,21 @@ jobs:
         env:
           BUILD_NPROC: ${{ steps.nproc.outputs.nproc }}
         run: |
-          ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}"
+          set -o pipefail
+          ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log
 
+      - name: Show test failure summary
+        if: ${{ failure() && !inputs.build_only }}
+        working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }}
+        run: |
+          if [ ! -f unittest.log ]; then
+            echo "unittest.log not found; embedded tests may not have run."
+            exit 0
+          fi
+
+          if ! grep -E "failed" unittest.log; then
+            echo "Log present but no failure lines found in unittest.log."
+          fi
       - name: Debug failure (Linux)
         if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }}
         run: |

.github/workflows/reusable-clang-tidy-files.yml

@@ -78,9 +78,9 @@ jobs:
         id: run_clang_tidy
         continue-on-error: true
         env:
-          FILES: ${{ inputs.files }}
+          TARGETS: ${{ inputs.files != '' && inputs.files || 'src tests' }}
         run: |
-          run-clang-tidy -j ${{ steps.nproc.outputs.nproc }} -p "$BUILD_DIR" $FILES 2>&1 | tee clang-tidy-output.txt
+          run-clang-tidy -j ${{ steps.nproc.outputs.nproc }} -p "${BUILD_DIR}" ${TARGETS} 2>&1 | tee clang-tidy-output.txt
 
       - name: Upload clang-tidy output
         if: steps.run_clang_tidy.outcome != 'success'

.github/workflows/reusable-clang-tidy.yml

@@ -22,7 +22,8 @@ jobs:
     if: ${{ inputs.check_only_changed }}
     runs-on: ubuntu-latest
     outputs:
-      any_changed: ${{ steps.changed_files.outputs.any_changed }}
+      clang_tidy_config_changed: ${{ steps.changed_clang_tidy.outputs.any_changed }}
+      any_cpp_changed: ${{ steps.changed_files.outputs.any_changed }}
       all_changed_files: ${{ steps.changed_files.outputs.all_changed_files }}
     steps:
       - name: Checkout repository
@@ -38,10 +39,17 @@ jobs:
             **/*.ipp
           separator: " "
 
+      - name: Get changed clang-tidy configuration
+        id: changed_clang_tidy
+        uses: tj-actions/changed-files@7dee1b0c1557f278e5c7dc244927139d78c0e22a # v47.0.4
+        with:
+          files: |
+            .clang-tidy
+
   run-clang-tidy:
     needs: [determine-files]
-    if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.any_changed == 'true') }}
+    if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.any_cpp_changed == 'true' || needs.determine-files.outputs.clang_tidy_config_changed == 'true') }}
     uses: ./.github/workflows/reusable-clang-tidy-files.yml
     with:
-      files: ${{ inputs.check_only_changed && needs.determine-files.outputs.all_changed_files || '' }}
+      files: ${{ (needs.determine-files.outputs.clang_tidy_config_changed == 'true' && '') || (inputs.check_only_changed && needs.determine-files.outputs.all_changed_files || '') }}
       create_issue_on_failure: ${{ inputs.create_issue_on_failure }}

CONTRIBUTING.md

@@ -251,6 +251,29 @@ pip3 install pre-commit
 pre-commit install
 ```
 
+## Clang-tidy
+
+All code must pass `clang-tidy` checks according to the settings in [`.clang-tidy`](./.clang-tidy).
+
+There is a Continuous Integration job that runs clang-tidy on pull requests. The CI will check:
+
+- All changed C++ files (`.cpp`, `.h`, `.ipp`) when only code files are modified
+- **All files in the repository** when the `.clang-tidy` configuration file is changed
+
+This ensures that configuration changes don't introduce new warnings across the codebase.
+
+### Running clang-tidy locally
+
+Before running clang-tidy, you must build the project to generate required files (particularly protobuf headers). Refer to [`BUILD.md`](./BUILD.md) for build instructions.
+
+Then run clang-tidy on your local changes:
+
+```
+run-clang-tidy -p build src tests
+```
+
+This will check all source files in the `src` and `tests` directories using the compile commands from your `build` directory.
+
 ## Contracts and instrumentation
 
 We are using [Antithesis](https://antithesis.com/) for continuous fuzzing,

src/test/app/Vault_test.cpp

@@ -5340,20 +5340,20 @@ class Vault_test : public beast::unit_test::suite
             env.close();
 
             // 2. Mantissa larger than uint64 max
+            env.set_parse_failure_expected(true);
             try
             {
                 tx[sfAssetsMaximum] = "18446744073709551617e5";  // uint64 max + 1
                 env(tx, THISLINE);
-                BEAST_EXPECT(false);
+                BEAST_EXPECTS(false, "Expected parse_error for mantissa larger than uint64 max");
             }
             catch (parse_error const& e)
             {
                 using namespace std::string_literals;
                 BEAST_EXPECT(
-                    e.what() ==
-                    "invalidParamsField 'tx_json.AssetsMaximum' has invalid "
-                    "data."s);
+                    e.what() == "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."s);
             }
+            env.set_parse_failure_expected(false);
         }
     }