From fb8ba568c3a1204883c09d0bbbd8f2a021371cb9 Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Thu, 13 Aug 2026 06:20:13 -0500 Subject: [PATCH 1/3] docs: correct the x25519 sibling record + add tools/check_upstream_pins.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three documentation-integrity defects in the x25519/nistcurves pin record, all verified mechanically, plus the tool that would have caught the last one. 1. CLAUDE.md claimed in four places that UCI is "the supported sibling-on path" for USE_X25519_SIBLING=1. Neither backend links. Re-measured at f0127a0 with fresh submodules: make USE_X25519_SIBLING=1 (ip65) X25519_RODATA overflows CRYPTO_OVERLAY by 2048 bytes LIB_NISTCURVES_P256_CODE overflows CRYPTO_RESIDENT by 103 bytes make BACKEND=uci USE_X25519_SIBLING=1 (UCI) <- previously undocumented LIB_NISTCURVES_P256_CODE overflows CRYPTO_HOT by 381 bytes make BACKEND=uci (control) -> links clean Root cause recorded with measured segment sizes: the sibling's CRYPTO_CODE claim is 4,207 B against the in-tree pair's 2,769 B. 2. CLAUDE.md cited `make -C libs/x25519 lib-x25519-scalarmult` as the integration wrapper's entry point. No such target exists upstream at any tag, and build_x25519.sh does not call `make -C` at all — it stages three sources, sed-rewrites .segment "CODE", and hand-emits the X25519_RODATA / X25519_BSS modules. Documented as it is, since that is what a version bump must be migrated through. 3. The nistcurves pin was recorded as v0.3.0 in the Crypto ABI section while the actual gitlink is v0.6.0 and the benchmark tables below it are v0.6.0 numbers — the file contradicted itself. Also records that the pinned x25519 v0.6.0 carries upstream c64-x25519 #64 (RFC 7748 decodeUCoordinate MSB desync, fixed in v0.7.0), and that the in-tree implementation is NOT affected because it still writes the mask back into x25_u. Verified: tools/test_x25519.py --slow 73/73 on the in-tree build, including RFC 7748 vector 2 whose u has bit 255 set. tools/check_upstream_pins.py reports pin-vs-upstream-tag drift for every submodule. Stdlib + git only, one `git ls-remote --tags` per submodule. It exists because `git submodule status` renders versions through `git describe` without --tags — annotated tags only — and c64-x25519 tagged v0.6.0 lightweight, so an exactly-on-v0.6.0 pin renders as "v0.5.0-5-g95fdd70" and reads like a stale pin. The script resolves refs/tags/^{} when present and the bare ref otherwise, and reads the gitlink from `git ls-tree` so it works on an uninitialised submodule. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 131 +++++++++++++++--- tools/check_upstream_pins.py | 253 +++++++++++++++++++++++++++++++++++ 2 files changed, 362 insertions(+), 22 deletions(-) create mode 100755 tools/check_upstream_pins.py diff --git a/CLAUDE.md b/CLAUDE.md index 51d3e7b..6d1b12b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,9 +32,9 @@ Variables: - `BACKEND=ip65|uci` — select networking backend cfg (`cfg/c64-https-$(BACKEND).cfg`; default ip65) - `USE_X25519_SIBLING=1` — swap the in-tree X25519 for the - `libs/x25519@v0.6.0` sibling (UCI only — ip65 - has a tracked code/rodata overflow; see - "Known issues") + `libs/x25519@v0.6.0` sibling. **Does not link + on either backend at present** — both + overflow, differently; see "Known issues" - `EMBED_P256_OVERLAY=1` — stage the P-256 verify image into the CRYPTO_OVERLAY slot at PRG-load (UCI; mutually exclusive with USE_X25519_SIBLING / @@ -80,7 +80,9 @@ buffers in the crypto BSS — see per-module headers for details): X25519 / field arithmetic Default: in-tree `src/crypto/{x25519,fe25519}.s`. Opt-in: sibling `libs/x25519@v0.6.0` via `make USE_X25519_SIBLING=1` - (UCI backend only — see Known issues for the ip65 fit limitation). + — **currently unbuildable on both backends**, and the pinned + v0.6.0 additionally carries an upstream correctness bug; see + Known issues before relying on either fact. The v0.6.0 pin is c64-lib-contract-aligned (SPEC §8.1) and adds the bank-2 drop + RAM-reclaim work; older v0.4.0 pin is historical only. Sibling and in-tree both expose the same ABI: @@ -95,7 +97,7 @@ buffers in the crypto BSS — see per-module headers for details): SHA-256 (in-tree; no sibling) sha256_init, sha256_update, sha256_final - ECDSA P-256 (`libs/nistcurves@v0.3.0` sibling, + ECDSA P-256 (`libs/nistcurves@v0.6.0` sibling, c64-lib-contract SPEC §1-§8.1 aligned) ecdsa_verify_256 — TLS dispatcher in src/crypto/ecdsa_verify.s packs the BE struct + calls the sibling entry @@ -519,20 +521,79 @@ Five latent bugs and three new ones were cleared to get here: "Memory layout" below). Earlier-pin caveats (Phase C.1 hang, v0.3.0 retry rollback, v0.4.0 H2 defensive REU re-inits) are all superseded by v0.6.0; the file-level history lives in the c64-x25519 - repo's CHANGELOG. The ip65-side `LIB_NISTCURVES_P256_BSS` overflow - that used to be recorded here was the *default* build's overflow - and is fixed (see the CRYPTO_COLD_SHADOW entry under "Memory - layout"). `USE_X25519_SIBLING=1` under ip65 still does NOT link, - but the failure has moved and is now a different problem than the - old BSS overflow (measured 2026-07-29 post-refit): `X25519_RODATA` - overflows `CRYPTO_OVERLAY` by 2,048 B and - `LIB_NISTCURVES_P256_CODE` overflows `CRYPTO_RESIDENT` by 103 B — - i.e. a code/rodata placement problem, not the BSS one - c64-nist-curves#54 tracks. ip65's `CRYPTO_OVERLAY` is only 4,212 B - against UCI's 7.5 KB, which is the root of it. UCI remains the - supported sibling-on path. See `tools/integration/build_x25519.sh` - for the - `make -C libs/x25519 lib-x25519-scalarmult` wrapper. + repo's release notes. The ip65-side `LIB_NISTCURVES_P256_BSS` + overflow that used to be recorded here was the *default* build's + overflow and is fixed (see the CRYPTO_COLD_SHADOW entry under + "Memory layout"). + + **`USE_X25519_SIBLING=1` links on NEITHER backend** (re-measured + 2026-08-13 at f0127a0, fresh submodules, cc65 from homebrew). The + two failures are different and independent: + + make USE_X25519_SIBLING=1 # ip65 + X25519_RODATA overflows CRYPTO_OVERLAY by 2048 bytes + LIB_NISTCURVES_P256_CODE overflows CRYPTO_RESIDENT by 103 bytes + + make BACKEND=uci USE_X25519_SIBLING=1 # UCI + LIB_NISTCURVES_P256_CODE overflows CRYPTO_HOT by 381 bytes + + make BACKEND=uci # control: links clean + + Earlier revisions of this file called UCI "the supported + sibling-on path"; that was wrong — nothing links the sibling + today, and no shipped artifact contains it. The UCI overflow is + simply the sibling's larger code claim: sibling CRYPTO_CODE is + 4,207 B (fe25519 2,711 + x25519 698 + x25519_init 798) against the + in-tree pair's 2,769 B (fe25519 2,093 + x25519 676), i.e. +1,438 B + into a CRYPTO_HOT with ~1,057 B of slack. ip65 additionally has + only a 4,212 B `CRYPTO_OVERLAY` (vs UCI's 7.5 KB) to hold + `X25519_RODATA` (2,304 B) + `X25519_BSS` (1,536 B) on top of + TLS_CODE + CRYPTO_AUX_CODE. + + **The pinned v0.6.0 also carries an upstream correctness bug.** + c64-x25519 #64 (fixed in v0.7.0): `x25519_scalarmult` returns + deterministically wrong results for a peer u-coordinate with bit + 255 set, across v0.4.0-v0.6.0. v0.4.0 stopped writing the RFC 7748 + `decodeUCoordinate` mask back into `x25_u` but left the ladder's + `z_3 = x_1 * (DA-CB)^2` site reading the unmasked buffer, so + x1 = x3 + 19 (mod p). **The in-tree implementation is NOT + affected** — `src/crypto/x25519.s` still writes the mask back + (`sta x25_u+31`), so its x_1 read sees the masked value; + `tools/test_x25519.py --slow` RFC 7748 vector 2 (whose u ends + `0x93`, bit 255 set) PASSes on the in-tree build, 73/73. + That same vector would catch the sibling bug the moment the + sibling links, so **do not flip the sibling default at the v0.6.0 + pin** — bump to >= v0.7.0 first. + + Upstream is at v0.8.0. A bump is not a one-line submodule move: + v0.8.0 renames `CODE`/`DATA` to `LIB_X25519_CODE`/`LIB_X25519_DATA` + and adds `LIB_X25519_INIT_CODE`, all three of which the consumer + cfgs must declare (`LIB_X25519_INIT_CODE` must be the last + file-emitting segment before any bss-type segment in a file-backed + area), and v0.7.0's #64 fix adds an `x25_x1` buffer that + `build_x25519.sh`'s hand-written BSS stub does not yet export. + v0.8.0 also ships an `X25519_ONCHIP_MUL` profile with no REU + surface at all (`LIB_X25519_REU_BANKS_USED = 0`, no + `reu_fetch_mul_row` export) — which would dissolve the + `USE_NISTCURVES_ONCHIP` / `USE_X25519_SIBLING` mutual exclusion at + `Makefile:90`, whose stated reason is that both archives export + that symbol. Note that v0.8.0's headline + `LIB_X25519_RESIDENT_BYTES` drop (9224 -> 8383) is **not** a + shrink: 826 B of it simply moved to the reclaimable + `LIB_X25519_COLD_BYTES`, and our wrapper stages only three of the + sibling's sources anyway, so upstream manifest deltas do not + subtract from the overflows above. + + `tools/integration/build_x25519.sh` is the integration wrapper. It + does **not** call `make -C libs/x25519` — it stages three sibling + sources, sed-rewrites `.segment "CODE"` to `CRYPTO_CODE`, and + hand-emits the `X25519_RODATA` / `X25519_BSS` data modules. (An + earlier revision of this file pointed at a + `make -C libs/x25519 lib-x25519-scalarmult` target; no such target + exists upstream at any tag. The real targets are `lib`, + `lib-verify`, `lib-x25519-1764`, and — from v0.8.0 — + `lib-x25519-onchip`.) Both the sed rewrite and the hand-written + data modules are what a version bump has to be migrated through. - **CRYPTO_OVERLAY collisions are now caught by MemoryPolicy.** All `tools/uci/*.py` test scripts derive their scratch DMA addresses from a `MemoryArbiter` backed by a c64-https-aware `MemoryPolicy` @@ -598,11 +659,13 @@ a verify-path bug — see "VICE harness gotcha" in the Known issues list. With `-reu` enabled, `tools/test_x509.py` 3c PASSes cleanly in ~60 s wall-clock under VICE warp. -Under the current `libs/nistcurves@v0.3.0` pin (post-PR #55, -c64-lib-contract-aligned) the U64E 48 MHz handshake measures **82.1 s** +Under the `libs/nistcurves@v0.3.0` pin (post-PR #55, +c64-lib-contract-aligned) the U64E 48 MHz handshake measured **82.1 s** end-to-end (verified 2026-05-20 against the local listener; the prior v0.2.0 measurement was 86.7 s, and the pre-Phase-C.4 in-tree path was -~110 s). +~110 s). The pin has since moved to **v0.6.0** — the tables below are +the current numbers; the v0.3.0 row is kept only as the REU-profile +baseline. On the **C64 Ultimate** (10.53.21.158, see "C64 Ultimate notes"), measured 2026-07-19 with the INNER=217 fence and boot-at-speed flow: @@ -1089,6 +1152,30 @@ All 7 pass as of the ca65-conversion branch (97/97 assertions). The `tools/uci/` scripts cover the UCI backend on U64E hardware (see the "UCI test scripts" subsection above). +### Upstream pin drift — `tools/check_upstream_pins.py` + +Reports, for every submodule in `.gitmodules`, which release the pin +corresponds to and what upstream has tagged since. Stdlib + `git` +only, one `git ls-remote --tags` per submodule, so it is fast and +schedulable. + + tools/check_upstream_pins.py # table + tools/check_upstream_pins.py --json # machine-readable + tools/check_upstream_pins.py --strict # exit 1 on drift + tools/check_upstream_pins.py --submodule libs/x25519 + +**Do not read pins off `git submodule status`.** It renders versions +via `git describe` *without* `--tags`, which considers annotated tags +only, so a lightweight tag is invisible to it. c64-x25519 tagged +`v0.6.0` lightweight while `v0.5.0` and `v0.7.0` are annotated — +so `git submodule status` renders the exactly-on-`v0.6.0` pin as +`v0.5.0-5-g95fdd70`, which reads as "five commits past a release" and +has already been mistaken for a stale pin. `check_upstream_pins.py` +resolves `refs/tags/^{}` when the peeled ref exists and the bare +ref otherwise, so both tag kinds behave identically. It also reads the +gitlink from `git ls-tree` rather than the working copy, so it is +correct for a submodule that was never `--init`'d. + ### VICE ip65 rig (hardware-free e2e) `tests/test_vice_https_macos.py` runs the **full HTTPS handshake + GET diff --git a/tools/check_upstream_pins.py b/tools/check_upstream_pins.py new file mode 100755 index 0000000..36071c1 --- /dev/null +++ b/tools/check_upstream_pins.py @@ -0,0 +1,253 @@ +#!/usr/bin/env python3 +"""tools/check_upstream_pins.py — report submodule pin drift against upstream tags. + +Answers one question for every submodule in ``.gitmodules``: *what release is +this pinned to, and what has upstream shipped since?* + +Dependency-free (stdlib + ``git`` only), no network libraries, no API tokens, +no GitHub CLI. One ``git ls-remote --tags`` per submodule, so a full run is a +couple of seconds. Safe to schedule. + + tools/check_upstream_pins.py # human-readable table + tools/check_upstream_pins.py --json # machine-readable + tools/check_upstream_pins.py --strict # exit 1 if any pin has drifted + tools/check_upstream_pins.py --submodule libs/x25519 + +Why this exists rather than ``git submodule status`` +---------------------------------------------------- +``git submodule status`` renders its version via ``git describe`` **without** +``--tags``, which considers *annotated* tags only. A lightweight tag is +invisible to it. c64-x25519 tagged ``v0.6.0`` lightweight while ``v0.5.0`` and +``v0.7.0`` are annotated, so ``git submodule status`` renders the exactly-on- +``v0.6.0`` pin as ``v0.5.0-5-g95fdd70`` — which reads as "five commits past a +release" and is how a correct pin came to look like a documentation bug. + +This script resolves tags through ``refs/tags/^{}`` when the peeled ref +is present and the bare ref otherwise, so lightweight and annotated tags are +treated identically. + +It also reads the pinned SHA out of the git tree (``git ls-tree HEAD ``) +rather than the working copy, so it is correct for a submodule that has never +been ``git submodule update --init``'d, and immune to a dirty local checkout. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import sys + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +# Matches the common vX.Y.Z / X.Y.Z shapes; anything else sorts as non-semver +# and is reported but never treated as "latest". +_SEMVER_RE = re.compile(r"^v?(\d+)\.(\d+)\.(\d+)$") + + +def _git(*args: str, cwd: str = REPO_ROOT) -> str: + """Run git, return stdout stripped. Raises CalledProcessError on failure.""" + return subprocess.run( + ["git", *args], + cwd=cwd, + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +def parse_gitmodules(path: str) -> list[dict]: + """Parse .gitmodules into [{name, path, url}] without a config library.""" + mods: list[dict] = [] + current: dict | None = None + try: + with open(path, encoding="utf-8") as fh: + lines = fh.readlines() + except FileNotFoundError: + return mods + + for raw in lines: + line = raw.strip() + header = re.match(r'^\[submodule\s+"(.+)"\]$', line) + if header: + current = {"name": header.group(1), "path": None, "url": None} + mods.append(current) + continue + if current is None or "=" not in line: + continue + key, _, value = line.partition("=") + key = key.strip() + if key in ("path", "url"): + current[key] = value.strip() + + return [m for m in mods if m["path"] and m["url"]] + + +def pinned_sha(sub_path: str, ref: str = "HEAD") -> str | None: + """The gitlink SHA recorded in the tree — not the working copy.""" + try: + out = _git("ls-tree", ref, "--", sub_path) + except subprocess.CalledProcessError: + return None + # Format: "160000 commit \t" + for line in out.splitlines(): + fields = line.split() + if len(fields) >= 3 and fields[1] == "commit": + return fields[2] + return None + + +def remote_tags(url: str) -> dict[str, str]: + """Map tag name -> commit SHA for every tag on the remote. + + Handles annotated and lightweight tags uniformly: ``git ls-remote --tags`` + emits ``refs/tags/`` for every tag plus ``refs/tags/^{}`` carrying the + *dereferenced commit* for annotated ones. The peeled entry wins when both + are present, so the value is always a commit SHA, never a tag-object SHA. + """ + try: + out = _git("ls-remote", "--tags", url) + except subprocess.CalledProcessError as exc: + raise RuntimeError(f"git ls-remote failed for {url}: {exc.stderr.strip()}") from exc + + bare: dict[str, str] = {} + peeled: dict[str, str] = {} + for line in out.splitlines(): + parts = line.split("\t") + if len(parts) != 2: + continue + sha, ref = parts[0].strip(), parts[1].strip() + if not ref.startswith("refs/tags/"): + continue + name = ref[len("refs/tags/"):] + if name.endswith("^{}"): + peeled[name[:-3]] = sha + else: + bare[name] = sha + + return {name: peeled.get(name, sha) for name, sha in bare.items()} + + +def semver_key(tag: str) -> tuple[int, int, int] | None: + m = _SEMVER_RE.match(tag) + return (int(m.group(1)), int(m.group(2)), int(m.group(3))) if m else None + + +def inspect(mod: dict, ref: str) -> dict: + result = { + "name": mod["name"], + "path": mod["path"], + "url": mod["url"], + "pinned_sha": None, + "pinned_tag": None, + "latest_tag": None, + "latest_sha": None, + "releases_behind": None, + "drifted": False, + "error": None, + } + + sha = pinned_sha(mod["path"], ref) + if sha is None: + result["error"] = f"no gitlink for {mod['path']} at {ref}" + return result + result["pinned_sha"] = sha + + try: + tags = remote_tags(mod["url"]) + except RuntimeError as exc: + result["error"] = str(exc) + return result + + for name, tag_sha in sorted(tags.items()): + if tag_sha == sha: + result["pinned_tag"] = name + break + + semver_tags = sorted( + ((semver_key(n), n) for n in tags if semver_key(n)), + key=lambda pair: pair[0], + ) + if semver_tags: + latest_key, latest_name = semver_tags[-1] + result["latest_tag"] = latest_name + result["latest_sha"] = tags[latest_name] + result["drifted"] = tags[latest_name] != sha + pinned_key = semver_key(result["pinned_tag"]) if result["pinned_tag"] else None + if pinned_key is not None: + result["releases_behind"] = sum(1 for k, _ in semver_tags if k > pinned_key) + elif latest_key: + result["releases_behind"] = None # untagged pin: distance undefined + + return result + + +def render(rows: list[dict]) -> str: + lines = [] + for row in rows: + lines.append(f"{row['path']}") + if row["error"]: + lines.append(f" ERROR: {row['error']}") + continue + pin = row["pinned_tag"] or "(no tag points here)" + lines.append(f" pinned {row['pinned_sha'][:12]} {pin}") + if row["latest_tag"]: + marker = " <-- DRIFT" if row["drifted"] else " (current)" + behind = row["releases_behind"] + behind_txt = f", {behind} release(s) behind" if behind else "" + lines.append( + f" upstream {row['latest_sha'][:12]} {row['latest_tag']}" + f"{marker}{behind_txt}" + ) + else: + lines.append(" upstream (no semver tags found)") + return "\n".join(lines) + + +def main(argv: list[str]) -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("--json", action="store_true", help="emit JSON instead of a table") + ap.add_argument( + "--strict", + action="store_true", + help="exit 1 when any pin is behind the latest upstream semver tag", + ) + ap.add_argument( + "--ref", + default="HEAD", + help="git ref whose pins to inspect (default: HEAD)", + ) + ap.add_argument( + "--submodule", + action="append", + metavar="PATH", + help="restrict to this submodule path (repeatable)", + ) + args = ap.parse_args(argv) + + mods = parse_gitmodules(os.path.join(REPO_ROOT, ".gitmodules")) + if args.submodule: + wanted = set(args.submodule) + mods = [m for m in mods if m["path"] in wanted] + if not mods: + print("no submodules to check", file=sys.stderr) + return 2 + + rows = [inspect(m, args.ref) for m in mods] + + if args.json: + print(json.dumps(rows, indent=2)) + else: + print(render(rows)) + + if any(r["error"] for r in rows): + return 2 + if args.strict and any(r["drifted"] for r in rows): + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) From 0080274606c08b5d3ed96fee3218f93fb7d314df Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Thu, 13 Aug 2026 06:36:05 -0500 Subject: [PATCH 2/3] test(x25519): run the RFC 7748 vectors by default; name any skipped group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tools/test_x25519.py reported "RESULTS: 71/71 passed, 0/71 failed" while skipping both RFC 7748 scalarmult vectors. Those two are the only end-to-end x25519_scalarmult coverage in the file — everything else in the 71 is field arithmetic — so the default invocation certified a clean pass without ever multiplying a scalar. Skipped groups were never added to test_groups, so they left the denominator entirely and the counters could not express the gap. Same skip-as-pass shape as audit finding F3 (credit: Lane C confirmed the mechanism independently). The gate's justification was a "~100 min each" comment. Measured under VICE warp on the in-tree ip65 build: full suite with both vectors 37.9 s full suite without them 4.8 s ~16.5 s per vector — the comment was off by a factor of ~360, and the gate was buying 33 seconds while hiding the only test that matters. So both halves: - The vectors now run BY DEFAULT. `--fast` skips them; `--slow` is kept as an accepted no-op so existing invocations don't break. - Any skipped group is recorded and named in the verdict: RESULTS: 71/71 passed, 0/71 failed -- 2 group(s) SKIPPED: \ x25519 RFC 7748 vector 1, x25519 RFC 7748 vector 2 WARNING: end-to-end x25519_scalarmult coverage did NOT run; \ this run does not certify X25519. An unqualified clean pass is no longer printable over a group that did not run. Exit code is unchanged (0 iff nothing failed): unlike F3's missing-label case, `--fast` is an explicit operator choice, so it is not an error — but it can no longer be a silent one. run_tests now returns (passed, failed, skipped_groups). tools/run_all_tests.py is the only in-tree importer; it unpacks the third value and raises if it is ever non-empty, so a future gate cannot quietly drop coverage from the aggregate verdict. That run gains the vectors too: x25519 72/72 -> 73/73, aggregate 255/255, +34.8 s for the suite. Also corrected in passing: the vectors were labelled "RFC 7748 Section 6.1" but are the §5.2 scalarmult vectors (§6.1 is the Alice/Bob DH pair), and U_2 ends 0x93 — bit 255 set — which makes vector 2 the decodeUCoordinate MSB regression test that catches upstream c64-x25519 #64, the bug live in our pinned libs/x25519 v0.6.0. Noted at the vectors so nobody drops it as redundant. Verified: default -> 73/73, both vectors PASS, 38.2 s --fast -> 71/71 + both groups named + warning --slow -> 73/73 (back-compat no-op) run_all_tests.py --skip-slow -> x25519 73/73, TOTAL 255/255 Co-Authored-By: Claude Opus 5 (1M context) --- tools/run_all_tests.py | 12 ++++++- tools/test_x25519.py | 78 ++++++++++++++++++++++++++++++++---------- 2 files changed, 70 insertions(+), 20 deletions(-) diff --git a/tools/run_all_tests.py b/tools/run_all_tests.py index cf5403f..1bb6e21 100644 --- a/tools/run_all_tests.py +++ b/tools/run_all_tests.py @@ -88,7 +88,17 @@ def run_test_suite(name, transport, labels, seed): elif name == "x25519": from test_x25519 import run_tests as x25519_run - passed, failed = x25519_run(transport, labels, seed=seed) + # run_tests also reports groups it skipped. This caller never + # sets test_x25519.FAST, so the RFC 7748 scalarmult vectors + # always run here (+~33 s) and the list is empty; assert it + # rather than dropping it, so a future gate cannot silently + # remove coverage from the aggregate verdict. + passed, failed, x25519_skipped = x25519_run( + transport, labels, seed=seed) + if x25519_skipped: + raise AssertionError( + "x25519 suite skipped groups in the aggregate run: " + + ", ".join(x25519_skipped)) except Exception as e: import traceback diff --git a/tools/test_x25519.py b/tools/test_x25519.py index fd41de7..d7b3ac3 100644 --- a/tools/test_x25519.py +++ b/tools/test_x25519.py @@ -2,14 +2,29 @@ """test_x25519.py -- fe25519 field arithmetic and X25519 key exchange tests. Tests fe_add, fe_sub, fe_mul, fe_sqr, fe_inv, fe_cswap, fe_mul_a24, -fe_copy, fe_zero, fe_one, x25519_clamp, and (with --slow) x25519_scalarmult -against Python reference implementations and RFC 7748 test vectors. +fe_copy, fe_zero, fe_one, x25519_clamp, and x25519_scalarmult against +Python reference implementations and RFC 7748 test vectors. + +The two RFC 7748 scalarmult vectors run BY DEFAULT. They are the only +end-to-end `x25519_scalarmult` coverage in this file -- everything else +is field arithmetic -- so a run that omits them certifies nothing about +X25519 itself. They used to be gated behind `--slow` on the strength of +a "~100 min each" comment; measured under VICE warp on the in-tree ip65 +build they cost **~16.5 s each** (full suite 37.9 s with them, 4.8 s +without). The gate was buying 33 seconds and hiding the only test that +matters. `--fast` still skips them, and any skip is now named in the +summary line rather than silently leaving the denominator. Uses the binary monitor test harness -- jsr() is event-based via checkpoints, so no polling or retry wrappers are needed. Usage: - python3 tools/test_x25519.py [--seed S] [--verbose] [--slow] + python3 tools/test_x25519.py [--seed S] [--verbose] [--fast] + + --fast skip the RFC 7748 scalarmult vectors (~33 s). The summary + line then reports them as SKIPPED. + --slow accepted and ignored; the vectors it used to enable are + now the default. """ import os @@ -29,7 +44,7 @@ LABELS_PATH = os.path.join(PROJECT_ROOT, "build", "labels.txt") VERBOSE = False -SLOW = False +FAST = False # p = 2^255 - 19 P = (1 << 255) - 19 @@ -77,7 +92,13 @@ def clamp_ref(scalar): return bytes(s) -# RFC 7748 Section 6.1 test vectors +# RFC 7748 Section 5.2 test vectors (the scalarmult vectors -- Section 6.1 +# is the Alice/Bob Diffie-Hellman pair, which these are not). +# +# U_2 ends 0x93, i.e. bit 255 of the u-coordinate is SET. That makes +# vector 2 the RFC 7748 decodeUCoordinate MSB-masking regression test: +# it is the vector that caught upstream c64-x25519 #64, the bug present +# in our pinned libs/x25519 v0.6.0. Do not drop it as "redundant". SCALAR_1 = bytes.fromhex( "a546e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449ac4") U_1 = bytes.fromhex( @@ -634,16 +655,21 @@ def run_tests(transport, labels, seed): lambda: test_x25519_clamp(transport, labels, rng)), ] - if SLOW: - test_groups += [ - ("x25519 RFC 7748 vector 1", - lambda: test_x25519_rfc7748_vector1(transport, labels)), - ("x25519 RFC 7748 vector 2", - lambda: test_x25519_rfc7748_vector2(transport, labels)), - ] + skipped_groups = [] + scalarmult_groups = [ + ("x25519 RFC 7748 vector 1", + lambda: test_x25519_rfc7748_vector1(transport, labels)), + ("x25519 RFC 7748 vector 2", + lambda: test_x25519_rfc7748_vector2(transport, labels)), + ] + if FAST: + # A skipped group must not silently leave the denominator: record + # it so the verdict can name it. These two are the only end-to-end + # x25519_scalarmult coverage in the file. + skipped_groups += [name for name, _ in scalarmult_groups] + print("\n (--fast: skipping x25519 scalarmult vectors, ~33 s)") else: - print("\n (x25519 scalarmult tests skipped -- " - "use --slow to enable, ~100 min each)") + test_groups += scalarmult_groups for name, test_fn in test_groups: print(f"\n--- {name} ---") @@ -659,11 +685,11 @@ def run_tests(transport, labels, seed): import traceback traceback.print_exc() - return total_passed, total_failed + return total_passed, total_failed, skipped_groups def main(): - global VERBOSE, SLOW + global VERBOSE, FAST os.chdir(PROJECT_ROOT) seed = random.randint(0, 2**32 - 1) @@ -676,8 +702,12 @@ def main(): elif args[i] == "--verbose": VERBOSE = True i += 1 + elif args[i] == "--fast": + FAST = True + i += 1 elif args[i] == "--slow": - SLOW = True + # Back-compat no-op: the vectors --slow used to enable now + # run by default. Kept so existing invocations don't break. i += 1 else: i += 1 @@ -747,13 +777,23 @@ def main(): # after jsr() returns (prevents crash when BASIC ROM is banked out) write_bytes(transport, 0x0339, bytes([0x4C, 0x39, 0x03])) - passed, failed = run_tests(transport, labels, seed) + passed, failed, skipped_groups = run_tests(transport, labels, seed) mgr.release(inst) total = passed + failed print(f"\n{'='*60}") - print(f"RESULTS: {passed}/{total} passed, {failed}/{total} failed") + summary = f"RESULTS: {passed}/{total} passed, {failed}/{total} failed" + if skipped_groups: + # Never print an unqualified clean pass over a group that did not + # run. Skipped assertions leave the denominator entirely, so the + # counters alone cannot express the gap -- name it explicitly. + summary += (f" -- {len(skipped_groups)} group(s) SKIPPED: " + + ", ".join(skipped_groups)) + print(summary) + if skipped_groups: + print("WARNING: end-to-end x25519_scalarmult coverage did NOT run; " + "this run does not certify X25519.") print(f"{'='*60}") sys.exit(0 if failed == 0 else 1) From 54498ccc8accfdf3c3c8dc55a7fe1cf76a7290a2 Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Thu, 13 Aug 2026 06:49:45 -0500 Subject: [PATCH 3/3] docs(tools): make check_upstream_pins' own cost claim executable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-application of this branch's own finding. The script's docstring said a full run is "a couple of seconds" — true, but I had never measured it, which is the exact shape of the `~100 min each` comment in test_x25519.py that this branch just deleted: a cost assertion sitting in prose where no run can contradict it, quietly setting policy. Measured instead: 2.1 s wall-clock, 0.15 s CPU, 3 submodules, warm DNS. Network-bound, scales with submodule count rather than repo size. Also verified rather than assumed, by stubbing subprocess.run: exactly 3 `git ls-remote --tags` and 3 `git ls-tree HEAD` for 3 submodules, so the "one per submodule" claim is accurate. Imports are stdlib-only (__future__, argparse, json, os, re, subprocess, sys). All three claims held; none needed correcting. The change is that they are now specific, dated, and paired with the command that falsifies them — a vague-but-true cost note is one refactor away from a false one. Docstring only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- tools/check_upstream_pins.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tools/check_upstream_pins.py b/tools/check_upstream_pins.py index 36071c1..fb11cfc 100755 --- a/tools/check_upstream_pins.py +++ b/tools/check_upstream_pins.py @@ -5,8 +5,16 @@ this pinned to, and what has upstream shipped since?* Dependency-free (stdlib + ``git`` only), no network libraries, no API tokens, -no GitHub CLI. One ``git ls-remote --tags`` per submodule, so a full run is a -couple of seconds. Safe to schedule. +no GitHub CLI. Exactly one ``git ls-remote --tags`` and one ``git ls-tree`` per +submodule. Safe to schedule. + +Cost, measured rather than asserted (2026-08-13, 3 submodules, warm DNS): +**2.1 s wall-clock, 0.15 s of it CPU** — i.e. network-bound, and it scales +with submodule count, not repo size. Re-check with ``time +tools/check_upstream_pins.py``; the git-call count is verifiable by stubbing +``subprocess.run``. Stated concretely on purpose: this whole script exists +because of a claim nobody could execute, and a vague-but-true cost note is +one refactor away from a false one. tools/check_upstream_pins.py # human-readable table tools/check_upstream_pins.py --json # machine-readable