From ea0a6904f0c0326729623848529c2e5b6ba71dac Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Fri, 24 Jul 2026 16:52:45 +0100 Subject: [PATCH 01/11] chore: Verify tooling version for Nix-managed environments (#7862) --- .github/workflows/check-tools.yml | 114 ++++++++++++++++++++++++++++ bin/check-tools.sh | 21 +++-- docs/build/nix.md | 8 ++ nix/check-tools/README.md | 49 ++++++++++++ nix/check-tools/macos.txt | 47 ++++++++++++ nix/check-tools/nix-nixos-amd64.txt | 55 ++++++++++++++ nix/check-tools/nix-nixos-arm64.txt | 55 ++++++++++++++ 7 files changed, 341 insertions(+), 8 deletions(-) create mode 100644 .github/workflows/check-tools.yml create mode 100644 nix/check-tools/README.md create mode 100644 nix/check-tools/macos.txt create mode 100644 nix/check-tools/nix-nixos-amd64.txt create mode 100644 nix/check-tools/nix-nixos-arm64.txt diff --git a/.github/workflows/check-tools.yml b/.github/workflows/check-tools.yml new file mode 100644 index 0000000000..02e2038de0 --- /dev/null +++ b/.github/workflows/check-tools.yml @@ -0,0 +1,114 @@ +# Verifies the committed snapshots of `bin/check-tools.sh` output for each Nix +# environment (see nix/check-tools/). If the environment changes — a new image +# tag, an updated flake.lock, a different tool list — without the matching +# snapshot being regenerated and committed, this workflow fails so the drift is +# caught in review. +# +# To regenerate the snapshots, see nix/check-tools/README.md. +name: Check tools + +on: + pull_request: + paths: + - ".github/workflows/check-tools.yml" + - ".github/scripts/strategy-matrix/linux.json" + - "bin/check-tools.sh" + - "nix/check-tools/**" + - "flake.nix" + - "flake.lock" + - "rust-toolchain.toml" + push: + branches: + - "develop" + paths: + - ".github/workflows/check-tools.yml" + - ".github/scripts/strategy-matrix/linux.json" + - "bin/check-tools.sh" + - "nix/check-tools/**" + - "flake.nix" + - "flake.lock" + - "rust-toolchain.toml" + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +defaults: + run: + shell: bash + +jobs: + # The nix-nixos image tag is pinned alongside the build matrix in linux.json, + # so snapshots are checked against the exact image CI builds against. + linux-image-tag: + runs-on: ubuntu-latest + outputs: + tag: ${{ steps.tag.outputs.tag }} + steps: + - name: Checkout repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Read nix image tag + id: tag + run: echo "tag=$(jq -r .image_tag .github/scripts/strategy-matrix/linux.json)" >>"${GITHUB_OUTPUT}" + + # One job for all environments; they differ only in whether the tools come + # from the nix-nixos container (Linux) or `nix develop` (macOS). + check-tools: + needs: linux-image-tag + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu + snapshot: nix/check-tools/nix-nixos-amd64.txt + nix_develop: false + - runner: ubuntu-arm64 + snapshot: nix/check-tools/nix-nixos-arm64.txt + nix_develop: false + - runner: macos-26-apple-clang-21 + snapshot: nix/check-tools/macos.txt + nix_develop: true + runs-on: ${{ matrix.runner }} + # Linux runs inside the pinned nix-nixos image; macOS runs natively and uses + # the flake's dev shell instead (see the run step below). + container: ${{ !matrix.nix_develop && format('ghcr.io/xrplf/xrpld/nix-nixos:{0}', needs.linux-image-tag.outputs.tag) || null }} + steps: + - name: Checkout repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Prepare runner + uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 + with: + enable_ccache: false + + - name: Regenerate snapshot + env: + CHECK_TOOLS_SKIP_CLONE: "1" + # check-tools.sh skips some macOS tools when CI is set; the snapshots + # capture the full `nix develop` environment, so unset it here. + CI: "" + run: | + if [ "${{ matrix.nix_develop }}" = "true" ]; then + # `nix develop` prints the dev-shell greeting first; keep only the + # check-tools.sh output (from the "Detected OS:" line onward). + nix --extra-experimental-features "nix-command flakes" develop \ + -c bash bin/check-tools.sh | sed -n '/^Detected OS:/,$p' >"${{ matrix.snapshot }}" + else + bash bin/check-tools.sh >"${{ matrix.snapshot }}" + fi + + - name: Verify snapshot is up to date + run: | + if ! git diff --exit-code -- "${{ matrix.snapshot }}"; then + echo "::error::${{ matrix.snapshot }} is out of date. Regenerate it (see nix/check-tools/README.md) and commit the result." + exit 1 + fi + + - name: Upload regenerated snapshot + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: check-tools-${{ runner.os }}-${{ runner.arch }} + path: ${{ matrix.snapshot }} diff --git a/bin/check-tools.sh b/bin/check-tools.sh index 7886bcf8b0..e230302742 100755 --- a/bin/check-tools.sh +++ b/bin/check-tools.sh @@ -30,8 +30,10 @@ missing=() checked=0 # check [probe-command...] -# Runs the probe (default: " --version") quietly. Records as -# missing if the command is not found or exits non-zero. +# Runs the probe (default: " --version"), capturing both stdout and +# stderr, and prints one aligned line: the status, the name, and the first +# non-blank line of the probe output (its version). Records as missing +# if the command is not found or exits non-zero. check() { local name="$1" shift @@ -40,10 +42,11 @@ check() { probe=("${name}" --version) fi - echo "Checking ${name}..." checked=$((checked + 1)) - if "${probe[@]}" | head -n 1; then - printf ' [ ok ] %s\n' "${name}" + local output version + if output="$("${probe[@]}" 2>&1)"; then + version="$(printf '%s\n' "${output}" | grep -m1 '[^[:space:]]' || true)" + printf ' [ ok ] %-20s %s\n' "${name}" "${version}" else printf ' [MISS] %s\n' "${name}" missing+=("${name}") @@ -85,12 +88,14 @@ if [ "${os}" = "linux" ] || [ "${os}" = "macos" ]; then check file check less check make - check netstat which netstat + # net-tools netstat reports "net-tools X.Y"; macOS ships BSD netstat with no + # version flag, so fall back to a presence marker there. + check netstat sh -c 'command -v netstat >/dev/null && { netstat --version 2>&1 | grep -m1 -oE "net-tools [0-9.]+" || echo present; }' check ninja - check perl + check perl perl -e 'print "$^V\n"' check pkg-config check vim - check zip + check zip bash -c 'zip --version 2>&1 | grep -m1 -oE "Zip [0-9.]+"' # These tools are present in our Linux CI images and in local development # setups, but not in the macOS CI environment. So check them everywhere diff --git a/docs/build/nix.md b/docs/build/nix.md index b95b82fc42..d0001294e3 100644 --- a/docs/build/nix.md +++ b/docs/build/nix.md @@ -154,6 +154,14 @@ conan install .. --output-folder . --build '*' --settings build_type=Release To update `flake.lock` to the latest revision use `nix flake update` command. +## Tooling snapshots + +The tool versions in each Nix environment are recorded in +[`nix/check-tools/`](../../nix/check-tools) and verified by CI. If you change the +environment (bump the CI image tag, update `flake.lock`, or edit the tool list in +`bin/check-tools.sh`), CI fails until you regenerate and commit the affected +snapshot — see [`nix/check-tools/README.md`](../../nix/check-tools/README.md). + ## Troubleshooting See [Troubleshooting Nix problems](./nix_troubleshooting.md) for common issues, diff --git a/nix/check-tools/README.md b/nix/check-tools/README.md new file mode 100644 index 0000000000..fe9ab3e250 --- /dev/null +++ b/nix/check-tools/README.md @@ -0,0 +1,49 @@ +# check-tools snapshots + +These files capture the output of [`bin/check-tools.sh`](../../bin/check-tools.sh) +— the versions of the development tooling — in each Nix environment: + +| File | Environment | +| --------------------- | ----------------------------------- | +| `nix-nixos-amd64.txt` | `nix-nixos` CI image, `linux/amd64` | +| `nix-nixos-arm64.txt` | `nix-nixos` CI image, `linux/arm64` | +| `macos.txt` | macOS, inside `nix develop` | + +The [`check-tools`](../../.github/workflows/check-tools.yml) workflow regenerates +each snapshot in its environment and fails if it differs from the committed file. +So if you change the environment (bump the image tag in +[`linux.json`](../../.github/scripts/strategy-matrix/linux.json), update +`flake.lock`, change the tool list in `check-tools.sh`, …) you must regenerate +and commit the affected snapshots. + +Each snapshot is `check-tools.sh` stdout with the git-clone connectivity check +skipped (`CHECK_TOOLS_SKIP_CLONE=1`), so it contains only deterministic version +data. On macOS the dev-shell greeting that `nix develop` prints first is dropped +with `sed -n '/^Detected OS:/,$p'`. + +## Regenerating + +The two Linux snapshots come from the `nix-nixos` image (Docker or a compatible +runtime such as Apple `container`). The image tag is pinned in `linux.json`: + +```bash +img="ghcr.io/xrplf/xrpld/nix-nixos:$(jq -r .image_tag .github/scripts/strategy-matrix/linux.json)" + +for arch in amd64 arm64; do + container run --rm -i -e CHECK_TOOLS_SKIP_CLONE=1 -a "${arch}" --entrypoint bash "${img}" -s \ + "nix/check-tools/nix-nixos-${arch}.txt" +done +``` + +(With Docker, replace `container run … -a "${arch}"` with +`docker run … --platform "linux/${arch}"`.) + +The macOS snapshot is generated locally. `CI=` is unset so `check-tools.sh` +checks the full dev-shell tool set (it otherwise skips some tools when `CI` is +set): + +```bash +CI= nix develop -c bash -c 'CHECK_TOOLS_SKIP_CLONE=1 bash bin/check-tools.sh' | + sed -n '/^Detected OS:/,$p' \ + >nix/check-tools/macos.txt +``` diff --git a/nix/check-tools/macos.txt b/nix/check-tools/macos.txt new file mode 100644 index 0000000000..af9814e9cb --- /dev/null +++ b/nix/check-tools/macos.txt @@ -0,0 +1,47 @@ +Detected OS: macos (Darwin arm64) + +Core build tools: + [ ok ] cmake cmake version 4.1.2 + [ ok ] conan Conan version 2.28.1 + [ ok ] git git version 2.54.0 + [ ok ] python3 Python 3.13.13 + +Development tooling: + [ ok ] ccache ccache version 4.13.6 + [ ok ] clang clang version 21.1.8 + [ ok ] clang++ clang version 21.1.8 + [ ok ] ClangBuildAnalyzer ClangBuildAnalyzer 1.6.0 + [ ok ] curl curl 8.20.0 (aarch64-apple-darwin25.3.0) libcurl/8.20.0 OpenSSL/3.6.2 zlib/1.3.2 libssh2/1.11.1 nghttp2/1.69.0 mit-krb5/1.22.1 + [ ok ] file file-5.47 + [ ok ] less less 692 (PCRE2 regular expressions) + [ ok ] make GNU Make 4.4.1 + [ ok ] netstat present + [ ok ] ninja 1.13.2 + [ ok ] perl v5.42.0 + [ ok ] pkg-config 0.29.2 + [ ok ] vim VIM - Vi IMproved 9.2 (2026 Feb 14, compiled Jan 01 1980 00:00:00) + [ ok ] zip Zip 3.0 + [ ok ] clang-format clang-format version 21.1.8 + [ ok ] dot dot - graphviz version 12.2.1 (0) + [ ok ] doxygen 1.16.1 + [ ok ] gcovr gcovr 8.4 + [ ok ] gh gh version 2.94.0 (nixpkgs) + [ ok ] git-cliff git-cliff 2.13.1 + [ ok ] git-lfs git-lfs/3.7.1 (3.7.1; darwin arm64; go 1.26.3) + [ ok ] gpg gpg (GnuPG) 2.4.9 + [ ok ] pre-commit pre-commit 4.5.1 + [ ok ] run-clang-tidy usage: run-clang-tidy [-h] [-allow-enabling-alpha-checkers] + +Rust toolchain: + [ ok ] cargo cargo 1.95.0 (f2d3ce0bd 2026-03-21) + [ ok ] cargo-audit cargo-audit-audit 0.22.1 + [ ok ] cargo-llvm-cov cargo-llvm-cov 0.8.5 + [ ok ] cargo-nextest cargo-nextest 0.9.137 + [ ok ] clippy clippy 0.1.95 (59807616e1 2026-04-14) + [ ok ] rust-analyzer rust-analyzer 1.95.0 (59807616 2026-04-14) + [ ok ] rustc rustc 1.95.0 (59807616e 2026-04-14) + [ ok ] rustfmt rustfmt 1.9.0-stable (59807616e1 2026-04-14) + +Skipping git-over-HTTPS check (CHECK_TOOLS_SKIP_CLONE is set). + +All 36 checked tools are present and runnable. diff --git a/nix/check-tools/nix-nixos-amd64.txt b/nix/check-tools/nix-nixos-amd64.txt new file mode 100644 index 0000000000..b922cca4a8 --- /dev/null +++ b/nix/check-tools/nix-nixos-amd64.txt @@ -0,0 +1,55 @@ +Detected OS: linux (Linux x86_64) + +Core build tools: + [ ok ] cmake cmake version 4.1.2 + [ ok ] conan Conan version 2.28.1 + [ ok ] git git version 2.54.0 + [ ok ] python3 Python 3.13.13 + +Development tooling: + [ ok ] ccache ccache version 4.13.6 + [ ok ] clang clang version 22.1.7 + [ ok ] clang++ clang version 22.1.7 + [ ok ] ClangBuildAnalyzer ClangBuildAnalyzer 1.6.0 + [ ok ] curl curl 8.20.0 (x86_64-pc-linux-gnu) libcurl/8.20.0 OpenSSL/3.6.2 zlib/1.3.2 libssh2/1.11.1 nghttp2/1.69.0 mit-krb5/1.22.1 + [ ok ] file file-5.47 + [ ok ] less less 692 (PCRE2 regular expressions) + [ ok ] make GNU Make 4.4.1 + [ ok ] netstat net-tools 2.10 + [ ok ] ninja 1.13.2 + [ ok ] perl v5.42.0 + [ ok ] pkg-config 0.29.2 + [ ok ] vim VIM - Vi IMproved 9.2 (2026 Feb 14, compiled Jan 01 1980 00:00:00) + [ ok ] zip Zip 3.0 + [ ok ] clang-format clang-format version 22.1.7 + [ ok ] dot dot - graphviz version 12.2.1 (0) + [ ok ] doxygen 1.16.1 + [ ok ] gcovr gcovr 8.4 + [ ok ] gh gh version 2.94.0 (nixpkgs) + [ ok ] git-cliff git-cliff 2.13.1 + [ ok ] git-lfs git-lfs/3.7.1 (3.7.1; linux amd64; go 1.26.3) + [ ok ] gpg gpg (GnuPG) 2.4.9 + [ ok ] pre-commit pre-commit 4.5.1 + [ ok ] run-clang-tidy usage: run-clang-tidy [-h] [-allow-enabling-alpha-checkers] + +Rust toolchain: + [ ok ] cargo cargo 1.95.0 (f2d3ce0bd 2026-03-21) + [ ok ] cargo-audit cargo-audit-audit 0.22.1 + [ ok ] cargo-llvm-cov cargo-llvm-cov 0.8.5 + [ ok ] cargo-nextest cargo-nextest 0.9.137 + [ ok ] clippy clippy 0.1.95 (59807616e1 2026-04-14) + [ ok ] rust-analyzer rust-analyzer 1.95.0 (5980761 2026-04-14) + [ ok ] rustc rustc 1.95.0 (59807616e 2026-04-14) + [ ok ] rustfmt rustfmt 1.9.0-stable (59807616e1 2026-04-14) + +GCC toolchain: + [ ok ] gcc gcc (GCC) 15.2.0 + [ ok ] g++ g++ (GCC) 15.2.0 + [ ok ] gcov gcov (GCC) 15.2.0 + +Mold: + [ ok ] mold mold 2.41.0 (compatible with GNU ld) + +Skipping git-over-HTTPS check (CHECK_TOOLS_SKIP_CLONE is set). + +All 40 checked tools are present and runnable. diff --git a/nix/check-tools/nix-nixos-arm64.txt b/nix/check-tools/nix-nixos-arm64.txt new file mode 100644 index 0000000000..5267839682 --- /dev/null +++ b/nix/check-tools/nix-nixos-arm64.txt @@ -0,0 +1,55 @@ +Detected OS: linux (Linux aarch64) + +Core build tools: + [ ok ] cmake cmake version 4.1.2 + [ ok ] conan Conan version 2.28.1 + [ ok ] git git version 2.54.0 + [ ok ] python3 Python 3.13.13 + +Development tooling: + [ ok ] ccache ccache version 4.13.6 + [ ok ] clang clang version 22.1.7 + [ ok ] clang++ clang version 22.1.7 + [ ok ] ClangBuildAnalyzer ClangBuildAnalyzer 1.6.0 + [ ok ] curl curl 8.20.0 (aarch64-unknown-linux-gnu) libcurl/8.20.0 OpenSSL/3.6.2 zlib/1.3.2 libssh2/1.11.1 nghttp2/1.69.0 mit-krb5/1.22.1 + [ ok ] file file-5.47 + [ ok ] less less 692 (PCRE2 regular expressions) + [ ok ] make GNU Make 4.4.1 + [ ok ] netstat net-tools 2.10 + [ ok ] ninja 1.13.2 + [ ok ] perl v5.42.0 + [ ok ] pkg-config 0.29.2 + [ ok ] vim VIM - Vi IMproved 9.2 (2026 Feb 14, compiled Jan 01 1980 00:00:00) + [ ok ] zip Zip 3.0 + [ ok ] clang-format clang-format version 22.1.7 + [ ok ] dot dot - graphviz version 12.2.1 (0) + [ ok ] doxygen 1.16.1 + [ ok ] gcovr gcovr 8.4 + [ ok ] gh gh version 2.94.0 (nixpkgs) + [ ok ] git-cliff git-cliff 2.13.1 + [ ok ] git-lfs git-lfs/3.7.1 (3.7.1; linux arm64; go 1.26.3) + [ ok ] gpg gpg (GnuPG) 2.4.9 + [ ok ] pre-commit pre-commit 4.5.1 + [ ok ] run-clang-tidy usage: run-clang-tidy [-h] [-allow-enabling-alpha-checkers] + +Rust toolchain: + [ ok ] cargo cargo 1.95.0 (f2d3ce0bd 2026-03-21) + [ ok ] cargo-audit cargo-audit-audit 0.22.1 + [ ok ] cargo-llvm-cov cargo-llvm-cov 0.8.5 + [ ok ] cargo-nextest cargo-nextest 0.9.137 + [ ok ] clippy clippy 0.1.95 (59807616e1 2026-04-14) + [ ok ] rust-analyzer rust-analyzer 1.95.0 (5980761 2026-04-14) + [ ok ] rustc rustc 1.95.0 (59807616e 2026-04-14) + [ ok ] rustfmt rustfmt 1.9.0-stable (59807616e1 2026-04-14) + +GCC toolchain: + [ ok ] gcc gcc (GCC) 15.2.0 + [ ok ] g++ g++ (GCC) 15.2.0 + [ ok ] gcov gcov (GCC) 15.2.0 + +Mold: + [ ok ] mold mold 2.41.0 (compatible with GNU ld) + +Skipping git-over-HTTPS check (CHECK_TOOLS_SKIP_CLONE is set). + +All 40 checked tools are present and runnable. From fecfc0cf3faf88799e1bc3b40dff821b1572ffc0 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Fri, 24 Jul 2026 18:30:20 +0100 Subject: [PATCH 02/11] chore: Fix clang version in devshell (#7860) Co-authored-by: Ayaz Salikhov --- .github/workflows/check-tools.yml | 14 +++++++------- nix/check-tools/macos.txt | 6 +++--- ...ix-nixos-amd64.txt => nix-ubuntu-amd64.txt} | 0 ...ix-nixos-arm64.txt => nix-ubuntu-arm64.txt} | 0 nix/packages.nix | 18 +++++++++++++++++- 5 files changed, 27 insertions(+), 11 deletions(-) rename nix/check-tools/{nix-nixos-amd64.txt => nix-ubuntu-amd64.txt} (100%) rename nix/check-tools/{nix-nixos-arm64.txt => nix-ubuntu-arm64.txt} (100%) diff --git a/.github/workflows/check-tools.yml b/.github/workflows/check-tools.yml index 02e2038de0..6daaf98114 100644 --- a/.github/workflows/check-tools.yml +++ b/.github/workflows/check-tools.yml @@ -13,7 +13,7 @@ on: - ".github/workflows/check-tools.yml" - ".github/scripts/strategy-matrix/linux.json" - "bin/check-tools.sh" - - "nix/check-tools/**" + - "nix/**" - "flake.nix" - "flake.lock" - "rust-toolchain.toml" @@ -24,7 +24,7 @@ on: - ".github/workflows/check-tools.yml" - ".github/scripts/strategy-matrix/linux.json" - "bin/check-tools.sh" - - "nix/check-tools/**" + - "nix/**" - "flake.nix" - "flake.lock" - "rust-toolchain.toml" @@ -61,11 +61,11 @@ jobs: fail-fast: false matrix: include: - - runner: ubuntu - snapshot: nix/check-tools/nix-nixos-amd64.txt + - runner: ubuntu-latest + snapshot: nix/check-tools/nix-ubuntu-amd64.txt nix_develop: false - - runner: ubuntu-arm64 - snapshot: nix/check-tools/nix-nixos-arm64.txt + - runner: ubuntu-24.04-arm + snapshot: nix/check-tools/nix-ubuntu-arm64.txt nix_develop: false - runner: macos-26-apple-clang-21 snapshot: nix/check-tools/macos.txt @@ -73,7 +73,7 @@ jobs: runs-on: ${{ matrix.runner }} # Linux runs inside the pinned nix-nixos image; macOS runs natively and uses # the flake's dev shell instead (see the run step below). - container: ${{ !matrix.nix_develop && format('ghcr.io/xrplf/xrpld/nix-nixos:{0}', needs.linux-image-tag.outputs.tag) || null }} + container: ${{ !matrix.nix_develop && format('ghcr.io/xrplf/xrpld/nix-ubuntu:{0}', needs.linux-image-tag.outputs.tag) || null }} steps: - name: Checkout repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 diff --git a/nix/check-tools/macos.txt b/nix/check-tools/macos.txt index af9814e9cb..93cc926181 100644 --- a/nix/check-tools/macos.txt +++ b/nix/check-tools/macos.txt @@ -8,8 +8,8 @@ Core build tools: Development tooling: [ ok ] ccache ccache version 4.13.6 - [ ok ] clang clang version 21.1.8 - [ ok ] clang++ clang version 21.1.8 + [ ok ] clang clang version 22.1.7 + [ ok ] clang++ clang version 22.1.7 [ ok ] ClangBuildAnalyzer ClangBuildAnalyzer 1.6.0 [ ok ] curl curl 8.20.0 (aarch64-apple-darwin25.3.0) libcurl/8.20.0 OpenSSL/3.6.2 zlib/1.3.2 libssh2/1.11.1 nghttp2/1.69.0 mit-krb5/1.22.1 [ ok ] file file-5.47 @@ -21,7 +21,7 @@ Development tooling: [ ok ] pkg-config 0.29.2 [ ok ] vim VIM - Vi IMproved 9.2 (2026 Feb 14, compiled Jan 01 1980 00:00:00) [ ok ] zip Zip 3.0 - [ ok ] clang-format clang-format version 21.1.8 + [ ok ] clang-format clang-format version 22.1.7 [ ok ] dot dot - graphviz version 12.2.1 (0) [ ok ] doxygen 1.16.1 [ ok ] gcovr gcovr 8.4 diff --git a/nix/check-tools/nix-nixos-amd64.txt b/nix/check-tools/nix-ubuntu-amd64.txt similarity index 100% rename from nix/check-tools/nix-nixos-amd64.txt rename to nix/check-tools/nix-ubuntu-amd64.txt diff --git a/nix/check-tools/nix-nixos-arm64.txt b/nix/check-tools/nix-ubuntu-arm64.txt similarity index 100% rename from nix/check-tools/nix-nixos-arm64.txt rename to nix/check-tools/nix-ubuntu-arm64.txt diff --git a/nix/packages.nix b/nix/packages.nix index 3cf0f57c3e..01ab2ecf9a 100644 --- a/nix/packages.nix +++ b/nix/packages.nix @@ -16,7 +16,23 @@ let exec ${pkgs.python3}/bin/python3 ${llvmPackages.clang-unwrapped}/bin/run-clang-tidy "$@" ''; - rustToolchain = pkgs.rust-bin.fromRustupToolchainFile ../rust-toolchain.toml; + # rust-overlay's toolchain propagates the *default* stdenv.cc onto the PATH (so + # cargo has a linker). That default may be different from the clang we pin here, + # so it shadows our clang and the build can silently use a different compiler + # version. Drop that cc from every propagation channel instead of pinning a + # replacement: the toolchain then carries no compiler and cargo just uses the + # active shell's stdenv cc. Must cover all channels — rust-overlay uses both + # propagatedBuildInputs and depsHostHostPropagated. + rustToolchainBase = pkgs.rust-bin.fromRustupToolchainFile ../rust-toolchain.toml; + rustToolchain = + let + defaultCc = pkgs.stdenv.cc; # default compiler from nixpkgs stdenv + withoutDefaultCc = builtins.filter (dep: (dep.outPath or "") != defaultCc.outPath); + in + rustToolchainBase.overrideAttrs (old: { + propagatedBuildInputs = withoutDefaultCc (old.propagatedBuildInputs or [ ]); + depsHostHostPropagated = withoutDefaultCc (old.depsHostHostPropagated or [ ]); + }); # Nix wraps its toolchain so that binaries are exposed only under unsuffixed # names (gcc, g++, clang-tidy, ...). Several tools probe for a From b878818e809455cb82dc055a473e066d309fa9bf Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 27 Jul 2026 09:55:01 +0000 Subject: [PATCH 03/11] ci: [DEPENDABOT] bump actions/checkout from 7.0.0 to 7.0.1 (#7871) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/check-pr-description.yml | 2 +- .github/workflows/check-tools.yml | 4 ++-- .github/workflows/on-pr.yml | 2 +- .github/workflows/publish-docs.yml | 2 +- .github/workflows/reusable-build-test-config.yml | 2 +- .github/workflows/reusable-check-levelization.yml | 2 +- .github/workflows/reusable-check-rename.yml | 2 +- .github/workflows/reusable-clang-tidy.yml | 2 +- .github/workflows/reusable-package.yml | 4 ++-- .github/workflows/reusable-strategy-matrix.yml | 2 +- .github/workflows/reusable-upload-recipe.yml | 2 +- .github/workflows/upload-conan-deps.yml | 2 +- 12 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/check-pr-description.yml b/.github/workflows/check-pr-description.yml index 744449f216..f8e7b6cdc4 100644 --- a/.github/workflows/check-pr-description.yml +++ b/.github/workflows/check-pr-description.yml @@ -23,7 +23,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Write PR body to file env: diff --git a/.github/workflows/check-tools.yml b/.github/workflows/check-tools.yml index 6daaf98114..af20c5f17e 100644 --- a/.github/workflows/check-tools.yml +++ b/.github/workflows/check-tools.yml @@ -47,7 +47,7 @@ jobs: tag: ${{ steps.tag.outputs.tag }} steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Read nix image tag id: tag @@ -76,7 +76,7 @@ jobs: container: ${{ !matrix.nix_develop && format('ghcr.io/xrplf/xrpld/nix-ubuntu:{0}', needs.linux-image-tag.outputs.tag) || null }} steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index 13c807ffca..1cd97305da 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -52,7 +52,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Determine changed files # This step checks whether any files have changed that should # cause the next jobs to run. We do it this way rather than diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index 49f5c021a3..c1e67e2010 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -44,7 +44,7 @@ jobs: container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-40cdf49 steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 6372bb6328..74425febe8 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -110,7 +110,7 @@ jobs: uses: XRPLF/actions/cleanup-workspace@c7d9ce5ebb03c752a354889ecd870cadfc2b1cd4 - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 diff --git a/.github/workflows/reusable-check-levelization.yml b/.github/workflows/reusable-check-levelization.yml index 88c95ac3ba..7f547f2ab6 100644 --- a/.github/workflows/reusable-check-levelization.yml +++ b/.github/workflows/reusable-check-levelization.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Check levelization run: python .github/scripts/levelization/generate.py - name: Check for differences diff --git a/.github/workflows/reusable-check-rename.yml b/.github/workflows/reusable-check-rename.yml index 9a91e98ee3..874c8adcde 100644 --- a/.github/workflows/reusable-check-rename.yml +++ b/.github/workflows/reusable-check-rename.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Check definitions run: .github/scripts/rename/definitions.sh . - name: Check copyright notices diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index 90f24bc464..3c19b58a12 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -40,7 +40,7 @@ jobs: issues: write steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 diff --git a/.github/workflows/reusable-package.yml b/.github/workflows/reusable-package.yml index 55bc20dc5c..e1c11ac677 100644 --- a/.github/workflows/reusable-package.yml +++ b/.github/workflows/reusable-package.yml @@ -27,7 +27,7 @@ jobs: matrix: ${{ steps.generate.outputs.matrix }} steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set up Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 @@ -54,7 +54,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Download pre-built binary uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 diff --git a/.github/workflows/reusable-strategy-matrix.yml b/.github/workflows/reusable-strategy-matrix.yml index de8d9cfc8e..12f11b0fbe 100644 --- a/.github/workflows/reusable-strategy-matrix.yml +++ b/.github/workflows/reusable-strategy-matrix.yml @@ -23,7 +23,7 @@ jobs: matrix: ${{ steps.generate.outputs.matrix }} steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set up Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 diff --git a/.github/workflows/reusable-upload-recipe.yml b/.github/workflows/reusable-upload-recipe.yml index 0f00ce7ca0..bce4da2df6 100644 --- a/.github/workflows/reusable-upload-recipe.yml +++ b/.github/workflows/reusable-upload-recipe.yml @@ -47,7 +47,7 @@ jobs: CONAN_PASSWORD_XRPLF: ${{ secrets.remote_password }} steps: - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Generate build version number id: version diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 8a02c4c2db..80a75a1fbf 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -65,7 +65,7 @@ jobs: uses: XRPLF/actions/cleanup-workspace@c7d9ce5ebb03c752a354889ecd870cadfc2b1cd4 - name: Checkout repository - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 From 20801d98acd36f1aa4bc283ccc00d46db6d20f14 Mon Sep 17 00:00:00 2001 From: Andrzej Budzanowski Date: Mon, 27 Jul 2026 13:58:15 +0200 Subject: [PATCH 04/11] test: Improve the server status test to not race and randomly fail (#7304) Co-authored-by: Alex Kremer --- src/test/jtx/AbstractClient.h | 11 +++ src/test/jtx/Env.h | 70 ++++++++++++++++-- src/test/jtx/WSClient_test.cpp | 44 ++++++++++- src/test/jtx/impl/JSONRPCClient.cpp | 92 +++++++++++++++++++++-- src/test/jtx/impl/WSClient.cpp | 101 ++++++++++++++++++-------- src/test/server/ServerStatus_test.cpp | 48 ++++++++---- 6 files changed, 307 insertions(+), 59 deletions(-) diff --git a/src/test/jtx/AbstractClient.h b/src/test/jtx/AbstractClient.h index f9d8de0768..58f57f67a5 100644 --- a/src/test/jtx/AbstractClient.h +++ b/src/test/jtx/AbstractClient.h @@ -40,6 +40,17 @@ public: */ [[nodiscard]] virtual unsigned version() const = 0; + + /** + * Close the client's connection to the server. + * + * Releases the connection the client holds against the server's per-port + * connection limit. After this call the client must not be used to + * invoke() again. Tests use this to deterministically free the slot + * rather than waiting for the server's idle timeout to drop it. + */ + virtual void + disconnect() = 0; }; } // namespace xrpl::test diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index 0df62c7e9b..a175fd5006 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -482,11 +482,52 @@ public: app().getNumberOfThreads() == 1, "syncClose() is only useful on an application with a single thread"); auto const result = close(); - auto serverBarrier = std::make_shared>(); - auto future = serverBarrier->get_future(); - boost::asio::post(app().getIOContext(), [serverBarrier]() { serverBarrier->set_value(); }); - auto const status = future.wait_for(timeout); - return result && status == std::future_status::ready; + return result && drainServerIo(timeout); + } + + /** + * Disconnect the Env's built-in client and wait for the server to + * register the dropped connection. + * + * Env holds one persistent client connection to the server's RPC port for + * its whole lifetime (see client()), and that connection counts against + * the port's connection limit. Tests that need a known starting occupancy + * can call this to deterministically release that slot instead of waiting + * out the server's localhost idle timeout. + * + * The server decrements its per-port connection count in the peer's + * destructor, which runs when the io_context processes the end-of-stream + * on the closed socket. After closing the client this drains the server's + * io_context twice: the first barrier guarantees the reactor has reaped + * the closed socket and queued the peer's teardown, and the second + * guarantees that teardown (and therefore the count decrement) has run. + * + * This is only sound when the server uses a single io_context thread, so + * that draining establishes ordering against the teardown - configure the + * Env with singleThreadIo() (as syncClose() also requires). Like + * syncClose(), it relies on loopback teardown latency being negligible. + * + * @param timeout Maximum time to wait for each barrier task to execute + * @return true if both barriers executed within timeout, false otherwise + */ + [[nodiscard]] bool + disconnectClient(std::chrono::steady_clock::duration timeout = std::chrono::seconds{1}) + { + XRPL_ASSERT( + app().getNumberOfThreads() == 1, + "disconnectClient() is only useful on an application with a single " + "thread"); + + bundle_.client->disconnect(); + + // Drain the server's single io thread twice: the first barrier flushes + // the reactor's reap of the closed socket (queuing the peer teardown), + // the second flushes that teardown - and therefore the connection-count + // decrement. Both run unconditionally so a timed-out first drain does + // not short-circuit the second. + bool const reaped = drainServerIo(timeout); + bool const toreDown = drainServerIo(timeout); + return reaped && toreDown; } /** @@ -846,6 +887,25 @@ public: } private: + /** + * Drain the (single) server io_context thread once. + * + * Posts a barrier task to the server's io_context and blocks until it + * runs, so every task queued before it has been processed. Only meaningful + * with a single io thread (see syncClose()/disconnectClient()). + * + * @param timeout Maximum time to wait for the barrier task to execute + * @return true if the barrier ran within timeout, false otherwise + */ + [[nodiscard]] bool + drainServerIo(std::chrono::steady_clock::duration timeout) + { + auto barrier = std::make_shared>(); + auto future = barrier->get_future(); + boost::asio::post(app().getIOContext(), [barrier]() { barrier->set_value(); }); + return future.wait_for(timeout) == std::future_status::ready; + } + void fund(bool setDefaultRipple, STAmount const& amount, Account const& account); diff --git a/src/test/jtx/WSClient_test.cpp b/src/test/jtx/WSClient_test.cpp index d77e0f948b..801ca50504 100644 --- a/src/test/jtx/WSClient_test.cpp +++ b/src/test/jtx/WSClient_test.cpp @@ -13,8 +13,9 @@ class WSClient_test : public beast::unit_test::Suite { public: void - run() override + testSmoke() { + testcase("smoke"); using namespace jtx; Env env(*this); auto wsc = makeWSClient(env.app().config()); @@ -28,6 +29,47 @@ public: auto jv = wsc->getMsg(std::chrono::seconds(1)); pass(); } + + void + testGracefulDisconnect() + { + testcase("graceful disconnect"); + using namespace jtx; + using namespace std::chrono; + + Env env(*this); + auto wsc = makeWSClient(env.app().config()); + + // Put real traffic on the connection before closing it. + json::Value stream; + stream["streams"] = json::ValueType::Array; + stream["streams"].append("ledger"); + auto const sub = wsc->invoke("subscribe", stream); + BEAST_EXPECT(sub.isMember("result") || sub.isMember("status")); + + // disconnect() performs a graceful WebSocket closing handshake and + // blocks until the server acknowledges. On loopback that completes in + // well under its internal 1s timeout; only a broken async_close/ack + // coordination would fall through to the force-close path at ~1s. A + // generous bound keeps this from flaking under load while still + // catching that regression. + auto const start = steady_clock::now(); + wsc->disconnect(); + auto const elapsed = duration_cast(steady_clock::now() - start); + BEAST_EXPECT(elapsed < milliseconds{750}); + + // disconnect() must be idempotent: a second call (and the subsequent + // destructor) must not hang, double-close, or crash. + wsc->disconnect(); + pass(); + } + + void + run() override + { + testSmoke(); + testGracefulDisconnect(); + } }; BEAST_DEFINE_TESTSUITE(WSClient, jtx, xrpl); diff --git a/src/test/jtx/impl/JSONRPCClient.cpp b/src/test/jtx/impl/JSONRPCClient.cpp index 495fc5a657..06474c3616 100644 --- a/src/test/jtx/impl/JSONRPCClient.cpp +++ b/src/test/jtx/impl/JSONRPCClient.cpp @@ -14,18 +14,23 @@ #include #include +#include #include #include #include #include #include #include +#include #include #include #include #include #include +#include +#include +#include #include #include #include @@ -84,6 +89,40 @@ class JSONRPCClient : public AbstractClient boost::beast::multi_buffer bout_; unsigned rpcVersion_; + bool disconnected_ = false; + + // Errors that mean the persistent keep-alive connection was dropped by the + // server (rather than a genuine protocol failure), so the request can be + // safely retried on a fresh connection. + static bool + droppedConnection(boost::system::error_code const& ec) + { + namespace error = boost::asio::error; + static auto const kDroppedConnectionErrors = std::to_array({ + boost::beast::http::error::end_of_stream, + error::eof, + error::connection_reset, + error::connection_aborted, + error::broken_pipe, + error::not_connected, + }); + + return std::ranges::any_of( + kDroppedConnectionErrors, + [&ec](boost::system::error_code const& e) { return ec == e; }); + } + + // Tear down and re-establish the socket to ep_, discarding any buffered + // bytes left over from the dropped connection. + void + reconnect() + { + boost::system::error_code ec; + stream_.close(ec); + bin_.clear(); + stream_.connect(ep_); + } + public: explicit JSONRPCClient(Config const& cfg, unsigned rpcVersion) : ep_(getEndpoint(cfg)), stream_(ios_), rpcVersion_(rpcVersion) @@ -91,12 +130,10 @@ public: stream_.connect(ep_); } - /* - Return value is an Object type with up to three keys: - status - error - result - */ + // Return value is an Object type with up to three keys: + // status + // error + // result json::Value invoke(std::string const& cmd, json::Value const& params) override { @@ -104,6 +141,13 @@ public: using namespace boost::asio; using namespace std::string_literals; + // Once disconnect() has released the slot, the client must not be + // reused (see AbstractClient::disconnect). Refuse rather than let the + // failed write/read below trip the reconnect path and silently + // re-consume a connection slot, which would defeat disconnectClient(). + if (disconnected_) + Throw("JSONRPCClient::invoke called after disconnect()"); + request req; req.method(boost::beast::http::verb::post); req.target("/"); @@ -131,10 +175,29 @@ public: req.body() = to_string(jr); } req.prepare_payload(); - write(stream_, req); + // The client keeps a single keep-alive connection for its whole + // lifetime, but the server drops idle localhost connections after a few + // seconds (BaseHTTPPeer::kTimeoutSecondsLocal). If a slow gap between + // requests let the server close the socket, the write/read here fails + // with end_of_stream; reconnect and retry the request exactly once. response res; - read(stream_, bin_, res); + auto writeAndRead = [&] { + write(stream_, req); + read(stream_, bin_, res); + }; + try + { + writeAndRead(); + } + catch (boost::system::system_error const& e) + { + if (!droppedConnection(e.code())) + throw; + reconnect(); + res = {}; + writeAndRead(); + } json::Reader jr; json::Value jv; @@ -151,6 +214,19 @@ public: { return rpcVersion_; } + + void + disconnect() override + { + if (disconnected_) + return; + + disconnected_ = true; + + boost::system::error_code ec; + stream_.shutdown(boost::asio::ip::tcp::socket::shutdown_both, ec); + stream_.close(ec); + } }; std::unique_ptr diff --git a/src/test/jtx/impl/WSClient.cpp b/src/test/jtx/impl/WSClient.cpp index ca322415fb..a8702c12d9 100644 --- a/src/test/jtx/impl/WSClient.cpp +++ b/src/test/jtx/impl/WSClient.cpp @@ -2,6 +2,7 @@ #include +#include #include #include #include @@ -112,10 +113,11 @@ class WSClientImpl : public WSClient bool peerClosed_ = false; - // synchronize destructor - bool b0_ = false; - std::mutex m0_; - std::condition_variable cv0_; + // disconnect() waits on this until the read loop ends (for any reason: + // the server acknowledged our close, or a timeout force-closed the socket). + static constexpr auto kDisconnectTimeout = std::chrono::seconds{1}; + xrpl::Mutex readEnded_; + std::condition_variable readEndCv_; // synchronize message queue std::mutex m_; @@ -127,23 +129,26 @@ class WSClientImpl : public WSClient void cleanup() { - boost::asio::post(ios_, boost::asio::bind_executor(strand_, [this] { - if (!peerClosed_) - { - ws_.async_close( - {}, boost::asio::bind_executor(strand_, [&](error_code) { - try - { - stream_.cancel(); - } - // NOLINTNEXTLINE(bugprone-empty-catch) - catch (boost::system::system_error const&) - { - // ignored - } - })); - } - })); + boost::asio::post( + ios_, // + boost::asio::bind_executor(strand_, [this] { + if (!peerClosed_) + { + ws_.async_close( + {}, // + boost::asio::bind_executor(strand_, [&](error_code) { + try + { + stream_.cancel(); + } + // NOLINTNEXTLINE(bugprone-empty-catch) + catch (boost::system::system_error const&) + { + // ignored + } + })); + } + })); work_ = std::nullopt; thread_.join(); } @@ -289,6 +294,44 @@ public: return rpcVersion_; } + void + disconnect() override + { + // Perform a graceful WebSocket closing handshake and block until the + // read loop ends, so the server observes a clean close (not a RST) and + // has finished tearing the connection down by the time we return. + // If the server already closed, the wait below returns immediately. + boost::asio::post( + ios_, + boost::asio::bind_executor( + strand_, // + [this] { + if (!peerClosed_) + { + ws_.async_close( + boost::beast::websocket::close_code::normal, + boost::asio::bind_executor(strand_, [](error_code) {})); + } + })); + + auto lock = readEnded_.lock(); + readEndCv_.wait_for(lock, kDisconnectTimeout, [&lock] { return *lock; }); + + // On timeout (server gone or not replying) force the socket closed so + // the outstanding read ends and the worker thread can later be joined. + if (!*lock) + { + boost::asio::post( + ios_, + boost::asio::bind_executor( + strand_, // + [this] { + boost::system::error_code ec; + stream_.close(ec); + })); + } + } + private: void onReadMsg(error_code const& ec) @@ -297,33 +340,31 @@ private: { if (ec == boost::beast::websocket::error::closed) peerClosed_ = true; + + *readEnded_.lock() = true; + readEndCv_.notify_all(); + return; } json::Value jv; json::Reader jr; + jr.parse(bufferString(rb_.data()), jv); rb_.consume(rb_.size()); + auto m = std::make_shared(std::move(jv)); { std::scoped_lock const lock(m_); msgs_.push_front(m); cv_.notify_all(); } + ws_.async_read( rb_, boost::asio::bind_executor(strand_, [this](error_code const& ec, std::size_t) { onReadMsg(ec); })); } - - // Called when the read op terminates - void - onReadDone() - { - std::scoped_lock const lock(m0_); - b0_ = true; - cv0_.notify_all(); - } }; std::unique_ptr diff --git a/src/test/server/ServerStatus_test.cpp b/src/test/server/ServerStatus_test.cpp index 60ea622616..5adf6a08f5 100644 --- a/src/test/server/ServerStatus_test.cpp +++ b/src/test/server/ServerStatus_test.cpp @@ -558,10 +558,12 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En using namespace test::jtx; using namespace boost::asio; using namespace boost::beast::http; - Env env{*this, envconfig([&](std::unique_ptr cfg) { + // Run the server with a single io thread so disconnectClient() below + // can deterministically drain the server's io_context (see its docs). + Env env{*this, singleThreadIo(envconfig([&](std::unique_ptr cfg) { (*cfg)[Sections::kPortRpc].set(Keys::kLimit, std::to_string(limit)); return cfg; - })}; + }))}; auto const section = env.app().config().section(Sections::kPortRpc); // NOLINTBEGIN(bugprone-unchecked-optional-access) @@ -580,16 +582,27 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En BEAST_EXPECT(!ec); std::vector> clients; - int connectionCount{1}; // starts at 1 because the Env already has one - // for JSONRPCCLient - // for nonzero limits, go one past the limit, although failures happen - // at the limit, so this really leads to the last two clients failing. - // for zero limit, pick an arbitrary nonzero number of clients - all - // should connect fine. + // Env owns a persistent JSON-RPC HTTP client connection to port_rpc as + // part of startup, which counts against this port's connection limit. + // This test wants a known starting occupancy of zero, so for nonzero + // limits it deterministically drops that hidden client and waits for + // the server to register the disconnect before opening its own clients. + // + // Starting from zero is important because the port limit rejects once + // the incremented connection count reaches the configured limit. With a + // zero baseline and N = limit + 1 test-owned clients, exactly the last + // two requests should be rejected. + if (limit != 0) + BEAST_EXPECT(env.disconnectClient()); + + // For nonzero limits, go one past the limit. The port rejects at the + // limit, not only above it, so this yields the last two clients + // failing. For zero limit, pick an arbitrary nonzero number of clients + // and expect them all to succeed. int const testTo = (limit == 0) ? 50 : limit + 1; - while (connectionCount < testTo) + while (static_cast(clients.size()) < testTo) { clients.emplace_back(ip::tcp::socket{ios}, boost::beast::multi_buffer{}); async_connect(clients.back().first, it, yield[ec]); @@ -597,19 +610,24 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En auto req = makeHTTPRequest(ip, port, to_string(jr), {}); async_write(clients.back().first, req, yield[ec]); BEAST_EXPECT(!ec); - ++connectionCount; } - int readCount = 0; + int successfulReads = 0; for (auto& [soc, buf] : clients) { boost::beast::http::response resp; async_read(soc, buf, resp, yield[ec]); - ++readCount; - // expect the reads to fail for the clients that connected at or - // above the limit. If limit is 0, all reads should succeed - BEAST_EXPECT((limit == 0 || readCount < limit - 1) ? (!ec) : bool(ec)); + if (!ec) + ++successfulReads; } + + // This test cares about the exact number of accepted requests, not which + // specific client observed the rejection. With a zero baseline (the + // hidden Env client dropped above), the server accepts until the + // connection count reaches the limit: all clients for limit 0, else + // limit - 1 of the limit + 1 clients (the last two are rejected). + int const expectedReads = (limit == 0) ? static_cast(clients.size()) : limit - 1; + BEAST_EXPECT(successfulReads == expectedReads); } void From 29120dfcbd1a6903dc0bc129f57935433ccff717 Mon Sep 17 00:00:00 2001 From: Andrzej Budzanowski Date: Mon, 27 Jul 2026 15:00:14 +0200 Subject: [PATCH 05/11] test: Migrate `nodestore` tests from Beast to GTest (#7292) Co-authored-by: Marek Foss Co-authored-by: Alex Kremer --- .../scripts/levelization/results/ordering.txt | 3 - .github/scripts/rename/namespace.sh | 3 - include/xrpl/core/ServiceRegistry.h | 6 +- include/xrpl/nodestore/Backend.h | 4 +- include/xrpl/nodestore/Database.h | 6 +- include/xrpl/nodestore/DatabaseRotating.h | 6 +- include/xrpl/nodestore/DummyScheduler.h | 4 +- include/xrpl/nodestore/Factory.h | 4 +- include/xrpl/nodestore/Manager.h | 4 +- include/xrpl/nodestore/NodeObject.h | 2 +- include/xrpl/nodestore/Scheduler.h | 4 +- include/xrpl/nodestore/Task.h | 4 +- include/xrpl/nodestore/Types.h | 4 +- include/xrpl/nodestore/detail/BatchWriter.h | 4 +- .../xrpl/nodestore/detail/DatabaseNodeImp.h | 6 +- .../nodestore/detail/DatabaseRotatingImp.h | 6 +- include/xrpl/nodestore/detail/DecodedBlob.h | 4 +- include/xrpl/nodestore/detail/EncodedBlob.h | 8 +- include/xrpl/nodestore/detail/ManagerImp.h | 4 +- include/xrpl/nodestore/detail/codec.h | 6 +- include/xrpl/nodestore/detail/varint.h | 4 +- include/xrpl/shamap/Family.h | 4 +- src/benchmarks/libxrpl/nodestore/Backend.cpp | 4 +- src/benchmarks/libxrpl/nodestore/Database.cpp | 4 +- .../libxrpl/nodestore/NodeStoreBench.h | 4 +- src/libxrpl/nodestore/BatchWriter.cpp | 6 +- src/libxrpl/nodestore/Database.cpp | 14 +- src/libxrpl/nodestore/DatabaseNodeImp.cpp | 4 +- src/libxrpl/nodestore/DatabaseRotatingImp.cpp | 8 +- src/libxrpl/nodestore/DecodedBlob.cpp | 6 +- src/libxrpl/nodestore/DummyScheduler.cpp | 4 +- src/libxrpl/nodestore/ManagerImp.cpp | 6 +- .../nodestore/backend/MemoryFactory.cpp | 10 +- src/libxrpl/nodestore/backend/NuDBFactory.cpp | 6 +- src/libxrpl/nodestore/backend/NullFactory.cpp | 4 +- .../nodestore/backend/RocksDBFactory.cpp | 12 +- src/test/app/SHAMapStore_test.cpp | 6 +- src/test/nodestore/Backend_test.cpp | 110 ----- src/test/nodestore/Basics_test.cpp | 73 --- ...abase_test.cpp => DatabaseConfig_test.cpp} | 316 ++----------- src/test/nodestore/NuDBFactory_test.cpp | 443 ------------------ src/test/nodestore/TestBase.h | 202 -------- src/test/nodestore/import_test.cpp | 4 +- src/test/nodestore/varint_test.cpp | 57 --- src/tests/libxrpl/CMakeLists.txt | 1 + src/tests/libxrpl/helpers/CaptureSink.h | 50 ++ src/tests/libxrpl/helpers/TestFamily.h | 10 +- .../libxrpl/helpers/TestServiceRegistry.h | 2 +- src/tests/libxrpl/nodestore/Backend.cpp | 182 +++++++ src/tests/libxrpl/nodestore/Basics.cpp | 41 ++ src/tests/libxrpl/nodestore/Database.cpp | 248 ++++++++++ src/tests/libxrpl/nodestore/NuDBFactory.cpp | 297 ++++++++++++ src/tests/libxrpl/nodestore/TestBase.h | 169 +++++++ src/tests/libxrpl/nodestore/varint.cpp | 46 ++ src/tests/libxrpl/shamap/common.h | 10 +- src/xrpld/app/ledger/AccountStateSF.h | 4 +- src/xrpld/app/ledger/InboundLedger.h | 2 +- src/xrpld/app/ledger/TransactionStateSF.h | 4 +- src/xrpld/app/ledger/detail/InboundLedger.cpp | 2 +- src/xrpld/app/ledger/detail/LedgerMaster.cpp | 2 +- src/xrpld/app/main/Application.cpp | 10 +- src/xrpld/app/main/NodeStoreScheduler.cpp | 8 +- src/xrpld/app/main/NodeStoreScheduler.h | 10 +- src/xrpld/app/misc/SHAMapStore.h | 4 +- src/xrpld/app/misc/SHAMapStoreImp.cpp | 22 +- src/xrpld/app/misc/SHAMapStoreImp.h | 12 +- src/xrpld/shamap/NodeFamily.h | 6 +- 67 files changed, 1235 insertions(+), 1310 deletions(-) delete mode 100644 src/test/nodestore/Backend_test.cpp delete mode 100644 src/test/nodestore/Basics_test.cpp rename src/test/nodestore/{Database_test.cpp => DatabaseConfig_test.cpp} (59%) delete mode 100644 src/test/nodestore/NuDBFactory_test.cpp delete mode 100644 src/test/nodestore/TestBase.h delete mode 100644 src/test/nodestore/varint_test.cpp create mode 100644 src/tests/libxrpl/helpers/CaptureSink.h create mode 100644 src/tests/libxrpl/nodestore/Backend.cpp create mode 100644 src/tests/libxrpl/nodestore/Basics.cpp create mode 100644 src/tests/libxrpl/nodestore/Database.cpp create mode 100644 src/tests/libxrpl/nodestore/NuDBFactory.cpp create mode 100644 src/tests/libxrpl/nodestore/TestBase.h create mode 100644 src/tests/libxrpl/nodestore/varint.cpp diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index 709ba4d6d4..5577c363fd 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -130,12 +130,9 @@ test.ledger > xrpl.json test.ledger > xrpl.ledger test.ledger > xrpl.protocol test.nodestore > test.jtx -test.nodestore > test.unit_test test.nodestore > xrpl.basics -test.nodestore > xrpl.config test.nodestore > xrpld.core test.nodestore > xrpl.nodestore -test.nodestore > xrpl.protocol test.nodestore > xrpl.rdb test.overlay > test.jtx test.overlay > test.unit_test diff --git a/.github/scripts/rename/namespace.sh b/.github/scripts/rename/namespace.sh index bb186bc8bc..94a9205f76 100755 --- a/.github/scripts/rename/namespace.sh +++ b/.github/scripts/rename/namespace.sh @@ -46,9 +46,6 @@ for DIRECTORY in "${DIRECTORIES[@]}"; do done done -# Special case for NuDBFactory that has ripple twice in the test suite name. -${SED_COMMAND} -i -E 's/(BEAST_DEFINE_TESTSUITE.+)ripple(.+)/\1xrpl\2/g' src/test/nodestore/NuDBFactory_test.cpp - DIRECTORY=$1 find "${DIRECTORY}" -type f -name "*.md" | while read -r FILE; do echo "Processing file: ${FILE}" diff --git a/include/xrpl/core/ServiceRegistry.h b/include/xrpl/core/ServiceRegistry.h index 592964134b..2747ecd9e8 100644 --- a/include/xrpl/core/ServiceRegistry.h +++ b/include/xrpl/core/ServiceRegistry.h @@ -15,9 +15,9 @@ namespace xrpl { // Forward declarations -namespace NodeStore { +namespace node_store { class Database; -} // namespace NodeStore +} // namespace node_store namespace Resource { class Manager; } // namespace Resource @@ -164,7 +164,7 @@ public: getResourceManager() = 0; // Storage services - virtual NodeStore::Database& + virtual node_store::Database& getNodeStore() = 0; virtual SHAMapStore& diff --git a/include/xrpl/nodestore/Backend.h b/include/xrpl/nodestore/Backend.h index 564a874c5e..85d076bcfb 100644 --- a/include/xrpl/nodestore/Backend.h +++ b/include/xrpl/nodestore/Backend.h @@ -13,7 +13,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { /** * A backend used for the NodeStore. @@ -163,4 +163,4 @@ public: fdRequired() const = 0; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/Database.h b/include/xrpl/nodestore/Database.h index 96ba91bd76..902cfc9e03 100644 --- a/include/xrpl/nodestore/Database.h +++ b/include/xrpl/nodestore/Database.h @@ -25,7 +25,7 @@ namespace xrpl { class Section; } // namespace xrpl -namespace xrpl::NodeStore { +namespace xrpl::node_store { /** * Persistency layer for NodeObject @@ -248,7 +248,7 @@ protected: void storeStats(std::uint64_t count, std::uint64_t sz) { - XRPL_ASSERT(count <= sz, "xrpl::NodeStore::Database::storeStats : valid inputs"); + XRPL_ASSERT(count <= sz, "xrpl::node_store::Database::storeStats : valid inputs"); storeCount_ += count; storeSz_ += sz; } @@ -308,4 +308,4 @@ private: threadEntry(); }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/DatabaseRotating.h b/include/xrpl/nodestore/DatabaseRotating.h index 5381b5c435..21b8b422c7 100644 --- a/include/xrpl/nodestore/DatabaseRotating.h +++ b/include/xrpl/nodestore/DatabaseRotating.h @@ -9,7 +9,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { /* This class has two key-value store Backend objects for persisting SHAMap * records. This facilitates online deletion of data. New backends are @@ -38,7 +38,7 @@ public: */ virtual void rotate( - std::unique_ptr&& newBackend, + std::unique_ptr&& newBackend, std::function const& f) = 0; @@ -56,4 +56,4 @@ public: setRotationInFlight(bool inFlight) = 0; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/DummyScheduler.h b/include/xrpl/nodestore/DummyScheduler.h index 49b0d37462..fc7a040b5a 100644 --- a/include/xrpl/nodestore/DummyScheduler.h +++ b/include/xrpl/nodestore/DummyScheduler.h @@ -3,7 +3,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { /** * Simple NodeStore Scheduler that just performs the tasks synchronously. @@ -21,4 +21,4 @@ public: onBatchWrite(BatchWriteReport const& report) override; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/Factory.h b/include/xrpl/nodestore/Factory.h index a18023a8a8..568e9b0712 100644 --- a/include/xrpl/nodestore/Factory.h +++ b/include/xrpl/nodestore/Factory.h @@ -14,7 +14,7 @@ namespace xrpl { class Section; } // namespace xrpl -namespace xrpl::NodeStore { +namespace xrpl::node_store { /** * Base class for backend factories. @@ -70,4 +70,4 @@ public: } }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/Manager.h b/include/xrpl/nodestore/Manager.h index 54d99fe94b..1b869e4bca 100644 --- a/include/xrpl/nodestore/Manager.h +++ b/include/xrpl/nodestore/Manager.h @@ -10,7 +10,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { /** * Singleton for managing NodeStore factories and back ends. @@ -98,4 +98,4 @@ public: beast::Journal journal) = 0; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/NodeObject.h b/include/xrpl/nodestore/NodeObject.h index b96d65fa12..db139b9aa8 100644 --- a/include/xrpl/nodestore/NodeObject.h +++ b/include/xrpl/nodestore/NodeObject.h @@ -8,7 +8,7 @@ #include #include -// VFALCO NOTE Intentionally not in the NodeStore namespace +// VFALCO NOTE Intentionally not in the node_store namespace namespace xrpl { diff --git a/include/xrpl/nodestore/Scheduler.h b/include/xrpl/nodestore/Scheduler.h index 5d93a80eaa..40c36ce8ab 100644 --- a/include/xrpl/nodestore/Scheduler.h +++ b/include/xrpl/nodestore/Scheduler.h @@ -4,7 +4,7 @@ #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { enum class FetchType { Synchronous, Async }; @@ -71,4 +71,4 @@ public: onBatchWrite(BatchWriteReport const& report) = 0; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/Task.h b/include/xrpl/nodestore/Task.h index 59fe648476..22cec62eae 100644 --- a/include/xrpl/nodestore/Task.h +++ b/include/xrpl/nodestore/Task.h @@ -1,6 +1,6 @@ #pragma once -namespace xrpl::NodeStore { +namespace xrpl::node_store { /** * Derived classes perform scheduled tasks. @@ -17,4 +17,4 @@ struct Task performScheduledTask() = 0; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/Types.h b/include/xrpl/nodestore/Types.h index 872d948a36..21af6fa68b 100644 --- a/include/xrpl/nodestore/Types.h +++ b/include/xrpl/nodestore/Types.h @@ -5,7 +5,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { // This is only used to pre-allocate the array for // batch objects and does not affect the amount written. @@ -36,4 +36,4 @@ enum class Status { */ using Batch = std::vector>; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/detail/BatchWriter.h b/include/xrpl/nodestore/detail/BatchWriter.h index b89df0da14..6ac3428752 100644 --- a/include/xrpl/nodestore/detail/BatchWriter.h +++ b/include/xrpl/nodestore/detail/BatchWriter.h @@ -9,7 +9,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { /** * Batch-writing assist logic. @@ -86,4 +86,4 @@ private: Batch writeSet_; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/detail/DatabaseNodeImp.h b/include/xrpl/nodestore/detail/DatabaseNodeImp.h index 6f2fca682f..33a2e27939 100644 --- a/include/xrpl/nodestore/detail/DatabaseNodeImp.h +++ b/include/xrpl/nodestore/detail/DatabaseNodeImp.h @@ -22,7 +22,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { class DatabaseNodeImp : public Database { @@ -68,7 +68,7 @@ public: XRPL_ASSERT( backend_, - "xrpl::NodeStore::DatabaseNodeImp::DatabaseNodeImp : non-null " + "xrpl::node_store::DatabaseNodeImp::DatabaseNodeImp : non-null " "backend"); } @@ -138,4 +138,4 @@ private: } }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/detail/DatabaseRotatingImp.h b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h index ecbe9a513d..9b566195ef 100644 --- a/include/xrpl/nodestore/detail/DatabaseRotatingImp.h +++ b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h @@ -16,7 +16,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { class DatabaseRotatingImp : public DatabaseRotating { @@ -41,7 +41,7 @@ public: void rotate( - std::unique_ptr&& newBackend, + std::unique_ptr&& newBackend, std::function const& f) override; @@ -94,4 +94,4 @@ private: forEach(std::function)> f) override; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/detail/DecodedBlob.h b/include/xrpl/nodestore/detail/DecodedBlob.h index d0cc5e3404..bd90ff2f1b 100644 --- a/include/xrpl/nodestore/detail/DecodedBlob.h +++ b/include/xrpl/nodestore/detail/DecodedBlob.h @@ -4,7 +4,7 @@ #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { /** * Parsed key/value blob into NodeObject components. @@ -49,4 +49,4 @@ private: int dataBytes_; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/detail/EncodedBlob.h b/include/xrpl/nodestore/detail/EncodedBlob.h index d668cdccd8..d171df3cc9 100644 --- a/include/xrpl/nodestore/detail/EncodedBlob.h +++ b/include/xrpl/nodestore/detail/EncodedBlob.h @@ -12,7 +12,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { /** * Convert a NodeObject from in-memory to database format. @@ -68,7 +68,7 @@ class EncodedBlob public: explicit EncodedBlob(std::shared_ptr const& obj) : size_([&obj]() { - XRPL_ASSERT(obj, "xrpl::NodeStore::EncodedBlob::EncodedBlob : non-null input"); + XRPL_ASSERT(obj, "xrpl::node_store::EncodedBlob::EncodedBlob : non-null input"); if (!obj) throw std::runtime_error("EncodedBlob: unseated std::shared_ptr used."); @@ -88,7 +88,7 @@ public: XRPL_ASSERT( ((ptr_ == payload_.data()) && (size_ <= payload_.size())) || ((ptr_ != payload_.data()) && (size_ > payload_.size())), - "xrpl::NodeStore::EncodedBlob::~EncodedBlob : valid payload " + "xrpl::node_store::EncodedBlob::~EncodedBlob : valid payload " "pointer"); if (ptr_ != payload_.data()) @@ -114,4 +114,4 @@ public: } }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/detail/ManagerImp.h b/include/xrpl/nodestore/detail/ManagerImp.h index fc84b0aa57..f1653b45dc 100644 --- a/include/xrpl/nodestore/detail/ManagerImp.h +++ b/include/xrpl/nodestore/detail/ManagerImp.h @@ -13,7 +13,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { class ManagerImp : public Manager { @@ -57,4 +57,4 @@ public: beast::Journal journal) override; }; -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/detail/codec.h b/include/xrpl/nodestore/detail/codec.h index 2f69d532be..a3dfa7c944 100644 --- a/include/xrpl/nodestore/detail/codec.h +++ b/include/xrpl/nodestore/detail/codec.h @@ -21,7 +21,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { template std::pair @@ -269,7 +269,7 @@ nodeobjectCompress(void const* in, std::size_t inSize, BufferFactory&& bf) case 1: // lz4 { std::uint8_t* p = nullptr; - auto const lzr = NodeStore::lz4Compress(in, inSize, [&p, &vn, &bf](std::size_t n) { + auto const lzr = node_store::lz4Compress(in, inSize, [&p, &vn, &bf](std::size_t n) { p = reinterpret_cast(bf(vn + n)); return p + vn; }); @@ -316,4 +316,4 @@ filterInner(void* in, std::size_t inSize) } } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/nodestore/detail/varint.h b/include/xrpl/nodestore/detail/varint.h index 5a65545d3a..afbf71cdea 100644 --- a/include/xrpl/nodestore/detail/varint.h +++ b/include/xrpl/nodestore/detail/varint.h @@ -6,7 +6,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { // This is a variant of the base128 varint format from // google protocol buffers: @@ -123,4 +123,4 @@ write(nudb::detail::ostream& os, std::size_t t) writeVarint(os.data(sizeVarint(t)), t); } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/include/xrpl/shamap/Family.h b/include/xrpl/shamap/Family.h index 7624b3e600..467d41e4cc 100644 --- a/include/xrpl/shamap/Family.h +++ b/include/xrpl/shamap/Family.h @@ -26,10 +26,10 @@ public: explicit Family() = default; virtual ~Family() = default; - virtual NodeStore::Database& + virtual node_store::Database& db() = 0; - [[nodiscard]] virtual NodeStore::Database const& + [[nodiscard]] virtual node_store::Database const& db() const = 0; virtual beast::Journal const& diff --git a/src/benchmarks/libxrpl/nodestore/Backend.cpp b/src/benchmarks/libxrpl/nodestore/Backend.cpp index 7db0185053..f854dbd3a7 100644 --- a/src/benchmarks/libxrpl/nodestore/Backend.cpp +++ b/src/benchmarks/libxrpl/nodestore/Backend.cpp @@ -17,7 +17,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { namespace { constexpr std::size_t kPoolSizes[] = {1000, 10000, 100000}; @@ -326,4 +326,4 @@ registerStoreBatch(BackendConfig const& bc) }(); } // namespace -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/benchmarks/libxrpl/nodestore/Database.cpp b/src/benchmarks/libxrpl/nodestore/Database.cpp index 2303075ab9..cd4337b603 100644 --- a/src/benchmarks/libxrpl/nodestore/Database.cpp +++ b/src/benchmarks/libxrpl/nodestore/Database.cpp @@ -18,7 +18,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { namespace { // Number of distinct objects pre-generated per run. @@ -240,4 +240,4 @@ registerWorkload(BackendConfig const& bc, Workload const& w) }(); } // namespace -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h b/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h index fe6c2a350e..57abf42e89 100644 --- a/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h +++ b/src/benchmarks/libxrpl/nodestore/NodeStoreBench.h @@ -32,7 +32,7 @@ // Shared helpers for the NodeStore benchmarks. // -namespace xrpl::NodeStore { +namespace xrpl::node_store { // Fill `bytes` of memory at `buffer` with random bits drawn from `g`. template @@ -315,4 +315,4 @@ backendConfigs() return kConfigs; } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/BatchWriter.cpp b/src/libxrpl/nodestore/BatchWriter.cpp index e0a1fbf20c..92dfa09e0c 100644 --- a/src/libxrpl/nodestore/BatchWriter.cpp +++ b/src/libxrpl/nodestore/BatchWriter.cpp @@ -11,7 +11,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { BatchWriter::BatchWriter(Callback& callback, Scheduler& scheduler) : callback_(callback), scheduler_(scheduler) @@ -72,7 +72,7 @@ BatchWriter::writeBatch() writeSet_.swap(set); XRPL_ASSERT( - writeSet_.empty(), "xrpl::NodeStore::BatchWriter::writeBatch : writes not set"); + writeSet_.empty(), "xrpl::node_store::BatchWriter::writeBatch : writes not set"); writeLoad_ = set.size(); if (set.empty()) @@ -107,4 +107,4 @@ BatchWriter::waitForWriting() writeCondition_.wait(sl); } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/Database.cpp b/src/libxrpl/nodestore/Database.cpp index ac51dbfb2c..f9de660042 100644 --- a/src/libxrpl/nodestore/Database.cpp +++ b/src/libxrpl/nodestore/Database.cpp @@ -30,7 +30,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { Database::Database( Scheduler& scheduler, @@ -43,7 +43,7 @@ Database::Database( , requestBundle_(get(config, Keys::kRqBundle, 4)) , readThreads_(std::max(1, readThreads)) { - XRPL_ASSERT(readThreads, "xrpl::NodeStore::Database::Database : nonzero threads input"); + XRPL_ASSERT(readThreads, "xrpl::node_store::Database::Database : nonzero threads input"); if (earliestLedgerSeq_ < 1) Throw("Invalid earliest_seq"); @@ -89,7 +89,7 @@ Database::Database( { XRPL_ASSERT( !it->second.empty(), - "xrpl::NodeStore::Database::Database : non-empty " + "xrpl::node_store::Database::Database : non-empty " "data"); auto const& hash = it->first; @@ -164,7 +164,7 @@ Database::stop() { XRPL_ASSERT( steady_clock::now() - start < 30s, - "xrpl::NodeStore::Database::stop : maximum stop duration"); + "xrpl::node_store::Database::stop : maximum stop duration"); std::this_thread::yield(); } @@ -213,7 +213,7 @@ Database::importInternal(Backend& dstBackend, Database& srcDB) }; srcDB.forEach([&](std::shared_ptr nodeObject) { - XRPL_ASSERT(nodeObject, "xrpl::NodeStore::Database::importInternal : non-null node"); + XRPL_ASSERT(nodeObject, "xrpl::node_store::Database::importInternal : non-null node"); if (!nodeObject) // This should never happen return; @@ -257,7 +257,7 @@ Database::fetchNodeObject( void Database::getCountsJson(json::Value& obj) { - XRPL_ASSERT(obj.isObject(), "xrpl::NodeStore::Database::getCountsJson : valid input type"); + XRPL_ASSERT(obj.isObject(), "xrpl::node_store::Database::getCountsJson : valid input type"); { std::unique_lock const lock(readLock_); @@ -276,4 +276,4 @@ Database::getCountsJson(json::Value& obj) obj[jss::node_reads_duration_us] = std::to_string(fetchDurationUs_); } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/DatabaseNodeImp.cpp b/src/libxrpl/nodestore/DatabaseNodeImp.cpp index 9323d69131..1b880ac658 100644 --- a/src/libxrpl/nodestore/DatabaseNodeImp.cpp +++ b/src/libxrpl/nodestore/DatabaseNodeImp.cpp @@ -15,7 +15,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { void DatabaseNodeImp::store(NodeObjectType type, Blob&& data, uint256 const& hash, std::uint32_t) @@ -125,4 +125,4 @@ DatabaseNodeImp::fetchNodeObject( return nodeObject; } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/DatabaseRotatingImp.cpp b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp index 23a48a3bf3..81b1d6b297 100644 --- a/src/libxrpl/nodestore/DatabaseRotatingImp.cpp +++ b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp @@ -22,7 +22,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { DatabaseRotatingImp::DatabaseRotatingImp( Scheduler& scheduler, @@ -43,7 +43,7 @@ DatabaseRotatingImp::DatabaseRotatingImp( void DatabaseRotatingImp::rotate( - std::unique_ptr&& newBackend, + std::unique_ptr&& newBackend, std::function const& f) { // Pass these two names to the callback function @@ -52,7 +52,7 @@ DatabaseRotatingImp::rotate( // Hold on to current archive backend pointer until after the // callback finishes. Only then will the archive directory be // deleted. - std::shared_ptr oldArchiveBackend; + std::shared_ptr oldArchiveBackend; std::uint64_t copyForwards = 0; { std::scoped_lock const lock(mutex_); @@ -232,4 +232,4 @@ DatabaseRotatingImp::forEach(std::function)> f) archive->forEach(f); } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/DecodedBlob.cpp b/src/libxrpl/nodestore/DecodedBlob.cpp index 9740462ae8..321089d40f 100644 --- a/src/libxrpl/nodestore/DecodedBlob.cpp +++ b/src/libxrpl/nodestore/DecodedBlob.cpp @@ -10,7 +10,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { DecodedBlob::DecodedBlob(void const* key, void const* value, int valueBytes) : key_(key) { @@ -55,7 +55,7 @@ DecodedBlob::DecodedBlob(void const* key, void const* value, int valueBytes) : k std::shared_ptr DecodedBlob::createObject() { - XRPL_ASSERT(success_, "xrpl::NodeStore::DecodedBlob::createObject : valid object type"); + XRPL_ASSERT(success_, "xrpl::node_store::DecodedBlob::createObject : valid object type"); std::shared_ptr object; @@ -69,4 +69,4 @@ DecodedBlob::createObject() return object; } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/DummyScheduler.cpp b/src/libxrpl/nodestore/DummyScheduler.cpp index 1f93ed3d0f..32cd14cbdc 100644 --- a/src/libxrpl/nodestore/DummyScheduler.cpp +++ b/src/libxrpl/nodestore/DummyScheduler.cpp @@ -3,7 +3,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { void DummyScheduler::scheduleTask(Task& task) @@ -22,4 +22,4 @@ DummyScheduler::onBatchWrite(BatchWriteReport const& report) { } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/ManagerImp.cpp b/src/libxrpl/nodestore/ManagerImp.cpp index a3db22ce74..c78ccd5761 100644 --- a/src/libxrpl/nodestore/ManagerImp.cpp +++ b/src/libxrpl/nodestore/ManagerImp.cpp @@ -22,7 +22,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { ManagerImp& ManagerImp::instance() @@ -112,7 +112,7 @@ ManagerImp::erase(Factory& factory) std::scoped_lock const _(mutex_); auto const iter = std::ranges::find_if(list_, [&factory](Factory* other) { return other == &factory; }); - XRPL_ASSERT(iter != list_.end(), "xrpl::NodeStore::ManagerImp::erase : valid input"); + XRPL_ASSERT(iter != list_.end(), "xrpl::node_store::ManagerImp::erase : valid input"); list_.erase(iter); } @@ -135,4 +135,4 @@ Manager::instance() return ManagerImp::instance(); } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/backend/MemoryFactory.cpp b/src/libxrpl/nodestore/backend/MemoryFactory.cpp index 22557d652e..39d2123bc9 100644 --- a/src/libxrpl/nodestore/backend/MemoryFactory.cpp +++ b/src/libxrpl/nodestore/backend/MemoryFactory.cpp @@ -24,7 +24,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { struct MemoryDB { @@ -132,7 +132,7 @@ public: Status fetch(uint256 const& hash, std::shared_ptr* pObject) override { - XRPL_ASSERT(db_, "xrpl::NodeStore::MemoryBackend::fetch : non-null database"); + XRPL_ASSERT(db_, "xrpl::node_store::MemoryBackend::fetch : non-null database"); std::scoped_lock const _(db_->mutex); @@ -149,7 +149,7 @@ public: void store(std::shared_ptr const& object) override { - XRPL_ASSERT(db_, "xrpl::NodeStore::MemoryBackend::store : non-null database"); + XRPL_ASSERT(db_, "xrpl::node_store::MemoryBackend::store : non-null database"); std::scoped_lock const _(db_->mutex); db_->table.emplace(object->getHash(), object); } @@ -169,7 +169,7 @@ public: void forEach(std::function)> f) override { - XRPL_ASSERT(db_, "xrpl::NodeStore::MemoryBackend::forEach : non-null database"); + XRPL_ASSERT(db_, "xrpl::node_store::MemoryBackend::forEach : non-null database"); for (auto const& e : db_->table) f(e.second); } @@ -216,4 +216,4 @@ MemoryFactory::createInstance( return std::make_unique(keyBytes, keyValues, journal); } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/backend/NuDBFactory.cpp b/src/libxrpl/nodestore/backend/NuDBFactory.cpp index 38ea34258f..bbf37f3edf 100644 --- a/src/libxrpl/nodestore/backend/NuDBFactory.cpp +++ b/src/libxrpl/nodestore/backend/NuDBFactory.cpp @@ -44,7 +44,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { class NuDBBackend : public Backend { @@ -136,7 +136,7 @@ public: { // LCOV_EXCL_START UNREACHABLE( - "xrpl::NodeStore::NuDBBackend::open : database is already " + "xrpl::node_store::NuDBBackend::open : database is already " "open"); JLOG(j.error()) << "database is already open"; return; @@ -441,4 +441,4 @@ registerNuDBFactory(Manager& manager) static NuDBFactory const kInstance{manager}; } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/backend/NullFactory.cpp b/src/libxrpl/nodestore/backend/NullFactory.cpp index 0c76cb9938..feef3d37d0 100644 --- a/src/libxrpl/nodestore/backend/NullFactory.cpp +++ b/src/libxrpl/nodestore/backend/NullFactory.cpp @@ -13,7 +13,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { class NullBackend : public Backend { @@ -125,4 +125,4 @@ registerNullFactory(Manager& manager) static NullFactory const kInstance{manager}; } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/libxrpl/nodestore/backend/RocksDBFactory.cpp b/src/libxrpl/nodestore/backend/RocksDBFactory.cpp index 673b0daae0..4b7a1171fe 100644 --- a/src/libxrpl/nodestore/backend/RocksDBFactory.cpp +++ b/src/libxrpl/nodestore/backend/RocksDBFactory.cpp @@ -42,7 +42,7 @@ #include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { class RocksDBEnv : public rocksdb::EnvWrapper { @@ -231,7 +231,7 @@ public: { // LCOV_EXCL_START UNREACHABLE( - "xrpl::NodeStore::RocksDBBackend::open : database is already " + "xrpl::node_store::RocksDBBackend::open : database is already " "open"); JLOG(journal.error()) << "database is already open"; return; @@ -279,7 +279,7 @@ public: Status fetch(uint256 const& hash, std::shared_ptr* pObject) override { - XRPL_ASSERT(db, "xrpl::NodeStore::RocksDBBackend::fetch : non-null database"); + XRPL_ASSERT(db, "xrpl::node_store::RocksDBBackend::fetch : non-null database"); pObject->reset(); Status status = Status::Ok; @@ -339,7 +339,7 @@ public: { XRPL_ASSERT( db, - "xrpl::NodeStore::RocksDBBackend::storeBatch : non-null " + "xrpl::node_store::RocksDBBackend::storeBatch : non-null " "database"); rocksdb::WriteBatch wb; @@ -369,7 +369,7 @@ public: void forEach(std::function)> f) override { - XRPL_ASSERT(db, "xrpl::NodeStore::RocksDBBackend::forEach : non-null database"); + XRPL_ASSERT(db, "xrpl::node_store::RocksDBBackend::forEach : non-null database"); rocksdb::ReadOptions const options; std::unique_ptr it(db->NewIterator(options)); @@ -468,6 +468,6 @@ registerRocksDBFactory(Manager& manager) static RocksDBFactory const kInstance{manager}; } -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store #endif diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index c219bd8737..537ee4c177 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -489,7 +489,7 @@ public: lastRotated = ledgerSeq - 1; } - std::unique_ptr + std::unique_ptr makeBackendRotating(jtx::Env& env, NodeStoreScheduler& scheduler, std::string path) { Section section{env.app().config().section(Sections::kNodeDatabase)}; @@ -500,7 +500,7 @@ public: newPath = path; section.set(Keys::kPath, newPath.string()); - auto backend{NodeStore::Manager::instance().makeBackend( + auto backend{node_store::Manager::instance().makeBackend( section, megabytes(env.app().config().getValueFor(SizedItem::BurstSize, std::nullopt)), scheduler, @@ -549,7 +549,7 @@ public: auto archiveBackend = makeBackendRotating(env, scheduler, archiveDb); static constexpr int kReadThreads = 4; - auto dbr = std::make_unique( + auto dbr = std::make_unique( scheduler, kReadThreads, std::move(writableBackend), diff --git a/src/test/nodestore/Backend_test.cpp b/src/test/nodestore/Backend_test.cpp deleted file mode 100644 index 65601b0cf5..0000000000 --- a/src/test/nodestore/Backend_test.cpp +++ /dev/null @@ -1,110 +0,0 @@ -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include - -namespace xrpl::NodeStore { - -// Tests the Backend interface -// -class Backend_test : public TestBase -{ -public: - void - testBackend(std::string const& type, std::uint64_t const seedValue, int numObjsToTest = 2000) - { - DummyScheduler scheduler; - - testcase("Backend type=" + type); - - Section params; - beast::TempDir const tempDir; - params.set(Keys::kType, type); - params.set(Keys::kPath, tempDir.path()); - - beast::xor_shift_engine rng(seedValue); - - // Create a batch - auto batch = createPredictableBatch(numObjsToTest, rng()); - - using beast::Severity; - test::SuiteJournal journal("Backend_test", *this); - - { - // Open the backend - std::unique_ptr backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - backend->open(); - - // Write the batch - storeBatch(*backend, batch); - - { - // Read it back in - Batch copy; - fetchCopyOfBatch(*backend, ©, batch); - BEAST_EXPECT(areBatchesEqual(batch, copy)); - } - - { - // Reorder and read the copy again - std::shuffle(batch.begin(), batch.end(), rng); - Batch copy; - fetchCopyOfBatch(*backend, ©, batch); - BEAST_EXPECT(areBatchesEqual(batch, copy)); - } - } - - { - // Re-open the backend - std::unique_ptr backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - backend->open(); - - // Read it back in - Batch copy; - fetchCopyOfBatch(*backend, ©, batch); - // Canonicalize the source and destination batches - std::ranges::sort(batch, LessThan{}); - std::ranges::sort(copy, LessThan{}); - BEAST_EXPECT(areBatchesEqual(batch, copy)); - } - } - - //-------------------------------------------------------------------------- - - void - run() override - { - std::uint64_t const seedValue = 50; - - testBackend("nudb", seedValue); - -#if XRPL_ROCKSDB_AVAILABLE - testBackend("rocksdb", seedValue); -#endif - -#ifdef XRPL_ENABLE_SQLITE_BACKEND_TESTS - testBackend("sqlite", seedValue); -#endif - } -}; - -BEAST_DEFINE_TESTSUITE(Backend, nodestore, xrpl); - -} // namespace xrpl::NodeStore diff --git a/src/test/nodestore/Basics_test.cpp b/src/test/nodestore/Basics_test.cpp deleted file mode 100644 index 7d77b18630..0000000000 --- a/src/test/nodestore/Basics_test.cpp +++ /dev/null @@ -1,73 +0,0 @@ -#include - -#include -#include -#include -#include - -#include -#include - -namespace xrpl::NodeStore { - -// Tests predictable batches, and NodeObject blob encoding -// -class NodeStoreBasic_test : public TestBase -{ -public: - // Make sure predictable object generation works! - void - testBatches(std::uint64_t const seedValue) - { - testcase("batch"); - - auto batch1 = createPredictableBatch(kNumObjectsToTest, seedValue); - - auto batch2 = createPredictableBatch(kNumObjectsToTest, seedValue); - - BEAST_EXPECT(areBatchesEqual(batch1, batch2)); - - auto batch3 = createPredictableBatch(kNumObjectsToTest, seedValue + 1); - - BEAST_EXPECT(!areBatchesEqual(batch1, batch3)); - } - - // Checks encoding/decoding blobs - void - testBlobs(std::uint64_t const seedValue) - { - testcase("encoding"); - - auto batch = createPredictableBatch(kNumObjectsToTest, seedValue); - - for (auto const& expected : batch) - { - EncodedBlob const encoded(expected); - - DecodedBlob decoded(encoded.getKey(), encoded.getData(), encoded.getSize()); - - BEAST_EXPECT(decoded.wasOk()); - - if (decoded.wasOk()) - { - std::shared_ptr const object(decoded.createObject()); - - BEAST_EXPECT(isSame(expected, object)); - } - } - } - - void - run() override - { - std::uint64_t const seedValue = 50; - - testBatches(seedValue); - - testBlobs(seedValue); - } -}; - -BEAST_DEFINE_TESTSUITE(NodeStoreBasic, nodestore, xrpl); - -} // namespace xrpl::NodeStore diff --git a/src/test/nodestore/Database_test.cpp b/src/test/nodestore/DatabaseConfig_test.cpp similarity index 59% rename from src/test/nodestore/Database_test.cpp rename to src/test/nodestore/DatabaseConfig_test.cpp index bb8ec7d4fd..1f7f0f67bd 100644 --- a/src/test/nodestore/Database_test.cpp +++ b/src/test/nodestore/DatabaseConfig_test.cpp @@ -1,44 +1,21 @@ #include #include #include -#include -#include #include -#include #include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include #include -#include -#include -#include #include -#include -#include #include -namespace xrpl::NodeStore { +namespace xrpl::node_store { -class Database_test : public TestBase +class DatabaseConfig_test : public beast::unit_test::Suite { - test::SuiteJournal journal_; - public: - Database_test() : journal_("Database_test", *this) - { - } - void testConfig() { @@ -73,8 +50,8 @@ public: Env env = [&]() { auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kSafetyLevel, "high"); + auto& section = p->section("sqlite"); + section.set("safety_level", "high"); } p->ledgerHistory = 100'000'000; @@ -102,8 +79,8 @@ public: Env env = [&]() { auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kSafetyLevel, "low"); + auto& section = p->section("sqlite"); + section.set("safety_level", "low"); } p->ledgerHistory = 100'000'000; @@ -131,10 +108,10 @@ public: Env env = [&]() { auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kJournalMode, "off"); - section.set(Keys::kSynchronous, "extra"); - section.set(Keys::kTempStore, "default"); + auto& section = p->section("sqlite"); + section.set("journal_mode", "off"); + section.set("synchronous", "extra"); + section.set("temp_store", "default"); } return Env( @@ -145,7 +122,7 @@ public: }(); // No warning, even though higher risk settings were used because - // LEDGER_HISTORY is small + // ledgerHistory is small BEAST_EXPECT(!found); auto const s = setupDatabaseCon(env.app().config()); if (BEAST_EXPECT(s.globalPragma->size() == 3)) @@ -163,10 +140,10 @@ public: Env env = [&]() { auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kJournalMode, "off"); - section.set(Keys::kSynchronous, "extra"); - section.set(Keys::kTempStore, "default"); + auto& section = p->section("sqlite"); + section.set("journal_mode", "off"); + section.set("synchronous", "extra"); + section.set("temp_store", "default"); } p->ledgerHistory = 50'000'000; @@ -178,7 +155,7 @@ public: }(); // No warning, even though higher risk settings were used because - // LEDGER_HISTORY is small + // ledgerHistory is small BEAST_EXPECT(found); auto const s = setupDatabaseCon(env.app().config()); if (BEAST_EXPECT(s.globalPragma->size() == 3)) @@ -199,11 +176,11 @@ public: auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kSafetyLevel, "low"); - section.set(Keys::kJournalMode, "off"); - section.set(Keys::kSynchronous, "extra"); - section.set(Keys::kTempStore, "default"); + auto& section = p->section("sqlite"); + section.set("safety_level", "low"); + section.set("journal_mode", "off"); + section.set("synchronous", "extra"); + section.set("temp_store", "default"); } try @@ -230,9 +207,9 @@ public: auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kSafetyLevel, "high"); - section.set(Keys::kJournalMode, "off"); + auto& section = p->section("sqlite"); + section.set("safety_level", "high"); + section.set("journal_mode", "off"); } try @@ -259,9 +236,9 @@ public: auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kSafetyLevel, "low"); - section.set(Keys::kSynchronous, "extra"); + auto& section = p->section("sqlite"); + section.set("safety_level", "low"); + section.set("synchronous", "extra"); } try @@ -288,9 +265,9 @@ public: auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kSafetyLevel, "high"); - section.set(Keys::kTempStore, "default"); + auto& section = p->section("sqlite"); + section.set("safety_level", "high"); + section.set("temp_store", "default"); } try @@ -317,8 +294,8 @@ public: auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kSafetyLevel, "slow"); + auto& section = p->section("sqlite"); + section.set("safety_level", "slow"); } try @@ -345,8 +322,8 @@ public: auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kJournalMode, "fast"); + auto& section = p->section("sqlite"); + section.set("journal_mode", "fast"); } try @@ -373,8 +350,8 @@ public: auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kSynchronous, "instant"); + auto& section = p->section("sqlite"); + section.set("synchronous", "instant"); } try @@ -401,8 +378,8 @@ public: auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kTempStore, "network"); + auto& section = p->section("sqlite"); + section.set("temp_store", "network"); } try @@ -436,9 +413,9 @@ public: Env env = [&]() { auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kPageSize, "512"); - section.set(Keys::kJournalSizeLimit, "2582080"); + auto& section = p->section("sqlite"); + section.set("page_size", "512"); + section.set("journal_size_limit", "2582080"); } return Env(*this, std::move(p)); }(); @@ -457,8 +434,8 @@ public: bool found = false; auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kPageSize, "256"); + auto& section = p->section("sqlite"); + section.set("page_size", "256"); } try { @@ -480,8 +457,8 @@ public: bool found = false; auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kPageSize, "131072"); + auto& section = p->section("sqlite"); + section.set("page_size", "131072"); } try { @@ -503,8 +480,8 @@ public: bool found = false; auto p = test::jtx::envconfig(); { - auto& section = p->section(Sections::kSqlite); - section.set(Keys::kPageSize, "513"); + auto& section = p->section("sqlite"); + section.set("page_size", "513"); } try { @@ -522,208 +499,13 @@ public: } } - //-------------------------------------------------------------------------- - - void - testImport( - std::string const& destBackendType, - std::string const& srcBackendType, - std::int64_t seedValue) - { - DummyScheduler scheduler; - - beast::TempDir const nodeDb; - Section srcParams; - srcParams.set(Keys::kType, srcBackendType); - srcParams.set(Keys::kPath, nodeDb.path()); - - // Create a batch - auto batch = createPredictableBatch(kNumObjectsToTest, seedValue); - - // Write to source db - { - std::unique_ptr src = - Manager::instance().makeDatabase(megabytes(4), scheduler, 2, srcParams, journal_); - storeBatch(*src, batch); - } - - Batch copy; - - { - // Re-open the db - std::unique_ptr src = - Manager::instance().makeDatabase(megabytes(4), scheduler, 2, srcParams, journal_); - - // Set up the destination database - beast::TempDir const destDb; - Section destParams; - destParams.set(Keys::kType, destBackendType); - destParams.set(Keys::kPath, destDb.path()); - - std::unique_ptr dest = - Manager::instance().makeDatabase(megabytes(4), scheduler, 2, destParams, journal_); - - testcase("import into '" + destBackendType + "' from '" + srcBackendType + "'"); - - // Do the import - dest->importDatabase(*src); - - // Get the results of the import - fetchCopyOfBatch(*dest, ©, batch); - } - - // Canonicalize the source and destination batches - std::ranges::sort(batch, LessThan{}); - std::ranges::sort(copy, LessThan{}); - BEAST_EXPECT(areBatchesEqual(batch, copy)); - } - - //-------------------------------------------------------------------------- - - void - testNodeStore( - std::string const& type, - bool const testPersistence, - std::int64_t const seedValue, - int numObjsToTest = 2000) - { - DummyScheduler scheduler; - - std::string const s = "NodeStore backend '" + type + "'"; - - testcase(s); - - beast::TempDir const nodeDb; - Section nodeParams; - nodeParams.set(Keys::kType, type); - nodeParams.set(Keys::kPath, nodeDb.path()); - - beast::xor_shift_engine rng(seedValue); - - // Create a batch - auto batch = createPredictableBatch(numObjsToTest, rng()); - - { - // Open the database - std::unique_ptr db = - Manager::instance().makeDatabase(megabytes(4), scheduler, 2, nodeParams, journal_); - - // Write the batch - storeBatch(*db, batch); - - { - // Read it back in - Batch copy; - fetchCopyOfBatch(*db, ©, batch); - BEAST_EXPECT(areBatchesEqual(batch, copy)); - } - - { - // Reorder and read the copy again - std::shuffle(batch.begin(), batch.end(), rng); - Batch copy; - fetchCopyOfBatch(*db, ©, batch); - BEAST_EXPECT(areBatchesEqual(batch, copy)); - } - } - - if (testPersistence) - { - // Re-open the database without the ephemeral DB - std::unique_ptr db = - Manager::instance().makeDatabase(megabytes(4), scheduler, 2, nodeParams, journal_); - - // Read it back in - Batch copy; - fetchCopyOfBatch(*db, ©, batch); - - // Canonicalize the source and destination batches - std::ranges::sort(batch, LessThan{}); - std::ranges::sort(copy, LessThan{}); - BEAST_EXPECT(areBatchesEqual(batch, copy)); - } - - if (type == "memory") - { - // Verify default earliest ledger sequence - { - std::unique_ptr db = Manager::instance().makeDatabase( - megabytes(4), scheduler, 2, nodeParams, journal_); - BEAST_EXPECT(db->earliestLedgerSeq() == kXrpLedgerEarliestSeq); - } - - // Set an invalid earliest ledger sequence - try - { - nodeParams.set(Keys::kEarliestSeq, "0"); - std::unique_ptr const db = Manager::instance().makeDatabase( - megabytes(4), scheduler, 2, nodeParams, journal_); - } - catch (std::runtime_error const& e) - { - BEAST_EXPECT(std::strcmp(e.what(), "Invalid earliest_seq") == 0); - } - - { - // Set a valid earliest ledger sequence - nodeParams.set(Keys::kEarliestSeq, "1"); - std::unique_ptr db = Manager::instance().makeDatabase( - megabytes(4), scheduler, 2, nodeParams, journal_); - - // Verify database uses the earliest ledger sequence setting - BEAST_EXPECT(db->earliestLedgerSeq() == 1); - } - - // Create another database that attempts to set the value again - try - { - // Set to default earliest ledger sequence - nodeParams.set(Keys::kEarliestSeq, std::to_string(kXrpLedgerEarliestSeq)); - std::unique_ptr const db2 = Manager::instance().makeDatabase( - megabytes(4), scheduler, 2, nodeParams, journal_); - } - catch (std::runtime_error const& e) - { - BEAST_EXPECT(std::strcmp(e.what(), "earliest_seq set more than once") == 0); - } - } - } - - //-------------------------------------------------------------------------- - void run() override { - std::int64_t const seedValue = 50; - testConfig(); - - testNodeStore("memory", false, seedValue); - - // Persistent backend tests - { - testNodeStore("nudb", true, seedValue); - -#if XRPL_ROCKSDB_AVAILABLE - testNodeStore("rocksdb", true, seedValue); -#endif - } - - // Import tests - { - testImport("nudb", "nudb", seedValue); - -#if XRPL_ROCKSDB_AVAILABLE - testImport("rocksdb", "rocksdb", seedValue); -#endif - -#if XRPL_ENABLE_SQLITE_BACKEND_TESTS - testImport("sqlite", "sqlite", seedValue); -#endif - } } }; -BEAST_DEFINE_TESTSUITE(Database, nodestore, xrpl); +BEAST_DEFINE_TESTSUITE(DatabaseConfig, nodestore, xrpl); -} // namespace xrpl::NodeStore +} // namespace xrpl::node_store diff --git a/src/test/nodestore/NuDBFactory_test.cpp b/src/test/nodestore/NuDBFactory_test.cpp deleted file mode 100644 index d0675b3893..0000000000 --- a/src/test/nodestore/NuDBFactory_test.cpp +++ /dev/null @@ -1,443 +0,0 @@ -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include -#include -#include - -namespace xrpl::NodeStore { - -class NuDBFactory_test : public TestBase -{ -private: - // Helper function to create a Section with specified parameters - static Section - createSection(std::string const& path, std::string const& blockSize = "") - { - Section params; - params.set(Keys::kType, "nudb"); - params.set(Keys::kPath, path); - if (!blockSize.empty()) - params.set(Keys::kNudbBlockSize, blockSize); - return params; - } - - // Helper function to create a backend and test basic functionality - bool - testBackendFunctionality(Section const& params, std::size_t expectedBlocksize) - { - try - { - DummyScheduler scheduler; - test::SuiteJournal journal("NuDBFactory_test", *this); - - auto backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - - if (!BEAST_EXPECT(backend)) - return false; - - if (!BEAST_EXPECT(backend->getBlockSize() == expectedBlocksize)) - return false; - - backend->open(); - - if (!BEAST_EXPECT(backend->isOpen())) - return false; - - // Test basic store/fetch functionality - auto batch = createPredictableBatch(10, 12345); - storeBatch(*backend, batch); - - Batch copy; - fetchCopyOfBatch(*backend, ©, batch); - - backend->close(); - - return areBatchesEqual(batch, copy); - } - catch (...) - { - return false; - } - } - - // Helper function to test log messages - void - testLogMessage(Section const& params, beast::Severity level, std::string const& expectedMessage) - { - test::StreamSink sink(level); - beast::Journal const journal(sink); - - DummyScheduler scheduler; - auto backend = Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - - std::string const logOutput = sink.messages().str(); - BEAST_EXPECT(logOutput.contains(expectedMessage)); - } - - // Helper function to test power of two validation - void - testPowerOfTwoValidation(std::string const& size, bool shouldWork) - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), size); - - test::StreamSink sink(beast::Severity::Warning); - beast::Journal const journal(sink); - - DummyScheduler scheduler; - auto backend = Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - - std::string const logOutput = sink.messages().str(); - bool const hasWarning = logOutput.contains("Invalid nudb_block_size"); - - BEAST_EXPECT(hasWarning == !shouldWork); - } - -public: - void - testDefaultBlockSize() - { - testcase("Default block size (no nudb_block_size specified)"); - - beast::TempDir const tempDir; - auto params = createSection(tempDir.path()); - - // Should work with default 4096 block size - BEAST_EXPECT(testBackendFunctionality(params, 4096)); - } - - void - testValidBlockSizes() - { - testcase("Valid block sizes"); - - std::vector const validSizes = {4096, 8192, 16384, 32768}; - - for (auto const& size : validSizes) - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), to_string(size)); - - BEAST_EXPECT(testBackendFunctionality(params, size)); - } - // Empty value is ignored by the config parser, so uses the - // default - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), ""); - - BEAST_EXPECT(testBackendFunctionality(params, 4096)); - } - - void - testInvalidBlockSizes() - { - testcase("Invalid block sizes"); - - std::vector const invalidSizes = { - "2048", // Too small - "1024", // Too small - "65536", // Too large - "131072", // Too large - "5000", // Not power of 2 - "6000", // Not power of 2 - "10000", // Not power of 2 - "0", // Zero - "-1", // Negative - "abc", // Non-numeric - "4k", // Invalid format - "4096.5" // Decimal - }; - - for (auto const& size : invalidSizes) - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), size); - - // Fails - BEAST_EXPECT(!testBackendFunctionality(params, 4096)); - } - - // Test whitespace cases separately since lexical_cast may handle them - std::vector const whitespaceInvalidSizes = { - "4096 ", // Trailing space - might be handled by lexical_cast - " 4096" // Leading space - might be handled by lexical_cast - }; - - for (auto const& size : whitespaceInvalidSizes) - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), size); - - // Fails - BEAST_EXPECT(!testBackendFunctionality(params, 4096)); - } - } - - void - testLogMessages() - { - testcase("Log message verification"); - - // Test valid custom block size logging - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), "8192"); - - testLogMessage(params, beast::Severity::Info, "Using custom NuDB block size: 8192"); - } - - // Test invalid block size failure - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), "5000"); - - test::StreamSink sink(beast::Severity::Warning); - beast::Journal const journal(sink); - - DummyScheduler scheduler; - try - { - auto backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - fail(); - } - catch (std::exception const& e) - { - std::string const logOutput{e.what()}; - BEAST_EXPECT(logOutput.contains("Invalid nudb_block_size: 5000")); - BEAST_EXPECT(logOutput.contains("Must be power of 2 between 4096 and 32768")); - } - } - - // Test non-numeric value failure - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), "invalid"); - - test::StreamSink sink(beast::Severity::Warning); - beast::Journal const journal(sink); - - DummyScheduler scheduler; - try - { - auto backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - - fail(); - } - catch (std::exception const& e) - { - std::string const logOutput{e.what()}; - BEAST_EXPECT(logOutput.contains("Invalid nudb_block_size value: invalid")); - } - } - } - - void - testPowerOfTwoValidation() - { - testcase("Power of 2 validation logic"); - - // Test edge cases around valid range - std::vector> const testCases = { - {"4095", false}, // Just below minimum - {"4096", true}, // Minimum valid - {"4097", false}, // Just above minimum, not power of 2 - {"8192", true}, // Valid power of 2 - {"8193", false}, // Just above valid power of 2 - {"16384", true}, // Valid power of 2 - {"32768", true}, // Maximum valid - {"32769", false}, // Just above maximum - {"65536", false} // Power of 2 but too large - }; - - for (auto const& [size, shouldWork] : testCases) - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), size); - - // We test the validation logic by catching exceptions for invalid - // values - test::StreamSink sink(beast::Severity::Warning); - beast::Journal const journal(sink); - - DummyScheduler scheduler; - try - { - auto backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - BEAST_EXPECT(shouldWork); - } - catch (std::exception const& e) - { - std::string const logOutput{e.what()}; - BEAST_EXPECT(logOutput.contains("Invalid nudb_block_size")); - } - } - } - - void - testBothConstructorVariants() - { - testcase("Both constructor variants work with custom block size"); - - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), "16384"); - - DummyScheduler scheduler; - test::SuiteJournal journal("NuDBFactory_test", *this); - - // Test first constructor (without nudb::context) - { - auto backend1 = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - BEAST_EXPECT(backend1 != nullptr); - BEAST_EXPECT(testBackendFunctionality(params, 16384)); - } - - // Test second constructor (with nudb::context) - // Note: This would require access to nudb::context, which might not be - // easily testable without more complex setup. For now, we test that - // the factory can create backends with the first constructor. - } - - void - testConfigurationParsing() - { - testcase("Configuration parsing edge cases"); - - // Test that whitespace is handled correctly - std::vector const validFormats = { - "8192" // Basic valid format - }; - - // Test whitespace handling separately since lexical_cast behavior may - // vary - std::vector const whitespaceFormats = { - " 8192", // Leading space - may or may not be handled by - // lexical_cast - "8192 " // Trailing space - may or may not be handled by - // lexical_cast - }; - - // Test basic valid format - for (auto const& format : validFormats) - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), format); - - test::StreamSink sink(beast::Severity::Info); - beast::Journal const journal(sink); - - DummyScheduler scheduler; - auto backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - - // Should log success message for valid values - std::string const logOutput = sink.messages().str(); - bool const hasSuccessMessage = logOutput.contains("Using custom NuDB block size"); - BEAST_EXPECT(hasSuccessMessage); - } - - // Test whitespace formats - these should work if lexical_cast handles - // them - for (auto const& format : whitespaceFormats) - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), format); - - // Use a lower threshold to capture both info and warning messages - test::StreamSink sink(beast::Severity::Debug); - beast::Journal const journal(sink); - - DummyScheduler scheduler; - try - { - auto backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - fail(); - } - catch (...) - { - // Fails - BEAST_EXPECT(!testBackendFunctionality(params, 8192)); - } - } - } - - void - testDataPersistence() - { - testcase("Data persistence with different block sizes"); - - std::vector const blockSizes = {"4096", "8192", "16384", "32768"}; - - for (auto const& size : blockSizes) - { - beast::TempDir const tempDir; - auto params = createSection(tempDir.path(), size); - - DummyScheduler scheduler; - test::SuiteJournal journal("NuDBFactory_test", *this); - - // Create test data - auto batch = createPredictableBatch(50, 54321); - - // Store data - { - auto backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - backend->open(); - storeBatch(*backend, batch); - backend->close(); - } - - // Retrieve data in new backend instance - { - auto backend = - Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); - backend->open(); - - Batch copy; - fetchCopyOfBatch(*backend, ©, batch); - - BEAST_EXPECT(areBatchesEqual(batch, copy)); - backend->close(); - } - } - } - - void - run() override - { - testDefaultBlockSize(); - testValidBlockSizes(); - testInvalidBlockSizes(); - testLogMessages(); - testPowerOfTwoValidation(); - testBothConstructorVariants(); - testConfigurationParsing(); - testDataPersistence(); - } -}; - -BEAST_DEFINE_TESTSUITE(NuDBFactory, xrpl_core, xrpl); - -} // namespace xrpl::NodeStore diff --git a/src/test/nodestore/TestBase.h b/src/test/nodestore/TestBase.h deleted file mode 100644 index 235e76501f..0000000000 --- a/src/test/nodestore/TestBase.h +++ /dev/null @@ -1,202 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -#include -#include -#include -#include - -namespace xrpl::NodeStore { - -/** - * Binary function that satisfies the strict-weak-ordering requirement. - * - * This compares the hashes of both objects and returns true if - * the first hash is considered to go before the second. - * - * @see std::sort - */ -struct LessThan -{ - bool - operator()(std::shared_ptr const& lhs, std::shared_ptr const& rhs) - const noexcept - { - return lhs->getHash() < rhs->getHash(); - } -}; - -/** - * Returns `true` if objects are identical. - */ -inline bool -isSame(std::shared_ptr const& lhs, std::shared_ptr const& rhs) -{ - return (lhs->getType() == rhs->getType()) && (lhs->getHash() == rhs->getHash()) && - (lhs->getData() == rhs->getData()); -} - -// Some common code for the unit tests -// -class TestBase : public beast::unit_test::Suite -{ -public: - // Tunable parameters - // - static std::size_t const kMinPayloadBytes = 1; - static std::size_t const kMaxPayloadBytes = 2000; - static int const kNumObjectsToTest = 2000; - -public: - // Create a predictable batch of objects - static Batch - createPredictableBatch(int numObjects, std::uint64_t seed) - { - Batch batch; - batch.reserve(numObjects); - - beast::xor_shift_engine rng(seed); - - for (int i = 0; i < numObjects; ++i) - { - NodeObjectType const type = [&] { - switch (randInt(rng, 3)) - { - case 0: - return NodeObjectType::Ledger; - case 1: - return NodeObjectType::AccountNode; - case 2: - return NodeObjectType::TransactionNode; - case 3: - default: - return NodeObjectType::Unknown; - } - }(); - - uint256 hash; - beast::rngfill(hash.begin(), hash.size(), rng); - - Blob blob(randInt(rng, kMinPayloadBytes, kMaxPayloadBytes)); - beast::rngfill(blob.data(), blob.size(), rng); - - batch.push_back(NodeObject::createObject(type, std::move(blob), hash)); - } - - return batch; - } - - // Compare two batches for equality - static bool - areBatchesEqual(Batch const& lhs, Batch const& rhs) - { - bool result = true; - - if (lhs.size() == rhs.size()) - { - for (int i = 0; i < lhs.size(); ++i) - { - if (!isSame(lhs[i], rhs[i])) - { - result = false; - break; - } - } - } - else - { - result = false; - } - - return result; - } - - // Store a batch in a backend - static void - storeBatch(Backend& backend, Batch const& batch) - { - for (auto const& object : batch) - { - backend.store(object); - } - } - - // Get a copy of a batch in a backend - void - fetchCopyOfBatch(Backend& backend, Batch* pCopy, Batch const& batch) - { - pCopy->clear(); - pCopy->reserve(batch.size()); - - for (auto const& expected : batch) - { - std::shared_ptr object; - - Status const status = backend.fetch(expected->getHash(), &object); - - BEAST_EXPECT(status == Status::Ok); - - if (status == Status::Ok) - { - BEAST_EXPECT(object != nullptr); - - pCopy->push_back(object); - } - } - } - - void - fetchMissing(Backend& backend, Batch const& batch) - { - for (auto const& expected : batch) - { - std::shared_ptr object; - - Status const status = backend.fetch(expected->getHash(), &object); - - BEAST_EXPECT(status == Status::NotFound); - } - } - - // Store all objects in a batch - static void - storeBatch(Database& db, Batch const& batch) - { - for (auto const& object : batch) - { - Blob data(object->getData()); - - db.store(object->getType(), std::move(data), object->getHash(), db.earliestLedgerSeq()); - } - } - - // Fetch all the hashes in one batch, into another batch. - static void - fetchCopyOfBatch(Database& db, Batch* pCopy, Batch const& batch) - { - pCopy->clear(); - pCopy->reserve(batch.size()); - - for (auto const& expected : batch) - { - std::shared_ptr const object = db.fetchNodeObject(expected->getHash(), 0); - - if (object != nullptr) - pCopy->push_back(object); - } - } -}; - -} // namespace xrpl::NodeStore diff --git a/src/test/nodestore/import_test.cpp b/src/test/nodestore/import_test.cpp index d8c4a96713..c30d77029d 100644 --- a/src/test/nodestore/import_test.cpp +++ b/src/test/nodestore/import_test.cpp @@ -195,7 +195,7 @@ fmtdur(std::chrono::duration const& d) } // namespace detail -namespace NodeStore { +namespace node_store { //------------------------------------------------------------------------------ @@ -552,5 +552,5 @@ BEAST_DEFINE_TESTSUITE_MANUAL(import, nodestore, xrpl); //------------------------------------------------------------------------------ -} // namespace NodeStore +} // namespace node_store } // namespace xrpl diff --git a/src/test/nodestore/varint_test.cpp b/src/test/nodestore/varint_test.cpp deleted file mode 100644 index 68e88d831a..0000000000 --- a/src/test/nodestore/varint_test.cpp +++ /dev/null @@ -1,57 +0,0 @@ -#include -#include - -#include -#include -#include -#include - -namespace xrpl::NodeStore::tests { - -class varint_test : public beast::unit_test::Suite -{ -public: - void - testVarints(std::vector vv) - { - testcase("encode, decode"); - for (auto const v : vv) - { - std::array::kMax> vi{}; - auto const n0 = writeVarint(vi.data(), v); - expect(n0 > 0, "write error"); - expect(n0 == sizeVarint(v), "size error"); - std::size_t v1 = 0; - auto const n1 = readVarint(vi.data(), n0, v1); - expect(n1 == n0, "read error"); - expect(v == v1, "wrong value"); - } - } - - void - run() override - { - testVarints( - {0, - 1, - 2, - 126, - 127, - 128, - 253, - 254, - 255, - 16127, - 16128, - 16129, - 0xff, - 0xffff, - 0xffffffff, - 0xffffffffffffUL, - 0xffffffffffffffffUL}); - } -}; - -BEAST_DEFINE_TESTSUITE(varint, nodestore, xrpl); - -} // namespace xrpl::NodeStore::tests diff --git a/src/tests/libxrpl/CMakeLists.txt b/src/tests/libxrpl/CMakeLists.txt index 4828e03815..8e4ece1234 100644 --- a/src/tests/libxrpl/CMakeLists.txt +++ b/src/tests/libxrpl/CMakeLists.txt @@ -35,6 +35,7 @@ set(test_modules shamap tx protocol_autogen + nodestore ) if(NOT WIN32) list(APPEND test_modules net) diff --git a/src/tests/libxrpl/helpers/CaptureSink.h b/src/tests/libxrpl/helpers/CaptureSink.h new file mode 100644 index 0000000000..9918d13f9e --- /dev/null +++ b/src/tests/libxrpl/helpers/CaptureSink.h @@ -0,0 +1,50 @@ +#pragma once + +#include + +#include +#include +#include + +namespace xrpl::test { + +class CaptureSink : public beast::Journal::Sink +{ + mutable std::mutex mutex_; + std::stringstream strm_; + +public: + explicit CaptureSink(beast::Severity threshold = beast::Severity::Debug) + : Sink{threshold, false} + { + } + + void + write(beast::Severity level, std::string const& text) override + { + if (level < threshold()) + return; + writeAlways(level, text); + } + + void + writeAlways(beast::Severity /*level*/, std::string const& text) override + { + // Journal sinks may be written to concurrently (e.g. from a backend's background workers), + // so serialize access to strm_. write() funnels into writeAlways(), so the lock lives here + // only: locking in both would self-deadlock on this non-recursive mutex. + std::scoped_lock const lock(mutex_); + strm_ << text << '\n'; + } + + [[nodiscard]] std::string + messages() const + { + // Returns a snapshot of the captured output. Takes the lock so the read is safe even if a + // writer is still active. + std::scoped_lock const lock(mutex_); + return strm_.str(); + } +}; + +} // namespace xrpl::test diff --git a/src/tests/libxrpl/helpers/TestFamily.h b/src/tests/libxrpl/helpers/TestFamily.h index 1a11d3bb68..8a599ab4da 100644 --- a/src/tests/libxrpl/helpers/TestFamily.h +++ b/src/tests/libxrpl/helpers/TestFamily.h @@ -29,11 +29,11 @@ namespace xrpl::test { class TestFamily : public Family { private: - std::unique_ptr db_; + std::unique_ptr db_; TestStopwatch clock_; std::shared_ptr fbCache_; std::shared_ptr tnCache_; - NodeStore::DummyScheduler scheduler_; + node_store::DummyScheduler scheduler_; beast::Journal j_; public: @@ -51,16 +51,16 @@ public: Section config; config.set(Keys::kType, "memory"); config.set(Keys::kPath, "TestFamily"); - db_ = NodeStore::Manager::instance().makeDatabase(megabytes(4), scheduler_, 1, config, j); + db_ = node_store::Manager::instance().makeDatabase(megabytes(4), scheduler_, 1, config, j); } - NodeStore::Database& + node_store::Database& db() override { return *db_; } - [[nodiscard]] NodeStore::Database const& + [[nodiscard]] node_store::Database const& db() const override { return *db_; diff --git a/src/tests/libxrpl/helpers/TestServiceRegistry.h b/src/tests/libxrpl/helpers/TestServiceRegistry.h index 5475b54dc6..f7b09bccd1 100644 --- a/src/tests/libxrpl/helpers/TestServiceRegistry.h +++ b/src/tests/libxrpl/helpers/TestServiceRegistry.h @@ -220,7 +220,7 @@ public: } // Storage services - NodeStore::Database& + node_store::Database& getNodeStore() override { throw std::logic_error("TestServiceRegistry::getNodeStore() not implemented"); diff --git a/src/tests/libxrpl/nodestore/Backend.cpp b/src/tests/libxrpl/nodestore/Backend.cpp new file mode 100644 index 0000000000..eb78851429 --- /dev/null +++ b/src/tests/libxrpl/nodestore/Backend.cpp @@ -0,0 +1,182 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace xrpl::node_store { + +namespace { + +std::vector +backendTypes() +{ + std::vector types{"nudb"}; +#if XRPL_ROCKSDB_AVAILABLE + types.emplace_back("rocksdb"); +#endif +#ifdef XRPL_ENABLE_SQLITE_BACKEND_TESTS + types.emplace_back("sqlite"); +#endif + return types; +} + +// Run work(i) for every i in [0, n) spread across numThreads threads, handing +// out indices via a shared atomic counter (mirrors the old Timing_test +// parallel-for so the N items are partitioned, not duplicated). +template +void +parallelFor(std::size_t n, std::size_t numThreads, Work work) +{ + std::atomic next{0}; + auto const runner = [&] { + for (std::size_t i = next++; i < n; i = next++) + work(i); + }; + + auto threads = std::views::iota(std::size_t{0}, numThreads) | + std::views::transform([&](std::size_t) { return std::thread{runner}; }) | + std::ranges::to(); + + std::ranges::for_each(threads, &std::thread::join); +} + +} // namespace + +class BackendTypeTest : public ::testing::TestWithParam +{ +protected: + void + SetUp() override + { + params_.set("type", GetParam()); + params_.set("path", tempDir_.path()); + + beast::xor_shift_engine rng(kSeedValue); + batch_ = createPredictableBatch(kNumObjects, rng()); + } + + std::unique_ptr + makeOpenBackend() + { + auto backend = Manager::instance().makeBackend(params_, megabytes(4), scheduler_, journal_); + backend->open(); + return backend; + } + + DummyScheduler scheduler_; + beast::TempDir const tempDir_; + beast::Journal const journal_{TestSink::instance()}; + Section params_; + Batch batch_; +}; + +TEST_P(BackendTypeTest, store_and_fetch) +{ + auto backend = makeOpenBackend(); + storeBatch(*backend, batch_); + + { + SCOPED_TRACE("read in original order"); + auto const copy = fetchCopyOfBatch(*backend, batch_); + EXPECT_EQ(batch_, copy); + } + + { + SCOPED_TRACE("read in shuffled order"); + beast::xor_shift_engine rng(kSeedValue); + std::shuffle(batch_.begin(), batch_.end(), rng); + auto const copy = fetchCopyOfBatch(*backend, batch_); + EXPECT_EQ(batch_, copy); + } +} + +TEST_P(BackendTypeTest, persists_after_reopen) +{ + { + auto backend = makeOpenBackend(); + storeBatch(*backend, batch_); + } + + // re-open a fresh backend instance over the same path + auto backend = makeOpenBackend(); + auto copy = fetchCopyOfBatch(*backend, batch_); + std::ranges::sort(batch_, LessThan{}); + std::ranges::sort(copy, LessThan{}); + EXPECT_EQ(batch_, copy); +} + +// missing-key path. Replaces the correctness half of Timing_test::doMissing +// (and the missing branch of doMixed): every fetch on an empty backend must +// report Status::NotFound. +TEST_P(BackendTypeTest, fetch_missing) +{ + auto backend = makeOpenBackend(); + // deliberately do NOT store batch_ — every key must be absent + fetchMissing(*backend, batch_); +} + +// concurrent store/fetch correctness. Replaces the correctness half of the +// multi-threaded Timing_test workloads (which only ran manually, never in CI): +// many threads store disjoint objects, then many threads fetch and verify each +// round-trips. Doubles as a thread-safety smoke test for the backend. +TEST_P(BackendTypeTest, concurrent_store_and_fetch) +{ + // The SQLite backend is not designed for concurrent writers (and the old + // Timing_test only exercised nudb/rocksdb under threads). + if (GetParam() == "sqlite") + GTEST_SKIP() << "sqlite backend is not exercised under concurrency"; + + for (auto const numThreads : {4uz, 8uz}) + { + SCOPED_TRACE("threads=" + std::to_string(numThreads)); + + auto backend = makeOpenBackend(); + + // concurrent stores of disjoint objects + parallelFor(batch_.size(), numThreads, [&](std::size_t i) { backend->store(batch_[i]); }); + + // concurrent fetches, each verifying its object round-trips. Worker + // threads only touch an atomic counter; the EXPECT runs on the main + // thread after join to avoid relying on cross-thread assertion support. + std::atomic mismatches{0}; + parallelFor(batch_.size(), numThreads, [&](std::size_t i) { + std::shared_ptr result; + if (backend->fetch(batch_[i]->getHash(), &result) != Status::Ok || !result || + !isSame(result, batch_[i])) + { + ++mismatches; + } + }); + EXPECT_EQ(mismatches.load(), 0u); + + backend->close(); + } +} + +INSTANTIATE_TEST_SUITE_P( + BackendTypes, + BackendTypeTest, + ::testing::ValuesIn(backendTypes()), + [](::testing::TestParamInfo const& info) { return info.param; }); + +} // namespace xrpl::node_store diff --git a/src/tests/libxrpl/nodestore/Basics.cpp b/src/tests/libxrpl/nodestore/Basics.cpp new file mode 100644 index 0000000000..5bb902af0d --- /dev/null +++ b/src/tests/libxrpl/nodestore/Basics.cpp @@ -0,0 +1,41 @@ +#include +#include +#include + +#include +#include + +#include +#include +#include + +namespace xrpl::node_store { + +TEST(NodeStoreBasics, predictable_batches) +{ + auto const batch1 = createPredictableBatch(kNumObjectsToTest, kSeedValue); + auto const batch2 = createPredictableBatch(kNumObjectsToTest, kSeedValue); + EXPECT_EQ(batch1, batch2); + + auto const batch3 = createPredictableBatch(kNumObjectsToTest, kSeedValue + 1); + EXPECT_NE(batch1, batch3); +} + +TEST(NodeStoreBasics, blob_encoding) +{ + auto const batch = createPredictableBatch(kNumObjectsToTest, kSeedValue); + for (std::size_t i = 0; i < batch.size(); ++i) + { + SCOPED_TRACE("blob index=" + std::to_string(i)); + EncodedBlob const encoded(batch[i]); + DecodedBlob decoded(encoded.getKey(), encoded.getData(), encoded.getSize()); + EXPECT_TRUE(decoded.wasOk()); + if (decoded.wasOk()) + { + std::shared_ptr const object(decoded.createObject()); + EXPECT_TRUE(isSame(batch[i], object)); + } + } +} + +} // namespace xrpl::node_store diff --git a/src/tests/libxrpl/nodestore/Database.cpp b/src/tests/libxrpl/nodestore/Database.cpp new file mode 100644 index 0000000000..23087a2f84 --- /dev/null +++ b/src/tests/libxrpl/nodestore/Database.cpp @@ -0,0 +1,248 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace xrpl::node_store { + +namespace { + +std::vector +allBackends() +{ + std::vector types{"memory", "nudb"}; +#if XRPL_ROCKSDB_AVAILABLE + types.emplace_back("rocksdb"); +#endif + return types; +} + +std::vector +persistentBackends() +{ + std::vector types{"nudb"}; +#if XRPL_ROCKSDB_AVAILABLE + types.emplace_back("rocksdb"); +#endif + return types; +} + +std::vector +importBackends() +{ + std::vector types{"nudb"}; +#if XRPL_ROCKSDB_AVAILABLE + types.emplace_back("rocksdb"); +#endif +#ifdef XRPL_ENABLE_SQLITE_BACKEND_TESTS + types.emplace_back("sqlite"); +#endif + return types; +} + +} // namespace + +// Shared setup for the parameterized Database tests: builds the node params, +// journal and a predictable batch per test, mirroring Backend.cpp's fixture. +class NodeStoreDatabaseTestBase : public ::testing::TestWithParam +{ +protected: + void + SetUp() override + { + nodeParams_.set("type", GetParam()); + nodeParams_.set("path", nodeDb_.path()); + + beast::xor_shift_engine rng(kSeedValue); + batch_ = createPredictableBatch(kNumObjects, rng()); + } + + std::unique_ptr + makeDatabase() + { + return Manager::instance().makeDatabase(megabytes(4), scheduler_, 2, nodeParams_, journal_); + } + + DummyScheduler scheduler_; + beast::TempDir const nodeDb_; + beast::Journal const journal_{TestSink::instance()}; + Section nodeParams_; + Batch batch_; +}; + +class NodeStoreDatabaseTest : public NodeStoreDatabaseTestBase +{ +}; + +class NodeStoreDatabasePersistenceTest : public NodeStoreDatabaseTestBase +{ +}; + +TEST_P(NodeStoreDatabaseTest, store_and_fetch) +{ + auto db = makeDatabase(); + + storeBatch(*db, batch_); + + { + SCOPED_TRACE("read in original order"); + auto const copy = fetchCopyOfBatch(*db, batch_); + EXPECT_EQ(batch_, copy); + } + + { + SCOPED_TRACE("read in shuffled order"); + beast::xor_shift_engine rng(kSeedValue); + std::shuffle(batch_.begin(), batch_.end(), rng); + auto const copy = fetchCopyOfBatch(*db, batch_); + EXPECT_EQ(batch_, copy); + } +} + +TEST_P(NodeStoreDatabasePersistenceTest, round_trip) +{ + { + auto db = makeDatabase(); + storeBatch(*db, batch_); + } + + // re-open without the ephemeral db + auto db = makeDatabase(); + + auto copy = fetchCopyOfBatch(*db, batch_); + std::ranges::sort(batch_, LessThan{}); + std::ranges::sort(copy, LessThan{}); + EXPECT_EQ(batch_, copy); +} + +// missing-key path at the Database layer. Mirrors Backend's fetch_missing — +// fetching keys that were never stored must return nullptr (NotFound). +TEST_P(NodeStoreDatabaseTest, fetch_missing) +{ + auto db = makeDatabase(); + + // never store: every key must be absent + fetchMissing(*db, batch_); +} + +INSTANTIATE_TEST_SUITE_P( + NodeStoreBackends, + NodeStoreDatabaseTest, + ::testing::ValuesIn(allBackends()), + [](::testing::TestParamInfo const& info) { return info.param; }); + +INSTANTIATE_TEST_SUITE_P( + PersistentBackends, + NodeStoreDatabasePersistenceTest, + ::testing::ValuesIn(persistentBackends()), + [](::testing::TestParamInfo const& info) { return info.param; }); + +TEST(NodeStoreDatabase, memory_earliest_seq) +{ + DummyScheduler scheduler; + beast::TempDir const nodeDb; + Section nodeParams; + nodeParams.set("type", "memory"); + nodeParams.set("path", nodeDb.path()); + + beast::Journal const journal(TestSink::instance()); + + // default earliest ledger sequence + { + auto db = Manager::instance().makeDatabase(megabytes(4), scheduler, 2, nodeParams, journal); + EXPECT_EQ(db->earliestLedgerSeq(), kXrpLedgerEarliestSeq); + } + + // invalid earliest_seq value + { + nodeParams.set("earliest_seq", "0"); + try + { + auto db = + Manager::instance().makeDatabase(megabytes(4), scheduler, 2, nodeParams, journal); + FAIL() << "expected runtime_error for earliest_seq=0"; + } + catch (std::runtime_error const& e) + { + EXPECT_STREQ(e.what(), "Invalid earliest_seq"); + } + } + + // valid earliest_seq value + { + nodeParams.set("earliest_seq", "1"); + auto db = Manager::instance().makeDatabase(megabytes(4), scheduler, 2, nodeParams, journal); + EXPECT_EQ(db->earliestLedgerSeq(), 1u); + } +} + +class DatabaseImportTest : public ::testing::TestWithParam +{ +}; + +TEST_P(DatabaseImportTest, same_backend) +{ + auto const type = GetParam(); + + DummyScheduler scheduler; + beast::Journal const journal(TestSink::instance()); + + beast::TempDir const srcDir; + Section srcParams; + srcParams.set("type", type); + srcParams.set("path", srcDir.path()); + + auto batch = createPredictableBatch(kNumObjects, kSeedValue); + + // write to source db + { + auto src = Manager::instance().makeDatabase(megabytes(4), scheduler, 2, srcParams, journal); + storeBatch(*src, batch); + } + + Batch copy; + { + // re-open source and import into a fresh destination + auto src = Manager::instance().makeDatabase(megabytes(4), scheduler, 2, srcParams, journal); + + beast::TempDir const destDir; + Section destParams; + destParams.set("type", type); + destParams.set("path", destDir.path()); + + auto dest = + Manager::instance().makeDatabase(megabytes(4), scheduler, 2, destParams, journal); + + dest->importDatabase(*src); + copy = fetchCopyOfBatch(*dest, batch); + } + + std::ranges::sort(batch, LessThan{}); + std::ranges::sort(copy, LessThan{}); + EXPECT_EQ(batch, copy); +} + +INSTANTIATE_TEST_SUITE_P( + ImportBackends, + DatabaseImportTest, + ::testing::ValuesIn(importBackends()), + [](::testing::TestParamInfo const& info) { return info.param; }); + +} // namespace xrpl::node_store diff --git a/src/tests/libxrpl/nodestore/NuDBFactory.cpp b/src/tests/libxrpl/nodestore/NuDBFactory.cpp new file mode 100644 index 0000000000..c126984630 --- /dev/null +++ b/src/tests/libxrpl/nodestore/NuDBFactory.cpp @@ -0,0 +1,297 @@ +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace xrpl::node_store { + +namespace { + +Section +makeSection(std::string const& path, std::string const& blockSize = "") +{ + Section params; + params.set("type", "nudb"); + params.set("path", path); + if (!blockSize.empty()) + params.set("nudb_block_size", blockSize); + return params; +} + +void +runRoundTrip(Section const& params, std::size_t expectedBlocksize) +{ + DummyScheduler scheduler; + beast::Journal const journal(TestSink::instance()); + auto backend = Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); + + ASSERT_TRUE(backend); + ASSERT_EQ(backend->getBlockSize(), expectedBlocksize); + backend->open(); + ASSERT_TRUE(backend->isOpen()); + + auto const batch = createPredictableBatch(10, 12345); + storeBatch(*backend, batch); + + auto const copy = fetchCopyOfBatch(*backend, batch); + + backend->close(); + EXPECT_EQ(batch, copy); +} + +} // namespace + +TEST(NuDBFactory, default_block_size) +{ + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path()); + ASSERT_NO_FATAL_FAILURE(runRoundTrip(params, 4096)); +} + +TEST(NuDBFactory, valid_block_sizes) +{ + auto const kValidSizes = std::to_array({4096, 8192, 16384, 32768}); + for (auto const size : kValidSizes) + { + SCOPED_TRACE("size=" + std::to_string(size)); + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), std::to_string(size)); + ASSERT_NO_FATAL_FAILURE(runRoundTrip(params, size)); + } + + // empty value is ignored by config parser; default (4096) is used + { + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), ""); + ASSERT_NO_FATAL_FAILURE(runRoundTrip(params, 4096)); + } +} + +TEST(NuDBFactory, invalid_block_sizes) +{ + std::vector const kInvalidSizes = { + "2048", // too small + "1024", // too small + "65536", // too large + "131072", // too large + "5000", // not power of 2 + "6000", // not power of 2 + "10000", // not power of 2 + "0", // zero + "-1", // negative + "abc", // non-numeric + "4k", // invalid format + "4096.5"}; // decimal + + for (auto const& size : kInvalidSizes) + { + SCOPED_TRACE("size='" + size + "'"); + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), size); + EXPECT_THROW(runRoundTrip(params, 4096), std::exception); + } + + // whitespace handling — lexical_cast may or may not strip; treat as invalid + std::vector const kWhitespaceSizes = {"4096 ", " 4096"}; + for (auto const& size : kWhitespaceSizes) + { + SCOPED_TRACE("size='" + size + "'"); + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), size); + EXPECT_THROW(runRoundTrip(params, 4096), std::exception); + } +} + +TEST(NuDBFactory, log_messages) +{ + // valid custom block size emits info log + { + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), "8192"); + test::CaptureSink sink(beast::Severity::Info); + beast::Journal const journal(sink); + + DummyScheduler scheduler; + [[maybe_unused]] auto backend = + Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); + + EXPECT_TRUE(sink.messages().contains("Using custom NuDB block size: 8192")); + } + + // invalid block size throws with informative message + { + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), "5000"); + test::CaptureSink sink(beast::Severity::Warning); + beast::Journal const journal(sink); + DummyScheduler scheduler; + try + { + auto backend = + Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); + FAIL() << "expected exception for invalid block size 5000"; + } + catch (std::exception const& e) + { + std::string const what{e.what()}; + EXPECT_TRUE(what.contains("Invalid nudb_block_size: 5000")); + EXPECT_TRUE(what.contains("Must be power of 2 between 4096 and 32768")); + } + } + + // non-numeric value throws + { + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), "invalid"); + test::CaptureSink sink(beast::Severity::Warning); + beast::Journal const journal(sink); + DummyScheduler scheduler; + try + { + auto backend = + Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); + FAIL() << "expected exception for non-numeric block size"; + } + catch (std::exception const& e) + { + std::string const what{e.what()}; + EXPECT_TRUE(what.contains("Invalid nudb_block_size value: invalid")); + } + } +} + +TEST(NuDBFactory, power_of_two_validation) +{ + std::vector> const kCASES = { + {"4095", false}, // just below minimum + {"4096", true}, // minimum valid + {"4097", false}, // not power of 2 + {"8192", true}, // valid power of 2 + {"8193", false}, // not power of 2 + {"16384", true}, // valid power of 2 + {"32768", true}, // maximum valid + {"32769", false}, // just above maximum + {"65536", false}}; // power of 2 but too large + + for (auto const& [size, shouldWork] : kCASES) + { + SCOPED_TRACE("size=" + size + " shouldWork=" + (shouldWork ? "true" : "false")); + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), size); + test::CaptureSink sink(beast::Severity::Warning); + beast::Journal const journal(sink); + DummyScheduler scheduler; + try + { + auto backend = + Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); + EXPECT_TRUE(shouldWork); + } + catch (std::exception const& e) + { + // A throw is only expected for sizes that should NOT work; if a + // valid size throws, fail here instead of silently matching the + // message below (which would mask the regression). + EXPECT_FALSE(shouldWork); + std::string const what{e.what()}; + EXPECT_TRUE(what.contains("Invalid nudb_block_size")); + } + } +} + +TEST(NuDBFactory, both_constructor_variants) +{ + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), "16384"); + DummyScheduler scheduler; + beast::Journal const journal(TestSink::instance()); + + auto backend1 = Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); + EXPECT_NE(backend1, nullptr); + ASSERT_NO_FATAL_FAILURE(runRoundTrip(params, 16384)); + + // Test second constructor (with nudb::context) + // Note: This would require access to nudb::context, which might not be + // easily testable without more complex setup. For now, we test that + // the factory can create backends with the first constructor. +} + +TEST(NuDBFactory, configuration_parsing) +{ + // basic valid format emits success log + { + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), "8192"); + test::CaptureSink sink(beast::Severity::Info); + beast::Journal const journal(sink); + DummyScheduler scheduler; + [[maybe_unused]] auto backend = + Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); + EXPECT_TRUE(sink.messages().contains("Using custom NuDB block size")); + } + + // Test whitespace handling separately since lexical_cast behavior may vary + std::vector const kWhitespaceFormats = {" 8192", "8192 "}; + for (auto const& format : kWhitespaceFormats) + { + SCOPED_TRACE("format='" + format + "'"); + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), format); + test::CaptureSink sink(beast::Severity::Debug); + beast::Journal const journal(sink); + DummyScheduler scheduler; + EXPECT_ANY_THROW(Manager::instance().makeBackend(params, megabytes(4), scheduler, journal)); + } +} + +TEST(NuDBFactory, data_persistence) +{ + std::vector const kBlockSizes = {"4096", "8192", "16384", "32768"}; + for (auto const& size : kBlockSizes) + { + SCOPED_TRACE("size=" + size); + beast::TempDir const tempDir; + auto const params = makeSection(tempDir.path(), size); + DummyScheduler scheduler; + beast::Journal const journal(TestSink::instance()); + + // Create test data + auto const batch = createPredictableBatch(50, 54321); + + // Store data + { + auto backend = + Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); + backend->open(); + storeBatch(*backend, batch); + backend->close(); + } + + // Retrieve data in new backend instance + { + auto backend = + Manager::instance().makeBackend(params, megabytes(4), scheduler, journal); + backend->open(); + auto const copy = fetchCopyOfBatch(*backend, batch); + EXPECT_EQ(batch, copy); + backend->close(); + } + } +} + +} // namespace xrpl::node_store diff --git a/src/tests/libxrpl/nodestore/TestBase.h b/src/tests/libxrpl/nodestore/TestBase.h new file mode 100644 index 0000000000..5a262ac7bb --- /dev/null +++ b/src/tests/libxrpl/nodestore/TestBase.h @@ -0,0 +1,169 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include +#include +#include +#include +#include + +namespace xrpl::node_store { + +constexpr std::size_t kMinPayloadBytes = 1; +constexpr std::size_t kMaxPayloadBytes = 2000; +constexpr int kNumObjectsToTest = 2000; +constexpr int kNumObjects = 2000; +constexpr std::uint64_t kSeedValue = 50; + +struct LessThan +{ + bool + operator()(std::shared_ptr const& lhs, std::shared_ptr const& rhs) + const noexcept + { + return lhs->getHash() < rhs->getHash(); + } +}; + +[[nodiscard]] inline bool +isSame(std::shared_ptr const& lhs, std::shared_ptr const& rhs) +{ + return (lhs->getType() == rhs->getType()) && (lhs->getHash() == rhs->getHash()) && + (lhs->getData() == rhs->getData()); +} + +[[nodiscard]] inline Batch +createPredictableBatch(std::size_t numObjects, std::uint64_t seed) +{ + Batch batch; + batch.reserve(numObjects); + + beast::xor_shift_engine rng(seed); + + for (auto i = 0uz; i < numObjects; ++i) + { + NodeObjectType const type = [&] { + switch (randInt(rng, 3)) + { + case 0: + return NodeObjectType::Ledger; + case 1: + return NodeObjectType::AccountNode; + case 2: + return NodeObjectType::TransactionNode; + case 3: + default: + return NodeObjectType::Unknown; + } + }(); + + uint256 hash; + beast::rngfill(hash.begin(), hash.size(), rng); + + Blob blob(randInt(rng, kMinPayloadBytes, kMaxPayloadBytes)); + beast::rngfill(blob.data(), blob.size(), rng); + + batch.emplace_back(NodeObject::createObject(type, std::move(blob), hash)); + } + + return batch; +} + +inline void +storeBatch(Backend& backend, Batch const& batch) +{ + for (auto const& obj : batch) + backend.store(obj); +} + +[[nodiscard]] inline Batch +fetchCopyOfBatch(Backend& backend, Batch const& batch) +{ + Batch copy; + copy.reserve(batch.size()); + + for (auto i = 0uz; i < batch.size(); ++i) + { + SCOPED_TRACE("fetchCopyOfBatch index=" + std::to_string(i)); + std::shared_ptr object; + Status const status = backend.fetch(batch[i]->getHash(), &object); + EXPECT_EQ(status, Status::Ok); + if (status == Status::Ok) + { + EXPECT_NE(object, nullptr); + copy.emplace_back(object); + } + } + return copy; +} + +inline void +fetchMissing(Backend& backend, Batch const& batch) +{ + for (auto i = 0uz; i < batch.size(); ++i) + { + SCOPED_TRACE("fetchMissing index=" + std::to_string(i)); + std::shared_ptr object; + Status const status = backend.fetch(batch[i]->getHash(), &object); + EXPECT_EQ(status, Status::NotFound); + } +} + +inline void +storeBatch(Database& db, Batch const& batch) +{ + for (auto const& obj : batch) + { + Blob data(obj->getData()); + db.store(obj->getType(), std::move(data), obj->getHash(), db.earliestLedgerSeq()); + } +} + +[[nodiscard]] inline Batch +fetchCopyOfBatch(Database& db, Batch const& batch) +{ + Batch copy; + copy.reserve(batch.size()); + + for (auto const& obj : batch) + { + std::shared_ptr const result = db.fetchNodeObject(obj->getHash(), 0); + if (result != nullptr) + copy.emplace_back(result); + } + return copy; +} + +inline void +fetchMissing(Database& db, Batch const& batch) +{ + for (auto i = 0uz; i < batch.size(); ++i) + { + SCOPED_TRACE("fetchMissing(Database) index=" + std::to_string(i)); + EXPECT_EQ(db.fetchNodeObject(batch[i]->getHash(), 0), nullptr); + } +} + +} // namespace xrpl::node_store + +namespace xrpl { + +[[nodiscard]] inline bool +operator==(node_store::Batch const& lhs, node_store::Batch const& rhs) +{ + return std::ranges::equal(lhs, rhs, node_store::isSame); +} + +} // namespace xrpl diff --git a/src/tests/libxrpl/nodestore/varint.cpp b/src/tests/libxrpl/nodestore/varint.cpp new file mode 100644 index 0000000000..fee96f314b --- /dev/null +++ b/src/tests/libxrpl/nodestore/varint.cpp @@ -0,0 +1,46 @@ +#include + +#include + +#include +#include +#include +#include +#include + +using namespace xrpl::node_store; + +TEST(varint, encode_decode) +{ + std::vector const kVALUES = { + 0, + 1, + 2, + 126, + 127, + 128, + 253, + 254, + 255, + 16127, + 16128, + 16129, + 0xff, + 0xffff, + 0xffffffff, + 0xffffffffffffUL, + 0xffffffffffffffffUL}; + + for (auto const v : kVALUES) + { + SCOPED_TRACE("value=" + std::to_string(v)); + std::array::kMax> vi{}; + auto const n0 = writeVarint(vi.data(), v); + EXPECT_GT(n0, 0u) << "write error"; + EXPECT_EQ(n0, sizeVarint(v)) << "size error"; + std::size_t v1 = 0; + auto const n1 = readVarint(vi.data(), n0, v1); + EXPECT_EQ(n1, n0) << "read error"; + EXPECT_EQ(v1, v) << "wrong value"; + } +} diff --git a/src/tests/libxrpl/shamap/common.h b/src/tests/libxrpl/shamap/common.h index 5b44f2b251..91401d2973 100644 --- a/src/tests/libxrpl/shamap/common.h +++ b/src/tests/libxrpl/shamap/common.h @@ -24,13 +24,13 @@ namespace xrpl::tests { class TestNodeFamily : public Family { private: - std::unique_ptr db_; + std::unique_ptr db_; std::shared_ptr fbCache_; std::shared_ptr tnCache_; TestStopwatch clock_; - NodeStore::DummyScheduler scheduler_; + node_store::DummyScheduler scheduler_; beast::Journal const j_; @@ -49,17 +49,17 @@ public: Section testSection; testSection.set(Keys::kType, "memory"); testSection.set(Keys::kPath, "SHAMap_test"); - db_ = NodeStore::Manager::instance().makeDatabase( + db_ = node_store::Manager::instance().makeDatabase( megabytes(4), scheduler_, 1, testSection, j); } - NodeStore::Database& + node_store::Database& db() override { return *db_; } - [[nodiscard]] NodeStore::Database const& + [[nodiscard]] node_store::Database const& db() const override { return *db_; diff --git a/src/xrpld/app/ledger/AccountStateSF.h b/src/xrpld/app/ledger/AccountStateSF.h index f5117db4d4..5c1d260c9a 100644 --- a/src/xrpld/app/ledger/AccountStateSF.h +++ b/src/xrpld/app/ledger/AccountStateSF.h @@ -18,7 +18,7 @@ namespace xrpl { class AccountStateSF : public SHAMapSyncFilter { public: - AccountStateSF(NodeStore::Database& db, AbstractFetchPackContainer& fp) : db_(db), fp_(fp) + AccountStateSF(node_store::Database& db, AbstractFetchPackContainer& fp) : db_(db), fp_(fp) { } @@ -34,7 +34,7 @@ public: getNode(SHAMapHash const& nodeHash) const override; private: - NodeStore::Database& db_; + node_store::Database& db_; AbstractFetchPackContainer& fp_; }; diff --git a/src/xrpld/app/ledger/InboundLedger.h b/src/xrpld/app/ledger/InboundLedger.h index d8a9ddf46b..31ca4169ce 100644 --- a/src/xrpld/app/ledger/InboundLedger.h +++ b/src/xrpld/app/ledger/InboundLedger.h @@ -136,7 +136,7 @@ private: addPeers(); void - tryDB(NodeStore::Database& srcDB); + tryDB(node_store::Database& srcDB); void done(); diff --git a/src/xrpld/app/ledger/TransactionStateSF.h b/src/xrpld/app/ledger/TransactionStateSF.h index a3f7e7f55a..b8c1b2c835 100644 --- a/src/xrpld/app/ledger/TransactionStateSF.h +++ b/src/xrpld/app/ledger/TransactionStateSF.h @@ -18,7 +18,7 @@ namespace xrpl { class TransactionStateSF : public SHAMapSyncFilter { public: - TransactionStateSF(NodeStore::Database& db, AbstractFetchPackContainer& fp) : db_(db), fp_(fp) + TransactionStateSF(node_store::Database& db, AbstractFetchPackContainer& fp) : db_(db), fp_(fp) { } @@ -34,7 +34,7 @@ public: getNode(SHAMapHash const& nodeHash) const override; private: - NodeStore::Database& db_; + node_store::Database& db_; AbstractFetchPackContainer& fp_; }; diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp index 627a5d574f..55a2a9d283 100644 --- a/src/xrpld/app/ledger/detail/InboundLedger.cpp +++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp @@ -224,7 +224,7 @@ InboundLedger::neededStateHashes(int max, SHAMapSyncFilter const* filter) const // See how much of the ledger data is stored locally // Data found in a fetch pack will be stored void -InboundLedger::tryDB(NodeStore::Database& srcDB) +InboundLedger::tryDB(node_store::Database& srcDB) { if (!haveHeader_) { diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index 2bd83b0f18..9a0335fc3c 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -653,7 +653,7 @@ LedgerMaster::tryFill(std::shared_ptr ledger) std::uint32_t minHas = seq; std::uint32_t maxHas = seq; - NodeStore::Database& nodeStore{app_.getNodeStore()}; + node_store::Database& nodeStore{app_.getNodeStore()}; while (!app_.getJobQueue().isStopping() && seq > 0) { { diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index d329475874..5c8fdad37c 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -232,7 +232,7 @@ public: std::unique_ptr resourceManager_; - std::unique_ptr nodeStore_; + std::unique_ptr nodeStore_; NodeFamily nodeFamily_; std::unique_ptr orderBookDB_; std::unique_ptr pathRequestManager_; @@ -655,7 +655,7 @@ public: return tempNodeCache_; } - NodeStore::Database& + node_store::Database& getNodeStore() override { return *nodeStore_; @@ -860,9 +860,9 @@ public: if (config_->doImport) { auto j = logs_->journal("NodeObject"); - NodeStore::DummyScheduler dummyScheduler; - std::unique_ptr source = - NodeStore::Manager::instance().makeDatabase( + node_store::DummyScheduler dummyScheduler; + std::unique_ptr source = + node_store::Manager::instance().makeDatabase( megabytes(config_->getValueFor(SizedItem::BurstSize, std::nullopt)), dummyScheduler, 0, diff --git a/src/xrpld/app/main/NodeStoreScheduler.cpp b/src/xrpld/app/main/NodeStoreScheduler.cpp index 7892503f90..c3ac5d78cf 100644 --- a/src/xrpld/app/main/NodeStoreScheduler.cpp +++ b/src/xrpld/app/main/NodeStoreScheduler.cpp @@ -12,7 +12,7 @@ NodeStoreScheduler::NodeStoreScheduler(JobQueue& jobQueue) : jobQueue_(jobQueue) } void -NodeStoreScheduler::scheduleTask(NodeStore::Task& task) +NodeStoreScheduler::scheduleTask(node_store::Task& task) { if (jobQueue_.isStopped()) return; @@ -26,19 +26,19 @@ NodeStoreScheduler::scheduleTask(NodeStore::Task& task) } void -NodeStoreScheduler::onFetch(NodeStore::FetchReport const& report) +NodeStoreScheduler::onFetch(node_store::FetchReport const& report) { if (jobQueue_.isStopped()) return; jobQueue_.addLoadEvents( - report.fetchType == NodeStore::FetchType::Async ? JtNsAsyncRead : JtNsSyncRead, + report.fetchType == node_store::FetchType::Async ? JtNsAsyncRead : JtNsSyncRead, 1, report.elapsed); } void -NodeStoreScheduler::onBatchWrite(NodeStore::BatchWriteReport const& report) +NodeStoreScheduler::onBatchWrite(node_store::BatchWriteReport const& report) { if (jobQueue_.isStopped()) return; diff --git a/src/xrpld/app/main/NodeStoreScheduler.h b/src/xrpld/app/main/NodeStoreScheduler.h index 8bfd1607ae..09a48d5be1 100644 --- a/src/xrpld/app/main/NodeStoreScheduler.h +++ b/src/xrpld/app/main/NodeStoreScheduler.h @@ -7,19 +7,19 @@ namespace xrpl { /** - * A NodeStore::Scheduler which uses the JobQueue. + * A node_store::Scheduler which uses the JobQueue. */ -class NodeStoreScheduler : public NodeStore::Scheduler +class NodeStoreScheduler : public node_store::Scheduler { public: explicit NodeStoreScheduler(JobQueue& jobQueue); void - scheduleTask(NodeStore::Task& task) override; + scheduleTask(node_store::Task& task) override; void - onFetch(NodeStore::FetchReport const& report) override; + onFetch(node_store::FetchReport const& report) override; void - onBatchWrite(NodeStore::BatchWriteReport const& report) override; + onBatchWrite(node_store::BatchWriteReport const& report) override; private: JobQueue& jobQueue_; diff --git a/src/xrpld/app/misc/SHAMapStore.h b/src/xrpld/app/misc/SHAMapStore.h index df696c685f..eeb04df53d 100644 --- a/src/xrpld/app/misc/SHAMapStore.h +++ b/src/xrpld/app/misc/SHAMapStore.h @@ -43,7 +43,7 @@ public: [[nodiscard]] virtual std::uint32_t clampFetchDepth(std::uint32_t fetchDepth) const = 0; - virtual std::unique_ptr + virtual std::unique_ptr makeNodeStore(int readThreads) = 0; /** @@ -101,5 +101,5 @@ public: //------------------------------------------------------------------------------ std::unique_ptr -makeSHAMapStore(Application& app, NodeStore::Scheduler& scheduler, beast::Journal journal); +makeSHAMapStore(Application& app, node_store::Scheduler& scheduler, beast::Journal journal); } // namespace xrpl diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 9b5f412fc5..e41837d206 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -94,7 +94,7 @@ SHAMapStoreImp::SavedStateDB::setLastRotated(LedgerIndex seq) SHAMapStoreImp::SHAMapStoreImp( Application& app, - NodeStore::Scheduler& scheduler, + node_store::Scheduler& scheduler, beast::Journal journal) : app_(app) , scheduler_(scheduler) @@ -165,7 +165,7 @@ SHAMapStoreImp::SHAMapStoreImp( } } -std::unique_ptr +std::unique_ptr SHAMapStoreImp::makeNodeStore(int readThreads) { auto nscfg = app_.config().section(Sections::kNodeDatabase); @@ -185,7 +185,7 @@ SHAMapStoreImp::makeNodeStore(int readThreads) std::to_string(app_.config().getValueFor(SizedItem::TreeCacheAge, std::nullopt))); } - std::unique_ptr db; + std::unique_ptr db; if (deleteInterval_ != 0u) { @@ -201,7 +201,7 @@ SHAMapStoreImp::makeNodeStore(int readThreads) // Create NodeStore with two backends to allow online deletion of // data - auto dbr = std::make_unique( + auto dbr = std::make_unique( scheduler_, readThreads, std::move(writableBackend), @@ -210,11 +210,11 @@ SHAMapStoreImp::makeNodeStore(int readThreads) app_.getJournal(kNodeStoreName)); fdRequired_ += dbr->fdRequired(); dbRotating_ = dbr.get(); - db.reset(dynamic_cast(dbr.release())); + db.reset(dynamic_cast(dbr.release())); } else { - db = NodeStore::Manager::instance().makeDatabase( + db = node_store::Manager::instance().makeDatabase( megabytes(app_.config().getValueFor(SizedItem::BurstSize, std::nullopt)), scheduler_, readThreads, @@ -257,7 +257,7 @@ SHAMapStoreImp::copyNode(std::uint64_t& nodeCount, SHAMapTreeNode const& node) { // Copy a single record from node to dbRotating_ auto obj = dbRotating_->fetchNodeObject( - node.getHash().asUInt256(), 0, NodeStore::FetchType::Synchronous, true); + node.getHash().asUInt256(), 0, node_store::FetchType::Synchronous, true); if (!obj) { XRPL_ASSERT(node.cowid() == 0, "SHAMapStoreImp::copyNode : rescued node must be clean"); @@ -374,7 +374,7 @@ SHAMapStoreImp::run() // exception) also clear the flag. struct RotationExposureGuard { - NodeStore::DatabaseRotating& db; + node_store::DatabaseRotating& db; ~RotationExposureGuard() { db.setRotationInFlight(false); @@ -516,7 +516,7 @@ SHAMapStoreImp::dbPaths() boost::filesystem::remove_all(p); } -std::unique_ptr +std::unique_ptr SHAMapStoreImp::makeBackendRotating(std::string path) { Section section{app_.config().section(Sections::kNodeDatabase)}; @@ -535,7 +535,7 @@ SHAMapStoreImp::makeBackendRotating(std::string path) } section.set(Keys::kPath, newPath.string()); - auto backend{NodeStore::Manager::instance().makeBackend( + auto backend{node_store::Manager::instance().makeBackend( section, megabytes(app_.config().getValueFor(SizedItem::BurstSize, std::nullopt)), scheduler_, @@ -702,7 +702,7 @@ SHAMapStoreImp::minimumOnline() const //------------------------------------------------------------------------------ std::unique_ptr -makeSHAMapStore(Application& app, NodeStore::Scheduler& scheduler, beast::Journal journal) +makeSHAMapStore(Application& app, node_store::Scheduler& scheduler, beast::Journal journal) { return std::make_unique(app, scheduler, journal); } diff --git a/src/xrpld/app/misc/SHAMapStoreImp.h b/src/xrpld/app/misc/SHAMapStoreImp.h index a0ca59ecc8..8a1b7504b9 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.h +++ b/src/xrpld/app/misc/SHAMapStoreImp.h @@ -81,9 +81,9 @@ private: // minimum ledger to maintain online. std::atomic minimumOnline_; - NodeStore::Scheduler& scheduler_; + node_store::Scheduler& scheduler_; beast::Journal const journal_; - NodeStore::DatabaseRotating* dbRotating_ = nullptr; + node_store::DatabaseRotating* dbRotating_ = nullptr; SavedStateDB stateDb_; std::thread thread_; bool stop_ = false; @@ -119,7 +119,7 @@ private: static constexpr auto kNodeStoreName = "NodeStore"; public: - SHAMapStoreImp(Application& app, NodeStore::Scheduler& scheduler, beast::Journal journal); + SHAMapStoreImp(Application& app, node_store::Scheduler& scheduler, beast::Journal journal); std::uint32_t clampFetchDepth(std::uint32_t fetchDepth) const override @@ -127,7 +127,7 @@ public: return (deleteInterval_ != 0u) ? std::min(fetchDepth, deleteInterval_) : fetchDepth; } - std::unique_ptr + std::unique_ptr makeNodeStore(int readThreads) override; LedgerIndex @@ -180,7 +180,7 @@ private: void dbPaths(); - std::unique_ptr + std::unique_ptr makeBackendRotating(std::string path = std::string()); template @@ -191,7 +191,7 @@ private: for (auto const& key : cache.getKeys()) { - dbRotating_->fetchNodeObject(key, 0, NodeStore::FetchType::Synchronous, true); + dbRotating_->fetchNodeObject(key, 0, node_store::FetchType::Synchronous, true); if (!(++check % checkHealthInterval_) && healthWait() == HealthResult::Stopping) return true; } diff --git a/src/xrpld/shamap/NodeFamily.h b/src/xrpld/shamap/NodeFamily.h index d532f13ecc..1307d76886 100644 --- a/src/xrpld/shamap/NodeFamily.h +++ b/src/xrpld/shamap/NodeFamily.h @@ -33,13 +33,13 @@ public: NodeFamily(Application& app, CollectorManager& cm); - NodeStore::Database& + node_store::Database& db() override { return db_; } - [[nodiscard]] NodeStore::Database const& + [[nodiscard]] node_store::Database const& db() const override { return db_; @@ -80,7 +80,7 @@ public: private: Application& app_; - NodeStore::Database& db_; + node_store::Database& db_; beast::Journal const j_; std::shared_ptr fbCache_; From e0aa50c4bee5e93f96cb6d54ca999061eb13fd35 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Mon, 27 Jul 2026 15:18:01 +0100 Subject: [PATCH 06/11] ci: Update CI image and prepare-runner action (#7874) --- .github/scripts/strategy-matrix/linux.json | 2 +- .github/workflows/check-tools.yml | 2 +- .github/workflows/publish-docs.yml | 4 ++-- .github/workflows/reusable-build-test-config.yml | 2 +- .github/workflows/reusable-clang-tidy.yml | 4 ++-- .github/workflows/reusable-upload-recipe.yml | 2 +- .github/workflows/upload-conan-deps.yml | 2 +- nix/check-tools/README.md | 16 ++++++++-------- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 2a0b5e8e0e..60f3da09f1 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -1,5 +1,5 @@ { - "image_tag": "sha-40cdf49", + "image_tag": "sha-fecfc0c", "configs": { "ubuntu": [ { diff --git a/.github/workflows/check-tools.yml b/.github/workflows/check-tools.yml index af20c5f17e..99dddd7d96 100644 --- a/.github/workflows/check-tools.yml +++ b/.github/workflows/check-tools.yml @@ -79,7 +79,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 + uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f with: enable_ccache: false diff --git a/.github/workflows/publish-docs.yml b/.github/workflows/publish-docs.yml index c1e67e2010..a3e096315c 100644 --- a/.github/workflows/publish-docs.yml +++ b/.github/workflows/publish-docs.yml @@ -41,13 +41,13 @@ env: jobs: build: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-40cdf49 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-fecfc0c steps: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 + uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f with: enable_ccache: false diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 74425febe8..c1b8e4195b 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -113,7 +113,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 + uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f with: enable_ccache: ${{ inputs.ccache_enabled }} diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index 3c19b58a12..a6c1e6ae56 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -34,7 +34,7 @@ jobs: needs: [determine-files] if: ${{ needs.determine-files.outputs.cpp_changed_files != '' || needs.determine-files.outputs.need_full_run == 'true' }} runs-on: ["self-hosted", "Linux", "X64", "heavy"] - container: "ghcr.io/xrplf/xrpld/nix-debian:sha-40cdf49" + container: "ghcr.io/xrplf/xrpld/nix-debian:sha-fecfc0c" permissions: contents: read issues: write @@ -43,7 +43,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 + uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f with: enable_ccache: false diff --git a/.github/workflows/reusable-upload-recipe.yml b/.github/workflows/reusable-upload-recipe.yml index bce4da2df6..b4ab638dee 100644 --- a/.github/workflows/reusable-upload-recipe.yml +++ b/.github/workflows/reusable-upload-recipe.yml @@ -40,7 +40,7 @@ defaults: jobs: upload: runs-on: ubuntu-latest - container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-40cdf49 + container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-fecfc0c env: REMOTE_NAME: ${{ inputs.remote_name }} CONAN_LOGIN_USERNAME_XRPLF: ${{ secrets.remote_username }} diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 80a75a1fbf..eb58650bdf 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -68,7 +68,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Prepare runner - uses: XRPLF/actions/prepare-runner@e4b6449d55a61c002d7c3fdfa6c20f721ede0606 + uses: XRPLF/actions/prepare-runner@c00c22ada3bd6bcda48fcb0d62fbbab49fec8a0f with: enable_ccache: false diff --git a/nix/check-tools/README.md b/nix/check-tools/README.md index fe9ab3e250..5b7538f2ca 100644 --- a/nix/check-tools/README.md +++ b/nix/check-tools/README.md @@ -3,11 +3,11 @@ These files capture the output of [`bin/check-tools.sh`](../../bin/check-tools.sh) — the versions of the development tooling — in each Nix environment: -| File | Environment | -| --------------------- | ----------------------------------- | -| `nix-nixos-amd64.txt` | `nix-nixos` CI image, `linux/amd64` | -| `nix-nixos-arm64.txt` | `nix-nixos` CI image, `linux/arm64` | -| `macos.txt` | macOS, inside `nix develop` | +| File | Environment | +| ---------------------- | ------------------------------------ | +| `nix-ubuntu-amd64.txt` | `nix-ubuntu` CI image, `linux/amd64` | +| `nix-ubuntu-arm64.txt` | `nix-ubuntu` CI image, `linux/arm64` | +| `macos.txt` | macOS, inside `nix develop` | The [`check-tools`](../../.github/workflows/check-tools.yml) workflow regenerates each snapshot in its environment and fails if it differs from the committed file. @@ -23,15 +23,15 @@ with `sed -n '/^Detected OS:/,$p'`. ## Regenerating -The two Linux snapshots come from the `nix-nixos` image (Docker or a compatible +The two Linux snapshots come from the `nix-ubuntu` image (Docker or a compatible runtime such as Apple `container`). The image tag is pinned in `linux.json`: ```bash -img="ghcr.io/xrplf/xrpld/nix-nixos:$(jq -r .image_tag .github/scripts/strategy-matrix/linux.json)" +img="ghcr.io/xrplf/xrpld/nix-ubuntu:$(jq -r .image_tag .github/scripts/strategy-matrix/linux.json)" for arch in amd64 arm64; do container run --rm -i -e CHECK_TOOLS_SKIP_CLONE=1 -a "${arch}" --entrypoint bash "${img}" -s \ - "nix/check-tools/nix-nixos-${arch}.txt" + "nix/check-tools/nix-ubuntu-${arch}.txt" done ``` From 9466ecb5c222dec63afd002dc0334a13c1f866bf Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Mon, 27 Jul 2026 15:32:53 +0100 Subject: [PATCH 07/11] ci: Group github-actions dependabot updates (#7876) --- .github/dependabot.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index da7a30dc77..fcac44c44c 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -15,3 +15,7 @@ updates: commit-message: prefix: "ci: [DEPENDABOT] " target-branch: develop + groups: + github-actions: + patterns: + - "*" From 6c9c7f0555cee008961f2167060f98a338c95aac Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Mon, 27 Jul 2026 17:34:53 +0100 Subject: [PATCH 08/11] chore: Trivial gtest migrations (#7865) --- .cspell.config.yaml | 1 - .../nodestore/detail/{varint.h => Varint.h} | 14 +- include/xrpl/nodestore/detail/codec.h | 10 +- src/test/beast/beast_Zero_test.cpp | 115 -------------- src/test/protocol/ApiVersion_test.cpp | 41 ----- src/test/protocol/Serializer_test.cpp | 52 ------- src/tests/libxrpl/CMakeLists.txt | 4 +- src/tests/libxrpl/beast/Zero.cpp | 92 +++++++++++ src/tests/libxrpl/nodestore/Codec.cpp | 146 ++++++++++++++++++ src/tests/libxrpl/nodestore/Varint.cpp | 46 ++++++ src/tests/libxrpl/nodestore/varint.cpp | 46 ------ src/tests/libxrpl/protocol/ApiVersion.cpp | 26 ++++ src/tests/libxrpl/protocol/Serializer.cpp | 49 ++++++ 13 files changed, 374 insertions(+), 268 deletions(-) rename include/xrpl/nodestore/detail/{varint.h => Varint.h} (91%) delete mode 100644 src/test/beast/beast_Zero_test.cpp delete mode 100644 src/test/protocol/ApiVersion_test.cpp delete mode 100644 src/test/protocol/Serializer_test.cpp create mode 100644 src/tests/libxrpl/beast/Zero.cpp create mode 100644 src/tests/libxrpl/nodestore/Codec.cpp create mode 100644 src/tests/libxrpl/nodestore/Varint.cpp delete mode 100644 src/tests/libxrpl/nodestore/varint.cpp create mode 100644 src/tests/libxrpl/protocol/ApiVersion.cpp create mode 100644 src/tests/libxrpl/protocol/Serializer.cpp diff --git a/.cspell.config.yaml b/.cspell.config.yaml index 9cd8417362..e3764fd2a2 100644 --- a/.cspell.config.yaml +++ b/.cspell.config.yaml @@ -174,7 +174,6 @@ words: - MPTAMM - MPTDEX - Merkle - - Metafuncton - misprediction - missingok - mptbalance diff --git a/include/xrpl/nodestore/detail/varint.h b/include/xrpl/nodestore/detail/Varint.h similarity index 91% rename from include/xrpl/nodestore/detail/varint.h rename to include/xrpl/nodestore/detail/Varint.h index afbf71cdea..5474cdc8b4 100644 --- a/include/xrpl/nodestore/detail/varint.h +++ b/include/xrpl/nodestore/detail/Varint.h @@ -13,18 +13,18 @@ namespace xrpl::node_store { // https://developers.google.com/protocol-buffers/docs/encoding#varints // field tag -struct varint; +struct Varint; -// Metafuncton to return largest +// Metafunction to return largest // possible size of T represented as varint. // T must be unsigned template > -struct varint_traits; +struct VarintTraits; template -struct varint_traits +struct VarintTraits { - explicit varint_traits() = default; + explicit VarintTraits() = default; static constexpr std::size_t kMax = ((8 * sizeof(T)) + 6) / 7; }; @@ -104,7 +104,7 @@ writeVarint(void* p0, std::size_t v) template void read(nudb::detail::istream& is, std::size_t& u) - requires(std::is_same_v) + requires(std::is_same_v) { auto p0 = is(1); auto p1 = p0; @@ -118,7 +118,7 @@ read(nudb::detail::istream& is, std::size_t& u) template void write(nudb::detail::ostream& os, std::size_t t) - requires(std::is_same_v) + requires(std::is_same_v) { writeVarint(os.data(sizeVarint(t)), t); } diff --git a/include/xrpl/nodestore/detail/codec.h b/include/xrpl/nodestore/detail/codec.h index a3dfa7c944..47ad2da50a 100644 --- a/include/xrpl/nodestore/detail/codec.h +++ b/include/xrpl/nodestore/detail/codec.h @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include @@ -59,7 +59,7 @@ lz4Compress(void const* in, std::size_t inSize, BufferFactory&& bf) using std::runtime_error; using namespace nudb::detail; std::pair result; - std::array::kMax> vi{}; + std::array::kMax> vi{}; auto const n = writeVarint(vi.data(), inSize); auto const outMax = LZ4_compressBound(inSize); auto* out = reinterpret_cast(bf(n + outMax)); @@ -240,7 +240,7 @@ nodeobjectCompress(void const* in, std::size_t inSize, BufferFactory&& bf) auto* out = reinterpret_cast(bf(result.second)); result.first = out; ostream os(out, result.second); - write(os, type); + write(os, type); write(os, mask); write(os, vh.data(), n * 32); return result; @@ -252,13 +252,13 @@ nodeobjectCompress(void const* in, std::size_t inSize, BufferFactory&& bf) auto* out = reinterpret_cast(bf(result.second)); result.first = out; ostream os(out, result.second); - write(os, type); + write(os, type); write(os, vh.data(), n * 32); return result; } } - std::array::kMax> vi{}; + std::array::kMax> vi{}; static constexpr std::size_t kCodecType = 1; auto const vn = writeVarint(vi.data(), kCodecType); diff --git a/src/test/beast/beast_Zero_test.cpp b/src/test/beast/beast_Zero_test.cpp deleted file mode 100644 index bb61844caa..0000000000 --- a/src/test/beast/beast_Zero_test.cpp +++ /dev/null @@ -1,115 +0,0 @@ -#include -#include - -namespace beast { - -struct AdlTester -{ -}; - -int -signum(AdlTester) -{ - return 0; -} - -namespace inner_adl_test { - -struct AdlTester2 -{ -}; - -int -signum(AdlTester2) -{ - return 0; -} - -} // namespace inner_adl_test - -class Zero_test : public beast::unit_test::Suite -{ -private: - struct IntegerWrapper - { - int value; - - IntegerWrapper(int v) : value(v) - { - } - - [[nodiscard]] int - signum() const - { - return value; - } - }; - -public: - void - expectSame(bool result, bool correct, char const* message) - { - expect(result == correct, message); - } - - void - testLhsZero(IntegerWrapper x) - { - expectSame(x >= kZero, x.signum() >= 0, "lhs greater-than-or-equal-to"); - expectSame(x > kZero, x.signum() > 0, "lhs greater than"); - expectSame(x == kZero, x.signum() == 0, "lhs equal to"); - expectSame(x != kZero, x.signum() != 0, "lhs not equal to"); - expectSame(x < kZero, x.signum() < 0, "lhs less than"); - expectSame(x <= kZero, x.signum() <= 0, "lhs less-than-or-equal-to"); - } - - void - testLhsZero() - { - testcase("lhs zero"); - - testLhsZero(-7); - testLhsZero(0); - testLhsZero(32); - } - - void - testRhsZero(IntegerWrapper x) - { - expectSame(kZero >= x, 0 >= x.signum(), "rhs greater-than-or-equal-to"); - expectSame(kZero > x, 0 > x.signum(), "rhs greater than"); - expectSame(kZero == x, 0 == x.signum(), "rhs equal to"); - expectSame(kZero != x, 0 != x.signum(), "rhs not equal to"); - expectSame(kZero < x, 0 < x.signum(), "rhs less than"); - expectSame(kZero <= x, 0 <= x.signum(), "rhs less-than-or-equal-to"); - } - - void - testRhsZero() - { - testcase("rhs zero"); - - testRhsZero(-4); - testRhsZero(0); - testRhsZero(64); - } - - void - testAdl() - { - expect(AdlTester{} == kZero, "ADL failure!"); - expect(inner_adl_test::AdlTester2{} == kZero, "ADL failure!"); - } - - void - run() override - { - testLhsZero(); - testRhsZero(); - testAdl(); - } -}; - -BEAST_DEFINE_TESTSUITE(Zero, beast, beast); - -} // namespace beast diff --git a/src/test/protocol/ApiVersion_test.cpp b/src/test/protocol/ApiVersion_test.cpp deleted file mode 100644 index c41fa6f6c0..0000000000 --- a/src/test/protocol/ApiVersion_test.cpp +++ /dev/null @@ -1,41 +0,0 @@ -#include -#include - -namespace xrpl::test { -struct ApiVersion_test : beast::unit_test::Suite -{ - void - run() override - { - { - testcase("API versions invariants"); - - static_assert(RPC::kApiMinimumSupportedVersion <= RPC::kApiMaximumSupportedVersion); - static_assert(RPC::kApiMinimumSupportedVersion <= RPC::kApiMaximumValidVersion); - static_assert(RPC::kApiMaximumSupportedVersion <= RPC::kApiMaximumValidVersion); - static_assert(RPC::kApiBetaVersion <= RPC::kApiMaximumValidVersion); - - BEAST_EXPECT(true); - } - - { - // Update when we change versions - testcase("API versions"); - - static_assert(RPC::kApiMinimumSupportedVersion >= 1); - static_assert(RPC::kApiMinimumSupportedVersion < 2); - static_assert(RPC::kApiMaximumSupportedVersion >= 2); - static_assert(RPC::kApiMaximumSupportedVersion < 3); - static_assert(RPC::kApiMaximumValidVersion >= 3); - static_assert(RPC::kApiMaximumValidVersion < 4); - static_assert(RPC::kApiBetaVersion >= 3); - static_assert(RPC::kApiBetaVersion < 4); - - BEAST_EXPECT(true); - } - } -}; - -BEAST_DEFINE_TESTSUITE(ApiVersion, protocol, xrpl); - -} // namespace xrpl::test diff --git a/src/test/protocol/Serializer_test.cpp b/src/test/protocol/Serializer_test.cpp deleted file mode 100644 index b490e0476b..0000000000 --- a/src/test/protocol/Serializer_test.cpp +++ /dev/null @@ -1,52 +0,0 @@ -#include -#include - -#include -#include -#include - -namespace xrpl { - -struct Serializer_test : public beast::unit_test::Suite -{ - void - run() override - { - { - std::initializer_list const values = { - std::numeric_limits::min(), - -1, - 0, - 1, - std::numeric_limits::max()}; - for (std::int32_t const value : values) - { - Serializer s; - s.add32(value); - BEAST_EXPECT(s.size() == 4); - SerialIter sit(s.slice()); - BEAST_EXPECT(sit.geti32() == value); - } - } - { - std::initializer_list const values = { - std::numeric_limits::min(), - -1, - 0, - 1, - std::numeric_limits::max()}; - for (std::int64_t const value : values) - { - Serializer s; - s.add64(value); - BEAST_EXPECT(s.size() == 8); - SerialIter sit(s.slice()); - BEAST_EXPECT(sit.geti64() == value); - } - } - } -}; - -BEAST_DEFINE_TESTSUITE(Serializer, protocol, xrpl); - -} // namespace xrpl diff --git a/src/tests/libxrpl/CMakeLists.txt b/src/tests/libxrpl/CMakeLists.txt index 8e4ece1234..c45f55b5f2 100644 --- a/src/tests/libxrpl/CMakeLists.txt +++ b/src/tests/libxrpl/CMakeLists.txt @@ -27,15 +27,17 @@ target_link_libraries(xrpl_tests PRIVATE GTest::gtest GTest::gmock xrpl.libxrpl) # supported on Windows. set(test_modules basics + beast consensus crypto json + nodestore peerfinder + protocol resource shamap tx protocol_autogen - nodestore ) if(NOT WIN32) list(APPEND test_modules net) diff --git a/src/tests/libxrpl/beast/Zero.cpp b/src/tests/libxrpl/beast/Zero.cpp new file mode 100644 index 0000000000..2ac725509a --- /dev/null +++ b/src/tests/libxrpl/beast/Zero.cpp @@ -0,0 +1,92 @@ +#include + +#include + +namespace beast { + +struct AdlTester +{ +}; + +int +signum(AdlTester) +{ + return 0; +} + +namespace inner_adl_test { + +struct AdlTester2 +{ +}; + +int +signum(AdlTester2) +{ + return 0; +} + +} // namespace inner_adl_test + +namespace { + +struct IntegerWrapper +{ + int value; + + IntegerWrapper(int v) : value(v) + { + } + + [[nodiscard]] int + signum() const + { + return value; + } +}; + +void +testLhsZero(IntegerWrapper x) +{ + EXPECT_EQ(x >= kZero, x.signum() >= 0); + EXPECT_EQ(x > kZero, x.signum() > 0); + EXPECT_EQ(x == kZero, x.signum() == 0); + EXPECT_EQ(x != kZero, x.signum() != 0); + EXPECT_EQ(x < kZero, x.signum() < 0); + EXPECT_EQ(x <= kZero, x.signum() <= 0); +} + +void +testRhsZero(IntegerWrapper x) +{ + EXPECT_EQ(kZero >= x, 0 >= x.signum()); + EXPECT_EQ(kZero > x, 0 > x.signum()); + EXPECT_EQ(kZero == x, 0 == x.signum()); + EXPECT_EQ(kZero != x, 0 != x.signum()); + EXPECT_EQ(kZero < x, 0 < x.signum()); + EXPECT_EQ(kZero <= x, 0 <= x.signum()); +} + +} // namespace + +TEST(Zero, lhs) +{ + testLhsZero(-7); + testLhsZero(0); + testLhsZero(32); +} + +TEST(Zero, rhs) +{ + testRhsZero(-4); + testRhsZero(0); + testRhsZero(64); +} + +TEST(Zero, adl) +{ + EXPECT_TRUE(AdlTester{} == kZero); + EXPECT_TRUE(inner_adl_test::AdlTester2{} == kZero); +} + +} // namespace beast diff --git a/src/tests/libxrpl/nodestore/Codec.cpp b/src/tests/libxrpl/nodestore/Codec.cpp new file mode 100644 index 0000000000..f31878f3c5 --- /dev/null +++ b/src/tests/libxrpl/nodestore/Codec.cpp @@ -0,0 +1,146 @@ +#include + +#include +#include + +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +using namespace xrpl; +using namespace xrpl::node_store; + +namespace { + +// v1 inner-node layout: 16 hashes of 32 bytes each +constexpr std::size_t kHashCount = 16; +constexpr std::size_t kHashSize = 32; + +std::vector +makeInnerNode(std::size_t nonEmptySlots) +{ + using namespace nudb::detail; + + static constexpr std::size_t kInnerNodeSize = 525; + + std::array hashes{}; + for (auto slot = 0uz; slot < nonEmptySlots; ++slot) + { + for (auto byte = 0uz; byte < kHashSize; ++byte) + { + std::size_t const offset = (slot * kHashSize) + byte; + hashes[offset] = static_cast((offset % 255) + 1); + } + } + + std::vector blob(kInnerNodeSize); + ostream os(blob.data(), blob.size()); + write(os, 0); // index + write(os, 0); // unused + write(os, static_cast(NodeObjectType::Unknown)); + write(os, static_cast(HashPrefix::InnerNode)); + write(os, hashes.data(), hashes.size()); + + return blob; +} + +std::uint8_t +codecType(std::pair const& compressed) +{ + return static_cast(compressed.first)[0]; +} + +} // namespace + +// All 16 hash slots populated - "full v1 inner node" +TEST(Codec, inner_node_full_roundtrip) +{ + static constexpr std::uint8_t kTypeInnerNodeFull = 3; + + auto const blob = makeInnerNode(kHashCount); + + nudb::detail::buffer compressBuf; + auto const compressed = nodeobjectCompress(blob.data(), blob.size(), compressBuf); + + EXPECT_EQ(codecType(compressed), kTypeInnerNodeFull); + EXPECT_EQ(compressed.second, sizeVarint(kTypeInnerNodeFull) + (kHashCount * kHashSize)); + + nudb::detail::buffer decompressBuf; + auto const restored = nodeobjectDecompress(compressed.first, compressed.second, decompressBuf); + + EXPECT_EQ(restored.second, blob.size()); + EXPECT_EQ(std::memcmp(restored.first, blob.data(), blob.size()), 0); +} + +// Some hash slots empty - "compressed v1 inner node" +TEST(Codec, inner_node_compressed_roundtrip) +{ + static constexpr std::uint8_t kTypeInnerNodeCompressed = 2; + static constexpr std::size_t kNonEmpty = 5; + auto const blob = makeInnerNode(kNonEmpty); + + nudb::detail::buffer compressBuf; + auto const compressed = nodeobjectCompress(blob.data(), blob.size(), compressBuf); + + EXPECT_EQ(codecType(compressed), kTypeInnerNodeCompressed); + EXPECT_EQ( + compressed.second, + sizeVarint(kTypeInnerNodeCompressed) + sizeof(std::uint16_t) + (kNonEmpty * kHashSize)); + EXPECT_LT(compressed.second, blob.size()); + + nudb::detail::buffer decompressBuf; + auto const restored = nodeobjectDecompress(compressed.first, compressed.second, decompressBuf); + + EXPECT_EQ(restored.second, blob.size()); + EXPECT_EQ(std::memcmp(restored.first, blob.data(), blob.size()), 0); +} + +// Anything that is not a v1 inner node - lz4 compressed +TEST(Codec, lz4_roundtrip) +{ + // A payload that is deliberately not a v1 inner node (any size other than 525), filled with a + // short repeating pattern so lz4 actually shrinks it. + static constexpr std::size_t kNonInnerNodeSize = 1000; + static constexpr std::size_t kBytePatternPeriod = 7; + static constexpr std::uint8_t kTypeLz4 = 1; + + std::vector blob(kNonInnerNodeSize); + for (auto i = 0uz; i < blob.size(); ++i) + blob[i] = static_cast(i % kBytePatternPeriod); + + nudb::detail::buffer compressBuf; + auto const compressed = nodeobjectCompress(blob.data(), blob.size(), compressBuf); + + EXPECT_EQ(codecType(compressed), kTypeLz4); + + nudb::detail::buffer decompressBuf; + auto const restored = nodeobjectDecompress(compressed.first, compressed.second, decompressBuf); + + EXPECT_EQ(restored.second, blob.size()); + EXPECT_EQ(std::memcmp(restored.first, blob.data(), blob.size()), 0); +} + +// An uncompressed blob is never produced by the compressor but must still decode: leading varint 0 +// followed by the raw payload. +TEST(Codec, uncompressed_passthrough) +{ + static constexpr std::uint8_t kTypeUncompressed = 0; + static constexpr auto payload = std::to_array({0xde, 0xad, 0xbe, 0xef, 0x2a}); + + std::vector blob; + blob.push_back(kTypeUncompressed); // leading varint type tag + blob.insert(blob.end(), payload.begin(), payload.end()); + + nudb::detail::buffer decompressBuf; + auto const restored = nodeobjectDecompress(blob.data(), blob.size(), decompressBuf); + + EXPECT_EQ(restored.second, payload.size()); + EXPECT_EQ(std::memcmp(restored.first, payload.data(), payload.size()), 0); +} diff --git a/src/tests/libxrpl/nodestore/Varint.cpp b/src/tests/libxrpl/nodestore/Varint.cpp new file mode 100644 index 0000000000..3652fd5631 --- /dev/null +++ b/src/tests/libxrpl/nodestore/Varint.cpp @@ -0,0 +1,46 @@ +#include + +#include + +#include +#include +#include +#include + +using namespace xrpl::node_store; + +TEST(Varint, encode_decode) +{ + static constexpr auto kValues = std::to_array({ + 0, + 1, + 2, + 126, + 127, + 128, + 253, + 254, + 255, + 16127, + 16128, + 16129, + 0xff, + 0xffff, + 0xffffffff, + 0xffffffffffffUL, + std::numeric_limits::max(), + }); + + for (auto const value : kValues) + { + std::array::kMax> buffer{}; + auto const bytesWritten = writeVarint(buffer.data(), value); + EXPECT_GT(bytesWritten, 0u); + EXPECT_EQ(bytesWritten, sizeVarint(value)); + + std::size_t decoded = 0; + auto const bytesRead = readVarint(buffer.data(), bytesWritten, decoded); + EXPECT_EQ(bytesRead, bytesWritten); + EXPECT_EQ(value, decoded); + } +} diff --git a/src/tests/libxrpl/nodestore/varint.cpp b/src/tests/libxrpl/nodestore/varint.cpp deleted file mode 100644 index fee96f314b..0000000000 --- a/src/tests/libxrpl/nodestore/varint.cpp +++ /dev/null @@ -1,46 +0,0 @@ -#include - -#include - -#include -#include -#include -#include -#include - -using namespace xrpl::node_store; - -TEST(varint, encode_decode) -{ - std::vector const kVALUES = { - 0, - 1, - 2, - 126, - 127, - 128, - 253, - 254, - 255, - 16127, - 16128, - 16129, - 0xff, - 0xffff, - 0xffffffff, - 0xffffffffffffUL, - 0xffffffffffffffffUL}; - - for (auto const v : kVALUES) - { - SCOPED_TRACE("value=" + std::to_string(v)); - std::array::kMax> vi{}; - auto const n0 = writeVarint(vi.data(), v); - EXPECT_GT(n0, 0u) << "write error"; - EXPECT_EQ(n0, sizeVarint(v)) << "size error"; - std::size_t v1 = 0; - auto const n1 = readVarint(vi.data(), n0, v1); - EXPECT_EQ(n1, n0) << "read error"; - EXPECT_EQ(v1, v) << "wrong value"; - } -} diff --git a/src/tests/libxrpl/protocol/ApiVersion.cpp b/src/tests/libxrpl/protocol/ApiVersion.cpp new file mode 100644 index 0000000000..8af7787102 --- /dev/null +++ b/src/tests/libxrpl/protocol/ApiVersion.cpp @@ -0,0 +1,26 @@ +#include + +#include + +using namespace xrpl; + +TEST(ApiVersion, invariants) +{ + static_assert(RPC::kApiMinimumSupportedVersion <= RPC::kApiMaximumSupportedVersion); + static_assert(RPC::kApiMinimumSupportedVersion <= RPC::kApiMaximumValidVersion); + static_assert(RPC::kApiMaximumSupportedVersion <= RPC::kApiMaximumValidVersion); + static_assert(RPC::kApiBetaVersion <= RPC::kApiMaximumValidVersion); +} + +// Update when we change versions +TEST(ApiVersion, versions) +{ + static_assert(RPC::kApiMinimumSupportedVersion >= 1); + static_assert(RPC::kApiMinimumSupportedVersion < 2); + static_assert(RPC::kApiMaximumSupportedVersion >= 2); + static_assert(RPC::kApiMaximumSupportedVersion < 3); + static_assert(RPC::kApiMaximumValidVersion >= 3); + static_assert(RPC::kApiMaximumValidVersion < 4); + static_assert(RPC::kApiBetaVersion >= 3); + static_assert(RPC::kApiBetaVersion < 4); +} diff --git a/src/tests/libxrpl/protocol/Serializer.cpp b/src/tests/libxrpl/protocol/Serializer.cpp new file mode 100644 index 0000000000..fc5742444a --- /dev/null +++ b/src/tests/libxrpl/protocol/Serializer.cpp @@ -0,0 +1,49 @@ +#include + +#include + +#include +#include +#include + +using namespace xrpl; + +TEST(Serializer, add32_roundtrip) +{ + static constexpr auto kValues = std::to_array({ + std::numeric_limits::min(), + -1, + 0, + 1, + std::numeric_limits::max(), + }); + + for (std::int32_t const value : kValues) + { + Serializer s; + s.add32(value); + EXPECT_EQ(s.size(), 4); + SerialIter sit(s.slice()); + EXPECT_EQ(sit.geti32(), value); + } +} + +TEST(Serializer, add64_roundtrip) +{ + static constexpr auto kValues = std::to_array({ + std::numeric_limits::min(), + -1, + 0, + 1, + std::numeric_limits::max(), + }); + + for (std::int64_t const value : kValues) + { + Serializer s; + s.add64(value); + EXPECT_EQ(s.size(), 8); + SerialIter sit(s.slice()); + EXPECT_EQ(sit.geti64(), value); + } +} From 2db631c915d6f2d3b8df812c122e97de368be2bd Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 27 Jul 2026 13:42:07 -0400 Subject: [PATCH 09/11] ci: Change `server_definitions` upload config name (#7878) --- .github/workflows/reusable-build-test-config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index c1b8e4195b..3023f70cdf 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -266,7 +266,7 @@ jobs: ./xrpld --definitions | python3 -m json.tool >server_definitions.json - name: Upload server definitions - if: ${{ github.event.repository.visibility == 'public' && inputs.config_name == 'debian-gcc-release-amd64' }} + if: ${{ github.event.repository.visibility == 'public' && inputs.config_name == 'ubuntu-gcc-debug-amd64-coverage' }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: server-definitions From ff7fff2cf2e92e99be02487eabba87c5bcab42fc Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Mon, 27 Jul 2026 19:39:53 +0100 Subject: [PATCH 10/11] style: Make clang-tidy format files using clang-format rules (#7880) --- .clang-tidy | 2 ++ .github/workflows/reusable-clang-tidy.yml | 2 +- CONTRIBUTING.md | 6 ++++-- bin/pre-commit/clang_tidy_check.py | 6 +++++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 68fc9e75fc..41df5470ff 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -75,6 +75,8 @@ Checks: "-*, # readability-static-accessed-through-instance, # this check is probably unnecessary. It makes the code less readable # --- +FormatStyle: file + CheckOptions: bugprone-unsafe-functions.ReportMoreUnsafeFunctions: true bugprone-unused-return-value.CheckedReturnTypes: ::std::error_code;::std::error_condition;::std::errc diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index a6c1e6ae56..f1fdc0569a 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -95,7 +95,7 @@ jobs: TARGETS: ${{ needs.determine-files.outputs.need_full_run != 'true' && needs.determine-files.outputs.cpp_changed_files || 'include src tests' }} run: | set -o pipefail - run-clang-tidy -j ${{ steps.nproc.outputs.nproc }} -p "${BUILD_DIR}" -quiet -fix -allow-no-checks ${TARGETS} 2>&1 | tee "${OUTPUT_FILE}" + run-clang-tidy -j ${{ steps.nproc.outputs.nproc }} -p "${BUILD_DIR}" -quiet -fix -format -allow-no-checks ${TARGETS} 2>&1 | tee "${OUTPUT_FILE}" - name: Print filtered clang-tidy errors if: ${{ steps.run_clang_tidy.outcome != 'success' }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7632741e35..fc385cf6ed 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -348,12 +348,14 @@ run-clang-tidy -p build -allow-no-checks src tests ``` This will check all source files in the `src`, `include` and `tests` directories using the compile commands from your `build` directory. -If you wish to automatically fix whatever clang-tidy finds _and_ is capable of fixing, add `-fix` to the above command: +If you wish to automatically fix whatever clang-tidy finds _and_ is capable of fixing, add `-fix -format` to the above command: ``` -run-clang-tidy -p build -quiet -fix -allow-no-checks src tests +run-clang-tidy -p build -quiet -fix -format -allow-no-checks src tests ``` +`-format` reformats the fixed code with [`.clang-format`](./.clang-format); without it the fixes are inserted in LLVM style and the `clang-format` hook rewrites them afterwards. + ## Contracts and instrumentation We are using [Antithesis](https://antithesis.com/) for continuous fuzzing, diff --git a/bin/pre-commit/clang_tidy_check.py b/bin/pre-commit/clang_tidy_check.py index cf4808d2ea..118d9619e2 100755 --- a/bin/pre-commit/clang_tidy_check.py +++ b/bin/pre-commit/clang_tidy_check.py @@ -144,7 +144,11 @@ def main(): + files ) canonicalize_fix_paths(Path(fixes_dir)) - applied = subprocess.run([clang_apply_replacements, fixes_dir]) + # `FormatStyle` in .clang-tidy does not reach this path, + # so ask for the repository style here. + applied = subprocess.run( + [clang_apply_replacements, "--format", "--style=file", fixes_dir] + ) return result.returncode or applied.returncode From 86832edc70aff0eacedcfeda7260230da54439fb Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Mon, 27 Jul 2026 19:48:53 +0100 Subject: [PATCH 11/11] chore: Move semantic version tests to gtest (#7872) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- include/xrpl/beast/core/SemanticVersion.h | 6 +- src/libxrpl/beast/core/SemanticVersion.cpp | 4 +- src/test/beast/SemanticVersion_test.cpp | 266 ---------------- src/tests/libxrpl/beast/SemanticVersion.cpp | 333 ++++++++++++++++++++ 4 files changed, 338 insertions(+), 271 deletions(-) delete mode 100644 src/test/beast/SemanticVersion_test.cpp create mode 100644 src/tests/libxrpl/beast/SemanticVersion.cpp diff --git a/include/xrpl/beast/core/SemanticVersion.h b/include/xrpl/beast/core/SemanticVersion.h index 338942c252..c2e395e3f4 100644 --- a/include/xrpl/beast/core/SemanticVersion.h +++ b/include/xrpl/beast/core/SemanticVersion.h @@ -17,14 +17,14 @@ namespace beast { class SemanticVersion { public: - using identifier_list = std::vector; + using IdentifierList = std::vector; int majorVersion; int minorVersion; int patchVersion; - identifier_list preReleaseIdentifiers; - identifier_list metaData; + IdentifierList preReleaseIdentifiers; + IdentifierList metaData; SemanticVersion(); diff --git a/src/libxrpl/beast/core/SemanticVersion.cpp b/src/libxrpl/beast/core/SemanticVersion.cpp index a99437f8f2..f902a14b07 100644 --- a/src/libxrpl/beast/core/SemanticVersion.cpp +++ b/src/libxrpl/beast/core/SemanticVersion.cpp @@ -15,7 +15,7 @@ namespace beast { std::string -printIdentifiers(SemanticVersion::identifier_list const& list) +printIdentifiers(SemanticVersion::IdentifierList const& list) { std::string ret; @@ -115,7 +115,7 @@ extractIdentifier(std::string& value, bool allowLeadingZeroes, std::string& inpu bool extractIdentifiers( - SemanticVersion::identifier_list& identifiers, + SemanticVersion::IdentifierList& identifiers, bool allowLeadingZeroes, std::string& input) { diff --git a/src/test/beast/SemanticVersion_test.cpp b/src/test/beast/SemanticVersion_test.cpp deleted file mode 100644 index d7079080c8..0000000000 --- a/src/test/beast/SemanticVersion_test.cpp +++ /dev/null @@ -1,266 +0,0 @@ -#include -#include - -#include - -namespace beast { - -class SemanticVersion_test : public unit_test::Suite -{ - using identifier_list = SemanticVersion::identifier_list; - -public: - void - checkPass(std::string const& input, bool shouldPass = true) - { - SemanticVersion v; - - if (shouldPass) - { - BEAST_EXPECT(v.parse(input)); - BEAST_EXPECT(v.print() == input); - } - else - { - BEAST_EXPECT(!v.parse(input)); - } - } - - void - checkFail(std::string const& input) - { - checkPass(input, false); - } - - // check input and input with appended metadata - void - checkMeta(std::string const& input, bool shouldPass) - { - checkPass(input, shouldPass); - - checkPass(input + "+a", shouldPass); - checkPass(input + "+1", shouldPass); - checkPass(input + "+a.b", shouldPass); - checkPass(input + "+ab.cd", shouldPass); - - checkFail(input + "!"); - checkFail(input + "+"); - checkFail(input + "++"); - checkFail(input + "+!"); - checkFail(input + "+."); - checkFail(input + "+a.!"); - } - - void - checkMetaFail(std::string const& input) - { - checkMeta(input, false); - } - - // check input, input with appended release data, - // input with appended metadata, and input with both - // appended release data and appended metadata - // - void - checkRelease(std::string const& input, bool shouldPass = true) - { - checkMeta(input, shouldPass); - - checkMeta(input + "-1", shouldPass); - checkMeta(input + "-a", shouldPass); - checkMeta(input + "-a1", shouldPass); - checkMeta(input + "-a1.b1", shouldPass); - checkMeta(input + "-ab.cd", shouldPass); - checkMeta(input + "--", shouldPass); - - checkMetaFail(input + "+"); - checkMetaFail(input + "!"); - checkMetaFail(input + "-"); - checkMetaFail(input + "-!"); - checkMetaFail(input + "-."); - checkMetaFail(input + "-a.!"); - checkMetaFail(input + "-0.a"); - } - - // Checks the major.minor.version string alone and with all - // possible combinations of release identifiers and metadata. - // - void - check(std::string const& input, bool shouldPass = true) - { - checkRelease(input, shouldPass); - } - - void - negcheck(std::string const& input) - { - check(input, false); - } - - void - testParse() - { - testcase("parsing"); - - check("0.0.0"); - check("1.2.3"); - check("2147483647.2147483647.2147483647"); // max int - - // negative values - negcheck("-1.2.3"); - negcheck("1.-2.3"); - negcheck("1.2.-3"); - - // missing parts - negcheck(""); - negcheck("1"); - negcheck("1."); - negcheck("1.2"); - negcheck("1.2."); - negcheck(".2.3"); - - // whitespace - negcheck(" 1.2.3"); - negcheck("1 .2.3"); - negcheck("1.2 .3"); - negcheck("1.2.3 "); - - // leading zeroes - negcheck("01.2.3"); - negcheck("1.02.3"); - negcheck("1.2.03"); - } - - static identifier_list - ids() - { - return identifier_list(); - } - - static identifier_list - ids(std::string const& s1) - { - identifier_list v; - v.push_back(s1); - return v; - } - - static identifier_list - ids(std::string const& s1, std::string const& s2) - { - identifier_list v; - v.push_back(s1); - v.push_back(s2); - return v; - } - - static identifier_list - ids(std::string const& s1, std::string const& s2, std::string const& s3) - { - identifier_list v; - v.push_back(s1); - v.push_back(s2); - v.push_back(s3); - return v; - } - - // Checks the decomposition of the input into appropriate values - void - checkValues( - std::string const& input, - int majorVersion, - int minorVersion, - int patchVersion, - identifier_list const& preReleaseIdentifiers = identifier_list(), - identifier_list const& metaData = identifier_list()) - { - SemanticVersion v; - - BEAST_EXPECT(v.parse(input)); - - BEAST_EXPECT(v.majorVersion == majorVersion); - BEAST_EXPECT(v.minorVersion == minorVersion); - BEAST_EXPECT(v.patchVersion == patchVersion); - - BEAST_EXPECT(v.preReleaseIdentifiers == preReleaseIdentifiers); - BEAST_EXPECT(v.metaData == metaData); - } - - void - testValues() - { - testcase("values"); - - checkValues("0.1.2", 0, 1, 2); - checkValues("1.2.3", 1, 2, 3); - checkValues("1.2.3-rc1", 1, 2, 3, ids("rc1")); - checkValues("1.2.3-rc1.debug", 1, 2, 3, ids("rc1", "debug")); - checkValues("1.2.3-rc1.debug.asm", 1, 2, 3, ids("rc1", "debug", "asm")); - checkValues("1.2.3+full", 1, 2, 3, ids(), ids("full")); - checkValues("1.2.3+full.prod", 1, 2, 3, ids(), ids("full", "prod")); - checkValues("1.2.3+full.prod.x86", 1, 2, 3, ids(), ids("full", "prod", "x86")); - checkValues( - "1.2.3-rc1.debug.asm+full.prod.x86", - 1, - 2, - 3, - ids("rc1", "debug", "asm"), - ids("full", "prod", "x86")); - } - - // makes sure the left version is less than the right - void - checkLessInternal(std::string const& lhs, std::string const& rhs) - { - SemanticVersion left; - SemanticVersion right; - - BEAST_EXPECT(left.parse(lhs)); - BEAST_EXPECT(right.parse(rhs)); - - BEAST_EXPECT(compare(left, left) == 0); - BEAST_EXPECT(compare(right, right) == 0); - BEAST_EXPECT(compare(left, right) < 0); - BEAST_EXPECT(compare(right, left) > 0); - - BEAST_EXPECT(left < right); - BEAST_EXPECT(right > left); - BEAST_EXPECT(left == left); - BEAST_EXPECT(right == right); - } - - void - checkLess(std::string const& lhs, std::string const& rhs) - { - checkLessInternal(lhs, rhs); - checkLessInternal(lhs + "+meta", rhs); - checkLessInternal(lhs, rhs + "+meta"); - checkLessInternal(lhs + "+meta", rhs + "+meta"); - } - - void - testCompare() - { - testcase("comparisons"); - - checkLess("1.0.0-alpha", "1.0.0-alpha.1"); - checkLess("1.0.0-alpha.1", "1.0.0-alpha.beta"); - checkLess("1.0.0-alpha.beta", "1.0.0-beta"); - checkLess("1.0.0-beta", "1.0.0-beta.2"); - checkLess("1.0.0-beta.2", "1.0.0-beta.11"); - checkLess("1.0.0-beta.11", "1.0.0-rc.1"); - checkLess("1.0.0-rc.1", "1.0.0"); - checkLess("0.9.9", "1.0.0"); - } - - void - run() override - { - testParse(); - testValues(); - testCompare(); - } -}; - -BEAST_DEFINE_TESTSUITE(SemanticVersion, beast, beast); -} // namespace beast diff --git a/src/tests/libxrpl/beast/SemanticVersion.cpp b/src/tests/libxrpl/beast/SemanticVersion.cpp new file mode 100644 index 0000000000..21b33c9476 --- /dev/null +++ b/src/tests/libxrpl/beast/SemanticVersion.cpp @@ -0,0 +1,333 @@ +#include + +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace beast { +namespace { + +using IdentifierList = SemanticVersion::IdentifierList; + +// Version strings are not valid C++ identifiers, so squash their punctuation to +// turn one into a gtest parameter name. +std::string +identifierFor(std::string_view version) +{ + std::string name{version}; + std::ranges::replace_if( + name, [](char c) { return !std::isalnum(c, std::locale::classic()); }, '_'); + if (!name.empty() && std::isdigit(name.front(), std::locale::classic())) + name.insert(0, "v_"); + return name; +} + +// Pre-release and metadata suffixes, each applied to a "major.minor.patch" base. +// The valid ones leave a well-formed base well-formed; the invalid ones make any +// base malformed. +constexpr auto kValidPreRelease = + std::to_array({"", "-1", "-a", "-a1", "-a1.b1", "-ab.cd", "--"}); +constexpr auto kInvalidPreRelease = + std::to_array({"+", "!", "-", "-!", "-.", "-a.!", "-0.a"}); +constexpr auto kValidMetaData = std::to_array({"", "+a", "+1", "+a.b", "+ab.cd"}); +constexpr auto kInvalidMetaData = + std::to_array({"!", "+", "++", "+!", "+.", "+a.!"}); + +// Assembles base + preRelease + metaData and checks whether it parses. A version +// we accept must also round-trip through print(). +void +expectParse( + std::string_view base, + std::string_view preRelease, + std::string_view metaData, + bool shouldPass) +{ + auto const input = std::string{base}.append(preRelease).append(metaData); + SCOPED_TRACE(::testing::Message() << '"' << input << '"'); + + SemanticVersion v; + + if (shouldPass) + { + EXPECT_TRUE(v.parse(input)); + EXPECT_EQ(v.print(), input); + } + else + { + EXPECT_FALSE(v.parse(input)); + } +} + +struct ParseCase +{ + std::string_view testName; + std::string_view base; + bool shouldPass; +}; + +std::string +parseCaseName(::testing::TestParamInfo const& info) +{ + return std::string{info.param.testName}; +} + +constexpr auto kParseCases = std::to_array({ + {.testName = "zeroes", .base = "0.0.0", .shouldPass = true}, + {.testName = "simple", .base = "1.2.3", .shouldPass = true}, + {.testName = "max_int", .base = "2147483647.2147483647.2147483647", .shouldPass = true}, + + // negative values + {.testName = "negative_major", .base = "-1.2.3", .shouldPass = false}, + {.testName = "negative_minor", .base = "1.-2.3", .shouldPass = false}, + {.testName = "negative_patch", .base = "1.2.-3", .shouldPass = false}, + + // missing parts + {.testName = "empty", .base = "", .shouldPass = false}, + {.testName = "major_only", .base = "1", .shouldPass = false}, + {.testName = "major_then_dot", .base = "1.", .shouldPass = false}, + {.testName = "major_and_minor", .base = "1.2", .shouldPass = false}, + {.testName = "major_minor_then_dot", .base = "1.2.", .shouldPass = false}, + {.testName = "missing_major", .base = ".2.3", .shouldPass = false}, + + // whitespace + {.testName = "leading_space", .base = " 1.2.3", .shouldPass = false}, + {.testName = "space_after_major", .base = "1 .2.3", .shouldPass = false}, + {.testName = "space_after_minor", .base = "1.2 .3", .shouldPass = false}, + {.testName = "trailing_space", .base = "1.2.3 ", .shouldPass = false}, + + // leading zeroes + {.testName = "leading_zero_in_major", .base = "01.2.3", .shouldPass = false}, + {.testName = "leading_zero_in_minor", .base = "1.02.3", .shouldPass = false}, + {.testName = "leading_zero_in_patch", .base = "1.2.03", .shouldPass = false}, +}); + +struct ValuesCase +{ + std::string_view testName; + std::string_view input; + int majorVersion; + int minorVersion; + int patchVersion; + IdentifierList preReleaseIdentifiers{}; // NOLINT(readability-redundant-member-init) + IdentifierList metaData{}; // NOLINT(readability-redundant-member-init) +}; + +std::string +valuesCaseName(::testing::TestParamInfo const& info) +{ + return std::string{info.param.testName}; +} + +std::vector const kValuesCases{ + { + .testName = "zero_major", + .input = "0.1.2", + .majorVersion = 0, + .minorVersion = 1, + .patchVersion = 2, + }, + { + .testName = "simple", + .input = "1.2.3", + .majorVersion = 1, + .minorVersion = 2, + .patchVersion = 3, + }, + { + .testName = "one_pre_release_identifier", + .input = "1.2.3-rc1", + .majorVersion = 1, + .minorVersion = 2, + .patchVersion = 3, + .preReleaseIdentifiers = {"rc1"}, + }, + { + .testName = "two_pre_release_identifiers", + .input = "1.2.3-rc1.debug", + .majorVersion = 1, + .minorVersion = 2, + .patchVersion = 3, + .preReleaseIdentifiers = {"rc1", "debug"}, + }, + { + .testName = "three_pre_release_identifiers", + .input = "1.2.3-rc1.debug.asm", + .majorVersion = 1, + .minorVersion = 2, + .patchVersion = 3, + .preReleaseIdentifiers = {"rc1", "debug", "asm"}, + }, + { + .testName = "one_metadata_identifier", + .input = "1.2.3+full", + .majorVersion = 1, + .minorVersion = 2, + .patchVersion = 3, + .metaData = {"full"}, + }, + { + .testName = "two_metadata_identifiers", + .input = "1.2.3+full.prod", + .majorVersion = 1, + .minorVersion = 2, + .patchVersion = 3, + .metaData = {"full", "prod"}, + }, + { + .testName = "three_metadata_identifiers", + .input = "1.2.3+full.prod.x86", + .majorVersion = 1, + .minorVersion = 2, + .patchVersion = 3, + .metaData = {"full", "prod", "x86"}, + }, + { + .testName = "pre_release_and_metadata", + .input = "1.2.3-rc1.debug.asm+full.prod.x86", + .majorVersion = 1, + .minorVersion = 2, + .patchVersion = 3, + .preReleaseIdentifiers = {"rc1", "debug", "asm"}, + .metaData = {"full", "prod", "x86"}, + }, +}; + +struct OrderCase +{ + std::string_view lesser; + std::string_view greater; +}; + +std::string +orderCaseName(::testing::TestParamInfo const& info) +{ + return identifierFor(info.param.lesser) + "_below_" + identifierFor(info.param.greater); +} + +constexpr auto kOrderCases = std::to_array({ + {.lesser = "1.0.0-alpha", .greater = "1.0.0-alpha.1"}, + {.lesser = "1.0.0-alpha.1", .greater = "1.0.0-alpha.beta"}, + {.lesser = "1.0.0-alpha.beta", .greater = "1.0.0-beta"}, + {.lesser = "1.0.0-beta", .greater = "1.0.0-beta.2"}, + {.lesser = "1.0.0-beta.2", .greater = "1.0.0-beta.11"}, + {.lesser = "1.0.0-beta.11", .greater = "1.0.0-rc.1"}, + {.lesser = "1.0.0-rc.1", .greater = "1.0.0"}, + {.lesser = "0.9.9", .greater = "1.0.0"}, +}); + +} // namespace + +class SemanticVersionParse : public ::testing::TestWithParam +{ +}; + +// Exercises the base string on its own and with every combination of appended +// pre-release identifiers and metadata. +TEST_P(SemanticVersionParse, pre_release_and_metadata_combinations) +{ + auto const& [testName, base, shouldPass] = GetParam(); + + for (auto const preRelease : kValidPreRelease) + { + for (auto const metaData : kValidMetaData) + expectParse(base, preRelease, metaData, shouldPass); + + for (auto const metaData : kInvalidMetaData) + expectParse(base, preRelease, metaData, false); + } + + // A malformed pre-release section poisons the whole string, whatever + // metadata follows it. + for (auto const preRelease : kInvalidPreRelease) + { + for (auto const metaData : kValidMetaData) + expectParse(base, preRelease, metaData, false); + + for (auto const metaData : kInvalidMetaData) + expectParse(base, preRelease, metaData, false); + } +} + +INSTANTIATE_TEST_SUITE_P( + Inputs, + SemanticVersionParse, + ::testing::ValuesIn(kParseCases), + parseCaseName); + +class SemanticVersionValues : public ::testing::TestWithParam +{ +}; + +TEST_P(SemanticVersionValues, decomposes_into_components) +{ + auto const& expected = GetParam(); + + SemanticVersion v; + EXPECT_TRUE(v.parse(expected.input)); + + EXPECT_EQ(v.majorVersion, expected.majorVersion); + EXPECT_EQ(v.minorVersion, expected.minorVersion); + EXPECT_EQ(v.patchVersion, expected.patchVersion); + + EXPECT_EQ(v.preReleaseIdentifiers, expected.preReleaseIdentifiers); + EXPECT_EQ(v.metaData, expected.metaData); +} + +INSTANTIATE_TEST_SUITE_P( + Inputs, + SemanticVersionValues, + ::testing::ValuesIn(kValuesCases), + valuesCaseName); + +class SemanticVersionOrder : public ::testing::TestWithParam +{ +}; + +TEST_P(SemanticVersionOrder, lesser_precedes_greater) +{ + auto const& [lesser, greater] = GetParam(); + + // Metadata takes no part in precedence, so attaching it to either side must + // leave the ordering untouched. + static constexpr auto kMetaData = std::to_array({"", "+meta"}); + + for (auto const lesserMetaData : kMetaData) + { + for (auto const greaterMetaData : kMetaData) + { + auto const lesserInput = std::string{lesser}.append(lesserMetaData); + auto const greaterInput = std::string{greater}.append(greaterMetaData); + SCOPED_TRACE( + ::testing::Message() << '"' << lesserInput << "\" < \"" << greaterInput << '"'); + + SemanticVersion lesserVersion; + SemanticVersion greaterVersion; + EXPECT_TRUE(lesserVersion.parse(lesserInput)); + EXPECT_TRUE(greaterVersion.parse(greaterInput)); + + EXPECT_EQ(compare(lesserVersion, lesserVersion), 0); + EXPECT_EQ(compare(greaterVersion, greaterVersion), 0); + EXPECT_LT(compare(lesserVersion, greaterVersion), 0); + EXPECT_GT(compare(greaterVersion, lesserVersion), 0); + + EXPECT_LT(lesserVersion, greaterVersion); + EXPECT_GT(greaterVersion, lesserVersion); + EXPECT_EQ(lesserVersion, lesserVersion); + EXPECT_EQ(greaterVersion, greaterVersion); + } + } +} + +INSTANTIATE_TEST_SUITE_P( + Pairs, + SemanticVersionOrder, + ::testing::ValuesIn(kOrderCases), + orderCaseName); + +} // namespace beast