Merge branch 'dev' into coverage

* Speed up the generation of coverage reports by using multiple cores.

* Add codecov step to coverage workflow.

.codecov.yml

@@ -0,0 +1,6 @@
+coverage:
+  status:
+    project:
+      default:
+        target: 60%
+        threshold: 2%

.github/actions/xahau-ga-build/action.yml

@@ -2,6 +2,14 @@ name: build
 description: 'Builds the project with ccache integration'
 
 inputs:
+  cmake-target:
+    description: 'CMake target to build'
+    required: false
+    default: all
+  cmake-args:
+    description: 'Additional CMake arguments'
+    required: false
+    default: null
   generator:
     description: 'CMake generator to use'
     required: true
@@ -20,6 +28,10 @@ inputs:
     description: 'C++ compiler to use'
     required: false
     default: ''
+  gcov:
+    description: 'Gcov to use'
+    required: false
+    default: ''
   compiler-id:
     description: 'Unique identifier: compiler-version-stdlib[-gccversion] (e.g. clang-14-libstdcxx-gcc11, gcc-13-libstdcxx)'
     required: false
@@ -122,6 +134,11 @@ runs:
           export CXX="${{ inputs.cxx }}"
         fi
 
+        if [ -n "${{ inputs.gcov }}" ]; then
+          ln -sf /usr/bin/${{ inputs.gcov }} /usr/local/bin/gcov
+          export CMAKE_BUILD_PARALLEL_LEVEL=$(nproc)
+        fi
+
         # Create wrapper toolchain that overlays ccache on top of Conan's toolchain
         # This enables ccache for the main app build without affecting Conan dependency builds
         if [ "${{ inputs.ccache_enabled }}" = "true" ]; then
@@ -183,7 +200,8 @@ runs:
           -G "${{ inputs.generator }}" \
           ${CMAKE_CXX_FLAGS:+-DCMAKE_CXX_FLAGS="$CMAKE_CXX_FLAGS"} \
           -DCMAKE_TOOLCHAIN_FILE:FILEPATH=${TOOLCHAIN_FILE} \
-          -DCMAKE_BUILD_TYPE=${{ inputs.configuration }}
+          -DCMAKE_BUILD_TYPE=${{ inputs.configuration }} \
+          ${{ inputs.cmake-args }}
 
     - name: Show ccache config before build
       if: inputs.ccache_enabled == 'true'
@@ -207,7 +225,7 @@ runs:
           VERBOSE_FLAG="-- -v"
         fi
 
-        cmake --build . --config ${{ inputs.configuration }} --parallel $(nproc) ${VERBOSE_FLAG}
+        cmake --build . --config ${{ inputs.configuration }} --parallel $(nproc) ${VERBOSE_FLAG} --target ${{ inputs.cmake-target }}
 
     - name: Show ccache statistics
       if: inputs.ccache_enabled == 'true'

.github/workflows/xahau-ga-nix.yml

@@ -47,7 +47,8 @@ jobs:
                   "cxx": "g++-11",
                   "compiler_version": 11,
                   "stdlib": "libstdcxx",
-                  "configuration": "Debug"
+                  "configuration": "Debug",
+                  "job_type": "build"
               },
               {
                   "compiler_id": "gcc-13-libstdcxx",
@@ -55,8 +56,19 @@ jobs:
                   "cc": "gcc-13",
                   "cxx": "g++-13",
                   "compiler_version": 13,
+                  "configuration": "Debug",
+                  "job_type": "build"
+              },
+              {
+                  "compiler_id": "gcc-13-libstdcxx",
+                  "compiler": "gcc",
+                  "cc": "gcc-13",
+                  "cxx": "g++-13",
+                  "gcov": "gcov-13",
+                  "compiler_version": 13,
                   "stdlib": "libstdcxx",
-                  "configuration": "Debug"
+                  "configuration": "Debug",
+                  "job_type": "coverage"
               },
               {
                   "compiler_id": "clang-14-libstdcxx-gcc11",
@@ -66,7 +78,8 @@ jobs:
                   "compiler_version": 14,
                   "stdlib": "libstdcxx",
                   "clang_gcc_toolchain": 11,
-                  "configuration": "Debug"
+                  "configuration": "Debug",
+                  "job_type": "build"
               },
               {
                   "compiler_id": "clang-16-libstdcxx-gcc13",
@@ -76,7 +89,8 @@ jobs:
                   "compiler_version": 16,
                   "stdlib": "libstdcxx",
                   "clang_gcc_toolchain": 13,
-                  "configuration": "Debug"
+                  "configuration": "Debug",
+                  "job_type": "build"
               },
               {
                   "compiler_id": "clang-17-libcxx",
@@ -85,7 +99,8 @@ jobs:
                   "cxx": "clang++-17",
                   "compiler_version": 17,
                   "stdlib": "libcxx",
-                  "configuration": "Debug"
+                  "configuration": "Debug",
+                  "job_type": "build"
               },
               {
                   # Clang 18 - testing if it's faster than Clang 17 with libc++
@@ -96,14 +111,16 @@ jobs:
                   "cxx": "clang++-18",
                   "compiler_version": 18,
                   "stdlib": "libcxx",
-                  "configuration": "Debug"
+                  "configuration": "Debug",
+                  "job_type": "build"
               }
           ]
           
           # Minimal matrix for PRs and feature branches
           minimal_matrix = [
               full_matrix[1],  # gcc-13 (middle-ground gcc)
-              full_matrix[2]   # clang-14 (mature, stable clang)
+              full_matrix[2],  # gcc-13 coverage
+              full_matrix[3]   # clang-14 (mature, stable clang)
           ]
           
           # Determine which matrix to use based on the target branch
@@ -161,14 +178,21 @@ jobs:
           # Select the appropriate matrix
           if use_full:
               if force_full:
-                  print(f"Using FULL matrix (6 configs) - forced by [ci-nix-full-matrix] tag")
+                  print(f"Using FULL matrix (7 configs) - forced by [ci-nix-full-matrix] tag")
               else:
-                  print(f"Using FULL matrix (6 configs) - targeting main branch")
+                  print(f"Using FULL matrix (7 configs) - targeting main branch")
               matrix = full_matrix
           else:
-              print(f"Using MINIMAL matrix (2 configs) - feature branch/PR")
+              print(f"Using MINIMAL matrix (3 configs) - feature branch/PR")
               matrix = minimal_matrix
-          
+
+          # Add runs_on based on job_type
+          for entry in matrix:
+              if entry.get("job_type") == "coverage":
+                  entry["runs_on"] = '["self-hosted", "generic", 24.04]'
+              else:
+                  entry["runs_on"] = '["self-hosted", "generic", 20.04]'
+
           # Output the matrix as JSON
           output = json.dumps({"include": matrix})
           with open(os.environ['GITHUB_OUTPUT'], 'a') as f:
@@ -176,7 +200,7 @@ jobs:
 
   build:
     needs: matrix-setup
-    runs-on: [self-hosted, generic, 20.04]
+    runs-on: ${{ fromJSON(matrix.runs_on) }}
     container:
       image: ubuntu:24.04
       volumes:
@@ -205,7 +229,7 @@ jobs:
           apt-get install -y software-properties-common
           add-apt-repository ppa:ubuntu-toolchain-r/test -y
           apt-get update
-          apt-get install -y python3 python-is-python3 pipx
+          apt-get install -y git python3 python-is-python3 pipx
           pipx ensurepath
           apt-get install -y cmake ninja-build ${{ matrix.cc }} ${{ matrix.cxx }} ccache
           apt-get install -y perl # for openssl build
@@ -276,6 +300,12 @@ jobs:
           pipx install "conan>=2.0,<3"
           echo "$HOME/.local/bin" >> $GITHUB_PATH
 
+          # Install gcovr for coverage jobs
+          if [ "${{ matrix.job_type }}" = "coverage" ]; then
+            pipx install "gcovr>=7,<9"
+            apt-get install -y lcov
+          fi
+
       - name: Check environment
         run: |
           echo "PATH:"
@@ -285,6 +315,13 @@ jobs:
           which ${{ matrix.cc }} && ${{ matrix.cc }} --version || echo "${{ matrix.cc }} not found"
           which ${{ matrix.cxx }} && ${{ matrix.cxx }} --version || echo "${{ matrix.cxx }} not found"
           which ccache && ccache --version || echo "ccache not found"
+
+          # Check gcovr for coverage jobs
+          if [ "${{ matrix.job_type }}" = "coverage" ]; then
+            which gcov && gcov --version || echo "gcov not found"
+            which gcovr && gcovr --version || echo "gcovr not found"
+          fi
+
           echo "---- Full Environment ----"
           env
 
@@ -312,6 +349,7 @@ jobs:
           gha_cache_enabled: 'false' # Disable caching for self hosted runner
 
       - name: Build
+        if: matrix.job_type == 'build'
         uses: ./.github/actions/xahau-ga-build
         with:
           generator: Ninja
@@ -326,7 +364,25 @@ jobs:
           clang_gcc_toolchain: ${{ matrix.clang_gcc_toolchain || '' }}
           ccache_max_size: '100G'
 
+      - name: Build (Coverage)
+        if: matrix.job_type == 'coverage'
+        uses: ./.github/actions/xahau-ga-build
+        with:
+          generator: Ninja
+          configuration: ${{ matrix.configuration }}
+          build_dir: ${{ env.build_dir }}
+          cc: ${{ matrix.cc }}
+          cxx: ${{ matrix.cxx }}
+          gcov: ${{ matrix.gcov }}
+          compiler-id: ${{ matrix.compiler_id }}
+          cache_version: ${{ env.CACHE_VERSION }}
+          main_branch: ${{ env.MAIN_BRANCH_NAME }}
+          cmake-args: '-Dcoverage=ON -Dcoverage_format=xml -DCODE_COVERAGE_VERBOSE=ON -DCMAKE_CXX_FLAGS="-O0" -DCMAKE_C_FLAGS="-O0"'
+          cmake-target: 'coverage'
+          ccache_max_size: '100G'
+
       - name: Set artifact name
+        if: matrix.job_type == 'build'
         id: set-artifact-name
         run: |
           ARTIFACT_NAME="build-output-nix-${{ github.run_id }}-${{ matrix.compiler }}-${{ matrix.configuration }}"
@@ -339,6 +395,7 @@ jobs:
           ls -la ${{ env.build_dir }} || echo "Build directory not found or empty"
 
       - name: Run tests
+        if: matrix.job_type == 'build'
         run: |
           # Ensure the binary exists before trying to run
           if [ -f "${{ env.build_dir }}/rippled" ]; then
@@ -347,3 +404,33 @@ jobs:
              echo "Error: rippled executable not found in ${{ env.build_dir }}"
              exit 1
           fi
+
+      # Coverage-specific steps
+      - name: Move coverage report
+        if: matrix.job_type == 'coverage'
+        shell: bash
+        run: |
+          mv "${{ env.build_dir }}/coverage.xml" ./
+
+      - name: Archive coverage report
+        if: matrix.job_type == 'coverage'
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage.xml
+          path: coverage.xml
+          retention-days: 30
+
+      - name: Upload coverage report
+        if: matrix.job_type == 'coverage'
+        uses: wandalen/wretry.action/main@v3
+        with:
+          action: codecov/codecov-action@v4.3.0
+          with: |
+            files: coverage.xml
+            fail_ci_if_error: true
+            disable_search: true
+            verbose: true
+            plugin: noop
+            token: ${{ secrets.CODECOV_TOKEN }}
+          attempt_limit: 5
+          attempt_delay: 210000 # in milliseconds

BUILD.md

@@ -258,12 +258,72 @@ can't build earlier Boost versions.
    generator. Pass `--help` to see the rest of the command line options.
 
 
+## Coverage report
+
+The coverage report is intended for developers using compilers GCC
+or Clang (including Apple Clang). It is generated by the build target `coverage`,
+which is only enabled when the `coverage` option is set, e.g. with
+`--options coverage=True` in `conan` or `-Dcoverage=ON` variable in `cmake`
+
+Prerequisites for the coverage report:
+
+- [gcovr tool][gcovr] (can be installed e.g. with [pip][python-pip])
+- `gcov` for GCC (installed with the compiler by default) or
+- `llvm-cov` for Clang (installed with the compiler by default)
+- `Debug` build type
+
+A coverage report is created when the following steps are completed, in order:
+
+1. `rippled` binary built with instrumentation data, enabled by the `coverage`
+   option mentioned above
+2. completed run of unit tests, which populates coverage capture data
+3. completed run of the `gcovr` tool (which internally invokes either `gcov` or `llvm-cov`)
+   to assemble both instrumentation data and the coverage capture data into a coverage report
+
+The above steps are automated into a single target `coverage`. The instrumented
+`rippled` binary can also be used for regular development or testing work, at
+the cost of extra disk space utilization and a small performance hit
+(to store coverage capture). In case of a spurious failure of unit tests, it is
+possible to re-run the `coverage` target without rebuilding the `rippled` binary
+(since it is simply a dependency of the coverage report target). It is also possible
+to select only specific tests for the purpose of the coverage report, by setting
+the `coverage_test` variable in `cmake`
+
+The default coverage report format is `html-details`, but the user
+can override it to any of the formats listed in `Builds/CMake/CodeCoverage.cmake`
+by setting the `coverage_format` variable in `cmake`. It is also possible
+to generate more than one format at a time by setting the `coverage_extra_args`
+variable in `cmake`. The specific command line used to run the `gcovr` tool will be
+displayed if the `CODE_COVERAGE_VERBOSE` variable is set.
+
+By default, the code coverage tool runs parallel unit tests with `--unittest-jobs`
+ set to the number of available CPU cores. This may cause spurious test
+errors on Apple. Developers can override the number of unit test jobs with
+the `coverage_test_parallelism` variable in `cmake`.
+
+Example use with some cmake variables set:
+
+```
+cd .build
+conan install .. --output-folder . --build missing --settings build_type=Debug
+cmake -DCMAKE_BUILD_TYPE=Debug -Dcoverage=ON -Dcoverage_test_parallelism=2 -Dcoverage_format=html-details -Dcoverage_extra_args="--json coverage.json" -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake ..
+cmake --build . --target coverage
+```
+
+After the `coverage` target is completed, the generated coverage report will be
+stored inside the build directory, as either of:
+
+- file named `coverage.`_extension_ , with a suitable extension for the report format, or
+- directory named `coverage`, with the `index.html` and other files inside, for the `html-details` or `html-nested` report formats.
+
+
 ## Options
 
 | Option | Default Value | Description |
 | --- | ---| ---|
 | `assert` | OFF | Enable assertions.
 | `reporting` | OFF | Build the reporting mode feature. |
+| `coverage` | OFF | Prepare the coverage report. |
 | `tests` | ON | Build tests. |
 | `unity` | ON | Configure a unity build. |
 | `san` | N/A | Enable a sanitizer with Clang. Choices are `thread` and `address`. |
@@ -456,6 +516,10 @@ but it is more convenient to put them in a [profile][profile].
 
 [1]: https://github.com/conan-io/conan-center-index/issues/13168
 [5]: https://en.wikipedia.org/wiki/Unity_build
+[6]: https://github.com/boostorg/beast/issues/2648
+[7]: https://github.com/boostorg/beast/issues/2661
+[gcovr]: https://gcovr.com/en/stable/getting-started.html
+[python-pip]: https://packaging.python.org/en/latest/guides/installing-using-pip-and-virtual-environments/
 [build_type]: https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html
 [runtime]: https://cmake.org/cmake/help/latest/variable/CMAKE_MSVC_RUNTIME_LIBRARY.html
 [toolchain]: https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html

Builds/CMake/CodeCoverage.cmake

@@ -0,0 +1,440 @@
+# Copyright (c) 2012 - 2017, Lars Bilke
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without modification,
+# are permitted provided that the following conditions are met:
+#
+# 1. Redistributions of source code must retain the above copyright notice, this
+#    list of conditions and the following disclaimer.
+#
+# 2. Redistributions in binary form must reproduce the above copyright notice,
+#    this list of conditions and the following disclaimer in the documentation
+#    and/or other materials provided with the distribution.
+#
+# 3. Neither the name of the copyright holder nor the names of its contributors
+#    may be used to endorse or promote products derived from this software without
+#    specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR
+# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+# CHANGES:
+#
+# 2012-01-31, Lars Bilke
+# - Enable Code Coverage
+#
+# 2013-09-17, Joakim Söderberg
+# - Added support for Clang.
+# - Some additional usage instructions.
+#
+# 2016-02-03, Lars Bilke
+# - Refactored functions to use named parameters
+#
+# 2017-06-02, Lars Bilke
+# - Merged with modified version from github.com/ufz/ogs
+#
+# 2019-05-06, Anatolii Kurotych
+# - Remove unnecessary --coverage flag
+#
+# 2019-12-13, FeRD (Frank Dana)
+# - Deprecate COVERAGE_LCOVR_EXCLUDES and COVERAGE_GCOVR_EXCLUDES lists in favor
+#   of tool-agnostic COVERAGE_EXCLUDES variable, or EXCLUDE setup arguments.
+# - CMake 3.4+: All excludes can be specified relative to BASE_DIRECTORY
+# - All setup functions: accept BASE_DIRECTORY, EXCLUDE list
+# - Set lcov basedir with -b argument
+# - Add automatic --demangle-cpp in lcovr, if 'c++filt' is available (can be
+#   overridden with NO_DEMANGLE option in setup_target_for_coverage_lcovr().)
+# - Delete output dir, .info file on 'make clean'
+# - Remove Python detection, since version mismatches will break gcovr
+# - Minor cleanup (lowercase function names, update examples...)
+#
+# 2019-12-19, FeRD (Frank Dana)
+# - Rename Lcov outputs, make filtered file canonical, fix cleanup for targets
+#
+# 2020-01-19, Bob Apthorpe
+# - Added gfortran support
+#
+# 2020-02-17, FeRD (Frank Dana)
+# - Make all add_custom_target()s VERBATIM to auto-escape wildcard characters
+#   in EXCLUDEs, and remove manual escaping from gcovr targets
+#
+# 2021-01-19, Robin Mueller
+# - Add CODE_COVERAGE_VERBOSE option which will allow to print out commands which are run
+# - Added the option for users to set the GCOVR_ADDITIONAL_ARGS variable to supply additional
+#   flags to the gcovr command
+#
+# 2020-05-04, Mihchael Davis
+#     - Add -fprofile-abs-path to make gcno files contain absolute paths
+#     - Fix BASE_DIRECTORY not working when defined
+#     - Change BYPRODUCT from folder to index.html to stop ninja from complaining about double defines
+#
+# 2021-05-10, Martin Stump
+#     - Check if the generator is multi-config before warning about non-Debug builds
+#
+# 2022-02-22, Marko Wehle
+#     - Change gcovr output from -o <filename> for --xml <filename> and --html <filename> output respectively.
+#       This will allow for Multiple Output Formats at the same time by making use of GCOVR_ADDITIONAL_ARGS, e.g. GCOVR_ADDITIONAL_ARGS "--txt".
+#
+# 2022-09-28, Sebastian Mueller
+#     - fix append_coverage_compiler_flags_to_target to correctly add flags
+#     - replace "-fprofile-arcs -ftest-coverage" with "--coverage" (equivalent)
+#
+# 2024-01-04, Bronek Kozicki
+#     - remove setup_target_for_coverage_lcov (slow) and setup_target_for_coverage_fastcov (no support for Clang)
+#     - fix Clang support by adding find_program( ... llvm-cov )
+#     - add Apple Clang support by adding execute_process( COMMAND xcrun -f llvm-cov ... )
+#     - add CODE_COVERAGE_GCOV_TOOL to explicitly select gcov tool and disable find_program
+#     - replace both functions setup_target_for_coverage_gcovr_* with a single setup_target_for_coverage_gcovr
+#     - add support for all gcovr output formats
+#
+# USAGE:
+#
+# 1. Copy this file into your cmake modules path.
+#
+# 2. Add the following line to your CMakeLists.txt (best inside an if-condition
+#    using a CMake option() to enable it just optionally):
+#      include(CodeCoverage)
+#
+# 3. Append necessary compiler flags for all supported source files:
+#      append_coverage_compiler_flags()
+#    Or for specific target:
+#      append_coverage_compiler_flags_to_target(YOUR_TARGET_NAME)
+#
+# 3.a (OPTIONAL) Set appropriate optimization flags, e.g. -O0, -O1 or -Og
+#
+# 4. If you need to exclude additional directories from the report, specify them
+#    using full paths in the COVERAGE_EXCLUDES variable before calling
+#    setup_target_for_coverage_*().
+#    Example:
+#      set(COVERAGE_EXCLUDES
+#          '${PROJECT_SOURCE_DIR}/src/dir1/*'
+#          '/path/to/my/src/dir2/*')
+#    Or, use the EXCLUDE argument to setup_target_for_coverage_*().
+#    Example:
+#      setup_target_for_coverage_gcovr(
+#          NAME coverage
+#          EXECUTABLE testrunner
+#          EXCLUDE "${PROJECT_SOURCE_DIR}/src/dir1/*" "/path/to/my/src/dir2/*")
+#
+# 4.a NOTE: With CMake 3.4+, COVERAGE_EXCLUDES or EXCLUDE can also be set
+#     relative to the BASE_DIRECTORY (default: PROJECT_SOURCE_DIR)
+#     Example:
+#       set(COVERAGE_EXCLUDES "dir1/*")
+#       setup_target_for_coverage_gcovr(
+#           NAME coverage
+#           EXECUTABLE testrunner
+#           FORMAT html-details
+#           BASE_DIRECTORY "${PROJECT_SOURCE_DIR}/src"
+#           EXCLUDE "dir2/*")
+#
+# 4.b If you need to pass specific options to gcovr, specify them in
+#     GCOVR_ADDITIONAL_ARGS variable.
+#     Example:
+#       set (GCOVR_ADDITIONAL_ARGS --exclude-throw-branches --exclude-noncode-lines -s)
+#       setup_target_for_coverage_gcovr(
+#           NAME coverage
+#           EXECUTABLE testrunner
+#           EXCLUDE "src/dir1" "src/dir2")
+#
+# 5. Use the functions described below to create a custom make target which
+#    runs your test executable and produces a code coverage report.
+#
+# 6. Build a Debug build:
+#      cmake -DCMAKE_BUILD_TYPE=Debug ..
+#      make
+#      make my_coverage_target
+
+include(CMakeParseArguments)
+
+option(CODE_COVERAGE_VERBOSE "Verbose information" FALSE)
+
+# Check prereqs
+find_program( GCOVR_PATH gcovr PATHS ${CMAKE_SOURCE_DIR}/scripts/test)
+
+if(DEFINED CODE_COVERAGE_GCOV_TOOL)
+  set(GCOV_TOOL "${CODE_COVERAGE_GCOV_TOOL}")
+elseif(DEFINED ENV{CODE_COVERAGE_GCOV_TOOL})
+  set(GCOV_TOOL "$ENV{CODE_COVERAGE_GCOV_TOOL}")
+elseif("${CMAKE_CXX_COMPILER_ID}" MATCHES "(Apple)?[Cc]lang")
+  if(APPLE)
+    execute_process( COMMAND xcrun -f llvm-cov
+      OUTPUT_VARIABLE LLVMCOV_PATH
+      OUTPUT_STRIP_TRAILING_WHITESPACE
+    )
+  else()
+    find_program( LLVMCOV_PATH llvm-cov )
+  endif()
+  if(LLVMCOV_PATH)
+    set(GCOV_TOOL "${LLVMCOV_PATH} gcov")
+  endif()
+elseif("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
+  find_program( GCOV_PATH gcov )
+  set(GCOV_TOOL "${GCOV_PATH}")
+endif()
+
+# Check supported compiler (Clang, GNU and Flang)
+get_property(LANGUAGES GLOBAL PROPERTY ENABLED_LANGUAGES)
+foreach(LANG ${LANGUAGES})
+  if("${CMAKE_${LANG}_COMPILER_ID}" MATCHES "(Apple)?[Cc]lang")
+    if("${CMAKE_${LANG}_COMPILER_VERSION}" VERSION_LESS 3)
+      message(FATAL_ERROR "Clang version must be 3.0.0 or greater! Aborting...")
+    endif()
+  elseif(NOT "${CMAKE_${LANG}_COMPILER_ID}" MATCHES "GNU"
+         AND NOT "${CMAKE_${LANG}_COMPILER_ID}" MATCHES "(LLVM)?[Ff]lang")
+    message(FATAL_ERROR "Compiler is not GNU or Flang! Aborting...")
+  endif()
+endforeach()
+
+set(COVERAGE_COMPILER_FLAGS "-g --coverage"
+    CACHE INTERNAL "")
+if(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Clang)")
+    include(CheckCXXCompilerFlag)
+    check_cxx_compiler_flag(-fprofile-abs-path HAVE_cxx_fprofile_abs_path)
+    if(HAVE_cxx_fprofile_abs_path)
+        set(COVERAGE_CXX_COMPILER_FLAGS "${COVERAGE_COMPILER_FLAGS} -fprofile-abs-path")
+    endif()
+    include(CheckCCompilerFlag)
+    check_c_compiler_flag(-fprofile-abs-path HAVE_c_fprofile_abs_path)
+    if(HAVE_c_fprofile_abs_path)
+        set(COVERAGE_C_COMPILER_FLAGS "${COVERAGE_COMPILER_FLAGS} -fprofile-abs-path")
+    endif()
+endif()
+
+set(CMAKE_Fortran_FLAGS_COVERAGE
+    ${COVERAGE_COMPILER_FLAGS}
+    CACHE STRING "Flags used by the Fortran compiler during coverage builds."
+    FORCE )
+set(CMAKE_CXX_FLAGS_COVERAGE
+    ${COVERAGE_COMPILER_FLAGS}
+    CACHE STRING "Flags used by the C++ compiler during coverage builds."
+    FORCE )
+set(CMAKE_C_FLAGS_COVERAGE
+    ${COVERAGE_COMPILER_FLAGS}
+    CACHE STRING "Flags used by the C compiler during coverage builds."
+    FORCE )
+set(CMAKE_EXE_LINKER_FLAGS_COVERAGE
+    ""
+    CACHE STRING "Flags used for linking binaries during coverage builds."
+    FORCE )
+set(CMAKE_SHARED_LINKER_FLAGS_COVERAGE
+    ""
+    CACHE STRING "Flags used by the shared libraries linker during coverage builds."
+    FORCE )
+mark_as_advanced(
+    CMAKE_Fortran_FLAGS_COVERAGE
+    CMAKE_CXX_FLAGS_COVERAGE
+    CMAKE_C_FLAGS_COVERAGE
+    CMAKE_EXE_LINKER_FLAGS_COVERAGE
+    CMAKE_SHARED_LINKER_FLAGS_COVERAGE )
+
+get_property(GENERATOR_IS_MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
+if(NOT (CMAKE_BUILD_TYPE STREQUAL "Debug" OR GENERATOR_IS_MULTI_CONFIG))
+    message(WARNING "Code coverage results with an optimised (non-Debug) build may be misleading")
+endif() # NOT (CMAKE_BUILD_TYPE STREQUAL "Debug" OR GENERATOR_IS_MULTI_CONFIG)
+
+if(CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_Fortran_COMPILER_ID STREQUAL "GNU")
+    link_libraries(gcov)
+endif()
+
+# Defines a target for running and collection code coverage information
+# Builds dependencies, runs the given executable and outputs reports.
+# NOTE! The executable should always have a ZERO as exit code otherwise
+# the coverage generation will not complete.
+#
+# setup_target_for_coverage_gcovr(
+#     NAME ctest_coverage                    # New target name
+#     EXECUTABLE ctest -j ${PROCESSOR_COUNT} # Executable in PROJECT_BINARY_DIR
+#     DEPENDENCIES executable_target         # Dependencies to build first
+#     BASE_DIRECTORY "../"                   # Base directory for report
+#                                            #  (defaults to PROJECT_SOURCE_DIR)
+#     FORMAT "cobertura"                     # Output format, one of:
+#                                            #  xml cobertura sonarqube json-summary
+#                                            #  json-details coveralls csv txt
+#                                            #  html-single html-nested html-details
+#                                            #  (xml is an alias to cobertura;
+#                                            #  if no format is set, defaults to xml)
+#     EXCLUDE "src/dir1/*" "src/dir2/*"      # Patterns to exclude (can be relative
+#                                            #  to BASE_DIRECTORY, with CMake 3.4+)
+# )
+# The user can set the variable GCOVR_ADDITIONAL_ARGS to supply additional flags to the
+# GCVOR command.
+function(setup_target_for_coverage_gcovr)
+    set(options NONE)
+    set(oneValueArgs BASE_DIRECTORY NAME FORMAT)
+    set(multiValueArgs EXCLUDE EXECUTABLE EXECUTABLE_ARGS DEPENDENCIES)
+    cmake_parse_arguments(Coverage "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
+
+    if(NOT GCOV_TOOL)
+        message(FATAL_ERROR "Could not find gcov or llvm-cov tool! Aborting...")
+    endif()
+
+    if(NOT GCOVR_PATH)
+        message(FATAL_ERROR "Could not find gcovr tool! Aborting...")
+    endif()
+
+    # Set base directory (as absolute path), or default to PROJECT_SOURCE_DIR
+    if(DEFINED Coverage_BASE_DIRECTORY)
+        get_filename_component(BASEDIR ${Coverage_BASE_DIRECTORY} ABSOLUTE)
+    else()
+        set(BASEDIR ${PROJECT_SOURCE_DIR})
+    endif()
+
+    if(NOT DEFINED Coverage_FORMAT)
+        set(Coverage_FORMAT xml)
+    endif()
+
+    if("--output" IN_LIST GCOVR_ADDITIONAL_ARGS)
+        message(FATAL_ERROR "Unsupported --output option detected in GCOVR_ADDITIONAL_ARGS! Aborting...")
+    else()
+        if((Coverage_FORMAT STREQUAL "html-details")
+            OR (Coverage_FORMAT STREQUAL "html-nested"))
+            set(GCOVR_OUTPUT_FILE ${PROJECT_BINARY_DIR}/${Coverage_NAME}/index.html)
+            set(GCOVR_CREATE_FOLDER ${PROJECT_BINARY_DIR}/${Coverage_NAME})
+        elseif(Coverage_FORMAT STREQUAL "html-single")
+            set(GCOVR_OUTPUT_FILE ${Coverage_NAME}.html)
+        elseif((Coverage_FORMAT STREQUAL "json-summary")
+            OR (Coverage_FORMAT STREQUAL "json-details")
+            OR (Coverage_FORMAT STREQUAL "coveralls"))
+            set(GCOVR_OUTPUT_FILE ${Coverage_NAME}.json)
+        elseif(Coverage_FORMAT STREQUAL "txt")
+            set(GCOVR_OUTPUT_FILE ${Coverage_NAME}.txt)
+        elseif(Coverage_FORMAT STREQUAL "csv")
+            set(GCOVR_OUTPUT_FILE ${Coverage_NAME}.csv)
+        else()
+            set(GCOVR_OUTPUT_FILE ${Coverage_NAME}.xml)
+        endif()
+    endif()
+
+    if((Coverage_FORMAT STREQUAL "cobertura")
+        OR (Coverage_FORMAT STREQUAL "xml"))
+        list(APPEND GCOVR_ADDITIONAL_ARGS --cobertura "${GCOVR_OUTPUT_FILE}" )
+        list(APPEND GCOVR_ADDITIONAL_ARGS --cobertura-pretty )
+        set(Coverage_FORMAT cobertura) # overwrite xml
+    elseif(Coverage_FORMAT STREQUAL "sonarqube")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --sonarqube "${GCOVR_OUTPUT_FILE}" )
+    elseif(Coverage_FORMAT STREQUAL "json-summary")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --json-summary "${GCOVR_OUTPUT_FILE}" )
+        list(APPEND GCOVR_ADDITIONAL_ARGS --json-summary-pretty)
+    elseif(Coverage_FORMAT STREQUAL "json-details")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --json "${GCOVR_OUTPUT_FILE}" )
+        list(APPEND GCOVR_ADDITIONAL_ARGS --json-pretty)
+    elseif(Coverage_FORMAT STREQUAL "coveralls")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --coveralls "${GCOVR_OUTPUT_FILE}" )
+        list(APPEND GCOVR_ADDITIONAL_ARGS --coveralls-pretty)
+    elseif(Coverage_FORMAT STREQUAL "csv")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --csv "${GCOVR_OUTPUT_FILE}" )
+    elseif(Coverage_FORMAT STREQUAL "txt")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --txt "${GCOVR_OUTPUT_FILE}" )
+    elseif(Coverage_FORMAT STREQUAL "html-single")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --html "${GCOVR_OUTPUT_FILE}" )
+        list(APPEND GCOVR_ADDITIONAL_ARGS --html-self-contained)
+    elseif(Coverage_FORMAT STREQUAL "html-nested")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --html-nested "${GCOVR_OUTPUT_FILE}" )
+    elseif(Coverage_FORMAT STREQUAL "html-details")
+        list(APPEND GCOVR_ADDITIONAL_ARGS --html-details "${GCOVR_OUTPUT_FILE}" )
+    else()
+        message(FATAL_ERROR "Unsupported output style ${Coverage_FORMAT}! Aborting...")
+    endif()
+
+    # Collect excludes (CMake 3.4+: Also compute absolute paths)
+    set(GCOVR_EXCLUDES "")
+    foreach(EXCLUDE ${Coverage_EXCLUDE} ${COVERAGE_EXCLUDES} ${COVERAGE_GCOVR_EXCLUDES})
+        if(CMAKE_VERSION VERSION_GREATER 3.4)
+            get_filename_component(EXCLUDE ${EXCLUDE} ABSOLUTE BASE_DIR ${BASEDIR})
+        endif()
+        list(APPEND GCOVR_EXCLUDES "${EXCLUDE}")
+    endforeach()
+    list(REMOVE_DUPLICATES GCOVR_EXCLUDES)
+
+    # Combine excludes to several -e arguments
+    set(GCOVR_EXCLUDE_ARGS "")
+    foreach(EXCLUDE ${GCOVR_EXCLUDES})
+        list(APPEND GCOVR_EXCLUDE_ARGS "-e")
+        list(APPEND GCOVR_EXCLUDE_ARGS "${EXCLUDE}")
+    endforeach()
+
+    # Set up commands which will be run to generate coverage data
+    # Run tests
+    set(GCOVR_EXEC_TESTS_CMD
+        ${Coverage_EXECUTABLE} ${Coverage_EXECUTABLE_ARGS}
+    )
+
+    # Create folder
+    if(DEFINED GCOVR_CREATE_FOLDER)
+        set(GCOVR_FOLDER_CMD
+            ${CMAKE_COMMAND} -E make_directory ${GCOVR_CREATE_FOLDER})
+    else()
+        set(GCOVR_FOLDER_CMD echo) # dummy
+    endif()
+
+    # Running gcovr
+    set(GCOVR_CMD
+        ${GCOVR_PATH}
+        --gcov-executable ${GCOV_TOOL}
+        --gcov-ignore-parse-errors=negative_hits.warn_once_per_file
+        -r ${BASEDIR}
+        ${GCOVR_ADDITIONAL_ARGS}
+        ${GCOVR_EXCLUDE_ARGS}
+        --object-directory=${PROJECT_BINARY_DIR}
+    )
+
+    if(CODE_COVERAGE_VERBOSE)
+        message(STATUS "Executed command report")
+
+        message(STATUS "Command to run tests: ")
+        string(REPLACE ";" " " GCOVR_EXEC_TESTS_CMD_SPACED "${GCOVR_EXEC_TESTS_CMD}")
+        message(STATUS "${GCOVR_EXEC_TESTS_CMD_SPACED}")
+
+        if(NOT GCOVR_FOLDER_CMD STREQUAL "echo")
+            message(STATUS "Command to create a folder: ")
+            string(REPLACE ";" " " GCOVR_FOLDER_CMD_SPACED "${GCOVR_FOLDER_CMD}")
+            message(STATUS "${GCOVR_FOLDER_CMD_SPACED}")
+        endif()
+
+        message(STATUS "Command to generate gcovr coverage data: ")
+        string(REPLACE ";" " " GCOVR_CMD_SPACED "${GCOVR_CMD}")
+        message(STATUS "${GCOVR_CMD_SPACED}")
+    endif()
+
+    add_custom_target(${Coverage_NAME}
+        COMMAND ${GCOVR_EXEC_TESTS_CMD}
+        COMMAND ${GCOVR_FOLDER_CMD}
+        COMMAND ${GCOVR_CMD}
+
+        BYPRODUCTS ${GCOVR_OUTPUT_FILE}
+        WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
+        DEPENDS ${Coverage_DEPENDENCIES}
+        VERBATIM # Protect arguments to commands
+        COMMENT "Running gcovr to produce code coverage report."
+    )
+
+    # Show info where to find the report
+    add_custom_command(TARGET ${Coverage_NAME} POST_BUILD
+        COMMAND ;
+        COMMENT "Code coverage report saved in ${GCOVR_OUTPUT_FILE} formatted as ${Coverage_FORMAT}"
+    )
+endfunction() # setup_target_for_coverage_gcovr
+
+function(append_coverage_compiler_flags)
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${COVERAGE_COMPILER_FLAGS}" PARENT_SCOPE)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${COVERAGE_COMPILER_FLAGS}" PARENT_SCOPE)
+    set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} ${COVERAGE_COMPILER_FLAGS}" PARENT_SCOPE)
+    message(STATUS "Appending code coverage compiler flags: ${COVERAGE_COMPILER_FLAGS}")
+endfunction() # append_coverage_compiler_flags
+
+# Setup coverage for specific library
+function(append_coverage_compiler_flags_to_target name)
+    separate_arguments(_flag_list NATIVE_COMMAND "${COVERAGE_COMPILER_FLAGS}")
+    target_compile_options(${name} PRIVATE ${_flag_list})
+    if(CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_Fortran_COMPILER_ID STREQUAL "GNU")
+        target_link_libraries(${name} PRIVATE gcov)
+    endif()
+endfunction()

Builds/CMake/RippledCov.cmake

@@ -2,97 +2,37 @@
    coverage report target
 #]===================================================================]
 
-if (coverage)
-  if (is_clang)
-    if (APPLE)
-      execute_process (COMMAND xcrun -f llvm-profdata
-        OUTPUT_VARIABLE LLVM_PROFDATA
-        OUTPUT_STRIP_TRAILING_WHITESPACE)
-    else ()
-      find_program (LLVM_PROFDATA llvm-profdata)
-    endif ()
-    if (NOT LLVM_PROFDATA)
-      message (WARNING "unable to find llvm-profdata - skipping coverage_report target")
-    endif ()
+if(NOT coverage)
+  message(FATAL_ERROR "Code coverage not enabled! Aborting ...")
+endif()
 
-    if (APPLE)
-      execute_process (COMMAND xcrun -f llvm-cov
-        OUTPUT_VARIABLE LLVM_COV
-        OUTPUT_STRIP_TRAILING_WHITESPACE)
-    else ()
-      find_program (LLVM_COV llvm-cov)
-    endif ()
-    if (NOT LLVM_COV)
-      message (WARNING "unable to find llvm-cov - skipping coverage_report target")
-    endif ()
+if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
+  message(WARNING "Code coverage on Windows is not supported, ignoring 'coverage' flag")
+  return()
+endif()
 
-    set (extract_pattern "")
-    if (coverage_core_only)
-      set (extract_pattern "${CMAKE_CURRENT_SOURCE_DIR}/src/ripple/")
-    endif ()
+include(CodeCoverage)
 
-    if (LLVM_COV AND LLVM_PROFDATA)
-      add_custom_target (coverage_report
-        USES_TERMINAL
-        COMMAND ${CMAKE_COMMAND} -E echo "Generating coverage - results will be in ${CMAKE_BINARY_DIR}/coverage/index.html."
-        COMMAND ${CMAKE_COMMAND} -E echo "Running rippled tests."
-        COMMAND rippled --unittest$<$<BOOL:${coverage_test}>:=${coverage_test}> --quiet --unittest-log
-        COMMAND ${LLVM_PROFDATA}
-          merge -sparse default.profraw -o rip.profdata
-        COMMAND ${CMAKE_COMMAND} -E echo "Summary of coverage:"
-        COMMAND ${LLVM_COV}
-          report -instr-profile=rip.profdata
-          $<TARGET_FILE:rippled> ${extract_pattern}
-        # generate html report
-        COMMAND ${LLVM_COV}
-          show -format=html -output-dir=${CMAKE_BINARY_DIR}/coverage
-          -instr-profile=rip.profdata
-          $<TARGET_FILE:rippled> ${extract_pattern}
-        BYPRODUCTS coverage/index.html)
-    endif ()
-  elseif (is_gcc)
-    find_program (LCOV lcov)
-    if (NOT LCOV)
-      message (WARNING "unable to find lcov - skipping coverage_report target")
-    endif ()
+# The instructions for these commands come from the `CodeCoverage` module,
+# which was copied from https://github.com/bilke/cmake-modules, commit fb7d2a3,
+# then locally changed (see CHANGES: section in `CodeCoverage.cmake`)
 
-    find_program (GENHTML genhtml)
-    if (NOT GENHTML)
-      message (WARNING "unable to find genhtml - skipping coverage_report target")
-    endif ()
+set(GCOVR_ADDITIONAL_ARGS ${coverage_extra_args})
+if(NOT GCOVR_ADDITIONAL_ARGS STREQUAL "")
+  separate_arguments(GCOVR_ADDITIONAL_ARGS)
+endif()
 
-    set (extract_pattern "*")
-    if (coverage_core_only)
-      set (extract_pattern "*/src/ripple/*")
-    endif ()
+list(APPEND GCOVR_ADDITIONAL_ARGS
+  --exclude-throw-branches
+  --exclude-noncode-lines
+  --exclude-unreachable-branches -s
+  -j ${coverage_test_parallelism})
 
-    if (LCOV AND GENHTML)
-      add_custom_target (coverage_report
-        USES_TERMINAL
-        COMMAND ${CMAKE_COMMAND} -E echo "Generating coverage- results will be in ${CMAKE_BINARY_DIR}/coverage/index.html."
-        # create baseline info file
-        COMMAND ${LCOV}
-          --no-external -d "${CMAKE_CURRENT_SOURCE_DIR}" -c -d . -i -o baseline.info
-          | grep -v "ignoring data for external file"
-        # run tests
-        COMMAND ${CMAKE_COMMAND} -E echo "Running rippled tests for coverage report."
-        COMMAND rippled --unittest$<$<BOOL:${coverage_test}>:=${coverage_test}> --quiet --unittest-log
-        # Create test coverage data file
-        COMMAND ${LCOV}
-          --no-external -d "${CMAKE_CURRENT_SOURCE_DIR}" -c -d . -o tests.info
-          | grep -v "ignoring data for external file"
-        # Combine baseline and test coverage data
-        COMMAND ${LCOV}
-          -a baseline.info -a tests.info -o lcov-all.info
-        # extract our files
-        COMMAND ${LCOV}
-          -e lcov-all.info "${extract_pattern}" -o lcov.info
-        COMMAND ${CMAKE_COMMAND} -E echo "Summary of coverage:"
-        COMMAND ${LCOV} --summary lcov.info
-        # generate HTML report
-        COMMAND ${GENHTML}
-          -o ${CMAKE_BINARY_DIR}/coverage lcov.info
-        BYPRODUCTS coverage/index.html)
-    endif ()
-  endif ()
-endif ()
+setup_target_for_coverage_gcovr(
+  NAME coverage
+  FORMAT ${coverage_format}
+  EXECUTABLE rippled
+  EXECUTABLE_ARGS --unittest$<$<BOOL:${coverage_test}>:=${coverage_test}> --unittest-jobs ${coverage_test_parallelism} --quiet --unittest-log
+  EXCLUDE "src/test" "${CMAKE_BINARY_DIR}/proto_gen" "${CMAKE_BINARY_DIR}/proto_gen_grpc"
+  DEPENDENCIES rippled
+)

Builds/CMake/RippledInterface.cmake

@@ -23,15 +23,15 @@ target_compile_options (opts
   INTERFACE
     $<$<AND:$<BOOL:${is_gcc}>,$<COMPILE_LANGUAGE:CXX>>:-Wsuggest-override>
     $<$<BOOL:${perf}>:-fno-omit-frame-pointer>
-    $<$<AND:$<BOOL:${is_gcc}>,$<BOOL:${coverage}>>:-fprofile-arcs -ftest-coverage>
-    $<$<AND:$<BOOL:${is_clang}>,$<BOOL:${coverage}>>:-fprofile-instr-generate -fcoverage-mapping>
+    $<$<AND:$<BOOL:${is_gcc}>,$<BOOL:${coverage}>>:-g --coverage -fprofile-abs-path>
+    $<$<AND:$<BOOL:${is_clang}>,$<BOOL:${coverage}>>:-g --coverage>
     $<$<BOOL:${profile}>:-pg>
     $<$<AND:$<BOOL:${is_gcc}>,$<BOOL:${profile}>>:-p>)
 
 target_link_libraries (opts
   INTERFACE
-    $<$<AND:$<BOOL:${is_gcc}>,$<BOOL:${coverage}>>:-fprofile-arcs -ftest-coverage>
-    $<$<AND:$<BOOL:${is_clang}>,$<BOOL:${coverage}>>:-fprofile-instr-generate -fcoverage-mapping>
+    $<$<AND:$<BOOL:${is_gcc}>,$<BOOL:${coverage}>>:-g --coverage -fprofile-abs-path>
+    $<$<AND:$<BOOL:${is_clang}>,$<BOOL:${coverage}>>:-g --coverage>
     $<$<BOOL:${profile}>:-pg>
     $<$<AND:$<BOOL:${is_gcc}>,$<BOOL:${profile}>>:-p>)
 

Builds/CMake/RippledSanity.cmake

@@ -2,6 +2,8 @@
    convenience variables and sanity checks
 #]===================================================================]
 
+include(ProcessorCount)
+
 if (NOT ep_procs)
   ProcessorCount(ep_procs)
   if (ep_procs GREATER 1)

Builds/CMake/RippledSettings.cmake

@@ -2,121 +2,129 @@
    declare user options/settings
 #]===================================================================]
 
-option (assert "Enables asserts, even in release builds" OFF)
+include(ProcessorCount)
 
-option (reporting "Build rippled with reporting mode enabled" OFF)
+ProcessorCount(PROCESSOR_COUNT)
 
-option (tests "Build tests" ON)
+option(assert "Enables asserts, even in release builds" OFF)
 
-option (unity "Creates a build using UNITY support in cmake. This is the default" ON)
-if (unity)
-  if (NOT is_ci)
-    set (CMAKE_UNITY_BUILD_BATCH_SIZE 15 CACHE STRING "")
-  endif ()
-endif ()
-if (is_gcc OR is_clang)
-  option (coverage "Generates coverage info." OFF)
-  option (profile "Add profiling flags" OFF)
-  set (coverage_test "" CACHE STRING
+option(reporting "Build rippled with reporting mode enabled" OFF)
+
+option(tests "Build tests" ON)
+
+option(unity "Creates a build using UNITY support in cmake. This is the default" ON)
+if(unity)
+  if(NOT is_ci)
+    set(CMAKE_UNITY_BUILD_BATCH_SIZE 15 CACHE STRING "")
+  endif()
+endif()
+if(is_gcc OR is_clang)
+  option(coverage "Generates coverage info." OFF)
+  option(profile "Add profiling flags" OFF)
+  set(coverage_test_parallelism "${PROCESSOR_COUNT}" CACHE STRING
+    "Unit tests parallelism for the purpose of coverage report.")
+  set(coverage_format "html-details" CACHE STRING
+    "Output format of the coverage report.")
+  set(coverage_extra_args "" CACHE STRING
+    "Additional arguments to pass to gcovr.")
+  set(coverage_test "" CACHE STRING
     "On gcc & clang, the specific unit test(s) to run for coverage. Default is all tests.")
-  if (coverage_test AND NOT coverage)
-    set (coverage ON CACHE BOOL "gcc/clang only" FORCE)
-  endif ()
-  option (coverage_core_only
-    "Include only src/ripple files when generating coverage report. \
-    Set to OFF to include all sources in coverage report."
-    ON)
-  option (wextra "compile with extra gcc/clang warnings enabled" ON)
-else ()
-  set (profile OFF CACHE BOOL "gcc/clang only" FORCE)
-  set (coverage OFF CACHE BOOL "gcc/clang only" FORCE)
-  set (wextra OFF CACHE BOOL "gcc/clang only" FORCE)
-endif ()
-if (is_linux)
-  option (BUILD_SHARED_LIBS "build shared ripple libraries" OFF)
-  option (static "link protobuf, openssl, libc++, and boost statically" ON)
-  option (perf "Enables flags that assist with perf recording" OFF)
-  option (use_gold "enables detection of gold (binutils) linker" ON)
-else ()
+  if(coverage_test AND NOT coverage)
+    set(coverage ON CACHE BOOL "gcc/clang only" FORCE)
+  endif()
+  option(wextra "compile with extra gcc/clang warnings enabled" ON)
+else()
+  set(profile OFF CACHE BOOL "gcc/clang only" FORCE)
+  set(coverage OFF CACHE BOOL "gcc/clang only" FORCE)
+  set(wextra OFF CACHE BOOL "gcc/clang only" FORCE)
+endif()
+if(is_linux)
+  option(BUILD_SHARED_LIBS "build shared ripple libraries" OFF)
+  option(static "link protobuf, openssl, libc++, and boost statically" ON)
+  option(perf "Enables flags that assist with perf recording" OFF)
+  option(use_gold "enables detection of gold (binutils) linker" ON)
+  option(use_mold "enables detection of mold (binutils) linker" ON)
+else()
   # we are not ready to allow shared-libs on windows because it would require
   # export declarations. On macos it's more feasible, but static openssl
   # produces odd linker errors, thus we disable shared lib builds for now.
-  set (BUILD_SHARED_LIBS OFF CACHE BOOL "build shared ripple libraries - OFF for win/macos" FORCE)
-  set (static ON CACHE BOOL "static link, linux only. ON for WIN/macos" FORCE)
-  set (perf OFF CACHE BOOL "perf flags, linux only" FORCE)
-  set (use_gold OFF CACHE BOOL "gold linker, linux only" FORCE)
-endif ()
-if (is_clang)
-  option (use_lld "enables detection of lld linker" ON)
-else ()
-  set (use_lld OFF CACHE BOOL "try lld linker, clang only" FORCE)
-endif ()
-option (jemalloc "Enables jemalloc for heap profiling" OFF)
-option (werr "treat warnings as errors" OFF)
-option (local_protobuf
+  set(BUILD_SHARED_LIBS OFF CACHE BOOL "build shared ripple libraries - OFF for win/macos" FORCE)
+  set(static ON CACHE BOOL "static link, linux only. ON for WIN/macos" FORCE)
+  set(perf OFF CACHE BOOL "perf flags, linux only" FORCE)
+  set(use_gold OFF CACHE BOOL "gold linker, linux only" FORCE)
+  set(use_mold OFF CACHE BOOL "mold linker, linux only" FORCE)
+endif()
+if(is_clang)
+  option(use_lld "enables detection of lld linker" ON)
+else()
+  set(use_lld OFF CACHE BOOL "try lld linker, clang only" FORCE)
+endif()
+option(jemalloc "Enables jemalloc for heap profiling" OFF)
+option(werr "treat warnings as errors" OFF)
+option(local_protobuf
   "Force a local build of protobuf instead of looking for an installed version." OFF)
-option (local_grpc
+option(local_grpc
   "Force a local build of gRPC instead of looking for an installed version." OFF)
 
 # this one is a string and therefore can't be an option
-set (san "" CACHE STRING "On gcc & clang, add sanitizer instrumentation")
-set_property (CACHE san PROPERTY STRINGS ";undefined;memory;address;thread")
-if (san)
-  string (TOLOWER ${san} san)
-  set (SAN_FLAG "-fsanitize=${san}")
-  set (SAN_LIB "")
-  if (is_gcc)
-    if (san STREQUAL "address")
-      set (SAN_LIB "asan")
-    elseif (san STREQUAL "thread")
-      set (SAN_LIB "tsan")
-    elseif (san STREQUAL "memory")
-      set (SAN_LIB "msan")
-    elseif (san STREQUAL "undefined")
-      set (SAN_LIB "ubsan")
-    endif ()
-  endif ()
-  set (_saved_CRL ${CMAKE_REQUIRED_LIBRARIES})
-  set (CMAKE_REQUIRED_LIBRARIES "${SAN_FLAG};${SAN_LIB}")
-  check_cxx_compiler_flag (${SAN_FLAG} COMPILER_SUPPORTS_SAN)
-  set (CMAKE_REQUIRED_LIBRARIES ${_saved_CRL})
-  if (NOT COMPILER_SUPPORTS_SAN)
-    message (FATAL_ERROR "${san} sanitizer does not seem to be supported by your compiler")
-  endif ()
-endif ()
-set (container_label "" CACHE STRING "tag to use for package building containers")
-option (packages_only
+set(san "" CACHE STRING "On gcc & clang, add sanitizer instrumentation")
+set_property(CACHE san PROPERTY STRINGS ";undefined;memory;address;thread")
+if(san)
+  string(TOLOWER ${san} san)
+  set(SAN_FLAG "-fsanitize=${san}")
+  set(SAN_LIB "")
+  if(is_gcc)
+    if(san STREQUAL "address")
+      set(SAN_LIB "asan")
+    elseif(san STREQUAL "thread")
+      set(SAN_LIB "tsan")
+    elseif(san STREQUAL "memory")
+      set(SAN_LIB "msan")
+    elseif(san STREQUAL "undefined")
+      set(SAN_LIB "ubsan")
+    endif()
+  endif()
+  set(_saved_CRL ${CMAKE_REQUIRED_LIBRARIES})
+  set(CMAKE_REQUIRED_LIBRARIES "${SAN_FLAG};${SAN_LIB}")
+  check_cxx_compiler_flag(${SAN_FLAG} COMPILER_SUPPORTS_SAN)
+  set(CMAKE_REQUIRED_LIBRARIES ${_saved_CRL})
+  if(NOT COMPILER_SUPPORTS_SAN)
+    message(FATAL_ERROR "${san} sanitizer does not seem to be supported by your compiler")
+  endif()
+endif()
+set(container_label "" CACHE STRING "tag to use for package building containers")
+option(packages_only
   "ONLY generate package building targets. This is special use-case and almost \
    certainly not what you want. Use with caution as you won't be able to build \
    any compiled targets locally." OFF)
-option (have_package_container
+option(have_package_container
   "Sometimes you already have the tagged container you want to use for package \
    building and you don't want docker to rebuild it. This flag will detach the \
    dependency of the package build from the container build. It's an advanced \
    use case and most likely you should not be touching this flag." OFF)
 
 # the remaining options are obscure and rarely used
-option (beast_no_unit_test_inline
+option(beast_no_unit_test_inline
   "Prevents unit test definitions from being inserted into global table"
   OFF)
-option (single_io_service_thread
+option(single_io_service_thread
   "Restricts the number of threads calling io_service::run to one. \
   This can be useful when debugging."
   OFF)
-option (boost_show_deprecated
+option(boost_show_deprecated
   "Allow boost to fail on deprecated usage. Only useful if you're trying\
   to find deprecated calls."
   OFF)
-option (beast_hashers
+option(beast_hashers
   "Use local implementations for sha/ripemd hashes (experimental, not recommended)"
   OFF)
 
-if (WIN32)
-  option (beast_disable_autolink "Disables autolinking of system libraries on WIN32" OFF)
-else ()
-  set (beast_disable_autolink OFF CACHE BOOL "WIN32 only" FORCE)
-endif ()
-if (coverage)
-  message (STATUS "coverage build requested - forcing Debug build")
-  set (CMAKE_BUILD_TYPE Debug CACHE STRING "build type" FORCE)
-endif ()
+if(WIN32)
+  option(beast_disable_autolink "Disables autolinking of system libraries on WIN32" OFF)
+else()
+  set(beast_disable_autolink OFF CACHE BOOL "WIN32 only" FORCE)
+endif()
+if(coverage)
+  message(STATUS "coverage build requested - forcing Debug build")
+  set(CMAKE_BUILD_TYPE Debug CACHE STRING "build type" FORCE)
+endif()

CMakeLists.txt

@@ -64,7 +64,6 @@ include (CheckCXXCompilerFlag)
 include (FetchContent)
 include (ExternalProject)
 include (CMakeFuncs) # must come *after* ExternalProject b/c it overrides one function in EP
-include (ProcessorCount)
 if (target)
   message (FATAL_ERROR "The target option has been removed - use native cmake options to control build")
 endif ()
@@ -143,11 +142,14 @@ target_link_libraries(ripple_libs INTERFACE
   SQLite::SQLite3
 )
 
+if(coverage)
+  include(RippledCov)
+endif()
+
 ###
 
 include(RippledCore)
 include(RippledInstall)
-include(RippledCov)
 include(RippledMultiConfig)
 include(RippledDocs)
 include(RippledValidatorKeys)

src/ripple/app/tx/impl/Transactor.cpp

@@ -289,40 +289,8 @@ Transactor::calculateBaseFee(ReadView const& view, STTx const& tx)
 
     // Each signer adds one more baseFee to the minimum required fee
     // for the transaction.
-    std::size_t signerCount = 0;
-    if (tx.isFieldPresent(sfSigners))
-    {
-        // Define recursive lambda to count all leaf signers
-        std::function<std::size_t(STArray const&)> countSigners;
-
-        countSigners = [&](STArray const& signers) -> std::size_t {
-            std::size_t count = 0;
-
-            for (auto const& signer : signers)
-            {
-                if (signer.isFieldPresent(sfSigners))
-                {
-                    // This is a nested signer - recursively count its signers
-                    count += countSigners(signer.getFieldArray(sfSigners));
-                }
-                else
-                {
-                    // This is a leaf signer (one who actually signs)
-                    // Count it only if it has signing fields (not just a
-                    // placeholder)
-                    if (signer.isFieldPresent(sfSigningPubKey) &&
-                        signer.isFieldPresent(sfTxnSignature))
-                    {
-                        count += 1;
-                    }
-                }
-            }
-
-            return count;
-        };
-
-        signerCount = countSigners(tx.getFieldArray(sfSigners));
-    }
+    std::size_t const signerCount =
+        tx.isFieldPresent(sfSigners) ? tx.getFieldArray(sfSigners).size() : 0;
 
     XRPAmount hookExecutionFee{0};
     uint64_t burden{1};
@@ -955,282 +923,157 @@ NotTEC
 Transactor::checkMultiSign(PreclaimContext const& ctx)
 {
     auto const id = ctx.tx.getAccountID(sfAccount);
-
-    // Set max depth based on feature flag
-    bool const allowNested = ctx.view.rules().enabled(featureNestedMultiSign);
-    int const maxDepth = allowNested ? 4 : 1;
-
-    // Define recursive lambda for checking signers at any depth
-    // ancestors tracks the signing chain to detect cycles
-    std::function<NotTEC(
-        AccountID const&, STArray const&, int, std::set<AccountID>)>
-        validateSigners;
-
-    validateSigners = [&](AccountID const& acc,
-                          STArray const& signers,
-                          int depth,
-                          std::set<AccountID> ancestors) -> NotTEC {
-        // Cycle detection: if we're already validating this account up the
-        // chain it cannot contribute - but this isn't an error, just
-        // unavailable weight
-        if (ancestors.count(acc))
-        {
-            JLOG(ctx.j.trace())
-                << "checkMultiSign: Cyclic signer detected: " << acc;
-            return tesSUCCESS;
-        }
-
-        // Check depth limit
-        if (depth > maxDepth)
-        {
-            if (allowNested)
-            {
-                JLOG(ctx.j.trace())
-                    << "checkMultiSign: Multi-signing depth limit exceeded.";
-                return tefBAD_SIGNATURE;
-            }
-
-            JLOG(ctx.j.warn())
-                << "checkMultiSign: Nested multisigning disabled.";
-            return temMALFORMED;
-        }
-
-        ancestors.insert(acc);
-
-        // Get the SignerList for the account we're validating signers for
-        std::shared_ptr<STLedgerEntry const> sleAllowedSigners =
-            ctx.view.read(keylet::signers(acc));
-
-        if (!sleAllowedSigners)
-        {
-            JLOG(ctx.j.trace()) << "checkMultiSign: Account " << acc
-                                << " not set up for multi-signing.";
-            return tefNOT_MULTI_SIGNING;
-        }
-
-        uint32_t const quorum = sleAllowedSigners->getFieldU32(sfSignerQuorum);
-        uint32_t sum{0};
-
-        auto allowedSigners =
-            SignerEntries::deserialize(*sleAllowedSigners, ctx.j, "ledger");
-        if (!allowedSigners)
-            return allowedSigners.error();
-
-        // Build lookup map for O(1) signer validation and weight retrieval
-        std::map<AccountID, uint16_t> signerWeights;
-        uint32_t totalWeight{0}, cyclicWeight{0};
-        for (auto const& entry : *allowedSigners)
-        {
-            signerWeights[entry.account] = entry.weight;
-            totalWeight += entry.weight;
-            if (ancestors.count(entry.account))
-                cyclicWeight += entry.weight;
-        }
-
-        // Walk the signers array, validating each signer
-        // Signers must be in strict ascending order for consensus
-        std::optional<AccountID> prevSigner;
-
-        for (auto const& signerEntry : signers)
-        {
-            AccountID const signer = signerEntry.getAccountID(sfAccount);
-            bool const isNested = signerEntry.isFieldPresent(sfSigners);
-
-            // Enforce strict ascending order (required for consensus)
-            if (prevSigner && signer <= *prevSigner)
-            {
-                JLOG(ctx.j.trace())
-                    << "checkMultiSign: Signers not in strict ascending order: "
-                    << signer << " <= " << *prevSigner;
-                return temMALFORMED;
-            }
-            prevSigner = signer;
-
-            // Skip cyclic signers - they cannot contribute at this level
-            if (ancestors.count(signer))
-            {
-                JLOG(ctx.j.trace())
-                    << "checkMultiSign: Skipping cyclic signer: " << signer;
-                continue;
-            }
-
-            // Lookup signer in authorized set
-            auto const weightIt = signerWeights.find(signer);
-            if (weightIt == signerWeights.end())
-            {
-                JLOG(ctx.j.trace())
-                    << "checkMultiSign: Invalid signer " << signer
-                    << " not in signer list for " << acc;
-                return tefBAD_SIGNATURE;
-            }
-            uint16_t const weight = weightIt->second;
-
-            // Check if this signer has nested signers (delegation)
-            if (isNested)
-            {
-                // This is a nested multi-signer that delegates to sub-signers
-                if (signerEntry.isFieldPresent(sfSigningPubKey) ||
-                    signerEntry.isFieldPresent(sfTxnSignature))
-                {
-                    JLOG(ctx.j.trace()) << "checkMultiSign: Signer " << signer
-                                        << " cannot have both nested signers "
-                                           "and signature fields.";
-                    return tefBAD_SIGNATURE;
-                }
-
-                // Recursively validate the nested signers against signer's
-                // signer list
-                STArray const& nestedSigners =
-                    signerEntry.getFieldArray(sfSigners);
-                NotTEC result = validateSigners(
-                    signer, nestedSigners, depth + 1, ancestors);
-                if (!isTesSuccess(result))
-                    return result;
-
-                // Nested signers met their quorum - add this signer's weight
-                sum += weight;
-
-                JLOG(ctx.j.trace())
-                    << "checkMultiSign: Nested signer " << signer
-                    << " validated, weight=" << weight << ", depth=" << depth
-                    << ", sum=" << sum << "/" << quorum;
-            }
-            else
-            {
-                // This is a leaf signer - validate signature
-                if (!signerEntry.isFieldPresent(sfSigningPubKey) ||
-                    !signerEntry.isFieldPresent(sfTxnSignature))
-                {
-                    JLOG(ctx.j.trace())
-                        << "checkMultiSign: Leaf signer " << signer
-                        << " must have SigningPubKey and TxnSignature.";
-                    return tefBAD_SIGNATURE;
-                }
-
-                auto const spk = signerEntry.getFieldVL(sfSigningPubKey);
-
-                if (!publicKeyType(makeSlice(spk)))
-                {
-                    JLOG(ctx.j.trace())
-                        << "checkMultiSign: Unknown public key type for signer "
-                        << signer;
-                    return tefBAD_SIGNATURE;
-                }
-
-                AccountID const signingAcctIDFromPubKey =
-                    calcAccountID(PublicKey(makeSlice(spk)));
-
-                auto sleTxSignerRoot = ctx.view.read(keylet::account(signer));
-
-                if (signingAcctIDFromPubKey == signer)
-                {
-                    if (sleTxSignerRoot)
-                    {
-                        std::uint32_t const signerAccountFlags =
-                            sleTxSignerRoot->getFieldU32(sfFlags);
-
-                        if (signerAccountFlags & lsfDisableMaster)
-                        {
-                            JLOG(ctx.j.trace())
-                                << "checkMultiSign: Signer " << signer
-                                << " has lsfDisableMaster set.";
-                            return tefMASTER_DISABLED;
-                        }
-                    }
-                }
-                else
-                {
-                    if (!sleTxSignerRoot)
-                    {
-                        JLOG(ctx.j.trace())
-                            << "checkMultiSign: Non-phantom signer " << signer
-                            << " lacks account root.";
-                        return tefBAD_SIGNATURE;
-                    }
-
-                    if (!sleTxSignerRoot->isFieldPresent(sfRegularKey))
-                    {
-                        JLOG(ctx.j.trace()) << "checkMultiSign: Signer "
-                                            << signer << " lacks RegularKey.";
-                        return tefBAD_SIGNATURE;
-                    }
-                    if (signingAcctIDFromPubKey !=
-                        sleTxSignerRoot->getAccountID(sfRegularKey))
-                    {
-                        JLOG(ctx.j.trace())
-                            << "checkMultiSign: Signer " << signer
-                            << " pubkey doesn't match RegularKey.";
-                        return tefBAD_SIGNATURE;
-                    }
-                }
-
-                // Valid leaf signer - add their weight
-                sum += weight;
-
-                JLOG(ctx.j.trace())
-                    << "checkMultiSign: Leaf signer " << signer
-                    << " validated, weight=" << weight << ", depth=" << depth
-                    << ", sum=" << sum << "/" << quorum;
-            }
-        }
-
-        // Calculate effective quorum, relaxing for cyclic lockout scenarios
-        // Sanity check: cyclicWeight must not exceed totalWeight (underflow
-        // guard)
-        if (cyclicWeight > totalWeight)
-        {
-            JLOG(ctx.j.error()) << "checkMultiSign: Invariant violation for "
-                                << acc << ": cyclicWeight (" << cyclicWeight
-                                << ") > totalWeight (" << totalWeight << ")";
-            return tefINTERNAL;
-        }
-
-        uint32_t effectiveQuorum = quorum;
-        uint32_t const maxAchievable = totalWeight - cyclicWeight;
-
-        if (cyclicWeight > 0 && maxAchievable < quorum)
-        {
-            JLOG(ctx.j.warn())
-                << "checkMultiSign: Cyclic lockout detected for " << acc
-                << ": relaxing quorum from " << quorum << " to "
-                << maxAchievable << " (total=" << totalWeight
-                << ", cyclic=" << cyclicWeight << ")";
-            effectiveQuorum = maxAchievable;
-        }
-
-        // Sanity check: effectiveQuorum of 0 means all signers are cyclic -
-        // this is an irrecoverable misconfiguration
-        if (effectiveQuorum == 0)
-        {
-            JLOG(ctx.j.warn()) << "checkMultiSign: All signers for " << acc
-                               << " are cyclic - no valid signing path exists.";
-            return tefBAD_QUORUM;
-        }
-
-        // Check if accumulated weight meets required quorum
-        if (sum < effectiveQuorum)
-        {
-            JLOG(ctx.j.trace()) << "checkMultiSign: Quorum not met for " << acc
-                                << " at depth " << depth << " (sum=" << sum
-                                << ", required=" << effectiveQuorum << ")";
-            return tefBAD_QUORUM;
-        }
-
-        return tesSUCCESS;
-    };
-
-    STArray const& entries(ctx.tx.getFieldArray(sfSigners));
-
-    // Initial call with empty ancestor set - the function inserts acc after
-    // cycle check
-    NotTEC result = validateSigners(id, entries, 1, {});
-    if (!isTesSuccess(result))
+    // Get mTxnAccountID's SignerList and Quorum.
+    std::shared_ptr<STLedgerEntry const> sleAccountSigners =
+        ctx.view.read(keylet::signers(id));
+    // If the signer list doesn't exist the account is not multi-signing.
+    if (!sleAccountSigners)
     {
         JLOG(ctx.j.trace())
-            << "checkMultiSign: Validation failed with " << transToken(result);
-        return result;
+            << "applyTransaction: Invalid: Not a multi-signing account.";
+        return tefNOT_MULTI_SIGNING;
     }
 
+    // We have plans to support multiple SignerLists in the future.  The
+    // presence and defaulted value of the SignerListID field will enable that.
+    assert(sleAccountSigners->isFieldPresent(sfSignerListID));
+    assert(sleAccountSigners->getFieldU32(sfSignerListID) == 0);
+
+    auto accountSigners =
+        SignerEntries::deserialize(*sleAccountSigners, ctx.j, "ledger");
+    if (!accountSigners)
+        return accountSigners.error();
+
+    // Get the array of transaction signers.
+    STArray const& txSigners(ctx.tx.getFieldArray(sfSigners));
+
+    // Walk the accountSigners performing a variety of checks and see if
+    // the quorum is met.
+
+    // Both the multiSigners and accountSigners are sorted by account.  So
+    // matching multi-signers to account signers should be a simple
+    // linear walk.  *All* signers must be valid or the transaction fails.
+    std::uint32_t weightSum = 0;
+    auto iter = accountSigners->begin();
+    for (auto const& txSigner : txSigners)
+    {
+        AccountID const txSignerAcctID = txSigner.getAccountID(sfAccount);
+
+        // Attempt to match the SignerEntry with a Signer;
+        while (iter->account < txSignerAcctID)
+        {
+            if (++iter == accountSigners->end())
+            {
+                JLOG(ctx.j.trace())
+                    << "applyTransaction: Invalid SigningAccount.Account.";
+                return tefBAD_SIGNATURE;
+            }
+        }
+        if (iter->account != txSignerAcctID)
+        {
+            // The SigningAccount is not in the SignerEntries.
+            JLOG(ctx.j.trace())
+                << "applyTransaction: Invalid SigningAccount.Account.";
+            return tefBAD_SIGNATURE;
+        }
+
+        // We found the SigningAccount in the list of valid signers.  Now we
+        // need to compute the accountID that is associated with the signer's
+        // public key.
+        auto const spk = txSigner.getFieldVL(sfSigningPubKey);
+
+        if (!publicKeyType(makeSlice(spk)))
+        {
+            JLOG(ctx.j.trace())
+                << "checkMultiSign: signing public key type is unknown";
+            return tefBAD_SIGNATURE;
+        }
+
+        AccountID const signingAcctIDFromPubKey =
+            calcAccountID(PublicKey(makeSlice(spk)));
+
+        // Verify that the signingAcctID and the signingAcctIDFromPubKey
+        // belong together.  Here is are the rules:
+        //
+        //   1. "Phantom account": an account that is not in the ledger
+        //      A. If signingAcctID == signingAcctIDFromPubKey and the
+        //         signingAcctID is not in the ledger then we have a phantom
+        //         account.
+        //      B. Phantom accounts are always allowed as multi-signers.
+        //
+        //   2. "Master Key"
+        //      A. signingAcctID == signingAcctIDFromPubKey, and signingAcctID
+        //         is in the ledger.
+        //      B. If the signingAcctID in the ledger does not have the
+        //         asfDisableMaster flag set, then the signature is allowed.
+        //
+        //   3. "Regular Key"
+        //      A. signingAcctID != signingAcctIDFromPubKey, and signingAcctID
+        //         is in the ledger.
+        //      B. If signingAcctIDFromPubKey == signingAcctID.RegularKey (from
+        //         ledger) then the signature is allowed.
+        //
+        // No other signatures are allowed.  (January 2015)
+
+        // In any of these cases we need to know whether the account is in
+        // the ledger.  Determine that now.
+        auto sleTxSignerRoot = ctx.view.read(keylet::account(txSignerAcctID));
+
+        if (signingAcctIDFromPubKey == txSignerAcctID)
+        {
+            // Either Phantom or Master.  Phantoms automatically pass.
+            if (sleTxSignerRoot)
+            {
+                // Master Key.  Account may not have asfDisableMaster set.
+                std::uint32_t const signerAccountFlags =
+                    sleTxSignerRoot->getFieldU32(sfFlags);
+
+                if (signerAccountFlags & lsfDisableMaster)
+                {
+                    JLOG(ctx.j.trace())
+                        << "applyTransaction: Signer:Account lsfDisableMaster.";
+                    return tefMASTER_DISABLED;
+                }
+            }
+        }
+        else
+        {
+            // May be a Regular Key.  Let's find out.
+            // Public key must hash to the account's regular key.
+            if (!sleTxSignerRoot)
+            {
+                JLOG(ctx.j.trace()) << "applyTransaction: Non-phantom signer "
+                                       "lacks account root.";
+                return tefBAD_SIGNATURE;
+            }
+
+            if (!sleTxSignerRoot->isFieldPresent(sfRegularKey))
+            {
+                JLOG(ctx.j.trace())
+                    << "applyTransaction: Account lacks RegularKey.";
+                return tefBAD_SIGNATURE;
+            }
+            if (signingAcctIDFromPubKey !=
+                sleTxSignerRoot->getAccountID(sfRegularKey))
+            {
+                JLOG(ctx.j.trace())
+                    << "applyTransaction: Account doesn't match RegularKey.";
+                return tefBAD_SIGNATURE;
+            }
+        }
+        // The signer is legitimate.  Add their weight toward the quorum.
+        weightSum += iter->weight;
+    }
+
+    // Cannot perform transaction if quorum is not met.
+    if (weightSum < sleAccountSigners->getFieldU32(sfSignerQuorum))
+    {
+        JLOG(ctx.j.trace())
+            << "applyTransaction: Signers failed to meet quorum.";
+        return tefBAD_QUORUM;
+    }
+
+    // Met the quorum.  Continue.
     return tesSUCCESS;
 }
 

src/ripple/protocol/Feature.h

@@ -74,7 +74,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 91;
+static constexpr std::size_t numFeatures = 90;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -378,7 +378,6 @@ extern uint256 const fixInvalidTxFlags;
 extern uint256 const featureExtendedHookState;
 extern uint256 const fixCronStacking;
 extern uint256 const fixHookAPI20251128;
-extern uint256 const featureNestedMultiSign;
 }  // namespace ripple
 
 #endif

src/ripple/protocol/impl/Feature.cpp

@@ -484,7 +484,6 @@ REGISTER_FIX    (fixInvalidTxFlags,             Supported::yes, VoteBehavior::De
 REGISTER_FEATURE(ExtendedHookState,             Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixCronStacking,               Supported::yes, VoteBehavior::DefaultYes);
 REGISTER_FIX    (fixHookAPI20251128,            Supported::yes, VoteBehavior::DefaultYes);
-REGISTER_FEATURE(NestedMultiSign,               Supported::yes, VoteBehavior::DefaultNo);
 
 // The following amendments are obsolete, but must remain supported
 // because they could potentially get enabled.

src/ripple/protocol/impl/InnerObjectFormats.cpp

@@ -44,9 +44,8 @@ InnerObjectFormats::InnerObjectFormats()
         sfSigner.getCode(),
         {
             {sfAccount, soeREQUIRED},
-            {sfSigningPubKey, soeOPTIONAL},
-            {sfTxnSignature, soeOPTIONAL},
-            {sfSigners, soeOPTIONAL},
+            {sfSigningPubKey, soeREQUIRED},
+            {sfTxnSignature, soeREQUIRED},
         });
 
     add(sfMajority.jsonName.c_str(),

src/ripple/protocol/impl/STTx.cpp

@@ -369,124 +369,64 @@ STTx::checkMultiSign(
     bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) ||
         (requireCanonicalSig == RequireFullyCanonicalSig::yes);
 
+    // Signers must be in sorted order by AccountID.
+    AccountID lastAccountID(beast::zero);
+
     bool const isWildcardNetwork =
         isFieldPresent(sfNetworkID) && getFieldU32(sfNetworkID) == 65535;
 
-    // Set max depth based on feature flag
-    int const maxDepth = rules.enabled(featureNestedMultiSign) ? 4 : 1;
+    for (auto const& signer : signers)
+    {
+        auto const accountID = signer.getAccountID(sfAccount);
 
-    // Define recursive lambda for checking signatures at any depth
-    std::function<Expected<void, std::string>(
-        STArray const&, AccountID const&, int)>
-        checkSignersArray;
+        // The account owner may not multisign for themselves.
+        if (accountID == txnAccountID)
+            return Unexpected("Invalid multisigner.");
 
-    checkSignersArray = [&](STArray const& signersArray,
-                            AccountID const& parentAccountID,
-                            int depth) -> Expected<void, std::string> {
-        // Check depth limit
-        if (depth > maxDepth)
-            return Unexpected("Multi-signing depth limit exceeded.");
+        // No duplicate signers allowed.
+        if (lastAccountID == accountID)
+            return Unexpected("Duplicate Signers not allowed.");
 
-        // There are well known bounds that the number of signers must be
-        // within.
-        if (signersArray.size() < minMultiSigners ||
-            signersArray.size() > maxMultiSigners(&rules))
-            return Unexpected("Invalid Signers array size.");
+        // Accounts must be in order by account ID.  No duplicates allowed.
+        if (lastAccountID > accountID)
+            return Unexpected("Unsorted Signers array.");
 
-        // Signers must be in sorted order by AccountID.
-        AccountID lastAccountID(beast::zero);
+        // The next signature must be greater than this one.
+        lastAccountID = accountID;
 
-        for (auto const& signer : signersArray)
+        // Verify the signature.
+        bool validSig = false;
+        try
         {
-            auto const accountID = signer.getAccountID(sfAccount);
+            Serializer s = dataStart;
+            finishMultiSigningData(accountID, s);
 
-            // The account owner may not multisign for themselves.
-            if (accountID == txnAccountID)
-                return Unexpected("Invalid multisigner.");
+            auto spk = signer.getFieldVL(sfSigningPubKey);
 
-            // No duplicate signers allowed.
-            if (lastAccountID == accountID)
-                return Unexpected("Duplicate Signers not allowed.");
-
-            // Accounts must be in order by account ID.  No duplicates allowed.
-            if (lastAccountID > accountID)
-                return Unexpected("Unsorted Signers array.");
-
-            // The next signature must be greater than this one.
-            lastAccountID = accountID;
-
-            // Check if this signer has nested signers
-            if (signer.isFieldPresent(sfSigners))
+            if (publicKeyType(makeSlice(spk)))
             {
-                // This is a nested multi-signer
-                if (maxDepth == 1)
-                {
-                    // amendment is not enabled, this is an error
-                    return Unexpected("FeatureNestedMultiSign is disabled");
-                }
+                Blob const signature = signer.getFieldVL(sfTxnSignature);
 
-                // Ensure it doesn't also have signature fields
-                if (signer.isFieldPresent(sfSigningPubKey) ||
-                    signer.isFieldPresent(sfTxnSignature))
-                    return Unexpected(
-                        "Signer cannot have both nested signers and signature "
-                        "fields.");
-
-                // Recursively check nested signers
-                STArray const& nestedSigners = signer.getFieldArray(sfSigners);
-                auto result =
-                    checkSignersArray(nestedSigners, accountID, depth + 1);
-                if (!result)
-                    return result;
-            }
-            else
-            {
-                // This is a leaf node - must have signature
-                if (!signer.isFieldPresent(sfSigningPubKey) ||
-                    !signer.isFieldPresent(sfTxnSignature))
-                    return Unexpected(
-                        "Leaf signer must have SigningPubKey and "
-                        "TxnSignature.");
-
-                // Verify the signature
-                bool validSig = false;
-                try
-                {
-                    Serializer s = dataStart;
-                    finishMultiSigningData(accountID, s);
-
-                    auto spk = signer.getFieldVL(sfSigningPubKey);
-
-                    if (publicKeyType(makeSlice(spk)))
-                    {
-                        Blob const signature =
-                            signer.getFieldVL(sfTxnSignature);
-
-                        // wildcard network gets a free pass
-                        validSig = isWildcardNetwork ||
-                            verify(PublicKey(makeSlice(spk)),
-                                   s.slice(),
-                                   makeSlice(signature),
-                                   fullyCanonical);
-                    }
-                }
-                catch (std::exception const&)
-                {
-                    // We assume any problem lies with the signature.
-                    validSig = false;
-                }
-                if (!validSig)
-                    return Unexpected(
-                        std::string("Invalid signature on account ") +
-                        toBase58(accountID) + ".");
+                // wildcard network gets a free pass
+                validSig = isWildcardNetwork ||
+                    verify(PublicKey(makeSlice(spk)),
+                           s.slice(),
+                           makeSlice(signature),
+                           fullyCanonical);
             }
         }
-
-        return {};
-    };
-
-    // Start the recursive check at depth 1
-    return checkSignersArray(signers, txnAccountID, 1);
+        catch (std::exception const&)
+        {
+            // We assume any problem lies with the signature.
+            validSig = false;
+        }
+        if (!validSig)
+            return Unexpected(
+                std::string("Invalid signature on account ") +
+                toBase58(accountID) + ".");
+    }
+    // All signatures verified.
+    return {};
 }
 
 //------------------------------------------------------------------------------

src/ripple/rpc/impl/TransactionSign.cpp

@@ -1183,21 +1183,12 @@ transactionSubmitMultiSigned(
     // The Signers array may only contain Signer objects.
     if (std::find_if_not(
             signers.begin(), signers.end(), [](STObject const& obj) {
-                if (obj.getCount() != 4 || !obj.isFieldPresent(sfAccount))
-                    return false;
-                // leaf signer
-                if (obj.isFieldPresent(sfSigningPubKey) &&
-                    obj.isFieldPresent(sfTxnSignature) &&
-                    !obj.isFieldPresent(sfSigners))
-                    return true;
-
-                // nested signer
-                if (!obj.isFieldPresent(sfSigningPubKey) &&
-                    !obj.isFieldPresent(sfTxnSignature) &&
-                    obj.isFieldPresent(sfSigners))
-                    return true;
-
-                return false;
+                return (
+                    // A Signer object always contains these fields and no
+                    // others.
+                    obj.isFieldPresent(sfAccount) &&
+                    obj.isFieldPresent(sfSigningPubKey) &&
+                    obj.isFieldPresent(sfTxnSignature) && obj.getCount() == 3);
             }) != signers.end())
     {
         return RPC::make_param_error(

src/test/app/MultiSign_test.cpp

@@ -1659,800 +1659,6 @@ public:
         BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
     }
 
-    void
-    test_nestedMultiSign(FeatureBitset features)
-    {
-        testcase("Nested MultiSign");
-
-#define STRINGIFY(x) #x
-#define TOSTRING(x) STRINGIFY(x)
-
-#define LINE_TO_HEX_STRING                                                \
-    []() -> std::string {                                                 \
-        const char* line = TOSTRING(__LINE__);                            \
-        int len = 0;                                                      \
-        while (line[len])                                                 \
-            len++;                                                        \
-        std::string result;                                               \
-        if (len % 2 == 1)                                                 \
-        {                                                                 \
-            result += (char)(0x00 * 16 + (line[0] - '0'));                \
-            line++;                                                       \
-        }                                                                 \
-        for (int i = 0; line[i]; i += 2)                                  \
-        {                                                                 \
-            result += (char)((line[i] - '0') * 16 + (line[i + 1] - '0')); \
-        }                                                                 \
-        return result;                                                    \
-    }()
-
-#define M(m) memo(m, "", "")
-#define L() memo(LINE_TO_HEX_STRING, "", "")
-
-        using namespace jtx;
-        Env env{*this, envconfig(), features};
-        // Env env{*this, envconfig(), features, nullptr,
-        // beast::severities::kTrace};
-
-        Account const alice{"alice", KeyType::secp256k1};
-        Account const becky{"becky", KeyType::ed25519};
-        Account const cheri{"cheri", KeyType::secp256k1};
-        Account const daria{"daria", KeyType::ed25519};
-        Account const edgar{"edgar", KeyType::secp256k1};
-        Account const fiona{"fiona", KeyType::ed25519};
-        Account const grace{"grace", KeyType::secp256k1};
-        Account const henry{"henry", KeyType::ed25519};
-        Account const f1{"f1", KeyType::ed25519};
-        Account const f2{"f2", KeyType::ed25519};
-        Account const f3{"f3", KeyType::ed25519};
-        env.fund(
-            XRP(1000),
-            alice,
-            becky,
-            cheri,
-            daria,
-            edgar,
-            fiona,
-            grace,
-            henry,
-            f1,
-            f2,
-            f3,
-            phase,
-            jinni,
-            acc10,
-            acc11,
-            acc12);
-        env.close();
-
-        auto const baseFee = env.current()->fees().base;
-
-        if (!features[featureNestedMultiSign])
-        {
-            // When feature is disabled, nested signing should fail
-            env(signers(f1, 1, {{f2, 1}}));
-            env(signers(f2, 1, {{f3, 1}}));
-            env.close();
-
-            std::uint32_t f1Seq = env.seq(f1);
-            env(noop(f1),
-                msig({msigner(f2, msigner(f3))}),
-                L(),
-                fee(3 * baseFee),
-                ter(temINVALID));
-            env.close();
-            BEAST_EXPECT(env.seq(f1) == f1Seq);
-            return;
-        }
-
-        // Test Case 1: Basic 2-level nested signing with quorum
-        {
-            // Set up signer lists with quorum requirements
-            env(signers(becky, 2, {{bogie, 1}, {demon, 1}, {ghost, 1}}));
-            env(signers(cheri, 3, {{haunt, 2}, {jinni, 2}}));
-            env.close();
-
-            // Alice requires quorum of 3 with weighted signers
-            env(signers(alice, 3, {{becky, 2}, {cheri, 2}, {daria, 1}}));
-            env.close();
-
-            // Test 1a: becky alone (weight 2) doesn't meet alice's quorum
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(becky, msigner(bogie), msigner(demon))}),
-                L(),
-                fee(4 * baseFee),
-                ter(tefBAD_QUORUM));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq);
-
-            // Test 1b: becky (2) + daria (1) meets quorum of 3
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig(
-                    {msigner(becky, msigner(bogie), msigner(demon)),
-                     msigner(daria)}),
-                L(),
-                fee(5 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-
-            // Test 1c: cheri's nested signers must meet her quorum
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig(
-                    {msigner(
-                         becky,
-                         msigner(bogie),
-                         msigner(demon)),  // becky has a satisfied quorum
-                     msigner(cheri, msigner(haunt))}),  // but cheri does not
-                                                        // (needs jinni too)
-                L(),
-                fee(5 * baseFee),
-                ter(tefBAD_QUORUM));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq);
-
-            // Test 1d: cheri with both signers meets her quorum
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig(
-                    {msigner(cheri, msigner(haunt), msigner(jinni)),
-                     msigner(daria)}),
-                L(),
-                fee(5 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-        }
-
-        // Test Case 2: 3-level maximum depth with quorum at each level
-        {
-            // Level 2: phase needs direct signatures (no deeper nesting)
-            env(signers(phase, 2, {{acc10, 1}, {acc11, 1}, {acc12, 1}}));
-
-            // Level 1: jinni needs weighted signatures
-            env(signers(jinni, 3, {{phase, 2}, {shade, 2}, {spook, 1}}));
-
-            // Level 0: edgar needs 2 from weighted signers
-            env(signers(edgar, 2, {{jinni, 1}, {bogie, 1}, {demon, 1}}));
-
-            // Alice now requires edgar with weight 3
-            env(signers(alice, 3, {{edgar, 3}, {fiona, 2}}));
-            env.close();
-
-            // Test 2a: 3-level signing with phase signing directly (not through
-            // nested signers)
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({
-                    msigner(
-                        edgar,
-                        msigner(
-                            jinni,
-                            msigner(phase),  // phase signs directly at level 3
-                            msigner(shade))  // jinni quorum: 2+2 = 4 >= 3 ✓
-                        )                    // edgar quorum: 1+0 = 1 < 2 ✗
-                }),
-                L(),
-                fee(4 * baseFee),
-                ter(tefBAD_QUORUM));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq);
-
-            // Test 2b: Edgar needs to meet his quorum too
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({
-                    msigner(
-                        edgar,
-                        msigner(
-                            jinni,
-                            msigner(phase),  // phase signs directly
-                            msigner(shade)),
-                        msigner(bogie))  // edgar quorum: 1+1 = 2 ✓
-                }),
-                L(),
-                fee(5 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-
-            // Test 2c: Use phase's signers (making it effectively 3-level from
-            // alice)
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(
-                    edgar,
-                    msigner(
-                        jinni,
-                        msigner(phase, msigner(acc10), msigner(acc11)),
-                        msigner(spook)),
-                    msigner(bogie))}),
-                L(),
-                fee(6 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-        }
-
-        // Test Case 3: Mixed levels - some direct, some nested at different
-        // depths (max 3)
-        {
-            // Set up mixed-level signing for alice
-            // grace has direct signers
-            env(signers(grace, 2, {{bogie, 1}, {demon, 1}}));
-
-            // henry has 2-level signers (henry -> becky -> bogie/demon)
-            env(signers(henry, 1, {{becky, 1}, {cheri, 1}}));
-
-            // edgar can be signed for by bogie
-            env(signers(edgar, 1, {{bogie, 1}, {shade, 1}}));
-
-            // Alice has mix of direct and nested signers at different weights
-            env(signers(
-                alice,
-                5,
-                {
-                    {daria, 1},  // direct signer
-                    {edgar, 2},  // has 2-level signers
-                    {fiona, 1},  // direct signer
-                    {grace, 2},  // has direct signers
-                    {henry, 2}   // has 2-level signers
-                }));
-            env.close();
-
-            // Test 3a: Mix of all levels meeting quorum exactly
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({
-                    msigner(daria),                  // weight 1, direct
-                    msigner(edgar, msigner(bogie)),  // weight 2, 2-level
-                    msigner(grace, msigner(bogie), msigner(demon))  // weight 2,
-                                                                    // 2-level
-                }),
-                L(),
-                fee(6 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-
-            // Test 3b: 3-level signing through henry
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig(
-                    {msigner(fiona),  // weight 1, direct
-                     msigner(
-                         grace, msigner(bogie)),  // weight 2, 2-level (partial)
-                     msigner(
-                         henry,  // weight 2, 3-level
-                         msigner(becky, msigner(bogie), msigner(demon)))}),
-                L(),
-                fee(6 * baseFee),
-                ter(tefBAD_QUORUM));  // grace didn't meet quorum
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq);
-
-            // Test 3c: Correct version with all quorums met
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig(
-                    {msigner(fiona),  // weight 1
-                     msigner(
-                         edgar, msigner(bogie), msigner(shade)),  // weight 2
-                     msigner(
-                         henry,  // weight 2
-                         msigner(becky, msigner(bogie), msigner(demon)))}),
-                L(),
-                fee(8 * baseFee));  // Total weight: 1+2+2 = 5 ✓
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-        }
-
-        // Test Case 4: Complex scenario with maximum signers at mixed depths
-        // (max 3)
-        {
-            // Create a signing tree that uses close to maximum signers
-            // and tests weight accumulation across all levels
-
-            // Set up for alice: needs 15 out of possible 20 weight
-            env(signers(
-                alice,
-                15,
-                {
-                    {becky, 3},  // will use 2-level
-                    {cheri, 3},  // will use 2-level
-                    {daria, 3},  // will use direct
-                    {edgar, 3},  // will use 2-level
-                    {fiona, 3},  // will use direct
-                    {grace, 3},  // will use direct
-                    {henry, 2}   // will use 2-level
-                }));
-            env.close();
-
-            // Complex multi-level transaction just meeting quorum
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({
-                    msigner(
-                        becky,  // weight 3, 2-level
-                        msigner(demon),
-                        msigner(ghost)),
-                    msigner(
-                        cheri,  // weight 3, 2-level
-                        msigner(haunt),
-                        msigner(jinni)),
-                    msigner(daria),  // weight 3, direct
-                    msigner(
-                        edgar,  // weight 3, 2-level
-                        msigner(bogie)),
-                    msigner(grace)  // weight 3, direct
-                }),
-                L(),
-                fee(10 * baseFee));  // Total weight: 3+3+3+3+3 = 15 ✓
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-
-            // Test 4b: Test with henry using 3-level depth (maximum)
-            // First set up henry's chain properly
-            env(signers(henry, 1, {{jinni, 1}}));
-            env(signers(jinni, 2, {{acc10, 1}, {acc11, 1}}));
-            env.close();
-
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig(
-                    {msigner(
-                         becky,            // weight 3
-                         msigner(demon)),  // becky quorum not met!
-                     msigner(
-                         cheri,  // weight 3
-                         msigner(haunt),
-                         msigner(jinni)),
-                     msigner(daria),  // weight 3
-                     msigner(
-                         henry,  // weight 2, 3-level depth
-                         msigner(jinni, msigner(acc10), msigner(acc11))),
-                     msigner(
-                         edgar,  // weight 3
-                         msigner(bogie),
-                         msigner(shade))}),
-                L(),
-                fee(10 * baseFee),
-                ter(tefBAD_QUORUM));  // becky's quorum not met
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq);
-        }
-
-        // Test Case 5: Edge case - single signer with maximum nesting (depth 3)
-        {
-            // Alice needs just one signer, but that signer uses depth up to 3
-            env(signers(alice, 1, {{becky, 1}}));
-            env.close();
-
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(becky, msigner(demon), msigner(ghost))}),
-                L(),
-                fee(4 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-
-            // Now with 3-level depth (maximum allowed)
-            // Structure: alice -> becky -> cheri -> jinni (jinni signs
-            // directly)
-            env(signers(becky, 1, {{cheri, 1}}));
-            env(signers(cheri, 1, {{jinni, 1}}));
-            // Note: We do NOT add signers to jinni to keep max depth at 3
-            env.close();
-
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(
-                    becky,
-                    msigner(
-                        cheri,
-                        msigner(jinni)))}),  // jinni signs directly (depth 3)
-                L(),
-                fee(4 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-        }
-
-        // Test Case 6: Simple cycle detection (A -> B -> A)
-        {
-            testcase("Cycle Detection - Simple");
-
-            // Reset signer lists for clean state
-            env(signers(alice, jtx::none));
-            env(signers(becky, jtx::none));
-            env.close();
-
-            // becky's signer list includes alice
-            // alice's signer list includes becky
-            // This creates: alice -> becky -> alice (cycle)
-            env(signers(alice, 1, {{becky, 1}, {bogie, 1}}));
-            env(signers(becky, 1, {{alice, 1}, {demon, 1}}));
-            env.close();
-
-            // Without cycle relaxation this would fail because:
-            // - alice needs becky (weight 1)
-            // - becky needs alice, but alice is ancestor -> cycle
-            // - becky's effective quorum relaxes since alice is unavailable
-            // - demon can satisfy becky's relaxed quorum
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(becky, msigner(demon))}),
-                L(),
-                fee(4 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-
-            // Test that direct signer still works normally
-            aliceSeq = env.seq(alice);
-            env(noop(alice), msig({msigner(bogie)}), L(), fee(3 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-        }
-
-        // Test Case 7: The specific lockout scenario
-        // onyx:{jade, nova:{ruby:{jade, nova}, jade}}
-        // All have quorum 2, only jade can actually sign
-        {
-            testcase("Cycle Detection - Complex Lockout");
-
-            Account const onyx{"onyx", KeyType::secp256k1};
-            Account const nova{"nova", KeyType::ed25519};
-            Account const ruby{"ruby", KeyType::secp256k1};
-            Account const jade{"jade", KeyType::ed25519};  // phantom signer
-
-            env.fund(XRP(1000), onyx, nova, ruby);
-            env.close();
-
-            // Set up signer lists FIRST (before disabling master keys)
-            // ruby: {jade, nova} with quorum 2
-            env(signers(ruby, 2, {{jade, 1}, {nova, 1}}));
-            // nova: {ruby, jade} with quorum 2
-            env(signers(nova, 2, {{jade, 1}, {ruby, 1}}));
-            // onyx: {jade, nova} with quorum 2
-            env(signers(onyx, 2, {{jade, 1}, {nova, 1}}));
-            env.close();
-
-            // NOW disable master keys (signer lists provide alternative)
-            env(fset(onyx, asfDisableMaster), sig(onyx));
-            env(fset(nova, asfDisableMaster), sig(nova));
-            env(fset(ruby, asfDisableMaster), sig(ruby));
-            env.close();
-
-            // The signing tree for onyx:
-            // onyx (quorum 2) -> jade (weight 1) + nova (weight 1)
-            //   nova (quorum 2) -> jade (weight 1) + ruby (weight 1)
-            //     ruby (quorum 2) -> jade (weight 1) + nova (weight 1, CYCLE!)
-            //
-            // Without cycle detection: ruby needs nova, but nova is ancestor ->
-            // stuck With cycle detection:
-            //   - At ruby level: nova is cyclic, cyclicWeight=1, totalWeight=2
-            //   - maxAchievable = 2-1 = 1 < quorum(2), so effectiveQuorum -> 1
-            //   - jade alone can satisfy ruby's relaxed quorum
-            //   - ruby satisfied -> nova gets ruby's weight
-            //   - nova: jade(1) + ruby(1) = 2 >= quorum(2) ✓
-            //   - onyx: jade(1) + nova(1) = 2 >= quorum(2) ✓
-
-            std::uint32_t onyxSeq = env.seq(onyx);
-            env(noop(onyx),
-                msig(
-                    {msigner(jade),
-                     msigner(
-                         nova,
-                         msigner(jade),
-                         msigner(
-                             ruby, msigner(jade)))}),  // nova is cyclic,
-                                                       // skipped at ruby level
-                L(),
-                fee(6 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(onyx) == onyxSeq + 1);
-        }
-
-        // Test Case 8: Cycle where all signers are cyclic (effectiveQuorum ==
-        // 0)
-        {
-            testcase("Cycle Detection - Total Lockout");
-
-            Account const alpha{"alpha", KeyType::secp256k1};
-            Account const beta{"beta", KeyType::ed25519};
-            Account const gamma{"gamma", KeyType::secp256k1};
-
-            env.fund(XRP(1000), alpha, beta, gamma);
-            env.close();
-
-            // Set up pure cycle signer lists FIRST
-            env(signers(alpha, 1, {{beta, 1}}));
-            env(signers(beta, 1, {{gamma, 1}}));
-            env(signers(gamma, 1, {{alpha, 1}}));
-            env.close();
-
-            // NOW disable master keys
-            env(fset(alpha, asfDisableMaster), sig(alpha));
-            env(fset(beta, asfDisableMaster), sig(beta));
-            env(fset(gamma, asfDisableMaster), sig(gamma));
-            env.close();
-
-            // This is a true lockout - no valid signing path exists.
-            // gamma appears as a leaf signer but has master disabled ->
-            // tefMASTER_DISABLED (The cycle detection would return
-            // tefBAD_QUORUM if gamma were nested, but there's no way to
-            // construct such a transaction since gamma's only signer is alpha,
-            // which is what we're trying to sign for)
-            std::uint32_t alphaSeq = env.seq(alpha);
-            env(noop(alpha),
-                msig({msigner(
-                    beta,
-                    msigner(gamma))}),  // gamma can't sign - master disabled
-                L(),
-                fee(4 * baseFee),
-                ter(tefMASTER_DISABLED));
-            env.close();
-            BEAST_EXPECT(env.seq(alpha) == alphaSeq);
-        }
-
-        // Test Case 9: Cycle at depth 3 (near max depth)
-        {
-            testcase("Cycle Detection - Deep Cycle");
-
-            // Reset signer lists
-            env(signers(alice, jtx::none));
-            env(signers(becky, jtx::none));
-            env(signers(cheri, jtx::none));
-            env(signers(daria, jtx::none));
-            env.close();
-
-            // Structure: alice -> becky -> cheri -> daria -> alice (cycle at
-            // depth 4)
-            env(signers(alice, 1, {{becky, 1}, {bogie, 1}}));
-            env(signers(becky, 1, {{cheri, 1}}));
-            env(signers(cheri, 1, {{daria, 1}}));
-            env(signers(daria, 1, {{alice, 1}, {demon, 1}}));
-            env.close();
-
-            // At depth 4, daria needs alice but alice is ancestor
-            // daria's quorum relaxes, demon can satisfy
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(
-                    becky, msigner(cheri, msigner(daria, msigner(demon))))}),
-                L(),
-                fee(6 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-        }
-
-        // Test Case 10: Multiple independent cycles in same tree
-        {
-            testcase("Cycle Detection - Multiple Cycles");
-
-            // Reset signer lists
-            env(signers(alice, jtx::none));
-            env(signers(becky, jtx::none));
-            env(signers(cheri, jtx::none));
-            env.close();
-
-            // alice -> {becky, cheri}
-            // becky -> {alice, bogie}  (cycle back to alice)
-            // cheri -> {alice, demon}  (another cycle back to alice)
-            env(signers(alice, 2, {{becky, 1}, {cheri, 1}}));
-            env(signers(becky, 2, {{alice, 1}, {bogie, 1}}));
-            env(signers(cheri, 2, {{alice, 1}, {demon, 1}}));
-            env.close();
-
-            // Both becky and cheri have cycles back to alice
-            // Both need their quorums relaxed
-            // bogie satisfies becky, demon satisfies cheri
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig(
-                    {msigner(becky, msigner(bogie)),
-                     msigner(cheri, msigner(demon))}),
-                L(),
-                fee(6 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-        }
-
-        // Test Case 11: Cycle with sufficient non-cyclic weight (no relaxation
-        // needed)
-        {
-            testcase("Cycle Detection - No Relaxation Needed");
-
-            // Reset signer lists
-            env(signers(alice, jtx::none));
-            env(signers(becky, jtx::none));
-            env.close();
-
-            // becky has alice in signer list but also has enough other signers
-            env(signers(alice, 1, {{becky, 1}}));
-            env(signers(becky, 2, {{alice, 1}, {bogie, 1}, {demon, 1}}));
-            env.close();
-
-            // becky quorum is 2, alice is cyclic (weight 1)
-            // totalWeight = 3, cyclicWeight = 1, maxAchievable = 2 >= quorum
-            // No relaxation needed, bogie + demon satisfy quorum normally
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(becky, msigner(bogie), msigner(demon))}),
-                L(),
-                fee(5 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-
-            // Should fail if only one non-cyclic signer provided
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(becky, msigner(bogie))}),
-                L(),
-                fee(4 * baseFee),
-                ter(tefBAD_QUORUM));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq);
-        }
-
-        // Test Case 12: Partial cycle - one branch cyclic, one not
-        {
-            testcase("Cycle Detection - Partial Cycle");
-
-            // Reset signer lists
-            env(signers(alice, jtx::none));
-            env(signers(becky, jtx::none));
-            env(signers(cheri, jtx::none));
-            env.close();
-
-            // alice -> {becky, cheri}
-            // becky -> {alice, bogie}  (cyclic)
-            // cheri -> {daria}  (not cyclic)
-            env(signers(alice, 2, {{becky, 1}, {cheri, 1}}));
-            env(signers(becky, 1, {{alice, 1}, {bogie, 1}}));
-            env(signers(cheri, 1, {{daria, 1}}));
-            env.close();
-
-            // becky's branch has cycle, cheri's doesn't
-            // Both contribute to alice's quorum
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig(
-                    {msigner(becky, msigner(bogie)),    // relaxed quorum
-                     msigner(cheri, msigner(daria))}),  // normal quorum
-                L(),
-                fee(6 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-        }
-
-        // Test Case 13: Diamond pattern with cycle
-        {
-            testcase("Cycle Detection - Diamond Pattern");
-
-            // Reset signer lists
-            env(signers(alice, jtx::none));
-            env(signers(becky, jtx::none));
-            env(signers(cheri, jtx::none));
-            env(signers(daria, jtx::none));
-            env.close();
-
-            // alice -> {becky, cheri}
-            // becky -> {daria}
-            // cheri -> {daria}
-            // daria -> {alice, bogie}  (cycle through both paths)
-            env(signers(alice, 2, {{becky, 1}, {cheri, 1}}));
-            env(signers(becky, 1, {{daria, 1}}));
-            env(signers(cheri, 1, {{daria, 1}}));
-            env(signers(daria, 1, {{alice, 1}, {bogie, 1}}));
-            env.close();
-
-            // Both paths converge at daria, which cycles back to alice
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig(
-                    {msigner(becky, msigner(daria, msigner(bogie))),
-                     msigner(cheri, msigner(daria, msigner(bogie)))}),
-                L(),
-                fee(7 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-        }
-
-        // Test Case 14: Cycle requiring maximum quorum relaxation
-        {
-            testcase("Cycle Detection - Maximum Relaxation");
-
-            Account const omega{"omega", KeyType::secp256k1};
-            Account const sigma{"sigma", KeyType::ed25519};
-
-            env.fund(XRP(1000), omega, sigma);
-            env.close();
-
-            // Reset alice and becky signer lists
-            env(signers(alice, jtx::none));
-            env(signers(becky, jtx::none));
-            env.close();
-
-            // Set up signer lists FIRST
-            env(signers(sigma, 1, {{omega, 1}, {bogie, 1}}));
-            env(signers(omega, 3, {{sigma, 2}, {alice, 1}, {becky, 1}}));
-            env(signers(alice, 1, {{omega, 1}, {demon, 1}}));
-            env(signers(becky, 1, {{omega, 1}, {ghost, 1}}));
-            env.close();
-
-            // NOW disable master keys
-            env(fset(omega, asfDisableMaster), sig(omega));
-            env(fset(sigma, asfDisableMaster), sig(sigma));
-            env.close();
-
-            // From omega's perspective when signing for omega:
-            // - sigma: needs omega (cyclic), so relaxes to bogie only
-            // - alice: needs omega (cyclic), so relaxes to demon only
-            // - becky: needs omega (cyclic), so relaxes to ghost only
-            // All signers need relaxation but can be satisfied
-            std::uint32_t omegaSeq = env.seq(omega);
-            env(noop(omega),
-                msig(
-                    {msigner(alice, msigner(demon)),
-                     msigner(becky, msigner(ghost)),
-                     msigner(sigma, msigner(bogie))}),
-                L(),
-                fee(7 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(omega) == omegaSeq + 1);
-        }
-
-        // Test Case 15: Cycle at exact max depth boundary
-        {
-            testcase("Cycle Detection - Max Depth Boundary");
-
-            // Reset signer lists
-            env(signers(alice, jtx::none));
-            env(signers(becky, jtx::none));
-            env(signers(cheri, jtx::none));
-            env(signers(daria, jtx::none));
-            env(signers(edgar, jtx::none));
-            env.close();
-
-            // Depth 4 is max: alice(1) -> becky(2) -> cheri(3) -> daria(4)
-            // daria cycles back but we're at max depth
-            env(signers(alice, 1, {{becky, 1}}));
-            env(signers(becky, 1, {{cheri, 1}}));
-            env(signers(cheri, 1, {{daria, 1}}));
-            env(signers(daria, 1, {{alice, 1}, {bogie, 1}}));
-            env.close();
-
-            // This should work - cycle detected and relaxed at depth 4
-            std::uint32_t aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(
-                    becky, msigner(cheri, msigner(daria, msigner(bogie))))}),
-                L(),
-                fee(6 * baseFee));
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-
-            // Now try to exceed depth (add edgar at depth 5)
-            env(signers(daria, 1, {{edgar, 1}}));
-            env(signers(edgar, 1, {{bogie, 1}}));
-            env.close();
-
-            // Transaction structure is rejected at preflight for exceeding
-            // nesting limits
-            aliceSeq = env.seq(alice);
-            env(noop(alice),
-                msig({msigner(
-                    becky,
-                    msigner(
-                        cheri,
-                        msigner(daria, msigner(edgar, msigner(bogie)))))}),
-                L(),
-                fee(7 * baseFee),
-                ter(temMALFORMED));  // Rejected at preflight for excessive
-                                     // nesting
-            env.close();
-            BEAST_EXPECT(env.seq(alice) == aliceSeq);
-        }
-    }
-
     void
     test_signerListSetFlags(FeatureBitset features)
     {
@@ -2503,7 +1709,6 @@ public:
         test_signForHash(features);
         test_signersWithTickets(features);
         test_signersWithTags(features);
-        test_nestedMultiSign(features);
     }
 
     void
@@ -2516,11 +1721,8 @@ public:
         // featureMultiSignReserve.  Limits on the number of signers
         // changes based on featureExpandedSignerList.  Test both with and
         // without.
-        testAll(
-            all - featureMultiSignReserve - featureExpandedSignerList -
-            featureNestedMultiSign);
-        testAll(all - featureExpandedSignerList - featureNestedMultiSign);
-        testAll(all - featureNestedMultiSign);
+        testAll(all - featureMultiSignReserve - featureExpandedSignerList);
+        testAll(all - featureExpandedSignerList);
         testAll(all);
 
         test_signerListSetFlags(all);

src/test/jtx/impl/multisign.cpp

@@ -66,45 +66,15 @@ signers(Account const& account, none_t)
 
 //------------------------------------------------------------------------------
 
-// Helper function to recursively sort nested signers
-void
-sortSignersRecursive(std::vector<msig::SignerPtr>& signers)
+msig::msig(std::vector<msig::Reg> signers_) : signers(std::move(signers_))
 {
-    // Sort current level by account ID
+    // Signatures must be applied in sorted order.
     std::sort(
         signers.begin(),
         signers.end(),
-        [](msig::SignerPtr const& lhs, msig::SignerPtr const& rhs) {
-            return lhs->id() < rhs->id();
+        [](msig::Reg const& lhs, msig::Reg const& rhs) {
+            return lhs.acct.id() < rhs.acct.id();
         });
-
-    // Recursively sort nested signers for each signer at this level
-    for (auto& signer : signers)
-    {
-        if (signer->isNested() && !signer->nested.empty())
-        {
-            sortSignersRecursive(signer->nested);
-        }
-    }
-}
-
-msig::msig(std::vector<msig::SignerPtr> signers_) : signers(std::move(signers_))
-{
-    // Recursively sort all signers at all nesting levels
-    // This ensures account IDs are in strictly ascending order at each level
-    sortSignersRecursive(signers);
-}
-
-msig::msig(std::vector<msig::Reg> signers_)
-{
-    // Convert Reg vector to SignerPtr vector for backward compatibility
-    signers.reserve(signers_.size());
-    for (auto const& s : signers_)
-        signers.push_back(s.toSigner());
-
-    // Recursively sort all signers at all nesting levels
-    // This ensures account IDs are in strictly ascending order at each level
-    sortSignersRecursive(signers);
 }
 
 void
@@ -123,47 +93,19 @@ msig::operator()(Env& env, JTx& jt) const
             env.test.log << pretty(jtx.jv) << std::endl;
             Rethrow();
         }
-
-        // Recursive function to build signer JSON
-        std::function<Json::Value(SignerPtr const&)> buildSignerJson;
-        buildSignerJson = [&](SignerPtr const& signer) -> Json::Value {
-            Json::Value jo;
-            jo[jss::Account] = signer->acct.human();
-
-            if (signer->isNested())
-            {
-                // For nested signers, we use the already-sorted nested vector
-                // (sorted during construction via sortSignersRecursive)
-                // This ensures account IDs are in strictly ascending order
-                auto& subJs = jo[sfSigners.getJsonName()];
-                for (std::size_t i = 0; i < signer->nested.size(); ++i)
-                {
-                    auto& subJo = subJs[i][sfSigner.getJsonName()];
-                    subJo = buildSignerJson(signer->nested[i]);
-                }
-            }
-            else
-            {
-                // This is a leaf signer - add signature
-                jo[jss::SigningPubKey] = strHex(signer->sig.pk().slice());
-
-                Serializer ss{buildMultiSigningData(*st, signer->acct.id())};
-                auto const sig = ripple::sign(
-                    *publicKeyType(signer->sig.pk().slice()),
-                    signer->sig.sk(),
-                    ss.slice());
-                jo[sfTxnSignature.getJsonName()] =
-                    strHex(Slice{sig.data(), sig.size()});
-            }
-
-            return jo;
-        };
-
         auto& js = jtx[sfSigners.getJsonName()];
         for (std::size_t i = 0; i < mySigners.size(); ++i)
         {
+            auto const& e = mySigners[i];
             auto& jo = js[i][sfSigner.getJsonName()];
-            jo = buildSignerJson(mySigners[i]);
+            jo[jss::Account] = e.acct.human();
+            jo[jss::SigningPubKey] = strHex(e.sig.pk().slice());
+
+            Serializer ss{buildMultiSigningData(*st, e.acct.id())};
+            auto const sig = ripple::sign(
+                *publicKeyType(e.sig.pk().slice()), e.sig.sk(), ss.slice());
+            jo[sfTxnSignature.getJsonName()] =
+                strHex(Slice{sig.data(), sig.size()});
         }
     };
 }

src/test/jtx/multisign.h

@@ -21,7 +21,6 @@
 #define RIPPLE_TEST_JTX_MULTISIGN_H_INCLUDED
 
 #include <cstdint>
-#include <memory>
 #include <optional>
 #include <test/jtx/Account.h>
 #include <test/jtx/amount.h>
@@ -66,48 +65,6 @@ signers(Account const& account, none_t);
 class msig
 {
 public:
-    // Recursive signer structure
-    struct Signer
-    {
-        Account acct;
-        Account sig;  // For leaf signers (same as acct for master key)
-        std::vector<std::shared_ptr<Signer>> nested;  // For nested signers
-
-        // Leaf signer constructor (regular signing)
-        Signer(Account const& masterSig) : acct(masterSig), sig(masterSig)
-        {
-        }
-
-        // Leaf signer constructor (with different signing key)
-        Signer(Account const& acct_, Account const& regularSig)
-            : acct(acct_), sig(regularSig)
-        {
-        }
-
-        // Nested signer constructor
-        Signer(
-            Account const& acct_,
-            std::vector<std::shared_ptr<Signer>> nested_)
-            : acct(acct_), nested(std::move(nested_))
-        {
-        }
-
-        bool
-        isNested() const
-        {
-            return !nested.empty();
-        }
-
-        AccountID
-        id() const
-        {
-            return acct.id();
-        }
-    };
-
-    using SignerPtr = std::shared_ptr<Signer>;
-
-    // For backward compatibility
     struct Reg
     {
         Account acct;
@@ -116,13 +73,16 @@ public:
         Reg(Account const& masterSig) : acct(masterSig), sig(masterSig)
         {
         }
+
         Reg(Account const& acct_, Account const& regularSig)
             : acct(acct_), sig(regularSig)
         {
         }
+
         Reg(char const* masterSig) : acct(masterSig), sig(masterSig)
         {
         }
+
         Reg(char const* acct_, char const* regularSig)
             : acct(acct_), sig(regularSig)
         {
@@ -133,32 +93,13 @@ public:
         {
             return acct < rhs.acct;
         }
-
-        // Convert to Signer
-        SignerPtr
-        toSigner() const
-        {
-            return std::make_shared<Signer>(acct, sig);
-        }
     };
 
-    std::vector<SignerPtr> signers;
+    std::vector<Reg> signers;
 
 public:
-    // Initializer list constructor - resolves brace-init ambiguity
-    msig(std::initializer_list<SignerPtr> signers_)
-        : msig(std::vector<SignerPtr>(signers_))
-    {
-        // handled by :
-    }
-
-    // Direct constructor with SignerPtr vector
-    explicit msig(std::vector<SignerPtr> signers_);
-
-    // Backward compatibility constructor
     msig(std::vector<Reg> signers_);
 
-    // Variadic constructor for backward compatibility
     template <class AccountType, class... Accounts>
     explicit msig(AccountType&& a0, Accounts&&... aN)
         : msig{std::vector<Reg>{
@@ -171,30 +112,6 @@ public:
     operator()(Env&, JTx& jt) const;
 };
 
-// Helper functions to create signers - renamed to avoid conflict with sig()
-// transaction modifier
-inline msig::SignerPtr
-msigner(Account const& acct)
-{
-    return std::make_shared<msig::Signer>(acct);
-}
-
-inline msig::SignerPtr
-msigner(Account const& acct, Account const& signingKey)
-{
-    return std::make_shared<msig::Signer>(acct, signingKey);
-}
-
-// Create nested signer with initializer list
-template <typename... Args>
-inline msig::SignerPtr
-msigner(Account const& acct, Args&&... args)
-{
-    std::vector<msig::SignerPtr> nested;
-    (nested.push_back(std::forward<Args>(args)), ...);
-    return std::make_shared<msig::Signer>(acct, std::move(nested));
-}
-
 //------------------------------------------------------------------------------
 
 /** The number of signer lists matches. */

src/test/protocol/STTx_test.cpp

@@ -1757,32 +1757,30 @@ public:
 
         // This lambda contains the bulk of the test code.
         auto testMalformedSigningAccount =
-            [this, &txn](
-                STObject const& signer, bool expectPass) -> bool /* passed */ {
-            // Create SigningAccounts array.
-            STArray signers(sfSigners, 1);
-            signers.push_back(signer);
+            [this, &txn](STObject const& signer, bool expectPass) {
+                // Create SigningAccounts array.
+                STArray signers(sfSigners, 1);
+                signers.push_back(signer);
 
-            // Insert signers into transaction.
-            STTx tempTxn(txn);
-            tempTxn.setFieldArray(sfSigners, signers);
+                // Insert signers into transaction.
+                STTx tempTxn(txn);
+                tempTxn.setFieldArray(sfSigners, signers);
 
-            Serializer rawTxn;
-            tempTxn.add(rawTxn);
-            SerialIter sit(rawTxn.slice());
-            bool serialized = false;
-            try
-            {
-                STTx copy(sit);
-                serialized = true;
-            }
-            catch (std::exception const&)
-            {
-                ;  // If it threw then serialization failed.
-            }
-            BEAST_EXPECT(serialized == expectPass);
-            return serialized == expectPass;
-        };
+                Serializer rawTxn;
+                tempTxn.add(rawTxn);
+                SerialIter sit(rawTxn.slice());
+                bool serialized = false;
+                try
+                {
+                    STTx copy(sit);
+                    serialized = true;
+                }
+                catch (std::exception const&)
+                {
+                    ;  // If it threw then serialization failed.
+                }
+                BEAST_EXPECT(serialized == expectPass);
+            };
 
         {
             // Test case 1.  Make a valid Signer object.
@@ -1792,15 +1790,13 @@ public:
             soTest1.setFieldVL(sfTxnSignature, saMultiSignature);
             testMalformedSigningAccount(soTest1, true);
         }
-
-        /*{ // RHNOTE: featureNestedMultiSign covers this in the
-        checkMultiSign()
+        {
             // Test case 2.  Omit sfSigningPubKey from SigningAccount.
             STObject soTest2(sfSigner);
             soTest2.setAccountID(sfAccount, id2);
             soTest2.setFieldVL(sfTxnSignature, saMultiSignature);
             testMalformedSigningAccount(soTest2, false);
-        }*/
+        }
         {
             // Test case 3.  Extra sfAmount in SigningAccount.
             STObject soTest3(sfSigner);

src/test/unit_test/multi_runner.h

@@ -332,7 +332,6 @@ multi_runner_child::run_multi(Pred pred)
     {
         if (!pred(*t))
             continue;
-
         try
         {
             failed = run(*t) || failed;