From cc582750c729885072b116c9ce7b5b333dd7a920 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 28 Jul 2026 19:53:19 -0700 Subject: [PATCH 1/7] chore: open #1736 platform-floor lane Co-Authored-By: Claude From 992781b2fccaa1c3844601915b43ecfd8265c54f Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 28 Jul 2026 20:05:13 -0700 Subject: [PATCH 2/7] fix(build): lower the Linux glibc floor to 2.31 and publish linux-arm64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shipped linux-x64 binary silently began requiring glibc >= 2.38, so it would not start on Ubuntu 22.04 LTS (2.35), Debian 12 (2.36) or Amazon Linux 2023 (2.34) — the three most common server distros. No code caused it: GitHub repointed `ubuntu-latest` at Ubuntu 24.04, and a glibc-linked binary runs on its BUILD glibc and anything newer, never anything older. The builder image, and nothing else, sets the supported-OS floor. Both Linux targets now build NATIVELY inside a pinned `debian:11` container (glibc 2.31), which clears every distro above plus Ubuntu 20.04, Debian 11 and RHEL 9. linux-arm64 is published for the first time, built on the free public-repo arm64 runner — so the vendored openssl-sys in the Chia-wallet-SDK graph is compiled by a native toolchain and the old 'cross-compiling OpenSSL is fragile' objection to arm64 no longer applies. The apt .deb moves into the same container and gains an arm64 leg; apt's arm64 index previously had no dig-node at all. The floor is now GATED, not merely claimed: it is declared once in .github/actions/setup-linux-build, asserted against the build container so image and number cannot drift, and asserted against every produced binary by scripts/check-glibc-floor.sh. That gate is pinned from both sides — an at-the-floor binary must pass and a one-minor-over binary must fail — and the release job additionally re-runs it with an impossible 2.0 floor, because a gate that cannot fail is a rubber stamp. Its version comparison is version-aware, not lexical: glibc 2.4 is OLDER than 2.31. Refs #1736, #1735, #1741 Co-Authored-By: Claude --- .github/actions/setup-linux-build/action.yml | 71 +++++++ .github/actions/stage-binaries/action.yml | 53 +++++ .github/workflows/build-binaries.yml | 196 +++++++++++++++---- .github/workflows/ci.yml | 20 ++ .github/workflows/package.yml | 58 ++++-- scripts/check-glibc-floor.sh | 66 +++++++ scripts/tests/check-glibc-floor.test.sh | 93 +++++++++ 7 files changed, 508 insertions(+), 49 deletions(-) create mode 100644 .github/actions/setup-linux-build/action.yml create mode 100644 .github/actions/stage-binaries/action.yml create mode 100755 scripts/check-glibc-floor.sh create mode 100755 scripts/tests/check-glibc-floor.test.sh diff --git a/.github/actions/setup-linux-build/action.yml b/.github/actions/setup-linux-build/action.yml new file mode 100644 index 0000000..f74c9f3 --- /dev/null +++ b/.github/actions/setup-linux-build/action.yml @@ -0,0 +1,71 @@ +# Prepares a bare old-glibc container to build the dig-node Linux artifacts, and DECLARES the +# canonical glibc floor those artifacts must honour. +# +# WHY (#1736): a glibc-linked binary runs on its BUILD glibc and anything newer, never anything +# older — so the builder image, and nothing else, sets the supported-OS floor. dig-node v0.64.0 +# silently began requiring glibc >= 2.38 (breaking Ubuntu 22.04 LTS, Debian 12 and Amazon Linux +# 2023) purely because GitHub repointed `ubuntu-latest` at Ubuntu 24.04. Every job that produces a +# shipped Linux artifact — the release binaries AND the apt `.deb` — therefore runs inside a PINNED +# container and calls this action, so there is one floor rather than one per workflow. +# +# The job must declare `container: debian:11`; this action asserts that the container it is running +# in really provides `glibc-floor`, so the image and the number cannot drift apart. +# +# Because this is a LOCAL composite action it can only run after `actions/checkout`, and checkout in +# a bare container needs `git` + `curl` + CA certificates already present — so each calling job +# bootstraps exactly those three packages first, then checks out, then calls this action. +name: Set up an old-glibc Linux build +description: Install the Rust + C toolchains in the pinned old-glibc container and assert its glibc matches the canonical floor. + +outputs: + glibc-floor: + description: "The canonical minimum glibc version every published dig-node Linux artifact must run on." + value: ${{ steps.floor.outputs.version }} + +runs: + using: composite + steps: + - name: Declare the canonical glibc floor + id: floor + shell: bash + # THE single source of truth for the floor. 2.31 (Debian 11) clears every distro #1736 + # reported broken — Ubuntu 22.04 (2.35), Debian 12 (2.36), Amazon Linux 2023 (2.34) — plus + # Ubuntu 20.04 (2.31), Debian 11 (2.31) and RHEL 9 (2.34). Raising it is a DELIBERATE act: + # change this value, the `container:` image in every calling job, SPEC.md §11.3 and the + # published docs together — never one alone. + run: echo "version=2.31" >>"$GITHUB_OUTPUT" + + - name: Install the Rust + C toolchains + shell: bash + # The container is a bare Debian 11 with neither toolchain. The C toolchain is not optional: + # `openssl-sys` builds vendored OpenSSL from source (needs perl + make) and several other + # -sys crates in the Chia-wallet-SDK graph want cmake/clang. `binutils` provides the + # `readelf` the floor gate reads. + run: | + set -eux + apt-get update -qq + apt-get install -y --no-install-recommends \ + ca-certificates curl git build-essential pkg-config perl make cmake clang binutils + + - name: Verify the container glibc IS the declared floor + shell: bash + run: | + set -eu + actual="$(ldd --version | head -n 1 | grep -oE '[0-9]+\.[0-9]+$')" + declared="${{ steps.floor.outputs.version }}" + echo "container glibc: $actual, declared floor: $declared" + test "$actual" = "$declared" || { + echo "::error::build container glibc $actual != the declared floor $declared. Update the floor in .github/actions/setup-linux-build/action.yml, every calling job's container: image, SPEC.md §11.3 and the docs TOGETHER." + exit 1 + } + + - name: Install Rust via rustup + shell: bash + # rustup, not Debian 11's rustc, which is far older than this workspace needs. Only the Rust + # toolchain comes from rustup — the glibc it links against is the container's, which is the + # entire point of the container. + run: | + set -eu + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs \ + | sh -s -- -y --default-toolchain stable --profile minimal --no-modify-path + echo "$HOME/.cargo/bin" >>"$GITHUB_PATH" diff --git a/.github/actions/stage-binaries/action.yml b/.github/actions/stage-binaries/action.yml new file mode 100644 index 0000000..a5826f9 --- /dev/null +++ b/.github/actions/stage-binaries/action.yml @@ -0,0 +1,53 @@ +# Stages the two release binaries under their CANONICAL asset names (SPEC §11.2) and uploads them. +# +# WHY this is a composite action rather than an inline step: the binaries are produced by TWO jobs +# in build-binaries.yml — the Windows/macOS matrix on its host runners, and the Linux matrix inside +# a pinned old-glibc container (#1736) — and the ONE thing that must never diverge between them is +# the asset naming every downstream consumer resolves (the dig-installer thin-shim, apt.dig.net's +# packaging template). Naming lives here, once. +name: Stage release binaries +description: Copy the built dig-node + dign binaries to dist/ under their canonical asset names and upload them. + +inputs: + version: + description: "Version string stamped into each artifact filename (e.g. `1.2.3` or `1.2.3-nightly.20260714.abc1234`)." + required: true + target: + description: "Rust target triple the binaries were built for (selects the target//release dir)." + required: true + out-name: + description: "Canonical OS/arch asset suffix, e.g. `linux-x64`, `linux-arm64`, `windows-x64.exe`." + required: true + exe-suffix: + description: "Executable suffix of the built binary — `.exe` on Windows, empty elsewhere." + required: false + default: "" + +runs: + using: composite + steps: + - name: Stage artifact (dual-named) + shell: bash + run: | + set -euo pipefail + VER="${{ inputs.version }}" + SUFFIX="${{ inputs.exe-suffix }}" + mkdir -p dist + # `dig-node` is the service binary; `dign` is its FIRST-CLASS alias (issue #548) — a + # separate [[bin]] over the same entrypoint, published under its own stem so the + # installer can resolve it via Repo::dign(). Both are required; a missing one is a + # release-shaped failure, so fail loudly rather than publishing half the pair. + for stem in dig-node dign; do + src="target/${{ inputs.target }}/release/${stem}${SUFFIX}" + test -f "$src" || { echo "::error::binary not produced: $src"; exit 1; } + # out-name already carries the `.exe` on Windows, so it is never appended twice. + cp "$src" "dist/${stem}-${VER}-${{ inputs.out-name }}" + done + ls -la dist + + - name: Upload build artifact + uses: actions/upload-artifact@v4 + with: + name: dig-node-${{ inputs.out-name }} + path: dist/* + if-no-files-found: error diff --git a/.github/workflows/build-binaries.yml b/.github/workflows/build-binaries.yml index 5cc953f..bab639c 100644 --- a/.github/workflows/build-binaries.yml +++ b/.github/workflows/build-binaries.yml @@ -21,9 +21,21 @@ # `dign` is a FIRST-CLASS alias binary (issue #548): a SEPARATE `[[bin]]` target sharing the SAME # entrypoint, published alongside `dig-node` under its own stem `dign---[.exe]`. # -# NO linux-arm64 asset is published: the Linux build graph pulls `openssl-sys` (via the Chia wallet -# SDK), so an aarch64-Linux cross-compile would also have to cross-compile OpenSSL — fragile, and -# no consumer requests it (SPEC §11.3). +# THE LINUX TARGETS BUILD INSIDE A PINNED OLD-GLIBC CONTAINER (`debian:11`, glibc 2.31), in their +# own `build-linux` job — NOT on the runner image (#1736). A glibc-linked binary runs on the build +# glibc and anything NEWER, never anything older, so the builder image silently dictates the +# supported-OS floor. When GitHub moved `ubuntu-latest` to Ubuntu 24.04 the v0.64.0 `linux-x64` +# binary began requiring glibc >= 2.38 and stopped starting on Ubuntu 22.04 LTS (2.35), Debian 12 +# (2.36) and Amazon Linux 2023 (2.34) — the three most common server distros — with no code change +# and no signal. Pinning the container makes the floor an explicit, reviewable decision, and +# `scripts/check-glibc-floor.sh` FAILS the build if a produced binary exceeds it (#1741 covers the +# arch half of the same problem). +# +# `linux-arm64` IS published (#1741): it builds NATIVELY on the free public-repo `ubuntu-24.04-arm` +# runner, so the vendored `openssl-sys` in the Chia-wallet-SDK graph is compiled by a native +# toolchain. The old SPEC §11.3 rationale for omitting arm64 was about CROSS-compiling OpenSSL; +# a native arm64 runner removes that objection entirely. Graviton/Ampere/Pi hosts are exactly the +# always-on machines a volunteer node runs on. # # The gate runs fmt + clippy ONLY — a PR's own CI (ci.yml) already ran the full # fmt/clippy/test/coverage set (`cargo llvm-cov nextest --workspace`) against the merged tree @@ -83,6 +95,9 @@ jobs: - run: cargo fmt --all --check - run: cargo clippy --all-targets --locked -- -D warnings + # Windows + macOS build on their HOST runner images. Neither has a glibc floor to defend: the + # Windows binary targets the MSVC CRT and macOS pins its floor via the SDK/deployment target, so + # only the Linux targets need the pinned-container treatment (`build-linux` below). build: name: build (${{ matrix.out_name }}) needs: check @@ -92,23 +107,17 @@ jobs: include: - os: windows-latest target: x86_64-pc-windows-msvc - bin: dig-node.exe + exe_suffix: ".exe" out_name: windows-x64.exe - - os: ubuntu-latest - target: x86_64-unknown-linux-gnu - bin: dig-node - out_name: linux-x64 # Both macOS arches build on the fast Apple-silicon macos-14 runner; the Intel binary is # CROSS-COMPILED there (host arm64 → target x86_64-apple-darwin). The only C/asm dep in # the macOS graph is `ring` (via rustls); `openssl-sys` is cfg-gated to non-Apple targets, # so there is no vendored-openssl cross-compile to worry about. - os: macos-14 # Apple silicon (arm64) target: aarch64-apple-darwin - bin: dig-node out_name: macos-arm64 - os: macos-14 # Apple silicon — cross-compiles the Intel binary target: x86_64-apple-darwin - bin: dig-node out_name: macos-x64 runs-on: ${{ matrix.os }} steps: @@ -136,32 +145,147 @@ jobs: # Build BOTH bins — `dign` is the first-class alias (issue #548), published alongside # `dig-node` under its own asset stem below. run: cargo build --release --locked --target ${{ matrix.target }} --bin dig-node --bin dign - - name: Stage artifact (dual-named) - id: stage - # The caller's `version` is stamped into the filename verbatim: a plain `1.2.3` for a - # stable tag, or the synthesized `1.2.3-nightly.YYYYMMDD.` for a nightly. - shell: bash + # The caller's `version` is stamped into the filename verbatim: a plain `1.2.3` for a stable + # tag, or the synthesized `1.2.3-nightly.YYYYMMDD.` for a nightly. Naming lives in + # the composite action so this job and `build-linux` can never diverge on it. + - uses: ./.github/actions/stage-binaries + with: + version: ${{ inputs.version }} + target: ${{ matrix.target }} + out-name: ${{ matrix.out_name }} + exe-suffix: ${{ matrix.exe_suffix }} + + # The LINUX targets, built NATIVELY inside a pinned old-glibc container so the supported-OS floor + # is a decision rather than a side effect of whatever image `ubuntu-latest` points at this month + # (#1736 — see the header). aarch64 builds on the free public-repo arm64 runner (#1741), which + # means the vendored `openssl-sys` in the Chia-wallet-SDK graph is never cross-compiled. + build-linux: + name: build (${{ matrix.out_name }}) + needs: check + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu-latest + target: x86_64-unknown-linux-gnu + out_name: linux-x64 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-gnu + out_name: linux-arm64 + runs-on: ${{ matrix.runner }} + # Debian 11 = glibc 2.31 = the floor declared by `.github/actions/setup-linux-build`, which + # asserts the two agree — so bumping this image without bumping the floor fails the build + # instead of silently shipping a higher floor. + container: debian:11 + steps: + # Bootstrap only what `actions/checkout` itself needs in a bare container; the full toolchain + # (and the floor assertion) comes from the local composite action, which cannot run until the + # repository is on disk. + - name: Bootstrap the container for checkout + run: | + set -eux + apt-get update -qq + apt-get install -y --no-install-recommends ca-certificates curl git + - uses: actions/checkout@v4 + with: + ref: ${{ inputs.ref }} + persist-credentials: false + - name: Trust the checkout (git runs as root in the container) + run: git config --global --add safe.directory "$GITHUB_WORKSPACE" + - id: setup + uses: ./.github/actions/setup-linux-build + - run: rustc --version && cargo --version + - uses: actions/cache@v4 + with: + path: | + ~/.cargo/registry/index + ~/.cargo/registry/cache + ~/.cargo/git/db + target + # The glibc floor is part of the cache key: artifacts built against a different glibc must + # never be restored into a build that claims a different floor. + key: linux-glibc${{ steps.setup.outputs.glibc-floor }}-build-${{ matrix.target }}-${{ hashFiles('**/Cargo.lock') }} + restore-keys: linux-glibc${{ steps.setup.outputs.glibc-floor }}-build-${{ matrix.target }}- + - name: Build release binaries (dig-node + dign alias) + # The triple is the HOST's here (native build, no cross-compilation), but it is passed + # explicitly anyway so the output lands in `target//release/` — the one path the + # shared staging action reads on every platform. + run: cargo build --release --locked --target ${{ matrix.target }} --bin dig-node --bin dign + - uses: ./.github/actions/stage-binaries + with: + version: ${{ inputs.version }} + target: ${{ matrix.target }} + out-name: ${{ matrix.out_name }} + - name: Assert the glibc floor of the produced binaries + # The gate that makes the floor durable. Both binaries are checked: `dign` is a separate + # link, so checking only `dig-node` would leave the alias unguarded. run: | - VER="${{ inputs.version }}" - mkdir -p dist - SRC="target/${{ matrix.target }}/release/${{ matrix.bin }}" - test -f "$SRC" || { echo "binary not produced: $SRC"; exit 1; } - # On Windows out_name already ends in .exe; on unix there is no extension. Publish the - # binary under the canonical dig-node-* name every consumer resolves (#585 dropped the - # duplicate legacy dig-companion-* copy). See the header comment. - cp "$SRC" "dist/dig-node-${VER}-${{ matrix.out_name }}" - # The `dign` first-class alias (issue #548): a SEPARATE binary that behaves byte-for-byte - # like `dig-node`, published under its own `dign-*` stem (same shape as `dig-node-*`) so - # the dig-installer resolves it via Repo::dign(). Its filename differs from `dig-node` - # only by the `dig-node`->`dign` substring (incl. the `.exe` suffix on Windows). - DIGN_BIN="${{ matrix.bin }}"; DIGN_BIN="${DIGN_BIN/dig-node/dign}" - DIGN_SRC="target/${{ matrix.target }}/release/${DIGN_BIN}" - test -f "$DIGN_SRC" || { echo "dign binary not produced: $DIGN_SRC"; exit 1; } - cp "$DIGN_SRC" "dist/dign-${VER}-${{ matrix.out_name }}" - ls -la dist - - name: Upload build artifact - uses: actions/upload-artifact@v4 + set -eu + for stem in dig-node dign; do + bin="dist/${stem}-${{ inputs.version }}-${{ matrix.out_name }}" + bash scripts/check-glibc-floor.sh "$bin" "${{ steps.setup.outputs.glibc-floor }}" + # Prove the gate is not a no-op against a REAL binary, not just against the unit + # tests' stub readelf: every ELF references at least the base glibc version + # (2.2.5 on x86_64, 2.17 on aarch64), so a floor of 2.0 MUST be rejected. A gate that + # cannot fail is a rubber stamp. + if bash scripts/check-glibc-floor.sh "$bin" 2.0 >/dev/null 2>&1; then + echo "::error::check-glibc-floor.sh accepted an impossible 2.0 floor for $bin — the gate is not working" + exit 1 + fi + done + + # PROOF, not inference: start each published Linux binary on the OLDEST distros it claims to + # support. `ldd --version` on the builder says what the binary was linked against; only executing + # it on the target says it STARTS there. #1736 was found by a binary that died at exec time. + # + # WHY `--version`: it is the one invocation that touches no config, no network, no filesystem + # state and no service manager, so it isolates "does the dynamic loader resolve this binary". + verify-linux-floor: + name: verify (${{ matrix.out_name }} on ${{ matrix.image }}) + needs: build-linux + strategy: + fail-fast: false + matrix: + include: + # The three distros #1736 reported broken, each on its own arch's runner so the binary + # executes natively rather than under emulation. + - runner: ubuntu-latest + out_name: linux-x64 + image: ubuntu:22.04 + - runner: ubuntu-latest + out_name: linux-x64 + image: debian:12 + - runner: ubuntu-latest + out_name: linux-x64 + image: amazonlinux:2023 + - runner: ubuntu-24.04-arm + out_name: linux-arm64 + image: ubuntu:22.04 + - runner: ubuntu-24.04-arm + out_name: linux-arm64 + image: debian:12 + - runner: ubuntu-24.04-arm + out_name: linux-arm64 + image: amazonlinux:2023 + runs-on: ${{ matrix.runner }} + steps: + - uses: actions/download-artifact@v4 with: name: dig-node-${{ matrix.out_name }} - path: dist/* - if-no-files-found: error + path: dist + - name: Run both binaries on ${{ matrix.image }} + run: | + set -eu + chmod +x dist/* + for stem in dig-node dign; do + bin="${stem}-${{ inputs.version }}-${{ matrix.out_name }}" + echo "::group::$bin on ${{ matrix.image }}" + # `ldd` FIRST and unconditionally: the dynamic loader names only the FIRST + # unsatisfied library, so a partial fix reads as a brand-new regression. Logging the + # full dependency set makes the real remaining gap visible in one run. + docker run --rm -v "$PWD/dist:/dist:ro" "${{ matrix.image }}" \ + sh -c "ldd /dist/$bin || true" + docker run --rm -v "$PWD/dist:/dist:ro" "${{ matrix.image }}" \ + /dist/$bin --version + echo "::endgroup::" + done diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dfee6bd..363e774 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,6 +30,26 @@ env: RUST_BACKTRACE: 1 jobs: + # The release path's CI-consumed shell scripts have their own tests, and they gate a real + # shipping property (#1736: the glibc floor of every published Linux artifact), so they run on + # every PR rather than only when a release is being built — a broken floor gate discovered at + # release time is discovered too late. + scripts: + name: Release-script tests + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: scripts/tests + run: | + set -eu + for t in scripts/tests/*.test.sh; do + echo "::group::$t" + bash "$t" + echo "::endgroup::" + done + fmt: name: Rustfmt runs-on: ubuntu-latest diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index c4ca4bb..c56866f 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -16,9 +16,11 @@ on: - "packaging/**" - "src/**" - "crates/**" + - "scripts/**" - "Cargo.toml" - "Cargo.lock" - ".github/workflows/package.yml" + - ".github/actions/**" push: tags: - "v*" @@ -30,18 +32,43 @@ env: CARGO_PROFILE_RELEASE_DEBUG: "0" jobs: - # ---- Ubuntu .deb ---------------------------------------------------------- + # ---- Ubuntu/Debian .deb (amd64 + arm64) ----------------------------------- + # + # Built inside the SAME pinned old-glibc container as the release binaries + # (`.github/actions/setup-linux-build`, glibc 2.31 — #1736). This .deb is what apt.dig.net serves, + # so building it on `ubuntu-latest` made `apt-get install dig-node` produce a binary that would + # not start on Ubuntu 22.04 LTS, Debian 12 or Amazon Linux 2023 — the same defect as the raw + # binary, on the path most users actually take. + # + # arm64 is built NATIVELY on the free public-repo arm64 runner (#1741): apt's arm64 index + # previously contained no `dig-node` at all, so every Graviton/Ampere/Pi host was unserved. deb: - name: build .deb (linux-x64) - runs-on: ubuntu-latest + name: build .deb (linux-${{ matrix.dpkg_arch }}) + runs-on: ${{ matrix.runner }} + container: debian:11 + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu-latest + target: x86_64-unknown-linux-gnu + dpkg_arch: amd64 + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-gnu + dpkg_arch: arm64 steps: + - name: Bootstrap the container for checkout + run: | + set -eux + apt-get update -qq + apt-get install -y --no-install-recommends ca-certificates curl git - uses: actions/checkout@v4 with: persist-credentials: false - - name: Install Rust - run: | - rustup toolchain install stable - rustup default stable + - name: Trust the checkout (git runs as root in the container) + run: git config --global --add safe.directory "$GITHUB_WORKSPACE" + - id: setup + uses: ./.github/actions/setup-linux-build - uses: actions/cache@v4 with: path: | @@ -49,8 +76,8 @@ jobs: ~/.cargo/registry/cache ~/.cargo/git/db target - key: ${{ runner.os }}-pkg-deb-${{ hashFiles('**/Cargo.lock') }} - restore-keys: ${{ runner.os }}-pkg-deb- + key: pkg-deb-glibc${{ steps.setup.outputs.glibc-floor }}-${{ matrix.target }}-${{ hashFiles('**/Cargo.lock') }} + restore-keys: pkg-deb-glibc${{ steps.setup.outputs.glibc-floor }}-${{ matrix.target }}- - name: Resolve version id: ver run: | @@ -58,16 +85,21 @@ jobs: V="$(grep -m1 '^version' Cargo.toml | sed -E 's/.*"(.*)".*/\1/')"; fi echo "v=$V" >> "$GITHUB_OUTPUT" - name: Build release binary - run: cargo build --release --locked --bin dig-node + run: cargo build --release --locked --target ${{ matrix.target }} --bin dig-node + - name: Assert the packaged binary honours the glibc floor + # Gate the .deb's payload, not just the raw release binary: apt is a separate delivery path + # and would otherwise be able to regress on its own. + run: bash scripts/check-glibc-floor.sh "target/${{ matrix.target }}/release/dig-node" "${{ steps.setup.outputs.glibc-floor }}" - name: Build .deb - run: bash packaging/linux/build-deb.sh target/release/dig-node "${{ steps.ver.outputs.v }}" amd64 dist + run: bash packaging/linux/build-deb.sh "target/${{ matrix.target }}/release/dig-node" "${{ steps.ver.outputs.v }}" "${{ matrix.dpkg_arch }}" dist - name: Validate .deb metadata run: | dpkg-deb --info dist/*.deb dpkg-deb --contents dist/*.deb - uses: actions/upload-artifact@v4 with: - name: dig-node-deb + # One artifact per arch — a shared name would make the two matrix legs collide. + name: dig-node-deb-${{ matrix.dpkg_arch }} path: dist/*.deb if-no-files-found: error @@ -175,7 +207,7 @@ jobs: # ---- Attach to the GitHub Release (tags only) ----------------------------- publish: name: Attach packages to the release - needs: [deb, pkg, msi] + needs: [deb, pkg, msi] # `deb` is a matrix; the whole matrix must be green before publishing if: github.ref_type == 'tag' runs-on: ubuntu-latest permissions: diff --git a/scripts/check-glibc-floor.sh b/scripts/check-glibc-floor.sh new file mode 100755 index 0000000..250e774 --- /dev/null +++ b/scripts/check-glibc-floor.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# +# Assert that a shipped Linux binary does not require a NEWER glibc than the declared floor. +# +# WHY this gate exists (#1736): dig-node v0.64.0's `linux-x64` binary silently began requiring +# glibc >= 2.38 — not from any code change, but because GitHub moved the `ubuntu-latest` runner to +# Ubuntu 24.04. That single image change broke installation on Ubuntu 22.04 LTS (2.35), Debian 12 +# (2.36) and Amazon Linux 2023 (2.34): the three most common server distros in existence. A +# supported-floor CLAIM in a doc cannot notice that happening; only a gate over the produced +# artifact can, so the release build runs this against every Linux binary it publishes. +# +# Usage: check-glibc-floor.sh +# Exit: 0 = binary's highest glibc requirement is <= +# 1 = the floor has RISEN (the regression this gate catches) +# 2 = usage error (never silently green — that is how a gate rots into a no-op) +set -uo pipefail + +# Overridable so the unit tests can feed exact adversarial symbol tables (scripts/tests/). +READELF="${READELF_BIN:-readelf}" + +binary="${1:-}" +floor="${2:-}" + +if [ -z "$binary" ] || [ -z "$floor" ]; then + echo "usage: $(basename "$0") e.g. dist/dig-node-1.2.3-linux-x64 2.31" >&2 + exit 2 +fi +if [ ! -f "$binary" ]; then + echo "check-glibc-floor: no such binary: $binary" >&2 + exit 2 +fi + +# `-V` reports the .gnu.version_r needs (the authoritative "this binary requires GLIBC_x.y" list); +# `--dyn-syms` catches per-symbol tags such as `memcpy@GLIBC_2.14`. Reading both means neither an +# unusual link layout nor a stripped version section can hide a requirement. +# +# The `[0-9]` after the underscore is load-bearing: it drops the unversioned `GLIBC_PRIVATE` tag, +# which would otherwise pollute the maximum. +requirements="$("$READELF" -V --dyn-syms --wide "$binary" 2>/dev/null | + grep -oE 'GLIBC_[0-9]+(\.[0-9]+)+' | sed 's/^GLIBC_//' | sort -Vu)" + +if [ -z "$requirements" ]; then + echo "check-glibc-floor: $binary requires no versioned glibc symbols (static or musl) — OK" + exit 0 +fi + +# `sort -V` compares each dotted component NUMERICALLY. A lexical sort would rank glibc 2.4 above +# 2.31 and reject a perfectly old binary, so the version-aware sort is required, not cosmetic. +highest="$(printf '%s\n' "$requirements" | tail -n 1)" +newest_of_pair="$(printf '%s\n%s\n' "$floor" "$highest" | sort -V | tail -n 1)" + +echo "check-glibc-floor: $binary requires glibc up to $highest (declared floor $floor)" +echo " all requirements: $(printf '%s ' $requirements)" + +if [ "$newest_of_pair" != "$floor" ]; then + cat >&2 <"$path" + chmod +x "$path" + echo "$path" +} + +# expect +expect() { + local name="$1" want="$2" stub="$3" floor="$4" + local out status + out="$(READELF_BIN="$stub" bash "$GATE" "$WORK/fake-binary" "$floor" 2>&1)" + status=$? + if [ "$status" -ne "$want" ]; then + printf 'FAIL %s: exit %s, want %s\n%s\n' "$name" "$status" "$want" "$out" + failures=$((failures + 1)) + else + printf 'ok %s\n' "$name" + fi +} + +: >"$WORK/fake-binary" + +# A binary needing exactly the floor is ACCEPTED — the at-bound side of the bound. Without this +# case a gate could reject everything and still look green on the over-bound case alone. +at_bound="$(stub_readelf at-bound \ + ' 0000: 0x0b792650 0x00 04 GLIBC_2.31' \ + ' 4: 0000000000000000 0 FUNC GLOBAL DEFAULT UND memcpy@GLIBC_2.14 (3)')" +expect 'at the floor passes' 0 "$at_bound" 2.31 + +# One minor OVER the floor is REJECTED — this is the regression the gate exists to catch: it is +# exactly what `ubuntu-latest` moving to 24.04 did to v0.64.0 (2.39 against a 2.35 target). +over_bound="$(stub_readelf over-bound \ + ' 0000: 0x0b792650 0x00 04 GLIBC_2.32' \ + ' 4: 0000000000000000 0 FUNC GLOBAL DEFAULT UND memcpy@GLIBC_2.14 (3)')" +expect 'one minor over the floor fails' 1 "$over_bound" 2.31 + +# VERSION-ORDER trap: glibc 2.4 is OLDER than 2.31, but a LEXICAL string comparison says +# "2.4" > "2.31" and would reject this binary. Only a numeric/version-aware comparison passes, +# so this fixture is what distinguishes the correct gate from the nearest wrong one. +lexical_trap="$(stub_readelf lexical-trap \ + ' 0000: 0x0b792650 0x00 04 GLIBC_2.4' \ + ' 0010: 0x09691a75 0x00 03 GLIBC_2.17')" +expect 'a 2.4 requirement is older than a 2.31 floor' 0 "$lexical_trap" 2.31 + +# GLIBC_PRIVATE carries no version number. An implementation that swept it into the requirement +# set would take it as the maximum (it sorts above any digit) and reject a binary that is in fact +# well under the floor. The other requirement here is DELIBERATELY below the floor so the expected +# result is PASS: expecting a failure here would be satisfied by the swallowing implementation too. +private_tag="$(stub_readelf private-tag \ + ' 0000: 0x09691f71 0x00 04 GLIBC_PRIVATE' \ + ' 0010: 0x09691a75 0x00 03 GLIBC_2.17')" +expect 'GLIBC_PRIVATE is not mistaken for a requirement' 0 "$private_tag" 2.31 + +# A binary with NO glibc requirement at all (a static/musl build) is accepted rather than +# crashing on an empty maximum. +no_glibc="$(stub_readelf no-glibc ' There is no dynamic section in this file.')" +expect 'no glibc requirement passes' 0 "$no_glibc" 2.31 + +# A missing floor argument is a MISUSE, not a pass — a gate invoked wrongly must never be +# silently green, which is how a CI gate rots into a no-op. +expect 'a missing floor is a usage error' 2 "$at_bound" '' + +if [ "$failures" -ne 0 ]; then + printf '\n%s test(s) failed\n' "$failures" + exit 1 +fi +printf '\nall check-glibc-floor tests passed\n' From fe976b9f326fe0cd7fc430ec1a4f2a5ac475a4e3 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 28 Jul 2026 20:22:25 -0700 Subject: [PATCH 3/7] fix(peer): cap per-peer pool addresses and reject wildcard contacts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three PR#118 gate follow-ups in the connected-pool address path. #1782 — a supersede republishes PoolEvent::PeerAdded and fires NO PeerRemoved, so the per-peer address list grew by one entry per distinct SocketAddr ever seen (measured: 5000 events, 5000 entries). Capped at 8, dig-dht's own MAX_ADDRESSES_PER_RECORD. Three downstream guards each stop a remote peer driving this today, so it is defence-in-depth: the cap is what keeps a future relaxation of any one of them from becoming a remote memory sink. Eviction is family-aware rather than a blind truncate, because the tail is where the other address family ends up and the downstream dial ladder is both IPv6-first and very short: a lone IPv4 fallback must survive IPv6 churn (§5.2 prefers IPv6, it does not exclude IPv4). #1784 — dig-nat reports [::]:0 as the remote of an accepted relayed circuit when no relay endpoint is configured. That wildcard flowed into the pool as the peer's fetchRange target. It is now rejected at the event, not filtered on read, so a peer that already has a working address cannot have it displaced from the front of the dial order by an address nothing can be reached at. The root cause is in dig-nat and is reported there; this is the node's own guard. #1783 — the PEX dial loop checked is_pool_peer, then spawned a dial that can run for CANDIDATE_DIAL_TIMEOUT before adopting. Since dig-gossip 0.17.12 a second connection for an identity SUPERSEDES the live slot and tears its session down, killing any in-flight range stream; before 0.17.12 it merely returned DuplicateConnection, which is why ignoring the race used to be safe. Membership is now re-checked after the dial resolves, at a single named adoption gate. Refs #1782, #1783, #1784 Co-Authored-By: Claude --- crates/dig-node-core/src/download.rs | 187 ++++++++++++++++++ crates/dig-node-core/src/peer.rs | 8 +- .../dig-node-core/src/seams/dig_peer/net.rs | 55 ++++++ .../dig-node-core/src/seams/dig_peer/pex.rs | 82 ++++++++ 4 files changed, 328 insertions(+), 4 deletions(-) diff --git a/crates/dig-node-core/src/download.rs b/crates/dig-node-core/src/download.rs index 064bbd2..d228563 100644 --- a/crates/dig-node-core/src/download.rs +++ b/crates/dig-node-core/src/download.rs @@ -348,6 +348,44 @@ pub(crate) fn pool_removal_reason(reason: GossipRemovalReason) -> PoolRemovalRea } } +/// The most addresses the connected pool keeps for ONE `peer_id`. +/// +/// WHY 8 (dig_ecosystem#1782): it is `dig-dht`'s own `MAX_ADDRESSES_PER_RECORD`, the limit every +/// address list this pool feeds is eventually cut to. Matching it means the pool never holds an +/// address that could not be published anyway. +/// +/// WHY a cap at all: `PeerAdded` is republished each time a fresh verified session supersedes a +/// stale slot for the same identity, and a supersede fires NO `PeerRemoved` — so without a cap the +/// list grows by one entry per distinct `SocketAddr` ever seen for that peer, forever (measured: +/// 5000 `PeerAdded` events → 5000 entries). Three independent guards downstream stop a remote peer +/// from driving that today, which makes this defence-in-depth rather than a live leak; the point of +/// the cap is that relaxing any ONE of those guards later must not turn this into a remote memory +/// sink. +pub(crate) const MAX_POOL_ADDRS_PER_PEER: usize = 8; + +/// Cut `addrs` (newest-first) down to at most `limit` entries, evicting the OLDEST address of +/// whichever address family currently has the most entries. +/// +/// WHY not a plain `truncate` (dig_ecosystem#1782, secondary): the tail is where the other address +/// FAMILY ends up. A peer reached over IPv6 that then becomes reachable over IPv4 accumulates fresh +/// IPv6 sessions; a blind truncate drops the lone IPv4 address off the end, and since the downstream +/// dial ladder is both IPv6-first and very short, losing it can make an otherwise-reachable peer +/// unreachable. Evicting from the larger family keeps at least one address of each family for as +/// long as the cap allows, which is what the IPv6-first/IPv4-FALLBACK rule (§5.2) actually requires: +/// IPv6 is preferred, not exclusive. +fn retain_newest_per_family(addrs: &mut Vec, limit: usize) { + while addrs.len() > limit { + let ipv6_count = addrs.iter().filter(|a| a.is_ipv6()).count(); + let evict_ipv6 = ipv6_count * 2 >= addrs.len(); + // Newest-first ordering means the LAST entry of a family is its oldest. + let victim = addrs + .iter() + .rposition(|a| a.is_ipv6() == evict_ipv6) + .expect("the majority family has at least one member"); + addrs.remove(victim); + } +} + /// The kind of a pool churn event, extracted from `dig_gossip::PoolEvent` at the call site so this /// module does not depend on dig-gossip's concrete type. The caller (`crate::peer`) destructures the /// gossip event into this + the raw 32-byte peer id, keeping the 1:1 map explicit and testable here. @@ -828,15 +866,29 @@ impl NodeContent { .unwrap_or_else(|poisoned| poisoned.into_inner()); match event { PoolEvent::PeerAdded { peer_id, addr } => { + // A wildcard / port-0 address is not a destination (#1784): dig-nat reports `[::]:0` + // as the remote of an accepted relayed circuit when no relay endpoint is configured. + // SKIP the event outright rather than record-then-filter, so a peer that already has + // a working address does not have it displaced from the front of the dial order by + // an address nothing can be reached at. + if !crate::seams::dig_peer::net::is_usable_contact(addr) { + return; + } // The NEWEST session's address leads the candidate's dial order, older ones trailing // as fallbacks (#1771). dig-gossip republishes `PeerAdded` when a fresh verified // session supersedes a stale slot for the same identity (#1691/#1703/#1762), which is // typically a MOVE — a dead relay circuit replaced by a direct dial. Appending would // leave the dead address first and spend a failed dial on it for every later fetch, // and dropping the older ones would discard a still-working fallback. + // + // Newest-first holds within the POOL's list; the eventual dial order additionally + // prefers IPv6 over IPv4 (dig-download's `dial_candidates` sorts by family first, + // per the ecosystem IPv6-first rule §5.2), so a freshly-adopted IPv4 address leads + // only the IPv4 group, not the whole ladder (#1785d). let addrs = pool.entry(peer_id.to_hex()).or_default(); addrs.retain(|known| known != addr); addrs.insert(0, *addr); + retain_newest_per_family(addrs, MAX_POOL_ADDRS_PER_PEER); } PoolEvent::PeerRemoved { peer_id, .. } => { pool.remove(&peer_id.to_hex()); @@ -1518,6 +1570,141 @@ pub(crate) mod tests { ContentId::resource([1; 32], root_bytes, [3; 32]) } + // -- connected-pool address bookkeeping (#1782 cap, #1784 wildcard guard) -------------------- + + /// A `NodeContent` with no real transport — enough to drive `on_pool_event` and read the pool + /// back. The download machinery is never exercised, so the mocks can be trivial. + fn pool_only_content(dir: &std::path::Path) -> Arc { + NodeContent::new( + Arc::new(MockProviderLocator::fixed(vec![])), + Arc::new(MockRangeTransport::new(MockContent::even(4, 1))), + MissMode::FetchThrough, + None, + dir, + ) + } + + /// Feed one `PeerAdded` for `peer` at `addr` through the real event path. + fn feed_added(content: &NodeContent, peer: [u8; 32], addr: &str) { + content.on_pool_event(&pool_event_to_selector( + peer, + PoolEventKind::Added { + addr: addr.parse().expect("test address parses"), + }, + )); + } + + /// The addresses the pool currently holds for `peer`, newest first. + fn pool_addrs(content: &NodeContent, peer: [u8; 32]) -> Vec { + let pool = content.connected_pool(); + let guard = pool.lock().unwrap(); + guard.get(&hex::encode(peer)).cloned().unwrap_or_default() + } + + /// #1782: a supersede republishes `PeerAdded` and fires NO `PeerRemoved`, so the address list + /// must be capped rather than growing once per distinct `SocketAddr` ever seen. 5000 is the + /// figure the security gate measured growing unbounded — well past `MAX_ADDRESSES_PER_RECORD`, + /// so the cap cannot pass by accident of a small fixture. + #[test] + fn a_peer_that_supersedes_forever_never_exceeds_the_address_cap() { + let td = tempfile::tempdir().unwrap(); + let content = pool_only_content(td.path()); + let peer = [0xA1; 32]; + + for port in 10_000..15_000u16 { + feed_added(&content, peer, &format!("203.0.113.7:{port}")); + } + + let addrs = pool_addrs(&content, peer); + assert_eq!( + addrs.len(), + MAX_POOL_ADDRS_PER_PEER, + "5000 superseding sessions must leave at most the cap, not 5000 entries" + ); + assert_eq!( + addrs[0].port(), + 14_999, + "the newest session still leads the pool's dial order" + ); + } + + /// #1782 secondary: the ONE IPv4 address of a mostly-IPv6 peer must survive continued IPv6 + /// churn. A plain `truncate` keeps the newest 8 — all IPv6 here — and silently drops the only + /// address of the fallback family, which the very short IPv6-first dial ladder downstream then + /// cannot recover. The IPv4 address is adopted BEFORE the churn precisely so a + /// newest-first-wins implementation cannot pass this by keeping it for recency. + #[test] + fn ipv6_churn_cannot_evict_a_peers_only_ipv4_address() { + let td = tempfile::tempdir().unwrap(); + let content = pool_only_content(td.path()); + let peer = [0xB2; 32]; + + feed_added(&content, peer, "203.0.113.7:9444"); + for n in 0..64 { + feed_added(&content, peer, &format!("[2001:db8::{n:x}]:9444")); + } + + let addrs = pool_addrs(&content, peer); + assert_eq!(addrs.len(), MAX_POOL_ADDRS_PER_PEER); + assert!( + addrs.iter().any(|a| a.is_ipv4()), + "the IPv4 fallback must survive IPv6 churn; got {addrs:?}" + ); + assert!( + addrs.iter().filter(|a| a.is_ipv6()).count() >= MAX_POOL_ADDRS_PER_PEER - 1, + "IPv6 still fills the rest of the list — IPv6 is PREFERRED, not evicted (§5.2)" + ); + } + + /// #1784: `[::]:0` — dig-nat's remote address for an accepted relayed circuit with no configured + /// relay endpoint — must never become a peer's contact. + /// + /// The peer is given a WORKING address first and the assertion is that the working address is + /// still there, still FIRST. That is what makes this test see the difference between skipping + /// the event and recording-then-filtering: a filter applied when the pool is READ would leave + /// the wildcard sitting at the head of the stored list, displacing the reachable address, and + /// would still satisfy a bare "the wildcard is not returned" assertion. + #[test] + fn a_wildcard_relay_address_never_displaces_a_peers_real_address() { + let td = tempfile::tempdir().unwrap(); + let content = pool_only_content(td.path()); + let peer = [0xC3; 32]; + + feed_added(&content, peer, "203.0.113.7:9444"); + feed_added(&content, peer, "[::]:0"); + + let addrs = pool_addrs(&content, peer); + assert_eq!( + addrs, + vec!["203.0.113.7:9444".parse::().unwrap()], + "the wildcard is dropped at the event, leaving the real address untouched and leading" + ); + } + + /// #1784, control: a peer whose ONLY reported address is the wildcard is absent from the pool + /// entirely — it must not appear as a fetch candidate with an unreachable target. Paired with + /// the test above so "absent" cannot be achieved by dropping every peer. + #[test] + fn a_peer_known_only_by_a_wildcard_address_is_not_a_pool_candidate() { + let td = tempfile::tempdir().unwrap(); + let content = pool_only_content(td.path()); + let wildcard_only = [0xD4; 32]; + let reachable = [0xE5; 32]; + + feed_added(&content, wildcard_only, "[::]:0"); + feed_added(&content, reachable, "203.0.113.9:9444"); + + assert!( + pool_addrs(&content, wildcard_only).is_empty(), + "an unreachable-only peer is not a candidate" + ); + assert_eq!( + pool_addrs(&content, reachable).len(), + 1, + "a reachable peer in the same pool is unaffected" + ); + } + // -- miss-mode resolution -------------------------------------------------------------------- #[test] diff --git a/crates/dig-node-core/src/peer.rs b/crates/dig-node-core/src/peer.rs index 815dd37..67c46f2 100644 --- a/crates/dig-node-core/src/peer.rs +++ b/crates/dig-node-core/src/peer.rs @@ -2738,7 +2738,7 @@ fn build_server_tls_config(node: &dig_nat::NodeCert) -> Result bool { + pub(crate) async fn host_has_ipv6_loopback() -> bool { tokio::net::TcpListener::bind("[::1]:0").await.is_ok() } @@ -3320,7 +3320,7 @@ mod tests { /// Build a real, freshly-started `GossipHandle` on the production-shaped dual-stack unspecified /// bind (`[::]:0`, §5.2) for the pool-handle tests. - async fn fresh_pool_handle(tag: &str, network: [u8; 32]) -> dig_gossip::GossipHandle { + pub(crate) async fn fresh_pool_handle(tag: &str, network: [u8; 32]) -> dig_gossip::GossipHandle { fresh_pool_handle_on(tag, network, crate::net::dual_stack_listen_addr(0)).await } @@ -3331,7 +3331,7 @@ mod tests { /// inbound loopback connections into the pool (the native-tls dual-stack accept quirk — the same /// family of `[::]`-v6only issue tracked for the extension-offline path), whereas a concrete /// loopback bind does, on every platform. Production still binds dual-stack `[::]` (`run_peer_network`). - async fn fresh_pool_handle_on( + pub(crate) async fn fresh_pool_handle_on( tag: &str, network: [u8; 32], listen_addr: std::net::SocketAddr, diff --git a/crates/dig-node-core/src/seams/dig_peer/net.rs b/crates/dig-node-core/src/seams/dig_peer/net.rs index 9af0ddb..d0a9fd2 100644 --- a/crates/dig-node-core/src/seams/dig_peer/net.rs +++ b/crates/dig-node-core/src/seams/dig_peer/net.rs @@ -82,6 +82,29 @@ pub fn is_advertisable_ipv4(ip: &Ipv4Addr) -> bool { !(ip.is_loopback() || ip.is_unspecified() || ip.is_link_local() || ip.is_broadcast()) } +/// Whether an address REPORTED BY A PEER OR THE POOL is a usable contact — one this node could +/// actually dial back — and so may be recorded as that peer's `fetchRange` target or DHT contact. +/// +/// WHY this exists (dig_ecosystem#1784): `dig-nat`'s accept path records `remote_addr` for an +/// accepted RELAYED circuit, and with no configured relay endpoint that address is the unspecified +/// wildcard `[::]:0`. It then flows through `PoolEvent::PeerAdded` into the connected pool as the +/// peer's fetch target AND into the DHT routing table as its contact. A wildcard address is not a +/// destination — a fetch to it can only fail — and worse, it consumes one of the very few dial +/// slots downstream, so a peer that IS reachable by another address can be rendered unreachable. +/// The root cause belongs in dig-nat; this is the node's own guard, applied where such an address +/// would enter node state. +/// +/// Rejects exactly two things, both of which mean "not a destination": +/// - an **unspecified** IP (`::` / `0.0.0.0`) — a bind wildcard, never a peer's location; +/// - port **0** — the "any port" sentinel, which nothing listens on. +/// +/// Loopback is deliberately ACCEPTED (unlike [`is_advertisable_ipv6`] / [`is_advertisable_ipv4`], +/// which decide what to advertise to the wider network): a loopback peer is genuinely dialable, and +/// single-host multi-node runs depend on it. +pub fn is_usable_contact(addr: &SocketAddr) -> bool { + !addr.ip().is_unspecified() && addr.port() != 0 +} + /// Discover a routable local IPv6 address, if the host has one. Uses the connect-a-UDP-socket trick: /// "connecting" a UDP socket to an off-host address forces the OS to select the local address it /// would route from, WITHOUT sending any packet. Returns the local IPv6 address only when it is @@ -433,6 +456,38 @@ pub fn build_node_nat_runtime( mod tests { use super::*; + /// The #1784 guard, from both sides. The rejected cases are the exact shapes dig-nat's accept + /// path can produce for a relayed circuit with no configured relay endpoint (`[::]:0`), plus the + /// two half-broken variants — a wildcard IP with a real port, and a real IP with port 0 — + /// because a guard that only recognises the fully-degenerate pair would pass either half + /// straight into the pool. + #[test] + fn a_wildcard_or_portless_address_is_not_a_usable_contact() { + for junk in ["[::]:0", "0.0.0.0:0", "[::]:9445", "0.0.0.0:9445", "1.2.3.4:0"] { + let addr: SocketAddr = junk.parse().unwrap(); + assert!( + !is_usable_contact(&addr), + "{junk} is not a destination and must never become a peer's contact" + ); + } + } + + /// The truthful control: real addresses — including LOOPBACK, which single-host multi-node runs + /// depend on — stay usable. Without this the guard could reject everything and still look + /// correct against the rejection cases alone. + #[test] + fn a_real_address_including_loopback_is_a_usable_contact() { + for good in [ + "203.0.113.7:9445", + "[2001:db8::7]:9445", + "127.0.0.1:9445", + "[::1]:9445", + ] { + let addr: SocketAddr = good.parse().unwrap(); + assert!(is_usable_contact(&addr), "{good} is dialable"); + } + } + #[test] fn dual_stack_listen_addr_is_ipv6_unspecified() { let addr = dual_stack_listen_addr(9444); diff --git a/crates/dig-node-core/src/seams/dig_peer/pex.rs b/crates/dig-node-core/src/seams/dig_peer/pex.rs index 78be192..b33ee62 100644 --- a/crates/dig-node-core/src/seams/dig_peer/pex.rs +++ b/crates/dig-node-core/src/seams/dig_peer/pex.rs @@ -626,6 +626,22 @@ impl PexPool for GossipPexPool { .await { Ok(conn) => { + // RE-CHECK pool membership now that the dial has resolved (#1783). The + // `is_pool_peer` check above happened at DECISION time; this dial ran for up + // to CANDIDATE_DIAL_TIMEOUT afterwards, and the identity may have joined the + // pool in between (an intervening PEX offer, the pool's own maintenance, or + // the peer dialing us). Adopting then is not harmless: since dig-gossip + // 0.17.12 a second connection for an identity SUPERSEDES the existing slot + // and tears down the displaced session, which kills any `fetchRange` stream + // in flight over it. Earlier versions merely returned `DuplicateConnection`, + // which is why this was previously safe to ignore. + if !should_adopt_dialed_peer(&handle, &peer_id) { + tracing::debug!( + peer = %peer_id, + "pex candidate joined the pool while the dial was in flight; dropping the redundant connection rather than superseding the live session" + ); + return; + } // Adoption dedups + caps; a duplicate/full/banned result is fine (already known). let _ = handle.adopt_nat_connection(conn).await; } @@ -652,6 +668,21 @@ impl PexPool for GossipPexPool { } } +/// Whether a PEX candidate whose dial has just SUCCEEDED should still be adopted into the pool. +/// +/// The answer is "only if the identity is still absent from the pool". The membership check that +/// authorised the dial happened before it, and a NAT dial can run for `CANDIDATE_DIAL_TIMEOUT`; if +/// the identity joined the pool in the meantime, adopting a second connection for it SUPERSEDES the +/// live slot and tears the existing session down (dig-gossip 0.17.12 onward), taking any in-flight +/// range stream with it. Dropping the redundant connection instead costs nothing — the peer is +/// already connected, which is the outcome the dial was for. +/// +/// Split out from the spawned dial task so the decision is unit-testable against a REAL pool, and so +/// there is exactly ONE place in this module that decides whether an adoption may proceed. +fn should_adopt_dialed_peer(handle: &dig_gossip::GossipHandle, peer_id: &dig_gossip::PeerId) -> bool { + !handle.is_pool_peer(peer_id) +} + /// Parse a 64-hex `peer_id` into a dig-gossip [`PeerId`](dig_gossip::PeerId) (`Bytes32`), or `None` if /// malformed. Pure so it is unit-tested without a handle. fn parse_gossip_peer_id(peer_id_hex: &str) -> Option { @@ -673,6 +704,57 @@ mod tests { format!("{b:02x}").repeat(32) } + /// #1783: once a PEX dial has resolved, an identity that is ALREADY in the pool must not be + /// adopted — adopting supersedes the live slot and tears its session (and any in-flight range + /// stream) down. + /// + /// Driven against a REAL pool holding a REAL connected peer rather than a stubbed membership + /// answer: node A dials node B over the IPv6 loopback, so A's pool genuinely contains B (the + /// DIALER's half registers synchronously on every platform, unlike the inbound half). An + /// unconnected identity in the SAME pool is the control — without it the test would also pass + /// against an implementation that refused every adoption. + #[tokio::test] + async fn a_candidate_already_in_the_pool_is_not_adopted_after_its_dial_resolves() { + if !crate::peer::tests::host_has_ipv6_loopback().await { + eprintln!("skipping: host has no usable IPv6 loopback stack"); + return; + } + // The gossip mTLS stack needs a process-global rustls provider; install it so this test is + // order-independent (production installs it during node bring-up). + let _ = rustls::crypto::ring::default_provider().install_default(); + + let network = [0x7bu8; 32]; + let node_a = crate::peer::tests::fresh_pool_handle("pex-adopt-a", network).await; + let node_b = crate::peer::tests::fresh_pool_handle_on( + "pex-adopt-b", + network, + "[::1]:0".parse().expect("parse [::1]:0"), + ) + .await; + let b_port = node_b + .__listen_bound_addr_for_tests() + .expect("node B bound listen addr") + .port(); + + let b_peer_hex = crate::peer::connect_peer(&node_a, &format!("[::1]:{b_port}")) + .await + .expect("node A dials node B over loopback mTLS"); + let b_peer_id = parse_gossip_peer_id(&b_peer_hex).expect("B's peer id is 64-hex"); + + assert!( + !should_adopt_dialed_peer(&node_a, &b_peer_id), + "B is already in A's pool — adopting a second connection would supersede the live session" + ); + + // Control: an identity A has never connected to is still adoptable, so the guard is + // membership-driven and not a blanket refusal. + let stranger = dig_gossip::PeerId::from([0x33u8; 32]); + assert!( + should_adopt_dialed_peer(&node_a, &stranger), + "an unconnected identity's verified dial is exactly what adoption is for" + ); + } + /// A capturing [`PexPool`] that records what PEX handed it, so the adapter is tested without a live /// pool or any network. #[derive(Default)] From d86f12f68ed88b575b1a324669c4b18591cf742c Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 28 Jul 2026 20:32:20 -0700 Subject: [PATCH 4/7] docs(spec): pin the Linux platform floor and the pool-churn idempotency contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SPEC 11.3 now specifies the five-asset matrix including linux-arm64, the arm64 .deb, and the glibc 2.31 floor as a HARD RULE over the whole Linux delivery surface, with the three mechanisms that enforce it — container assertion, per-binary gate, and executing the binaries on the oldest supported distros. Raising the floor is named as a coordinated act. dig-node-core SPEC 7.8 gains the contract the code always depended on but the spec never stated: PoolEvent::PeerAdded is REPUBLISHED for an already-added peer_id with no intervening PeerRemoved, so consumers must be keyed-idempotent and must bound what they accumulate per identity. Also states that a churn address is a hint that must be rejected when it is not a destination, and that newest-leads ordering holds within the pool's list only — the dial ladder is IPv6-first, so one family's churn must not evict a peer's only address of the other. The #1640 framing ceiling is marked RESOLVED where it was still documented as an open KNOWN CONSTRAINT: dig-nat 0.13 capped the sender and 0.14 validates at the receiver, both fail-closed, and this workspace is on that line. dig-node-core moves to 0.24.0 to stop understating its public surface, which now exports dig-nat 0.14 / dig-dht 0.8 types; the reason is recorded in the manifest. Refs #1785, #1736 Co-Authored-By: Claude --- Cargo.toml | 2 +- DEVELOPMENT_LOG.md | 9 ++ SPEC.md | 32 +++++-- crates/dig-node-core/Cargo.toml | 8 +- crates/dig-node-core/SPEC.md | 19 ++++ crates/dig-node-core/src/peer.rs | 90 ++++++++++++++++--- .../dig-node-core/src/seams/dig_peer/net.rs | 8 +- .../dig-node-core/src/seams/dig_peer/pex.rs | 5 +- 8 files changed, 151 insertions(+), 22 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3756a40..2367846 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.66.0" +version = "0.67.0" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over diff --git a/DEVELOPMENT_LOG.md b/DEVELOPMENT_LOG.md index 964c745..3cb8c25 100644 --- a/DEVELOPMENT_LOG.md +++ b/DEVELOPMENT_LOG.md @@ -828,6 +828,15 @@ A fail-closed guard is only good UX if it fires at the EARLIEST point the bad co ## Lane anchor — dig_ecosystem#1668 / #1640 (the range-frame ceiling) +**STATUS: RESOLVED at dig-nat, and the node is now ON the fixed line.** dig-nat 0.13 shipped the +capped/paged SENDER (fallible `RangeFrame::encode` + the paged prologue) and 0.14 the receiver-side +`ChunkLensAssembler`; both fail CLOSED, so an over-ceiling frame is a clean boundary error rather than +a silent mid-read decode failure. The 0.14 line (dig-gossip 0.17, dig-dht 0.8, dig-download 0.12, +dig-peer 0.7, dig-peer-selector 0.7) landed as the single atomic cascade the note below anticipated, +so the "this node cannot reach 0.14 yet" paragraph is HISTORY, kept for the lesson it carries about +duplicate wire crates. `RANGE_WINDOW` remains the node's per-frame SPLIT size and is no longer a +KNOWN CONSTRAINT. + **One confused quantity made every DIG read above ~48 KiB impossible, network-wide.** `RANGE_WINDOW` (3 MiB) bounds how much ONE REQUEST may ask for. `dig_nat::MAX_RANGE_FRAME_PAYLOAD` (32,768 B) bounds ONE FRAME. The serve path framed on the former against the latter — 96x over — and because `bytes` diff --git a/SPEC.md b/SPEC.md index d0e44a7..ace0879 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1856,11 +1856,33 @@ a dig-node release — the installer's pre-rename fallback targets the SEPARATE `DIG-Network/dig-companion` repo's own frozen historical releases, not this asset name — so it was pure release-noise. -11.3. **Matrix.** `windows-x64` (x86_64-pc-windows-msvc), `linux-x64` (x86_64-unknown-linux-gnu), -`macos-arm64` (aarch64-apple-darwin), `macos-x64` (x86_64-apple-darwin, cross-compiled on -macos-14). No linux-arm64 asset is published (the Linux build graph pulls `openssl-sys` via the -Chia wallet SDK; no consumer requests it — apt.dig.net skips arm64 non-fatally and the installer -rejects arm64 tokens). +11.3. **Matrix + the Linux platform floor (HARD RULE).** Five assets are published: +`windows-x64` (x86_64-pc-windows-msvc), `linux-x64` (x86_64-unknown-linux-gnu), `linux-arm64` +(aarch64-unknown-linux-gnu), `macos-arm64` (aarch64-apple-darwin), and `macos-x64` +(x86_64-apple-darwin, cross-compiled on macos-14). The apt `.deb` is published for both `amd64` and +`arm64`. + +**Every published Linux artifact — the raw binaries AND the `.deb` — MUST run on glibc 2.31 or +newer.** 2.31 is the supported floor, and it is a floor on the whole Linux delivery surface, not a +per-workflow choice. It clears Ubuntu 20.04+, Debian 11+, Amazon Linux 2023 and RHEL 9. + +A glibc-linked binary runs on its BUILD glibc and anything newer, never anything older, so the +BUILDER IMAGE alone determines this floor. Every job producing a Linux artifact therefore builds +inside a pinned old-glibc container (`debian:11`) via `.github/actions/setup-linux-build`, which is +the single place the floor is declared. It is enforced in three ways, all of which MUST hold: + +- the action asserts the container's own glibc equals the declared floor, so the image and the + number cannot drift apart; +- `scripts/check-glibc-floor.sh` asserts every produced binary's highest glibc requirement is at or + below the floor, and the release job re-runs it against an impossible floor to prove the gate can + still fail; +- `verify-linux-floor` EXECUTES each published binary in `ubuntu:22.04`, `debian:12` and + `amazonlinux:2023` containers on both architectures — a link-time claim is not proof that a binary + starts. + +Raising the floor is a DELIBERATE, coordinated act: the declared value, every calling job's +`container:` image, this section, and the published docs move together. Both Linux architectures +build NATIVELY (aarch64 on the arm64 runner), so no vendored-OpenSSL cross-compile is involved. 11.4. **Release hardening.** The release profile keeps `overflow-checks = true` (the read path does offset/length arithmetic over untrusted serialized input). diff --git a/crates/dig-node-core/Cargo.toml b/crates/dig-node-core/Cargo.toml index 0262599..3f7ef15 100644 --- a/crates/dig-node-core/Cargo.toml +++ b/crates/dig-node-core/Cargo.toml @@ -1,6 +1,12 @@ [package] name = "dig-node-core" -version = "0.23.0" +# The engine library's OWN version, independent of the released binary's (`[workspace.package].version` +# in the root manifest, which only dig-node-service tracks). It moves when this crate's PUBLIC surface +# moves: 0.24.0 acknowledges that the surface now exposes dig-nat 0.14 / dig-dht 0.8 types, a +# compatibility-relevant change even though the crate is consumed in-workspace and is not published to +# crates.io. Holding it at 0.23.0 would have overstated stability to anyone reading the manifest for +# what the exported types are (dig_ecosystem#1785c). +version = "0.24.0" edition = "2021" license = "GPL-2.0-only" description = "The canonical DIG node ENGINE library (crate `dig_node_core`): the JSON-RPC dispatch (`handle_rpc`, the same contract as rpc.dig.net), local-first content serve/fetch/redirect from LOCAL .dig store modules (via digstore_host::serve_blind), chain-anchored-root resolution, chain-watch + subscriptions + generation gap-fill, the LRU cache, and the full P2P stack. Shared UNCHANGED by both host shells: the `dig-node` OS-service binary (dig-node-service) and the DIG Browser's in-process cdylib (dig-runtime). Native Rust so the compiled-module serve path works." diff --git a/crates/dig-node-core/SPEC.md b/crates/dig-node-core/SPEC.md index c8777ca..7ba0918 100644 --- a/crates/dig-node-core/SPEC.md +++ b/crates/dig-node-core/SPEC.md @@ -862,6 +862,25 @@ node state or reads local resources MUST stay off the list. - The connected pool is owned by dig-gossip: discover → dial → maintain, with churn events (`PeerAdded`/`PeerRemoved`) that feed the selector registry (§7.6) and the PEX pool feeder (§7.7). +- **`PeerAdded` is REPUBLISHED for an already-added `peer_id`, and no `PeerRemoved` precedes it.** + When a fresh verified session supersedes a stale slot for the same identity — typically a dead relay + circuit replaced by a direct dial — the pool emits another `PeerAdded` for that `peer_id` at the new + address. Every consumer MUST therefore be KEYED-IDEMPOTENT: it MUST treat a repeat `PeerAdded` as an + UPSERT of that identity's state (adding the address, preserving learned quality) and MUST NOT assume + the first `PeerAdded` for an identity is exclusive, nor that a `PeerRemoved` will arrive between two + adds. A consumer that appends per event MUST bound what it accumulates: unbounded per-identity + growth is a memory sink driven purely by churn (the node caps its per-peer address list at + `dig-dht`'s `MAX_ADDRESSES_PER_RECORD`). +- **A churn address is a HINT, not a verified destination.** A reported address MUST be rejected before + it is recorded as a peer's contact or fetch target when it is not a destination at all — an + unspecified/wildcard IP (`::` / `0.0.0.0`) or port `0`. dig-nat reports the wildcard as the remote of + an accepted RELAYED circuit when no relay endpoint is configured, and such an address both fails + every dial and consumes a scarce dial slot that a reachable address could have used. +- **The newest session's address leads the pool's per-peer order**, older addresses trailing as + fallbacks. That ordering is WITHIN the pool's list only: the dial ladder built from it prefers IPv6 + over IPv4 first (§5.2), so a freshly-adopted IPv4 address leads the IPv4 group, not the whole ladder. + An implementation MUST NOT let one address family's churn evict a peer's only address of the other + family — IPv6 is preferred, not exclusive. - On every inbound peer/DHT RPC the responder folds the mTLS-verified caller into its routing table — the caller identity MUST come from the authenticated transport, never a wire field. - A node MAY rotate its network identity (cert → `peer_id`) to reduce long-term linkability; diff --git a/crates/dig-node-core/src/peer.rs b/crates/dig-node-core/src/peer.rs index 67c46f2..d15dc8e 100644 --- a/crates/dig-node-core/src/peer.rs +++ b/crates/dig-node-core/src/peer.rs @@ -114,18 +114,38 @@ pub fn dht_addr_from_gossip_addr(gossip: std::net::SocketAddr) -> std::net::Sock dht_addr } +/// A pool-reported gossip address turned into the DHT contact to store for that peer, or `None` when +/// the result would not be a usable contact. +/// +/// Composes the two things that must BOTH hold before an address enters the DHT routing table: +/// [`dht_addr_from_gossip_addr`]'s port shift, and [`crate::net::is_usable_contact`]'s "is this a +/// destination at all" check (dig_ecosystem#1784 — dig-nat reports `[::]:0` as the remote of an +/// accepted relayed circuit with no configured relay endpoint, and routing would otherwise store the +/// wildcard as that peer's contact, so every lookup seeded from it dead-ends). +/// +/// The check is applied to the MAPPED address, not the raw one: the port shift is what determines the +/// port actually stored, so a gossip port at or below the offset maps to port 0 — unusable — even +/// though the input looked fine. +pub(crate) fn dht_contact_from_pool_addr( + gossip: std::net::SocketAddr, +) -> Option { + let dht = dht_addr_from_gossip_addr(gossip); + crate::net::is_usable_contact(&dht).then_some(dht) +} + /// Per-window ciphertext cap for a `dig.fetchRange` frame (bytes) — the node window (3 MiB), the same /// cap the HTTP read path (`WINDOW`) uses. /// -/// KNOWN CONSTRAINT (dig_ecosystem#1640): this is chosen without reference to `dig-nat`'s wire framing -/// cap (`MAX_FRAMED_BODY`, 64 KiB) — a 3 MiB frame plus #1577's per-frame verification metadata -/// (`root`, the whole `chunk_lens` array, `total_length`, a base64 `inclusion_proof`) cannot actually -/// be base64-encoded and framed within that limit, so a real peer-to-peer window this large fails to -/// decode on arrival. The reshare leg's whole-module pull (`seams/dig_peer/module_serve.rs`) rides the -/// SAME constant and inherits the same defect for any module above a few tens of KB. The fix belongs -/// at `dig-nat` (publish the real payload ceiling; every framer targets it) — this constant is a -/// single source of truth for the per-frame split, so lowering it here (once the true ceiling is -/// known) requires touching only this line, not the streaming loops that consume it. +/// The framing ceiling this constant used to fight (dig_ecosystem#1640) is RESOLVED at `dig-nat`, +/// which is where it belonged: 0.13 made `RangeFrame::encode` FALLIBLE and payload-capped at the +/// sender and added the paged-prologue API (so #1577's per-frame verification metadata — `root`, the +/// `chunk_lens` array, `total_length`, the base64 `inclusion_proof` — is split across pages instead of +/// having to fit one frame), and 0.14 added the receiver-side `ChunkLensAssembler` validation. Both +/// fail CLOSED, so an over-ceiling frame is now a clean error at the boundary rather than a silent +/// decode failure mid-read. +/// +/// This value therefore remains the node's per-frame SPLIT size, and stays the single source of truth +/// for it: the streaming loops that consume it never need to change if it moves. pub const RANGE_WINDOW: usize = 3 * 1024 * 1024; /// Maximum concurrent accepted mTLS peer CONNECTIONS the listener will serve at once (audit #179 @@ -1831,7 +1851,11 @@ fn spawn_dht_routing_feed(dht: Arc, handle: dig_gossip::G for (peer_id, addr, _outbound) in handle.connected_pool_peers() { let mut bytes = [0u8; 32]; bytes.copy_from_slice(peer_id.as_ref()); - let dht_addr = dht_addr_from_gossip_addr(addr); + // Skip a peer whose pool address is not a destination (#1784) — routing must never seed a + // lookup from a wildcard contact. + let Some(dht_addr) = dht_contact_from_pool_addr(addr) else { + continue; + }; let dht = dht.clone(); tokio::spawn(async move { dht.add_peer(bytes, dht_addr).await; @@ -1852,8 +1876,11 @@ fn spawn_dht_routing_feed(dht: Arc, handle: dig_gossip::G dig_gossip::PoolEvent::PeerAdded { peer_id, addr } => { let mut bytes = [0u8; 32]; bytes.copy_from_slice(peer_id.as_ref()); - // Map the peer's gossip addr to its DHT addr before seeding routing (GAP 2). - dht.add_peer(bytes, dht_addr_from_gossip_addr(*addr)).await; + // Map the peer's gossip addr to its DHT addr before seeding routing (GAP 2), + // dropping an address that is not a destination (#1784). + if let Some(dht_addr) = dht_contact_from_pool_addr(*addr) { + dht.add_peer(bytes, dht_addr).await; + } } dig_gossip::PoolEvent::PeerRemoved { peer_id, .. } => { let mut bytes = [0u8; 32]; @@ -3141,6 +3168,40 @@ pub(crate) mod tests { assert_eq!(found.len(), 2, "two identities are two candidates"); } + /// #1784, DHT-routing half: the routing feed does NOT pass through the connected pool, so it + /// needs its own guard. A pool address that is not a destination yields no contact at all. + /// + /// `[::]:9446` and `203.0.113.7:9446` distinguish the two independent reasons an address can be + /// unusable — a wildcard IP, and a port that becomes 0 only AFTER the gossip→DHT shift — and the + /// latter is what proves the check runs on the MAPPED address: `203.0.113.7:1` looks perfectly + /// dialable until the offset is applied. + #[test] + fn a_wildcard_or_unshiftable_pool_address_yields_no_dht_contact() { + for junk in ["[::]:9446", "0.0.0.0:9446", "203.0.113.7:1", "[::]:0"] { + let addr: std::net::SocketAddr = junk.parse().unwrap(); + assert_eq!( + dht_contact_from_pool_addr(addr), + None, + "{junk} must not become a routing-table contact" + ); + } + } + + /// The control for the guard above: a real gossip address still yields the peer's DHT contact, + /// with the port SHIFTED — so the guard cannot be satisfied by returning the raw address, and + /// cannot be satisfied by rejecting everything. + #[test] + fn a_real_pool_address_yields_the_shifted_dht_contact() { + let gossip: std::net::SocketAddr = "203.0.113.7:9445".parse().unwrap(); + let contact = dht_contact_from_pool_addr(gossip).expect("a real address is a contact"); + assert_eq!(contact.ip(), gossip.ip()); + assert_eq!( + contact.port(), + 9445 - GOSSIP_TO_DHT_PORT_OFFSET, + "routing stores the peer's DHT/peer-RPC port, not its gossip port (#1575 GAP 2)" + ); + } + /// A fresh pool has no connected peers, so the per-peer array is empty (the honest "count only" /// state before any peer connects). Uses a real `GossipHandle` — the same type the node retains. #[tokio::test] @@ -3320,7 +3381,10 @@ pub(crate) mod tests { /// Build a real, freshly-started `GossipHandle` on the production-shaped dual-stack unspecified /// bind (`[::]:0`, §5.2) for the pool-handle tests. - pub(crate) async fn fresh_pool_handle(tag: &str, network: [u8; 32]) -> dig_gossip::GossipHandle { + pub(crate) async fn fresh_pool_handle( + tag: &str, + network: [u8; 32], + ) -> dig_gossip::GossipHandle { fresh_pool_handle_on(tag, network, crate::net::dual_stack_listen_addr(0)).await } diff --git a/crates/dig-node-core/src/seams/dig_peer/net.rs b/crates/dig-node-core/src/seams/dig_peer/net.rs index d0a9fd2..6fc1422 100644 --- a/crates/dig-node-core/src/seams/dig_peer/net.rs +++ b/crates/dig-node-core/src/seams/dig_peer/net.rs @@ -463,7 +463,13 @@ mod tests { /// straight into the pool. #[test] fn a_wildcard_or_portless_address_is_not_a_usable_contact() { - for junk in ["[::]:0", "0.0.0.0:0", "[::]:9445", "0.0.0.0:9445", "1.2.3.4:0"] { + for junk in [ + "[::]:0", + "0.0.0.0:0", + "[::]:9445", + "0.0.0.0:9445", + "1.2.3.4:0", + ] { let addr: SocketAddr = junk.parse().unwrap(); assert!( !is_usable_contact(&addr), diff --git a/crates/dig-node-core/src/seams/dig_peer/pex.rs b/crates/dig-node-core/src/seams/dig_peer/pex.rs index b33ee62..7f8af23 100644 --- a/crates/dig-node-core/src/seams/dig_peer/pex.rs +++ b/crates/dig-node-core/src/seams/dig_peer/pex.rs @@ -679,7 +679,10 @@ impl PexPool for GossipPexPool { /// /// Split out from the spawned dial task so the decision is unit-testable against a REAL pool, and so /// there is exactly ONE place in this module that decides whether an adoption may proceed. -fn should_adopt_dialed_peer(handle: &dig_gossip::GossipHandle, peer_id: &dig_gossip::PeerId) -> bool { +fn should_adopt_dialed_peer( + handle: &dig_gossip::GossipHandle, + peer_id: &dig_gossip::PeerId, +) -> bool { !handle.is_pool_peer(peer_id) } From a2f7f3443dbe1fabfaabf7dd7f8c936d05bc22ec Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 28 Jul 2026 20:36:03 -0700 Subject: [PATCH 5/7] chore: re-lock for the dig-node 0.67.0 / dig-node-core 0.24.0 versions Co-Authored-By: Claude --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dbba9bd..51e45f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2211,7 +2211,7 @@ dependencies = [ [[package]] name = "dig-node-core" -version = "0.23.0" +version = "0.24.0" dependencies = [ "async-trait", "axum", @@ -2266,7 +2266,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.66.0" +version = "0.67.0" dependencies = [ "async-trait", "axum", From 877152420ab317815561087505fdf8e5a3bb5278 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 28 Jul 2026 20:58:53 -0700 Subject: [PATCH 6/7] test(release): follow the dign asset guard into the staging action The asset-naming guard grepped build-binaries.yml for a literal staged path. Naming now lives in the stage-binaries composite action, because the Linux targets moved into a pinned old-glibc container and the resulting two build jobs must not be able to drift on the names downstream consumers resolve. The guard follows it, and is strengthened while there: it asserts the dign bin is built by BOTH build jobs (a count, so one job dropping the alias is caught), that neither job constructs an asset name itself, and that the legacy dig-companion copy is absent from both files rather than only the workflow. Co-Authored-By: Claude --- .../tests/release_workflow_dign_guard.rs | 75 ++++++++++++++----- 1 file changed, 58 insertions(+), 17 deletions(-) diff --git a/crates/dig-node-service/tests/release_workflow_dign_guard.rs b/crates/dig-node-service/tests/release_workflow_dign_guard.rs index cd03746..9952582 100644 --- a/crates/dig-node-service/tests/release_workflow_dign_guard.rs +++ b/crates/dig-node-service/tests/release_workflow_dign_guard.rs @@ -6,30 +6,65 @@ //! matcher (a separate follow-up, #548 step 3): here we assert the workflow actually //! EMITS the asset the installer will later resolve. The cross-OS build moved out of //! `release.yml` into the reusable `build-binaries.yml` (#592, so the stable + nightly -//! channels share ONE build); this guard follows it there. The workflow is embedded at -//! compile time so the check runs hermetically with no filesystem access. +//! channels share ONE build), and the ASSET NAMING then moved again, into the +//! `stage-binaries` composite action (dig_ecosystem#1736: the Linux targets had to move +//! into a pinned old-glibc container, giving two build jobs that must not be able to +//! diverge on the names they publish). This guard follows the naming to its current home. +//! Both files are embedded at compile time so the check runs hermetically. /// The reusable build workflow, embedded from the repo root (`crates/dig-node-service/tests` /// is three levels below it). const BUILD_YML: &str = include_str!("../../../.github/workflows/build-binaries.yml"); -/// The build step must compile the `dign` bin target beside `dig-node`; dropping +/// The composite action that OWNS the published asset names for every platform. +const STAGE_ACTION_YML: &str = include_str!("../../../.github/actions/stage-binaries/action.yml"); + +/// Every build job must compile the `dign` bin target beside `dig-node`; dropping /// `--bin dign` would silently stop shipping the alias. +/// +/// The COUNT is asserted, not just the presence: there are now TWO build jobs (the +/// Windows/macOS host-runner matrix and the containerized Linux matrix), and a guard that +/// accepted one occurrence would not notice one of them dropping the alias. +#[test] +fn every_build_job_builds_the_dign_bin() { + let jobs_building_dign = BUILD_YML.matches("--bin dig-node --bin dign").count(); + assert_eq!( + jobs_building_dign, 2, + "both build jobs (host-runner + containerized Linux) must \ + `cargo build … --bin dig-node --bin dign`; found {jobs_building_dign}" + ); +} + +/// The staging action must publish BOTH stems under `--` — the exact +/// shape the dig-installer resolves. #[test] -fn release_workflow_builds_the_dign_bin() { +fn the_staging_action_publishes_both_the_dig_node_and_dign_stems() { assert!( - BUILD_YML.contains("--bin dig-node --bin dign"), - "build-binaries.yml must `cargo build … --bin dig-node --bin dign`" + STAGE_ACTION_YML.contains("for stem in dig-node dign"), + "stage-binaries must stage BOTH the `dig-node` binary and the `dign` alias" + ); + assert!( + STAGE_ACTION_YML.contains(r#"cp "$src" "dist/${stem}-${VER}-${{ inputs.out-name }}""#), + "stage-binaries must publish each stem as `--`" ); } -/// The stage step must publish the alias under the `dign--` stem — the -/// exact shape the dig-installer resolves (matching `dig-node--`). +/// Naming lives in ONE place. If a build job ever stages an asset itself, the two jobs can +/// drift on the names downstream consumers resolve — precisely the failure the composite +/// action was extracted to prevent. #[test] -fn release_workflow_stages_the_dign_asset() { +fn no_build_job_stages_assets_outside_the_staging_action() { + assert!( + !BUILD_YML.contains("dist/dig-node-${VER}") && !BUILD_YML.contains("dist/dign-${VER}"), + "build-binaries.yml must delegate asset naming to ./.github/actions/stage-binaries, \ + never construct an asset name itself" + ); assert!( - BUILD_YML.contains("dist/dign-${VER}-${{ matrix.out_name }}"), - "build-binaries.yml must stage a `dign--` release asset" + BUILD_YML + .matches("./.github/actions/stage-binaries") + .count() + >= 2, + "every build job must stage through the shared action" ); } @@ -42,13 +77,19 @@ fn release_workflow_stages_the_dign_asset() { /// repo's own frozen historical releases (its own asset stem), not this asset name. /// /// So the duplicate is pure release-noise — the build must ship ONLY `dig-node-*` + `dign-*`. +/// Checked in BOTH files, since either could reintroduce the copy. #[test] fn release_workflow_no_longer_ships_the_legacy_dig_companion_asset() { // Scope to the STAGED asset path (`dist/dig-companion-…`), not any mention of the word — - // the header comment legitimately explains WHY the legacy copy was dropped. - assert!( - !BUILD_YML.contains("dist/dig-companion"), - "build-binaries.yml must NOT stage a duplicate legacy `dig-companion-*` asset \ - (#585) — ship only the canonical `dig-node-*` name + the `dign-*` alias" - ); + // the header comments legitimately explain WHY the legacy copy was dropped. + for (name, yml) in [ + ("build-binaries.yml", BUILD_YML), + ("stage-binaries/action.yml", STAGE_ACTION_YML), + ] { + assert!( + !yml.contains("dist/dig-companion"), + "{name} must NOT stage a duplicate legacy `dig-companion-*` asset \ + (#585) — ship only the canonical `dig-node-*` name + the `dign-*` alias" + ); + } } From 3f59bad3c28512c50163cc69612f301abeff7f31 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 28 Jul 2026 21:14:35 -0700 Subject: [PATCH 7/7] fix(ci): remove a SIGPIPE race from the glibc-floor container check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ldd --version | head -n 1` under `pipefail` fails intermittently with exit 141: head closes the pipe after the first line, ldd takes SIGPIPE, and the step dies. It passed on two runs before failing on a third, which is the worst shape for a gate — a false red that looks like a floor violation. The version is now sliced out of a captured string with no pipe at all, and an unparseable banner is an explicit error rather than an empty comparison that could silently match. Co-Authored-By: Claude --- .github/actions/setup-linux-build/action.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/actions/setup-linux-build/action.yml b/.github/actions/setup-linux-build/action.yml index f74c9f3..4ca379d 100644 --- a/.github/actions/setup-linux-build/action.yml +++ b/.github/actions/setup-linux-build/action.yml @@ -51,8 +51,22 @@ runs: shell: bash run: | set -eu - actual="$(ldd --version | head -n 1 | grep -oE '[0-9]+\.[0-9]+$')" + # NO PIPE. `ldd --version | head -n1` is a SIGPIPE race: `head` closes the pipe after the + # first line, `ldd` dies with SIGPIPE, and under `pipefail` the step fails with exit 141 + # — intermittently, depending on whether ldd had already finished writing. It passed twice + # before failing once. Read the output into a variable and slice it with shell expansion. + ldd_output="$(ldd --version 2>&1)" + IFS= read -r first_line <<<"$ldd_output" + # glibc's banner ends with the bare version, e.g. "ldd (Debian GLIBC 2.31-13) 2.31". + actual="${first_line##* }" declared="${{ steps.floor.outputs.version }}" + case "$actual" in + [0-9]*.[0-9]*) ;; + *) + echo "::error::could not parse a glibc version out of: $first_line" + exit 1 + ;; + esac echo "container glibc: $actual, declared floor: $declared" test "$actual" = "$declared" || { echo "::error::build container glibc $actual != the declared floor $declared. Update the floor in .github/actions/setup-linux-build/action.yml, every calling job's container: image, SPEC.md §11.3 and the docs TOGETHER."