docs: Correct the matrix tier the TSan CI config runs in

sanitizers.md said the ubuntu-clang-debug-amd64-tsan config runs TSan in the
full matrix. The config is flagged "extended" in linux.json, so it runs only on
the nightly schedule and on a manual run. Name the tier it is really in, and
say what triggers that tier, because the document explains it nowhere else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

.github/scripts/strategy-matrix/generate.py

@@ -60,6 +60,11 @@ def get_cmake_args(build_type: str, extra_args: str) -> str:
 # Every config must declare 'minimal'. Minimal configs form the reduced matrix
 # built for pull requests by default; the full matrix adds the rest.
 #
+# A Linux config may instead declare 'extended'. Neither the minimal nor the full
+# matrix includes such a config; only the extended matrix does, which the nightly
+# schedule and a manual run ask for. Use it for a config too expensive to run per
+# pull request. It is Linux only because nothing else needs it yet.
+#
 # Configs may also opt into 'benchmark' to smoke-run the benchmarks, or carry a
 # 'package' map to be packaged as well. Note that either applies to every entry
 # a config expands into, so only set them on configs that expand to a single
@@ -94,6 +99,7 @@ class LinuxConfig:
     build_type: list[str]
     arch: list[str]
     minimal: bool
+    extended: bool = False
     benchmark: bool = False  # if true, smoke-run the benchmarks after testing
     sanitizers: list[str] = dataclasses.field(default_factory=list)
     suffix: str = ""
@@ -103,6 +109,13 @@ class LinuxConfig:
     def __post_init__(self) -> None:
         if isinstance(self.package, dict):
             self.package = PackageConfig(**self.package)
+        # The two flags pull in opposite directions: 'minimal' asks for the
+        # smallest matrix, 'extended' for the largest. The filtering would drop
+        # such a config from the minimal matrix, which reads as the opposite of
+        # what it declared, so reject the pair instead.
+        assert not (
+            self.minimal and self.extended
+        ), "a config cannot be both 'minimal' and 'extended'."
 
 
 @dataclasses.dataclass
@@ -215,12 +228,65 @@ _ARCHS: dict[str, Architecture] = {
 }
 
 
-def expand_linux_matrix(linux: LinuxFile, minimal: bool) -> list[MatrixEntry]:
+def expand_linux_config(
+    distro: str, cfg: LinuxConfig, image_tag: str
+) -> list[MatrixEntry]:
+    """Expand one Linux config over the cross-product of its lists.
+
+    Kept apart from the size filtering in expand_linux_matrix so that
+    validate_linux_matrices can ask what a single config expands to without
+    repeating the cross-product.
+
+    @param distro The distro key the config is listed under in linux.json.
+    @param cfg The config to expand.
+    @param image_tag The tag of the nix image the entries build in.
+    @return One entry per (compiler, build type, sanitizer, architecture)
+        combination the config's lists produce.
+    """
+    # An empty sanitizers list means "one entry with no sanitizer".
+    effective_sanitizers = cfg.sanitizers or [""]
+    effective_archs = {arch: _ARCHS[arch] for arch in cfg.arch}
+
+    return [
+        MatrixEntry(
+            config_name=config_name(
+                distro, compiler, build_type, arch, cfg.suffix, sanitizer
+            ),
+            image=f"ghcr.io/xrplf/xrpld/nix-{distro}:{image_tag}",
+            cmake_args=get_cmake_args(build_type, cfg.extra_cmake_args),
+            cmake_target="all",
+            build_only=False,
+            benchmark=cfg.benchmark,
+            build_type=build_type,
+            architecture=arch_info,
+            sanitizers=sanitizer,
+            compiler=compiler,
+        )
+        for compiler, build_type, sanitizer, (arch, arch_info) in itertools.product(
+            cfg.compiler,
+            cfg.build_type,
+            effective_sanitizers,
+            effective_archs.items(),
+        )
+    ]
+
+
+def expand_linux_matrix(
+    linux: LinuxFile, minimal: bool, extended: bool = False
+) -> list[MatrixEntry]:
     """Expand a LinuxFile into a flat list of matrix entries.
 
     Each config entry is expanded over the cross-product of its
     compiler, build_type, sanitizers, and architecture lists. When 'minimal' is
     true, only configs flagged as minimal are included.
+
+    @param linux The parsed linux.json.
+    @param minimal Emit only the configs flagged 'minimal'.
+    @param extended Emit the configs flagged 'extended'. Both the minimal and
+        the full matrix leave those out, so this is the only way to get them.
+    @return One entry per combination the surviving configs expand into.
+    @note Do not set both 'minimal' and 'extended'. The command line rejects the
+        pair, but a direct caller gets the minimal matrix rather than an error.
     """
     entries: list[MatrixEntry] = []
 
@@ -228,37 +294,96 @@ def expand_linux_matrix(linux: LinuxFile, minimal: bool) -> list[MatrixEntry]:
         for cfg in configs:
             if minimal and not cfg.minimal:
                 continue
-            # An empty sanitizers list means "one entry with no sanitizer".
-            effective_sanitizers = cfg.sanitizers or [""]
-            effective_archs = {arch: _ARCHS[arch] for arch in cfg.arch}
-
-            for compiler, build_type, sanitizer, (arch, arch_info) in itertools.product(
-                cfg.compiler,
-                cfg.build_type,
-                effective_sanitizers,
-                effective_archs.items(),
-            ):
-                name = config_name(
-                    distro, compiler, build_type, arch, cfg.suffix, sanitizer
-                )
-                entries.append(
-                    MatrixEntry(
-                        config_name=name,
-                        image=f"ghcr.io/xrplf/xrpld/nix-{distro}:{linux.image_tag}",
-                        cmake_args=get_cmake_args(build_type, cfg.extra_cmake_args),
-                        cmake_target="all",
-                        build_only=False,
-                        benchmark=cfg.benchmark,
-                        build_type=build_type,
-                        architecture=arch_info,
-                        sanitizers=sanitizer,
-                        compiler=compiler,
-                    )
-                )
+            if not extended and cfg.extended:
+                continue
+            entries += expand_linux_config(distro, cfg, linux.image_tag)
 
     return entries
 
 
+def validate_linux_matrices(linux: LinuxFile) -> None:
+    """Check that the three matrix sizes nest, and hold the configs they should.
+
+    CI runs only the jobs this script emits, so a config that falls out of the
+    size it belongs to takes its coverage with it and fails nothing. These checks
+    run on every invocation, so the drop fails matrix generation instead.
+
+    The checks name no config, only the flags, so adding or removing a config
+    needs no edit here.
+
+    @param linux The parsed linux.json.
+    @raise AssertionError If two configs expand to the same name, if the sizes do
+        not nest, or if a config reaches a size it does not belong to.
+    """
+    minimal_names = {e.config_name for e in expand_linux_matrix(linux, minimal=True)}
+    full_names = {e.config_name for e in expand_linux_matrix(linux, minimal=False)}
+    extended_names = {
+        e.config_name for e in expand_linux_matrix(linux, minimal=False, extended=True)
+    }
+
+    # The names each config expands to, paired with the config, so that the
+    # checks below expand every config once.
+    per_config = [
+        (
+            distro,
+            cfg,
+            {e.config_name for e in expand_linux_config(distro, cfg, linux.image_tag)},
+        )
+        for distro, configs in linux.configs.items()
+        for cfg in configs
+    ]
+
+    # A config name is also the name of the artifacts the job uploads, so two
+    # configs that expand to the same name overwrite each other. The per-config
+    # checks below also need a name to belong to one config only.
+    all_names = [n for _, _, names in per_config for n in names]
+    duplicates = sorted({n for n in all_names if all_names.count(n) > 1})
+    assert not duplicates, f"configs expand to duplicate names: {duplicates}."
+
+    # The sizes nest, so a larger one only ever adds. Were 'extended' to replace
+    # the full matrix rather than widen it, the nightly would test less than a
+    # labeled pull request does, which is the opposite of the intent.
+    assert minimal_names <= full_names, (
+        "the minimal matrix is not part of the full one, missing: "
+        f"{sorted(minimal_names - full_names)}."
+    )
+    assert full_names <= extended_names, (
+        "the full matrix is not part of the extended one, missing: "
+        f"{sorted(full_names - extended_names)}."
+    )
+
+    # Every config reaches the sizes its flags ask for, and no others.
+    for distro, cfg, names in per_config:
+        if cfg.extended:
+            assert names <= extended_names, (
+                f"{distro} config flagged 'extended' is missing from the "
+                f"extended matrix: {sorted(names - extended_names)}."
+            )
+            assert not names & full_names, (
+                f"{distro} config flagged 'extended' also reaches the full "
+                f"matrix: {sorted(names & full_names)}."
+            )
+        else:
+            assert names <= full_names, (
+                f"{distro} config is missing from the full matrix: "
+                f"{sorted(names - full_names)}."
+            )
+        if cfg.minimal:
+            assert names <= minimal_names, (
+                f"{distro} config flagged 'minimal' is missing from the "
+                f"minimal matrix: {sorted(names - minimal_names)}."
+            )
+
+    # The 'extended' tier costs a flag here, a condition in
+    # reusable-strategy-matrix.yml, and the check after it. An empty tier leaves
+    # all three as dead weight that still reads as working, so require a holder.
+    # Checked last, because the checks above name the config that went missing.
+    assert extended_names > full_names, (
+        "no config is flagged 'extended', so the extended matrix is the full one. "
+        "Either flag the config that needs the tier, or remove the tier."
+    )
+
+
 def expand_linux_packaging(linux: LinuxFile) -> list[PackagingEntry]:
     """Generate the packaging matrix from the configs that carry a 'package' map.
 
@@ -367,7 +492,12 @@ if __name__ == "__main__":
         help="Emit the Linux packaging matrix instead of the build/test matrix.",
         action="store_true",
     )
-    parser.add_argument(
+    # Each flag picks a matrix size, and the sizes nest: minimal is a subset of
+    # the full matrix, which is a subset of extended. So one flag narrows and the
+    # other widens the same default, and asking for both has no answer. Argparse
+    # rejects the pair rather than letting the filters intersect into a surprise.
+    size = parser.add_mutually_exclusive_group()
+    size.add_argument(
         "-m",
         "--minimal",
         help="Emit only the minimal matrix (the configs flagged 'minimal'), "
@@ -375,21 +505,32 @@ if __name__ == "__main__":
         "emitted.",
         action="store_true",
     )
+    size.add_argument(
+        "-x",
+        "--extended",
+        help="Emit the extended matrix: the full one plus the configs flagged "
+        "'extended', which no other matrix includes. Used for the nightly "
+        "schedule and for a manual run.",
+        action="store_true",
+    )
     args = parser.parse_args()
 
     matrix: list[MatrixEntry] | list[PackagingEntry] = []
 
+    # Checked on every invocation, including the ones that emit another platform
+    # or the packaging matrix, so that no call can pass a broken linux.json.
+    linux = LinuxFile.load(THIS_DIR / "linux.json")
+    validate_linux_matrices(linux)
+
     if args.packaging:
-        matrix = expand_linux_packaging(LinuxFile.load(THIS_DIR / "linux.json"))
+        matrix = expand_linux_packaging(linux)
         # One list per format, so each install-test job installs the packages its
         # own format produced.
         for package_type, names in package_names_by_type(matrix).items():
             print(f"{package_type}_package_names={json.dumps(names)}")
     else:
         if args.config in ("linux", None):
-            matrix += expand_linux_matrix(
-                LinuxFile.load(THIS_DIR / "linux.json"), args.minimal
-            )
+            matrix += expand_linux_matrix(linux, args.minimal, args.extended)
         if args.config in ("macos", None):
             matrix += expand_platform_matrix(
                 PlatformFile.load(THIS_DIR / "macos.json"), args.minimal

.github/scripts/strategy-matrix/linux.json

@@ -38,6 +38,14 @@
         "minimal": false,
         "sanitizers": ["address", "undefinedbehavior"]
       },
+      {
+        "compiler": ["clang"],
+        "build_type": ["Debug"],
+        "arch": ["amd64"],
+        "minimal": false,
+        "extended": true,
+        "sanitizers": ["thread"]
+      },
 
       {
         "compiler": ["clang"],

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

@@ -117,6 +117,12 @@ jobs:
       VOIDSTAR_ENABLED: ${{ contains(inputs.cmake_args, '-Dvoidstar=ON') }}
       VALIDATOR_KEYS_ENABLED: ${{ contains(inputs.cmake_args, '-Dvalidator_keys=ON') }}
       SANITIZERS_ENABLED: ${{ inputs.sanitizers != '' }}
+      # One flag per sanitizer, so that a step needing only one of them does not
+      # have to parse the list. conan/profiles/sanitizers rejects any name that is
+      # not one of these three, so an unrecognized one never reaches a step.
+      ASAN_ENABLED: ${{ contains(inputs.sanitizers, 'address') }}
+      TSAN_ENABLED: ${{ contains(inputs.sanitizers, 'thread') }}
+      UBSAN_ENABLED: ${{ contains(inputs.sanitizers, 'undefinedbehavior') }}
       # The binaries reusable-package.yml consumes. A private repository skips
       # them except on a tag push, which is what produces its release packages.
       PACKAGING_ARTIFACTS_ENABLED: ${{ github.event.repository.visibility == 'public' || startsWith(github.ref, 'refs/tags/') }}
@@ -221,7 +227,12 @@ jobs:
               ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0"
           fi
           echo "ASAN_OPTIONS=${ASAN_OPTS}" >>${GITHUB_ENV}
-          echo "TSAN_OPTIONS=include=${SUPP}/runtime-tsan-options.txt:suppressions=${SUPP}/tsan.supp" >>${GITHUB_ENV}
+          # exitcode=0 is set here rather than in runtime-tsan-options.txt,
+          # because that file is also what the documented local command reads,
+          # and a local run must still exit nonzero on a finding. TSan exits 66
+          # on its own once it has reported anything, which would fail the job.
+          TSAN_OPTS="include=${SUPP}/runtime-tsan-options.txt:suppressions=${SUPP}/tsan.supp"
+          echo "TSAN_OPTIONS=${TSAN_OPTS}:exitcode=0" >>${GITHUB_ENV}
           echo "UBSAN_OPTIONS=include=${SUPP}/runtime-ubsan-options.txt:suppressions=${SUPP}/ubsan.supp" >>${GITHUB_ENV}
           echo "LSAN_OPTIONS=include=${SUPP}/runtime-lsan-options.txt:suppressions=${SUPP}/lsan.supp" >>${GITHUB_ENV}
 
@@ -329,11 +340,102 @@ jobs:
               exit 1
           fi
 
+      # Catch a build configured without the instrumentation it was asked for,
+      # which would otherwise run every test, pass, and report nothing.
+      #
+      # A sanitizer is checked by the symbols the binary carries, because
+      # instrumented code calls into its runtime, so those references are present
+      # whether the runtime is linked statically or as a shared library. That is
+      # what proves the -fsanitize flags reached the compiler. The version string
+      # cannot: cmake sets the SANITIZERS macro separately from those flags, so it
+      # names a sanitizer in a binary carrying none. It is checked as well, on top
+      # rather than instead, because BuildInfo.cpp is what a bug report quotes and
+      # it should agree with the binary.
+      #
+      # Asking each runtime to print its flag list was tried and dropped: GCC's
+      # libubsan ignores UBSAN_OPTIONS=help=1, so it called an instrumented gcc
+      # UBSan build uninstrumented.
+      #
+      # voidstar is a linked library rather than instrumentation, so the version
+      # string is all there is to check for it.
+      #
+      # The version call's exit code is ignored on purpose. ASan leaves leak
+      # reporting on and UBSan sets halt_on_error, either of which can end it
+      # nonzero on a healthy binary. Only the text decides, and it is printed.
       - name: Verify presence of instrumentation (Linux)
-        if: ${{ runner.os == 'Linux' && env.VOIDSTAR_ENABLED == 'true' }}
+        if: ${{ runner.os == 'Linux' && (env.VOIDSTAR_ENABLED == 'true' || env.SANITIZERS_ENABLED == 'true') }}
         working-directory: ${{ env.BUILD_DIR }}
         run: |
-          ./xrpld --version | grep libvoidstar
+          version="$(./xrpld --version 2>&1 || true)"
+          echo "${version}"
+          status=0
+
+          named() {
+              if grep -q "$2" <<<"${version}"; then
+                  echo "${1}: the version string names '${2}'."
+              else
+                  echo "${1} is enabled, but the version string does not name '${2}'."
+                  status=1
+              fi
+          }
+
+          if [ "${SANITIZERS_ENABLED}" = 'true' ]; then
+              # Whichever of these the image ships. Stopping here beats a check
+              # that quietly stops checking.
+              symbols="$(mktemp)"
+              if command -v nm >/dev/null 2>&1; then
+                  nm ./xrpld >"${symbols}" 2>/dev/null || true
+              elif command -v readelf >/dev/null 2>&1; then
+                  readelf -sW ./xrpld >"${symbols}" 2>/dev/null || true
+              elif command -v objdump >/dev/null 2>&1; then
+                  objdump -t ./xrpld >"${symbols}" 2>/dev/null || true
+              else
+                  echo 'Found none of nm, readelf or objdump, so the binary cannot be inspected.'
+                  exit 1
+              fi
+              if [ ! -s "${symbols}" ]; then
+                  echo 'Read no symbols from ./xrpld, so the instrumentation cannot be confirmed.'
+                  exit 1
+              fi
+
+              # Each pattern names the calls a sanitizer plants at an instrumented
+              # operation, rather than the ones that only prove its runtime is
+              # present. __asan_init and __tsan_init appear as soon as the flags
+              # reach the linker, so matching those would pass a build whose
+              # compile step never saw them. __tsan_func_entry is no better: it
+              # survives an ignorelist, while the read and write calls do not.
+              #
+              # How much this buys depends on how the runtime is linked, and both
+              # kinds are in the matrix. GCC links libasan and libubsan
+              # dynamically, so an instrumented call site is the only thing that
+              # can put these symbols in the binary. Clang links the TSan runtime
+              # statically, so the runtime's own definitions are there either way
+              # and this narrows to a check that the sanitizer reached the link.
+              # Nothing cheap distinguishes the two cases, and the flags come from
+              # one cmake variable, so losing only the compile half is unlikely.
+              instrumented() {
+                  if grep -qE "$2" "${symbols}"; then
+                      echo "${1}: the binary calls into its runtime, matching '${2}'."
+                  else
+                      echo "${1} is enabled, but no instrumented call site matches '${2}'."
+                      status=1
+                  fi
+              }
+
+              [ "${ASAN_ENABLED}" = 'true' ] &&
+                  instrumented address '__asan_report|__asan_memcpy|__asan_stack_malloc'
+              [ "${TSAN_ENABLED}" = 'true' ] &&
+                  instrumented thread '__tsan_read|__tsan_write|__tsan_atomic'
+              [ "${UBSAN_ENABLED}" = 'true' ] &&
+                  instrumented undefinedbehavior '__ubsan_handle'
+          fi
+
+          [ "${VOIDSTAR_ENABLED}" = 'true' ] && named voidstar libvoidstar
+          [ "${ASAN_ENABLED}" = 'true' ] && named address ASAN
+          [ "${TSAN_ENABLED}" = 'true' ] && named thread TSAN
+          [ "${UBSAN_ENABLED}" = 'true' ] && named undefinedbehavior UBSAN
+
+          exit "${status}"
 
       - name: Run the separate tests
         if: ${{ !inputs.build_only }}
@@ -350,6 +452,16 @@ jobs:
           # Coverage builds are slower due to instrumentation; use fewer parallel jobs to avoid flakiness
           [ "$COVERAGE_ENABLED" = "true" ] && BUILD_NPROC=$((BUILD_NPROC - 2))
 
+          # TSan keeps shadow memory for every byte a process touches, so each job
+          # costs several times what it does uninstrumented, and too many at once
+          # exhausts the machine. Cap the count rather than subtract from it,
+          # because the ceiling is total memory and does not scale with the core
+          # count. Raise it only against a run, and expect one of two failures:
+          # the OOM killer ends the process with code 137 before any suite
+          # finishes, or, worse, the runner loses contact with the server and the
+          # job stops with no log and no failing step.
+          [ "$TSAN_ENABLED" = "true" ] && BUILD_NPROC=4
+
           # The resolver/preload workaround is only correct for the ASan build:
           # a regular build doesn't hit the __dn_expand interceptor bug, and must
           # NOT have libasan injected. So only preload when xrpld is ASan-built.

.github/workflows/reusable-strategy-matrix.yml

@@ -21,6 +21,19 @@ jobs:
     runs-on: ubuntu-latest
     outputs:
       matrix: ${{ steps.generate.outputs.matrix }}
+    # Set at job level rather than on the step, so that the generate step and the
+    # check after it read the same values and cannot disagree.
+    env:
+      GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}', inputs.os) || '' }}
+      # Run only the minimal matrix for pull requests that are not yet
+      # labeled "Ready to merge" or "Full CI build". Any other event (merge
+      # queue, push, schedule, manual dispatch) runs the full matrix.
+      GENERATE_MINIMAL: ${{ (github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'Ready to merge') && !contains(github.event.pull_request.labels.*.name, 'Full CI build')) && '--minimal' || '' }}
+      # Configs flagged "extended" in the JSON are too expensive to run per
+      # pull request, so only the nightly schedule adds them. A manual run
+      # counts as well, because otherwise the only way to exercise one is to
+      # wait for the night.
+      GENERATE_EXTENDED: ${{ (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') && '--extended' || '' }}
     steps:
       - name: Checkout repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
@@ -33,10 +46,22 @@ jobs:
       - name: Generate strategy matrix
         working-directory: .github/scripts/strategy-matrix
         id: generate
+        run: ./generate.py ${GENERATE_CONFIG} ${GENERATE_MINIMAL} ${GENERATE_EXTENDED} >>"${GITHUB_OUTPUT}"
+
+      # generate.py checks that the extended matrix holds every config flagged
+      # "extended", but it never sees the event, so it cannot tell whether this
+      # run asked for them. A GENERATE_EXTENDED that stops matching the events
+      # would drop those configs and fail nothing, because CI runs only the jobs
+      # the matrix names. So compare what was generated against the extended
+      # matrix, on the events that must produce it.
+      - name: Check the extended configs reached the matrix
+        if: ${{ env.GENERATE_EXTENDED != '' }}
+        working-directory: .github/scripts/strategy-matrix
         env:
-          GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}', inputs.os) || '' }}
-          # Run only the minimal matrix for pull requests that are not yet
-          # labeled "Ready to merge" or "Full CI build". Any other event (merge
-          # queue, push, schedule, manual dispatch) runs the full matrix.
-          GENERATE_MINIMAL: ${{ (github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'Ready to merge') && !contains(github.event.pull_request.labels.*.name, 'Full CI build')) && '--minimal' || '' }}
-        run: ./generate.py ${GENERATE_CONFIG} ${GENERATE_MINIMAL} >>"${GITHUB_OUTPUT}"
+          GENERATED: ${{ steps.generate.outputs.matrix }}
+        run: |
+          expected="$(./generate.py ${GENERATE_CONFIG} --extended)"
+          if [[ "matrix=${GENERATED}" != "${expected}" ]]; then
+              echo "::error::the ${GITHUB_EVENT_NAME} matrix is not the extended one"
+              exit 1
+          fi

.pre-commit-config.yaml

@@ -53,11 +53,6 @@ repos:
         entry: ./bin/pre-commit/check_doxygen_style.py
         language: python
         types_or: [c++, c]
-      - id: fix-gtest-names
-        name: "fix gtest names: CamelCase suite, snake_case test case"
-        entry: ./bin/pre-commit/fix_gtest_names.py
-        language: python
-        types_or: [c++, c]
 
   - repo: https://github.com/pre-commit/mirrors-clang-format
     rev: f4d7745e17a28aad7eed2f4874ca8d1568c11c4c # frozen: v22.1.8

BUILD.md

@@ -273,7 +273,7 @@ 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 -Dxrpld=ON -Dtests=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 -DCMAKE_BUILD_TYPE=Debug -Dcoverage=ON -Dxrpld=ON -Dtests=ON -Dcoverage_format=html-details -Dcoverage_extra_args="--json coverage.json" -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake ..
 cmake --build . --target coverage
 ```
 

bin/pre-commit/fix_gtest_names.py

@@ -1,144 +0,0 @@
-#!/usr/bin/env python3
-
-"""
-Rewrites gtest names to the required style in this project: the suite name is
-CamelCase, the test-case name is snake_case.
-
-    TEST(SuiteName, test_case_name)
-
-The gtest `DISABLED_` prefix is kept verbatim on either name.
-
-Both conversions fold acronyms the way a reader expects:
-`SetAndResetAccountTxnID` -> `set_and_reset_account_txn_id`, not
-`set_and_reset_account_txn_i_d`.
-
-The first argument of `TEST_F`, `TEST_P`, `TYPED_TEST` and `TYPED_TEST_P` is a
-fixture class rather than a free identifier, so rewriting it here would leave
-the class it names behind. Those are reported for a human to rename (clang-tidy
-checks the class declaration itself, via readability-identifier-naming).
-
-Usage: ./bin/pre-commit/fix_gtest_names.py <file1> <file2> ...
-"""
-
-import re
-import sys
-from collections import Counter
-from pathlib import Path
-
-# A test-case definition, `MACRO(SuiteOrFixture, TestName)`, anchored at the
-# start of a line so that commented-out definitions and project macros that
-# merely look similar (`TEST_EXPECT(...)`) are left alone. The `\s*` between
-# arguments allows for a definition clang-format wrapped over several lines.
-PATTERN = re.compile(
-    r"(?P<head>^[ \t]*(?P<macro>TYPED_TEST_P|TYPED_TEST|TEST_F|TEST_P|TEST)\s*\(\s*)"
-    r"(?P<suite>\w+)(?P<mid>\s*,\s*)(?P<name>\w+)(?P<tail>\s*\))",
-    re.MULTILINE,
-)
-
-# The macros whose first argument names a fixture class, not a free identifier.
-FIXTURE_MACROS = ("TEST_F", "TEST_P", "TYPED_TEST", "TYPED_TEST_P")
-
-DISABLED = "DISABLED_"
-
-ACRONYM_BOUNDARY = re.compile(r"([A-Z]+)([A-Z][a-z])")
-WORD_BOUNDARY = re.compile(r"([a-z\d])([A-Z])")
-
-
-def _split_disabled(name: str) -> tuple[str, str]:
-    """Splits off gtest's `DISABLED_` prefix, which is kept verbatim."""
-    if name.startswith(DISABLED):
-        return DISABLED, name[len(DISABLED) :]
-    return "", name
-
-
-def snake_case(name: str) -> str:
-    """Returns the name in snake_case, leaving acronyms whole.
-
-    `SetAndResetAccountTxnID` -> `set_and_reset_account_txn_id`,
-    `parseStatRSSkB` -> `parse_stat_rs_sk_b`.
-    """
-    prefix, core = _split_disabled(name)
-    core = ACRONYM_BOUNDARY.sub(r"\1_\2", core)
-    return prefix + WORD_BOUNDARY.sub(r"\1_\2", core).lower()
-
-
-def camel_case(name: str) -> str:
-    """Returns the name in CamelCase, capitalizing each underscored word.
-
-    Only the letters that have to change are touched, so acronyms survive: a
-    conversion that went via snake_case would turn `SHAMapTest` into
-    `ShaMapTest`, whereas here it is already CamelCase and stays put.
-    `json_value` -> `JsonValue`, `parseStatRSSkB` -> `ParseStatRSSkB`.
-    """
-    prefix, core = _split_disabled(name)
-    return prefix + "".join(w[:1].upper() + w[1:] for w in core.split("_") if w)
-
-
-def _corrected(match: re.Match) -> tuple[str, str]:
-    """Returns the suite and test-case names this definition should end up with."""
-    suite = match["suite"]
-    return (
-        suite if match["macro"] in FIXTURE_MACROS else camel_case(suite),
-        snake_case(match["name"]),
-    )
-
-
-def fix_source(text: str) -> tuple[str, list[str]]:
-    """Returns the corrected text and one `line: message` report per bad name."""
-    # gtest joins the suite and test names into one class name, so two test
-    # cases whose joined names agree cannot coexist: `TEST(a, b_c)` and
-    # `TEST(a_b, c)` both define `a_b_c_Test`. A rename that would introduce
-    # such a clash is reported for a human instead of applied.
-    joined = Counter("_".join(_corrected(m)) for m in PATTERN.finditer(text))
-    reports = []
-
-    def rewrite(match: re.Match) -> str:
-        suite, name = match["suite"], match["name"]
-        new_suite, new_name = _corrected(match)
-        line = text.count("\n", 0, match.start()) + 1
-
-        if match["macro"] in FIXTURE_MACROS and camel_case(suite) != suite:
-            reports.append(
-                f"{line}: fixture '{suite}' is not CamelCase: rename the class "
-                f"to '{camel_case(suite)}' by hand"
-            )
-        if (new_suite, new_name) == (suite, name):
-            return match[0]
-        if joined[f"{new_suite}_{new_name}"] > 1:
-            reports.append(
-                f"{line}: cannot rename '{suite}, {name}' to '{new_suite}, "
-                f"{new_name}': another test case already generates that name"
-            )
-            return match[0]
-        if new_suite != suite:
-            reports.append(f"{line}: renamed suite '{suite}' to '{new_suite}'")
-        if new_name != name:
-            reports.append(f"{line}: renamed test case '{name}' to '{new_name}'")
-        return match["head"] + new_suite + match["mid"] + new_name + match["tail"]
-
-    return PATTERN.sub(rewrite, text), reports
-
-
-def fix_names(path: Path) -> bool:
-    """Corrects one file's gtest names, reporting each on stdout."""
-    original = path.read_text(encoding="utf-8")
-    fixed, reports = fix_source(original)
-    for report in reports:
-        print(f"{path}:{report}")
-    if fixed != original:
-        path.write_text(fixed, encoding="utf-8")
-    return not reports
-
-
-def main() -> int:
-    files = [Path(f) for f in sys.argv[1:]]
-    success = True
-
-    for path in files:
-        success &= fix_names(path)
-
-    return 0 if success else 1
-
-
-if __name__ == "__main__":
-    sys.exit(main())

bin/pre-commit/test_fix_gtest_names.py

@@ -1,259 +0,0 @@
-#!/usr/bin/env python3
-"""
-Tests for fix_gtest_names.py.
-
-Run directly (no test framework needed):
-    ./bin/pre-commit/test_fix_gtest_names.py
-or under pytest:
-    pytest bin/pre-commit/test_fix_gtest_names.py
-"""
-
-import sys
-import textwrap
-
-from fix_gtest_names import camel_case, fix_source, snake_case
-
-
-def dedent(text: str) -> str:
-    """Removes a fixture's common indentation and its leading newline.
-
-    Lets fixtures be written as indented triple-quoted here-docs while keeping
-    honest 1-based line numbers.
-    """
-    return textwrap.dedent(text).lstrip("\n")
-
-
-def fixed(text: str) -> str:
-    return fix_source(dedent(text))[0]
-
-
-def reports(text: str) -> list[str]:
-    return fix_source(dedent(text))[1]
-
-
-# --- conversion --------------------------------------------------------------
-
-
-def test_snake_case_conversion() -> None:
-    assert snake_case("BadInputs") == "bad_inputs"
-    assert snake_case("mulDiv") == "mul_div"
-    assert snake_case("already_snake") == "already_snake"
-    assert snake_case("base64") == "base64"
-
-
-def test_snake_case_keeps_acronyms_whole() -> None:
-    assert snake_case("SetAndResetAccountTxnID") == "set_and_reset_account_txn_id"
-    assert snake_case("XRPToIOU") == "xrp_to_iou"
-    assert snake_case("STAmountMath") == "st_amount_math"
-
-
-def test_camel_case_conversion() -> None:
-    assert camel_case("json_value") == "JsonValue"
-    assert camel_case("mulDiv") == "MulDiv"
-    assert camel_case("scope") == "Scope"
-    assert camel_case("base64") == "Base64"
-
-
-def test_camel_case_leaves_acronyms_alone() -> None:
-    # A snake_case round-trip would give `ShaMapTest` / `ParseStatmRsSkB` here.
-    assert camel_case("SHAMapTest") == "SHAMapTest"
-    assert camel_case("parseStatmRSSkB") == "ParseStatmRSSkB"
-    assert camel_case("XRPAmount") == "XRPAmount"
-    assert camel_case("CSPRNG") == "CSPRNG"
-
-
-def test_disabled_prefix_preserved() -> None:
-    assert snake_case("DISABLED_FooBar") == "DISABLED_foo_bar"
-    assert snake_case("DISABLED_foo_bar") == "DISABLED_foo_bar"
-    assert snake_case("DISABLED_") == "DISABLED_"
-    assert camel_case("DISABLED_foo_bar") == "DISABLED_FooBar"
-    assert camel_case("DISABLED_") == "DISABLED_"
-
-
-# --- what counts as a test definition ---------------------------------------
-
-
-def test_all_macros_recognized() -> None:
-    code = """
-    TEST(Suite, oneName)
-    TEST_F(Fixture, twoName)
-    TEST_P(Fixture, threeName)
-    TYPED_TEST(Fixture, fourName)
-    TYPED_TEST_P(Fixture, fiveName)
-    """
-    assert fixed(code) == dedent("""
-        TEST(Suite, one_name)
-        TEST_F(Fixture, two_name)
-        TEST_P(Fixture, three_name)
-        TYPED_TEST(Fixture, four_name)
-        TYPED_TEST_P(Fixture, five_name)
-        """)
-
-
-def test_conforming_definitions_untouched() -> None:
-    code = """
-    TEST(AccountSet, bad_inputs)
-    TEST_F(MutexMakeTest, default_constructor)
-    TEST(SHAMap, DISABLED_slow_path)
-    """
-    assert reports(code) == []
-    assert fixed(code) == dedent(code)
-
-
-def test_lookalikes_ignored() -> None:
-    code = """
-    // TEST(Suite, notATest)
-    TEST_EXPECT(someCall())
-    TEST_EXPECTS(amount == value, amount.getText())
-    INSTANTIATE_TEST_SUITE_P(Prefix, Fixture, testValues());
-    auto x = TEST(Suite, notATest);
-    TYPED_TEST_SUITE(Fixture, MyTypes);
-    """
-    assert reports(code) == []
-    assert fixed(code) == dedent(code)
-
-
-def test_indented_and_wrapped_definitions() -> None:
-    code = """
-    namespace ripple {
-        TEST(Suite, indentedName)
-    }
-    TEST_F(
-        SomeVeryLongFixtureName,
-        wrappedName)
-    """
-    assert fixed(code) == dedent("""
-        namespace ripple {
-            TEST(Suite, indented_name)
-        }
-        TEST_F(
-            SomeVeryLongFixtureName,
-            wrapped_name)
-        """)
-
-
-# --- rewriting --------------------------------------------------------------
-
-
-def test_only_the_two_names_are_rewritten() -> None:
-    code = """
-    TEST(mulDiv, mulDiv)
-    {
-        auto const mulDiv = 1;  // mulDiv stays
-    }
-    """
-    assert fixed(code) == dedent("""
-        TEST(MulDiv, mul_div)
-        {
-            auto const mulDiv = 1;  // mulDiv stays
-        }
-        """)
-
-
-def test_suite_name_camel_cased() -> None:
-    code = """
-    TEST(json_value, limits)
-    TEST(scope, ScopeExit)
-    """
-    assert reports(code) == [
-        "1: renamed suite 'json_value' to 'JsonValue'",
-        "2: renamed suite 'scope' to 'Scope'",
-        "2: renamed test case 'ScopeExit' to 'scope_exit'",
-    ]
-    assert fixed(code) == dedent("""
-        TEST(JsonValue, limits)
-        TEST(Scope, scope_exit)
-        """)
-
-
-def test_fixture_reported_but_not_renamed() -> None:
-    # The first argument names a class, so only a human (or clang-tidy) can
-    # rename it; the test-case name is still fixed.
-    code = """
-    TEST_F(my_fixture, someTest)
-    """
-    assert reports(code) == [
-        "1: fixture 'my_fixture' is not CamelCase: rename the class to "
-        "'MyFixture' by hand",
-        "1: renamed test case 'someTest' to 'some_test'",
-    ]
-    assert fixed(code) == dedent("""
-        TEST_F(my_fixture, some_test)
-        """)
-
-
-def test_reports_carry_line_numbers() -> None:
-    code = """
-    #include <foo.h>
-
-    TEST(Suite, firstName)
-
-    TEST(Suite, secondName)
-    """
-    assert reports(code) == [
-        "3: renamed test case 'firstName' to 'first_name'",
-        "5: renamed test case 'secondName' to 'second_name'",
-    ]
-
-
-# --- collisions -------------------------------------------------------------
-
-
-def test_collision_reported_and_not_applied() -> None:
-    # Both would define `Suite_mul_div_Test`.
-    code = """
-    TEST(Suite, mulDiv)
-    TEST(Suite, mul_div)
-    """
-    assert reports(code) == [
-        "1: cannot rename 'Suite, mulDiv' to 'Suite, mul_div': another test "
-        "case already generates that name"
-    ]
-    assert fixed(code) == dedent(code)
-
-
-def test_collision_between_converging_suites() -> None:
-    # Both suites camel-case to `SuiteA`, so both would define
-    # `SuiteA_one_test_Test`.
-    code = """
-    TEST(SuiteA, oneTest)
-    TEST(Suite_a, one_test)
-    """
-    assert [r.split(":")[1].strip() for r in reports(code)] == [
-        "cannot rename 'SuiteA, oneTest' to 'SuiteA, one_test'",
-        "cannot rename 'Suite_a, one_test' to 'SuiteA, one_test'",
-    ]
-    assert fixed(code) == dedent(code)
-
-
-def test_same_name_in_different_suites_is_not_a_collision() -> None:
-    code = """
-    TEST(SuiteOne, mulDiv)
-    TEST(SuiteTwo, mulDiv)
-    """
-    assert fixed(code) == dedent("""
-        TEST(SuiteOne, mul_div)
-        TEST(SuiteTwo, mul_div)
-        """)
-
-
-def main() -> int:
-    tests = sorted(
-        (name, fn)
-        for name, fn in globals().items()
-        if name.startswith("test_") and callable(fn)
-    )
-    failed = 0
-    for name, fn in tests:
-        try:
-            fn()
-            print(f"PASS {name}")
-        except AssertionError as exc:
-            failed += 1
-            print(f"FAIL {name}: {exc!r}")
-    print(f"\n{len(tests) - failed}/{len(tests)} passed")
-    return 1 if failed else 0
-
-
-if __name__ == "__main__":
-    sys.exit(main())

cmake/XrplSanitizers.cmake

@@ -77,19 +77,24 @@ if(is_clang)
     message(STATUS "  Ignorelist: ${ignorelist_path}")
 endif()
 
-# Define SANITIZERS macro for BuildInfo.cpp
+# Define the SANITIZERS macro for BuildInfo.cpp, plus one of XRPL_ASAN,
+# XRPL_TSAN and XRPL_UBSAN per active sanitizer, so that code can test for a
+# specific one with #ifdef instead of parsing the dot-joined SANITIZERS string.
 set(sanitizers_list)
 if(SANITIZERS MATCHES "address")
     set(enable_asan ON)
     list(APPEND sanitizers_list "ASAN")
+    target_compile_definitions(common INTERFACE XRPL_ASAN)
 endif()
 if(SANITIZERS MATCHES "thread")
     set(enable_tsan ON)
     list(APPEND sanitizers_list "TSAN")
+    target_compile_definitions(common INTERFACE XRPL_TSAN)
 endif()
 if(SANITIZERS MATCHES "undefinedbehavior")
     set(enable_ubsan ON)
     list(APPEND sanitizers_list "UBSAN")
+    target_compile_definitions(common INTERFACE XRPL_UBSAN)
 endif()
 
 if(sanitizers_list)

docs/build/sanitizers.md

@@ -99,6 +99,41 @@ export TSAN_OPTIONS="include=sanitizers/suppressions/runtime-tsan-options.txt:su
 
 More details [here](https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual).
 
+> [!IMPORTANT]
+> The `ubuntu-clang-debug-amd64-tsan` CI config runs TSan in the extended matrix
+> only, which is the nightly schedule and a manual run, and it reports nothing
+> back. `runtime-tsan-options.txt` sets `halt_on_error=false`, so the run
+> continues past a finding, and the workflow appends `exitcode=0` to
+> `TSAN_OPTIONS`, so the finding does not fail the job.
+> Both are needed: TSan exits 66 on its own once it has reported anything. Read
+> the job log to see findings. A test that must fail on one has to run in its own
+> step with `halt_on_error=1` and `exitcode=66`.
+>
+> `exitcode=0` lives in the workflow and not in `runtime-tsan-options.txt`,
+> because the local command above reads that same file. A local run keeps the
+> default nonzero exit, so a finding fails the command. `exitcode=0` also only
+> covers TSan's own exit path: a test that fails on its own still fails the job.
+>
+> Reporting nothing back is a first stage, not the end state. Once the set of
+> findings the job produces is known and stable, drop `exitcode=0` from the
+> workflow so that a new finding fails the job, and suppress what is left in
+> third-party code.
+
+> [!IMPORTANT]
+> Run TSan on Linux to check lock order. Linux reports an inversion as
+> `WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock)`, with no
+> `TSAN_OPTIONS` needed. macOS arm64 reports nothing, not even a genuine double
+> lock of a non-recursive mutex, and not with `detect_deadlocks=1` or raw
+> `pthread_mutex_t` either. The report strings are present in its runtime and the
+> flag defaults to true, so that is a platform limit rather than a configuration
+> mistake. Data race detection does work on macOS.
+
+> [!TIP]
+> The build defines `XRPL_TSAN` when TSan is active, and `XRPL_ASAN` and
+> `XRPL_UBSAN` for the other two. Use them to skip a test that only means
+> something under one sanitizer. Skip at run time rather than with `#ifdef`
+> around the body, so that every build still compiles the test.
+
 ### LeakSanitizer (LSan)
 
 LSan is automatically enabled with ASAN. To disable it:
@@ -150,15 +185,39 @@ More details [here](https://clang.llvm.org/docs/undefinedbehaviorSanitizer.html)
 
 ### [`tsan.supp`](../../sanitizers/suppressions/tsan.supp)
 
-- **Purpose**: Suppress ThreadSanitizer data race warnings
-- **Format**: `race:<pattern>` where pattern matches function/file names
+- **Purpose**: Suppress ThreadSanitizer warnings
+- **Format**: `<type>:<pattern>` where pattern matches function/file names, and
+  type is `race`, `deadlock`, `signal`, `mutex` or `called_from_lib`
 - **More info**: [ThreadSanitizer suppressions](https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions)
+- **Note**: Every `deadlock:` pattern must name a source file. One that names a
+  locking primitive instead, such as `pthread_rwlock_rdlock`, turns lock-order
+  checking off for every lock of that kind in the tree, which for that example is
+  every `std::shared_mutex` read lock. Suppress the file that reports the
+  inversion instead. A pattern of any type that names a file which has since
+  moved matches nothing, so check the paths when a subsystem is relocated.
 
 ### [`sanitizer-ignorelist.txt`](../../sanitizers/suppressions/sanitizer-ignorelist.txt)
 
 - **Purpose**: Compile-time ignorelist for all sanitizers
 - **Usage**: Passed via `-fsanitize-ignorelist=absolute/path/to/sanitizer-ignorelist.txt`
-- **Format**: `<level>:<pattern>` (e.g., `src:Workers.cpp`)
+- **Format**: `<entity>:<glob>` (e.g. `src:*Workers.cpp`)
+- **Note**: This file is not a suppressions file, and the syntax differs. Clang
+  looks up only the entities `src`, `fun`, `global`, `type` and `mainfile`. A
+  `race:`, `deadlock:` or `signal:` entry parses without an error and is then
+  never consulted, so it does nothing; those types belong in `tsan.supp`.
+- **Note**: The glob must match the whole path as the compiler receives it, which
+  the build makes absolute. So `src:core/detail/Workers.cpp` matches nothing,
+  while `src:*core/detail/Workers.cpp` matches.
+- **Note**: To confirm that an entry works, compile the file with and without the
+  entry and compare how many times the object references the calls the sanitizer
+  plants at an instrumented operation: `__tsan_read` and `__tsan_write`,
+  `__asan_report`, or `__ubsan_handle`. For example
+  `nm -u <file>.o | grep -cE '__tsan_(read|write)'`. A working entry lowers the
+  count, usually to zero, though code inlined from a header still counts because
+  a `src:` glob matches the file a function is defined in. Do not look for
+  `__tsan_func_entry`, which survives an ignorelist, or `__tsan_init`, which only
+  shows the runtime is present; either one reports an ignored file as
+  instrumented.
 
 ## Troubleshooting
 

include/xrpl/shamap/SHAMap.h

@@ -592,20 +592,6 @@ private:
     SHAMapLeafNode*
     belowHelper(NodePathStack& stack, BelowDirection direction) const;
 
-    /**
-     * Returns the nearest item strictly past `id`, in the given direction.
-     *
-     * Walks back up the path to `id`. At each inner node the branches beyond the one `id` takes
-     * hold the candidates, so the first non-empty one is the closest and the extreme leaf below
-     * it is the answer.
-     *
-     * @param id The key to search from, which need not be in the map.
-     * @param direction First to search upwards from `id`, Last to search downwards.
-     * @return An iterator to the item found, or end() if no item lies on that side of `id`.
-     */
-    ConstIterator
-    boundHelper(uint256 const& id, BelowDirection direction) const;
-
     // Simple descent
     // Get a child of the specified node
     SHAMapTreeNode*

sanitizers/suppressions/runtime-tsan-options.txt

@@ -1,3 +1,4 @@
 halt_on_error=false
 verbosity=1
+detect_deadlocks=1
 second_deadlock_stack=1

sanitizers/suppressions/sanitizer-ignorelist.txt

@@ -1,32 +1,23 @@
-# We were seeing some false positives and some repeated errors(since these are library files) in following files.
-# Clang will skip instrumenting the files added here.
-# We should fix the underlying issues(if any) and remove these entries.
-
-deadlock:libxrpl/beast/utility/beast_Journal.cpp
-deadlock:libxrpl/beast/utility/beast_PropertyStream.cpp
-deadlock:test/beast/beast_PropertyStream_test.cpp
-deadlock:xrpld/core/detail/Workers.cpp
-deadlock:xrpld/core/JobQueue.cpp
-
-race:libxrpl/beast/utility/beast_Journal.cpp
-race:libxrpl/beast/utility/beast_PropertyStream.cpp
-race:test/beast/beast_PropertyStream_test.cpp
-race:xrpld/core/detail/Workers.cpp
-race:xrpld/core/JobQueue.cpp
-
-signal:libxrpl/beast/utility/beast_Journal.cpp
-signal:libxrpl/beast/utility/beast_PropertyStream.cpp
-signal:test/beast/beast_PropertyStream_test.cpp
-signal:xrpld/core/detail/Workers.cpp
-signal:xrpld/core/JobQueue.cpp
-
-src:beast/utility/beast_Journal.cpp
-src:beast/utility/beast_PropertyStream.cpp
-src:core/detail/Workers.cpp
-src:core/JobQueue.cpp
-src:libxrpl/beast/utility/beast_Journal.cpp
-src:test/beast/beast_PropertyStream_test.cpp
-src:src/test/app/Invariants_test.cpp
+# Clang skips instrumenting the files listed here. Keep the list as short as it
+# can be: an entry here removes a file from every sanitizer's view, so it hides
+# real findings along with the false positive it was added for.
+#
+# This is a clang ignorelist, not a runtime suppressions file, and the two take
+# different syntax. Here an entry reads <entity>:<glob>, and the only entities
+# clang looks up are src, fun, global, type and mainfile. A `race:`, `deadlock:`
+# or `signal:` entry parses without an error and is then never consulted, so it
+# does nothing; those types belong in tsan.supp. The glob has to match the whole
+# path as the compiler receives it, which the build makes absolute, so an entry
+# needs a leading `*` to match anything at all. See docs/build/sanitizers.md.
+#
+# This list once held seven `src:` entries for beast_Journal.cpp,
+# beast_PropertyStream.cpp, Workers.cpp, JobQueue.cpp, the PropertyStream test
+# and Invariants_test.cpp, added against false positives. None carried the
+# leading `*`, so none ever matched, and every one of those files has been
+# instrumented the whole time. The sanitizer jobs pass that way, so the
+# suppressions are not needed; they are removed rather than repaired. Restore one
+# only with a finding that shows it is needed, and check it works by the method
+# in sanitizers.md.
 
 # ASan false positive: stack-use-after-scope in ErrorCodes.h inline functions.
 # When Clang inlines the StaticString overloads (e.g. invalidFieldError(StaticString)),

sanitizers/suppressions/tsan.supp

@@ -35,43 +35,52 @@ race:src/xrpld/app/main/BasicApp.cpp
 race:src/xrpld/app/main/GRPCServer.cpp
 race:src/xrpld/app/misc/detail/AmendmentTable.cpp
 race:src/xrpld/app/misc/FeeVoteImpl.cpp
-race:src/xrpld/app/rdb/detail/Wallet.cpp
+race:src/libxrpl/server/Wallet.cpp
 race:src/xrpld/overlay/detail/OverlayImpl.cpp
-race:src/xrpld/peerfinder/detail/PeerfinderManager.cpp
-race:src/xrpld/peerfinder/detail/SourceStrings.cpp
+race:src/libxrpl/peerfinder/PeerfinderManager.cpp
+race:src/libxrpl/peerfinder/SourceStrings.cpp
 race:src/xrpld/rpc/detail/ServerHandler.cpp
 race:xrpl/server/detail/Door.h
 race:xrpl/server/detail/Spawn.h
 race:xrpl/server/detail/ServerImpl.h
 race:xrpl/nodestore/detail/DatabaseNodeImp.h
 race:src/libxrpl/beast/utility/beast_Journal.cpp
-race:src/test/beast/LexicalCast_test.cpp
+race:src/tests/libxrpl/beast/LexicalCast.cpp
 race:ServerHandler
 
 # More suppressions in external library code.
 race:crtstuff.c
 race:pipe
 
-# Deadlock / lock-order-inversion suppressions
-# Note: GCC's TSAN may not fully support all deadlock suppression patterns
-deadlock:src/libxrpl/beast/utility/beast_Journal.cpp
-deadlock:src/libxrpl/beast/utility/beast_PropertyStream.cpp
-deadlock:src/test/beast/beast_PropertyStream_test.cpp
-deadlock:src/xrpld/core/detail/Workers.cpp
-deadlock:src/xrpld/app/misc/detail/Manifest.cpp
-deadlock:src/xrpld/app/misc/detail/ValidatorList.cpp
-deadlock:src/xrpld/app/misc/detail/ValidatorSite.cpp
+# Deadlock / lock-order-inversion suppressions: deliberately none.
+#
+# The TSan job exists to find lock-order inversions, and it cannot fail a build,
+# so a suppression here costs a finding and buys nothing. Ten used to sit below:
+#
+#   - pthread_create, pthread_rwlock_rdlock and boost::asio, which name locking
+#     primitives rather than source files. Each of those turns lock-order
+#     checking off for every lock of that kind in the tree, which for
+#     pthread_rwlock_rdlock is every std::shared_mutex read lock, and between
+#     them they covered any inversion reached through a strand or a thread start.
+#   - ValidatorList.cpp and ValidatorSite.cpp.
+#   - beast_Journal.cpp, beast_PropertyStream.cpp, the PropertyStream test,
+#     Workers.cpp and Manifest.cpp.
+#
+# Manifest.cpp was the expensive one. ManifestCache::save() invokes the caller's
+# predicate while holding its own lock, and the validator list takes the pair the
+# other way, so the two deadlock. Removing that one line surfaced the cycle 52
+# times in a single run, on the shutdown path.
+#
+# If one has to come back, it must name a source file, and check that the file is
+# where the pattern says: a suppression naming a moved file protects nothing.
+# Note also that GCC's TSAN may not fully support all deadlock suppression
+# patterns.
 
 signal:src/libxrpl/beast/utility/beast_Journal.cpp
-signal:src/xrpld/core/detail/Workers.cpp
-signal:src/xrpld/core/JobQueue.cpp
+signal:src/libxrpl/core/detail/Workers.cpp
+signal:src/libxrpl/core/detail/JobQueue.cpp
 signal:Workers::Worker
 
-# Aggressive suppressing of deadlock tsan errors
-deadlock:pthread_create
-deadlock:pthread_rwlock_rdlock
-deadlock:boost::asio
-
 # Suppress SEGV crashes in TSAN itself during stringbuf operations
 # This appears to be a GCC-15 TSAN instrumentation issue with basic_stringbuf::str()
 # Commonly triggered in beast::Journal::ScopedStream destructor

src/libxrpl/shamap/SHAMap.cpp

@@ -575,10 +575,8 @@ SHAMap::peekItem(uint256 const& id, SHAMapHash& hash) const
 }
 
 SHAMap::ConstIterator
-SHAMap::boundHelper(uint256 const& id, BelowDirection direction) const
+SHAMap::upperBound(uint256 const& id) const
 {
-    auto const searchingForward = direction == BelowDirection::First;
-
     NodePathStack stack;
     walkTowardsKey(id, &stack);
     while (!stack.empty())
@@ -586,45 +584,63 @@ SHAMap::boundHelper(uint256 const& id, BelowDirection direction) const
         auto const [node, nodeID] = stack.top();
         if (node->isLeaf())
         {
-            auto const& item = safeDowncast<SHAMapLeafNode const&>(*node).peekItem();
-            if (searchingForward ? (item->key() > id) : (item->key() < id))
-                return ConstIterator(this, item.get(), std::move(stack));
+            auto leaf = safeDowncast<SHAMapLeafNode*>(node.get());
+            if (leaf->peekItem()->key() > id)
+                return ConstIterator(this, leaf->peekItem().get(), std::move(stack));
         }
         else
         {
             auto& inner = safeDowncast<SHAMapInnerNode&>(*node);
-            auto const taken = selectBranch(nodeID, id);
-            auto const remaining = searchingForward ? (kBranchFactor - 1u - taken) : taken;
-
-            for (auto scanned = 0u; scanned < remaining; ++scanned)
+            for (auto branch = selectBranch(nodeID, id) + 1; branch < kBranchFactor; ++branch)
             {
-                auto const branch =
-                    searchingForward ? (taken + 1u + scanned) : (taken - 1u - scanned);
-                if (inner.isEmptyBranch(branch))
-                    continue;
-
-                stack.pushChild(descendThrow(inner, branch), branch);
-                auto const leaf = belowHelper(stack, direction);
-                if (leaf == nullptr)
-                    Throw<SHAMapMissingNode>(type_, id);
-                return ConstIterator(this, leaf->peekItem().get(), std::move(stack));
+                if (!inner.isEmptyBranch(branch))
+                {
+                    stack.pushChild(descendThrow(inner, branch), branch);
+                    auto leaf = belowHelper(stack, BelowDirection::First);
+                    if (leaf == nullptr)
+                        Throw<SHAMapMissingNode>(type_, id);
+                    return ConstIterator(this, leaf->peekItem().get(), std::move(stack));
+                }
             }
         }
         stack.pop();
     }
     return end();
 }
-
-SHAMap::ConstIterator
-SHAMap::upperBound(uint256 const& id) const
-{
-    return boundHelper(id, BelowDirection::First);
-}
-
 SHAMap::ConstIterator
 SHAMap::lowerBound(uint256 const& id) const
 {
-    return boundHelper(id, BelowDirection::Last);
+    NodePathStack stack;
+    walkTowardsKey(id, &stack);
+    while (!stack.empty())
+    {
+        auto const [node, nodeID] = stack.top();
+        if (node->isLeaf())
+        {
+            auto leaf = safeDowncast<SHAMapLeafNode*>(node.get());
+            if (leaf->peekItem()->key() < id)
+                return ConstIterator(this, leaf->peekItem().get(), std::move(stack));
+        }
+        else
+        {
+            auto& inner = safeDowncast<SHAMapInnerNode&>(*node);
+            for (auto branch = selectBranch(nodeID, id); branch > 0u;)
+            {
+                --branch;
+                if (!inner.isEmptyBranch(branch))
+                {
+                    stack.pushChild(descendThrow(inner, branch), branch);
+                    auto leaf = belowHelper(stack, BelowDirection::Last);
+                    if (leaf == nullptr)
+                        Throw<SHAMapMissingNode>(type_, id);
+                    return ConstIterator(this, leaf->peekItem().get(), std::move(stack));
+                }
+            }
+        }
+        stack.pop();
+    }
+    // TODO: what to return here?
+    return end();
 }
 
 bool

src/test/jtx/ConfidentialTransfer.h

@@ -3,19 +3,28 @@
 #include <test/jtx/Account.h>
 #include <test/jtx/Env.h>
 #include <test/jtx/mpt.h>
+#include <test/jtx/vault.h>
 
 #include <xrpl/basics/Buffer.h>
+#include <xrpl/basics/Slice.h>
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/basics/contract.h>
+#include <xrpl/basics/strHex.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/protocol/ConfidentialTransfer.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
 
+#include <utility/mpt_utility.h>
+
+#include <secp256k1.h>
+#include <secp256k1_mpt.h>
+
 #include <array>
 #include <cstddef>
 #include <cstdint>
+#include <cstring>
 #include <functional>
 #include <optional>
 #include <stdexcept>
@@ -45,66 +54,237 @@ protected:
         return *value;
     }
 
-    // Creates the MPT issuance on the given Env, authorizes and funds each
-    // holder, generates keys for the issuer, holders and optional auditor,
-    // registers the issuer/auditor keys, and converts part of each holder's
-    // balance to a confidential balance.
-    struct ConfidentialEnv
+    // Offset where the bulletproof begins in a send proof blob.
+    // Proof layout: [compact_sigma | bulletproof]
+    static constexpr size_t kBulletproofOffset = kEcSendProofLength - kEcDoubleBulletproofLength;
+
+    // Generate a forged aggregated bulletproof (double bulletproof) for
+    // the given values and blinding factors. Used to test that splicing
+    // a bulletproof claiming a different remaining balance is rejected.
+    // secp256k1 convention: returns 1 on success, 0 on failure.
+    static Buffer
+    getForgedBulletproof(
+        std::array<uint64_t, 2> const& values,
+        std::array<Buffer, 2> const& blindingFactors,
+        uint256 const& contextHash)
     {
-        // Per-holder configuration: the account, how much MPT to fund it
-        // with, and how much of that to convert to a confidential balance.
-        struct HolderInit
+        auto* const ctx = mpt_secp256k1_context();
+
+        secp256k1_pubkey h;
+        secp256k1_mpt_get_h_generator(ctx, &h);
+
+        Buffer proof(kEcDoubleBulletproofLength);
+        size_t proofLen = kEcDoubleBulletproofLength;
+
+        unsigned char blindings[64];
+        std::memcpy(blindings, blindingFactors[0].data(), 32);
+        std::memcpy(blindings + 32, blindingFactors[1].data(), 32);
+
+        if (secp256k1_bulletproof_prove_agg(
+                ctx,
+                proof.data(),
+                &proofLen,
+                values.data(),
+                blindings,
+                2,
+                &h,
+                contextHash.data()) == 0)
+            Throw<std::runtime_error>("Failed to generate forged bulletproof");
+
+        return proof;
+    }
+
+    // Generate a forged single bulletproof for a single value and blinding factor.
+    // Used to test ConvertBack overdraft prevention via bulletproof verification.
+    static Buffer
+    getForgedSingleBulletproof(
+        uint64_t value,
+        Buffer const& blindingFactor,
+        uint256 const& contextHash)
+    {
+        auto* const ctx = mpt_secp256k1_context();
+
+        secp256k1_pubkey h;
+        secp256k1_mpt_get_h_generator(ctx, &h);
+
+        Buffer proof(kEcSingleBulletproofLength);
+        size_t proofLen = kEcSingleBulletproofLength;
+
+        if (secp256k1_bulletproof_prove_agg(
+                ctx,
+                proof.data(),
+                &proofLen,
+                &value,
+                blindingFactor.data(),
+                1,  // m = 1 (single bulletproof)
+                &h,
+                contextHash.data()) == 0)
+            Throw<std::runtime_error>("Failed to generate forged single bulletproof");
+
+        return proof;
+    }
+
+    // Forges a ConvertBack proof (compact sigma + single bulletproof) whose
+    // sigma component claims claimedBalance (which may be wrong) while binding
+    // to the real pedersen commitment and encrypted spending balance
+    // ciphertext already on the ledger. The bulletproof component is built
+    // from realBalance so it stays honest.
+    // mpt_get_convert_back_proof does not allow to build a proof whose amount
+    // exceeds the holder's claimed balance.
+    static Buffer
+    getForgedConvertBackProof(
+        test::jtx::MPTTester& mpt,
+        test::jtx::Account const& holder,
+        uint64_t claimedBalance,
+        uint64_t realBalance,
+        uint64_t amt,
+        Buffer const& pedersenCommitment,
+        Buffer const& encryptedSpendingBalance,
+        Buffer const& pcBlindingFactor,
+        uint256 const& contextHash)
+    {
+        if (pedersenCommitment.size() != kCompressedEcPointLength)
+            Throw<std::runtime_error>("getForgedConvertBackProof: bad pedersenCommitment length");
+        if (encryptedSpendingBalance.size() != kEcGamalEncryptedTotalLength)
         {
-            test::jtx::Account account;
-            std::uint64_t payAmount = 1000;
-            std::uint64_t convertAmount = 100;
-        };
+            Throw<std::runtime_error>(
+                "getForgedConvertBackProof: bad encryptedSpendingBalance length");
+        }
+        if (amt > realBalance)
+            Throw<std::runtime_error>("getForgedConvertBackProof: amt exceeds realBalance");
 
-        test::jtx::MPTTester mpt;
+        auto* const ctx = mpt_secp256k1_context();
+        auto const holderPubKey = requireOptional(mpt.getPubKey(holder), "Missing holder pubkey");
+        auto const holderPrivKey =
+            requireOptional(mpt.getPrivKey(holder), "Missing holder privkey");
 
-        ConfidentialEnv(
-            test::jtx::Env& env,
-            test::jtx::Account const& issuer,
-            std::vector<HolderInit> const& holders,
-            std::uint32_t flags = tfMPTCanLock | tfMPTCanHoldConfidentialBalance | tfMPTCanTransfer,
-            std::optional<test::jtx::Account> auditor = std::nullopt);
+        secp256k1_pubkey pkHolder;
+        if (secp256k1_ec_pubkey_parse(
+                ctx, &pkHolder, holderPubKey.data(), kCompressedEcPointLength) != 1)
+            Throw<std::runtime_error>("Failed to parse holder's public key");
 
-    private:
-        static std::vector<test::jtx::Account>
-        extractAccounts(std::vector<HolderInit> const& holders);
-    };
+        secp256k1_pubkey pcB;
+        if (secp256k1_ec_pubkey_parse(
+                ctx, &pcB, pedersenCommitment.data(), kCompressedEcPointLength) != 1)
+            Throw<std::runtime_error>("Failed to parse pedersen commitment");
 
-    // Create an issuance that can hold confidential balances, with the listed
-    // holders funded and authorized, and a key pair generated for the issuer,
-    // every holder, and every extra key owner. The keys are
-    // generated but not registered.
-    static void
-    setupConfidentialIssuance(
-        test::jtx::MPTTester& mpt,
-        test::jtx::Account const& issuer,
-        std::vector<test::jtx::Account> const& holders,
-        std::vector<test::jtx::Account> const& keyOwners = {},
-        std::uint32_t flags = tfMPTCanTransfer | tfMPTCanHoldConfidentialBalance);
+        secp256k1_pubkey b1, b2;
+        if (secp256k1_ec_pubkey_parse(
+                ctx, &b1, encryptedSpendingBalance.data(), kCompressedEcPointLength) != 1 ||
+            secp256k1_ec_pubkey_parse(
+                ctx,
+                &b2,
+                encryptedSpendingBalance.data() + kCompressedEcPointLength,
+                kCompressedEcPointLength) != 1)
+            Throw<std::runtime_error>("Failed to parse balance ciphertext");
 
-    // Set up an MPT environment suitable for batch testing.
-    // alice is issuer; bob has 'bobAmt' in confidential spending; carol has
-    // 'carolAmt' in confidential spending; dave is initialised with pubkey but
-    // zero spending/inbox.
-    static void
-    setupBatchEnv(
-        test::jtx::MPTTester& mpt,
-        test::jtx::Account const& alice,
-        test::jtx::Account const& bob,
-        test::jtx::Account const& carol,
-        test::jtx::Account const& dave,
-        std::uint64_t bobAmt,
-        std::uint64_t carolAmt);
+        Buffer sigmaProof(SECP256K1_COMPACT_CONVERTBACK_PROOF_SIZE);
+        if (secp256k1_compact_convertback_prove(
+                ctx,
+                sigmaProof.data(),
+                claimedBalance,
+                holderPrivKey.data(),
+                pcBlindingFactor.data(),
+                &pkHolder,
+                &b1,
+                &b2,
+                &pcB,
+                contextHash.data()) != 1)
+            Throw<std::runtime_error>("Failed to generate convertback sigma proof");
+
+        auto const forgedBulletproof =
+            getForgedSingleBulletproof(realBalance - amt, pcBlindingFactor, contextHash);
+
+        Buffer proof(kEcConvertBackProofLength);
+        std::memcpy(proof.data(), sigmaProof.data(), SECP256K1_COMPACT_CONVERTBACK_PROOF_SIZE);
+        std::memcpy(
+            proof.data() + SECP256K1_COMPACT_CONVERTBACK_PROOF_SIZE,
+            forgedBulletproof.data(),
+            kEcSingleBulletproofLength);
+
+        return proof;
+    }
+
+    // Get a bad ciphertext with valid structure but cryptographic invalid for
+    // testing purposes. For preflight test purposes.
+    static Buffer const&
+    getBadCiphertext()
+    {
+        static Buffer const kBadCiphertext = []() {
+            Buffer buf(kEcGamalEncryptedTotalLength);
+            std::memset(buf.data(), 0xFF, kEcGamalEncryptedTotalLength);
+
+            buf.data()[0] = kEcCompressedPrefixEvenY;
+            buf.data()[kEcCiphertextComponentLength] = kEcCompressedPrefixEvenY;
+            return buf;
+        }();
+
+        return kBadCiphertext;
+    }
+
+    // Get a trivial buffer that is structurally and mathematically valid, but
+    // contains invalid data that does not match the ledger state. For preclaim
+    // test purposes.
+    static Buffer const&
+    getTrivialCiphertext()
+    {
+        static Buffer const kTrivialCiphertext = []() {
+            Buffer buf(kEcGamalEncryptedTotalLength);
+            std::memset(buf.data(), 0, kEcGamalEncryptedTotalLength);
+
+            buf.data()[0] = kEcCompressedPrefixEvenY;
+            buf.data()[kEcCiphertextComponentLength] = kEcCompressedPrefixEvenY;
+
+            buf.data()[kEcCiphertextComponentLength - 1] = 0x01;
+            buf.data()[kEcGamalEncryptedTotalLength - 1] = 0x01;
+
+            return buf;
+        }();
+
+        return kTrivialCiphertext;
+    }
+
+    // Returns a valid compressed EC point (33 bytes) that can pass preflight
+    // validation but contains invalid data for preclaim test purposes.
+    static Buffer const&
+    getTrivialCommitment()
+    {
+        static Buffer const kTrivialCommitment = []() {
+            Buffer buf(kEcPedersenCommitmentLength);
+            std::memset(buf.data(), 0, kEcPedersenCommitmentLength);
+
+            buf.data()[0] = kEcCompressedPrefixEvenY;
+            // Set last byte to make it a valid x-coordinate on the curve
+            buf.data()[kEcPedersenCommitmentLength - 1] = 0x01;
+
+            return buf;
+        }();
+
+        return kTrivialCommitment;
+    }
+
+    static std::string
+    getTrivialSendProofHex()
+    {
+        Buffer buf(kEcSendProofLength);
+        std::memset(buf.data(), 0, kEcSendProofLength);
+
+        for (std::size_t i = 0; i < kEcSendProofLength; i += kEcCiphertextComponentLength)
+        {
+            buf.data()[i] = kEcCompressedPrefixEvenY;
+            if (i + kEcCiphertextComponentLength - 1 < kEcSendProofLength)
+                buf.data()[i + kEcCiphertextComponentLength - 1] = 0x01;
+        }
+
+        return strHex(buf);
+    }
 
     // Helper struct to encapsulate common setup for integration tests.
     struct ConfidentialSendSetup
     {
         // Constants
         uint64_t sendAmount;
+        size_t nRecipients;
         uint32_t version;
 
         // Blinding factors
@@ -144,7 +324,55 @@ protected:
             test::jtx::Account const& dest,
             test::jtx::Account const& issuer,
             uint64_t amount,
-            std::optional<std::reference_wrapper<test::jtx::Account const>> auditor = std::nullopt);
+            std::optional<std::reference_wrapper<test::jtx::Account const>> auditor = std::nullopt)
+            : sendAmount(amount)
+            , nRecipients(auditor ? 4 : 3)
+            , version(mpt.getMPTokenVersion(sender))
+            , blindingFactor(generateBlindingFactor())
+            , amountBlindingFactor(blindingFactor)
+            , balanceBlindingFactor(generateBlindingFactor())
+            , senderAmt(mpt.encryptAmount(sender, amount, blindingFactor))
+            , destAmt(mpt.encryptAmount(dest, amount, blindingFactor))
+            , issuerAmt(mpt.encryptAmount(issuer, amount, blindingFactor))
+            , auditorAmt(
+                  auditor ? std::optional<Buffer>(
+                                mpt.encryptAmount(auditor->get(), amount, blindingFactor))
+                          : std::nullopt)
+            , amountCommitment(mpt.getPedersenCommitment(amount, amountBlindingFactor))
+            , senderPubKey(requireOptional(mpt.getPubKey(sender), "Missing sender public key"))
+            , destPubKey(requireOptional(mpt.getPubKey(dest), "Missing destination public key"))
+            , issuerPubKey(requireOptional(mpt.getPubKey(issuer), "Missing issuer public key"))
+            , auditorPubKey(auditor ? mpt.getPubKey(auditor->get()) : std::nullopt)
+            , prevSpending(requireOptional(
+                  mpt.getDecryptedBalance(sender, test::jtx::MPTTester::holderEncryptedSpending),
+                  "Missing sender spending balance"))
+            , prevEncryptedSpending(requireOptional(
+                  mpt.getEncryptedBalance(sender, test::jtx::MPTTester::holderEncryptedSpending),
+                  "Missing sender encrypted spending balance"))
+            , balanceCommitment(mpt.getPedersenCommitment(prevSpending, balanceBlindingFactor))
+        {
+            recipients.push_back({
+                .publicKey = Slice(senderPubKey),
+                .encryptedAmount = senderAmt,
+            });
+            recipients.push_back({
+                .publicKey = Slice(destPubKey),
+                .encryptedAmount = destAmt,
+            });
+            recipients.push_back({
+                .publicKey = Slice(issuerPubKey),
+                .encryptedAmount = issuerAmt,
+            });
+            if (auditor)
+            {
+                recipients.push_back({
+                    .publicKey =
+                        Slice(requireOptionalRef(auditorPubKey, "Missing auditor public key")),
+                    .encryptedAmount =
+                        requireOptionalRef(auditorAmt, "Missing auditor encrypted amount"),
+                });
+            }
+        }
 
         // Generate proof with current account sequence
         std::optional<Buffer>
@@ -152,78 +380,54 @@ protected:
             test::jtx::MPTTester& mpt,
             test::jtx::Env& env,
             test::jtx::Account const& sender,
-            test::jtx::Account const& dest) const;
+            test::jtx::Account const& dest) const
+        {
+            auto const ctxHash = getSendContextHash(
+                sender.id(), mpt.issuanceID(), env.seq(sender), dest.id(), version);
+
+            return mpt.getConfidentialSendProof(
+                sender,
+                sendAmount,
+                recipients,
+                blindingFactor,
+                ctxHash,
+                {
+                    .pedersenCommitment = amountCommitment,
+                    .amt = sendAmount,
+                    .encryptedAmt = senderAmt,
+                    .blindingFactor = amountBlindingFactor,
+                },
+                {
+                    .pedersenCommitment = balanceCommitment,
+                    .amt = prevSpending,
+                    .encryptedAmt = prevEncryptedSpending,
+                    .blindingFactor = balanceBlindingFactor,
+                });
+        }
 
         [[nodiscard]] test::jtx::MPTConfidentialSend
         sendArgs(
             test::jtx::Account const& sender,
             test::jtx::Account const& dest,
             Buffer const& proof,
-            std::optional<TER> err = std::nullopt) const;
+            std::optional<TER> err = std::nullopt) const
+        {
+            return {
+                .account = sender,
+                .dest = dest,
+                .amt = sendAmount,
+                .proof = strHex(proof),
+                .senderEncryptedAmt = senderAmt,
+                .destEncryptedAmt = destAmt,
+                .issuerEncryptedAmt = issuerAmt,
+                .auditorEncryptedAmt = auditorAmt,
+                .amountCommitment = amountCommitment,
+                .balanceCommitment = balanceCommitment,
+                .err = err,
+            };
+        }
     };
 
-    // Get a bad ciphertext with valid structure but cryptographic invalid for
-    // testing purposes. For preflight test purposes.
-    static Buffer const&
-    getBadCiphertext();
-
-    // Get a trivial buffer that is structurally and mathematically valid, but
-    // contains invalid data that does not match the ledger state. For preclaim
-    // test purposes.
-    static Buffer const&
-    getTrivialCiphertext();
-
-    // Returns a valid compressed EC point (33 bytes) that can pass preflight
-    // validation but contains invalid data for preclaim test purposes.
-    static Buffer const&
-    getTrivialCommitment();
-
-    // Returns a hex-encoded send proof of the correct length filled with
-    // placeholder data. It passes the proof length check in preflight but
-    // fails proof verification.
-    static std::string
-    getTrivialSendProofHex();
-
-    // Offset where the bulletproof begins in a send proof blob.
-    // Proof layout: [compact_sigma | bulletproof]
-    static constexpr size_t kBulletproofOffset = kEcSendProofLength - kEcDoubleBulletproofLength;
-
-    // Generate a forged aggregated bulletproof (double bulletproof) for
-    // the given values and blinding factors. Used to test that splicing
-    // a bulletproof claiming a different remaining balance is rejected.
-    static Buffer
-    getForgedBulletproof(
-        std::array<uint64_t, 2> const& values,
-        std::array<Buffer, 2> const& blindingFactors,
-        uint256 const& contextHash);
-
-    // Generate a forged single bulletproof for a single value and blinding factor.
-    // Used to test ConvertBack overdraft prevention via bulletproof verification.
-    static Buffer
-    getForgedSingleBulletproof(
-        uint64_t value,
-        Buffer const& blindingFactor,
-        uint256 const& contextHash);
-
-    // Forges a ConvertBack proof (compact sigma + single bulletproof) whose
-    // sigma component claims claimedBalance (which may be wrong) while binding
-    // to the real pedersen commitment and to the encrypted spending balance
-    // already on the ledger. The bulletproof component is built from the real
-    // remaining balance (realBalance - amt) so it stays honest.
-    // mpt_get_convert_back_proof validates its inputs before proving, so it
-    // cannot be used to build such an inconsistent proof.
-    static Buffer
-    getForgedConvertBackProof(
-        test::jtx::MPTTester& mpt,
-        test::jtx::Account const& holder,
-        uint64_t claimedBalance,
-        uint64_t realBalance,
-        uint64_t amt,
-        Buffer const& pedersenCommitment,
-        Buffer const& encryptedSpendingBalance,
-        Buffer const& pcBlindingFactor,
-        uint256 const& contextHash);
-
     // Forges a ConfidentialMPTSend proof (compact sigma + double bulletproof)
     // for setup.sendAmount against setup's real balance commitment/ciphertext.
     // mpt_get_confidential_send_proof does not allow to build a proof whose amount
@@ -234,7 +438,265 @@ protected:
         test::jtx::Env& env,
         test::jtx::Account const& sender,
         test::jtx::Account const& dest,
-        ConfidentialSendSetup const& setup);
+        ConfidentialSendSetup const& setup)
+    {
+        auto* const ctx = mpt_secp256k1_context();
+
+        secp256k1_pubkey c1;
+        std::vector<secp256k1_pubkey> c2Vec(setup.recipients.size());
+        std::vector<secp256k1_pubkey> pkVec(setup.recipients.size());
+        for (std::size_t i = 0; i < setup.recipients.size(); ++i)
+        {
+            auto const& r = setup.recipients[i];
+            if (i == 0 &&
+                secp256k1_ec_pubkey_parse(
+                    ctx, &c1, r.encryptedAmount.data(), kCompressedEcPointLength) != 1)
+                Throw<std::runtime_error>("Failed to parse C1");
+            if (secp256k1_ec_pubkey_parse(
+                    ctx,
+                    &c2Vec[i],
+                    r.encryptedAmount.data() + kCompressedEcPointLength,
+                    kCompressedEcPointLength) != 1)
+                Throw<std::runtime_error>("Failed to parse C2");
+            if (secp256k1_ec_pubkey_parse(
+                    ctx, &pkVec[i], r.publicKey.data(), kCompressedEcPointLength) != 1)
+                Throw<std::runtime_error>("Failed to parse recipient pubkey");
+        }
+
+        secp256k1_pubkey pkSender, pcAmount, pcBalance, b1, b2;
+        if (secp256k1_ec_pubkey_parse(
+                ctx, &pkSender, setup.senderPubKey.data(), kCompressedEcPointLength) != 1 ||
+            secp256k1_ec_pubkey_parse(
+                ctx, &pcAmount, setup.amountCommitment.data(), kCompressedEcPointLength) != 1 ||
+            secp256k1_ec_pubkey_parse(
+                ctx, &pcBalance, setup.balanceCommitment.data(), kCompressedEcPointLength) != 1 ||
+            secp256k1_ec_pubkey_parse(
+                ctx, &b1, setup.prevEncryptedSpending.data(), kCompressedEcPointLength) != 1 ||
+            secp256k1_ec_pubkey_parse(
+                ctx,
+                &b2,
+                setup.prevEncryptedSpending.data() + kCompressedEcPointLength,
+                kCompressedEcPointLength) != 1)
+            Throw<std::runtime_error>("Failed to parse commitments/ciphertext");
+
+        Buffer const senderPrivKey =
+            requireOptional(mpt.getPrivKey(sender), "Missing sender privkey");
+        auto const ctxHash = getSendContextHash(
+            sender.id(), mpt.issuanceID(), env.seq(sender), dest.id(), setup.version);
+
+        Buffer sigmaProof(SECP256K1_COMPACT_STANDARD_PROOF_SIZE);
+        if (secp256k1_compact_standard_prove(
+                ctx,
+                sigmaProof.data(),
+                setup.sendAmount,
+                setup.prevSpending,
+                setup.blindingFactor.data(),
+                senderPrivKey.data(),
+                setup.balanceBlindingFactor.data(),
+                setup.recipients.size(),
+                &c1,
+                c2Vec.data(),
+                pkVec.data(),
+                &pcAmount,
+                &pkSender,
+                &pcBalance,
+                &b1,
+                &b2,
+                ctxHash.data()) != 1)
+            Throw<std::runtime_error>("Failed to generate sigma proof");
+
+        // Wraps (mod 2^64) for overdrafts, unlike the ledger's own homomorphic
+        // commitment subtraction (mod the curve order) — that mismatch is
+        // exactly what makes the forged proof fail verification.
+        // Computed without a wrapping `uint64` subtract: Clang UBSan treats
+        // unsigned overflow as fatal (see incrementConfidentialVersion).
+        std::uint64_t const remaining = setup.sendAmount <= setup.prevSpending
+            ? setup.prevSpending - setup.sendAmount
+            : ~setup.sendAmount + setup.prevSpending + 1;
+
+        Buffer negAmountBf(kEcBlindingFactorLength);
+        Buffer remainingBf(kEcBlindingFactorLength);
+        secp256k1_mpt_scalar_negate(negAmountBf.data(), setup.amountBlindingFactor.data());
+        secp256k1_mpt_scalar_add(
+            remainingBf.data(), setup.balanceBlindingFactor.data(), negAmountBf.data());
+
+        auto const forgedBulletproof = getForgedBulletproof(
+            {setup.sendAmount, remaining}, {setup.amountBlindingFactor, remainingBf}, ctxHash);
+
+        Buffer combinedProof(kEcSendProofLength);
+        std::memcpy(combinedProof.data(), sigmaProof.data(), SECP256K1_COMPACT_STANDARD_PROOF_SIZE);
+        std::memcpy(
+            combinedProof.data() + SECP256K1_COMPACT_STANDARD_PROOF_SIZE,
+            forgedBulletproof.data(),
+            kEcDoubleBulletproofLength);
+
+        return combinedProof;
+    }
+
+    // Helper that wraps the boilerplate setup: Env + MPT creation, funding, key
+    // generation, and seeding each holder with a confidential balance.
+    // The caller supplies the issuer and any number of holders.
+    struct ConfidentialEnv
+    {
+        // Per-holder configuration: the account, how much MPT to fund it
+        // with, and how much of that to convert to a confidential balance.
+        struct HolderInit
+        {
+            test::jtx::Account account;
+            std::uint64_t payAmount = 1000;
+            std::uint64_t convertAmount = 100;
+        };
+
+        test::jtx::MPTTester mpt;
+
+        ConfidentialEnv(
+            test::jtx::Env& env,
+            test::jtx::Account const& issuer,
+            std::vector<HolderInit> const& holders,
+            std::uint32_t flags = tfMPTCanLock | tfMPTCanHoldConfidentialBalance | tfMPTCanTransfer,
+            std::optional<test::jtx::Account> auditor = std::nullopt)
+            : mpt{env, issuer, {.holders = extractAccounts(holders), .auditor = auditor}}
+        {
+            mpt.create({.ownerCount = 1, .flags = flags});
+
+            for (auto const& h : holders)
+            {
+                mpt.authorize({.account = h.account});
+                if ((flags & tfMPTRequireAuth) != 0)
+                    mpt.authorize({.account = issuer, .holder = h.account});
+                mpt.pay(issuer, h.account, h.payAmount);
+            }
+
+            mpt.generateKeyPair(issuer);
+            for (auto const& h : holders)
+                mpt.generateKeyPair(h.account);
+            if (auditor)
+                mpt.generateKeyPair(requireOptionalRef(auditor, "Missing auditor"));
+
+            mpt.set({
+                .account = issuer,
+                .issuerPubKey = mpt.getPubKey(issuer),
+                .auditorPubKey = auditor
+                    ? mpt.getPubKey(requireOptionalRef(auditor, "Missing auditor"))
+                    : std::optional<Buffer>{},
+            });
+
+            for (auto const& h : holders)
+            {
+                mpt.convert({
+                    .account = h.account,
+                    .amt = h.convertAmount,
+                    .holderPubKey = mpt.getPubKey(h.account),
+                });
+                mpt.mergeInbox({.account = h.account});
+            }
+        }
+
+    private:
+        static std::vector<test::jtx::Account>
+        extractAccounts(std::vector<HolderInit> const& holders)
+        {
+            std::vector<test::jtx::Account> accounts;
+            accounts.reserve(holders.size());
+            for (auto const& h : holders)
+                accounts.push_back(h.account);
+            return accounts;
+        }
+    };
+
+    // Create an issuance that can hold confidential balances, with the listed
+    // holders funded and authorized, and a key pair generated for the issuer,
+    // every holder, and every extra key owner. The keys are
+    // generated but not registered.
+    static void
+    setupConfidentialIssuance(
+        test::jtx::MPTTester& mpt,
+        test::jtx::Account const& issuer,
+        std::vector<test::jtx::Account> const& holders,
+        std::vector<test::jtx::Account> const& keyOwners = {},
+        std::uint32_t flags = tfMPTCanTransfer | tfMPTCanHoldConfidentialBalance);
+
+    // Set up an MPT environment suitable for batch testing.
+    // alice is issuer; bob has 'bobAmt' in confidential spending; carol has
+    // 'carolAmt' in confidential spending; dave is initialised with pubkey but
+    // zero spending/inbox.
+    static void
+    setupBatchEnv(
+        test::jtx::MPTTester& mpt,
+        test::jtx::Account const& alice,
+        test::jtx::Account const& bob,
+        test::jtx::Account const& carol,
+        test::jtx::Account const& dave,
+        std::uint64_t bobAmt,
+        std::uint64_t carolAmt)
+    {
+        using namespace test::jtx;
+        mpt.create({
+            .ownerCount = 1,
+            .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanHoldConfidentialBalance,
+        });
+        mpt.authorize({.account = bob});
+        mpt.authorize({.account = carol});
+        mpt.authorize({.account = dave});
+
+        if (bobAmt > 0)
+            mpt.pay(alice, bob, bobAmt);
+        if (carolAmt > 0)
+            mpt.pay(alice, carol, carolAmt);
+
+        mpt.generateKeyPair(alice);
+        mpt.generateKeyPair(bob);
+        mpt.generateKeyPair(carol);
+        mpt.generateKeyPair(dave);
+
+        mpt.set({
+            .account = alice,
+            .issuerPubKey = mpt.getPubKey(alice),
+        });
+
+        if (bobAmt > 0)
+        {
+            mpt.convert({
+                .account = bob,
+                .amt = bobAmt,
+                .holderPubKey = mpt.getPubKey(bob),
+            });
+            mpt.mergeInbox({.account = bob});
+        }
+        else
+        {
+            mpt.convert({
+                .account = bob,
+                .amt = 0,
+                .holderPubKey = mpt.getPubKey(bob),
+            });
+        }
+
+        if (carolAmt > 0)
+        {
+            mpt.convert({
+                .account = carol,
+                .amt = carolAmt,
+                .holderPubKey = mpt.getPubKey(carol),
+            });
+            mpt.mergeInbox({.account = carol});
+        }
+        else
+        {
+            mpt.convert({
+                .account = carol,
+                .amt = 0,
+                .holderPubKey = mpt.getPubKey(carol),
+            });
+        }
+
+        // dave: register pubkey only (0 spending/inbox)
+        mpt.convert({
+            .account = dave,
+            .amt = 0,
+            .holderPubKey = mpt.getPubKey(dave),
+        });
+    }
 };
 
 }  // namespace xrpl

src/test/jtx/impl/ConfidentialTransfer.cpp

@@ -1,89 +1,13 @@
 #include <test/jtx/ConfidentialTransfer.h>
 
 #include <test/jtx/Account.h>
-#include <test/jtx/Env.h>
 #include <test/jtx/mpt.h>
 
-#include <xrpl/basics/Buffer.h>
-#include <xrpl/basics/Slice.h>
-#include <xrpl/basics/base_uint.h>
-#include <xrpl/basics/contract.h>
-#include <xrpl/basics/strHex.h>
-#include <xrpl/protocol/ConfidentialTransfer.h>
-#include <xrpl/protocol/Protocol.h>
-#include <xrpl/protocol/TER.h>
-#include <xrpl/protocol/TxFlags.h>
-
-#include <utility/mpt_utility.h>
-
-#include <secp256k1.h>
-#include <secp256k1_mpt.h>
-
-#include <array>
-#include <cstddef>
 #include <cstdint>
-#include <cstring>
-#include <functional>
-#include <optional>
-#include <stdexcept>
-#include <string>
 #include <vector>
 
 namespace xrpl {
 
-ConfidentialTransferTestBase::ConfidentialEnv::ConfidentialEnv(
-    test::jtx::Env& env,
-    test::jtx::Account const& issuer,
-    std::vector<HolderInit> const& holders,
-    std::uint32_t flags,
-    std::optional<test::jtx::Account> auditor)
-    : mpt{env, issuer, {.holders = extractAccounts(holders), .auditor = auditor}}
-{
-    mpt.create({.ownerCount = 1, .flags = flags});
-
-    for (auto const& h : holders)
-    {
-        mpt.authorize({.account = h.account});
-        if ((flags & tfMPTRequireAuth) != 0)
-            mpt.authorize({.account = issuer, .holder = h.account});
-        mpt.pay(issuer, h.account, h.payAmount);
-    }
-
-    mpt.generateKeyPair(issuer);
-    for (auto const& h : holders)
-        mpt.generateKeyPair(h.account);
-    if (auditor)
-        mpt.generateKeyPair(requireOptionalRef(auditor, "Missing auditor"));
-
-    mpt.set({
-        .account = issuer,
-        .issuerPubKey = mpt.getPubKey(issuer),
-        .auditorPubKey = auditor ? mpt.getPubKey(requireOptionalRef(auditor, "Missing auditor"))
-                                 : std::optional<Buffer>{},
-    });
-
-    for (auto const& h : holders)
-    {
-        mpt.convert({
-            .account = h.account,
-            .amt = h.convertAmount,
-            .holderPubKey = mpt.getPubKey(h.account),
-        });
-        mpt.mergeInbox({.account = h.account});
-    }
-}
-
-std::vector<test::jtx::Account>
-ConfidentialTransferTestBase::ConfidentialEnv::extractAccounts(
-    std::vector<HolderInit> const& holders)
-{
-    std::vector<test::jtx::Account> accounts;
-    accounts.reserve(holders.size());
-    for (auto const& h : holders)
-        accounts.push_back(h.account);
-    return accounts;
-}
-
 void
 ConfidentialTransferTestBase::setupConfidentialIssuance(
     test::jtx::MPTTester& mpt,
@@ -110,478 +34,4 @@ ConfidentialTransferTestBase::setupConfidentialIssuance(
         mpt.generateKeyPair(keyOwner);
 }
 
-void
-ConfidentialTransferTestBase::setupBatchEnv(
-    test::jtx::MPTTester& mpt,
-    test::jtx::Account const& alice,
-    test::jtx::Account const& bob,
-    test::jtx::Account const& carol,
-    test::jtx::Account const& dave,
-    std::uint64_t bobAmt,
-    std::uint64_t carolAmt)
-{
-    using namespace test::jtx;
-    mpt.create({
-        .ownerCount = 1,
-        .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanHoldConfidentialBalance,
-    });
-    mpt.authorize({.account = bob});
-    mpt.authorize({.account = carol});
-    mpt.authorize({.account = dave});
-
-    if (bobAmt > 0)
-        mpt.pay(alice, bob, bobAmt);
-    if (carolAmt > 0)
-        mpt.pay(alice, carol, carolAmt);
-
-    mpt.generateKeyPair(alice);
-    mpt.generateKeyPair(bob);
-    mpt.generateKeyPair(carol);
-    mpt.generateKeyPair(dave);
-
-    mpt.set({
-        .account = alice,
-        .issuerPubKey = mpt.getPubKey(alice),
-    });
-
-    if (bobAmt > 0)
-    {
-        mpt.convert({
-            .account = bob,
-            .amt = bobAmt,
-            .holderPubKey = mpt.getPubKey(bob),
-        });
-        mpt.mergeInbox({.account = bob});
-    }
-    else
-    {
-        mpt.convert({
-            .account = bob,
-            .amt = 0,
-            .holderPubKey = mpt.getPubKey(bob),
-        });
-    }
-
-    if (carolAmt > 0)
-    {
-        mpt.convert({
-            .account = carol,
-            .amt = carolAmt,
-            .holderPubKey = mpt.getPubKey(carol),
-        });
-        mpt.mergeInbox({.account = carol});
-    }
-    else
-    {
-        mpt.convert({
-            .account = carol,
-            .amt = 0,
-            .holderPubKey = mpt.getPubKey(carol),
-        });
-    }
-
-    // dave: register pubkey only (0 spending/inbox)
-    mpt.convert({
-        .account = dave,
-        .amt = 0,
-        .holderPubKey = mpt.getPubKey(dave),
-    });
-}
-
-ConfidentialTransferTestBase::ConfidentialSendSetup::ConfidentialSendSetup(
-    test::jtx::MPTTester& mpt,
-    test::jtx::Account const& sender,
-    test::jtx::Account const& dest,
-    test::jtx::Account const& issuer,
-    uint64_t amount,
-    std::optional<std::reference_wrapper<test::jtx::Account const>> auditor)
-    : sendAmount(amount)
-    , version(mpt.getMPTokenVersion(sender))
-    , blindingFactor(generateBlindingFactor())
-    , amountBlindingFactor(blindingFactor)
-    , balanceBlindingFactor(generateBlindingFactor())
-    , senderAmt(mpt.encryptAmount(sender, amount, blindingFactor))
-    , destAmt(mpt.encryptAmount(dest, amount, blindingFactor))
-    , issuerAmt(mpt.encryptAmount(issuer, amount, blindingFactor))
-    , auditorAmt(
-          auditor ? std::optional<Buffer>(mpt.encryptAmount(auditor->get(), amount, blindingFactor))
-                  : std::nullopt)
-    , amountCommitment(mpt.getPedersenCommitment(amount, amountBlindingFactor))
-    , senderPubKey(requireOptional(mpt.getPubKey(sender), "Missing sender public key"))
-    , destPubKey(requireOptional(mpt.getPubKey(dest), "Missing destination public key"))
-    , issuerPubKey(requireOptional(mpt.getPubKey(issuer), "Missing issuer public key"))
-    , auditorPubKey(auditor ? mpt.getPubKey(auditor->get()) : std::nullopt)
-    , prevSpending(requireOptional(
-          mpt.getDecryptedBalance(sender, test::jtx::MPTTester::holderEncryptedSpending),
-          "Missing sender spending balance"))
-    , prevEncryptedSpending(requireOptional(
-          mpt.getEncryptedBalance(sender, test::jtx::MPTTester::holderEncryptedSpending),
-          "Missing sender encrypted spending balance"))
-    , balanceCommitment(mpt.getPedersenCommitment(prevSpending, balanceBlindingFactor))
-{
-    recipients.push_back({
-        .publicKey = Slice(senderPubKey),
-        .encryptedAmount = senderAmt,
-    });
-    recipients.push_back({
-        .publicKey = Slice(destPubKey),
-        .encryptedAmount = destAmt,
-    });
-    recipients.push_back({
-        .publicKey = Slice(issuerPubKey),
-        .encryptedAmount = issuerAmt,
-    });
-    if (auditor)
-    {
-        recipients.push_back({
-            .publicKey = Slice(requireOptionalRef(auditorPubKey, "Missing auditor public key")),
-            .encryptedAmount = requireOptionalRef(auditorAmt, "Missing auditor encrypted amount"),
-        });
-    }
-}
-
-std::optional<Buffer>
-ConfidentialTransferTestBase::ConfidentialSendSetup::generateProof(
-    test::jtx::MPTTester& mpt,
-    test::jtx::Env& env,
-    test::jtx::Account const& sender,
-    test::jtx::Account const& dest) const
-{
-    auto const ctxHash =
-        getSendContextHash(sender.id(), mpt.issuanceID(), env.seq(sender), dest.id(), version);
-
-    return mpt.getConfidentialSendProof(
-        sender,
-        sendAmount,
-        recipients,
-        blindingFactor,
-        ctxHash,
-        {
-            .pedersenCommitment = amountCommitment,
-            .amt = sendAmount,
-            .encryptedAmt = senderAmt,
-            .blindingFactor = amountBlindingFactor,
-        },
-        {
-            .pedersenCommitment = balanceCommitment,
-            .amt = prevSpending,
-            .encryptedAmt = prevEncryptedSpending,
-            .blindingFactor = balanceBlindingFactor,
-        });
-}
-
-test::jtx::MPTConfidentialSend
-ConfidentialTransferTestBase::ConfidentialSendSetup::sendArgs(
-    test::jtx::Account const& sender,
-    test::jtx::Account const& dest,
-    Buffer const& proof,
-    std::optional<TER> err) const
-{
-    return {
-        .account = sender,
-        .dest = dest,
-        .amt = sendAmount,
-        .proof = strHex(proof),
-        .senderEncryptedAmt = senderAmt,
-        .destEncryptedAmt = destAmt,
-        .issuerEncryptedAmt = issuerAmt,
-        .auditorEncryptedAmt = auditorAmt,
-        .amountCommitment = amountCommitment,
-        .balanceCommitment = balanceCommitment,
-        .err = err,
-    };
-}
-
-Buffer const&
-ConfidentialTransferTestBase::getBadCiphertext()
-{
-    static Buffer const kBadCiphertext = []() {
-        Buffer buf(kEcGamalEncryptedTotalLength);
-        std::memset(buf.data(), 0xFF, kEcGamalEncryptedTotalLength);
-
-        buf.data()[0] = kEcCompressedPrefixEvenY;
-        buf.data()[kEcCiphertextComponentLength] = kEcCompressedPrefixEvenY;
-        return buf;
-    }();
-
-    return kBadCiphertext;
-}
-
-Buffer const&
-ConfidentialTransferTestBase::getTrivialCiphertext()
-{
-    static Buffer const kTrivialCiphertext = []() {
-        Buffer buf(kEcGamalEncryptedTotalLength);
-        std::memset(buf.data(), 0, kEcGamalEncryptedTotalLength);
-
-        buf.data()[0] = kEcCompressedPrefixEvenY;
-        buf.data()[kEcCiphertextComponentLength] = kEcCompressedPrefixEvenY;
-
-        buf.data()[kEcCiphertextComponentLength - 1] = 0x01;
-        buf.data()[kEcGamalEncryptedTotalLength - 1] = 0x01;
-
-        return buf;
-    }();
-
-    return kTrivialCiphertext;
-}
-
-Buffer const&
-ConfidentialTransferTestBase::getTrivialCommitment()
-{
-    static Buffer const kTrivialCommitment = []() {
-        Buffer buf(kEcPedersenCommitmentLength);
-        std::memset(buf.data(), 0, kEcPedersenCommitmentLength);
-
-        buf.data()[0] = kEcCompressedPrefixEvenY;
-        // Set last byte to make it a valid x-coordinate on the curve
-        buf.data()[kEcPedersenCommitmentLength - 1] = 0x01;
-
-        return buf;
-    }();
-
-    return kTrivialCommitment;
-}
-
-std::string
-ConfidentialTransferTestBase::getTrivialSendProofHex()
-{
-    Buffer buf(kEcSendProofLength);
-    std::memset(buf.data(), 0, kEcSendProofLength);
-
-    for (std::size_t i = 0; i < kEcSendProofLength; i += kEcCiphertextComponentLength)
-    {
-        buf.data()[i] = kEcCompressedPrefixEvenY;
-        if (i + kEcCiphertextComponentLength - 1 < kEcSendProofLength)
-            buf.data()[i + kEcCiphertextComponentLength - 1] = 0x01;
-    }
-
-    return strHex(buf);
-}
-
-Buffer
-ConfidentialTransferTestBase::getForgedBulletproof(
-    std::array<uint64_t, 2> const& values,
-    std::array<Buffer, 2> const& blindingFactors,
-    uint256 const& contextHash)
-{
-    auto* const ctx = mpt_secp256k1_context();
-
-    secp256k1_pubkey h;
-    secp256k1_mpt_get_h_generator(ctx, &h);
-
-    Buffer proof(kEcDoubleBulletproofLength);
-    size_t proofLen = kEcDoubleBulletproofLength;
-
-    unsigned char blindings[64];
-    std::memcpy(blindings, blindingFactors[0].data(), 32);
-    std::memcpy(blindings + 32, blindingFactors[1].data(), 32);
-
-    if (secp256k1_bulletproof_prove_agg(
-            ctx, proof.data(), &proofLen, values.data(), blindings, 2, &h, contextHash.data()) == 0)
-        Throw<std::runtime_error>("Failed to generate forged bulletproof");
-
-    return proof;
-}
-
-Buffer
-ConfidentialTransferTestBase::getForgedSingleBulletproof(
-    uint64_t value,
-    Buffer const& blindingFactor,
-    uint256 const& contextHash)
-{
-    auto* const ctx = mpt_secp256k1_context();
-
-    secp256k1_pubkey h;
-    secp256k1_mpt_get_h_generator(ctx, &h);
-
-    Buffer proof(kEcSingleBulletproofLength);
-    size_t proofLen = kEcSingleBulletproofLength;
-
-    if (secp256k1_bulletproof_prove_agg(
-            ctx,
-            proof.data(),
-            &proofLen,
-            &value,
-            blindingFactor.data(),
-            1,  // m = 1 (single bulletproof)
-            &h,
-            contextHash.data()) == 0)
-        Throw<std::runtime_error>("Failed to generate forged single bulletproof");
-
-    return proof;
-}
-
-Buffer
-ConfidentialTransferTestBase::getForgedConvertBackProof(
-    test::jtx::MPTTester& mpt,
-    test::jtx::Account const& holder,
-    uint64_t claimedBalance,
-    uint64_t realBalance,
-    uint64_t amt,
-    Buffer const& pedersenCommitment,
-    Buffer const& encryptedSpendingBalance,
-    Buffer const& pcBlindingFactor,
-    uint256 const& contextHash)
-{
-    if (pedersenCommitment.size() != kCompressedEcPointLength)
-        Throw<std::runtime_error>("getForgedConvertBackProof: bad pedersenCommitment length");
-    if (encryptedSpendingBalance.size() != kEcGamalEncryptedTotalLength)
-    {
-        Throw<std::runtime_error>("getForgedConvertBackProof: bad encryptedSpendingBalance length");
-    }
-    if (amt > realBalance)
-        Throw<std::runtime_error>("getForgedConvertBackProof: amt exceeds realBalance");
-
-    auto* const ctx = mpt_secp256k1_context();
-    auto const holderPubKey = requireOptional(mpt.getPubKey(holder), "Missing holder pubkey");
-    auto const holderPrivKey = requireOptional(mpt.getPrivKey(holder), "Missing holder privkey");
-
-    secp256k1_pubkey pkHolder;
-    if (secp256k1_ec_pubkey_parse(ctx, &pkHolder, holderPubKey.data(), kCompressedEcPointLength) !=
-        1)
-        Throw<std::runtime_error>("Failed to parse holder's public key");
-
-    secp256k1_pubkey pcB;
-    if (secp256k1_ec_pubkey_parse(ctx, &pcB, pedersenCommitment.data(), kCompressedEcPointLength) !=
-        1)
-        Throw<std::runtime_error>("Failed to parse pedersen commitment");
-
-    secp256k1_pubkey b1, b2;
-    if (secp256k1_ec_pubkey_parse(
-            ctx, &b1, encryptedSpendingBalance.data(), kCompressedEcPointLength) != 1 ||
-        secp256k1_ec_pubkey_parse(
-            ctx,
-            &b2,
-            encryptedSpendingBalance.data() + kCompressedEcPointLength,
-            kCompressedEcPointLength) != 1)
-        Throw<std::runtime_error>("Failed to parse balance ciphertext");
-
-    Buffer sigmaProof(SECP256K1_COMPACT_CONVERTBACK_PROOF_SIZE);
-    if (secp256k1_compact_convertback_prove(
-            ctx,
-            sigmaProof.data(),
-            claimedBalance,
-            holderPrivKey.data(),
-            pcBlindingFactor.data(),
-            &pkHolder,
-            &b1,
-            &b2,
-            &pcB,
-            contextHash.data()) != 1)
-        Throw<std::runtime_error>("Failed to generate convertback sigma proof");
-
-    auto const forgedBulletproof =
-        getForgedSingleBulletproof(realBalance - amt, pcBlindingFactor, contextHash);
-
-    Buffer proof(kEcConvertBackProofLength);
-    std::memcpy(proof.data(), sigmaProof.data(), SECP256K1_COMPACT_CONVERTBACK_PROOF_SIZE);
-    std::memcpy(
-        proof.data() + SECP256K1_COMPACT_CONVERTBACK_PROOF_SIZE,
-        forgedBulletproof.data(),
-        kEcSingleBulletproofLength);
-
-    return proof;
-}
-
-Buffer
-ConfidentialTransferTestBase::getForgedSendProof(
-    test::jtx::MPTTester& mpt,
-    test::jtx::Env& env,
-    test::jtx::Account const& sender,
-    test::jtx::Account const& dest,
-    ConfidentialSendSetup const& setup)
-{
-    auto* const ctx = mpt_secp256k1_context();
-
-    secp256k1_pubkey c1;
-    std::vector<secp256k1_pubkey> c2Vec(setup.recipients.size());
-    std::vector<secp256k1_pubkey> pkVec(setup.recipients.size());
-    for (std::size_t i = 0; i < setup.recipients.size(); ++i)
-    {
-        auto const& r = setup.recipients[i];
-        if (i == 0 &&
-            secp256k1_ec_pubkey_parse(
-                ctx, &c1, r.encryptedAmount.data(), kCompressedEcPointLength) != 1)
-            Throw<std::runtime_error>("Failed to parse C1");
-        if (secp256k1_ec_pubkey_parse(
-                ctx,
-                &c2Vec[i],
-                r.encryptedAmount.data() + kCompressedEcPointLength,
-                kCompressedEcPointLength) != 1)
-            Throw<std::runtime_error>("Failed to parse C2");
-        if (secp256k1_ec_pubkey_parse(
-                ctx, &pkVec[i], r.publicKey.data(), kCompressedEcPointLength) != 1)
-            Throw<std::runtime_error>("Failed to parse recipient pubkey");
-    }
-
-    secp256k1_pubkey pkSender, pcAmount, pcBalance, b1, b2;
-    if (secp256k1_ec_pubkey_parse(
-            ctx, &pkSender, setup.senderPubKey.data(), kCompressedEcPointLength) != 1 ||
-        secp256k1_ec_pubkey_parse(
-            ctx, &pcAmount, setup.amountCommitment.data(), kCompressedEcPointLength) != 1 ||
-        secp256k1_ec_pubkey_parse(
-            ctx, &pcBalance, setup.balanceCommitment.data(), kCompressedEcPointLength) != 1 ||
-        secp256k1_ec_pubkey_parse(
-            ctx, &b1, setup.prevEncryptedSpending.data(), kCompressedEcPointLength) != 1 ||
-        secp256k1_ec_pubkey_parse(
-            ctx,
-            &b2,
-            setup.prevEncryptedSpending.data() + kCompressedEcPointLength,
-            kCompressedEcPointLength) != 1)
-        Throw<std::runtime_error>("Failed to parse commitments/ciphertext");
-
-    Buffer const senderPrivKey = requireOptional(mpt.getPrivKey(sender), "Missing sender privkey");
-    auto const ctxHash = getSendContextHash(
-        sender.id(), mpt.issuanceID(), env.seq(sender), dest.id(), setup.version);
-
-    Buffer sigmaProof(SECP256K1_COMPACT_STANDARD_PROOF_SIZE);
-    if (secp256k1_compact_standard_prove(
-            ctx,
-            sigmaProof.data(),
-            setup.sendAmount,
-            setup.prevSpending,
-            setup.blindingFactor.data(),
-            senderPrivKey.data(),
-            setup.balanceBlindingFactor.data(),
-            setup.recipients.size(),
-            &c1,
-            c2Vec.data(),
-            pkVec.data(),
-            &pcAmount,
-            &pkSender,
-            &pcBalance,
-            &b1,
-            &b2,
-            ctxHash.data()) != 1)
-        Throw<std::runtime_error>("Failed to generate sigma proof");
-
-    // Wraps (mod 2^64) for overdrafts, unlike the ledger's own homomorphic
-    // commitment subtraction (mod the curve order) — that mismatch is
-    // exactly what makes the forged proof fail verification.
-    // Computed without a wrapping `uint64` subtract: Clang UBSan treats
-    // unsigned overflow as fatal (see incrementConfidentialVersion).
-    std::uint64_t const remaining = setup.sendAmount <= setup.prevSpending
-        ? setup.prevSpending - setup.sendAmount
-        : ~setup.sendAmount + setup.prevSpending + 1;
-
-    Buffer negAmountBf(kEcBlindingFactorLength);
-    Buffer remainingBf(kEcBlindingFactorLength);
-    secp256k1_mpt_scalar_negate(negAmountBf.data(), setup.amountBlindingFactor.data());
-    secp256k1_mpt_scalar_add(
-        remainingBf.data(), setup.balanceBlindingFactor.data(), negAmountBf.data());
-
-    auto const forgedBulletproof = getForgedBulletproof(
-        {setup.sendAmount, remaining}, {setup.amountBlindingFactor, remainingBf}, ctxHash);
-
-    Buffer combinedProof(kEcSendProofLength);
-    std::memcpy(combinedProof.data(), sigmaProof.data(), SECP256K1_COMPACT_STANDARD_PROOF_SIZE);
-    std::memcpy(
-        combinedProof.data() + SECP256K1_COMPACT_STANDARD_PROOF_SIZE,
-        forgedBulletproof.data(),
-        kEcDoubleBulletproofLength);
-
-    return combinedProof;
-}
-
 }  // namespace xrpl

src/test/rpc/LedgerRPC_test.cpp

@@ -13,21 +13,17 @@
 #include <test/jtx/ter.h>
 
 #include <xrpld/app/misc/TxQ.h>
-#include <xrpld/rpc/CTID.h>
 
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/config/Constants.h>
-#include <xrpl/core/NetworkIDService.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/json/to_string.h>
 #include <xrpl/protocol/ErrorCodes.h>
-#include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
 #include <xrpl/protocol/jss.h>
 
-#include <cstdint>
 #include <memory>
 #include <string>
 #include <utility>
@@ -811,95 +807,6 @@ class LedgerRPC_test : public beast::unit_test::Suite
         }
     }
 
-    void
-    testLedgerExpandedTransactionsCTID()
-    {
-        testcase("Expanded Transactions CTID");
-        using namespace test::jtx;
-
-        Env env{*this};
-        Account const alice{"alice"};
-        env.fund(XRP(10000), alice);
-        env.close();
-
-        uint32_t const netID = env.app().getNetworkIDService().getNetworkID();
-
-        // API v2 non-binary: CTID present
-        {
-            json::Value jvParams;
-            jvParams[jss::ledger_index] = "validated";
-            jvParams[jss::transactions] = true;
-            jvParams[jss::expand] = true;
-            jvParams[jss::api_version] = 2;
-            auto const jrr = env.rpc("json", "ledger", to_string(jvParams))[jss::result];
-            BEAST_EXPECT(jrr[jss::status] == "success");
-            auto const& txns = jrr[jss::ledger][jss::transactions];
-            BEAST_EXPECT(txns.isArray() && txns.size() > 0);
-            for (auto const& txn : txns)
-            {
-                BEAST_EXPECT(txn.isMember(jss::ctid));
-                auto const expectedCtid = rpc::encodeCTID(
-                    jrr[jss::ledger][jss::ledger_index].asUInt(),
-                    txn[jss::meta][sfTransactionIndex.jsonName].asUInt(),
-                    netID);
-                // NOLINTBEGIN(bugprone-unchecked-optional-access)
-                if (BEAST_EXPECT(expectedCtid.has_value()))
-                    BEAST_EXPECT(txn[jss::ctid] == expectedCtid.value());
-                // NOLINTEND(bugprone-unchecked-optional-access)
-            }
-        }
-
-        // API v1 non-binary: CTID present
-        {
-            json::Value jvParams;
-            jvParams[jss::ledger_index] = "validated";
-            jvParams[jss::transactions] = true;
-            jvParams[jss::expand] = true;
-            auto const jrr = env.rpc("json", "ledger", to_string(jvParams))[jss::result];
-            BEAST_EXPECT(jrr[jss::status] == "success");
-            auto const& txns = jrr[jss::ledger][jss::transactions];
-            BEAST_EXPECT(txns.isArray() && txns.size() > 0);
-            for (auto const& txn : txns)
-            {
-                BEAST_EXPECT(txn.isMember(jss::ctid));
-            }
-        }
-
-        // Binary expanded: CTID present
-        {
-            json::Value jvParams;
-            jvParams[jss::ledger_index] = "validated";
-            jvParams[jss::transactions] = true;
-            jvParams[jss::expand] = true;
-            jvParams[jss::binary] = true;
-            jvParams[jss::api_version] = 2;
-            auto const jrr = env.rpc("json", "ledger", to_string(jvParams))[jss::result];
-            BEAST_EXPECT(jrr[jss::status] == "success");
-            auto const& txns = jrr[jss::ledger][jss::transactions];
-            BEAST_EXPECT(txns.isArray() && txns.size() > 0);
-            for (auto const& txn : txns)
-            {
-                BEAST_EXPECT(txn.isMember(jss::ctid));
-            }
-        }
-
-        // Non-expanded: transactions are plain hash strings, no CTID
-        {
-            json::Value jvParams;
-            jvParams[jss::ledger_index] = "validated";
-            jvParams[jss::transactions] = true;
-            jvParams[jss::api_version] = 2;
-            auto const jrr = env.rpc("json", "ledger", to_string(jvParams))[jss::result];
-            BEAST_EXPECT(jrr[jss::status] == "success");
-            auto const& txns = jrr[jss::ledger][jss::transactions];
-            BEAST_EXPECT(txns.isArray() && txns.size() > 0);
-            for (auto const& txn : txns)
-            {
-                BEAST_EXPECT(txn.isString());
-            }
-        }
-    }
-
 public:
     void
     run() override
@@ -915,7 +822,6 @@ public:
         testNoQueue();
         testQueue();
         testLedgerAccountsOption();
-        testLedgerExpandedTransactionsCTID();
     }
 };
 

src/tests/libxrpl/basics/MallocTrim.cpp

@@ -49,7 +49,7 @@ TEST(MallocTrimReport, structure)
 }
 
 #if defined(__GLIBC__) && BOOST_OS_LINUX
-TEST(ParseStatmRSSkB, standard_format)
+TEST(parseStatmRSSkB, standard_format)
 {
     using xrpl::detail::parseStatmRSSkB;
 
@@ -121,7 +121,7 @@ TEST(ParseStatmRSSkB, standard_format)
 }
 #endif
 
-TEST(MallocTrim, without_debug_logging)
+TEST(mallocTrim, without_debug_logging)
 {
     beast::Journal const journal{beast::Journal::getNullSink()};
 
@@ -144,7 +144,7 @@ TEST(MallocTrim, without_debug_logging)
 #endif
 }
 
-TEST(MallocTrim, empty_tag)
+TEST(mallocTrim, empty_tag)
 {
     beast::Journal const journal{beast::Journal::getNullSink()};
     MallocTrimReport const report = mallocTrim("", journal);
@@ -157,7 +157,7 @@ TEST(MallocTrim, empty_tag)
 #endif
 }
 
-TEST(MallocTrim, with_debug_logging)
+TEST(mallocTrim, with_debug_logging)
 {
     struct DebugSink : public beast::Journal::Sink
     {
@@ -194,7 +194,7 @@ TEST(MallocTrim, with_debug_logging)
 #endif
 }
 
-TEST(MallocTrim, repeated_calls)
+TEST(mallocTrim, repeated_calls)
 {
     beast::Journal const journal{beast::Journal::getNullSink()};
 

src/tests/libxrpl/basics/RangeSet.cpp

@@ -10,7 +10,7 @@
 
 using namespace xrpl;
 
-TEST(RangeSet, prev_missing)
+TEST(RangeSet, prevMissing)
 {
     // Set will include:
     // [ 0, 5]
@@ -36,7 +36,7 @@ TEST(RangeSet, prev_missing)
     }
 }
 
-TEST(RangeSet, to_string)
+TEST(RangeSet, toString)
 {
     RangeSet<std::uint32_t> set;
     EXPECT_EQ(to_string(set), "empty");
@@ -54,7 +54,7 @@ TEST(RangeSet, to_string)
     EXPECT_EQ(to_string(set), "1-2,6");
 }
 
-TEST(RangeSet, from_string)
+TEST(RangeSet, fromString)
 {
     RangeSet<std::uint32_t> set;
 

src/tests/libxrpl/basics/StringUtilities.cpp

@@ -290,7 +290,7 @@ TEST_F(StringUtilitiesTest, to_string)
     EXPECT_EQ(result, "hello");
 }
 
-TEST_F(StringUtilitiesTest, trim_whitespace)
+TEST_F(StringUtilitiesTest, trimWhitespace)
 {
     EXPECT_EQ(trimWhitespace(""), "");
     EXPECT_EQ(trimWhitespace("   "), "");
@@ -303,7 +303,7 @@ TEST_F(StringUtilitiesTest, trim_whitespace)
     EXPECT_EQ(trimWhitespace("  a b\tc  "), "a b\tc");
 }
 
-TEST_F(StringUtilitiesTest, to_lower)
+TEST_F(StringUtilitiesTest, toLower)
 {
     EXPECT_EQ(toLower(""), "");
     EXPECT_EQ(toLower("ABC"), "abc");
@@ -318,7 +318,7 @@ TEST_F(StringUtilitiesTest, to_lower)
 // Both helpers are documented as depending only on their input. Guard that by
 // checking the bytes just outside ASCII, which a locale-aware isspace/tolower
 // could classify differently.
-TEST_F(StringUtilitiesTest, trim_and_lower_ignore_locale)
+TEST_F(StringUtilitiesTest, trimAndLowerIgnoreLocale)
 {
     // 0xA0 is NO-BREAK SPACE in Latin-1 and is whitespace to some locales.
     std::string const nbsp("\xA0", 1);

src/tests/libxrpl/basics/base64.cpp

@@ -14,7 +14,7 @@ check(std::string const& in, std::string const& out)
     EXPECT_EQ(base64Decode(encoded), in);
 }
 
-TEST(Base64, base64)
+TEST(base64, base64)
 {
     // cspell: disable
     check("", "");

src/tests/libxrpl/basics/base_uint.cpp

@@ -128,7 +128,7 @@ struct BaseUintTest : public ::testing::Test
 
 using BaseUintDeathTest = BaseUintTest;
 
-TEST_F(BaseUintDeathTest, from_raw_size_mismatch)
+TEST_F(BaseUintDeathTest, fromRaw_size_mismatch)
 {
     // ENABLE_VOIDSTAR is a debug build, but does not crash on failed asserts. Rather than twist
     // these tests into knots to make them work, just skip them.

src/tests/libxrpl/basics/contract.cpp

@@ -6,7 +6,7 @@
 
 using namespace xrpl;
 
-TEST(Contract, contract)
+TEST(contract, contract)
 {
     try
     {

src/tests/libxrpl/basics/mulDiv.cpp

@@ -7,7 +7,7 @@
 
 using namespace xrpl;
 
-TEST(MulDiv, mul_div)
+TEST(mulDiv, mulDiv)
 {
     auto const max = std::numeric_limits<std::uint64_t>::max();
     std::uint64_t const max32 = std::numeric_limits<std::uint32_t>::max();

src/tests/libxrpl/basics/scope.cpp

@@ -6,7 +6,7 @@
 
 using namespace xrpl;
 
-TEST(Scope, scope_exit)
+TEST(scope, ScopeExit)
 {
     // ScopeExit always executes the functor on destruction,
     // unless release() is called
@@ -56,7 +56,7 @@ TEST(Scope, scope_exit)
     EXPECT_EQ(i, 5);
 }
 
-TEST(Scope, scope_fail)
+TEST(scope, ScopeFail)
 {
     // ScopeFail executes the functor on destruction only
     // if an exception is unwinding, unless release() is called
@@ -106,7 +106,7 @@ TEST(Scope, scope_fail)
     EXPECT_EQ(i, 5);
 }
 
-TEST(Scope, scope_success)
+TEST(scope, ScopeSuccess)
 {
     // ScopeSuccess executes the functor on destruction only
     // if an exception is not unwinding, unless release() is called

src/tests/libxrpl/basics/tagged_integer.cpp

@@ -105,7 +105,7 @@ static_assert(
 
 using TagInt = TaggedInteger<std::int32_t, Tag1>;
 
-TEST(TaggedInteger, comparison_operators)
+TEST(tagged_integer, comparison_operators)
 {
     TagInt const zero(0);
     TagInt const one(1);
@@ -131,7 +131,7 @@ TEST(TaggedInteger, comparison_operators)
     EXPECT_FALSE(one <= zero);
 }
 
-TEST(TaggedInteger, increment_decrement_operators)
+TEST(tagged_integer, increment_decrement_operators)
 {
     TagInt const zero(0);
     TagInt const one(1);
@@ -146,7 +146,7 @@ TEST(TaggedInteger, increment_decrement_operators)
     EXPECT_EQ(a, zero);
 }
 
-TEST(TaggedInteger, arithmetic_operators)
+TEST(tagged_integer, arithmetic_operators)
 {
     TagInt const a{-2};
     EXPECT_EQ(+a, TagInt{-2});
@@ -166,7 +166,7 @@ TEST(TaggedInteger, arithmetic_operators)
     EXPECT_EQ((TagInt{16} >> TagInt{2}), TagInt{4});
 }
 
-TEST(TaggedInteger, assignment_operators)
+TEST(tagged_integer, assignment_operators)
 {
     TagInt a{-2};
     TagInt b{0};

src/tests/libxrpl/crypto/csprng.cpp

@@ -6,7 +6,7 @@
 
 using namespace xrpl;
 
-TEST(Csprng, get_values)
+TEST(csprng, get_values)
 {
     auto& engine = cryptoPrng();
     auto randVal = engine();

src/tests/libxrpl/json/Value.cpp

@@ -21,7 +21,7 @@
 
 namespace xrpl {
 
-TEST(JsonValue, limits)
+TEST(json_value, limits)
 {
     using namespace json;
     static_assert(Value::kMinInt == Int(~(UInt(-1) / 2)));
@@ -29,7 +29,7 @@ TEST(JsonValue, limits)
     static_assert(Value::kMaxUInt == UInt(-1));
 }
 
-TEST(JsonValue, construct_and_compare_json_static_string)
+TEST(json_value, construct_and_compare_Json_StaticString)
 {
     static constexpr char kSample[]{"Contents of a json::StaticString"};
 
@@ -52,7 +52,7 @@ TEST(JsonValue, construct_and_compare_json_static_string)
     EXPECT_NE(kTest3, str);
 }
 
-TEST(JsonValue, different_types)
+TEST(json_value, different_types)
 {
     // Exercise ValueType constructor
     static constexpr json::StaticString kStaticStr{"staticStr"};
@@ -206,7 +206,7 @@ TEST(JsonValue, different_types)
     }
 }
 
-TEST(JsonValue, compare_strings)
+TEST(json_value, compare_strings)
 {
     auto doCompare = [&](json::Value const& lhs,
                          json::Value const& rhs,
@@ -560,7 +560,7 @@ TEST(JsonValue, compare_strings)
 #pragma pop_macro("DO_COMPARE")
 }
 
-TEST(JsonValue, bool)
+TEST(json_value, bool)
 {
     EXPECT_FALSE(json::Value());
 
@@ -583,7 +583,7 @@ TEST(JsonValue, bool)
     EXPECT_TRUE(bool(object));
 }
 
-TEST(JsonValue, bad_json)
+TEST(json_value, bad_json)
 {
     char const* s(R"({"method":"ledger","params":[{"ledger_index":1e300}]})");
 
@@ -607,7 +607,7 @@ parseValue(std::string const& doc)
 
 }  // namespace
 
-TEST(JsonValue, parse_double_valid)
+TEST(json_value, parse_double_valid)
 {
     // 1e300 is large but still representable, so it parses (unlike the out-of-range cases below).
     for (auto const& [text, expected] :
@@ -627,14 +627,14 @@ TEST(JsonValue, parse_double_valid)
     }
 }
 
-TEST(JsonValue, parse_double_out_of_range)
+TEST(json_value, parse_double_out_of_range)
 {
     // Magnitudes with no finite double representation are rejected.
     for (char const* oor : {"1e400", "-1e400", "0.001e500", "1e-400", "-1e-400", "123e-500"})
         EXPECT_FALSE(parseValue(oor).has_value()) << oor;
 }
 
-TEST(JsonValue, parse_double_malformed)
+TEST(json_value, parse_double_malformed)
 {
     // readNumber() collects any run of digits and '.eE+-' into a single Double
     // token, so these malformed tokens reach decodeDouble. Each has a valid
@@ -644,7 +644,7 @@ TEST(JsonValue, parse_double_malformed)
         EXPECT_FALSE(parseValue(bad).has_value()) << bad;
 }
 
-TEST(JsonValue, edge_cases)
+TEST(json_value, edge_cases)
 {
     std::uint32_t const maxUInt = std::numeric_limits<std::uint32_t>::max();
     std::int32_t const maxInt = std::numeric_limits<std::int32_t>::max();
@@ -791,7 +791,7 @@ TEST(JsonValue, edge_cases)
     }
 }
 
-TEST(JsonValue, copy)
+TEST(json_value, copy)
 {
     json::Value v1{2.5};
     EXPECT_TRUE(v1.isDouble());
@@ -812,7 +812,7 @@ TEST(JsonValue, copy)
     EXPECT_EQ(v1, v2);
 }
 
-TEST(JsonValue, move)
+TEST(json_value, move)
 {
     json::Value v1{2.5};
     EXPECT_TRUE(v1.isDouble());
@@ -831,7 +831,7 @@ TEST(JsonValue, move)
     EXPECT_NE(v1, v2);  // NOLINT(bugprone-use-after-move)
 }
 
-TEST(JsonValue, comparisons)
+TEST(json_value, comparisons)
 {
     json::Value a, b;
     auto testEquals = [&](std::string const& name) {
@@ -886,7 +886,7 @@ TEST(JsonValue, comparisons)
     testGreaterThan("big");
 }
 
-TEST(JsonValue, compact)
+TEST(json_value, compact)
 {
     json::Value j;
     json::Reader r;
@@ -909,7 +909,7 @@ TEST(JsonValue, compact)
     }
 }
 
-TEST(JsonValue, conversions)
+TEST(json_value, conversions)
 {
     // We have json::ValueType::Real but json::Value::asDouble.
     // TODO: What's the thinking here?
@@ -1125,7 +1125,7 @@ TEST(JsonValue, conversions)
     }
 }
 
-TEST(JsonValue, access_members)
+TEST(json_value, access_members)
 {
     json::Value val;
     EXPECT_EQ(val.type(), json::ValueType::Null);
@@ -1218,7 +1218,7 @@ TEST(JsonValue, access_members)
     }
 }
 
-TEST(JsonValue, remove_members)
+TEST(json_value, remove_members)
 {
     json::Value val;
     EXPECT_EQ(val.removeMember(std::string("member")).type(), json::ValueType::Null);
@@ -1245,7 +1245,7 @@ TEST(JsonValue, remove_members)
     EXPECT_EQ(val.size(), 0);
 }
 
-TEST(JsonValue, iterator)
+TEST(json_value, iterator)
 {
     {
         // Iterating an array.
@@ -1331,7 +1331,7 @@ TEST(JsonValue, iterator)
     }
 }
 
-TEST(JsonValue, nest_limits)
+TEST(json_value, nest_limits)
 {
     json::Reader r;
     {
@@ -1377,7 +1377,7 @@ TEST(JsonValue, nest_limits)
     }
 }
 
-TEST(JsonValue, memory_leak)
+TEST(json_value, memory_leak)
 {
     // When run with the address sanitizer, this test confirms there is no
     // memory leak with the scenarios below.

src/tests/libxrpl/ledger/AMMEntry.cpp

@@ -10,7 +10,7 @@
 
 namespace xrpl::test {
 
-TEST(AMMEntryTests, constructors)
+TEST(AMMEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/AccountRootEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(AccountRootEntryTests, constructors)
+TEST(AccountRootEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/AmendmentsEntry.cpp

@@ -7,7 +7,7 @@
 
 namespace xrpl::test {
 
-TEST(AmendmentsEntryTests, constructors)
+TEST(AmendmentsEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/BridgeEntry.cpp

@@ -11,7 +11,7 @@
 
 namespace xrpl::test {
 
-TEST(BridgeEntryTests, constructors)
+TEST(BridgeEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/CheckEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(CheckEntryTests, constructors)
+TEST(CheckEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/CredentialEntry.cpp

@@ -11,7 +11,7 @@
 
 namespace xrpl::test {
 
-TEST(CredentialEntryTests, constructors)
+TEST(CredentialEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/DIDEntry.cpp

@@ -7,7 +7,7 @@
 
 namespace xrpl::test {
 
-TEST(DIDEntryTests, constructors)
+TEST(DIDEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/DelegateEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(DelegateEntryTests, constructors)
+TEST(DelegateEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/DepositPreauthEntry.cpp

@@ -14,7 +14,7 @@
 
 namespace xrpl::test {
 
-TEST(DepositPreauthEntryTests, constructors)
+TEST(DepositPreauthEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/DirectoryNodeEntry.cpp

@@ -10,7 +10,7 @@
 
 namespace xrpl::test {
 
-TEST(DirectoryNodeEntryTests, constructors)
+TEST(DirectoryNodeEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/EscrowEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(EscrowEntryTests, constructors)
+TEST(EscrowEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/FeeSettingsEntry.cpp

@@ -7,7 +7,7 @@
 
 namespace xrpl::test {
 
-TEST(FeeSettingsEntryTests, constructors)
+TEST(FeeSettingsEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/LedgerHashesEntry.cpp

@@ -7,7 +7,7 @@
 
 namespace xrpl::test {
 
-TEST(LedgerHashesEntryTests, constructors)
+TEST(LedgerHashesEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/LoanBrokerEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(LoanBrokerEntryTests, constructors)
+TEST(LoanBrokerEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/LoanEntry.cpp

@@ -9,7 +9,7 @@
 
 namespace xrpl::test {
 
-TEST(LoanEntryTests, constructors)
+TEST(LoanEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/MPTokenEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(MPTokenEntryTests, constructors)
+TEST(MPTokenEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/MPTokenIssuanceEntry.cpp

@@ -10,7 +10,7 @@
 
 namespace xrpl::test {
 
-TEST(MPTokenIssuanceEntryTests, constructors)
+TEST(MPTokenIssuanceEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/NFTokenOfferEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(NFTokenOfferEntryTests, constructors)
+TEST(NFTokenOfferEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/NFTokenPageEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(NFTokenPageEntryTests, constructors)
+TEST(NFTokenPageEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/NegativeUNLEntry.cpp

@@ -7,7 +7,7 @@
 
 namespace xrpl::test {
 
-TEST(NegativeUNLEntryTests, constructors)
+TEST(NegativeUNLEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/OfferEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(OfferEntryTests, constructors)
+TEST(OfferEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/OracleEntry.cpp

@@ -9,7 +9,7 @@
 
 namespace xrpl::test {
 
-TEST(OracleEntryTests, constructors)
+TEST(OracleEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/PayChannelEntry.cpp

@@ -9,7 +9,7 @@
 
 namespace xrpl::test {
 
-TEST(PayChannelEntryTests, constructors)
+TEST(PayChannelEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/PermissionedDomainEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(PermissionedDomainEntryTests, constructors)
+TEST(PermissionedDomainEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/RippleStateEntry.cpp

@@ -10,7 +10,7 @@
 
 namespace xrpl::test {
 
-TEST(RippleStateEntryTests, constructors)
+TEST(RippleStateEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/SLEBase.cpp

@@ -149,7 +149,7 @@ protected:
     }
 };
 
-TEST_F(SLEBaseTests, read_only)
+TEST_F(SLEBaseTests, ReadOnly)
 {
     AccountRootEntryR const absent(bob_.id(), env_.getClosedLedger());
     EXPECT_FALSE(absent.exists());
@@ -168,7 +168,7 @@ TEST_F(SLEBaseTests, read_only)
     EXPECT_EQ(&present.readView(), &env_.getClosedLedger());
 }
 
-TEST_F(SLEBaseTests, adopt_sle)
+TEST_F(SLEBaseTests, AdoptSLE)
 {
     auto const sle = env_.getClosedLedger().read(keylet::account(alice_.id()));
     ASSERT_NE(sle, nullptr);
@@ -201,7 +201,7 @@ TEST_F(SLEBaseTests, adopt_sle)
         "writable entries must not be constructible from a bare SLE");
 }
 
-TEST_F(SLEBaseTests, writable_accessors)
+TEST_F(SLEBaseTests, WritableAccessors)
 {
     ApplyViewImpl av(&env_.getClosedLedger(), TapNone);
     beast::Journal const j{beast::Journal::getNullSink()};
@@ -236,7 +236,7 @@ TEST_F(SLEBaseTests, writable_accessors)
         !HasApplyView<AccountRootEntryR>, "applyView() must not exist on a read-only entry");
 }
 
-TEST_F(SLEBaseTests, apply_view_context_ctor)
+TEST_F(SLEBaseTests, ApplyViewContextCtor)
 {
     ApplyViewImpl av(&env_.getClosedLedger(), TapNone);
     beast::Journal const j{beast::Journal::getNullSink()};
@@ -260,7 +260,7 @@ TEST_F(SLEBaseTests, apply_view_context_ctor)
     EXPECT_EQ(fromCtx.rawSle(), fromView.rawSle());
 }
 
-TEST_F(SLEBaseTests, writable_lifecycle)
+TEST_F(SLEBaseTests, WritableLifecycle)
 {
     // A view we never apply, so nothing here reaches the ledger.
     ApplyViewImpl av(&env_.getClosedLedger(), TapNone);
@@ -318,7 +318,7 @@ TEST_F(SLEBaseTests, writable_lifecycle)
     }
 }
 
-TEST_F(SLEBaseTests, conversion)
+TEST_F(SLEBaseTests, Conversion)
 {
     ApplyViewImpl av(&env_.getClosedLedger(), TapNone);
 
@@ -336,7 +336,7 @@ TEST_F(SLEBaseTests, conversion)
     EXPECT_EQ(generic.type(), ltACCOUNT_ROOT);
 }
 
-TEST_F(SLEBaseTests, resolve_entry_peeks)
+TEST_F(SLEBaseTests, ResolveEntryPeeks)
 {
     // getOpenLedger() is an OpenView, which derives from ReadView but not
     // from ApplyView, so resolveEntry's dynamic_cast fails and this takes
@@ -368,7 +368,7 @@ TEST_F(SLEBaseTests, resolve_entry_peeks)
     EXPECT_EQ(readOnly->getFieldU32(sfSequence), bumped);
 }
 
-TEST_F(SLEBaseTests, throws_on_missing_entry)
+TEST_F(SLEBaseTests, ThrowsOnMissingEntry)
 {
     // A generic read-only entry has no static type to fall back on, so
     // type() must read it off the (absent) SLE and throw.
@@ -389,7 +389,7 @@ TEST_F(SLEBaseTests, throws_on_missing_entry)
     EXPECT_THROW(std::ignore = (*missing).getType(), std::logic_error);
 }
 
-TEST_F(SLEBaseTests, throws_on_missing_writable_entry)
+TEST_F(SLEBaseTests, ThrowsOnMissingWritableEntry)
 {
     // A view we never apply, so nothing here reaches the ledger.
     ApplyViewImpl av(&env_.getClosedLedger(), TapNone);

src/tests/libxrpl/ledger/SignerListEntry.cpp

@@ -7,7 +7,7 @@
 
 namespace xrpl::test {
 
-TEST(SignerListEntryTests, constructors)
+TEST(SignerListEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/SponsorshipEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(SponsorshipEntryTests, constructors)
+TEST(SponsorshipEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/TicketEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(TicketEntryTests, constructors)
+TEST(TicketEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/VaultEntry.cpp

@@ -8,7 +8,7 @@
 
 namespace xrpl::test {
 
-TEST(VaultEntryTests, constructors)
+TEST(VaultEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/XChainOwnedClaimIDEntry.cpp

@@ -12,7 +12,7 @@
 
 namespace xrpl::test {
 
-TEST(XChainOwnedClaimIDEntryTests, constructors)
+TEST(XChainOwnedClaimIDEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/ledger/XChainOwnedCreateAccountClaimIDEntry.cpp

@@ -13,7 +13,7 @@
 
 namespace xrpl::test {
 
-TEST(XChainOwnedCreateAccountClaimIDEntryTests, constructors)
+TEST(XChainOwnedCreateAccountClaimIDEntryTests, Constructors)
 {
     EntryTestEnv e;
 

src/tests/libxrpl/protocol/STXChainBridge.cpp

@@ -28,7 +28,7 @@ account(std::string_view hex)
 // getText() builds its string from eight substitutions of the same type, so a
 // transposed pair would still compile and still type check. Pin the output so
 // the field/value pairing is actually verified.
-TEST(STXChainBridge, get_text_pairs_each_field_with_its_value)
+TEST(STXChainBridge, getTextPairsEachFieldWithItsValue)
 {
     auto const lockingDoor = account("0102030405060708090A0B0C0D0E0F1011121314");
     auto const issuingDoor = account("14131211100F0E0D0C0B0A090807060504030201");
@@ -46,7 +46,7 @@ TEST(STXChainBridge, get_text_pairs_each_field_with_its_value)
     EXPECT_EQ(bridge.getText(), expected);
 }
 
-TEST(STXChainBridge, get_text_on_a_default_bridge)
+TEST(STXChainBridge, getTextOnADefaultBridge)
 {
     STXChainBridge const bridge;
     auto const text = bridge.getText();

src/tests/libxrpl/server/InfoSub.cpp

@@ -12,7 +12,7 @@ using namespace xrpl;
 // by subscribing the real cap through a WebSocket, which would exceed the frame
 // limit and drop the connection before the check runs) lets the boundary be
 // asserted exactly.
-TEST(InfoSubSubscriptionCap, boundary)
+TEST(InfoSubSubscriptionCap, Boundary)
 {
     constexpr std::size_t cap = kMaxSubscriptionsPerConnection;
 
@@ -30,7 +30,7 @@ TEST(InfoSubSubscriptionCap, boundary)
     EXPECT_TRUE(exceedsSubscriptionCap(cap - 1, 2));
 }
 
-TEST(InfoSubSubscriptionCap, no_overflow)
+TEST(InfoSubSubscriptionCap, NoOverflow)
 {
     constexpr std::size_t cap = kMaxSubscriptionsPerConnection;
     constexpr std::size_t max = std::numeric_limits<std::size_t>::max();
@@ -41,7 +41,7 @@ TEST(InfoSubSubscriptionCap, no_overflow)
     EXPECT_TRUE(exceedsSubscriptionCap(cap, max));
 }
 
-TEST(InfoSubSubscriptionCap, explicit_cap)
+TEST(InfoSubSubscriptionCap, ExplicitCap)
 {
     // A configured override is honored: the boundary tracks the passed cap, not
     // the built-in default. This is the seam doSubscribe uses to enforce a

src/tests/libxrpl/shamap/SHAMap.cpp

@@ -455,52 +455,6 @@ TEST_F(SHAMapTraversal, bounds_agree_with_iteration_for_absent_keys)
     }
 }
 
-TEST_F(SHAMapTraversal, bounds_on_empty_map_return_end)
-{
-    tests::TestNodeFamily f{j_};
-    SHAMap map{SHAMapType::FREE, f};
-    map.setUnbacked();
-
-    // The root is a childless inner node, so boundHelper's inner-node branch scans every branch on
-    // the requested side of the one id selects, finds them all empty, and falls through to end()
-    // rather than dereference a child.
-    EXPECT_EQ(map.upperBound(uint256{}), map.end());
-    EXPECT_EQ(map.lowerBound(uint256{}), map.end());
-
-    uint256 probe;
-    std::fill_n(probe.begin(), probe.size(), std::uint8_t{0xff});
-    EXPECT_EQ(map.upperBound(probe), map.end());
-    EXPECT_EQ(map.lowerBound(probe), map.end());
-}
-
-TEST_F(SHAMapTraversal, bounds_on_single_item_map_use_the_leaf_below_the_root)
-{
-    tests::TestNodeFamily f{j_};
-    SHAMap map{SHAMapType::FREE, f};
-
-    auto const key = deepFanOutKeys().front();
-    fillMap(map, {key});
-
-    // fillMap adds items in-process, so root_ stays an inner node with the single leaf below it.
-    // The stack holds both, so boundHelper examines the leaf first.
-    uint256 below = key;
-    --below;
-    uint256 above = key;
-    ++above;
-
-    auto const upper = map.upperBound(below);
-    ASSERT_NE(upper, map.end());
-    EXPECT_EQ(upper->key(), key);
-    EXPECT_EQ(map.upperBound(key), map.end());
-    EXPECT_EQ(map.upperBound(above), map.end());
-
-    auto const lower = map.lowerBound(above);
-    ASSERT_NE(lower, map.end());
-    EXPECT_EQ(lower->key(), key);
-    EXPECT_EQ(map.lowerBound(key), map.end());
-    EXPECT_EQ(map.lowerBound(below), map.end());
-}
-
 TEST_F(SHAMapTraversal, iteration_survives_deletions)
 {
     tests::TestNodeFamily f{j_};

src/tests/libxrpl/tx/AccountSet.cpp

@@ -43,7 +43,7 @@
 
 namespace xrpl::test {
 
-TEST(AccountSet, null_account_set)
+TEST(AccountSet, NullAccountSet)
 {
     TxTest env;
 
@@ -60,7 +60,7 @@ TEST(AccountSet, null_account_set)
     EXPECT_EQ(accountRoot.getFlags(), 0);
 }
 
-TEST(AccountSet, most_flags)
+TEST(AccountSet, MostFlags)
 {
     Account const alice("alice");
 
@@ -175,7 +175,7 @@ TEST(AccountSet, most_flags)
     });
 }
 
-TEST(AccountSet, set_and_reset_account_txn_id)
+TEST(AccountSet, SetAndResetAccountTxnID)
 {
     TxTest env;
     Account const alice("alice");
@@ -206,7 +206,7 @@ TEST(AccountSet, set_and_reset_account_txn_id)
     EXPECT_EQ(nowFlags, origFlags);
 }
 
-TEST(AccountSet, set_no_freeze)
+TEST(AccountSet, SetNoFreeze)
 {
     TxTest env;
     Account const alice("alice");
@@ -249,7 +249,7 @@ TEST(AccountSet, set_no_freeze)
     EXPECT_TRUE(env.getAccountRoot(alice).isFlag(lsfNoFreeze));
 }
 
-TEST(AccountSet, domain)
+TEST(AccountSet, Domain)
 {
     TxTest env;
     Account const alice("alice");
@@ -317,7 +317,7 @@ TEST(AccountSet, domain)
     }
 }
 
-TEST(AccountSet, message_key)
+TEST(AccountSet, MessageKey)
 {
     TxTest env;
     Account const alice("alice");
@@ -358,7 +358,7 @@ TEST(AccountSet, message_key)
         telBAD_PUBLIC_KEY);
 }
 
-TEST(AccountSet, wallet_id)
+TEST(AccountSet, WalletID)
 {
     TxTest env;
     Account const alice("alice");
@@ -391,7 +391,7 @@ TEST(AccountSet, wallet_id)
     EXPECT_FALSE(env.getAccountRoot(alice).hasWalletLocator());
 }
 
-TEST(AccountSet, email_hash)
+TEST(AccountSet, EmailHash)
 {
     TxTest env;
     Account const alice("alice");
@@ -422,7 +422,7 @@ TEST(AccountSet, email_hash)
     EXPECT_FALSE(env.getAccountRoot(alice).hasEmailHash());
 }
 
-TEST(AccountSet, transfer_rate)
+TEST(AccountSet, TransferRate)
 {
     struct TestCase
     {
@@ -473,7 +473,7 @@ TEST(AccountSet, transfer_rate)
     }
 }
 
-TEST(AccountSet, bad_inputs)
+TEST(AccountSet, BadInputs)
 {
     TxTest env;
     Account const alice("alice");
@@ -553,7 +553,7 @@ TEST(AccountSet, bad_inputs)
         tecNO_ALTERNATIVE_KEY);
 }
 
-TEST(AccountSet, require_auth_with_dir)
+TEST(AccountSet, RequireAuthWithDir)
 {
     TxTest env;
     Account const alice("alice");
@@ -601,7 +601,7 @@ TEST(AccountSet, require_auth_with_dir)
         tesSUCCESS);
 }
 
-TEST(AccountSet, ticket)
+TEST(AccountSet, Ticket)
 {
     TxTest env;
     Account const alice("alice");
@@ -660,7 +660,7 @@ TEST(AccountSet, ticket)
         tefNO_TICKET);
 }
 
-TEST(AccountSet, bad_signing_key)
+TEST(AccountSet, BadSigningKey)
 {
     TxTest env;
     Account const alice("alice");
@@ -684,7 +684,7 @@ TEST(AccountSet, bad_signing_key)
     EXPECT_FALSE(result.applied);
 }
 
-TEST(AccountSet, gateway)
+TEST(AccountSet, Gateway)
 {
     Account const alice("alice");
     Account const bob("bob");

src/xrpld/app/ledger/detail/LedgerToJson.cpp

@@ -3,7 +3,6 @@
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/misc/DeliverMax.h>
 #include <xrpld/app/misc/TxQ.h>
-#include <xrpld/rpc/CTID.h>
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/SyntheticFields.h>
 
@@ -12,7 +11,6 @@
 #include <xrpl/basics/chrono.h>
 #include <xrpl/basics/strHex.h>
 #include <xrpl/beast/utility/instrumentation.h>
-#include <xrpl/core/NetworkIDService.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/ledger/helpers/TokenHelpers.h>
 #include <xrpl/protocol/AccountID.h>
@@ -29,7 +27,6 @@
 #include <xrpl/protocol/jss.h>
 #include <xrpl/protocol/serialize.h>
 
-#include <cstdint>
 #include <exception>
 #include <memory>
 #include <string>
@@ -182,19 +179,6 @@ fillJsonTx(
         }
     }
 
-    // compute outgoing CTID
-    if (stMeta && stMeta->isFieldPresent(sfTransactionIndex))
-    {
-        uint32_t const lgrSeq = fill.ledger.seq();
-        uint32_t const txnIdx = stMeta->getFieldU32(sfTransactionIndex);
-        uint32_t netID = fill.context->app.getNetworkIDService().getNetworkID();
-        if (txn->isFieldPresent(sfNetworkID))
-            netID = txn->getFieldU32(sfNetworkID);
-
-        if (auto ctid = rpc::encodeCTID(lgrSeq, txnIdx, netID))
-            txJson[jss::ctid] = *ctid;
-    }
-
     if (((fill.options & static_cast<int>(LedgerFill::Options::OwnerFunds)) != 0) &&
         txn->getTxnType() == ttOFFER_CREATE)
     {