From 457f1ec97fcca49e2f9f002a296c1a208442be02 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sun, 31 May 2026 21:20:53 +0200 Subject: [PATCH 1/9] feat(api): add GET /api/history endpoint (#153) (#162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(api): add GET /api/history endpoint (#153) Adds paginated per-address transaction history reading from the account_history table populated by the migration-0008 trigger. * /api/history?address=&limit=&offset=, behind 'always' status (no feature gate). * Reuses the same hex decode + 32-byte length rules /api/balance applies; rejects missing address, invalid hex, limit outside [1, 200], negative offset with 400. * Newest-first ORDER BY changed_at DESC; offset beyond total returns empty items with the unfiltered total so the caller can detect end-of-list. * LEFT JOIN observed_inscriptions + pending_inscriptions on triggering_commit_txid so block_height + status surface once a future caller threads zkcoins.account_commit_txid through the upsert. Today both joined columns are NULL and txid/block_height remain null on the wire. * counterparty + memo intentionally null in v1: the current schema does not store the recipient address per-mutation and has no memo column. Out-of-scope for this PR: app UI changes, OpenAPI export, WebSocket push, the Zod schema in zk-coins/app. * README Features table entry added. * fix(api/history): round-2 review fixes — SQL-side filter, pending default, 422, one DB call Six fixes from the two independent reviews of #153: 1. Push `source IN ('mint','send','receive')` into both the page query and the filtered total so pagination is correct. The post-fetch `filter_map` stays as a defense-in-depth safety net but no longer actually drops rows. Closes a bug where `total` over-counted hidden rows and pages came back smaller than the requested `limit`. 2. Status default flips from `confirmed` to `pending`. A DB-committed `account_history` row only proves a server-side state change, not an on-chain confirmation. The new mapping: * pending_inscriptions.status='complete' -> 'confirmed' * pending_inscriptions.status='failed' -> 'failed' * pending_inscriptions.status IN ('constructed', 'commit_broadcast','reveal_broadcast') -> 'pending' * no pending row + observed_inscriptions.block_height IS NOT NULL -> 'confirmed' * no pending row + no observed row -> 'pending' The match goes through a new `PendingInscriptionStatus` enum so the `match` is exhaustive — a future schema state addition fails to compile (no `_ => "pending"` catch-all). 3. Collapse `count_account_history` + `list_account_history` into one round-trip via a CTE: one filtered-count CTE cross-joined to the LIMIT/OFFSET page CTE. The handler now has a single DB error branch, closing the dead-arm coverage gap the two-call layout left behind. The empty-page case still returns the real total (sentinel row) so the caller can drive pagination without a second query. 4. Switch all input-validation status codes from 400 to 422 to match `/api/balance` and the rest of the read surface. Framework-level 400 (axum Query rejection on non-integer limit) stays. 5. A non-null `prev_data` blob that fails to bincode-decode no longer silently collapses to `prev_balance = 0` (which would fabricate the full new balance as the delta). The row is dropped with a warn log. 6. TODOs for the deferred work reference the follow-up issues: * zk-coins/node#159 — thread `zkcoins.account_commit_txid` GUC * zk-coins/node#160 — capture counterparty_address per row (zk-coins/app#145 covers the typed-client wiring on the other repo.) * fix(api/history): hoist blob.len() out of tracing::warn! for coverage The tracing macro lazily evaluates its arguments based on the active log level, so under the default-off test subscriber blob.len() is never executed. Coverage Gate flagged it as the only uncovered line in the node + shared scope (99.97% lines, 1 missed). Pre-compute blob_len in a let binding so the line runs regardless of tracing config. --- README.md | 1 + node/src/db.rs | 154 +++++++++ node/src/db_tests.rs | 142 +++++++++ node/src/router.rs | 379 ++++++++++++++++++++++ node/src/router_tests.rs | 659 +++++++++++++++++++++++++++++++++++++++ node/tests/api_remote.rs | 125 ++++++++ 6 files changed, 1460 insertions(+) diff --git a/README.md b/README.md index 94d28fdc..e375642a 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,7 @@ API endpoints, background services, their activation status, and the tests that | Health check | `GET /health` | always | mvp | 100% (router) | | Network info | `GET /api/info` | env¹ | mvp | 100% (router) | | Get balance | `GET /api/balance?address=` | always | mvp | 100% (router) | +| List per-address history | `GET /api/history?address=&limit=&offset=` | always | mvp | 100% (router) | | List all addresses | `GET /api/address` | feature (`address-list`) | gate | 100% (router) | | Mint coins (single-phase) | `POST /api/mint` | always² | mvp | 100% (account_node) | | Send — phase 1 (generate proof) | `POST /api/send` | env² | mvp | 100% (router) | diff --git a/node/src/db.rs b/node/src/db.rs index 5e0fc220..85b38053 100644 --- a/node/src/db.rs +++ b/node/src/db.rs @@ -1279,6 +1279,160 @@ pub async fn load_root_indices( Ok(out) } +// ---- Account history listing (issue #153) --------------------------------- + +/// One row of the per-account history view returned by +/// [`list_account_history`]. Mirrors the columns of `account_history` +/// that the `/api/history` handler surfaces, plus the joined +/// `block_height` / `status` / `commit_txid` triple from +/// `observed_inscriptions` and `pending_inscriptions` (currently always +/// `None` because no code path threads `zkcoins.account_commit_txid` +/// through the upsert trigger — see the field docs). +#[derive(Debug, Clone)] +pub struct AccountHistoryRow { + /// `account_history.id` — server-internal monotonic id, always set. + /// Stable across restarts; safe to expose as the row identifier. + pub id: i64, + /// `account_history.changed_at` as a Unix epoch in seconds. + pub timestamp_secs: i64, + /// `account_history.source` — one of `mint` / `send` / `receive` / + /// `scanner` / `recovery`. The handler filters to the user-facing + /// trio before mapping to the `direction` enum on the wire. + pub source: String, + /// `account_history.prev_data` bincode blob, `None` for the first + /// row of an address (initial INSERT). Used by the handler to + /// compute the balance delta that becomes the `amount` field. + pub prev_data: Option>, + /// `account_history.new_data` bincode blob — never null per schema. + pub new_data: Vec, + /// `account_history.triggering_commit_txid` — the on-chain commit + /// txid that caused this state change, if known. Currently always + /// `None`: the schema + trigger machinery (migration 0009) supports + /// it via the `zkcoins.account_commit_txid` GUC but no Rust caller + /// sets that GUC today. Surfaced via `pending_inscriptions.commit_txid` + /// once a publisher path threads it through. + pub commit_txid: Option>, + /// `observed_inscriptions.block_height` for the matching commit, if + /// the scanner has integrated it. `None` while `commit_txid` is also + /// `None`. + pub block_height: Option, + /// `pending_inscriptions.status` for the matching commit (`pending`, + /// `commit_broadcast`, `reveal_broadcast`, `complete`, `failed`). + /// `None` while `commit_txid` is `None`. + pub pending_status: Option, +} + +/// Fetch the `limit` most recent user-facing `account_history` rows for +/// `address` (newest first, skipping the first `offset` rows) together +/// with the filtered `total` row count for pagination. The `address` +/// argument is the 32-byte raw form (BYTEA) — callers convert the +/// user-supplied hex via the same path `/api/balance` uses. +/// +/// Only rows whose `source` is in `('mint','send','receive')` are +/// counted or returned. `scanner` and `recovery` rows are internal +/// mutations the user did not initiate and the handler refuses to +/// surface them; pushing the filter into SQL means the page size and +/// the `total` agree (a post-fetch filter would drop rows after the +/// LIMIT and break pagination math). +/// +/// The two LEFT JOINs surface block_height + status when (and only +/// when) a future caller populates `account_history.triggering_commit_txid`. +/// Today both joined columns are always NULL; see +/// [`AccountHistoryRow::commit_txid`] for the rationale. +/// +/// `limit` and `offset` are caller-validated `i64`s (the handler clamps +/// `limit` to `[1, 200]` and rejects negative values upstream); they +/// bind directly into the query via `$2` / `$3`. +/// +/// Returns `(rows, total)`. `total` is the filtered count — every row +/// of `rows` is counted in `total`, and `total >= rows.len()` always. +/// One round-trip via `COUNT(*) OVER()` so the handler has a single +/// DB error branch (closes the `list_account_history` dead-arm gap a +/// two-query layout would leave behind). +/// +/// TODO(zk-coins/node#159): thread `zkcoins.account_commit_txid` +/// GUC through the publisher / mint / send paths so +/// `triggering_commit_txid` lights up here and the LEFT JOINs start +/// returning data instead of always-NULL. +pub async fn list_account_history( + pool: &PgPool, + address: &[u8], + limit: i64, + offset: i64, +) -> sqlx::Result<(Vec, i64)> { + use sqlx::Row; + // Single round-trip: a `total` CTE counts the filtered rows, the + // `page` CTE selects the LIMIT/OFFSET slice with the joins, and we + // cross-join the total onto every row of the page. When the page is + // empty (offset past total, or no rows at all) the outer query + // returns a single sentinel row with `id = NULL` so the handler + // still learns the real total without a second query — no + // dead-error-branch problem from a two-query layout. + let rows = sqlx::query( + "WITH \ + total AS ( \ + SELECT COUNT(*)::BIGINT AS n FROM account_history \ + WHERE address = $1 \ + AND source IN ('mint','send','receive') \ + ), \ + page AS ( \ + SELECT ah.id, \ + EXTRACT(EPOCH FROM ah.changed_at)::BIGINT AS ts_secs, \ + ah.source, ah.prev_data, ah.new_data, \ + ah.triggering_commit_txid, \ + oi.block_height, \ + pi.status AS pending_status \ + FROM account_history ah \ + LEFT JOIN observed_inscriptions oi \ + ON oi.commit_txid = ah.triggering_commit_txid \ + LEFT JOIN pending_inscriptions pi \ + ON pi.commit_txid = ah.triggering_commit_txid \ + WHERE ah.address = $1 \ + AND ah.source IN ('mint','send','receive') \ + ORDER BY ah.changed_at DESC, ah.id DESC \ + LIMIT $2 OFFSET $3 \ + ) \ + SELECT p.id, p.ts_secs, p.source, p.prev_data, p.new_data, \ + p.triggering_commit_txid, p.block_height, p.pending_status, \ + t.n AS total \ + FROM total t \ + LEFT JOIN page p ON TRUE", + ) + .bind(address) + .bind(limit) + .bind(offset) + .fetch_all(pool) + .await?; + + // `total` is identical on every row (cross-join from the singleton + // CTE); read it once. If the page CTE yielded zero rows, the LEFT + // JOIN keeps a single sentinel row with `id IS NULL` — skip it when + // mapping to `AccountHistoryRow`s but still read `total` off it. + let total = rows.first().map(|r| r.get::("total")).unwrap_or(0); + let items = rows + .into_iter() + .filter_map(|r| { + // Sentinel-row guard: when the page CTE is empty, the outer + // SELECT still returns one row (from the `total` CTE) with + // every `p.*` column NULL. `id` is NOT NULL on real rows, + // so its absence flags the sentinel. + let id: Option = r.try_get("id").ok().flatten(); + let id = id?; + Some(AccountHistoryRow { + id, + timestamp_secs: r.get("ts_secs"), + source: r.get("source"), + prev_data: r.get("prev_data"), + new_data: r.get("new_data"), + commit_txid: r.get("triggering_commit_txid"), + block_height: r.get("block_height"), + pending_status: r.get("pending_status"), + }) + }) + .collect(); + Ok((items, total)) +} + #[cfg(test)] #[path = "db_tests.rs"] mod tests; diff --git a/node/src/db_tests.rs b/node/src/db_tests.rs index 649e6a8a..8ddfe56f 100644 --- a/node/src/db_tests.rs +++ b/node/src/db_tests.rs @@ -1248,3 +1248,145 @@ async fn get_inscription_summary_rejects_invalid_kind_in_row() { .expect_err("summary must reject bogus kind"); assert!(matches!(err, sqlx::Error::Decode(_))); } + +// ---- list_account_history (issue #153) ------------------------------------ + +/// Insert a synthetic `account_history` row directly so the test can +/// pin the timestamp ordering without racing the trigger-driven path. +async fn plant_history_row( + pool: &PgPool, + address: &[u8], + source: &str, + new_balance: u64, + seconds_ago: i64, +) { + use crate::account_node::Account; + let mut a = Account::new(); + a.balance = new_balance; + let new_data = bincode::serialize(&a).expect("serialize account"); + sqlx::query( + "INSERT INTO account_history \ + (address, prev_data, new_data, source, changed_at) \ + VALUES ($1, NULL, $2, $3, NOW() - ($4 || ' seconds')::INTERVAL)", + ) + .bind(address) + .bind(&new_data) + .bind(source) + .bind(seconds_ago.to_string()) + .execute(pool) + .await + .expect("insert account_history row"); +} + +#[tokio::test] +async fn list_account_history_empty_returns_zero_total() { + let (pool, _c) = setup_pool().await; + let address = [0xaau8; 32]; + let (rows, total) = list_account_history(&pool, &address[..], 50, 0) + .await + .expect("list returns Ok"); + assert!(rows.is_empty()); + assert_eq!(total, 0); +} + +#[tokio::test] +async fn list_account_history_orders_newest_first_and_paginates() { + let (pool, _c) = setup_pool().await; + let address = [0xbbu8; 32]; + // Plant rows at 30 s, 20 s, 10 s ago — list must order + // newest-first (10 s, 20 s, 30 s). + plant_history_row(&pool, &address[..], "mint", 100, 30).await; + plant_history_row(&pool, &address[..], "receive", 200, 20).await; + plant_history_row(&pool, &address[..], "send", 150, 10).await; + + let (page, total) = list_account_history(&pool, &address[..], 50, 0) + .await + .unwrap(); + assert_eq!(total, 3); + assert_eq!(page.len(), 3); + assert_eq!(page[0].source, "send", "newest first"); + assert_eq!(page[1].source, "receive"); + assert_eq!(page[2].source, "mint"); + + // Limit + offset + let (page, total) = list_account_history(&pool, &address[..], 1, 1) + .await + .unwrap(); + assert_eq!(page.len(), 1); + assert_eq!(page[0].source, "receive"); + assert_eq!(total, 3, "total stays consistent across pages"); + + // Offset past total + let (page, total) = list_account_history(&pool, &address[..], 10, 99) + .await + .unwrap(); + assert!(page.is_empty()); + assert_eq!( + total, 3, + "empty page still surfaces the real total (no second-query branch needed)" + ); + + // Other address never appears. + let other = [0xccu8; 32]; + let (page, total) = list_account_history(&pool, &other[..], 10, 0) + .await + .unwrap(); + assert!(page.is_empty()); + assert_eq!(total, 0); +} + +#[tokio::test] +async fn list_account_history_surfaces_blob_and_metadata() { + let (pool, _c) = setup_pool().await; + let address = [0xddu8; 32]; + plant_history_row(&pool, &address[..], "mint", 12_345, 1).await; + let (rows, total) = list_account_history(&pool, &address[..], 10, 0) + .await + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(total, 1); + let row = &rows[0]; + assert_eq!(row.source, "mint"); + assert!(row.prev_data.is_none(), "first INSERT has no prev_data"); + assert!(row.commit_txid.is_none()); + assert!(row.block_height.is_none()); + assert!(row.pending_status.is_none()); + assert!(row.timestamp_secs > 0, "timestamp epoch derived"); + // new_data round-trips through bincode -> Account + let decoded: crate::account_node::Account = + bincode::deserialize(&row.new_data).expect("decode Account"); + assert_eq!(decoded.balance, 12_345); +} + +#[tokio::test] +async fn list_account_history_filters_scanner_and_recovery_in_sql() { + // Scanner / recovery rows must be filtered in SQL — pushing the + // filter into the query is what keeps `total` and the page length + // honest (a post-fetch filter on the page would drop rows AFTER the + // LIMIT and break pagination math). Issue #153 round-2 review fix. + let (pool, _c) = setup_pool().await; + let address = [0xeeu8; 32]; + plant_history_row(&pool, &address[..], "scanner", 50, 50).await; + plant_history_row(&pool, &address[..], "mint", 100, 40).await; + plant_history_row(&pool, &address[..], "recovery", 110, 30).await; + plant_history_row(&pool, &address[..], "send", 90, 20).await; + plant_history_row(&pool, &address[..], "receive", 200, 10).await; + + let (rows, total) = list_account_history(&pool, &address[..], 50, 0) + .await + .unwrap(); + assert_eq!( + total, 3, + "total = filtered count (mint + send + receive), excludes scanner/recovery" + ); + assert_eq!(rows.len(), 3); + let sources: Vec<&str> = rows.iter().map(|r| r.source.as_str()).collect(); + // Newest-first ordering preserved within the filter. + assert_eq!(sources, vec!["receive", "send", "mint"]); + assert!( + sources + .iter() + .all(|s| matches!(*s, "mint" | "send" | "receive")), + "no scanner / recovery rows leak past the SQL filter" + ); +} diff --git a/node/src/router.rs b/node/src/router.rs index a53a367a..48eb2432 100644 --- a/node/src/router.rs +++ b/node/src/router.rs @@ -338,6 +338,267 @@ pub struct AddressesResponse { addresses: Vec, } +// ----- /api/history (issue #153) ------------------------------------------ + +/// Default page size when `/api/history?limit` is omitted. +pub(crate) const HISTORY_DEFAULT_LIMIT: i64 = 50; +/// Hard cap on `/api/history?limit`. Anything outside `[1, MAX]` is a +/// 400 — clamping silently was rejected as a footgun (callers that pass +/// `limit=1000` should learn about the cap, not get an unexplained 200 +/// with 200 rows). +pub(crate) const HISTORY_MAX_LIMIT: i64 = 200; + +/// `?address=&limit=&offset=` query for `GET /api/history`. All three +/// are parsed via the typed `Query` extractor so axum surfaces a 400 on +/// a non-integer `limit` / `offset` without the handler having to +/// re-parse. +#[derive(Deserialize)] +pub(crate) struct HistoryQuery { + pub address: Option, + pub limit: Option, + pub offset: Option, +} + +/// One entry in the `/api/history` response. Field names match the +/// issue #153 contract verbatim; `null`-able fields use `Option` +/// with `serialize_with = Some` so the wire shape stays +/// `"field": null` rather than the field being elided. +/// +/// Memo / counterparty / block_height stay `null` today: the current +/// schema does not store the recipient address per-mutation +/// (`account_history` is keyed on the address that changed, not the +/// counterparty), no memo column exists, and `triggering_commit_txid` +/// is unset by every Rust caller — see [`db::AccountHistoryRow::commit_txid`] +/// for the GUC-plumbing story. +#[derive(Serialize)] +pub struct HistoryItem { + /// Server-internal monotonic id. Always set — sourced from + /// `account_history.id`. + pub id: i64, + /// Bitcoin txid (lower-case hex, 64 chars) of the commit inscription + /// for this state change, once the publisher has broadcast it. + /// `null` while no commit_txid is linked to the row. + pub txid: Option, + /// Unix epoch in seconds of the state change. + pub timestamp: i64, + /// `"send"`, `"receive"`, or `"mint"`. `scanner` / `recovery` + /// `account_history` rows are filtered out before the handler maps + /// to this enum. + pub direction: &'static str, + /// Absolute balance delta in sats (`|new_balance − prev_balance|`). + /// For a `receive` / `mint` this is the amount credited; for a + /// `send` this is the amount debited. + pub amount: u64, + /// Counterparty address (lower-case hex, 64 chars). Always `null` + /// in the current schema — see the type-level doc-comment. + pub counterparty: Option, + /// `"pending"`, `"confirmed"`, or `"failed"`. Every persisted + /// `account_history` row reflects a state mutation that committed + /// in Postgres, so the default is `"confirmed"`; the alternative + /// values surface once the `pending_inscriptions` join lights up. + pub status: &'static str, + /// Bitcoin block height that contains the commit inscription, or + /// `null` while the scanner has not integrated it (and while the + /// `commit_txid` link is missing). + pub block_height: Option, + /// Free-text memo attached to the operation. Always `null` — no + /// memo column exists in the current schema. + pub memo: Option, +} + +/// Paginated wrapper around [`HistoryItem`]. `total` is the unfiltered +/// count for the queried address (not the count of returned `items`) +/// so the caller can drive pagination without a separate query. +#[derive(Serialize)] +pub struct HistoryResponse { + pub items: Vec, + pub total: i64, + pub limit: i64, + pub offset: i64, +} + +/// JSON envelope returned by the validation-failure branches of +/// `get_history_handler`. Distinct from the existing `SendCoinResponse` +/// shape because `/api/history` is a read endpoint with no `success` / +/// `proof_id` machinery — a flat `{ "error": "..." }` is the contract +/// the issue documents. +#[derive(Serialize)] +pub struct HistoryErrorResponse { + pub error: &'static str, +} + +/// Decode the 64-char (or 64 char + 0x prefix) hex `address` argument +/// into the raw 32-byte form `account_history.address` is keyed on. +/// Reuses the exact decode + length rules `get_balance_handler` applies +/// — `Err` on non-hex characters or a length that does not unpack to +/// 32 bytes. +pub(crate) fn decode_history_address(raw: &str) -> Result<[u8; 32], &'static str> { + let bytes = hex::decode(raw.trim_start_matches("0x")).map_err(|_| "Invalid address hex")?; + if bytes.len() != 32 { + return Err("Address must be 32 bytes (64 hex chars)"); + } + let mut out = [0u8; 32]; + out.copy_from_slice(&bytes); + Ok(out) +} + +/// Map an `account_history.source` string into the user-facing +/// `direction` enum. Returns `None` for the `scanner` and `recovery` +/// sources, which are internal mutations the user did not initiate and +/// the handler filters out before serialising. +pub(crate) fn map_history_direction(source: &str) -> Option<&'static str> { + match source { + "mint" => Some("mint"), + "send" => Some("send"), + "receive" => Some("receive"), + // `scanner` and `recovery` are internal replays / operator-only + // mutations. Surface a `None` so the handler skips them. + _ => None, + } +} + +/// Recover the `balance` field out of a bincode-serialised +/// [`crate::account_node::Account`] blob. Returns `None` if the bytes +/// fail to round-trip — defensive, the handler treats a decode failure +/// as a missing prior balance (so the delta collapses to the absolute +/// new balance instead of producing a fabricated number). +pub(crate) fn balance_from_account_blob(blob: &[u8]) -> Option { + bincode::deserialize::(blob) + .ok() + .map(|a| a.balance) +} + +/// Typed mirror of the `pending_inscriptions.status` CHECK constraint +/// (migration 0003: `constructed`, `commit_broadcast`, `reveal_broadcast`, +/// `complete`, `failed`). Parsed via [`PendingInscriptionStatus::from_db_str`] +/// so the `match` in [`history_row_to_item`] can be exhaustive and a +/// future schema state addition forces compile-time attention — a plain +/// `match row.pending_status.as_deref()` on a `String` can't enforce +/// that. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum PendingInscriptionStatus { + Constructed, + CommitBroadcast, + RevealBroadcast, + Complete, + Failed, +} + +impl PendingInscriptionStatus { + /// Map a raw `pending_inscriptions.status` string to the enum. + /// Returns `None` for an unrecognised value — Postgres's CHECK + /// constraint prevents that in practice, but if it ever leaks the + /// handler degrades to `pending` rather than crash. + pub(crate) fn from_db_str(s: &str) -> Option { + match s { + "constructed" => Some(Self::Constructed), + "commit_broadcast" => Some(Self::CommitBroadcast), + "reveal_broadcast" => Some(Self::RevealBroadcast), + "complete" => Some(Self::Complete), + "failed" => Some(Self::Failed), + _ => None, + } + } +} + +/// Convert one [`db::AccountHistoryRow`] into a wire [`HistoryItem`]. +/// Returns `None` if the row's source is internal (`scanner` / +/// `recovery`), if the `new_data` blob fails to decode, or if a +/// non-null `prev_data` blob fails to decode (treating that as zero +/// would fabricate a full-balance delta — see the inner `match` for +/// the warn log). +pub(crate) fn history_row_to_item(row: &crate::db::AccountHistoryRow) -> Option { + let direction = map_history_direction(&row.source)?; + let new_balance = balance_from_account_blob(&row.new_data)?; + // `prev_data` is `None` on the first INSERT for an address — treat + // that as a from-zero delta so the very first mint / receive + // surfaces the full credit instead of disappearing. A `Some(blob)` + // that fails to decode is *not* the same as `None`: silently + // collapsing to zero would fabricate the full new balance as the + // delta. Drop the row instead and log a warn so an operator can + // notice the schema drift. + let prev_balance = match row.prev_data.as_deref() { + None => 0, + Some(blob) => match balance_from_account_blob(blob) { + Some(b) => b, + None => { + let blob_len = blob.len(); + tracing::warn!( + "history_row_to_item: row id={} address has un-decodable prev_data blob (len={}); dropping row to avoid fabricating a full-balance delta", + row.id, + blob_len, + ); + return None; + } + }, + }; + // Absolute delta — sends are debits (prev > new), mints / receives + // are credits (new > prev). The `direction` field already encodes + // the sign for the caller. + let amount = new_balance.max(prev_balance) - new_balance.min(prev_balance); + + // Wire status derived from `pending_inscriptions.status` (the + // authoritative state machine) joined to `observed_inscriptions` + // for the post-broadcast on-chain confirmation. A DB-committed + // `account_history` row only proves a server-side state change — + // *not* an on-chain confirmation — so the default before any + // matching inscription row exists is `pending`, not `confirmed`. + // + // The inner `match pending` is exhaustive over the + // [`PendingInscriptionStatus`] enum (which mirrors migration 0003's + // CHECK constraint). A future state added to the enum will fail to + // compile here — no silent `_ => "pending"` catch-all. + // + // The unknown-string case is handled separately via + // `from_db_str` returning `None`: Postgres's CHECK constraint + // already prevents that, but if it ever leaks we warn and degrade + // to `pending` rather than crash. + let pending_enum = row + .pending_status + .as_deref() + .map(|s| (s, PendingInscriptionStatus::from_db_str(s))); + let status = match pending_enum { + Some((_, Some(p))) => match p { + PendingInscriptionStatus::Complete => "confirmed", + PendingInscriptionStatus::Failed => "failed", + PendingInscriptionStatus::Constructed + | PendingInscriptionStatus::CommitBroadcast + | PendingInscriptionStatus::RevealBroadcast => "pending", + }, + Some((raw, None)) => { + tracing::warn!( + "history_row_to_item: unknown pending_inscriptions.status={:?} (id={}); defaulting to pending", + raw, + row.id, + ); + "pending" + } + // No pending_inscriptions row but the scanner has observed the + // inscription on-chain — it's confirmed even though we lost the + // pending row (the resumer prunes `complete` rows after a + // safe-depth threshold). + None if row.block_height.is_some() => "confirmed", + // Neither pending nor observed — the on-chain side is not yet + // known to us; the DB write alone does not warrant `confirmed`. + None => "pending", + }; + + Some(HistoryItem { + id: row.id, + txid: row.commit_txid.as_deref().map(hex::encode), + timestamp: row.timestamp_secs, + direction, + amount, + // TODO(zk-coins/node#160): capture `counterparty_address` per + // `account_history` row (schema change) so this stops being + // unconditionally null. + counterparty: None, + status, + block_height: row.block_height, + memo: None, + }) +} + #[derive(Deserialize)] pub struct SendCoinRequest { account_address: String, @@ -788,6 +1049,121 @@ async fn get_balance_handler( } } +/// `GET /api/history?address=&limit=&offset=` — paginated +/// per-address transaction history. Implements issue #153. +/// +/// Sort order is fixed `ORDER BY changed_at DESC` (newest first); the +/// matching test in `router_tests.rs` pins this so a future caller +/// cannot silently flip the order. +/// +/// Validation contract (all return HTTP 422 with a +/// [`HistoryErrorResponse`] — mirrors the `/api/balance` shape so the +/// whole read surface uses the same status for malformed input): +/// * `address` missing. +/// * `address` not valid 32-byte hex. +/// * `limit` outside `[1, 200]` (the issue's max=200 rule). `limit=0` +/// is rejected because a successful response with zero items would +/// be indistinguishable from "no rows", masking the misuse. +/// * `offset` negative. +/// +/// A successful response with `offset >= total` returns +/// `items: [], total: N` so the caller can detect end-of-list without +/// a second round-trip. +/// +/// Persistence: pure read from `account_history` (joined with +/// `observed_inscriptions` + `pending_inscriptions` for the future +/// txid/block_height/status link — see [`db::AccountHistoryRow`] for +/// the today-vs-tomorrow story). No new schema work. +async fn get_history_handler( + State(state): State, + axum::extract::Query(query): axum::extract::Query, +) -> impl IntoResponse { + // Resolve defaults first so the rest of the validation block can + // assume concrete values. `Option::get().copied().unwrap_or(...)` + // would also work but the field is already an `Option` from + // the typed extractor — `unwrap_or` is the same shape. + let limit = query.limit.unwrap_or(HISTORY_DEFAULT_LIMIT); + let offset = query.offset.unwrap_or(0); + + // --- validation --- + let address_hex = match query.address.as_deref() { + Some(s) if !s.is_empty() => s, + _ => { + return ( + StatusCode::UNPROCESSABLE_ENTITY, + Json(HistoryErrorResponse { + error: "Missing required `address` query parameter", + }), + ) + .into_response(); + } + }; + let address_bytes = match decode_history_address(address_hex) { + Ok(b) => b, + Err(msg) => { + return ( + StatusCode::UNPROCESSABLE_ENTITY, + Json(HistoryErrorResponse { error: msg }), + ) + .into_response(); + } + }; + if !(1..=HISTORY_MAX_LIMIT).contains(&limit) { + return ( + StatusCode::UNPROCESSABLE_ENTITY, + Json(HistoryErrorResponse { + error: "limit must be in [1, 200]", + }), + ) + .into_response(); + } + if offset < 0 { + return ( + StatusCode::UNPROCESSABLE_ENTITY, + Json(HistoryErrorResponse { + error: "offset must be non-negative", + }), + ) + .into_response(); + } + + // --- DB read --- + // Single round-trip: page rows + filtered total in one query so the + // handler carries a single DB error branch. + let (rows, total) = + match db::list_account_history(&state.pool, &address_bytes, limit, offset).await { + Ok(t) => t, + Err(e) => { + tracing::warn!("get_history_handler: list query failed: {}", e); + return ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(HistoryErrorResponse { + error: "Database error while reading history", + }), + ) + .into_response(); + } + }; + + // Defense-in-depth safety net: the SQL already filters to + // mint/send/receive, so `filter_map` should never actually drop a + // row in normal operation. If it does, that's a schema drift bug — + // the post-fetch filter prevents a junk row from reaching the wire + // until someone fixes the SQL. + let items: Vec = rows.iter().filter_map(history_row_to_item).collect(); + + ( + StatusCode::OK, + Json(HistoryResponse { + items, + total, + limit, + offset, + }), + ) + .into_response() +} + #[cfg(feature = "address-list")] async fn get_address_handler(State(state): State) -> impl IntoResponse { let account_node = lock_or_recover(&state.account_node); @@ -1851,6 +2227,7 @@ struct RootResponse { struct RootEndpoints { info: &'static str, balance: &'static str, + history: &'static str, send: &'static str, receive: &'static str, commit: &'static str, @@ -1872,6 +2249,7 @@ async fn root_handler() -> impl IntoResponse { endpoints: RootEndpoints { info: "GET /api/info", balance: "GET /api/balance?address={hex}", + history: "GET /api/history?address={hex}&limit={n}&offset={n}", send: "POST /api/send", receive: "POST /api/receive", commit: "POST /api/commit", @@ -2243,6 +2621,7 @@ pub(crate) fn create_router(state: AppState) -> Router { .route("/health/publisher", get(publisher_health_handler)) .route("/api/info", get(info_handler)) .route("/api/balance", get(get_balance_handler)) + .route("/api/history", get(get_history_handler)) .route("/api/send", post(send_coin_handler)) .route("/api/receive", post(receive_coin_handler)) .route("/api/proof/:id", get(get_proof_handler)) diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index 731672d3..90e9143c 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -6307,3 +6307,662 @@ async fn commit_handler_atomic_tx_rollback_leaves_state_and_row_consistent() { .await .unwrap(); } + +// ======================================================================= +// GET /api/history — paginated per-address history (issue #153) +// +// The handler is read-only against `account_history`; tests below cover +// both the validation branches (dead pool — handler never reaches the +// query) and the live-DB branches (live Postgres 17 container, accounts +// upserted via `upsert_account_with_source` so the migration-0008 +// trigger fills the history rows). +// ======================================================================= + +/// Spin up a Postgres 17 testcontainer and return a migrated pool — +/// shared shape with the readiness / r2-probe live tests above. The +/// container handle must outlive the pool (testcontainers tears the +/// container down on `Drop`). +async fn history_live_pool() -> ( + Arc, + testcontainers::ContainerAsync, +) { + use testcontainers::{runners::AsyncRunner, ImageExt}; + use testcontainers_modules::postgres::Postgres; + + let pg_container = Postgres::default() + .with_tag("17") + .start() + .await + .expect("failed to start postgres container"); + let host = pg_container + .get_host() + .await + .expect("failed to get container host"); + let port = pg_container + .get_host_port_ipv4(5432) + .await + .expect("failed to get container port"); + let url = format!("postgres://postgres:postgres@{}:{}/postgres", host, port); + let pool = Arc::new( + crate::db::connect_and_migrate(&url) + .await + .expect("connect_and_migrate failed"), + ); + (pool, pg_container) +} + +/// Seed an `Account { balance, .. }` row for `address` via the +/// `upsert_account_with_source` path so the migration-0008 trigger +/// writes the matching `account_history` row with the requested +/// `source`. Returns the bincode bytes for the caller to chain a +/// second upsert that mutates the same account (the trigger captures +/// `prev_data` from the previous row). +async fn seed_account_history( + pool: &sqlx::PgPool, + address: &[u8; 32], + balance: u64, + source: &str, +) -> Vec { + let mut acct = Account::new(); + acct.balance = balance; + let bytes = bincode::serialize(&acct).expect("Account serializable"); + crate::db::upsert_account_with_source(pool, address.as_slice(), &bytes, source) + .await + .expect("upsert seeded account"); + bytes +} + +#[tokio::test] +async fn history_missing_address_returns_422() { + let req = Request::get("/api/history").body(Body::empty()).unwrap(); + let (status, body) = send_request(req).await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert!( + v["error"].as_str().unwrap_or("").contains("address"), + "expected address-related error, got {}", + body + ); +} + +#[tokio::test] +async fn history_empty_address_returns_422() { + // `?address=` (empty string) is treated as missing — same 422 path + // as the missing-param case, mirroring `/api/balance`. + let req = Request::get("/api/history?address=") + .body(Body::empty()) + .unwrap(); + let (status, _body) = send_request(req).await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); +} + +#[tokio::test] +async fn history_invalid_hex_returns_422() { + let req = Request::get("/api/history?address=not_hex") + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request(req).await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert!(v["error"] + .as_str() + .unwrap_or("") + .to_lowercase() + .contains("hex")); +} + +#[tokio::test] +async fn history_wrong_length_returns_422() { + // 16 bytes worth of hex — decoded successfully but not 32 bytes. + let address = format!("0x{}", "ab".repeat(16)); + let req = Request::get(format!("/api/history?address={}", address)) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request(req).await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert!(v["error"].as_str().unwrap_or("").contains("32 bytes")); +} + +#[tokio::test] +async fn history_limit_zero_returns_422() { + let address = "00".repeat(32); + let req = Request::get(format!("/api/history?address={}&limit=0", address)) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request(req).await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert!(v["error"].as_str().unwrap_or("").contains("limit")); +} + +#[tokio::test] +async fn history_limit_above_max_returns_422() { + let address = "00".repeat(32); + let req = Request::get(format!( + "/api/history?address={}&limit={}", + address, + HISTORY_MAX_LIMIT + 1 + )) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request(req).await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert!(v["error"].as_str().unwrap_or("").contains("limit")); +} + +#[tokio::test] +async fn history_negative_offset_returns_422() { + let address = "00".repeat(32); + let req = Request::get(format!("/api/history?address={}&offset=-1", address)) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request(req).await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert!(v["error"].as_str().unwrap_or("").contains("offset")); +} + +#[tokio::test] +async fn history_non_integer_limit_returns_400() { + // axum's typed `Query` extractor rejects a non-integer value with + // 400 (framework-level) before the handler runs — distinct from the + // 422s the handler emits for its own validation branches. + let address = "00".repeat(32); + let req = Request::get(format!("/api/history?address={}&limit=abc", address)) + .body(Body::empty()) + .unwrap(); + let (status, _body) = send_request(req).await; + assert_eq!(status, StatusCode::BAD_REQUEST); +} + +#[tokio::test] +async fn history_db_error_returns_500() { + // `test_state()` uses `dead_pool()` — the single + // `list_account_history` query fails fast and the handler surfaces + // 500 + the documented error string. Collapsing count + list into + // one query (round-2 fix) removes the previous dead-arm gap. + let address = "00".repeat(32); + let req = Request::get(format!("/api/history?address={}", address)) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request(req).await; + assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert!( + v["error"] + .as_str() + .unwrap_or("") + .to_lowercase() + .contains("database"), + "expected database error, got {}", + body + ); +} + +#[tokio::test] +async fn history_empty_result_returns_ok_with_zero_total() { + let (pool, _pg) = history_live_pool().await; + let state = live_test_state(pool); + let address = "ab".repeat(32); + let req = Request::get(format!("/api/history?address=0x{}", address)) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request_with_state(state, req).await; + assert_eq!(status, StatusCode::OK, "body={}", body); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert_eq!(v["total"], 0); + assert_eq!(v["limit"], HISTORY_DEFAULT_LIMIT); + assert_eq!(v["offset"], 0); + assert_eq!(v["items"].as_array().unwrap().len(), 0); +} + +#[tokio::test] +async fn history_happy_path_returns_items_newest_first() { + let (pool, _pg) = history_live_pool().await; + let address: [u8; 32] = [7u8; 32]; + + // Three mutations on the same address: 0 -> 100 (mint), + // 100 -> 250 (receive), 250 -> 150 (send). + seed_account_history(&pool, &address, 100, "mint").await; + seed_account_history(&pool, &address, 250, "receive").await; + seed_account_history(&pool, &address, 150, "send").await; + + let state = live_test_state(pool); + let req = Request::get(format!("/api/history?address=0x{}", hex::encode(address))) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request_with_state(state, req).await; + assert_eq!(status, StatusCode::OK, "body={}", body); + + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert_eq!( + v["total"], 3, + "total must reflect every account_history row" + ); + let items = v["items"].as_array().expect("items array"); + assert_eq!(items.len(), 3, "all three rows returned with default limit"); + + // Newest first: send (150), receive (250), mint (100). + assert_eq!(items[0]["direction"], "send"); + assert_eq!(items[0]["amount"], 100, "250 -> 150 is a 100 delta"); + // No pending_inscriptions row and no observed_inscriptions row for + // this address (the seed path doesn't thread the commit_txid GUC), + // so the wire status is `pending` — the DB write alone is not an + // on-chain confirmation. + assert_eq!(items[0]["status"], "pending"); + assert!( + items[0]["txid"].is_null(), + "txid is null pre-broadcast link" + ); + assert!(items[0]["counterparty"].is_null()); + assert!(items[0]["block_height"].is_null()); + assert!(items[0]["memo"].is_null()); + + assert_eq!(items[1]["direction"], "receive"); + assert_eq!(items[1]["amount"], 150, "100 -> 250 is a 150 delta"); + + assert_eq!(items[2]["direction"], "mint"); + assert_eq!(items[2]["amount"], 100, "0 -> 100 is a 100 delta"); + + // id field always present, monotonic descending (newest = highest id) + let id0 = items[0]["id"].as_i64().expect("id is i64"); + let id1 = items[1]["id"].as_i64().expect("id is i64"); + let id2 = items[2]["id"].as_i64().expect("id is i64"); + assert!(id0 > id1 && id1 > id2, "ids are monotonic descending"); +} + +#[tokio::test] +async fn history_pagination_offset_beyond_total_returns_empty_items_with_total() { + let (pool, _pg) = history_live_pool().await; + let address: [u8; 32] = [9u8; 32]; + seed_account_history(&pool, &address, 100, "mint").await; + seed_account_history(&pool, &address, 200, "receive").await; + + let state = live_test_state(pool); + let req = Request::get(format!( + "/api/history?address=0x{}&limit=10&offset=99", + hex::encode(address) + )) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request_with_state(state, req).await; + assert_eq!(status, StatusCode::OK, "body={}", body); + + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert_eq!(v["total"], 2, "total still reflects the seeded rows"); + assert_eq!(v["limit"], 10); + assert_eq!(v["offset"], 99); + assert_eq!( + v["items"].as_array().unwrap().len(), + 0, + "offset past total -> empty page" + ); +} + +#[tokio::test] +async fn history_limit_clamps_page_size() { + let (pool, _pg) = history_live_pool().await; + let address: [u8; 32] = [11u8; 32]; + // Five rows. + for (i, src) in ["mint", "receive", "send", "receive", "send"] + .iter() + .enumerate() + { + seed_account_history(&pool, &address, 100 + 50 * i as u64, src).await; + } + let state = live_test_state(pool); + let req = Request::get(format!( + "/api/history?address=0x{}&limit=2", + hex::encode(address) + )) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request_with_state(state, req).await; + assert_eq!(status, StatusCode::OK, "body={}", body); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert_eq!(v["total"], 5); + assert_eq!(v["limit"], 2); + assert_eq!(v["items"].as_array().unwrap().len(), 2); +} + +#[tokio::test] +async fn history_scanner_source_is_filtered_out() { + // `scanner` and `recovery` are internal mutations the user did not + // initiate; the SQL pushes the filter so they neither count toward + // `total` nor appear in `items`. A post-fetch filter (the previous + // behaviour) broke pagination — `total` over-counted and page sizes + // would have come back short of the requested `limit`. + let (pool, _pg) = history_live_pool().await; + let address: [u8; 32] = [13u8; 32]; + seed_account_history(&pool, &address, 100, "scanner").await; + seed_account_history(&pool, &address, 200, "mint").await; + + let state = live_test_state(pool); + let req = Request::get(format!("/api/history?address=0x{}", hex::encode(address))) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request_with_state(state, req).await; + assert_eq!(status, StatusCode::OK, "body={}", body); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert_eq!( + v["total"], 1, + "total reflects the filtered count (scanner row excluded)" + ); + let items = v["items"].as_array().unwrap(); + assert_eq!(items.len(), 1); + assert_eq!(items[0]["direction"], "mint"); +} + +#[tokio::test] +async fn history_pagination_walks_mixed_source_dataset_consistently() { + // Plant a mixed-source dataset and walk pagination across multiple + // pages. The client must see every user-facing row exactly once + // across consecutive pages, with `total` matching the cumulative + // page sizes — the SQL filter is what makes this true (a post-fetch + // filter would have left holes in pages and a `total` that no + // page-walk can hit). + let (pool, _pg) = history_live_pool().await; + let address: [u8; 32] = [17u8; 32]; + // Plant in chronological order; the handler returns newest-first. + // 4 user-facing rows (mint, receive, send, receive) interleaved with + // 3 internal rows (scanner, scanner, recovery) — the internal rows + // must never appear and must never count toward `total`. + seed_account_history(&pool, &address, 100, "mint").await; + seed_account_history(&pool, &address, 110, "scanner").await; + seed_account_history(&pool, &address, 250, "receive").await; + seed_account_history(&pool, &address, 260, "scanner").await; + seed_account_history(&pool, &address, 150, "send").await; + seed_account_history(&pool, &address, 160, "recovery").await; + seed_account_history(&pool, &address, 300, "receive").await; + + let state = live_test_state(pool); + let mut seen_directions: Vec = Vec::new(); + let mut total_seen_on_first_page: Option = None; + let mut offset: i64 = 0; + let limit: i64 = 2; + loop { + let req = Request::get(format!( + "/api/history?address=0x{}&limit={}&offset={}", + hex::encode(address), + limit, + offset + )) + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request_with_state(state.clone(), req).await; + assert_eq!(status, StatusCode::OK, "body={}", body); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + let total = v["total"].as_i64().expect("total i64"); + if total_seen_on_first_page.is_none() { + total_seen_on_first_page = Some(total); + } else { + assert_eq!( + total_seen_on_first_page, + Some(total), + "total must stay constant across pages" + ); + } + let items = v["items"].as_array().expect("items array"); + if items.is_empty() { + break; + } + // The page must never come back short of the requested `limit` + // unless we've hit the end — that's the property the post-fetch + // filter violated. + if (offset + items.len() as i64) < total { + assert_eq!( + items.len() as i64, + limit, + "page must be full while more rows remain (post-fetch filter would shrink this)" + ); + } + for it in items { + let d = it["direction"].as_str().expect("direction str").to_string(); + assert!( + matches!(d.as_str(), "mint" | "send" | "receive"), + "internal sources must never reach the wire, got {}", + d + ); + seen_directions.push(d); + } + offset += items.len() as i64; + if offset >= total { + break; + } + } + let total = total_seen_on_first_page.expect("at least one page seen"); + assert_eq!(total, 4, "filtered total = 4 user-facing rows"); + assert_eq!( + seen_directions.len() as i64, + total, + "pagination walk yields exactly `total` rows" + ); + // Newest-first: last receive (300), send (150), receive (250), mint (100). + assert_eq!(seen_directions, vec!["receive", "send", "receive", "mint"]); +} + +// --- Pure-function coverage for the helpers -------------------------------- + +#[test] +fn decode_history_address_accepts_with_and_without_0x_prefix() { + let plain = "ab".repeat(32); + let prefixed = format!("0x{}", plain); + assert!(decode_history_address(&plain).is_ok()); + assert!(decode_history_address(&prefixed).is_ok()); +} + +#[test] +fn decode_history_address_rejects_short_input() { + let bad = "ab".repeat(16); + let err = decode_history_address(&bad).unwrap_err(); + assert!(err.contains("32 bytes")); +} + +#[test] +fn decode_history_address_rejects_non_hex() { + let err = decode_history_address("zzzz").unwrap_err(); + assert!(err.to_lowercase().contains("hex")); +} + +#[test] +fn map_history_direction_covers_all_branches() { + assert_eq!(map_history_direction("mint"), Some("mint")); + assert_eq!(map_history_direction("send"), Some("send")); + assert_eq!(map_history_direction("receive"), Some("receive")); + assert_eq!(map_history_direction("scanner"), None); + assert_eq!(map_history_direction("recovery"), None); + assert_eq!(map_history_direction("anything-else"), None); +} + +#[test] +fn balance_from_account_blob_round_trips() { + let mut a = Account::new(); + a.balance = 42_000; + let bytes = bincode::serialize(&a).unwrap(); + assert_eq!(balance_from_account_blob(&bytes), Some(42_000)); + // Garbage bytes -> None (defensive). + assert!(balance_from_account_blob(&[0u8, 1, 2, 3]).is_none()); +} + +#[test] +fn history_row_to_item_handles_first_row_with_no_prev_data() { + let mut a = Account::new(); + a.balance = 5_000; + let new_bytes = bincode::serialize(&a).unwrap(); + let row = crate::db::AccountHistoryRow { + id: 42, + timestamp_secs: 1_700_000_000, + source: "mint".to_string(), + prev_data: None, + new_data: new_bytes, + commit_txid: None, + block_height: None, + pending_status: None, + }; + let item = history_row_to_item(&row).expect("item produced"); + assert_eq!(item.id, 42); + assert_eq!(item.direction, "mint"); + assert_eq!( + item.amount, 5_000, + "from-zero credit is the full new balance" + ); + // No pending_inscriptions row + no observed_inscriptions row = the + // on-chain side is not yet known. DB-committed alone is NOT a + // confirmation; wire status defaults to `pending`. + assert_eq!(item.status, "pending"); + assert!(item.txid.is_none()); +} + +#[test] +fn history_row_to_item_drops_unknown_source() { + let mut a = Account::new(); + a.balance = 1; + let row = crate::db::AccountHistoryRow { + id: 1, + timestamp_secs: 0, + source: "scanner".to_string(), + prev_data: None, + new_data: bincode::serialize(&a).unwrap(), + commit_txid: None, + block_height: None, + pending_status: None, + }; + assert!(history_row_to_item(&row).is_none()); +} + +#[test] +fn history_row_to_item_drops_undecodable_new_data() { + let row = crate::db::AccountHistoryRow { + id: 1, + timestamp_secs: 0, + source: "mint".to_string(), + prev_data: None, + new_data: vec![0xff; 4], // not a valid bincode Account + commit_txid: None, + block_height: None, + pending_status: None, + }; + assert!(history_row_to_item(&row).is_none()); +} + +#[test] +fn history_row_to_item_maps_pending_status_to_wire_status() { + let mut a = Account::new(); + a.balance = 100; + let bytes = bincode::serialize(&a).unwrap(); + let mk = |status: Option<&str>, block_height: Option| crate::db::AccountHistoryRow { + id: 1, + timestamp_secs: 0, + source: "send".to_string(), + prev_data: Some(bincode::serialize(&Account::new()).unwrap()), + new_data: bytes.clone(), + commit_txid: Some(vec![0xab; 32]), + block_height, + pending_status: status.map(str::to_string), + }; + // Every enum variant the migration-0003 CHECK constraint allows. + assert_eq!( + history_row_to_item(&mk(Some("failed"), Some(1))) + .unwrap() + .status, + "failed" + ); + assert_eq!( + history_row_to_item(&mk(Some("complete"), Some(1))) + .unwrap() + .status, + "confirmed" + ); + assert_eq!( + history_row_to_item(&mk(Some("constructed"), None)) + .unwrap() + .status, + "pending" + ); + assert_eq!( + history_row_to_item(&mk(Some("commit_broadcast"), None)) + .unwrap() + .status, + "pending" + ); + assert_eq!( + history_row_to_item(&mk(Some("reveal_broadcast"), None)) + .unwrap() + .status, + "pending" + ); + // No pending row + no observed row -> on-chain side is unknown -> pending. + assert_eq!( + history_row_to_item(&mk(None, None)).unwrap().status, + "pending" + ); + // No pending row but observed_inscriptions has a block height -> confirmed. + assert_eq!( + history_row_to_item(&mk(None, Some(42))).unwrap().status, + "confirmed" + ); + // Unknown pending_inscriptions.status (defensive — CHECK prevents + // it in practice). The handler degrades to `pending` and logs. + assert_eq!( + history_row_to_item(&mk(Some("nonsense_state"), None)) + .unwrap() + .status, + "pending" + ); + // commit_txid -> hex-encoded; block_height surfaced verbatim. + let item = history_row_to_item(&mk(Some("complete"), Some(123_456))).unwrap(); + assert_eq!(item.txid.as_deref(), Some("ab".repeat(32).as_str())); + assert_eq!(item.block_height, Some(123_456)); +} + +#[test] +fn history_row_to_item_drops_undecodable_prev_data() { + // A `Some(blob)` that fails to bincode-decode is NOT the same as + // `None` (first INSERT). Silently treating it as zero would + // fabricate a full-balance delta — the row is dropped instead. + let mut a = Account::new(); + a.balance = 5_000; + let row = crate::db::AccountHistoryRow { + id: 7, + timestamp_secs: 0, + source: "send".to_string(), + prev_data: Some(vec![0xff; 4]), // not a valid bincode Account + new_data: bincode::serialize(&a).unwrap(), + commit_txid: None, + block_height: None, + pending_status: None, + }; + assert!( + history_row_to_item(&row).is_none(), + "un-decodable prev_data must drop the row, not pretend prev_balance = 0" + ); +} + +#[test] +fn pending_inscription_status_from_db_str_round_trips_every_variant() { + // Mirrors migration-0003 CHECK constraint. Adding a state to + // `PendingInscriptionStatus` without updating this list fails CI. + assert_eq!( + PendingInscriptionStatus::from_db_str("constructed"), + Some(PendingInscriptionStatus::Constructed) + ); + assert_eq!( + PendingInscriptionStatus::from_db_str("commit_broadcast"), + Some(PendingInscriptionStatus::CommitBroadcast) + ); + assert_eq!( + PendingInscriptionStatus::from_db_str("reveal_broadcast"), + Some(PendingInscriptionStatus::RevealBroadcast) + ); + assert_eq!( + PendingInscriptionStatus::from_db_str("complete"), + Some(PendingInscriptionStatus::Complete) + ); + assert_eq!( + PendingInscriptionStatus::from_db_str("failed"), + Some(PendingInscriptionStatus::Failed) + ); + assert_eq!(PendingInscriptionStatus::from_db_str("unknown"), None); +} diff --git a/node/tests/api_remote.rs b/node/tests/api_remote.rs index 041311ea..dcb3b03a 100644 --- a/node/tests/api_remote.rs +++ b/node/tests/api_remote.rs @@ -503,6 +503,131 @@ async fn balance_invalid_hex_returns_422() { ); } +// --------------------------------------------------------------------------- +// /api/history — paginated per-address history (issue #153) +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn history_missing_address_returns_422() { + let resp = http_client() + .get(url("/api/history")) + .send() + .await + .expect("GET /api/history (no params)"); + assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY); +} + +#[tokio::test] +async fn history_invalid_hex_returns_422() { + let resp = http_client() + .get(url("/api/history?address=not_hex")) + .send() + .await + .expect("GET /api/history (bad hex)"); + assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY); + let body: Value = resp.json().await.expect("history body JSON"); + assert!( + body["error"].as_str().is_some(), + "422 body must carry an `error` string" + ); +} + +#[tokio::test] +async fn history_limit_above_max_returns_422() { + let address = format!("0x{}", "00".repeat(32)); + let resp = http_client() + .get(url(&format!("/api/history?address={}&limit=201", address))) + .send() + .await + .expect("GET /api/history (oversize limit)"); + assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY); +} + +#[tokio::test] +async fn history_unknown_address_returns_empty_page() { + let address = format!("0x{}", "11".repeat(32)); + let resp = http_client() + .get(url(&format!("/api/history?address={}", address))) + .send() + .await + .expect("GET /api/history (unknown addr)"); + assert_eq!(resp.status(), StatusCode::OK); + let body: Value = resp.json().await.expect("body JSON"); + assert_eq!(body["total"], 0); + assert_eq!(body["offset"], 0); + assert_eq!(body["limit"], 50); + assert_eq!(body["items"].as_array().unwrap().len(), 0); +} + +/// Live contract round-trip: mint into a freshly-generated address, +/// then probe `/api/history` and assert that the credit lands on the +/// minted account as a `direction: "mint"` row whose `amount` matches +/// the mint size. +/// +/// This is the only `/api/history` test that performs a state-mutating +/// call; the bookkeeping mirrors `mint_roundtrip_lands_balance_and_proof` +/// so the suite stays race-free against parallel runs. +#[tokio::test] +async fn history_after_mint_records_mint_row() { + let client = http_client(); + let alice = TestWallet::new(); + + assert_minting_balance_in_bounds(&client).await; + + let mint_resp = client + .post(url("/api/mint")) + .json(&json!({ + "account_address": alice.address_hex(), + "amount": MINT_AMOUNT, + })) + .send() + .await + .expect("POST /api/mint"); + assert_eq!(mint_resp.status(), StatusCode::OK); + let mint_body: Value = mint_resp.json().await.expect("mint body JSON"); + assert_eq!(mint_body["success"], Value::Bool(true)); + + // Wait for the mint credit to land on Alice's balance — same poll + // pattern the existing mint roundtrip uses; once balance >= MINT, + // the matching account_history row exists. + let _observed = poll_balance_at_least(&client, &alice.address_hex(), MINT_AMOUNT).await; + + let history_resp = client + .get(url(&format!( + "/api/history?address={}", + alice.address_hex() + ))) + .send() + .await + .expect("GET /api/history (post-mint)"); + assert_eq!(history_resp.status(), StatusCode::OK); + let body: Value = history_resp.json().await.expect("history body JSON"); + + assert!( + body["total"].as_i64().unwrap_or(0) >= 1, + "expected at least one history row, got body={}", + body + ); + let items = body["items"].as_array().expect("items array"); + assert!(!items.is_empty(), "items must not be empty"); + // Newest-first: the latest row is the mint credit we just landed. + let head = &items[0]; + assert_eq!(head["direction"], "mint"); + assert_eq!(head["amount"], MINT_AMOUNT); + // No Rust caller threads `zkcoins.account_commit_txid` through the + // mint path today (see the GUC TODO in db.rs::list_account_history), + // so `triggering_commit_txid` is NULL, the LEFT JOINs return NULL, + // and the on-chain side is not yet observable from `/api/history`: + // wire status is `pending`, not `confirmed`. The default flipped + // from `confirmed` -> `pending` in round 2 to stop misrepresenting + // DB-committed-only rows as on-chain confirmations. + assert_eq!(head["status"], "pending"); + assert!(head["id"].as_i64().is_some(), "id must be set"); + // Spec contract — these are nullable on the wire. + assert!(head["counterparty"].is_null() || head["counterparty"].is_string()); + assert!(head["memo"].is_null()); +} + #[tokio::test] async fn balance_wrong_length_returns_422() { // 16 bytes = 32 hex chars, the handler requires exactly 32 bytes From c67b3608b62be3db1b2a01c362faa940724833f0 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sun, 31 May 2026 21:21:21 +0200 Subject: [PATCH 2/9] =?UTF-8?q?docs:=20clarify=20operator=20language=20?= =?UTF-8?q?=E2=80=94=20api.zkcoins.app=20runs=20at=20zkcoins.app,=20not=20?= =?UTF-8?q?DFX=20(#158)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three spots in README.md leaked the DFX-as-operator framing: 1. Trust Model table row was "Yes — DFX runs the hosted node". The hosted node at api.zkcoins.app is operated by zkcoins.app (one of hopefully many such service providers). DFX is the underlying hosting / financial-services layer — invisible to wallet integrators and consumers of this README. 2. Configuration table description for ESPLORA_URL said "PRD: ... (DFX Mainnet stack)". Replaced with "On the api.zkcoins.app stack: PRD ..., DEV ...". Same meaning, correct attribution. 3. ESPLORA_WS_URL description analog — "on the DFX mempool/backend stack" → "self-hosted mempool/backend sidecar" (the relevant detail is "self-hosted vs external", not "whose hosting"). Memory: reference_zkcoins_org_structure documents the three-layer separation (zkCoins protocol / zkcoins.app service provider / DFX infra) so this distinction does not get muddled again. No code change. Pre-push sanity: fmt clean, clippy clean. --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e375642a..12f5c4fd 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ The **on-chain footprint stays private** — Plonky2 ensures that the public out | | Hosted (`api.zkcoins.app`) | Self-hosted | | --- | --- | --- | | On-chain privacy (vs. block explorers) | ✅ | ✅ | -| Operator sees plaintext transaction data | ❌ Yes — DFX runs the hosted node | ✅ No | +| Operator sees plaintext transaction data | ❌ Yes — `api.zkcoins.app` is operated by [zkcoins.app](https://zkcoins.app) | ✅ No | | Setup effort | ✅ None | ⚠️ Postgres + electrs + Bitcoin node | **If you need full transaction privacy, run your own node.** Every release is shipped as `zkcoins/node:latest` (see [Live](#live)), the build recipe is [`Dockerfile`](./Dockerfile), and runtime knobs are documented in [Configuration](#configuration). Point the [zkcoins.app](https://zkcoins.app) client at your self-hosted instance for end-to-end self-custody of transaction data. @@ -219,8 +219,8 @@ Features tagged `mvp` whose current test coverage is insufficient — these bloc | Variable | Default | Effect | | ----------------- | -------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `IS_MAINNET` | _(required, no default)_ | Exact string `true` or `false` — anything else panics. PRD sets `true`, DEV sets `false`. Drives the `Network` enum (Mainnet vs Signet) used for address derivation. Truthy values like `1`, `TRUE`, `yes` are rejected to prevent silent misconfiguration. | -| `ESPLORA_URL` | _(required, no default)_ | HTTP Esplora endpoint for the chain this stage serves. PRD: `http://electrs-mainnet:3000` (DFX Mainnet stack). DEV: `http://electrs-mutinynet:3000`. Self-host: your electrs URL. Empty string is treated as unset. | -| `ESPLORA_WS_URL` | _(required, no default)_ | Esplora-compatible WebSocket endpoint consumed by `scanner_ws` (issue #84). PRD: `wss://mempool.space/api/v1/ws`. DEV: `ws://mempool-api-mutinynet:8999/api/v1/ws` on the DFX mempool/backend stack. Empty string is treated as unset. | +| `ESPLORA_URL` | _(required, no default)_ | HTTP Esplora endpoint for the chain this stage serves. On the `api.zkcoins.app` stack: PRD `http://electrs-mainnet:3000`, DEV `http://electrs-mutinynet:3000`. Self-host: your electrs URL. Empty string is treated as unset. | +| `ESPLORA_WS_URL` | _(required, no default)_ | Esplora-compatible WebSocket endpoint consumed by `scanner_ws` (issue #84). On the `api.zkcoins.app` stack: PRD `wss://mempool.space/api/v1/ws`, DEV `ws://mempool-api-mutinynet:8999/api/v1/ws` (self-hosted mempool/backend sidecar). Empty string is treated as unset. | | `NETWORK_NAME` | `Mutinynet` / `Mainnet` | Human-readable name returned by `/api/info`. Default depends on `IS_MAINNET`. Purely cosmetic — has no behavioural effect on the scanner, publisher, or address derivation. | | `USERNAME_DOMAIN` | _(required, no default)_ | External hostname returned by `/api/info`. The client renders `@` from this. **Node panics on startup if unset.** PRD sets `zkcoins.app`, DEV sets `dev.zkcoins.app` — silent fallback would let a misconfigured stage reproduce the cross-network routing bug (#95) | | `PUBLISHER_KEY` | _(required, no default)_ | 32-byte hex private key for inscription publishing. Node panics on startup if unset. On `IS_MAINNET=true` an additional check refuses the well-known test key. | From 6aae4b0b9a2cb5c9d4f805f19b2ed5f8dcc9e459 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sun, 31 May 2026 21:21:42 +0200 Subject: [PATCH 3/9] perf(bootstrap): warmup prover in background; gate /health/ready on prover_warm (#154) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(bootstrap): warmup prover in a background task; gate /health/ready on prover_warm PR #147 paid the ~7 s Plonky2 cold-prove tax synchronously between load_from_pg and TcpListener::bind, which pushed API offline time per deploy from ~14 s (circuit build alone) to ~21 s (circuit build + cold prove). The user constraint is explicit: API must be reachable as soon as possible. PR #147 was closed for failing that constraint. This shape moves the warmup off the bootstrap-critical path: 1. TcpListener::bind returns at ~0.1 s. axum::serve starts draining connections — /health (liveness) is 200, /api/* paths return correct answers (a /api/mint or /api/send during the warmup window pays the ~7 s cold tax, but it serves correctly). 2. tokio::task::spawn_blocking launches AccountNode::warmup_prover on the blocking pool so the CPU-bound prove does not starve the tokio worker that owns axum::serve. 3. After ~21 s the warmup task flips the new prover_warm Arc to true. /health/ready transitions from 503 with {"status":"starting","prover":"warming","failures":["prover"]} to 200 with {"status":"ready","prover":"ready"}. A load balancer / Kuma monitor keyed on /health/ready holds traffic on the previous-generation pod through the warmup window; /health (liveness) is unaffected so container restart loops are not triggered. ZKCOINS_SKIP_BOOTSTRAP_WARMUP=1 skips the background task entirely (smoke tests in runtime_tests.rs). Three architecture decisions, codified in MIGRATION_RESEARCH.md §7.25: - spawn_blocking over tokio::spawn — Plonky2 prove is CPU-bound and would starve the tokio worker dispatching HTTP requests. - Arc over Arc> — flag is write-once + read-many, AtomicBool::store is a single instruction. - std::process::exit(1) over panic!() — a panic inside spawn_blocking only surfaces when the JoinHandle is awaited (it deliberately is not), so a bare panic would leave the node serving 503 forever. exit(1) crash-loops the container at the same severity as PR #147's synchronous expect(). CONTRIBUTING.md gains a Bootstrap timing section + a row for the new env var. AccountNode::warmup_prover + the warmup_prover_completes_successfully test were adapted from PR #147 with the return type switched to anyhow::Result for the runtime call site. A new router test asserts /health/ready returns 503 with the warming-tag payload when prover_warm is false. * docs(runtime): correct warmup-task scanner-ordering comment Reviewer caught: the prior comment claimed the scanner spawns AFTER start_rest_node returns. That is factually wrong — main.rs runs start_rest_node + run_scanner_ws concurrently via tokio::spawn. The correctness conclusion still holds because the scanner locks `state`, not `account_node`. Rewrite the comment to name the right invariant and the right contender (a user request that lands during the ~7 s warmup window). * test(coverage): drop unused .map_err closure in warmup_prover The previous shape `.map(|_| ()).map_err(|e| anyhow!("...{e}"))` left two never-called closures in the happy-path test, costing the 100% function + 100% line coverage gate. `?` propagation matches what `prove_initial` already returns (`anyhow::Result`) and is covered by the same single test. --- CONTRIBUTING.md | 46 +++++++++++++++ MIGRATION_RESEARCH.md | 66 ++++++++++++++++++++++ node/src/account_node.rs | 52 +++++++++++++++++ node/src/account_node_tests.rs | 18 ++++++ node/src/audit_tests.rs | 1 + node/src/router.rs | 65 +++++++++++++++++++-- node/src/router_tests.rs | 67 ++++++++++++++++++++++ node/src/runtime.rs | 100 ++++++++++++++++++++++++++++++++- node/src/runtime_tests.rs | 11 ++++ 9 files changed, 421 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7261bbb0..1f9be67f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -529,8 +529,54 @@ on startup if unset — there is no silent fallback. | `NETWORK_NAME` | `Mutinynet` / `Mainnet` | Human-readable name returned by `/api/info`. Derived from `IS_MAINNET` if unset. Purely cosmetic — no behavioural effect. | | `PROOFS_DIR` | `./proofs` | Directory for per-proof bincode files (see `Persistent State` below). | | `SCANNER_INITIAL_SETTLE_TIMEOUT_MS` | (runtime-defined) | Override for the scanner's initial-settle deadline; see `runtime.rs`. | +| `ZKCOINS_SKIP_BOOTSTRAP_WARMUP` | `false` | When `1`/`true`, skip the background Plonky2 prover warmup task at startup. Sets `prover_warm = true` immediately so `/health/ready` returns 200 the moment the listener binds. Set in the runtime smoke tests so pre-push wall stays bounded; production deploys leave it unset. See **Bootstrap timing** below. | | `RUST_LOG` | `info` | Log level (`debug`, `info`, `warn`, `error`). | +### Bootstrap timing + +The node bootstraps the HTTP listener and the Plonky2 prover in a +specific sequence so the API is reachable as quickly as possible: + +1. `~0.1 s` — `TcpListener::bind` returns. `/health` (liveness) is now + 200. The listener accepts connections and `axum::serve` starts + draining them. +2. `~0.1 s` — `tokio::task::spawn_blocking` is launched with + `AccountNode::warmup_prover`, a synthetic discardable + `prove_initial` that wakes the Rayon worker pool and the AOT- + compiled Plonky2 evaluator caches. The task runs CPU-bound on a + blocking-pool thread so the tokio worker that owns `axum::serve` is + not starved. +3. `~21 s` — `warmup_prover` returns Ok. The background task flips + `prover_warm = true`. `/health/ready` now returns 200 with + `prover: ready`. + +While step 3 is in progress, `/health/ready` returns 503 with +`{"ready":false,"failures":["prover"],"status":"starting","prover":"warming"}`. +A load balancer (or Kuma monitor) keyed on the readiness endpoint +keeps traffic on the previous-generation pod through the warmup +window — the new pod's `/health` still returns 200 so the container +runtime does not restart it. + +A user request that lands BEFORE the warmup completes still serves +correctly — it just pays the ~7 s cold-prove tax instead of the +steady-state ~5 s p50. The trade-off vs. the previous synchronous +shape (PR #147, closed): API offline time per deploy stays ~0.1 s +instead of ~21 s; the cold-tax shifts from the first +post-deploy user request to whichever request arrives during the +warmup window. + +Empirical numbers (dfxdev R2 probe, 2026-05-31): + +| Stage | Wall (ms) | Notes | +|---|---|---| +| `circuit_build_wall_ms` | 14214 | `Prover::new()` — paid by `load_from_pg` BEFORE the listener binds. | +| `prove_cold_wall_ms` | 7012 | First prove call after build — what the background warmup pays. | +| `prove_warm p50` | 4777 | Steady state — every request after the warmup task flips the flag. | + +Set `ZKCOINS_SKIP_BOOTSTRAP_WARMUP=1` to skip the warmup task entirely. +Used by the runtime smoke tests in `runtime_tests.rs`; production +deploys leave it unset. + ### Minimal local-dev env All chain-shaping vars are required — there are no defaults. Set them diff --git a/MIGRATION_RESEARCH.md b/MIGRATION_RESEARCH.md index 87c52804..26630f32 100644 --- a/MIGRATION_RESEARCH.md +++ b/MIGRATION_RESEARCH.md @@ -1419,6 +1419,72 @@ neither the original one: the client side. Always cross-check the request against a known-good client's wire format before blaming the server. +### 7.25 Bootstrap warmup: background over synchronous to preserve API availability — **codified** + +The dfxdev R2 probe (2026-05-31, see `node/src/bin/probe_r2.rs`) +measured a ~7 s cold-prove tax on the first `prove_initial` after +`Prover::new()` — paid in production by whichever user request +arrived first after a container restart, surfacing as a ~12 s +`/api/mint` instead of the steady-state ~5 s p50. Two shapes were +considered for hiding the tax inside the bootstrap. + +**Shape A: synchronous warmup before listener bind (PR #147, +closed).** Run `warmup_prover` synchronously between `load_from_pg` +and `TcpListener::bind`. Pushes API offline time per deploy from +~14 s (circuit build) to ~21 s (circuit build + cold prove). Net +benefit per deploy: every user request after the listener binds is +warm. Rejected because the offline-window grew by 50%; the user +constraint is explicit ("API soll wenn immer möglich SOFORT online +sein"). + +**Shape B: background warmup after listener bind (this PR).** Bind +the listener at ~0.1 s, then spawn `warmup_prover` on the +`tokio::task::spawn_blocking` pool so the CPU-bound prove runs on a +blocking-pool thread and does not starve the tokio worker that owns +`axum::serve`. Expose the warmup status as +`AppState::prover_warm: Arc` and gate `/health/ready` on +it: while the task is running the readiness probe returns 503 with +`{"status":"starting","prover":"warming","failures":["prover"]}`. A +load balancer keeps holding traffic on the previous-gen pod through +the ~21 s warmup window; the new pod's `/health` (liveness) returns +200 immediately so the container runtime does not restart it. A user +request that lands DURING the warmup still serves correctly — it +pays the ~7 s cold tax, which is the worst-case-equivalent cost to +the pre-PR-#147 shape but bounded to the ~21 s window instead of +"first request after every deploy". + +Three architecture decisions inside Shape B that are easy to get +wrong: + +1. **`spawn_blocking` over `tokio::spawn`.** Plonky2 `prove_initial` + is CPU-bound (Rayon worker pool, AOT-compiled evaluator caches); + running it on a tokio worker thread would starve every other + future on that worker for ~7 s — including the `axum::serve` + future, which is the entire point of binding the listener first. + `spawn_blocking` runs the closure on the blocking pool, leaving + the tokio workers free to dispatch HTTP requests. + +2. **`Arc` over `Arc>`.** The flag is + write-once + read-many. `AtomicBool::store(true, SeqCst)` is a + single instruction; `RwLock` would add a syscall on every + `/health/ready` read for a flag that flips exactly once per + process lifetime. + +3. **`std::process::exit(1)` over `panic!()`.** A panic inside the + `spawn_blocking` closure surfaces as a `JoinError` only when the + `JoinHandle` is awaited — but we deliberately do not await it + (the listener serves while the warmup runs). A bare `panic!()` + would leave the node running with `prover_warm = false` + permanently, never returning 200 on `/health/ready`. `exit(1)` + crash-loops the container immediately, matching the same severity + as the previous synchronous `expect()` shape. + +The user-visible behavioural change from Shape A to Shape B is the +small window where a request lands during warmup and pays the ~7 s +cold tax. That trade-off is documented in `CONTRIBUTING.md` +("Bootstrap timing") so an operator does not misread the warmup- +window p50 as a regression. + --- ## 8. Local Artifacts diff --git a/node/src/account_node.rs b/node/src/account_node.rs index 26fc9e22..28ef44d7 100644 --- a/node/src/account_node.rs +++ b/node/src/account_node.rs @@ -790,6 +790,58 @@ impl AccountNode { .insert(*zkcoins_program::types::MINTING_ADDRESS, mutated_minting); } + /// Run a synthetic discardable `prove_initial` to wake the Rayon + /// worker pool and warm the AOT-compiled Plonky2 evaluator caches. + /// + /// Called from a background `spawn_blocking` task spawned by + /// `runtime::start_rest_node` AFTER `TcpListener::bind` so the + /// HTTP listener is already serving traffic while this runs. + /// `/health/ready` exposes a `prover` flag that flips to `ready` + /// the moment this call returns Ok; load balancers / Kuma can use + /// the readiness endpoint to gate traffic during a rolling deploy + /// without holding the API itself offline. + /// + /// Empirical evidence (dfxdev R2 probe, 2026-05-31): + /// - `circuit_build_wall_ms = 14214` — `Prover::new()` (paid in + /// `load_from_pg` already, before this call). + /// - `prove_cold_wall_ms = 7012` — first prove call after build, + /// which is what this method pays during background warmup. + /// - `prove_warm p50 = 4777` — every subsequent prove call, + /// including the first user-facing request once the background + /// task has reported `prover_warm = true`. + /// + /// A user-facing `/api/mint` or `/api/send` that lands BEFORE the + /// background warmup completes still serves correctly, but pays + /// the cold-prove tax (~7 s instead of ~5 s). The deferred cost is + /// amortised by every subsequent request. + /// + /// `prove_initial` against a fresh `AccountState` (zero balance, + /// dummy pubkey, `ZERO_HASH` history root) is the cheapest valid + /// codepath that exercises the full circuit + Rayon spinup; the + /// resulting proof is discarded. No state mutation, no on-chain + /// side-effect. + /// + /// The mirrored helper in `node/src/bin/probe_r2.rs` is the + /// reference implementation that produced the numbers above; keep + /// the witness shape (fresh `AccountState::new(_)` + `ZERO_HASH`) in + /// sync if either side changes. + pub fn warmup_prover(&self) -> anyhow::Result<()> { + // 33-byte well-formed secp256k1-compressed pubkey placeholder. + // The circuit does not verify the pubkey is on-curve in + // `prove_initial`, only that the witness layout matches; the + // same `0x02` + ramp pattern is used by `probe_r2::dummy_pubkey` + // and by `script-plonky2::tests::dummy_pubkey`. + let mut pk = [0u8; 33]; + pk[0] = 0x02; + for (i, b) in pk.iter_mut().enumerate().skip(1) { + *b = (7u8).wrapping_add(i as u8); + } + let warmup_account_state = AccountState::new(pk); + self.prover + .prove_initial(&warmup_account_state, ZERO_HASH)?; + Ok(()) + } + /// Read-only handle on the shared [`State`] (SMT + MMR). Exposed so /// the startup invariant check in `runtime` can verify /// every persisted minting-account pubkey has a corresponding SMT diff --git a/node/src/account_node_tests.rs b/node/src/account_node_tests.rs index bf78f3c1..b578fa5a 100644 --- a/node/src/account_node_tests.rs +++ b/node/src/account_node_tests.rs @@ -1371,3 +1371,21 @@ fn test_send_coins_rejects_source_commitment_missing_from_history_mmr() { "desynced `state.prev_mmr_root` must surface the off-circuit history-MMR rejection at account_node.rs:419", ); } + +/// `warmup_prover` runs a synthetic `prove_initial` against a fresh +/// `AccountState` and discards the proof. It must return Ok on a +/// freshly-constructed `AccountNode` — that is the production +/// invariant: the same `Prover` will serve every subsequent +/// user-facing request, so a warmup failure means production requests +/// would also fail, and the bootstrap exits the process rather than +/// binding a listener that would serve 500s. This test exercises the +/// success arm. Pinned `#[ignore]`-able via cargo flags but kept in +/// the default suite because the coverage gate would otherwise treat +/// the helper as unreached. +#[test] +fn warmup_prover_completes_successfully() { + let state_arc = Arc::new(Mutex::new(State::new())); + let node = AccountNode::new(Arc::clone(&state_arc)); + node.warmup_prover() + .expect("warmup_prover must succeed on a fresh AccountNode"); +} diff --git a/node/src/audit_tests.rs b/node/src/audit_tests.rs index 7ee442bb..22d2c1e0 100644 --- a/node/src/audit_tests.rs +++ b/node/src/audit_tests.rs @@ -161,6 +161,7 @@ async fn build_state_with_pool() -> ( username_store: Arc::new(Mutex::new(crate::username::UsernameStore::new())), pool: Arc::new(pool), esplora_config: Arc::new(esplora_config), + prover_warm: Arc::new(std::sync::atomic::AtomicBool::new(true)), phase2_reached: Arc::new(tokio::sync::Notify::new()), phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), state_advance_release_lock: Arc::new(tokio::sync::Mutex::new(())), diff --git a/node/src/router.rs b/node/src/router.rs index 48eb2432..83821084 100644 --- a/node/src/router.rs +++ b/node/src/router.rs @@ -14,7 +14,7 @@ use shared::ClientAccount; use shared::{Invoice, ProofData}; use sqlx::PgPool; use std::collections::HashMap; -use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, Mutex, MutexGuard}; use tower_http::cors::CorsLayer; use zkcoins_program::hash::{digest_from_bytes, digest_to_bytes}; @@ -262,6 +262,23 @@ pub(crate) struct AppState { /// clones `NETWORK_CONFIG` into this slot so the runtime /// behaviour is unchanged. pub(crate) esplora_config: Arc, + /// Background-warmup readiness flag. Default `false` at bootstrap + /// start; flipped to `true` either (a) once the background + /// `spawn_blocking` task in `runtime::start_rest_node` reports that + /// `AccountNode::warmup_prover` returned Ok — at which point the + /// Rayon worker pool is warm and every subsequent `/api/mint` / + /// `/api/send` proof matches the steady-state ~5 s p50 — or (b) + /// immediately at bootstrap when `ZKCOINS_SKIP_BOOTSTRAP_WARMUP=1` + /// is set (no background task is spawned in that case). + /// + /// Consumed by `/health/ready`: while `prover_warm == false` the + /// handler returns 503 with a `prover: warming` tag so a rolling + /// deploy can keep the previous-generation pod taking traffic + /// until the new pod's prover is warm. The liveness probe + /// `/health` is unaffected — it returns 200 the moment the + /// listener binds, so container restart loops keyed on liveness + /// are not triggered during the ~21 s warmup window. + pub(crate) prover_warm: Arc, /// Test-only synchronisation primitive used by /// `mint_handler_concurrent_mint_during_proof_returns_503`. The /// production code path notifies via `notify_one()` after entering @@ -2084,12 +2101,29 @@ async fn r2_probe_history_handler( /// JSON body returned by `GET /health/ready`. `failures` is empty on a /// fully ready node; each failing dependency contributes one stable -/// short tag (`"db"`, `"esplora"`) so a Kuma monitor parses the cause -/// without having to scrape the status code in isolation. +/// short tag (`"db"`, `"esplora"`, `"prover"`) so a Kuma monitor parses +/// the cause without having to scrape the status code in isolation. +/// +/// `prover` is the background-warmup tag (see `AppState::prover_warm`): +/// while the bootstrap warmup task is still running, the readiness +/// probe reports `failures: ["prover"]` with `status: starting` and a +/// 503 so a load balancer keeps holding traffic on the previous-gen +/// pod. `/health` (liveness) is unaffected. #[derive(Serialize)] struct ReadyResponse { ready: bool, failures: Vec<&'static str>, + /// Lifecycle tag. `"starting"` while any failure is present, + /// `"ready"` once every dependency probe passes. Distinct from + /// `ready: bool` so a parsing consumer can branch on a short + /// string without re-deriving it from the bool + failures shape. + status: &'static str, + /// Background-warmup tag. `"warming"` while + /// `AppState::prover_warm == false`, `"ready"` afterwards. + /// Emitted on every response (regardless of overall readiness) so + /// a deploy dashboard can show the warmup progress separately + /// from the DB/Esplora probes. + prover: &'static str, } /// Readiness probe (`GET /health/ready`). @@ -2125,13 +2159,36 @@ async fn ready_handler(State(state): State) -> impl IntoResponse { failures.push("esplora"); } + // Background-warmup gate. `prover_warm` is flipped to true by the + // `spawn_blocking` task that `runtime::start_rest_node` launches + // immediately after binding the TCP listener (or directly at boot + // when `ZKCOINS_SKIP_BOOTSTRAP_WARMUP=1`). Until then a user + // request still succeeds but pays the ~7 s cold-prove tax — for + // the rolling-deploy use case the load balancer holds traffic on + // the previous-gen pod by treating this readiness probe as the + // gate, not the liveness probe. + let prover_warm = state.prover_warm.load(Ordering::SeqCst); + if !prover_warm { + failures.push("prover"); + } + let ready = failures.is_empty(); let status = if ready { StatusCode::OK } else { StatusCode::SERVICE_UNAVAILABLE }; - (status, Json(ReadyResponse { ready, failures })) + let lifecycle_status = if ready { "ready" } else { "starting" }; + let prover_status = if prover_warm { "ready" } else { "warming" }; + ( + status, + Json(ReadyResponse { + ready, + failures, + status: lifecycle_status, + prover: prover_status, + }), + ) } /// Ping the configured Esplora endpoint. A successful tip-height fetch diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index 90e9143c..f2c395b0 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -64,6 +64,12 @@ fn test_state() -> AppState { network_name: "Mutinynet".to_string(), ws_url: None, }), + // Tests construct the AppState with the prover already marked + // warm so handlers that only consult `prover_warm` indirectly + // (e.g. the readiness probe) don't observe a half-bootstrapped + // shape. The dedicated 503/warming-tag test below overrides + // this back to `false` to exercise the gating arm. + prover_warm: Arc::new(std::sync::atomic::AtomicBool::new(true)), phase2_reached: Arc::new(tokio::sync::Notify::new()), phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), state_advance_release_lock: Arc::new(tokio::sync::Mutex::new(())), @@ -2323,6 +2329,7 @@ async fn send_with_insufficient_funds_returns_422_with_error_string() { network_name: "Mutinynet".to_string(), ws_url: None, }), + prover_warm: Arc::new(std::sync::atomic::AtomicBool::new(true)), phase2_reached: Arc::new(tokio::sync::Notify::new()), phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), state_advance_release_lock: Arc::new(tokio::sync::Mutex::new(())), @@ -3485,6 +3492,11 @@ async fn ready_returns_200_when_db_and_esplora_reachable() { let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); assert_eq!(v["ready"], true); assert_eq!(v["failures"].as_array().unwrap().len(), 0); + // New fields introduced with the background-warmup feature: + // a 200 response means status is `ready` and prover is `ready`. + // The default `test_state()` shape flips `prover_warm` to true. + assert_eq!(v["status"], "ready"); + assert_eq!(v["prover"], "ready"); } #[tokio::test] @@ -3550,6 +3562,60 @@ async fn ready_returns_503_when_esplora_unreachable() { assert_eq!(failures, vec!["esplora".to_string()]); } +/// `prover_warm == false` (the bootstrap shape while the background +/// `spawn_blocking` task in `runtime::start_rest_node` is still +/// running) gates `/health/ready` to 503 with a `prover` failure tag +/// and a `status: starting` / `prover: warming` payload. The DB + +/// Esplora paths short-circuit to an unreachable mock so the failure +/// list contains only `prover` — proves the warmup gate is wired in +/// isolation from the other dependencies. No Postgres needed: the +/// failure path doesn't require a live pool because `SELECT 1` +/// against `dead_pool()` short-circuits to a connect error that +/// the handler treats as a `db` failure too — which is fine, the +/// test just asserts `prover` is present. +#[tokio::test] +async fn ready_returns_503_with_prover_warming_when_prover_not_warm() { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + // Esplora is healthy so it does NOT contribute to `failures`; the + // DB path falls through `dead_pool` and DOES contribute a `db` + // failure, but the assertion below only checks `prover` is + // present — the test is about the warmup gate, not the full + // failure-list shape. + let mock_server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/blocks/tip/height")) + .respond_with(ResponseTemplate::new(200).set_body_string("123456")) + .mount(&mock_server) + .await; + + // Build the state with the prover-warm flag flipped back to false. + // `ready_state` calls `test_state()` which defaults to `true`, so + // we override the field after construction. + let mut state = ready_state(dead_pool(), mock_server.uri()); + state.prover_warm = Arc::new(std::sync::atomic::AtomicBool::new(false)); + + let req = Request::get("/health/ready").body(Body::empty()).unwrap(); + let (status, body) = send_request_with_state(state, req).await; + + assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE, "body={}", body); + let v: serde_json::Value = serde_json::from_str(&body).expect("valid JSON"); + assert_eq!(v["ready"], false); + assert_eq!(v["status"], "starting"); + assert_eq!(v["prover"], "warming"); + let failures: Vec = v["failures"] + .as_array() + .unwrap() + .iter() + .map(|s| s.as_str().unwrap().to_string()) + .collect(); + assert!( + failures.contains(&"prover".to_string()), + "expected `prover` in failures, got {failures:?}" + ); +} + // ======================================================================= // GET /health/publisher — operational preflight // ======================================================================= @@ -3709,6 +3775,7 @@ fn mint_test_state() -> AppState { network_name: "Mutinynet".to_string(), ws_url: None, }), + prover_warm: Arc::new(std::sync::atomic::AtomicBool::new(true)), phase2_reached: Arc::new(tokio::sync::Notify::new()), phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), state_advance_release_lock: Arc::new(tokio::sync::Mutex::new(())), diff --git a/node/src/runtime.rs b/node/src/runtime.rs index fc58e2f2..efb9adf3 100644 --- a/node/src/runtime.rs +++ b/node/src/runtime.rs @@ -11,6 +11,7 @@ //! is measured normally. use std::net::SocketAddr; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use axum::http::StatusCode; @@ -95,8 +96,15 @@ pub async fn start_rest_node( let shared_username_store = Arc::new(Mutex::new(username_store)); + // Background-warmup readiness flag. Default `false`; flipped to + // `true` by either the background `spawn_blocking` task below (once + // `AccountNode::warmup_prover` returns Ok) or immediately if the + // operator set `ZKCOINS_SKIP_BOOTSTRAP_WARMUP=1`. Consumed by + // `/health/ready`; see the field doc on `AppState::prover_warm`. + let prover_warm = Arc::new(AtomicBool::new(false)); + let state = AppState { - account_node: shared_account_node, + account_node: Arc::clone(&shared_account_node), proof_store, minting_account, username_store: shared_username_store, @@ -104,6 +112,7 @@ pub async fn start_rest_node( // The readiness probe uses this to ping Esplora; in production // it points at the same `ESPLORA_URL` as the scanner / publisher. esplora_config: Arc::new(NETWORK_CONFIG.clone()), + prover_warm: Arc::clone(&prover_warm), #[cfg(test)] phase2_reached: Arc::new(tokio::sync::Notify::new()), #[cfg(test)] @@ -219,6 +228,95 @@ pub async fn start_rest_node( println!("REST API started at {}", socket_addr); let listener = TcpListener::bind(socket_addr).await?; + tracing::info!("Listener bound on {socket_addr}; API is reachable"); + + // Background-warmup. A fresh `Prover` carries a cold Rayon worker + // pool and uninitialised AOT-compiled Plonky2 evaluator caches; + // empirically (dfxdev R2 probe, 2026-05-31) the first + // `prove_initial` after `Prover::new()` takes ~7012 ms vs the + // steady-state p50 of ~4777 ms for every subsequent call. + // + // The previous shape (PR #147, closed) paid that tax synchronously + // before binding the listener and pushed API offline time per + // deploy from ~14 s to ~21 s. This shape instead binds the + // listener FIRST (the API is reachable at ~0.1 s), then spawns + // `AccountNode::warmup_prover` in a `spawn_blocking` task so the + // tokio worker that runs `axum::serve` is not starved by the + // CPU-bound Plonky2 prove. While the task is running a user + // request still serves correctly — it just pays the ~7 s cold tax + // — and `/health/ready` returns 503 with `prover: warming` so an + // LB / Kuma can hold traffic on the previous-gen pod during a + // rolling deploy. + // + // Opt-out via `ZKCOINS_SKIP_BOOTSTRAP_WARMUP=1`: the smoke tests + // in `runtime_tests.rs` set this so each `start_rest_node_*` test + // does not pay the ~7 s prove tax twice over. When set, + // `prover_warm` is flipped to `true` immediately so the readiness + // probe matches the production-ready shape. + let skip_warmup = std::env::var("ZKCOINS_SKIP_BOOTSTRAP_WARMUP") + .map(|v| v == "1" || v.eq_ignore_ascii_case("true")) + .unwrap_or(false); + let warmup_handle = if skip_warmup { + tracing::info!( + "Bootstrap warmup skipped via ZKCOINS_SKIP_BOOTSTRAP_WARMUP; \ + prover_warm = true (first user request will pay the ~7 s cold tax)" + ); + prover_warm.store(true, Ordering::SeqCst); + None + } else { + let account_node_for_warmup = Arc::clone(&shared_account_node); + let prover_warm_flag = Arc::clone(&prover_warm); + let handle = tokio::task::spawn_blocking(move || { + let warmup_t = std::time::Instant::now(); + // Hold the sync `Mutex` only for the duration of the + // prove call. The scanner — spawned in parallel by + // `main.rs` — locks `state`, not `account_node`, so it + // does not contend with this guard. The only realistic + // contender is a user request that lands during the + // ~7 s warmup window; that request blocks on + // `account_node.lock()` for the remainder of the warmup + // (then runs warm), which is the accepted trade-off + // documented in the function comment. The block is + // shorter (and aborts cleanly on shutdown) than the + // previous synchronous-bootstrap shape. + let result = { + let guard = account_node_for_warmup + .lock() + .expect("AccountNode mutex poisoned before bootstrap warmup"); + guard.warmup_prover() + }; + match result { + Ok(()) => { + tracing::info!( + elapsed_ms = warmup_t.elapsed().as_millis() as u64, + "Background warmup complete; prover ready" + ); + prover_warm_flag.store(true, Ordering::SeqCst); + } + Err(e) => { + // Same severity as the previous synchronous + // `expect()` — the same Prover serves every + // subsequent user request, so a warmup failure + // means production requests would also fail. + // Crash-loop the container rather than running + // a node that serves 5xx for the prove path. + tracing::error!(error = %e, "Background warmup failed — exiting"); + std::process::exit(1); + } + } + }); + tracing::info!("Bootstrap warmup spawned in background; listener serving now"); + Some(handle) + }; + // `warmup_handle` is intentionally not awaited: `axum::serve` + // owns the foreground future and the warmup runs to completion + // on its own. On graceful shutdown `axum::serve` returns first; + // the warmup task either completes naturally or is dropped when + // the tokio runtime shuts down. The binding keeps the JoinHandle + // alive (vs. `let _ =`) so a future shutdown signal can call + // `.abort()` once a signal handler is wired in. + let _warmup_handle = warmup_handle; + // `into_make_service_with_connect_info::()` exposes the // peer's TCP socket to extractors — the audit middleware reads it // through `ConnectInfo` and writes it to diff --git a/node/src/runtime_tests.rs b/node/src/runtime_tests.rs index 087ec5d7..bc9a0cf7 100644 --- a/node/src/runtime_tests.rs +++ b/node/src/runtime_tests.rs @@ -102,6 +102,14 @@ async fn start_rest_node_binds_and_serves_health() { std::env::set_var("IS_MAINNET", "false"); std::env::set_var("ESPLORA_URL", "http://127.0.0.1:1/api"); std::env::set_var("ESPLORA_WS_URL", "ws://127.0.0.1:1/api/v1/ws"); + // Smoke tests only need the listener to bind and serve `/health` + // / `/api/balance`; they MUST NOT pay the ~7 s background warmup + // tax (would double pre-push wall and add nothing to the bootstrap + // failure-mode coverage this file owns). With this flag set + // `prover_warm` is flipped to `true` immediately at bootstrap and + // no `spawn_blocking` task is started — same shape these tests + // had before the warmup feature landed. + std::env::set_var("ZKCOINS_SKIP_BOOTSTRAP_WARMUP", "1"); // PR-A3 moved all sibling-file state (accounts.bin, usernames.bin, // minting_num_pubkeys.bin) into Postgres; the bootstrap only needs @@ -219,6 +227,9 @@ async fn bootstrap_initial_minting_account_balance_is_goldilocks_safe() { std::env::set_var("IS_MAINNET", "false"); std::env::set_var("ESPLORA_URL", "http://127.0.0.1:1/api"); std::env::set_var("ESPLORA_WS_URL", "ws://127.0.0.1:1/api/v1/ws"); + // See the sibling smoke test for the rationale — skip the + // ~7 s background warmup so pre-push wall stays bounded. + std::env::set_var("ZKCOINS_SKIP_BOOTSTRAP_WARMUP", "1"); let tmp = std::env::temp_dir().join(format!( "zkcoins-balance-test-{}-{}", From 72192c6e218d88d118f0b88577c8e00f6a5c73cf Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 1 Jun 2026 22:40:45 +0200 Subject: [PATCH 4/9] ci: merge node-tests+coverage into one job; add ci:db / ci:prover subset gates (#167) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two structural changes to the heavy CI lane, each preserving 100% coverage gate strictness and the existing test set. 1. Merge `node-tests` and `coverage` into a single `test-and-coverage` job. The previous topology ran the same `-p node -p shared --all-features` test set TWICE on the same self-hosted m3-ultra pool (once plain, once under `cargo llvm-cov nextest`). The instrumented run already produces the test execution AND the coverage data, so the standalone `node-tests` job was pure duplication. After the merge: * Heavy gate still runs `cargo llvm-cov nextest … --fail-under-lines 100 --fail-under-functions 100` — same strictness, same ignore regex, same `-E 'not binary(api_remote)'` exclusion, same `RUSTFLAGS=--cfg coverage_nightly`. * Test set is `-p node -p shared --all-features` (matching the former `node-tests` scope); the coverage scope stays `-p node` via an additional `shared/src/.*\.rs$` entry in the ignore regex. * One m3-ultra agent slot is occupied per PR instead of two, directly reducing the Colima / Postgres-container pressure on dfx01 that produced sporadic `PoolTimedOut` flakes when multiple PRs ran the heavy lane in parallel. 2. Add two new subset-gate jobs for faster developer iteration: * `db-tests` — gated by the new `ci:db` label. Runs the Postgres-backed test surface (db / state / job_store / audit / username / r2_probe / publisher / runtime / commitment / the jobs-API router subset / the build_network_config crate-root tests / the persist+load account_node tests) under plain `cargo nextest run` without llvm-cov instrumentation. ~15 min. * `prover-tests` — gated by the new `ci:prover` label. Runs the full account_node send / mint / receive surface (Plonky2 happy paths and pure-Rust error paths) so anything touching account-node state transitions is covered. ~25 min. Both subsets carry an `&& !contains(... labels.*.name, 'ci:full')` guard so a PR labeled with both runs only the heavy gate (the superset). Both run on the same `[self-hosted, m3-ultra]` pool with the same env block, sccache config, DOCKER_HOST step, and Telegram-alert step as the heavy job. They are NOT a pre-merge gate; `ci:full` remains the authoritative check. Job topology now: lint-and-build → db-tests / prover-tests / test-and-coverage (parallel, all `needs: lint-and-build`) → notify-failure (needs: lint-and-build + test-and-coverage) All filter expressions consistently `^`-anchored to the module root to disambiguate from `::tests::` collisions. --- .github/workflows/ci.yaml | 500 ++++++++++++++++++++++++++------------ 1 file changed, 350 insertions(+), 150 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index e795dbc3..4f45a384 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -29,9 +29,10 @@ on: # `if:` guard on each job (saves self-hosted-runner time while # work is still in progress). # - # `labeled` / `unlabeled` are added so toggling the `ci:full` - # label triggers (or removes) the heavy self-hosted-runner jobs - # on demand — see the `node-tests` job below. + # `labeled` / `unlabeled` are added so toggling the `ci:full`, + # `ci:db`, or `ci:prover` labels triggers (or removes) the + # corresponding self-hosted-runner jobs on demand — see the + # `test-and-coverage`, `db-tests`, and `prover-tests` jobs below. pull_request: types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] @@ -69,17 +70,48 @@ permissions: env: CARGO_TERM_COLOR: always -# `lint-and-build` catches what GitHub-hosted Linux can cheaply catch: -# cross-platform compile bitrot and lint regressions. +# Job topology: # -# `node-tests` + `coverage` are the authoritative test + coverage gate. -# They run on the m3-ultra self-hosted runner pool (label `m3-ultra`, -# 6 agents on dfx01) — the documented hardware target (CONTRIBUTING.md -# § "Working on the Plonky2 Migration"). On `ubuntu-latest` the full -# suite repeatedly hit the 75-min timeout (issue #30); on the M3 Ultra -# it is ~60-90 min for a Rust change. Moving the gate into CI rather -# than the developer's laptop unblocks the developer on push -# (issue #40). +# * `lint-and-build` — GitHub-hosted Linux. Catches cross-platform +# compile bitrot and lint regressions cheaply. Gates everything +# below via `needs:`. +# +# * `db-tests` / `prover-tests` — narrow, label-gated subsets on the +# m3-ultra pool for fast developer iteration. They run plain +# `cargo nextest` (no llvm-cov instrumentation), enforce NO +# coverage gate, and only execute the tests relevant to the area +# the developer is working on. Two labels: +# - `ci:db` → Postgres / state / coordinator (~15 min) +# - `ci:prover` → Plonky2-heavy mint/send/receive (~25 min) +# Both are mutually exclusive with `ci:full`: a PR carrying +# `ci:full` skips the subset jobs because the heavy gate is a +# strict superset (runs every test the subsets do, plus the +# coverage gate). See the `if:` guard on each subset job. +# +# * `test-and-coverage` — the authoritative test + coverage gate. +# Single heavy job (~60-90 min on a self-hosted M3 Ultra runner — +# one of 6 agents on dfx01 sharing the host's 96 GB / 28 cores). +# Gated behind the `ci:full` label so we don't burn runner time on +# every speculative PR — apply the label when the PR is ready for +# the authoritative gate. The Release PR (`develop -> main`) gets +# the label applied automatically by auto-release-pr.yaml. +# +# Why test + coverage are merged into one job: the previous topology +# had a `node-tests` job and a separate `coverage` job, both +# running the SAME nextest suite (`coverage` simply wrapped nextest +# in `cargo llvm-cov nextest`). That doubled wall-clock and m3-ultra +# agent usage on every Release PR for no signal benefit — llvm-cov +# under nextest produces both test execution AND coverage data in a +# single binary run. Merging them keeps the 100% lines + functions +# gate intact (same ignore regex, same `not binary(api_remote)` +# exclusion) while running the heavy suite once per PR. +# +# The documented hardware target is the M3 Ultra (CONTRIBUTING.md +# § "Working on the Plonky2 Migration"). On `ubuntu-latest` the +# full suite repeatedly hit the 75-min timeout (issue #30); on the +# M3 Ultra it is ~60-90 min for a Rust change. Moving the gate +# into CI rather than the developer's laptop unblocks the developer +# on push (issue #40). # # Runner ops: see scripts/ci-runner/README.md. jobs: @@ -151,20 +183,30 @@ jobs: - name: Build node (all features — self-host opt-in build) run: cargo build -p node --all-features - node-tests: - name: Node + Shared Tests (M3 Ultra) - # Heavy job (~60-90 min on a self-hosted M3 Ultra runner — one of - # 6 agents on dfx01 sharing the host's 96 GB / 28 cores). Gated - # behind the `ci:full` label so we don't burn runner time on every - # speculative PR — apply the label when the PR is ready for the - # authoritative test+coverage gate. The Release PR - # (`develop -> main`) gets the label applied automatically by - # auto-release-pr.yaml. (See `coverage` job below for why the same - # guard is repeated there.) - if: contains(github.event.pull_request.labels.*.name, 'ci:full') + db-tests: + name: DB Subset Tests (M3 Ultra) + # Narrow label-gated subset for fast developer iteration on + # Postgres / state / coordinator changes. Runs ONLY the tests + # that touch the storage layer, the coordinator state machine, + # the username registry, the audit log, the publisher/runtime + # plumbing, and the router job endpoints. Estimated ~15 min on + # an M3 Ultra agent. + # + # Mutually exclusive with `ci:full`: if a PR carries `ci:full`, + # the heavy `test-and-coverage` job already runs the entire + # suite (including everything below) plus the coverage gate, so + # running this subset would just waste an m3-ultra agent slot. + # The `&& !contains(... 'ci:full')` clause enforces that. + # + # Plain `cargo nextest` (no llvm-cov wrapping): subset gates are + # for iteration speed; the authoritative 100% coverage gate + # stays exclusive to `test-and-coverage` / `ci:full`. + if: >- + contains(github.event.pull_request.labels.*.name, 'ci:db') + && !contains(github.event.pull_request.labels.*.name, 'ci:full') needs: lint-and-build runs-on: [self-hosted, m3-ultra] - timeout-minutes: 120 + timeout-minutes: 45 env: # All three chain-shaping env vars are required by the node # bootstrap — no defaults exist (see @@ -181,73 +223,185 @@ jobs: # is irrelevant for the `info_returns_*` assertions (they only # check non-empty + shape). USERNAME_DOMAIN: test.zkcoins.local - # `PUBLISHER_KEY` is required on every network (no default — see - # `node/src/lib.rs`). The previous `1234567890abcdef…` fallback - # was a publicly-known test key that drainer bots swept within - # minutes of any on-chain top-up; the fallback was removed - # network-wide in the "require PUBLISHER_KEY on every network" - # hardening. The value below is a syntactically valid 32-byte - # hex placeholder (`0000…0001`) chosen so a future grep for the - # burned `1234…` key returns empty across the repo + CI config; - # it is NOT a secret and MUST NEVER be reused on any chain that - # holds value. The same value is hard-coded in the test mocks at - # `node/src/router_tests.rs` so the wiremock'd publisher address - # path matches the lazy_static-derived `PUBLISHER_ADDRESS`. + # `PUBLISHER_KEY` is required on every network (no default — + # see `node/src/lib.rs`). The previous `1234567890abcdef…` + # fallback was a publicly-known test key that drainer bots + # swept within minutes of any on-chain top-up; the fallback was + # removed network-wide in the "require PUBLISHER_KEY on every + # network" hardening. The value below is a syntactically valid + # 32-byte hex placeholder (`0000…0001`) chosen so a future grep + # for the burned `1234…` key returns empty across the repo + + # CI config; it is NOT a secret and MUST NEVER be reused on any + # chain that holds value. The same value is hard-coded in the + # test mocks at `node/src/router_tests.rs` so the wiremock'd + # publisher address path matches the lazy_static-derived + # `PUBLISHER_ADDRESS`. PUBLISHER_KEY: "0000000000000000000000000000000000000000000000000000000000000001" - # `db_tests` use the `testcontainers` crate, which talks to the local - # Docker daemon. The self-hosted runner runs Colima (not Docker - # Desktop), whose socket lives under the runner user's home - # directory. `testcontainers` defaults to `/var/run/docker.sock`, - # which does not exist on Colima, so the `Set DOCKER_HOST` step - # below points it at the real socket via `$HOME` — same value the - # `docker info` step picks up implicitly via the default `docker` - # context. + # `db_tests` use the `testcontainers` crate, which talks to the + # local Docker daemon. The self-hosted runner runs Colima (not + # Docker Desktop), whose socket lives under the runner user's + # home directory. `testcontainers` defaults to + # `/var/run/docker.sock`, which does not exist on Colima, so + # the `Set DOCKER_HOST` step below points it at the real + # socket via `$HOME` — same value the `docker info` step picks + # up implicitly via the default `docker` context. # `sccache` wraps `rustc` and caches compiled crates across CI - # runs. The M3 Ultra runner agents are self-hosted, so the cache - # lives on local disk and survives between jobs — the speedup is - # biggest for PR pushes that re-touch the same dependency set. + # runs. The M3 Ultra runner agents are self-hosted, so the + # cache lives on local disk and survives between jobs — the + # speedup is biggest for PR pushes that re-touch the same + # dependency set. RUSTC_WRAPPER: sccache # Bump cache cap above sccache's 10-GiB default. The cache is - # user-level (~/Library/Caches/Mozilla.sccache) and shared by every - # m3-ultra agent on the host; with 3+ parallel agents the 10-GiB - # default thrashed — writes from one agent evicted hits another - # had not consumed yet. 50 GiB fits the current working set with - # room to grow; the host has >600 GiB free disk. The server only - # reads SCCACHE_CACHE_SIZE at start, so the install step below - # restarts it when the running cap differs from this value. + # user-level (~/Library/Caches/Mozilla.sccache) and shared by + # every m3-ultra agent on the host; with 3+ parallel agents + # the 10-GiB default thrashed — writes from one agent evicted + # hits another had not consumed yet. 50 GiB fits the current + # working set with room to grow; the host has >600 GiB free + # disk. The server only reads SCCACHE_CACHE_SIZE at start, so + # the install step below restarts it when the running cap + # differs from this value. SCCACHE_CACHE_SIZE: "50G" steps: - name: Checkout uses: actions/checkout@v4 # The launchd-spawned runner agent inherits a minimal PATH that - # includes /opt/homebrew/bin (where a stable Rust lives) but not - # ~/.cargo/bin (where rustup proxies live). Without this step, - # `cargo` resolves to Homebrew's stable cargo, the rust-toolchain - # file pinning nightly is ignored, and dependencies that need - # `#![feature(...)]` (e.g. plonky2_field) fail to compile. Prepend - # ~/.cargo/bin so the rustup proxy is found first and reads the - # workspace rust-toolchain. + # includes /opt/homebrew/bin (where a stable Rust lives) but + # not ~/.cargo/bin (where rustup proxies live). Without this + # step, `cargo` resolves to Homebrew's stable cargo, the + # rust-toolchain file pinning nightly is ignored, and + # dependencies that need `#![feature(...)]` (e.g. plonky2_field) + # fail to compile. Prepend ~/.cargo/bin so the rustup proxy is + # found first and reads the workspace rust-toolchain. - name: Prepend ~/.cargo/bin to PATH (use rustup proxy, not Homebrew Rust) run: echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" # Point `testcontainers` at the Colima socket under the runner - # user's home; see the `DOCKER_HOST` comment in the job env block - # above. Set in a step (not the static `env:` block) so the path - # resolves from `$HOME` at runtime instead of being hard-coded. + # user's home; see the `DOCKER_HOST` comment in the job env + # block above. Set in a step (not the static `env:` block) so + # the path resolves from `$HOME` at runtime instead of being + # hard-coded. - name: Set DOCKER_HOST for Colima socket run: echo "DOCKER_HOST=unix://$HOME/.colima/default/docker.sock" >> "$GITHUB_ENV" - # `sccache` (compile cache) and `cargo-nextest` (test runner) are - # installed once per runner via Homebrew. Re-running on a host - # where they already exist is a no-op. Start sccache's server - # explicitly so the first compile step has a warm cache daemon - # and print stats up-front for visibility in the run log. + # `sccache` (compile cache) and `cargo-nextest` (test runner) + # are installed once per runner via Homebrew. Re-running on a + # host where they already exist is a no-op. Start sccache's + # server explicitly so the first compile step has a warm cache + # daemon and print stats up-front for visibility in the run + # log. # # If a server is already running with a different cap than the - # requested SCCACHE_CACHE_SIZE (e.g. carried over from a previous - # workflow version), stop it so the next --start-server picks up - # the new env value. The on-disk cache files survive the restart. + # requested SCCACHE_CACHE_SIZE (e.g. carried over from a + # previous workflow version), stop it so the next + # --start-server picks up the new env value. The on-disk cache + # files survive the restart. + - name: Ensure sccache + cargo-nextest are installed + run: | + command -v sccache >/dev/null || brew install sccache + command -v cargo-nextest >/dev/null || brew install cargo-nextest + if ! sccache --show-stats 2>/dev/null | grep -qE "Max cache size +50 GiB"; then + sccache --stop-server >/dev/null 2>&1 || true + fi + sccache --start-server >/dev/null 2>&1 || true + sccache --show-stats + + # `db_tests` use testcontainers to spin up a real Postgres 17 + # per test. The runner host has Docker (via Colima) available + # on PATH; fail fast with a readable error if it ever goes + # away, instead of letting the test suite die 5 minutes into + # the run with a hard-to-read bollard error. + - name: Verify Docker is reachable (testcontainers dependency) + run: docker info > /dev/null + + # Subset filter — DB / state / coordinator paths only. + # Module path notes (verified against node/src/ tree on this branch): + # - `_tests.rs` files are mounted via `mod tests;` + # under the owning module (e.g. `db::tests::*`, + # `state::tests::*`). + # - `main_tests.rs` is mounted by `lib.rs` at crate root as + # `mod tests` — so its tests appear as `tests::*` in + # nextest output (NOT `main::tests::*`). + # - `job_store::tests::*` and `router::tests::jobs_*` are + # included for forward compatibility with the jobs-API + # stack landing in app#141 / node#161-#163; if a pattern + # matches no tests today it is a harmless no-op. + # - `shared` crate tests live under `commitment::tests::*` + # and are pulled in by `-p shared`. + # `api_remote` is the live-DEV-node integration test + # (node/tests/api_remote.rs). It targets + # `https://dev-api.zkcoins.app` by default and is meant to run + # AFTER a deploy, from the `api-e2e` job in deploy-dev.yaml — + # not against whatever DEV currently runs while a PR is still + # open. Excluded here for the same reason as in + # `test-and-coverage`. + - name: Run DB subset (release, plain nextest, no coverage) + run: | + cargo nextest run -p node -p shared --release --all-features --test-threads 1 \ + -E 'not binary(api_remote) & (test(/^db::tests::/) + test(/^state::tests::/) + test(/^job_store::tests::/) + test(/^audit::tests::/) + test(/^username::tests::/) + test(/^router::tests::jobs_/) + test(/^r2_probe::tests::/) + test(/^tests::build_network_config_/) + test(/^account_node::tests::test_persist/) + test(/^account_node::tests::test_load/) + test(/^publisher::tests::/) + test(/^runtime::tests::/) + test(/^commitment::tests::/))' + + - name: sccache stats (post-build) + if: always() + run: sccache --show-stats + + # Mirror of the `notify-failure` job downstream, scoped to this + # subset so the operator sees DB-subset failures too (the + # `notify-failure` job only fires when one of its `needs:` + # transitions to `failure`, and chaining subset jobs into that + # list would make a single subset failure mask the heavy gate's + # status under the workflow-level conclusion). + - name: Telegram alert on failure + if: failure() + env: + TG_TOKEN: ${{ secrets.TELEGRAM_BOT_TOKEN }} + TG_CHAT: ${{ secrets.TELEGRAM_CHAT_ID }} + run: | + TEXT=$'❌ '"${{ github.workflow }}"$' / db-tests failed\nRepo: '"${{ github.repository }}"$'\nBranch: '"${{ github.ref_name }}"$'\nRun: '"${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + curl -sS -X POST "https://api.telegram.org/bot${TG_TOKEN}/sendMessage" \ + --data-urlencode "chat_id=${TG_CHAT}" \ + --data-urlencode "text=${TEXT}" \ + -d "parse_mode=HTML" \ + -d "disable_web_page_preview=true" + + prover-tests: + name: Prover Subset Tests (M3 Ultra) + # Narrow label-gated subset for fast developer iteration on + # Plonky2 prover changes. Runs ONLY the mint / send / receive + # flows in `account_node::tests` plus the persist/load roundtrip + # (which exercises the wallet end-to-end). Estimated ~25 min on + # an M3 Ultra agent. + # + # Mutually exclusive with `ci:full` — see the matching comment + # on `db-tests` above for the rationale. + if: >- + contains(github.event.pull_request.labels.*.name, 'ci:prover') + && !contains(github.event.pull_request.labels.*.name, 'ci:full') + needs: lint-and-build + runs-on: [self-hosted, m3-ultra] + timeout-minutes: 60 + env: + # Mirror of the `db-tests` env block above — see there for + # rationale on each var. The env shape is identical because + # both subset jobs share the same bootstrap requirements + # (chain-shaping vars are mandatory, the publisher key must + # match the wiremock'd mocks in `router_tests.rs`). + IS_MAINNET: "false" + ESPLORA_URL: http://127.0.0.1:1/api + ESPLORA_WS_URL: ws://127.0.0.1:1/api/v1/ws + USERNAME_DOMAIN: test.zkcoins.local + PUBLISHER_KEY: "0000000000000000000000000000000000000000000000000000000000000001" + RUSTC_WRAPPER: sccache + SCCACHE_CACHE_SIZE: "50G" + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Prepend ~/.cargo/bin to PATH (use rustup proxy, not Homebrew Rust) + run: echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" + + - name: Set DOCKER_HOST for Colima socket + run: echo "DOCKER_HOST=unix://$HOME/.colima/default/docker.sock" >> "$GITHUB_ENV" + - name: Ensure sccache + cargo-nextest are installed run: | command -v sccache >/dev/null || brew install sccache @@ -258,80 +412,104 @@ jobs: sccache --start-server >/dev/null 2>&1 || true sccache --show-stats - # The `db_tests` added in PR-A1 use testcontainers to spin up a - # real Postgres 17 per test. The runner host has Docker (via - # Colima) available on PATH; fail fast with a readable error - # if it ever goes away, instead of letting the test suite die - # 5 minutes into the run with a hard-to-read bollard error. + # The `test_persist_and_load_from_pg_roundtrip` test in this + # subset uses testcontainers, so Docker must be reachable. - name: Verify Docker is reachable (testcontainers dependency) run: docker info > /dev/null - # `cargo nextest` replaces `cargo test`: process-per-test isolation - # plus smart scheduling (slow tests start first). `--test-threads 1` - # is preserved — the repo invariant is that tests run serially to - # avoid testcontainers port races and shared-state pollution. - # `api_remote` is the live-DEV-node verification integration test - # (node/tests/api_remote.rs). It targets `https://dev-api.zkcoins.app` - # by default and is meant to run AFTER a deploy, from the `api-e2e` - # job in deploy-dev.yaml — not against whatever DEV currently runs - # while a PR is still open. Excluding it here keeps `node-tests` - # hermetic: only unit + non-remote integration tests run; remote - # verification fires post-deploy as the merge-then-deploy gate. - - name: Run node + shared tests (release, all features) - run: cargo nextest run -p node -p shared --release --all-features --test-threads 1 -E 'not binary(api_remote)' + # Subset filter — the full account_node send/mint/receive + # surface. Includes both Plonky2-heavy happy paths and pure-Rust + # error-path tests (e.g. `test_send_coins_returns_err_for_unknown_account`, + # `test_send_coins_rejects_too_many_invoices`), so the gate is + # conservative and runs anything touching account-node state + # transitions. The `test_persist_and_load_from_pg_roundtrip` test + # exercises the wallet end-to-end (build → persist → reload → + # reuse), so it lives in BOTH subsets by design; nextest + # deduplicates within a single run, this is harmless when both + # subsets are run on separate PR labels. + - name: Run Prover subset (release, plain nextest, no coverage) + run: | + cargo nextest run -p node -p shared --release --all-features --test-threads 1 \ + -E 'not binary(api_remote) & (test(/^account_node::tests::test_mint/) + test(/^account_node::tests::test_send/) + test(/^account_node::tests::test_receive/) + test(/^account_node::tests::test_persist_and_load_from_pg_roundtrip/) + test(/^account_node::tests::test_wallet_operations/))' - name: sccache stats (post-build) if: always() run: sccache --show-stats - coverage: - name: Coverage Gate (100% lines + functions) - # Runs in parallel with `node-tests` (not after) — both jobs - # exercise the same suite (nextest vs. nextest-under-llvm-cov), so - # serializing them only doubled wall-clock on every Release PR. - # The `ci:full` label gate is duplicated explicitly here because the - # chain through `node-tests` (which carried the guard) is broken. + # Mirror of the `notify-failure` job downstream — see the + # matching comment on `db-tests` for the rationale. + - name: Telegram alert on failure + if: failure() + env: + TG_TOKEN: ${{ secrets.TELEGRAM_BOT_TOKEN }} + TG_CHAT: ${{ secrets.TELEGRAM_CHAT_ID }} + run: | + TEXT=$'❌ '"${{ github.workflow }}"$' / prover-tests failed\nRepo: '"${{ github.repository }}"$'\nBranch: '"${{ github.ref_name }}"$'\nRun: '"${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + curl -sS -X POST "https://api.telegram.org/bot${TG_TOKEN}/sendMessage" \ + --data-urlencode "chat_id=${TG_CHAT}" \ + --data-urlencode "text=${TEXT}" \ + -d "parse_mode=HTML" \ + -d "disable_web_page_preview=true" + + test-and-coverage: + name: Tests + Coverage Gate (M3 Ultra, 100% lines + functions) + # Authoritative heavy gate: runs the full nextest suite under + # llvm-cov instrumentation, producing both test execution AND + # coverage data in a single binary run. Replaces the previous + # `node-tests` + `coverage` pair (the two jobs ran the same + # nextest suite — see the file header for the merge rationale). + # + # Gated behind the `ci:full` label so we don't burn runner time + # on every speculative PR. The Release PR (`develop -> main`) + # gets the label applied automatically by auto-release-pr.yaml. if: contains(github.event.pull_request.labels.*.name, 'ci:full') needs: lint-and-build runs-on: [self-hosted, m3-ultra] - timeout-minutes: 90 + timeout-minutes: 120 env: # All three chain-shaping env vars are required by the node - # bootstrap (see `lib::build_network_config_from_env`). Mirror - # the `node-tests` env block above — `127.0.0.1:1` endpoints - # fail fast for any commit-pipeline / scanner-WS path. + # bootstrap (see `lib::build_network_config_from_env`). CI uses + # `127.0.0.1:1` endpoints so any test that exercises the commit + # pipeline / scanner WS fails fast instead of reaching a public + # third-party host (a previous Mutinynet-flavoured silent + # fallback used to add >60 s per test). IS_MAINNET: "false" ESPLORA_URL: http://127.0.0.1:1/api ESPLORA_WS_URL: ws://127.0.0.1:1/api/v1/ws + # `USERNAME_DOMAIN` is required by the node bootstrap (no + # default — see node/src/main.rs and issue #95). The test value + # is irrelevant for the `info_returns_*` assertions (they only + # check non-empty + shape). USERNAME_DOMAIN: test.zkcoins.local - # `PUBLISHER_KEY` is required on every network (no default — see - # `node/src/lib.rs`); the value mirrors `node-tests` above and is - # a syntactically valid 32-byte hex placeholder, NOT a secret. - # MUST match `node/src/router_tests.rs` and the `node-tests` env - # block — the test mocks derive the wiremock'd publisher address - # from this key. + # `PUBLISHER_KEY` is required on every network (no default — + # see `node/src/lib.rs`); the value mirrors the subset jobs + # above and is a syntactically valid 32-byte hex placeholder, + # NOT a secret. MUST match `node/src/router_tests.rs` — the + # test mocks derive the wiremock'd publisher address from this + # key. PUBLISHER_KEY: "0000000000000000000000000000000000000000000000000000000000000001" - # `db_tests` use the `testcontainers` crate; see `node-tests` - # above for the rationale. `DOCKER_HOST` is set in a step below - # so the Colima socket path resolves from `$HOME` at runtime. - # Same sccache wrapper as `node-tests`; reuses the same on-disk - # cache populated by the previous job in the same workflow run. + # `db_tests` use the `testcontainers` crate; see the subset + # jobs above for the rationale. `DOCKER_HOST` is set in a step + # below so the Colima socket path resolves from `$HOME` at + # runtime. + # Same sccache wrapper as the subset jobs; reuses the same + # on-disk cache populated by previous runs on the same runner. RUSTC_WRAPPER: sccache - # See `node-tests` env block above for the 50-GiB rationale. + # See the subset jobs above for the 50-GiB rationale. SCCACHE_CACHE_SIZE: "50G" # Activate the workspace's `coverage_nightly` cfg gate so the # `#[cfg_attr(coverage_nightly, coverage(off))]` annotations # (14× repo-wide, plus the platform-detection helpers in # `node/src/r2_probe.rs`) actually take effect under # `cargo llvm-cov`. cargo-llvm-cov does NOT auto-set this cfg — - # without it every `coverage(off)` in the workspace is inert and - # llvm-cov counts the excluded fns / lines as uncovered, which - # silently broke the 100%-line + 100%-function gate the moment - # the first annotation landed in the `node` crate. Set only on - # the coverage job: the `lint-and-build` job runs stable 1.81.0 - # and would reject `feature(coverage_attribute)`, and - # `node-tests` doesn't need the cfg (test execution is - # orthogonal to the gate). + # without it every `coverage(off)` in the workspace is inert + # and llvm-cov counts the excluded fns / lines as uncovered, + # which silently broke the 100%-line + 100%-function gate the + # moment the first annotation landed in the `node` crate. Set + # only on this job: the `lint-and-build` job runs stable + # 1.81.0 and would reject `feature(coverage_attribute)`, and + # the subset jobs run plain nextest (no instrumentation) so + # the cfg has no effect there. RUSTFLAGS: "--cfg coverage_nightly" steps: - name: Checkout @@ -340,14 +518,14 @@ jobs: - name: Prepend ~/.cargo/bin to PATH (use rustup proxy, not Homebrew Rust) run: echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" - # See `node-tests` job above for the rationale; resolves the + # See the subset jobs above for the rationale; resolves the # Colima socket path from `$HOME` at runtime. - name: Set DOCKER_HOST for Colima socket run: echo "DOCKER_HOST=unix://$HOME/.colima/default/docker.sock" >> "$GITHUB_ENV" - # Same install gate as `node-tests`. Idempotent: no-op on a - # warm runner where both tools already exist. See `node-tests` - # for why we conditionally restart the sccache server. + # Same install gate as the subset jobs. Idempotent: no-op on a + # warm runner where both tools already exist. See the subset + # jobs for why we conditionally restart the sccache server. - name: Ensure sccache + cargo-nextest are installed run: | command -v sccache >/dev/null || brew install sccache @@ -358,27 +536,45 @@ jobs: sccache --start-server >/dev/null 2>&1 || true sccache --show-stats - # Coverage runs the same `db_tests` as `node-tests` and so - # needs Docker reachable for testcontainers. See the matching - # check in the `node-tests` job for the rationale. + # Same `db_tests` as the subset jobs and so needs Docker + # reachable for testcontainers. See the matching check in the + # subset jobs for the rationale. - name: Verify Docker is reachable (testcontainers dependency) run: docker info > /dev/null - # `cargo llvm-cov nextest` is the nextest-aware coverage subcommand: - # collects llvm-cov data while driving the suite through nextest, - # so the 100% line/function gate and the test execution share a - # single binary run (same as the old `cargo llvm-cov -- ...` form). + # `cargo llvm-cov nextest` is the nextest-aware coverage + # subcommand: collects llvm-cov data while driving the suite + # through nextest, so the 100% line/function gate and the test + # execution share a single binary run. This is the merge of + # the previous `node-tests` + `coverage` pair — the previous + # `node-tests` job ran the same nextest invocation without the + # `cargo llvm-cov` wrapper, which produced no extra signal. + # + # `-p node -p shared --all-features` matches the previous + # `node-tests` test set exactly (the previous `coverage` job + # was scoped to `-p node` because the coverage GATE is only + # measured against the `node` crate; the merge keeps that gate + # scope while widening the EXECUTED set to `-p node -p shared` + # so the shared crate's `commitment::tests::*` keep running in + # the heavy gate — they were part of `node-tests` before). + # The `shared/src/commitment.rs` entry in --ignore-filename-regex + # keeps the coverage gate strictness identical to the previous + # `-p node`-scoped gate: the shared crate's source files are + # excluded from the 100% measurement, only the `node` crate is + # gated. `--all-features` likewise mirrors the previous + # `node-tests` invocation so opt-in feature-gated code paths + # still execute. # # The `api_remote` integration test (node/tests/api_remote.rs) - # is excluded for the same reason as in `node-tests` above: it - # targets the live DEV node and belongs in the post-deploy - # `api-e2e` job, not the hermetic coverage gate. The MVP coverage - # scope is measured by the rest of the suite, which covers the - # in-process axum handlers via oneshot(). - - name: Run llvm-cov (MVP scope, 100% line + function gate) + # is excluded: it targets the live DEV node and belongs in + # the post-deploy `api-e2e` job in deploy-dev.yaml, not the + # hermetic gate. The MVP coverage scope is measured by the + # rest of the suite, which covers the in-process axum handlers + # via oneshot(). + - name: Run llvm-cov nextest (MVP scope, 100% line + function gate) run: | - cargo llvm-cov nextest --release -p node --show-missing-lines \ - --ignore-filename-regex 'main\.rs|lib\.rs|publisher\.rs|runtime\.rs|scanner_runtime\.rs|scanner_ws\.rs|_tests\.rs$|bin/.*\.rs$' \ + cargo llvm-cov nextest --release -p node -p shared --all-features --show-missing-lines \ + --ignore-filename-regex 'main\.rs|lib\.rs|publisher\.rs|runtime\.rs|scanner_runtime\.rs|scanner_ws\.rs|_tests\.rs$|bin/.*\.rs$|shared/src/.*\.rs$' \ --fail-under-lines 100 \ --fail-under-functions 100 \ --test-threads 1 \ @@ -400,10 +596,10 @@ jobs: run: | echo "--- llvm-cov report: --show-missing-lines (text) ---" cargo llvm-cov report --release --show-missing-lines \ - --ignore-filename-regex 'main\.rs|lib\.rs|publisher\.rs|runtime\.rs|scanner_runtime\.rs|scanner_ws\.rs|_tests\.rs$|bin/.*\.rs$' || true + --ignore-filename-regex 'main\.rs|lib\.rs|publisher\.rs|runtime\.rs|scanner_runtime\.rs|scanner_ws\.rs|_tests\.rs$|bin/.*\.rs$|shared/src/.*\.rs$' || true echo "--- llvm-cov report: per-file json (filter < 100%) ---" cargo llvm-cov report --release --json \ - --ignore-filename-regex 'main\.rs|lib\.rs|publisher\.rs|runtime\.rs|scanner_runtime\.rs|scanner_ws\.rs|_tests\.rs$|bin/.*\.rs$' \ + --ignore-filename-regex 'main\.rs|lib\.rs|publisher\.rs|runtime\.rs|scanner_runtime\.rs|scanner_ws\.rs|_tests\.rs$|bin/.*\.rs$|shared/src/.*\.rs$' \ | jq -r '.data[0].files[] | select(.summary.lines.percent < 100 or .summary.functions.percent < 100) | {filename, lines: .summary.lines, functions: .summary.functions}' \ @@ -413,15 +609,19 @@ jobs: if: always() run: sccache --show-stats - # Telegram alert on workflow failure. Modelled as a separate job (not - # an inline step) so job-level failures — timeout, OOM, runner crash — - # still fire the alert. `if: failure()` evaluates against the whole - # `needs:` group: any listed job transitioning to `failure` triggers - # it, while skipped jobs (node-tests / coverage on a non-ci:full PR, - # or all jobs on a draft PR) and manual cancellation stay silent. + # Telegram alert on workflow failure for the heavy gate. Modelled + # as a separate job (not an inline step) so job-level failures — + # timeout, OOM, runner crash — still fire the alert. `if: failure()` + # evaluates against the whole `needs:` group: any listed job + # transitioning to `failure` triggers it, while skipped jobs + # (`test-and-coverage` on a non-ci:full PR, or all jobs on a draft + # PR) and manual cancellation stay silent. The subset jobs + # (`db-tests` / `prover-tests`) fire their own inline Telegram + # alerts so a subset failure does not get masked by the heavy + # gate's status under the workflow-level conclusion. notify-failure: name: Telegram alert on failure - needs: [lint-and-build, node-tests, coverage] + needs: [lint-and-build, test-and-coverage] if: failure() runs-on: ubuntu-latest steps: From 206bf87903b011cba70d23740dc3c3cd44fe3fb2 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 1 Jun 2026 22:41:07 +0200 Subject: [PATCH 5/9] fix(api/history): include coin_queue in balance read so first mint surfaces as 50k delta (#168) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `balance_from_account_blob` previously read only `Account.balance`, which is the *settled* balance after sends. The mint and receive paths push the credited coin into `coin_queue` without touching the `balance` field — `Account::get_balance()` is the only call that sums both. Reading just `balance` here made every first-mint history row collapse to `new_balance = 0, prev_balance = 0, amount = 0`, so the wire item reported `amount = 0` for a 50_000-sat credit. The existing unit test masked this because its fixture set `a.balance = 5_000` directly, a shape no production caller produces on the mint or receive path. That test now documents that it pins the settled-balance variant (a valid post-send shape), and a new sibling test `history_row_to_item_balance_from_coin_queue_only` in `account_node_tests` walks the real mint flow (`execute_send_coins` + `receive_coin`) to pin the previously- uncovered queue-only case end to end — including a direct assertion on `balance_from_account_blob` itself. E2E (api_remote::history_after_mint_records_mint_row) flagged this against dev-api on PR #166 (Release develop->main). --- node/src/account_node_tests.rs | 102 +++++++++++++++++++++++++++++++++ node/src/router.rs | 25 ++++++-- node/src/router_tests.rs | 8 +++ 3 files changed, 131 insertions(+), 4 deletions(-) diff --git a/node/src/account_node_tests.rs b/node/src/account_node_tests.rs index b578fa5a..5e959b20 100644 --- a/node/src/account_node_tests.rs +++ b/node/src/account_node_tests.rs @@ -1389,3 +1389,105 @@ fn warmup_prover_completes_successfully() { node.warmup_prover() .expect("warmup_prover must succeed on a fresh AccountNode"); } + +/// Pins the **queue-only** shape produced by the production mint / +/// receive paths: the credited coin lives in `Account.coin_queue` while +/// `Account.balance` remains `0` until a subsequent send drains the +/// queue. `router::balance_from_account_blob` must mirror +/// `Account::get_balance()` and surface the sum, otherwise the +/// `/api/history` row for a first mint reports `amount = 0` (the bug +/// the `history_after_mint_records_mint_row` E2E flagged on PR #166). +/// +/// Lives in `account_node_tests` because constructing a realistic +/// `CoinProof` requires the full prover + state fixtures — the lighter +/// settled-balance shape (`balance > 0, coin_queue == []`) is still +/// covered in `router_tests::history_row_to_item_handles_first_row_with_no_prev_data`. +#[test] +fn history_row_to_item_balance_from_coin_queue_only() { + let state_arc = Arc::new(Mutex::new(State::new())); + let mut node = AccountNode::new(Arc::clone(&state_arc)); + + let mut minting = TestAccountData::new_minting_account(); + node.import_account( + minting.address, + Account { + proof: None, + coin_queue: vec![], + coin_history: SparseMerkleTree::new(), + balance: 1_000_000, + num_sends: 0, + commitment_public_key: None, + }, + ); + + let recipient = TestAccountData::new_generic(&[42u8; 32], Network::Signet); + const MINT_AMOUNT: u64 = 50_000; + + // Mint flow: the minting account sends MINT_AMOUNT to a fresh + // recipient. `receive_coin` then pushes the resulting `CoinProof` + // into the recipient's `coin_queue` without touching `balance` — + // this is the exact write `commit_mint_tx` produces for a real + // first-mint history row. + let mut coin_proofs = minting + .execute_send_coins( + &mut node, + vec![Invoice::new(MINT_AMOUNT, recipient.address)], + ) + .expect("mint send_coins"); + state_arc + .lock() + .unwrap() + .update( + &coin_proofs + .iter() + .map(|x| x.commitment.clone().unwrap()) + .collect::>(), + ) + .expect("state.update"); + node.receive_coin(coin_proofs.pop().expect("at least one coin")) + .expect("recipient receive_coin"); + + let recipient_account = node + .accounts + .get(&recipient.address) + .expect("recipient account present after receive_coin"); + assert_eq!( + recipient_account.balance, 0, + "settled balance is still 0 — the credit sits in coin_queue" + ); + assert_eq!( + recipient_account.coin_queue.len(), + 1, + "exactly one queued coin" + ); + assert_eq!(recipient_account.coin_queue[0].coin.amount, MINT_AMOUNT); + + // Direct helper assertion: balance_from_account_blob must include + // the queue contribution. + let new_data = bincode::serialize(recipient_account).expect("bincode serialize"); + assert_eq!( + crate::router::balance_from_account_blob(&new_data), + Some(MINT_AMOUNT), + "balance_from_account_blob must sum balance + coin_queue (mirrors Account::get_balance)" + ); + + // End-to-end through history_row_to_item: a first mint row + // (prev_data = None) must surface `amount = MINT_AMOUNT`. + let row = crate::db::AccountHistoryRow { + id: 7, + timestamp_secs: 1_700_000_000, + source: "mint".to_string(), + prev_data: None, + new_data, + commit_txid: None, + block_height: None, + pending_status: None, + }; + let item = crate::router::history_row_to_item(&row).expect("item produced"); + assert_eq!(item.id, 7); + assert_eq!(item.direction, "mint"); + assert_eq!( + item.amount, MINT_AMOUNT, + "first mint must surface the full credit (regression: was 0 when balance_from_account_blob read only Account.balance)" + ); +} diff --git a/node/src/router.rs b/node/src/router.rs index 83821084..50ae31dd 100644 --- a/node/src/router.rs +++ b/node/src/router.rs @@ -474,15 +474,32 @@ pub(crate) fn map_history_direction(source: &str) -> Option<&'static str> { } } -/// Recover the `balance` field out of a bincode-serialised +/// Recover the usable balance out of a bincode-serialised /// [`crate::account_node::Account`] blob. Returns `None` if the bytes /// fail to round-trip — defensive, the handler treats a decode failure /// as a missing prior balance (so the delta collapses to the absolute /// new balance instead of producing a fabricated number). +/// +/// Mirrors [`crate::account_node::Account::get_balance`]: the settled +/// `balance` field plus pending receives sitting in `coin_queue`. +/// Mints and receives push the credited coin into `coin_queue` without +/// touching `balance` until a subsequent send drains the queue into +/// `coin_history`; reading only `a.balance` here would report `0` for +/// the very transactions the history endpoint is meant to surface (the +/// E2E suite catches this as `amount = 0` on first mint). +/// +/// `saturating_add` is used because the two summands come out of an +/// untrusted on-disk blob; in practice overflow is impossible (per-coin +/// amounts and `Account.balance` are both bounded by the minting +/// account's supply), but capping at `u64::MAX` is preferable to a +/// panic on a corrupted row. pub(crate) fn balance_from_account_blob(blob: &[u8]) -> Option { - bincode::deserialize::(blob) - .ok() - .map(|a| a.balance) + let a = bincode::deserialize::(blob).ok()?; + let queued: u64 = a + .coin_queue + .iter() + .fold(0u64, |acc, cp| acc.saturating_add(cp.coin.amount)); + Some(a.balance.saturating_add(queued)) } /// Typed mirror of the `pending_inscriptions.status` CHECK constraint diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index f2c395b0..f1e373da 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -6853,6 +6853,14 @@ fn balance_from_account_blob_round_trips() { assert!(balance_from_account_blob(&[0u8, 1, 2, 3]).is_none()); } +/// Covers the **settled-balance** shape of an `Account` blob: a post-send +/// account whose `coin_queue` has been drained into `coin_history` and +/// whose remaining funds sit in the `balance` field. The companion +/// **queue-only** shape (the actual production write produced by +/// `commit_mint_tx` / `receive_coin` for a credit) requires a real +/// `CoinProof` and is pinned in +/// `account_node_tests::history_row_to_item_balance_from_coin_queue_only` +/// where the prover fixtures live. #[test] fn history_row_to_item_handles_first_row_with_no_prev_data() { let mut a = Account::new(); From c44a5e6a3b2054e45117bd571b6490d50f372588 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Tue, 2 Jun 2026 01:07:01 +0200 Subject: [PATCH 6/9] =?UTF-8?q?docs(contributing):=20anchor=20trust=20mode?= =?UTF-8?q?l=20=E2=80=94=20node=20is=20trusted,=20wallet=20is=20thin=20(#1?= =?UTF-8?q?71)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CONTRIBUTING.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1f9be67f..76418a3e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,13 +2,26 @@ This guide covers everything you need to develop, test, and deploy the zkCoins backend. -The first section, "Working on the Plonky2 Migration", documents the project invariants, the decision recipe for "should this go in the MVP?", the pre-push checklist, and the known foot-guns. It applies to all work on `develop` after the 2026-05-18 SP1 → Plonky2 cutover. The rest of this file is the dev guide for day-to-day node work. +## Trust model — node is trusted, wallet is thin + +zkCoins is built around a single trust assumption: **the wallet trusts the node it talks to.** The only line the node is not allowed to cross is the wallet's private key — that stays in the wallet. Everything else may be delegated. + +This is a hard project rule. It shapes every design and implementation decision: + +- **No anti-node logic in the wallet or SDK.** No client-side proof verification, no scan loops, no view-key / spend-key splits, no consistency checks against a second node, no "node integrity" indicators in the UI. If a feature exists to reduce trust in the node, it does not belong in the wallet or SDK. +- **Self-hosting is the escape hatch.** Users who do not want to trust the public operator run their own node. The wallet must always be able to switch to a different node by changing a single configuration value. +- **The node is built so that self-hosting is easy.** Single container, documented configuration, deterministic state, no operator-specific dependencies. +- **The SDK and wallet stay thin.** They expose seed + address + the small set of operations every familiar wallet SDK exposes. Integrators (Cake Wallet, LayerZ, BlueWallet, …) should be able to wire zkCoins up with the same effort as adding a second Bitcoin-family chain. + +When in doubt about whether a feature belongs in the wallet, SDK, or node: if it exists to reduce trust in the node, build it node-side, or document self-hosting as the answer. This rule is mirrored verbatim in [`zk-coins/node`](https://github.com/zk-coins/node/blob/develop/CONTRIBUTING.md), [`zk-coins/sdk`](https://github.com/zk-coins/sdk/blob/develop/CONTRIBUTING.md), [`zk-coins/app`](https://github.com/zk-coins/app/blob/develop/CONTRIBUTING.md), and [`zk-coins/docs`](https://github.com/zk-coins/docs/blob/develop/CONTRIBUTING.md). --- ## Working on the Plonky2 Migration -Canonical entry point for any session (agent or human) picking up the +This section documents the project invariants, the decision recipe for "should this go in the MVP?", the pre-push checklist, and the known foot-guns. It applies to all work on `develop` after the 2026-05-18 SP1 → Plonky2 cutover. The rest of this file is the dev guide for day-to-day node work. + +It is the canonical entry point for any session (agent or human) picking up the codebase without prior context. The Plonky2 migration (PR [#17](https://github.com/zk-coins/node/pull/17)) merged on 2026-05-18; this section captures the project invariants that survive the migration. Read this section, then dive into the linked From 6b39bee274f4e764ac0bc8ff52171ffe10f71228 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Tue, 2 Jun 2026 01:51:59 +0200 Subject: [PATCH 7/9] ci(auto-release-pr): create both Promote+Release PRs as draft (#173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror of zk-coins/app#151 — the GITHUB_TOKEN that opens these PRs hits GitHub's anti-recursion policy and silently skips ci.yaml, leaving every staging-bound and develop-bound auto-PR without a pre-merge CI gate. Creating as DRAFT lets the operator's explicit `gh pr ready` toggle fire the `ready_for_review` event that IS allowed to trigger downstream workflows, so the full Lint & Build plus (with `ci:full` already applied at creation) Node + Shared Tests + Coverage Gate run against the actual PR HEAD before merge. Both workflows in this repo carry the same one-line addition: - auto-release-pr-staging.yaml (staging → develop) - auto-release-pr.yaml (develop → main, keeps ci:full label) Operator UX: one extra click. `gh pr ready ` (or the UI button) promotes the PR + runs CI in a single step. --- .github/workflows/auto-release-pr-staging.yaml | 6 ++++++ .github/workflows/auto-release-pr.yaml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/.github/workflows/auto-release-pr-staging.yaml b/.github/workflows/auto-release-pr-staging.yaml index b59606cd..ed848c3d 100644 --- a/.github/workflows/auto-release-pr-staging.yaml +++ b/.github/workflows/auto-release-pr-staging.yaml @@ -64,7 +64,13 @@ jobs: "- [ ] Merge to promote staging to develop (deploys to DEV)" \ > /tmp/pr-body.md + # Created as DRAFT so the operator's `gh pr ready` is the + # explicit gate that fires a `ready_for_review` event and + # triggers ci.yaml — PRs opened via GITHUB_TOKEN would + # otherwise hit GitHub's anti-recursion policy and skip + # downstream workflows entirely. gh pr create \ + --draft \ --base develop \ --head staging \ --title "Promote: staging -> develop" \ diff --git a/.github/workflows/auto-release-pr.yaml b/.github/workflows/auto-release-pr.yaml index 44d1feba..461f1a02 100644 --- a/.github/workflows/auto-release-pr.yaml +++ b/.github/workflows/auto-release-pr.yaml @@ -68,7 +68,13 @@ jobs: --description "Run heavy M3 Ultra test + coverage jobs on this PR" \ 2>/dev/null || true + # Created as DRAFT — same rationale as auto-release-pr-staging.yaml: + # the operator's explicit `gh pr ready` toggle fires the + # `ready_for_review` event that triggers ci.yaml. The `ci:full` + # label is still applied at creation so the heavy M3 Ultra + # tests + Coverage Gate run as soon as the PR is marked ready. gh pr create \ + --draft \ --base main \ --head develop \ --title "Release: develop -> main" \ From 9bf6e8d1f72126918bda2fb4ca5eae102f158f9d Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Tue, 2 Jun 2026 02:43:49 +0200 Subject: [PATCH 8/9] test(router/lnurl): cover localhost branch of lnurlp_handler scheme selection (#172) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lnurlp_handler picks http:// for hosts containing 'localhost' and https:// otherwise. The https arm is already pinned by lnurlp_known_address_returns_pay_request; the http arm (router.rs:2647) was uncovered, which broke the 100%-line coverage gate at 3265/3266 lines = 99.97%. New test lnurlp_localhost_host_returns_http_callback issues the same .well-known/lnurlp/ request the existing test does, but with Host: localhost:8080, and asserts the callback URL starts with http://localhost:8080/. Closes the 1-line gap without changing production code. Surfaced as part of the dfxai runner pool smoke test on PR #169 — see DFXServer/server commit 4347a4a for the new dfxai CI host. --- node/src/router_tests.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index f1e373da..651b30e5 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -499,6 +499,38 @@ async fn lnurlp_known_address_returns_pay_request() { assert!(resp.metadata.contains("zkCoins")); } +#[cfg(feature = "lnurl")] +#[tokio::test] +async fn lnurlp_localhost_host_returns_http_callback() { + // Pins the `host.contains("localhost")` branch of `lnurlp_handler`'s + // scheme selection: when the request's Host header points at a local + // dev instance, the LNURL callback URL must be served back as `http://` + // so wallets following the redirect don't hit a TLS error against + // the dev node. The api.zkcoins.app path (covered by + // `lnurlp_known_address_returns_pay_request`) already pins the + // `https://` arm. + let full_hex = hex::encode(zkcoins_program::hash::digest_to_bytes( + &zkcoins_program::types::MINTING_ADDRESS, + )); + let prefix = &full_hex[..8]; + + let uri = format!("/.well-known/lnurlp/{}", prefix); + let req = Request::get(&uri) + .header("host", "localhost:8080") + .body(Body::empty()) + .unwrap(); + let (status, body) = send_request(req).await; + + assert_eq!(status, StatusCode::OK); + + let resp: LnurlpResponse = serde_json::from_str(&body).expect("valid JSON"); + assert!( + resp.callback.starts_with("http://localhost:8080/"), + "callback should use http://localhost:8080 — got {}", + resp.callback + ); +} + // --- GET /lnurl/pay/{username} --- #[cfg(feature = "lnurl")] From 38db7b61e44d61984c6f487565208cea92b9da7c Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Tue, 2 Jun 2026 09:46:04 +0200 Subject: [PATCH 9/9] fix(ci/deploy-dev): smoke-test gates on /health/ready, not /api/info (#174) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-deploy smoke test was checking `/api/info` for HTTP 200, but after #154 the node binds the HTTP listener BEFORE the Plonky2 prover warmup completes — `/api/info` returns 200 within seconds while `/health/ready` stays at `{"ready":false,"prover":"warming"}` for the 10-30 s warmup window. Downstream jobs (API E2E preflight against `/health/ready` + `/health/publisher`) raced the warmup: the E2E job picked the runner up ~4 s after the deploy job reported success, hit `/health/ready` once, got back the warming snapshot, and failed with `::error::/health/ready not ready` — observed empirically on Release PR #166's run https://github.com/zk-coins/node/actions/runs/26793933906/job/78986599030. Switch the smoke loop to `/health/ready` + a `jq '.ready == true'` assertion, keeping the 30-attempt × 10-s budget (~5 min) so a genuine bootstrap stall still surfaces with the same timeout behaviour. The deploy job now only reports success once the node is actually ready for traffic, which removes the race the E2E preflight was tripping over. `/api/info` is no longer a deploy-success signal. The E2E preflight retains its explicit `/health/ready` + publisher-wallet gate as a sanity check (still a single shot — it relies on the smoke test having already enforced readiness). The deploy job runs on `ubuntu-24.04-arm` where `jq` is part of the default GitHub-hosted image; no install step needed here. --- .github/workflows/deploy-dev.yaml | 43 +++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/.github/workflows/deploy-dev.yaml b/.github/workflows/deploy-dev.yaml index fa140756..32ce85b8 100644 --- a/.github/workflows/deploy-dev.yaml +++ b/.github/workflows/deploy-dev.yaml @@ -102,28 +102,45 @@ jobs: ${{ secrets.DEPLOY_DEV_USER }}@${{ secrets.DEPLOY_DEV_HOST }} \ "$DEPLOY_CMD" - # Post-deploy smoke test: hit the public endpoint until /api/info - # answers 200 or we give up. A green "Build and deploy to DEV" - # without this step was historically misleading — a runtime-bootstrap - # panic left the container Up-but-unresponsive while the workflow - # reported success. Failing this step blocks the auto-release PR - # from collecting a green check and surfaces the regression in CI. + # Post-deploy smoke test: hit the public endpoint until + # /health/ready reports `ready: true` (or we give up). A green + # "Build and deploy to DEV" without this step was historically + # misleading — a runtime-bootstrap panic left the container + # Up-but-unresponsive while the workflow reported success. + # Failing this step blocks the auto-release PR from collecting + # a green check and surfaces the regression in CI. + # + # `/health/ready` (not `/api/info`) is the load-bearing gate. + # Post-#154 the node binds the HTTP listener immediately and + # warms the Plonky2 prover in a background task; `/api/info` + # returns 200 within seconds, but `/health/ready` stays at + # `{"ready":false,"prover":"warming"}` for the 10-30 s warmup. + # Downstream jobs (E2E preflight, smoke tests against the + # publisher wallet) gated on `/health/ready` and were racing + # the warmup — observed empirically in + # https://github.com/zk-coins/node/actions/runs/26793933906/job/78986599030 + # (Release PR #166, prover still warming at +4 s after the + # E2E job picked the runner up). Polling `/health/ready` here + # means the deploy job only reports success once the node is + # actually ready for traffic. - name: Smoke test public endpoint run: | set -euo pipefail - URL="https://dev-api.zkcoins.app/api/info" + URL="https://dev-api.zkcoins.app/health/ready" for i in $(seq 1 30); do - code=$(curl -sS -o /tmp/info.json -w '%{http_code}' --max-time 10 "$URL" || echo "000") - if [ "$code" = "200" ]; then - echo "DEV /api/info responded 200 after ${i} attempt(s):" - cat /tmp/info.json + body=$(curl -sS -o /tmp/ready.json -w '%{http_code}' --max-time 10 "$URL" || echo "000") + code="$body" + if [ "$code" = "200" ] && jq -e '.ready == true' /tmp/ready.json > /dev/null 2>&1; then + echo "DEV /health/ready reports ready=true after ${i} attempt(s):" + cat /tmp/ready.json echo exit 0 fi - echo "[$i/30] $URL -> ${code} (waiting 10 s)" + ready_snap=$(jq -c '. // "(no body)"' /tmp/ready.json 2>/dev/null || echo "(non-json)") + echo "[$i/30] $URL -> ${code} ${ready_snap} (waiting 10 s)" sleep 10 done - echo "::error::DEV /api/info never returned 200 within ~5 min after deploy" + echo "::error::DEV /health/ready never reported ready=true within ~5 min after deploy" exit 1 # Functional verification of the deployed DEV node.