Release: develop -> main - #179
Merged
Merged
Conversation
… sweep (2026-06-02) (#177) * bench(results): Apple M5 Max vs Apple M3 Ultra (probe_r2 + HTTP mint sweep) First Apple M5 Max measurement of the Plonky2 prover hot path, captured against the same git_sha era as the Apple M3 Ultra baseline persisted on 2026-05-31 in r2_probe_runs (host_id 1). Headline numbers (probe_r2, synthetic): circuit_build: 14214 -> 8245 ms (-42%) prove_cold: 7012 -> 6129 ms (-13%) prove_warm_p50: 4777 -> 4350 ms (-9%) peak_rss: 4112 -> 3938 MiB (-4%) HTTP /api/mint sweep on M5 Max (10 samples, empty state, unfunded publisher -> broadcast 503): n=10 p50=6.906s p90=7.056s min=6.611s max=7.118s All three ROADMAP-step-9 R2 budgets (warm <= 5 s, cold <= 30 s, RSS <= 64 GiB) pass on M5 Max. Persisted JSON has its hostname field scrubbed to a generic label; the raw fingerprint is kept only in the persisted DB row. * docs(bench): unified results table — proof type x hardware Restructure scripts/bench/results/README.md to lead with a single table mapping each proof phase to its wall time on each hardware target, with both synthetic (probe_r2) and live (HTTP) numbers in one place. Adds the explicit caveat that the R2 ideal budget (<= 1 s warm prove) is not reachable via per-generation Apple silicon upgrades alone — Plonky3 / circuit optimisation remains the dominant lever.
… sweep (2026-06-02) (#177) (#178) * bench(results): Apple M5 Max vs Apple M3 Ultra (probe_r2 + HTTP mint sweep) First Apple M5 Max measurement of the Plonky2 prover hot path, captured against the same git_sha era as the Apple M3 Ultra baseline persisted on 2026-05-31 in r2_probe_runs (host_id 1). Headline numbers (probe_r2, synthetic): circuit_build: 14214 -> 8245 ms (-42%) prove_cold: 7012 -> 6129 ms (-13%) prove_warm_p50: 4777 -> 4350 ms (-9%) peak_rss: 4112 -> 3938 MiB (-4%) HTTP /api/mint sweep on M5 Max (10 samples, empty state, unfunded publisher -> broadcast 503): n=10 p50=6.906s p90=7.056s min=6.611s max=7.118s All three ROADMAP-step-9 R2 budgets (warm <= 5 s, cold <= 30 s, RSS <= 64 GiB) pass on M5 Max. Persisted JSON has its hostname field scrubbed to a generic label; the raw fingerprint is kept only in the persisted DB row. * docs(bench): unified results table — proof type x hardware Restructure scripts/bench/results/README.md to lead with a single table mapping each proof phase to its wall time on each hardware target, with both synthetic (probe_r2) and live (HTTP) numbers in one place. Adds the explicit caveat that the R2 ideal budget (<= 1 s warm prove) is not reachable via per-generation Apple silicon upgrades alone — Plonky3 / circuit optimisation remains the dominant lever. Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
github-actions
Bot
added
the
ci:full
* feat(openapi): generate OpenAPI 3.x spec from handler annotations Annotate every public `/api/*` handler with `#[utoipa::path]` and derive `ToSchema` on request/response types. Build a process-wide `ApiDoc` from those annotations and serve it at `GET /openapi.json` (cached JSON) and `GET /docs` (Swagger UI pinned to swagger-ui-dist@5.32.6). Feature-gated handlers (`address-list`, `username-claim`, `lnurl`) carry their own sub-doc that merges into the main spec only when the feature compiles in, so the document describes the exact wire surface of the running binary. Refs: #155 * refactor(openapi): bundle Swagger UI, drop hardcoded URLs, fix feature combos - Remove static `servers(...)` block; spec inherits "same host" via OpenAPI default - Bundle Swagger UI assets via `utoipa-swagger-ui`; no external CDN dependency - Fix `cargo clippy` with single-feature builds (`lnurl` only, etc.) - Smoke test verifies relative asset URLs and absence of hardcoded hosts * refactor(openapi): polish handler annotations for production - Replace placeholder ellipsis `…` (U+2026) in `SendCoinRequest` PublicKey schema examples with valid 33-byte compressed-hex pubkeys (secp256k1 generator point and its double) so client generators that parse the `example` as hex do not trip over a non-hex character. - Drop redundant `#[schema(rename = "...")]` on `LnurlpResponse` — utoipa already picks up the corresponding `#[serde(rename)]` and duplicating the directive risks future drift between serde and schema. Verified the generated spec still emits `minSendable` / `maxSendable` after removal. - Replace remaining non-ASCII ellipsis in doc and source comments with ASCII `...` for consistency. Handler tag grouping is left as-is: every annotated handler already carries a `tag` (`Accounts`, `Coins`, `Inscriptions`, `Node`, `Usernames`, `LNURL`), so Swagger UI groups them all correctly instead of dropping any into the default bucket. * test(openapi): cover async HTTP handlers and Swagger asset paths The smoke test in `tests/openapi_smoke.rs` exercises the in-memory spec and HTML string paths but never enters the async handlers (`openapi_json_handler`, `docs_handler`, `swagger_asset_handler`). Coverage Gate flagged lines 195-237 of `node/src/openapi.rs` as uncovered. Add sibling-style `openapi_tests.rs` exercising: - OK + JSON content-type from `openapi_json_handler` - OK + HTML content-type from `docs_handler` - bundled CSS and JS asset serving via `swagger_asset_handler` - 404 from `swagger_asset_handler` for unknown files - `swagger_ui_config` cache identity * test(db): harden setup_pool + connect_and_migrate against shared-host load The m3-ultra CI runner (dfx01) co-resides with ~20 production containers (Vaultwarden, Grafana, Loki, dEURO, …) and occasional manual `cargo nextest` runs from operators. Under that load the testcontainers `postgres:17` "ready" log signal fires correctly, but the subsequent SQLx pool connect can stall past the 30s default `acquire_timeout` waiting on the Colima vNIC to complete the TCP handshake. The failure surfaced as a one-shot `sqlx::Error::PoolTimedOut` in `db::tests::*` whenever multiple Heavy CI runs were in-flight on the same host (PR-CIs + production + ad-hoc tests = loadavg ~75 on 28 cores). Two independent hardenings: 1. `db.rs::connect_and_migrate` — bump `acquire_timeout` from the 30s default to 60s. The healthy path connects in <500ms so this only changes behaviour when the host is starved; in that window it is the difference between a flake and a pass. Production bootstrap inherits the same bound — a 30s vs 60s acquire timeout on a node that has been alive for milliseconds is not user- facing latency, and a freshly-started Postgres sidecar under Docker Compose orchestration can also need >30s to finish its first checkpoint pass on a busy host. 2. `db_tests.rs::setup_pool` — wrap the container-start-and-connect sequence in a 3-attempt retry with linear backoff (500ms / 1000ms). The previous container is dropped on each retry so a hung Postgres process never poisons the next attempt. The retry only fires when the host is transiently overloaded; a healthy run still hits the first attempt and pays no overhead. The aggregate effect is that one test panicking with PoolTimedOut no longer cancels the rest of the 371-test suite — the run either re-converges on the next attempt or reports a real container-engine outage three retries deep. * fix(db): retry connect_and_migrate on transient host-load failures The previous fix to `db_tests::setup_pool` retried at the container-start + connect pair, but the 20+ test files that call `connect_and_migrate` from their own ad-hoc `setup_pool` did not inherit it. Coverage Round 2 surfaced a second transient failure mode at `state_tests.rs:48`: connect_and_migrate failed: Protocol("unexpected response from SSLRequest: 0x48 (sqlx_postgres::connection::tls:95)") `0x48 = 'H'` — the testcontainers ready-signal had fired, but the first byte SQLx saw on the wire was garbage instead of the protocol handshake. Same root cause as the earlier `PoolTimedOut`: on the shared m3-ultra host (loadavg ~75 on 28 cores when this fired) the Colima vNIC delivered the bgwriter/autovacuum log lines before the listener-side socket had finished its first message exchange. Move the retry inside `connect_and_migrate` itself so every call site — not just `db_tests::setup_pool` — benefits. Three-attempt retry with linear 500ms / 1000ms backoff, classified for the two documented transient sqlx error kinds (`PoolTimedOut`, `Protocol(... SSLRequest ...)`). Auth / migration / host-not-found errors stay non-retryable so a real misconfiguration still fails fast in <1s. Mark the retry loop + classifier `#[cfg_attr(coverage_nightly, coverage(off))]` — both are defensive against host-load conditions that the deterministic test harness cannot reproduce on demand. The healthy-path `try_connect_and_migrate` worker stays fully covered by every test that hits a Postgres testcontainer. * feat(openapi): cover /, /health, /health/ready, /health/publisher, /api/history Brings the always-on wire surface into the generated spec without any hand-written drift surface: - `GET /` (root_handler) — service identification + endpoint map - `GET /health` (health_handler) — promote from inline closure to a named handler so the liveness probe carries a `#[utoipa::path]` annotation matching the readiness / publisher probes - `GET /health/ready` (ready_handler) — DB + Esplora + prover-warm gate; both 200 and 503 responses share `ReadyResponse` so Kuma and load-balancer integrations can branch on `status` / `failures` without scraping the HTTP code - `GET /health/publisher` (publisher_health_handler) — UTXO state of the publisher wallet (deploy-dev preflight gate) - `GET /api/history` (get_history_handler) — paginated per-address history (issue #153); 422 / 500 branches reuse the documented `HistoryErrorResponse` envelope so wallet error handling stays in sync with the server contract All new handlers picked up `pub(crate)` visibility plus a `#[utoipa::path]` block; new response structs derive `ToSchema`. `openapi.rs` registers each handler in `paths(...)` and each response type in `components(schemas(...))`. The smoke suite is extended to require every new route under `spec_lists_every_always_on_route` plus `HistoryResponse`, `HistoryItem`, `HistoryErrorResponse`, and `ReadyResponse` under `spec_registers_critical_schemas` so a future regression on either contract fails CI fast. `CONTRIBUTING.md` gains a `REST API & OpenAPI` section: the exposed route table, the four-step recipe for adding a new endpoint, and the existing drift guards. The project-structure tree calls out `openapi.rs` next to `router.rs` so contributors find the spec assembly without spelunking. * refactor(openapi): reviewer-loop polish — tag, 503 schema, root endpoint map Tightens consistency surfaced by the post-rebase review: - Unify `/api/balance` + `/api/history` + `/api/address` under `tag = "Accounts"` (read endpoints keyed on an address). `/api/history` was tagged `"Coins"` in the previous commit; aligned with the pre-existing `/api/balance` annotation and updated CONTRIBUTING table + recipe accordingly. - Promote the `/health/publisher` 503 body from an ad-hoc `serde_json::json!({...})` to a typed `PublisherHealthErrorResponse` with `ToSchema`, registered under `components(schemas(...))` and bound to the 503 entry in the `responses(...)` block. The 200 branch drops the redundant `serde_json::to_value(...)` wrapper now that both arms go through `into_response()`. - Extend `RootEndpoints` to mirror every always-on route: `/api/mint`, `/api/username/resolve/{username}`, `/health/ready`, `/health/publisher`, `/openapi.json`, `/docs`. The struct's doc-comment now explicitly justifies what is omitted (feature-gated routes, admin endpoints) so the endpoint map is not silently stale. - Document the meta + admin exclusion policy in the OpenAPI section of CONTRIBUTING: `/openapi.json`, `/docs`, `/docs/{file}`, and `/api/admin/*` are intentionally outside `paths(...)` and any future admin route should follow the same rule. * fix(openapi): flatten swagger_asset_handler — drop unreachable Err arm The previous shape carried a third `Err(_) => 500` match arm to satisfy the `Result<Option<SwaggerFile>, Box<dyn Error>>` signature of `utoipa_swagger_ui::serve`. Inspection of utoipa-swagger-ui 9.0.2 (`src/lib.rs::serve`) shows the function only returns `Err` in two situations: - the bundled `swagger-initializer.js` bytes fail UTF-8 decoding — impossible because the `vendored` feature bakes a known-good UTF-8 bundle in at compile time - the oauth config formatter errors — impossible because our `swagger_ui_config()` builds a `Config` without an oauth section Both invariants are structurally enforced by our build, so the arm was dead code and surfaced as a single uncovered line under the 100% line-coverage gate. Flatten the Result with `.expect(...)` — the panic message documents the contract and a future upstream change that violates either invariant would surface within minutes via the readiness probe. Updates the two test doc-comments referencing `Ok(Some)` / `Ok(None)` to match the post-flatten shape.
…mit (#161) * feat(jobs): introduce async Job-API; remove synchronous mint/send/commit Replaces the synchronous `/api/{mint,send,commit}` surface with an admit-and-poll Job-API. Every wallet-triggered prove operation now returns 202 with a `job_id` immediately; the heavy prove + broadcast work runs in a background dispatcher loop driven by a new `jobs` table. ## Routing changes Removed (legacy synchronous): - `POST /api/send` - `POST /api/mint` - `POST /api/commit` Added (Job-API admit + poll): - `POST /api/jobs/mint` — admit a mint job - `POST /api/jobs/send` — admit a send job - `GET /api/jobs/{job_id}` — poll job state (Retry-After: 2) - `POST /api/jobs/{job_id}/commit` — attach signed commitment - `POST /api/jobs/{job_id}/cancel` — cancel still-queued jobs ## New crates / modules - `node/src/job_store.rs` — typed wrapper around the `jobs` table (CreateResult / JobKind / JobStatus enums, idempotency-key handling). - `node/src/job_dispatcher.rs` — async dispatcher loop; consumes `JobEnvelope`s from an mpsc channel, dispatches by `JobKind`. - `node/src/flow.rs` — the prove + broadcast flows extracted from the old handler bodies, now driven by the dispatcher. - `node/migrations/0014_jobs.sql` — `jobs` table + indexes + CHECK constraints mirroring the typed enums. ## Dependencies - `uuid v1` (`v4`, `serde`) — `jobs.public_id` column. - `dashmap v6` — `job_notify_map` for `commit` wake-up notifications. - `chrono v0.4` — `TIMESTAMPTZ` round-trip on the timestamp columns. - `sqlx` features: `uuid`, `chrono`. ## OpenAPI integration The Job-API admit + poll surface is fully covered by `#[utoipa::path]` annotations on every new handler; the response and request envelopes (`JobAcceptedResponse`, `JobStatusResponse`, `JobErrorResponse`, plus the kept-around `SendCoinRequest` / `MintRequest` / `CommitRequest` shapes the Job-API still consumes) carry `ToSchema` derives and are registered under `components(schemas(...))`. Surviving always-on handlers from #157 keep their annotations; the smoke test `spec_lists_every_always_on_route` now requires every new `/api/jobs/*` path. ## Coverage The PR is iterated to 100% line + function coverage under `cargo llvm-cov nextest --all-features` per the project's standing quality gate. Legacy mint/send/commit handler tests removed alongside the routes; new tests live in `router_tests.rs` covering the admit + poll + commit + cancel paths plus dispatcher-loop behaviour in `job_store_tests.rs`. ## Migration story `migrations/0014_jobs.sql` adds the new `jobs` table; existing data is untouched. Wallets must move to the admit-and-poll surface — the synchronous routes return 404 after this PR lands. ## Root endpoint map `root_handler` now advertises the Job-API routes alongside the operational endpoints (`/health/ready`, `/health/publisher`, `/openapi.json`, `/docs`, `/api/username/resolve/{username}`); the `RootEndpoints` doc-comment documents the omissions (feature-gated routes, admin endpoints, meta). * ci(coverage): expose per-function uncovered list + upload HTML artifact on gate failure When the heavy gate fails, the operator currently sees: - per-file `--show-missing-lines` text (sometimes elides files with branch-only deltas, observed on this PR's failure) - per-file JSON summary (lines/functions percent only) This leaves the actual gap obscured. Solving it required re-running the ~50 min heavy gate locally — that is the wrong incentive, especially on shared-runner builds where a re-run blocks another PR. This commit adds two diagnostics to the existing failure step: - **Per-function symbol list.** A jq filter over the JSON report emits one line per uncovered function as `file:line\tsymbol_name`. The operator sees `node/src/router.rs:1234\tmy_handler` immediately and can target the missing test from the CI log without leaving the browser. - **HTML report upload.** `cargo llvm-cov report --html` generates a Codecov-style browsable report; the new `actions/upload-artifact@v4` step uploads it under `llvm-cov-html-{run_id}-{run_attempt}` for 14 days. The operator downloads the artifact, opens `index.html`, and clicks straight to the file/line/function that's red — same workflow as a local `cargo llvm-cov --open` without paying the ~50 min reproduction cost. Both diagnostics are gated on `if: failure()` so the green path pays nothing. The ignore-regex is pulled into a single `$IGNORE` shell variable so the four invocations stay in sync — the previous duplicated literal made it easy to drift one filter relative to another. * fix(jobs): drop unreachable response_body fallback closure in admit_and_enqueue The idempotent-replay arm for `Completed` jobs called job.response_body.clone().unwrap_or_else(|| serde_json::json!({})) to fall back on an empty JSON object if `response_body` was somehow absent. `JobStore::complete` sets `response_body` on the row before flipping the status to `Completed` (the matching INSERT is non-nullable on the value side), so the closure is unreachable in practice — a `None` would mean the row was hand-edited or the schema invariant broke. The defensive empty-object fallback only existed because the wallet would otherwise receive a 500 on an event that can't happen. llvm-cov scored the closure as a separate uncovered function plus an uncovered line (the closure body), which fails the 100% line + function gate — the only delta keeping PR #161 red after the rebase. Switching to `.expect(...)` documents the invariant inline and surfaces a violation as a fast-panic instead of a silent empty body, matching how this codebase handles the other "structurally unreachable but stdlib forces a fallback" branches (see the `utoipa_swagger_ui::serve(...)` flatten in `openapi.rs`). Found by the new per-file HTML coverage artifact added in the previous commit — a 50 min heavy gate's worth of guessing replaced by reading the artifact for 30 seconds.
* feat(jobs): introduce JobNotifier broadcast channel for phase fan-out
Refactor the dispatcher's per-job coordination primitive from a bare
`Arc<Notify>` into a `JobNotifier` struct that pairs the existing
commit-wake Notify with a `tokio::sync::broadcast::Sender<JobPhaseEvent>`.
Every dispatcher status-persistence site
(set_status / set_awaiting_signature / complete / fail) now also
publishes a `JobPhaseEvent` so future SSE subscribers can observe
real-time phase transitions; the existing commit-wake path is
unchanged. The cancel handler also publishes a terminal `cancelled`
event so attached listeners see the close.
This is the plumbing layer for the SSE push channel — no new endpoint
yet, no new behaviour observable from outside the dispatcher. The
`AppState` field type widens from
`Arc<DashMap<Uuid, Arc<Notify>>>` to `JobNotifyMap` (alias for
`Arc<DashMap<Uuid, Arc<JobNotifier>>>`); the runtime resumer and the
router commit handler are adjusted to the new shape. Test fixtures
construct `JobNotifier::new()` instead of `Notify::new()`.
* feat(jobs): add SSE push channel GET /api/jobs/:id/stream
Server-Sent Events endpoint that streams real-time phase transitions
to wallets without the ~2s poll tax. Layered on top of the
`JobNotifier::phase_tx` broadcast channel introduced in the previous
commit:
- Handler loads the row up-front (404 surfaces with the standard JSON
shape, not as an empty stream) and immediately emits an initial
`event: phase` (or `event: complete` for terminal jobs) with the
current snapshot so re-attached wallets see the latest state without
waiting on the next dispatcher transition.
- Subscribes a fresh `broadcast::Receiver` per open stream; forwards
every subsequent `JobPhaseEvent` as `event: phase`, closes on the
first terminal `event: complete`.
- `KeepAlive::new().interval(25 s)` heartbeat survives Cloudflare
Tunnel's ~100 s idle drop without doubling bandwidth.
- Polling fallback (`GET /api/jobs/:id` from PR1) is unchanged — SSE
is additive.
Pure event-builder helpers (`initial_event_from_job`,
`event_from_phase`) stay testable in isolation; the long-lived
forwarding loop in `build_phase_stream` is annotated
`#[cfg_attr(coverage_nightly, coverage(off))]` because its
`tokio::select!` arms depend on real-time broadcast deliveries the
deterministic harness cannot fully cover — same exclusion pattern as
`scanner_ws::run_subscription_loop`.
Tests cover 404, 500-on-db-error, terminal-job-immediate-close
(completed + failed), initial-state-for-non-terminal, end-to-end
phase-transition fan-out, and the cancel-handler publishing path. The
`async-stream` crate is promoted from a transitive to a direct dep
so the router resolves it deterministically.
* docs(jobs): document SSE push channel across SPEC/CONTRIBUTING/MIGRATION_RESEARCH/README/ROADMAP
- SPEC.md §11.2.1: add the GET /api/jobs/:id/stream endpoint row plus
the wire-shape event examples (phase / complete frames + failure /
cancel variants).
- CONTRIBUTING.md Job-API lifecycle: document SSE as the push-based
channel alongside polling, including the broadcast-channel +
per-stream Receiver pattern.
- MIGRATION_RESEARCH.md §7.28: full architectural rationale for the
SSE layer — why broadcast over watch, the 25 s heartbeat rationale,
Cloudflare Tunnel constraints, fallback semantics, coverage scope.
- README.md endpoints table: add the stream row.
- ROADMAP.md Step 9: mark Phase 2 done with pointers to PR2.
* test(jobs/sse): widen test timeouts to absorb shared-runner load
The three SSE stream tests guard the request future and broadcast
recv with 5s/5s/1s tokio timeouts. Under sequential nextest
(test-threads=1) on the shared m3-ultra runner, the per-test DB
setup (fresh schema + migrations) can stretch the wall time of the
guarded section past 5s when CI load pressures the Postgres pool
shared across PRs.
Raise the request-future guards to 30s and the broadcast recv to
10s. The values still bound a stuck handler / lost event well
inside a job's real lifetime budget; they just stop reporting a
loaded runner as a code failure.
No production timeouts change.
* feat(openapi): annotate /api/jobs/{job_id}/stream and register in spec
#161 added utoipa annotations to every other Job-API admit + poll
handler; the SSE push channel introduced by this PR was the last
hold-out because the JobNotifier + Sse<Stream> response shape did
not exist when #161 landed.
Wire-up:
- `stream_job_handler` promoted to `pub(crate)` so the macro can
reference it.
- `#[utoipa::path]` documents the SSE contract: `text/event-stream`
body on 200, `JobErrorResponse` JSON on 404/500. The 200
description names the two event types (`phase`, `complete`), the
heartbeat-comment cadence, and the "stream closes after first
`event: complete`" rule so a wallet author can implement the
consumer side from the spec alone.
- `crate::router::stream_job_handler` added to `ApiDoc::paths(...)`
between `get_job_handler` and `receive_coin_handler` so the spec
surfaces the new route alongside the poll endpoint.
- `openapi_smoke::spec_lists_every_always_on_route` extended to
require `/api/jobs/{job_id}/stream` — drift on the SSE contract
now fails CI fast.
No coverage impact: `stream_job_handler` was already exercised by
the SSE integration tests added in commit `dcfc232`; the annotation
adds zero new runtime code, only the compile-time `__path_*`
generated by `utoipa::path`.
* docs(jobs/sse): reviewer-loop polish — cleanup race, EventSource retry, connection cap
Three doc-only clarifications surfaced by the post-rebase review on
#163:
- **router.rs `or_insert_with` comment.** The cleanup race between the
dispatcher's terminal-publish and `notify_map.remove()` is safe —
a fresh subscriber that opens in the gap reads the already-terminal
row and emits `complete` from the initial-state snapshot, never
depending on the orphaned broadcast subscriber. Documented inline
so a future maintainer does not re-add a "fix" that breaks the
property.
- **MIGRATION_RESEARCH §7.28 — EventSource reconnect layering.** The
wallet's built-in `EventSource` retry runs before the explicit poll
fallback kicks in, so a `Lagged → end-of-stream` is observed by the
wallet as a routine browser-side reconnect (3 s default backoff,
cap), not a hard failure. Capturing the layering so the
reviewer-asked "what does the wallet do on Lagged?" has a written
answer.
- **MIGRATION_RESEARCH §7.28 — concurrent-connection cap.** No
per-node SSE concurrency limit today; the MVP wallet population
fits in low single digits and the work-in-flight is bounded by the
prove queue. The future "N>100 wallets self-host" case needs
either a `Semaphore`-backed `max_sse_streams` or a reverse-proxy
rule — documented as deferred so it does not vanish into the
post-MVP backlog.
Two reviewer-flagged items intentionally NOT addressed:
- The `format!("{:?}", event)` Debug-substring checks on the pure
helpers — axum 0.7's `Sse::Event::Debug` impl is stable and a
shape change would fail all 10 tests in lockstep (a clean signal).
The proposed swap to `parse_sse_events` would require routing each
Event through axum's response-body machinery, which is more
ceremony than the brittleness it removes.
- A unit test that overflows `broadcast::channel(2)` to exercise
the `Err(_) → end-of-stream` arm. The arm sits inside the
`coverage(off)`-annotated `build_phase_stream` loop (real-wall-
clock broadcast deliveries the deterministic harness cannot
exhaustively cover, same pattern as `scanner_ws::run_subscription_
loop`). The behaviour is covered by the documented end-of-stream
contract; the test would only re-prove the broadcast crate's
semantics, not the handler's.
* docs(jobs/sse): tighten EventSource attribution
Two textual nits from reviewer round 2:
- Replace "Mozilla / WHATWG spec default: 3 s, exponential cap" with
the accurate "WHATWG defines a UA-implemented reconnection time
settable via the `retry:` field; Firefox/Chrome ramp from ~3 s in
practice". The previous wording mis-attributed exponential backoff
to the spec when it's actually a UA implementation choice.
- Drop the BitBox `useEventSource` analogy. The hook does not in
fact wrap `EventSource` with a poll fallback (it just renders the
stream), so the cross-reference was decorative; removing it keeps
the section self-contained.
* fix(jobs/sse): drop unreachable json_data fallback closures in SSE event builders
`initial_event_from_job` and `event_from_phase` both built the SSE
frame with
Event::default()
.event(name)
.json_data(payload)
.unwrap_or_else(|_| Event::default().event(name).data("{}"))
The fallback closure existed defensively in case `json_data` failed.
But `payload` in both call sites is a `serde_json::Value` built
inline above with no custom `Serialize` impls — `Event::json_data`'s
error path is only reachable for custom impls that serialise into
non-UTF-8 bytes, which JSON's ASCII-superset output cannot violate.
The closures were therefore structurally unreachable; llvm-cov
counted each as a separate uncovered function plus uncovered line,
failing the 100% line + function gate (2 missed functions + 2
missed lines, both at the closure sites).
Switch both to `.expect("Event::json_data cannot fail for a freshly
built serde_json::Value")`. The expect documents the invariant
inline and a violation would surface as a fast-panic instead of
silently emitting an empty `{}` body — same pattern as the
`response_body.expect(...)` flatten that landed in #161 (`fix(jobs):
drop unreachable response_body fallback closure in
admit_and_enqueue`) and the `utoipa_swagger_ui::serve(...).expect`
flatten in #157.
Pinpointed in 30 seconds via the HTML coverage artifact added in
the previous CI workflow tweak — the second time this diagnostic
has paid for itself in two days.
* feat(openapi): generate OpenAPI 3.x spec from handler annotations (#157) * feat(openapi): generate OpenAPI 3.x spec from handler annotations Annotate every public `/api/*` handler with `#[utoipa::path]` and derive `ToSchema` on request/response types. Build a process-wide `ApiDoc` from those annotations and serve it at `GET /openapi.json` (cached JSON) and `GET /docs` (Swagger UI pinned to swagger-ui-dist@5.32.6). Feature-gated handlers (`address-list`, `username-claim`, `lnurl`) carry their own sub-doc that merges into the main spec only when the feature compiles in, so the document describes the exact wire surface of the running binary. Refs: #155 * refactor(openapi): bundle Swagger UI, drop hardcoded URLs, fix feature combos - Remove static `servers(...)` block; spec inherits "same host" via OpenAPI default - Bundle Swagger UI assets via `utoipa-swagger-ui`; no external CDN dependency - Fix `cargo clippy` with single-feature builds (`lnurl` only, etc.) - Smoke test verifies relative asset URLs and absence of hardcoded hosts * refactor(openapi): polish handler annotations for production - Replace placeholder ellipsis `…` (U+2026) in `SendCoinRequest` PublicKey schema examples with valid 33-byte compressed-hex pubkeys (secp256k1 generator point and its double) so client generators that parse the `example` as hex do not trip over a non-hex character. - Drop redundant `#[schema(rename = "...")]` on `LnurlpResponse` — utoipa already picks up the corresponding `#[serde(rename)]` and duplicating the directive risks future drift between serde and schema. Verified the generated spec still emits `minSendable` / `maxSendable` after removal. - Replace remaining non-ASCII ellipsis in doc and source comments with ASCII `...` for consistency. Handler tag grouping is left as-is: every annotated handler already carries a `tag` (`Accounts`, `Coins`, `Inscriptions`, `Node`, `Usernames`, `LNURL`), so Swagger UI groups them all correctly instead of dropping any into the default bucket. * test(openapi): cover async HTTP handlers and Swagger asset paths The smoke test in `tests/openapi_smoke.rs` exercises the in-memory spec and HTML string paths but never enters the async handlers (`openapi_json_handler`, `docs_handler`, `swagger_asset_handler`). Coverage Gate flagged lines 195-237 of `node/src/openapi.rs` as uncovered. Add sibling-style `openapi_tests.rs` exercising: - OK + JSON content-type from `openapi_json_handler` - OK + HTML content-type from `docs_handler` - bundled CSS and JS asset serving via `swagger_asset_handler` - 404 from `swagger_asset_handler` for unknown files - `swagger_ui_config` cache identity * test(db): harden setup_pool + connect_and_migrate against shared-host load The m3-ultra CI runner (dfx01) co-resides with ~20 production containers (Vaultwarden, Grafana, Loki, dEURO, …) and occasional manual `cargo nextest` runs from operators. Under that load the testcontainers `postgres:17` "ready" log signal fires correctly, but the subsequent SQLx pool connect can stall past the 30s default `acquire_timeout` waiting on the Colima vNIC to complete the TCP handshake. The failure surfaced as a one-shot `sqlx::Error::PoolTimedOut` in `db::tests::*` whenever multiple Heavy CI runs were in-flight on the same host (PR-CIs + production + ad-hoc tests = loadavg ~75 on 28 cores). Two independent hardenings: 1. `db.rs::connect_and_migrate` — bump `acquire_timeout` from the 30s default to 60s. The healthy path connects in <500ms so this only changes behaviour when the host is starved; in that window it is the difference between a flake and a pass. Production bootstrap inherits the same bound — a 30s vs 60s acquire timeout on a node that has been alive for milliseconds is not user- facing latency, and a freshly-started Postgres sidecar under Docker Compose orchestration can also need >30s to finish its first checkpoint pass on a busy host. 2. `db_tests.rs::setup_pool` — wrap the container-start-and-connect sequence in a 3-attempt retry with linear backoff (500ms / 1000ms). The previous container is dropped on each retry so a hung Postgres process never poisons the next attempt. The retry only fires when the host is transiently overloaded; a healthy run still hits the first attempt and pays no overhead. The aggregate effect is that one test panicking with PoolTimedOut no longer cancels the rest of the 371-test suite — the run either re-converges on the next attempt or reports a real container-engine outage three retries deep. * fix(db): retry connect_and_migrate on transient host-load failures The previous fix to `db_tests::setup_pool` retried at the container-start + connect pair, but the 20+ test files that call `connect_and_migrate` from their own ad-hoc `setup_pool` did not inherit it. Coverage Round 2 surfaced a second transient failure mode at `state_tests.rs:48`: connect_and_migrate failed: Protocol("unexpected response from SSLRequest: 0x48 (sqlx_postgres::connection::tls:95)") `0x48 = 'H'` — the testcontainers ready-signal had fired, but the first byte SQLx saw on the wire was garbage instead of the protocol handshake. Same root cause as the earlier `PoolTimedOut`: on the shared m3-ultra host (loadavg ~75 on 28 cores when this fired) the Colima vNIC delivered the bgwriter/autovacuum log lines before the listener-side socket had finished its first message exchange. Move the retry inside `connect_and_migrate` itself so every call site — not just `db_tests::setup_pool` — benefits. Three-attempt retry with linear 500ms / 1000ms backoff, classified for the two documented transient sqlx error kinds (`PoolTimedOut`, `Protocol(... SSLRequest ...)`). Auth / migration / host-not-found errors stay non-retryable so a real misconfiguration still fails fast in <1s. Mark the retry loop + classifier `#[cfg_attr(coverage_nightly, coverage(off))]` — both are defensive against host-load conditions that the deterministic test harness cannot reproduce on demand. The healthy-path `try_connect_and_migrate` worker stays fully covered by every test that hits a Postgres testcontainer. * feat(openapi): cover /, /health, /health/ready, /health/publisher, /api/history Brings the always-on wire surface into the generated spec without any hand-written drift surface: - `GET /` (root_handler) — service identification + endpoint map - `GET /health` (health_handler) — promote from inline closure to a named handler so the liveness probe carries a `#[utoipa::path]` annotation matching the readiness / publisher probes - `GET /health/ready` (ready_handler) — DB + Esplora + prover-warm gate; both 200 and 503 responses share `ReadyResponse` so Kuma and load-balancer integrations can branch on `status` / `failures` without scraping the HTTP code - `GET /health/publisher` (publisher_health_handler) — UTXO state of the publisher wallet (deploy-dev preflight gate) - `GET /api/history` (get_history_handler) — paginated per-address history (issue #153); 422 / 500 branches reuse the documented `HistoryErrorResponse` envelope so wallet error handling stays in sync with the server contract All new handlers picked up `pub(crate)` visibility plus a `#[utoipa::path]` block; new response structs derive `ToSchema`. `openapi.rs` registers each handler in `paths(...)` and each response type in `components(schemas(...))`. The smoke suite is extended to require every new route under `spec_lists_every_always_on_route` plus `HistoryResponse`, `HistoryItem`, `HistoryErrorResponse`, and `ReadyResponse` under `spec_registers_critical_schemas` so a future regression on either contract fails CI fast. `CONTRIBUTING.md` gains a `REST API & OpenAPI` section: the exposed route table, the four-step recipe for adding a new endpoint, and the existing drift guards. The project-structure tree calls out `openapi.rs` next to `router.rs` so contributors find the spec assembly without spelunking. * refactor(openapi): reviewer-loop polish — tag, 503 schema, root endpoint map Tightens consistency surfaced by the post-rebase review: - Unify `/api/balance` + `/api/history` + `/api/address` under `tag = "Accounts"` (read endpoints keyed on an address). `/api/history` was tagged `"Coins"` in the previous commit; aligned with the pre-existing `/api/balance` annotation and updated CONTRIBUTING table + recipe accordingly. - Promote the `/health/publisher` 503 body from an ad-hoc `serde_json::json!({...})` to a typed `PublisherHealthErrorResponse` with `ToSchema`, registered under `components(schemas(...))` and bound to the 503 entry in the `responses(...)` block. The 200 branch drops the redundant `serde_json::to_value(...)` wrapper now that both arms go through `into_response()`. - Extend `RootEndpoints` to mirror every always-on route: `/api/mint`, `/api/username/resolve/{username}`, `/health/ready`, `/health/publisher`, `/openapi.json`, `/docs`. The struct's doc-comment now explicitly justifies what is omitted (feature-gated routes, admin endpoints) so the endpoint map is not silently stale. - Document the meta + admin exclusion policy in the OpenAPI section of CONTRIBUTING: `/openapi.json`, `/docs`, `/docs/{file}`, and `/api/admin/*` are intentionally outside `paths(...)` and any future admin route should follow the same rule. * fix(openapi): flatten swagger_asset_handler — drop unreachable Err arm The previous shape carried a third `Err(_) => 500` match arm to satisfy the `Result<Option<SwaggerFile>, Box<dyn Error>>` signature of `utoipa_swagger_ui::serve`. Inspection of utoipa-swagger-ui 9.0.2 (`src/lib.rs::serve`) shows the function only returns `Err` in two situations: - the bundled `swagger-initializer.js` bytes fail UTF-8 decoding — impossible because the `vendored` feature bakes a known-good UTF-8 bundle in at compile time - the oauth config formatter errors — impossible because our `swagger_ui_config()` builds a `Config` without an oauth section Both invariants are structurally enforced by our build, so the arm was dead code and surfaced as a single uncovered line under the 100% line-coverage gate. Flatten the Result with `.expect(...)` — the panic message documents the contract and a future upstream change that violates either invariant would surface within minutes via the readiness probe. Updates the two test doc-comments referencing `Ok(Some)` / `Ok(None)` to match the post-flatten shape. * feat(jobs): introduce async Job-API; remove synchronous mint/send/commit (#161) * feat(jobs): introduce async Job-API; remove synchronous mint/send/commit Replaces the synchronous `/api/{mint,send,commit}` surface with an admit-and-poll Job-API. Every wallet-triggered prove operation now returns 202 with a `job_id` immediately; the heavy prove + broadcast work runs in a background dispatcher loop driven by a new `jobs` table. ## Routing changes Removed (legacy synchronous): - `POST /api/send` - `POST /api/mint` - `POST /api/commit` Added (Job-API admit + poll): - `POST /api/jobs/mint` — admit a mint job - `POST /api/jobs/send` — admit a send job - `GET /api/jobs/{job_id}` — poll job state (Retry-After: 2) - `POST /api/jobs/{job_id}/commit` — attach signed commitment - `POST /api/jobs/{job_id}/cancel` — cancel still-queued jobs ## New crates / modules - `node/src/job_store.rs` — typed wrapper around the `jobs` table (CreateResult / JobKind / JobStatus enums, idempotency-key handling). - `node/src/job_dispatcher.rs` — async dispatcher loop; consumes `JobEnvelope`s from an mpsc channel, dispatches by `JobKind`. - `node/src/flow.rs` — the prove + broadcast flows extracted from the old handler bodies, now driven by the dispatcher. - `node/migrations/0014_jobs.sql` — `jobs` table + indexes + CHECK constraints mirroring the typed enums. ## Dependencies - `uuid v1` (`v4`, `serde`) — `jobs.public_id` column. - `dashmap v6` — `job_notify_map` for `commit` wake-up notifications. - `chrono v0.4` — `TIMESTAMPTZ` round-trip on the timestamp columns. - `sqlx` features: `uuid`, `chrono`. ## OpenAPI integration The Job-API admit + poll surface is fully covered by `#[utoipa::path]` annotations on every new handler; the response and request envelopes (`JobAcceptedResponse`, `JobStatusResponse`, `JobErrorResponse`, plus the kept-around `SendCoinRequest` / `MintRequest` / `CommitRequest` shapes the Job-API still consumes) carry `ToSchema` derives and are registered under `components(schemas(...))`. Surviving always-on handlers from #157 keep their annotations; the smoke test `spec_lists_every_always_on_route` now requires every new `/api/jobs/*` path. ## Coverage The PR is iterated to 100% line + function coverage under `cargo llvm-cov nextest --all-features` per the project's standing quality gate. Legacy mint/send/commit handler tests removed alongside the routes; new tests live in `router_tests.rs` covering the admit + poll + commit + cancel paths plus dispatcher-loop behaviour in `job_store_tests.rs`. ## Migration story `migrations/0014_jobs.sql` adds the new `jobs` table; existing data is untouched. Wallets must move to the admit-and-poll surface — the synchronous routes return 404 after this PR lands. ## Root endpoint map `root_handler` now advertises the Job-API routes alongside the operational endpoints (`/health/ready`, `/health/publisher`, `/openapi.json`, `/docs`, `/api/username/resolve/{username}`); the `RootEndpoints` doc-comment documents the omissions (feature-gated routes, admin endpoints, meta). * ci(coverage): expose per-function uncovered list + upload HTML artifact on gate failure When the heavy gate fails, the operator currently sees: - per-file `--show-missing-lines` text (sometimes elides files with branch-only deltas, observed on this PR's failure) - per-file JSON summary (lines/functions percent only) This leaves the actual gap obscured. Solving it required re-running the ~50 min heavy gate locally — that is the wrong incentive, especially on shared-runner builds where a re-run blocks another PR. This commit adds two diagnostics to the existing failure step: - **Per-function symbol list.** A jq filter over the JSON report emits one line per uncovered function as `file:line\tsymbol_name`. The operator sees `node/src/router.rs:1234\tmy_handler` immediately and can target the missing test from the CI log without leaving the browser. - **HTML report upload.** `cargo llvm-cov report --html` generates a Codecov-style browsable report; the new `actions/upload-artifact@v4` step uploads it under `llvm-cov-html-{run_id}-{run_attempt}` for 14 days. The operator downloads the artifact, opens `index.html`, and clicks straight to the file/line/function that's red — same workflow as a local `cargo llvm-cov --open` without paying the ~50 min reproduction cost. Both diagnostics are gated on `if: failure()` so the green path pays nothing. The ignore-regex is pulled into a single `$IGNORE` shell variable so the four invocations stay in sync — the previous duplicated literal made it easy to drift one filter relative to another. * fix(jobs): drop unreachable response_body fallback closure in admit_and_enqueue The idempotent-replay arm for `Completed` jobs called job.response_body.clone().unwrap_or_else(|| serde_json::json!({})) to fall back on an empty JSON object if `response_body` was somehow absent. `JobStore::complete` sets `response_body` on the row before flipping the status to `Completed` (the matching INSERT is non-nullable on the value side), so the closure is unreachable in practice — a `None` would mean the row was hand-edited or the schema invariant broke. The defensive empty-object fallback only existed because the wallet would otherwise receive a 500 on an event that can't happen. llvm-cov scored the closure as a separate uncovered function plus an uncovered line (the closure body), which fails the 100% line + function gate — the only delta keeping PR #161 red after the rebase. Switching to `.expect(...)` documents the invariant inline and surfaces a violation as a fast-panic instead of a silent empty body, matching how this codebase handles the other "structurally unreachable but stdlib forces a fallback" branches (see the `utoipa_swagger_ui::serve(...)` flatten in `openapi.rs`). Found by the new per-file HTML coverage artifact added in the previous commit — a 50 min heavy gate's worth of guessing replaced by reading the artifact for 30 seconds. * feat(jobs): SSE push channel GET /api/jobs/:id/stream (#163) * feat(jobs): introduce JobNotifier broadcast channel for phase fan-out Refactor the dispatcher's per-job coordination primitive from a bare `Arc<Notify>` into a `JobNotifier` struct that pairs the existing commit-wake Notify with a `tokio::sync::broadcast::Sender<JobPhaseEvent>`. Every dispatcher status-persistence site (set_status / set_awaiting_signature / complete / fail) now also publishes a `JobPhaseEvent` so future SSE subscribers can observe real-time phase transitions; the existing commit-wake path is unchanged. The cancel handler also publishes a terminal `cancelled` event so attached listeners see the close. This is the plumbing layer for the SSE push channel — no new endpoint yet, no new behaviour observable from outside the dispatcher. The `AppState` field type widens from `Arc<DashMap<Uuid, Arc<Notify>>>` to `JobNotifyMap` (alias for `Arc<DashMap<Uuid, Arc<JobNotifier>>>`); the runtime resumer and the router commit handler are adjusted to the new shape. Test fixtures construct `JobNotifier::new()` instead of `Notify::new()`. * feat(jobs): add SSE push channel GET /api/jobs/:id/stream Server-Sent Events endpoint that streams real-time phase transitions to wallets without the ~2s poll tax. Layered on top of the `JobNotifier::phase_tx` broadcast channel introduced in the previous commit: - Handler loads the row up-front (404 surfaces with the standard JSON shape, not as an empty stream) and immediately emits an initial `event: phase` (or `event: complete` for terminal jobs) with the current snapshot so re-attached wallets see the latest state without waiting on the next dispatcher transition. - Subscribes a fresh `broadcast::Receiver` per open stream; forwards every subsequent `JobPhaseEvent` as `event: phase`, closes on the first terminal `event: complete`. - `KeepAlive::new().interval(25 s)` heartbeat survives Cloudflare Tunnel's ~100 s idle drop without doubling bandwidth. - Polling fallback (`GET /api/jobs/:id` from PR1) is unchanged — SSE is additive. Pure event-builder helpers (`initial_event_from_job`, `event_from_phase`) stay testable in isolation; the long-lived forwarding loop in `build_phase_stream` is annotated `#[cfg_attr(coverage_nightly, coverage(off))]` because its `tokio::select!` arms depend on real-time broadcast deliveries the deterministic harness cannot fully cover — same exclusion pattern as `scanner_ws::run_subscription_loop`. Tests cover 404, 500-on-db-error, terminal-job-immediate-close (completed + failed), initial-state-for-non-terminal, end-to-end phase-transition fan-out, and the cancel-handler publishing path. The `async-stream` crate is promoted from a transitive to a direct dep so the router resolves it deterministically. * docs(jobs): document SSE push channel across SPEC/CONTRIBUTING/MIGRATION_RESEARCH/README/ROADMAP - SPEC.md §11.2.1: add the GET /api/jobs/:id/stream endpoint row plus the wire-shape event examples (phase / complete frames + failure / cancel variants). - CONTRIBUTING.md Job-API lifecycle: document SSE as the push-based channel alongside polling, including the broadcast-channel + per-stream Receiver pattern. - MIGRATION_RESEARCH.md §7.28: full architectural rationale for the SSE layer — why broadcast over watch, the 25 s heartbeat rationale, Cloudflare Tunnel constraints, fallback semantics, coverage scope. - README.md endpoints table: add the stream row. - ROADMAP.md Step 9: mark Phase 2 done with pointers to PR2. * test(jobs/sse): widen test timeouts to absorb shared-runner load The three SSE stream tests guard the request future and broadcast recv with 5s/5s/1s tokio timeouts. Under sequential nextest (test-threads=1) on the shared m3-ultra runner, the per-test DB setup (fresh schema + migrations) can stretch the wall time of the guarded section past 5s when CI load pressures the Postgres pool shared across PRs. Raise the request-future guards to 30s and the broadcast recv to 10s. The values still bound a stuck handler / lost event well inside a job's real lifetime budget; they just stop reporting a loaded runner as a code failure. No production timeouts change. * feat(openapi): annotate /api/jobs/{job_id}/stream and register in spec #161 added utoipa annotations to every other Job-API admit + poll handler; the SSE push channel introduced by this PR was the last hold-out because the JobNotifier + Sse<Stream> response shape did not exist when #161 landed. Wire-up: - `stream_job_handler` promoted to `pub(crate)` so the macro can reference it. - `#[utoipa::path]` documents the SSE contract: `text/event-stream` body on 200, `JobErrorResponse` JSON on 404/500. The 200 description names the two event types (`phase`, `complete`), the heartbeat-comment cadence, and the "stream closes after first `event: complete`" rule so a wallet author can implement the consumer side from the spec alone. - `crate::router::stream_job_handler` added to `ApiDoc::paths(...)` between `get_job_handler` and `receive_coin_handler` so the spec surfaces the new route alongside the poll endpoint. - `openapi_smoke::spec_lists_every_always_on_route` extended to require `/api/jobs/{job_id}/stream` — drift on the SSE contract now fails CI fast. No coverage impact: `stream_job_handler` was already exercised by the SSE integration tests added in commit `dcfc232`; the annotation adds zero new runtime code, only the compile-time `__path_*` generated by `utoipa::path`. * docs(jobs/sse): reviewer-loop polish — cleanup race, EventSource retry, connection cap Three doc-only clarifications surfaced by the post-rebase review on #163: - **router.rs `or_insert_with` comment.** The cleanup race between the dispatcher's terminal-publish and `notify_map.remove()` is safe — a fresh subscriber that opens in the gap reads the already-terminal row and emits `complete` from the initial-state snapshot, never depending on the orphaned broadcast subscriber. Documented inline so a future maintainer does not re-add a "fix" that breaks the property. - **MIGRATION_RESEARCH §7.28 — EventSource reconnect layering.** The wallet's built-in `EventSource` retry runs before the explicit poll fallback kicks in, so a `Lagged → end-of-stream` is observed by the wallet as a routine browser-side reconnect (3 s default backoff, cap), not a hard failure. Capturing the layering so the reviewer-asked "what does the wallet do on Lagged?" has a written answer. - **MIGRATION_RESEARCH §7.28 — concurrent-connection cap.** No per-node SSE concurrency limit today; the MVP wallet population fits in low single digits and the work-in-flight is bounded by the prove queue. The future "N>100 wallets self-host" case needs either a `Semaphore`-backed `max_sse_streams` or a reverse-proxy rule — documented as deferred so it does not vanish into the post-MVP backlog. Two reviewer-flagged items intentionally NOT addressed: - The `format!("{:?}", event)` Debug-substring checks on the pure helpers — axum 0.7's `Sse::Event::Debug` impl is stable and a shape change would fail all 10 tests in lockstep (a clean signal). The proposed swap to `parse_sse_events` would require routing each Event through axum's response-body machinery, which is more ceremony than the brittleness it removes. - A unit test that overflows `broadcast::channel(2)` to exercise the `Err(_) → end-of-stream` arm. The arm sits inside the `coverage(off)`-annotated `build_phase_stream` loop (real-wall- clock broadcast deliveries the deterministic harness cannot exhaustively cover, same pattern as `scanner_ws::run_subscription_ loop`). The behaviour is covered by the documented end-of-stream contract; the test would only re-prove the broadcast crate's semantics, not the handler's. * docs(jobs/sse): tighten EventSource attribution Two textual nits from reviewer round 2: - Replace "Mozilla / WHATWG spec default: 3 s, exponential cap" with the accurate "WHATWG defines a UA-implemented reconnection time settable via the `retry:` field; Firefox/Chrome ramp from ~3 s in practice". The previous wording mis-attributed exponential backoff to the spec when it's actually a UA implementation choice. - Drop the BitBox `useEventSource` analogy. The hook does not in fact wrap `EventSource` with a poll fallback (it just renders the stream), so the cross-reference was decorative; removing it keeps the section self-contained. * fix(jobs/sse): drop unreachable json_data fallback closures in SSE event builders `initial_event_from_job` and `event_from_phase` both built the SSE frame with Event::default() .event(name) .json_data(payload) .unwrap_or_else(|_| Event::default().event(name).data("{}")) The fallback closure existed defensively in case `json_data` failed. But `payload` in both call sites is a `serde_json::Value` built inline above with no custom `Serialize` impls — `Event::json_data`'s error path is only reachable for custom impls that serialise into non-UTF-8 bytes, which JSON's ASCII-superset output cannot violate. The closures were therefore structurally unreachable; llvm-cov counted each as a separate uncovered function plus uncovered line, failing the 100% line + function gate (2 missed functions + 2 missed lines, both at the closure sites). Switch both to `.expect("Event::json_data cannot fail for a freshly built serde_json::Value")`. The expect documents the invariant inline and a violation would surface as a fast-panic instead of silently emitting an empty `{}` body — same pattern as the `response_body.expect(...)` flatten that landed in #161 (`fix(jobs): drop unreachable response_body fallback closure in admit_and_enqueue`) and the `utoipa_swagger_ui::serve(...).expect` flatten in #157. Pinpointed in 30 seconds via the HTML coverage artifact added in the previous CI workflow tweak — the second time this diagnostic has paid for itself in two days. --------- Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
…Opt B) (#182) * perf(tests): shared Postgres container + per-test schema (issue #181 Opt B) Replaces the per-test `Postgres::default().start()` model with a single shared Postgres container that every test process attaches to via testcontainers' `with_reuse(ReuseDirective::Always)` and a stable container name (`zkcoins-test-shared-pg`). Each test still gets a fully isolated state via a UUID-named schema with `search_path` pinned to it; migrations are run per-schema. The reuse flag is load-bearing: `cargo nextest` defaults to one process per test, so a process-local `OnceCell<Postgres>` does not actually share state across tests — it degrades to one container per test. Verified on a local M5 Max (OrbStack): 6 db-tests finish in 1.5 s with exactly 1 container running, vs. ~24 s with 6 containers under the old per-process model. CI gains three `docker rm -f zkcoins-test-shared-pg` cleanup steps (one per test job, always-on) so the shared container does not leak across PR runs on the self-hosted runner. Coverage gate's `--ignore-filename-regex` is extended to skip `test_db.rs` — the new `#[cfg(test)]`-only test-infra module would otherwise drag its Drop-future uncovered lines into the 100% gate. `db_tests::connect_and_migrate_creates_all_tables` is rewritten to route through the real `db::connect_and_migrate` (via the `?options=-c search_path=<schema>` URL trick) so the success-path of that function stays covered. Expected wall on the M3 Ultra runner per issue #181: 47 min → ~37 min at `--test-threads=1` (Optimisation A — flipping the test isolation to multi-thread — is a follow-up that depends on this landing first; see #181 Recommendation section). Test files migrated to the shared helper: db_tests, state_tests, r2_probe_tests, username_tests, main_tests, runtime_tests, router_tests (incl. the jobs_test_state factory from #161), job_store_tests, account_node_tests, audit_tests, publisher_tests. * test(db): include jobs table in connect_and_migrate assertion Migration 0014 (introduced by #161, async Job-API) adds the jobs table to the production schema. The rebase of #182 onto staging left the hard-coded expected-tables list in connect_and_migrate_creates_all_tables unchanged, so the assertion sees an extra row ("jobs") it does not expect and fails fast under nextest's default fail-fast mode — masking the rest of the suite. Adds "jobs" at its alphabetic position and bumps the migration range in the comment from 0001-0013 to 0001-0014.
#183) With per-test schema isolation + shared-container reuse from #182, the suite is parallel-safe. This PR: - Flips `--test-threads=1` to `--test-threads=8` across the 3 CI test jobs (db-tests, prover-tests, test-and-coverage) and the matching CONTRIBUTING.md references. - Adds a `fs2` cross-process file lock around `init_shared_pg` in test_db.rs. testcontainers 0.27 does NOT atomicise its attach-or-create path: 8 concurrent nextest processes all see "container not present", all POST /containers/create, 1 wins and 7 fail with Docker 409 Conflict. The lock serialises the attach-or-create call; the container creation cost (~3 s once) amortises across the whole test run. - runtime_tests.rs: env mutation consolidated behind a `OnceLock`-backed `ensure_test_env()` so concurrent callers do not race on process-wide env. `PROOFS_DIR` removed from env entirely and passed as a parameter on `start_rest_node` (main.rs reads the env at the binary edge). - router_tests.rs: 2 hard-coded `/tmp/zkcoins-*-proofs` paths replaced with `tempfile::tempdir().keep()` so each parallel test gets a unique ProofStore directory and `next_id` cannot race. Empirical on an Apple M5 Max workstation (OrbStack): a wide DB + state + router + username + audit subset of 146 tests passes under --test-threads=8 in 183 s wall (CPU 1325 %, exactly one shared postgres:17 container live during the run). Expected on the M3 Ultra runner per #181: 47 min (pre-Opt-B) -> ~44 min (after #182, measured) -> ~10-12 min (after this PR).
* perf(tests): shared Postgres container + per-test schema (Issue #181 Opt B) (#182) * perf(tests): shared Postgres container + per-test schema (issue #181 Opt B) Replaces the per-test `Postgres::default().start()` model with a single shared Postgres container that every test process attaches to via testcontainers' `with_reuse(ReuseDirective::Always)` and a stable container name (`zkcoins-test-shared-pg`). Each test still gets a fully isolated state via a UUID-named schema with `search_path` pinned to it; migrations are run per-schema. The reuse flag is load-bearing: `cargo nextest` defaults to one process per test, so a process-local `OnceCell<Postgres>` does not actually share state across tests — it degrades to one container per test. Verified on a local M5 Max (OrbStack): 6 db-tests finish in 1.5 s with exactly 1 container running, vs. ~24 s with 6 containers under the old per-process model. CI gains three `docker rm -f zkcoins-test-shared-pg` cleanup steps (one per test job, always-on) so the shared container does not leak across PR runs on the self-hosted runner. Coverage gate's `--ignore-filename-regex` is extended to skip `test_db.rs` — the new `#[cfg(test)]`-only test-infra module would otherwise drag its Drop-future uncovered lines into the 100% gate. `db_tests::connect_and_migrate_creates_all_tables` is rewritten to route through the real `db::connect_and_migrate` (via the `?options=-c search_path=<schema>` URL trick) so the success-path of that function stays covered. Expected wall on the M3 Ultra runner per issue #181: 47 min → ~37 min at `--test-threads=1` (Optimisation A — flipping the test isolation to multi-thread — is a follow-up that depends on this landing first; see #181 Recommendation section). Test files migrated to the shared helper: db_tests, state_tests, r2_probe_tests, username_tests, main_tests, runtime_tests, router_tests (incl. the jobs_test_state factory from #161), job_store_tests, account_node_tests, audit_tests, publisher_tests. * test(db): include jobs table in connect_and_migrate assertion Migration 0014 (introduced by #161, async Job-API) adds the jobs table to the production schema. The rebase of #182 onto staging left the hard-coded expected-tables list in connect_and_migrate_creates_all_tables unchanged, so the assertion sees an extra row ("jobs") it does not expect and fails fast under nextest's default fail-fast mode — masking the rest of the suite. Adds "jobs" at its alphabetic position and bumps the migration range in the comment from 0001-0013 to 0001-0014. * perf(tests): enable parallel execution (--test-threads=8) (#181 Opt A) (#183) With per-test schema isolation + shared-container reuse from #182, the suite is parallel-safe. This PR: - Flips `--test-threads=1` to `--test-threads=8` across the 3 CI test jobs (db-tests, prover-tests, test-and-coverage) and the matching CONTRIBUTING.md references. - Adds a `fs2` cross-process file lock around `init_shared_pg` in test_db.rs. testcontainers 0.27 does NOT atomicise its attach-or-create path: 8 concurrent nextest processes all see "container not present", all POST /containers/create, 1 wins and 7 fail with Docker 409 Conflict. The lock serialises the attach-or-create call; the container creation cost (~3 s once) amortises across the whole test run. - runtime_tests.rs: env mutation consolidated behind a `OnceLock`-backed `ensure_test_env()` so concurrent callers do not race on process-wide env. `PROOFS_DIR` removed from env entirely and passed as a parameter on `start_rest_node` (main.rs reads the env at the binary edge). - router_tests.rs: 2 hard-coded `/tmp/zkcoins-*-proofs` paths replaced with `tempfile::tempdir().keep()` so each parallel test gets a unique ProofStore directory and `next_id` cannot race. Empirical on an Apple M5 Max workstation (OrbStack): a wide DB + state + router + username + audit subset of 146 tests passes under --test-threads=8 in 183 s wall (CPU 1325 %, exactly one shared postgres:17 container live during the run). Expected on the M3 Ultra runner per #181: 47 min (pre-Opt-B) -> ~44 min (after #182, measured) -> ~10-12 min (after this PR). --------- Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
* test(api_remote): migrate E2E suite to async Job-API (#161 removed sync routes) PR #161 removed the synchronous /api/mint, /api/send and /api/commit routes and replaced them with the async Job-API. Migrate the api_remote.rs E2E suite to the new contract so the "API E2E against DEV" check passes again. Client flow: - mint: POST /api/jobs/mint (Idempotency-Key) -> 202 -> poll GET /api/jobs/:id to completed; result == legacy mint body. - send: POST /api/jobs/send (Idempotency-Key) -> 202; signature + timestamp + hex gates run inline (still 401/422 synchronously), poll to awaiting_signature which carries proof_id; fetch the send CoinProof, decode ash/ocr from the Plonky2 proof public inputs (commitment is None on the send proof), sign ash||ocr, POST /api/jobs/:id/commit -> 200 {status:"broadcasting"}, poll to completed; result == legacy commit body. Helpers added: random_idempotency_key, uuid_v4_like, poll_job_until_terminal, poll_job_until_status, mint_via_job, submit_send_job, ash_ocr_from_send_proof, commit_send_job, fetch_coin_proof, and a bounded retry for the send->commit->send scanner-indexing race (the async commit_flow no longer advances the in-process SMT synchronously, unlike the old /api/commit). Assertion changes vs. the removed sync routes (verified live against DEV): - Job-API validation errors use the JobErrorResponse envelope ({error: ...}) instead of the legacy {success:false,error:...}; negative-path body assertions updated accordingly (error string preserved, so the app KNOWN_SERVER_ERRORS lockstep still holds). - send_coins business failures (unknown account, insufficient funds) are no longer synchronous 404/422: the job is admitted (202) and fails asynchronously, so those tests now assert the terminal job error string. - mint/commit response state-hash + coins-root field-coverage tests now read the populated job result object. No production (non-test) code changed. Test names preserved, including *_roundtrip_* (deploy-prd --skip _roundtrip_ semantics) and feature_skip!. * test(api_remote): fix two deterministic residual failures Two test-only fixes; no production code touched. (A) second_send_roundtrip_succeeds_without_prev_commitment_pubkey_field Stop minting a second time into Alice before send #2. The extra mint pushed a fresh coin into Alice's coin_queue, forcing send #2 through send_coins_inner's in-coin loop. That loop inserts each spent coin id into account.coin_history BEFORE the prove, and the prove leg has no rollback on failure: a single transient prove failure (the genuine "Unable to get merkle proofs for provided public key" scanner race) leaves the coin in BOTH coin_queue and coin_history, after which every retry fails permanently with "Should provide an inclusion proof" and the retry budget can never clear it. Spending Alice's send #1 change directly from account.balance with an empty coin_queue skips the in-coin loop entirely, keeping retries idempotent and isolating the assertion to its subject: the omitted prev_commitment_pubkey. Intent unchanged — the second send still omits prev_commitment_pubkey and num_sends advances to 2. (B) history_unknown_address_returns_empty_page Use a freshly-generated keypair's address instead of a hardcoded one. DEV is a persistent, shared closed-env DB, so the hardcoded address had accumulated a history row and total == 0 no longer held. A random address is provably untouched, guaranteeing the empty-page contract. * test(api_remote): commit send #2 in second_send roundtrip to release dispatcher worker A send job left in awaiting_signature pins the single inline dispatcher worker for the full awaiting_signature_timeout (600s on DEV), starving every later test in the serial suite. Drive send #2 through commit so the worker is released and the roundtrip completes; surface the job_id from submit_send_no_prev_until_awaiting so the caller can commit. See #186 for the underlying node-side head-of-line-blocking issue.
Promote: staging -> develop
Remove the serializing needs: lint-and-build from db-tests, prover-tests, and test-and-coverage so the heavy M3 Ultra jobs start in parallel with lint-and-build instead of waiting behind it. The draft-skip + push behaviour previously inherited via that needs: is preserved by prepending the same (push || draft == false) guard to each heavy job's own if:. notify-failure keeps needs: [lint-and-build, test-and-coverage] — it is a fan-in failure aggregator, not work serialization. Trade-off: on a lint failure the M3 Ultra runner time is now spent regardless, in exchange for ~7-8 min faster feedback per heavy run.
) Remove the serializing needs: lint-and-build from db-tests, prover-tests, and test-and-coverage so the heavy M3 Ultra jobs start in parallel with lint-and-build instead of waiting behind it. The draft-skip + push behaviour previously inherited via that needs: is preserved by prepending the same (push || draft == false) guard to each heavy job's own if:. notify-failure keeps needs: [lint-and-build, test-and-coverage] — it is a fan-in failure aggregator, not work serialization. Trade-off: on a lint failure the M3 Ultra runner time is now spent regardless, in exchange for ~7-8 min faster feedback per heavy run. Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
TaprootFreak
marked this pull request as ready for review
June 2, 2026 21:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
Commits: 2 new commit(s)