release: mainnet-hardening bundle - #117
Merged
Merged
Conversation
Empty commit to seed the release branch. The actual changes land via: - #114 fix(ci): set ZKCOINS_E2E_ALLOW_FEATURE_TRIMMED_SERVER on PRD api-e2e - #115 fix(network-config): require ESPLORA_URL + ESPLORA_WS_URL on Mainnet The branch bundles the two PRs so they reach develop as a single "mainnet hardening" release rather than two independent develop pushes. The maintainer squash-merges this PR after both #114 and #115 land here.
…114) PRD image ships the same MVP-only binary as DEV per Dockerfile policy, but deploy-prd.yaml's api-e2e step was missing the escape-hatch env var that lets feature-gated tests (address-list, lnurl) skip cleanly instead of panicking the CI canary. PR #18 (develop→main auto-release) brought the feature-trimmed-server check from PR #105 to main, and the next PRD deploy (run 26441824314) failed with 4 panics on tests gated by features the MVP build never enables. Mirror deploy-dev.yaml's env block on the PRD api-e2e step.
…115) `NETWORK_CONFIG` silently fell back to the Mutinynet defaults (`https://mutinynet.com/api`, `wss://mutinynet.com/api/v1/ws`) when the env var was missing, regardless of `IS_MAINNET`. On DEV that matches the chain. On Mainnet it's a silent footgun: - An HTTP-only mismatch panics quickly on the first publisher round-trip and the operator sees the breakage immediately. - The new event-driven scanner (#84) instead subscribes to Mutinynet block events and tries to fetch them from the Mainnet HTTP Esplora. Every `get_block_txids` returns 404 and `scanner_runtime` enters a 5 s HTTP-retry loop that never updates `processed_blocks`: the service stays up, `/health/ready` reports green, no chain ingestion happens, no on-chain mint or send commit is ever picked up. The binary never self-heals after restart because no env-derived state has changed. `ESPLORA_URL` and `ESPLORA_WS_URL` are now both **required env vars when `IS_MAINNET=true`** (panic with diagnostic message, mirroring the existing `PUBLISHER_KEY` / `USERNAME_DOMAIN` / `DATABASE_URL` idiom in the same file). Empty / whitespace-only values are treated as unset so a `ESPLORA_URL=` line in a compose file panics with the same message instead of leaving `EsploraConfig.url = ""`. The `IS_MAINNET=false` (DEV / Mutinynet) path is unchanged — both URLs keep their Mutinynet defaults, the pre-push hook and the M3 Ultra coverage gate are unaffected. Implementation: pulled the env-resolution out of the `lazy_static!` block into a pure `build_network_config_from_env<F>(env: F)` so the panic rules are unit-testable without `std::env::set_var` (which would poison the `NETWORK_CONFIG` cell across tests in the same binary). Seven new `#[test]`s cover the headline shapes plus the empty-string and whitespace-only rejection paths. The guard is enforced at the `NETWORK_CONFIG` access path only. `scanner_ws::ScannerWsConfig::from_env` and `publisher.rs` still read `ESPLORA_WS_URL` independently with the Mutinynet fallback — in the main binary the panic in this builder fires first (main.rs dereferences `NETWORK_CONFIG` during bootstrap, before any scanner or publisher env read), so the structural bypass is unreachable today. Closing that bypass by having those sites consume `NETWORK_CONFIG.ws_url` directly is tracked as a follow-up.
Aligns the prose, ASCII trees, and anchor links across CONTRIBUTING, README, ROADMAP, SPEC, MIGRATION_RESEARCH, BRIDGE_MVP, BITVM_BRIDGE, MULTI_ASSET, LIGHTNING_ATOMIC_SWAP, and ARKADE_INTEGRATION with the post-rename module names (node/src/router.rs, the zkCoins node, etc.). External components (Bitcoin Core, electrs/Esplora, Postgres) keep their proper names; bitcoind's server=1 flag and Docker volume zkcoins_node-data are corrected accordingly.
…riptions/:txid (#113) * feat(db): persist inscription kind (mint vs send) + expose via /api/inscriptions/:txid Adds the `kind` column to `pending_inscriptions` so the DB alone tells you what a row represents — previously the table only persisted the publisher's commit/reveal crash-recovery state, and disambiguating mint vs user-send required either grepping container logs (`Sending commitment data` vs. `Broadcasting user commitment`) or deserializing the bincode `Commitment` blob and re-deriving the minting account's current pubkey index. Closed test environment (see CONTRIBUTING.md and the `feedback_zkcoins_closed_test_env` invariant): migration 0006 wipes existing `pending_inscriptions` rows before adding the NOT NULL column. Those rows are crash-recovery state, expected to be empty on a healthy server. Threaded `InscriptionKind` through `insert_pending_inscription` and `create_and_broadcast_inscription`; the two callers tag explicitly: * `router::mint_handler` → `InscriptionKind::Mint` * `runtime::broadcast_commit_and_deliver` → `InscriptionKind::Send` New endpoint `GET /api/inscriptions/:txid` returns the `(kind, status, commit_output_value, timestamps)` tuple for a given commit txid (display order, like every block explorer). Surfaces the DB row's semantics to operators without re-exposing the raw commitment/commit_tx/reveal_tx blobs. Tests updated to pass the new `kind` parameter; root response and test stubs follow. * Merge pull request #116 from zk-coins/feat/request-audit-log feat(audit): persist every HTTP request and response in request_log * feat(db): persist every node input and state transition (full database trail) (#118) Schema + helpers + critical-path wiring for the remaining persistence gaps. After this commit the DB answers every operator-forensic question without falling back to container logs: "what kind of operation was this?" (#113), "every HTTP request body the node received?" (#116), and now: "what did the publisher attempt against Esplora?", "which blocks did the scanner process?", "which inscriptions did it observe (own vs. external)?", "what did each account look like before this change?", "who tried to claim a name and were they refused?", "how long did the reveal-txid mining take?", "what happened during startup?". Closed test env (`feedback_zkcoins_closed_test_env`) + the server-is- not-a-privacy-boundary stance (`feedback_zkcoins_no_privacy_promise`) mean the new columns store everything cleartext — sender/recipient, amounts, signatures, raw commitments. Schema (migration 0008) ----------------------- * `pending_inscriptions.failure_reason TEXT` + `reveal_txid BYTEA` * `esplora_log` — outbound HTTP / WS calls against Esplora * `error_log` — application-level errors, structured, FK back to `request_log` * `block_log` — every processed Bitcoin block * `observed_inscriptions` — every commitment the scanner extracted, tagged own / external * `state_update_log` — every SMT/MMR transition with prev/new roots * `account_history` — every accounts row change, with old + new blob, populated by an `AFTER INSERT OR UPDATE` trigger so coverage is 100% regardless of caller * `username_claim_log` — every claim attempt, success or reject * `tx_mining_log` — reveal-txid prefix-mining stats * `coin_proof_store` — durable mirror of the in-memory `ProofStore` (schema only in this PR) * `boot_log` — startup / shutdown / migration events Wired in this PR ---------------- * `insert_pending_inscription` writes the explicit `reveal_txid`; `/api/inscriptions/:txid` response now includes `reveal_txid` and `failure_reason`. * `create_and_broadcast_inscription` populates `failure_reason` when the broadcast errors out, and persists a `tx_mining_log` row for every reveal-txid mining run (target prefix, nonces tried, duration, final nonce + txid). * `claim_username_handler` records every claim outcome (success + precheck reject + SQL race-loser) in `username_claim_log`. Pure- validation rejects (bad hex / bad signature format) are already captured via `request_log` from PR #116. * `scan_for_inscriptions` takes a `Option<PgPool>` and appends one `block_log` row per processed block (block_hash, height, inscription count, processing duration). * Scanner callback in `main.rs` writes `observed_inscriptions` for every commitment it deserialises — tagged `own` when a matching `pending_inscriptions` row exists, `external` otherwise. * `runtime::start_rest_node` emits a `boot_log` startup event with version, network, listen addr, pid as JSONB metadata. * `account_history` is filled automatically by a PL/pgSQL trigger on `accounts` (INSERT OR UPDATE) — 100% coverage of every existing and future caller, including manual psql edits. Callers with semantic context (`mint`, `send`, `receive`, `recovery`) can override the default `source = 'scanner'` via per-transaction GUC (`SET LOCAL zkcoins.account_source = 'mint'`). Schema + helpers ready, full wiring deferred -------------------------------------------- * `esplora_log` — helper present, instrumentation of the individual esplora-client call-sites (UTXO lookups, broadcast, get_tx, block data) deferred. Many touch points, mechanical. * `error_log` — helper present, replacement of the existing `eprintln!` paths with a `log_err!` macro deferred. ~50 sites across publisher / scanner / runtime / router. * `state_update_log` — helper present, instrumentation of the two state.update sites (mint_handler Phase-E + scanner callback) deferred so this PR's diff stays reviewable. * `coin_proof_store` — schema only; persisting the in-memory ProofStore is a behaviour change that warrants its own PR. CI parity verified locally -------------------------- * `cargo fmt --all --check` ✓ * `cargo clippy -p node -p shared -- -D warnings` ✓ * `cargo clippy -p node --all-features -- -D warnings` ✓ * `cargo check -p node --tests` ✓ * refactor(db): schema polish — fix semantic gaps in the full-trail stack (#119) * refactor(db): schema polish — fix semantic gaps in the full-trail stack Closes the consistency gaps surfaced by the post-#118 review: 1. `pending_inscriptions`: when a broadcast errors out, the row is now advanced to `status = 'failed'` AND `failure_reason` is set atomically (was: only failure_reason, status stayed at the last in-progress state). The CHECK-allowed 'failed' is no longer dead code; the discriminator pairs with the error chain. `update_pending_failure_reason` → `mark_pending_failed`. 2. `observed_inscriptions.integrated` is now actually flipped: after the scanner's `state.update` + atomic `persist_state_tx` land the commitment in SMT/MMR, the matching observed row is updated to `integrated = true, integrated_at = NOW()`. Idempotent via `WHERE integrated = FALSE`. Was: column always false. 3. `account_history_capture()` trigger now reads an optional `zkcoins.request_log_id` GUC. Callers with an HTTP request context can `SET LOCAL` it before the upsert, threading the link to `request_log` through without reimplementing the upsert in application code. The column existed but had no writer; this PR's trigger fills it when the caller provides context. 4. `request_log.client_ip` — new column populated by the audit middleware from `CF-Connecting-IP` (Cloudflare Tunnel is the only ingress on zkcoins-node), falling back to the first segment of `X-Forwarded-For`, then `remote_addr`. `remote_addr` stays as the literal TCP peer (always 127.0.0.1 behind cloudflared) for transport-level forensics. 5. `state_update_log.trigger` → `trigger_source`. Pure rename; no code wired yet so the migration is free. `trigger` collided with Postgres trigger vocabulary at every read. 6. `block_log` consolidated to a single timestamp: drop `received_at` (which was set to NOW() in the same INSERT as `processed_at` — dead weight), keep `processed_at NOT NULL DEFAULT NOW()` as the canonical "scanner saw + processed this block" timestamp. Separate WS-frame-receive logging is a future event-stream feature. 7. `pending_inscriptions.reveal_txid` → `NOT NULL`. Every code path since #118 fills it; the column had been nullable defensively against pre-existing rows, but migration 0006 wiped those, so the defensive nullability buys nothing. Migration deletes any in-flight `reveal_txid IS NULL` rows first (closed test env — see `feedback_zkcoins_migrations_may_wipe`). 8. `esplora_log.triggering_request_log_id` — new column analogous to `error_log.request_log_id`. Outbound Esplora chatter caused by an inbound HTTP request can now be joined back to its `request_log` row. Wiring is follow-up; the column is in place. Verified locally ---------------- * `cargo fmt --all --check` ✓ * `cargo clippy -p node -p shared -- -D warnings` ✓ * `cargo clippy -p node --all-features -- -D warnings` ✓ * `cargo check -p node --tests` ✓ * refactor(db): schema polish round 2 — type safety, FKs, logical checks (#120) Closes the remaining gaps from the round-2 review of the persistence stack. Pure tightening — no new semantics, only stronger constraints and a few cosmetic name cleanups. Type safety ----------- * Length CHECKs on every BYTEA column with a domain-fixed size: txid (32 B) on 7 tables, address (32 B Poseidon) on 5 tables, root hashes (32 B) on mmr_root_index + state_update_log, public_key (33 B compressed secp256k1) on observed_inscriptions, signature (64 B Schnorr BIP-340) on username_claim_log. * `block_log.block_height` nullable instead of sentinel `-1`. Code side now passes `Option<i64>` straight through. * `pending_inscriptions.reveal_txid` UNIQUE (was: only commit_txid). A duplicate reveal_txid is on-chain impossible; UNIQUE makes that an insert-time error instead of an undetected divergence. * The pre-existing partial index `pending_inscriptions_reveal_txid_idx WHERE reveal_txid IS NOT NULL` is dropped — column is NOT NULL since 0009, and the new UNIQUE constraint already builds the required B-Tree. * `tx_mining_log.commit_txid` NOT NULL + length CHECK + FK to `pending_inscriptions(commit_txid)`. The publisher path always sets it; the nullable + FK-less shape was schema drift. Foreign keys ------------ * `tx_mining_log.commit_txid` → `pending_inscriptions.commit_txid` ON DELETE CASCADE (publisher created both rows; lifetime is coupled). * `coin_proof_store.consumed_by_commit_txid` → `pending_inscriptions.commit_txid` ON DELETE SET NULL (proof can outlive the inscription it eventually fed). Logical-pair CHECKs ------------------- Mutually-exclusive flag/timestamp pairs are now enforced by the DB: * `observed_inscriptions`: `integrated` ⇔ `integrated_at IS NOT NULL` * `username_claim_log`: `success` ⇔ `reject_reason IS NULL` * `coin_proof_store`: `consumed_at` ⇔ `consumed_by_commit_txid` * `pending_inscriptions`: `status = 'failed'` ⇒ `failure_reason IS NOT NULL` (the reverse direction is allowed — retried rows may carry a stale reason in a non-failed status, harmless). Vocabulary alignment -------------------- * `esplora_log.triggered_by` → `trigger_source` and CHECK-constrained with the same vocabulary as `state_update_log.trigger_source` (`'mint','send','scanner','recovery','health','resume'`). One concept, one name, one enum. Existing rows with values outside the vocabulary are wiped first (closed test env). * `accounts.created_at`, `latest_block.created_at`, `smt_state.created_at`, `mmr_state.created_at` added — these four pre-existing tables only had `updated_at`, breaking the convention that every domain table tracks both ends of the lifetime. Performance indices ------------------- * `account_history (triggering_commit_txid, changed_at DESC)` partial WHERE NOT NULL — for "show all account changes triggered by inscription X". * `pending_inscriptions (kind, created_at DESC)` — for "all mints in the last hour" style queries. * `pending_inscriptions (updated_at DESC) WHERE status = 'failed'` — for "show all failed Sends". Enum CHECKs on free-text columns -------------------------------- * `boot_log.event_type IN ('startup','shutdown','migration', 'state_load','vault_sync')`. * `tx_mining_log.target_prefix ~ '^[0-9a-f]+$'` — lowercase hex shape, keeps the column flexible (the marker may change) while catching typo regressions. Cosmetic -------- * `account_history_capture()` → `accounts_history_capture()` (matches the table noun + trigger name). DROP + CREATE because Postgres trigger functions can't be renamed in place when the trigger references them. * `mmr_root_index.leaf_index` UNIQUE — `mmr.leaf_count()` is monotonic, a duplicate is a code bug. UNIQUE turns it into a constraint violation at insert time. Per `feedback_zkcoins_migrations_may_wipe`: rows that would block a new CHECK / NOT NULL are wiped first (closed test env). Verified locally ---------------- * `cargo fmt --all --check` ✓ * `cargo clippy -p node -p shared -- -D warnings` ✓ * `cargo clippy -p node --all-features -- -D warnings` ✓ * `cargo check -p node --tests` ✓
…r" references Updates doc-comments, migration notes, and historical planning records in program-plonky2 (CONTRIBUTING/SESSION_STATE/STEP4_REVIEW/STEP7_PREP plus the rustdoc on types.rs, merkle helpers, and circuit/main.rs), script-plonky2 (CONTRIBUTING + lib.rs rustdoc), and the shared commitment_tests module header to refer to the zkCoins node instead of the legacy "server" wording. No code logic changes.
TaprootFreak
changed the title
TaprootFreak
marked this pull request as ready for review
May 26, 2026 20:44
TaprootFreak
added
the
ci:full
Sweeps the remaining "server" prose in node/src/* and node/tests/* to match the post-rename crate identity. Renames three local Rust bindings whose names referenced the old crate: * minting_server_account -> minting_node_account (runtime.rs) * server_clone -> node_clone (router_tests.rs) * server_guard -> account_node_guard (router_tests.rs) Doc-comments and error/log messages on main.rs, router.rs, runtime.rs, scanner.rs, db.rs, account_node.rs, the matching test files, and node/tests/api_remote.rs now refer to "the node" or "the API" as appropriate. The env-var ZKCOINS_E2E_ALLOW_FEATURE_TRIMMED_SERVER is kept verbatim because it is a stable CI contract with deploy-dev.yaml. External names are untouched: StatusCode::INTERNAL_SERVER_ERROR, wiremock::MockServer + every mock_server/mint_broadcast_mock_server binding, scanner_ws_tests' spawn_ws_server (Esplora WS mock), and the bitcoind server=1 documentation example all keep their original spelling.
Replaces the remaining prose references to the legacy "server" name
in ci.yaml, deploy-dev.yaml, and deploy-prd.yaml — workflow_dispatch
descriptions, deploy-target labels ("deployed DEV/PRD server"),
test-target descriptions ("live-DEV-server verification"), and the
bootstrap-env comment all now refer to "the node" instead.
External names stay untouched: SSH ServerAlive*, github.server_url,
sccache --start-server / --stop-server, and the
ZKCOINS_E2E_ALLOW_FEATURE_TRIMMED_SERVER env-var contract with the
api_remote test macro all keep their original spelling.
Migration 0010 (round-2 polish) added BYTEA `octet_length = 32` CHECK constraints across the schema. Five existing tests deliberately plant corrupt-length rows directly via SQL to exercise the Rust-side defensive decode paths (`LoadAccountNodeError::BadAddressLength`, `sqlx::Error::Decode` from `load_latest_block` / `load_root_indices`, `UsernameStore::load_from_pg` bad-length error). With the DB-level CHECKs in place, the bad inserts get rejected before the Rust code even sees them — the test panics on the INSERT `.unwrap()` and the negative path is never exercised. Fix: drop the relevant constraint inside each per-test container before planting the bad row. Each test gets its own ephemeral testcontainers Postgres, so the drop is isolated and harmless. The constraint itself stays covered indirectly by the migration test (`connect_and_migrate` runs all migrations before the drop). Affected tests -------------- * `account_node_tests::test_load_from_pg_rejects_wrong_address_length` → DROP `accounts_address_length` * `db_tests::load_latest_block_rejects_wrong_length` (file: db_tests.rs:215) → DROP `latest_block_hash_length` * `username_tests::load_from_pg_rejects_wrong_address_length` → DROP `usernames_address_length` * `state_tests::test_load_root_indices_rejects_short_prev_root_blob` → DROP `mmr_root_index_prev_root_length` * `state_tests::test_load_root_indices_rejects_short_smt_root_blob` → DROP `mmr_root_index_smt_root_length` CI parity verified ------------------ * `cargo fmt --all --check` ✓ * `cargo clippy -p node -p shared -- -D warnings` ✓ * `cargo check -p node --tests` ✓
…ngth test The DB-level CHECK drop alone is not enough — the `accounts_history_trigger` fires AFTER INSERT on `accounts` and attempts to write the 7-byte address to `account_history`, where the matching `account_history_address_length` CHECK now blocks it. Disable the trigger before planting the corrupt row. The history path is not the subject of this test; we are exercising `LoadAccountNodeError::BadAddressLength` only. (Other 4 tests in the previous commit don't hit this cascade — there is no AFTER INSERT trigger on `usernames`, `latest_block`, or `mmr_root_index`.)
…node Resolve conflict in runtime.rs (boot_log block from release branch kept, "REST server" → "REST API" rename applied). Apply server → node rename to merge-introduced files: - node/migrations/0006_inscription_kind.sql, 0007_request_log.sql, 0008_full_database_trail.sql - node/src/db.rs (4 doc-comments) - node/src/router.rs (1 doc-comment)
The existing assertion listed only the pre-#113 set of 8 tables. After migrations 0006-0010 the schema is 19 tables + the `accounts_history_trigger`; the introspection-query assertion now reflects that.
chore: replace "server" with "node"/"API" across the repo
Migration 0010 added FK constraints from `tx_mining_log.commit_txid` and `coin_proof_store.consumed_by_commit_txid` to `pending_inscriptions(commit_txid)`. The existing `persist_state_and_mark_complete_tx_rollback_on_failure_leaves_state_untouched` test synthesizes a mid-tx failure by dropping `pending_inscriptions` — now blocked by the new FK dependencies unless CASCADE is passed. Switch the DROP to CASCADE. The dependent tables and their FK constraints are torn down too, which is fine for the test (it owns the throw-away container) and exercises the exact same rollback invariant.
The #119 refactor `update_pending_failure_reason → mark_pending_failed` also promoted `status` to `'failed'` on every broadcast error. That erases the state-machine distinction `resume_pending_inscriptions` needs: a row in `commit_broadcast` (commit landed, reveal failed) must stay in `commit_broadcast` so resume re-broadcasts only the reveal. Forcing `'failed'` on partial-success states made resume re-attempt the commit — chain saves us with `txn-already-known`, but the row has lost its truth. Revert to the pre-#119 shape: only `failure_reason` is mutated, `status` stays under the state machine's control. `status = 'failed'` is reserved for truly-terminal callers (retry exhaustion, operator-initiated abort) — none yet, but the CHECK enum keeps the slot. Surface symptom: `publisher::tests::broadcast_advances_to_commit_broadcast_after_commit_success` asserted `status = 'commit_broadcast'` post-failure, was getting `'failed'`. With this revert it passes again, and resume retains its full state-machine vocabulary. The `pending_inscriptions_failed_reason_required` CHECK from #120 still holds (one-way implication: `status='failed' ⇒ failure_reason IS NOT NULL`).
…ve paths
The `accounts_history_trigger` (migration 0008) reads
`current_setting('zkcoins.account_source', TRUE)` and defaults to
`'scanner'` when the GUC is unset. Without explicit tagging, every
HTTP-handler-driven account mutation was being recorded as
`source='scanner'` — erasing the semantic distinction the column
was added to capture.
Add `db::upsert_account_with_source(pool, address, data, source)`:
opens a `BEGIN/COMMIT` envelope, runs
`SELECT set_config('zkcoins.account_source', $1, true)` (the safe
parameterized equivalent of `SET LOCAL`), then upserts. The GUC's
local scope means the tag only applies to this transaction, never
bleeding into adjacent / concurrent ones.
`commit_mint_tx` already had a transaction — extend it to set the
GUC to `'mint'` at the top so the bundled per-recipient upserts all
get tagged consistently.
Call-site changes:
| Site | Source |
|---------------------------------------------------------|------------|
| `db::commit_mint_tx` (all rows) | `'mint'` |
| `router::receive_coin_handler` (line ~607) | `'receive'`|
| `runtime::broadcast_commit_and_deliver` (line ~286) | `'receive'`|
| — recipient row updated post-`receive_coin` | |
| `router::send_coin_handler` (line ~755) | `'send'` |
`db::upsert_account` (default `'scanner'`) stays in place for the
remaining callers: `account_node::persist_account`, which is driven
from the scanner callback in `main.rs`.
`account_history.source` queries now return the expected enum value
for every operator forensic question (`WHERE source = 'mint'` etc.).
Verified locally
----------------
* `cargo fmt --all --check` ✓
* `cargo clippy -p node -p shared -- -D warnings` ✓
* `cargo check -p node --tests` ✓
CI coverage gate landed at 92.89%L / 88.93%F because audit.rs had
zero tests and db.rs grew ~14 new insert/update helpers without
matching coverage. Adds:
* `node/src/audit_tests.rs` (new file, wired via `mod tests`):
- `headers_to_json` non-UTF-8 binary-hex branch
- `headers_to_json` repeated-header collapse branch
- `buffer_body` collect-error fallback to empty bytes
- `audit_middleware_persists_request_response_pair` end-to-end
against a real testcontainers pool — verifies all columns
incl. client_ip (CF-Connecting-IP path)
- `audit_middleware_falls_back_to_x_forwarded_for` — fallback
path when CF-Connecting-IP is absent
* `db_tests.rs` — happy-path INSERTs for every new helper:
insert_request_log, insert_esplora_log, insert_error_log,
insert_block_log (idempotent ON CONFLICT), insert_observed_inscription
+ mark_observed_inscription_integrated (full lifecycle),
insert_state_update_log, insert_account_history,
insert_username_claim_log, insert_tx_mining_log (covers FK to
pending_inscriptions), insert_boot_log, update_pending_failure_reason
(verifies status unchanged), upsert_account_with_source (verifies
the trigger writes account_history with the GUC-supplied source),
get_inscription_summary_by_commit_txid (Some + None + full-row
format incl. txid-display-order reversal). Plus the
`InscriptionKind::from_db_str` `_ => None` branch.
* `router_tests.rs` — new module covering `GET /api/inscriptions/:txid`:
bad-hex 422, wrong-length 422, unknown-txid 404, known-txid 200
with summary JSON, DB-error 500 (DROP TABLE … CASCADE to force
the SELECT to fail). Plus a `claim_username_precheck_reject_persists_log_row`
test that waits for the fire-and-forget `tokio::spawn`
username_claim_log insert to land, closing the spawn-body
coverage gap at router.rs:1766.
Verified locally
----------------
* `cargo fmt --all --check` ✓
* `cargo clippy -p node -p shared -- -D warnings` ✓
* `cargo check -p node --tests` ✓
Round-2 coverage tightening after the previous test batch lifted
coverage from 92.89% to ~99.6%. The remaining 13 lines are all
defensive arms that only fire on bogus DB state or DB-down errors:
* `audit.rs:61-64` — `headers_to_json` array-grow branch (3+ same
header repeats). Added `headers_to_json_third_repeat_pushes_into_existing_array`.
* `db.rs:1057-1060` (`load_pending_in_progress`) and `db.rs:1150-1153`
(`get_inscription_summary_by_commit_txid`) — Rust-side
`InscriptionKind::from_db_str` defence triggered when a row's
`kind` is outside the CHECK enum. Added two tests that drop the
status+kind CHECK constraints, plant a bogus row, and assert the
loader returns `sqlx::Error::Decode`.
* `router.rs:1767` — `eprintln!("Failed to persist username_claim_log: …")`
inside the fire-and-forget tokio::spawn. Added
`claim_username_log_spawn_handles_insert_error` which drops
`username_claim_log` table before issuing a precheck-rejected
claim; the spawned insert fails, the eprintln arm executes.
Verified locally
----------------
* `cargo fmt --all --check` ✓
* `cargo clippy -p node -p shared -- -D warnings` ✓
* `cargo check -p node --tests` ✓
TaprootFreak
changed the title
…g check (#122) * test(api_remote): extend mint/commit/balance/claim roundtrips with value-bearing field assertions Adds five new tests in Section 4 that assert every wallet-app-facing response field by content, not just by presence. Mirrors the existing strong-assertion block in `send_commit_roundtrip_moves_balance` so a server bug returning a placeholder zero-hash or a truncated string fails CI at the API layer instead of in the wallet's integration loop. - mint_response_carries_state_hash_and_coins_root - commit_response_carries_state_hash_and_coins_root - balance_response_carries_username_after_claim - claim_response_carries_address - balance_response_has_no_username_for_unclaimed_wallet The mint and commit tests are written against the expected contract (hash fields populated as 32-byte non-zero hex); the current server sets them to `None`, so the two tests surface that lockstep gap until the server is updated to emit the fields. * test(api_remote): assert structured error envelope on 4xx responses Every 4xx the wallet app consumes MUST deserialise as `{ success: false, error: <non-empty string> }` so the client can branch on the failure reason without re-reading the body. This commit: - extends `balance_invalid_hex_returns_422` and `balance_wrong_length_returns_422` with body content assertions (the balance handler uses a different envelope from `handler_error_response`, so the assertion documents that today's body is the bare `BalanceResponse { balance: 0 }` with no `error` field — surfaces any future refactor that swaps shapes) - adds `send_returns_structured_error_envelope` covering the `handler_error_response` shape used by every `/api/send` 4xx path * test(api_remote): lockstep check that server errors match app errorMessages.ts mapping Adds a lockstep test against `app/src/lib/api/errorMessages.ts :: KNOWN_SERVER_ERRORS` so a server-side error rename surfaces in CI instead of degrading wallet UX to `Serverfehler <status>: <raw>`. - new constant `APP_KNOWN_ERROR_STRINGS` mirrors the app's 19-entry list (13 from `map_send_coins_error`, 6 from `handler_error_response` call sites) - new test `error_strings_match_known_app_mapping` provokes each reachable string through a single amortised mint: reachable: Unknown account address, Signature verification failed, Request timestamp too old or in the future, prev_commitment_pubkey required for account update, Insufficient funds mismatch (server emits more-specific text): Invalid hex, Invalid address length operator-only / internal-state-only (documented, not provoked): In-coin not present in source's output_coins_root, Source commitment not present in history MMR, Coin is missing commitment, Should provide an inclusion proof, Coin should not exist in coin history tree, Coin should not exist in tree yet, Too many in-coins / out-coins for one transition, prove failed, internal error, Missing signature, Broadcast failed - extends `mint_invalid_hex_address_returns_422`, `mint_wrong_address_length_returns_422`, `send_bad_address_hex_returns_422`, `send_unknown_account_returns_404`, `send_bad_signature_returns_401`, `send_stale_timestamp_returns_401` with body content assertions so each negative-path test is also a per-string contract anchor * feat(api): close 4 contract gaps surfaced by api_remote tests Resolves the four red tests from the field-coverage suite by fixing the node side of each divergence (the app stays as-is for these; the app-side family-matching is a separate PR). N1 mint response: populate account_state_hash + output_coins_root (hex-encoded 32-byte digests) so wallet clients have everything needed to derive prev_commitment_pubkey for the next send without a second GET /api/proof/:id round-trip. N2 commit response: same pair populated in broadcast_commit_and_deliver, so commit-side flows can also pin the resulting state directly. N3 timestamp window: explicit check_timestamp_window helper runs BEFORE verify_send_signature in send/commit/claim handlers, emitting "Request timestamp too old or in the future" as its own 401 instead of collapsing into "Signature verification failed". Clock-skew misconfiguration now surfaces distinctly. N4 missing signature: signed handlers now reject absent signature/timestamp fields with 401 "Missing signature" / "Missing timestamp" upstream of crypto verification. Defence- in-depth Option-arms stay in verify_send_signature. Unit tests in router_tests.rs updated to the new response shape and to the dedicated timestamp string. * test(api_remote): activate Missing signature provocation now that 401 is wired Replaces the inline "unreachable" comment with a live provocation: POST /api/send with signature deliberately omitted now returns 401 with "Missing signature". Mirrors the handler-level gate added in the same PR. Inventory comment updated to match. * test(api_remote): fix two test setups after the auth-order tightening send_bad_address_hex_returns_422: sign the request body so it passes the new "Missing signature"/timestamp gates that fire upstream of the per-field hex validator. The hex parser still rejects "0xZZZZZZ" with 422; the test now exercises the hex branch as intended. error_strings_match_known_app_mapping: the "prev_commitment_pubkey required" branch is the AccountUpdate transition, which is unreachable from a wallet that only received a mint (account.proof is still None → AccountCreation path). Move the string to the documented-only list with router_tests + account_node_tests references; the unit-level coverage is sufficient and saves a publisher-UTXO per CI run. * test(router): cover send_handler stale-timestamp 401 branch Adds a handler-level unit test asserting that POST /api/send with a stale (year-1970) timestamp returns 401 with "Request timestamp too old or in the future", covering router.rs:675-676 which the existing helper-level `check_timestamp_window_*` tests and the live `send_stale_timestamp_returns_401` api_remote test exercise but the coverage-gate nextest pass did not reach.
TaprootFreak
added a commit
that referenced
this pull request
May 27, 2026
…hash (#125) After #117 merged to develop the dfxdev container started crash-looping with `Migrate(VersionMismatch(1))` and the deploy-dev smoke test returned 502 for ~5 min straight before the workflow failed. Root cause: commit ce4307c ("docs(migrations): replace remaining 'server' with 'node' in SQL comments") edited the comment lines in the already-applied migrations `0001_initial.sql` and `0003_pending_inscriptions.sql`. sqlx hashes the migration file content (comments included), so a deployed DB whose `_sqlx_migrations.checksum` reflects the pre-edit text refuses to boot with the post-edit binary. This is the exact same class of issue that PR #95 already had to hotfix (`13155c1 hotfix(migration): revert SQL comment edit to keep sqlx hash stable`) and that `feedback_sqlx_migration_hash` documents. Fix: restore both files to their pre-ce4307c byte-for-byte content. Pure cosmetic revert — the only difference is "node" → "server" in 6 lines of `--` comments. No schema, no logic, no data change. The container's first boot after this lands will match its existing `_sqlx_migrations` row and proceed past the migrate step. If the "node" / "server" vocabulary is eventually wanted in the migration prose, the right move is a NEW migration whose comments use the chosen vocabulary — the old ones must stay frozen for the checksum to match deployed databases.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Aggregator release branch bundling the work done on the Mainnet hardening track. Includes three feature tracks (#113, #114, #115), a repo-wide rename (#121: "server" → "node"/"API"), and a series of post-merge fix-up and coverage-tightening commits that brought the branch through
Lint & Build, the full M3 Ultra test suite (300 tests), and the 100% line + function coverage gate.Scope
0006→0010) growing the schema from 8 tables to 19 + 1 trigger; newaudit.rsmiddleware; newGET /api/inscriptions/:txidendpoint; operator-forensics coverage for HTTP requests, scanner block ingest, observed inscriptions, account history, username claim attempts, reveal-txid mining, and boot events.NETWORK_CONFIGpanics ifESPLORA_URL/ESPLORA_WS_URLare unset or empty whenIS_MAINNET=true. Closes the silent footgun behind the Release: develop -> main #18 event-driven scanner 5 s 404 retry loop.ZKCOINS_E2E_ALLOW_FEATURE_TRIMMED_SERVER=trueon thedeploy-prd.yamlapi-e2estep so feature-gated tests skip cleanly under the MVP-only image policy.accounts_history_triggercascade required existing negative tests to drop or disable specific constraints; corrected over-aggressivestatus='failed'promotion on broadcast partial-failure; routedaccount_history.sourcecorrectly for the mint / send / receive paths via per-transaction GUC; ~25 new coverage tests acrossaudit_tests.rs,db_tests.rs,router_tests.rs,main_tests.rsto bringaudit.rs,db.rs, androuter.rsto 100% line + function coverage.Migrations 0006 → 0010
0006_inscription_kind.sqlpending_inscriptions.kind(mint/send) + wipe pre-existing rows0007_request_log.sqlrequest_log— full HTTP audit (raw bodies, JSONB headers, status, latency, client_ip)0008_full_database_trail.sqlesplora_log,error_log,block_log,observed_inscriptions,state_update_log,account_history,username_claim_log,tx_mining_log,coin_proof_store,boot_log) +accounts_history_trigger+pending_inscriptions.{failure_reason, reveal_txid}0009_schema_polish.sqlreveal_txid NOT NULL,block_log.received_atdrop,state_update_log.trigger → trigger_source,request_log.client_ip,esplora_log.triggering_request_log_id, trigger reads optionalzkcoins.request_log_idGUC0010_schema_polish_round2.sqltx_mining_log/coin_proof_store→pending_inscriptions), 4× logical-pair CHECKs,created_aton accounts / latest_block / smt_state / mmr_state, performance indices, enum CHECKs,accounts_history_capturerename,mmr_root_index.leaf_index UNIQUEAfter this PR lands,
dfxdevruns the full schema on the next deploy; after the matchingdevelop → mainRelease PR,dfxprddoes too. Themempool/backendself-host from DFXServer/server#257 is the matching infra-side change for the Mainnet path.Persistence stack — what's wired, what's deferred
This release lands schema + critical-path wiring. Explicit follow-ups, with helpers + columns already in the schema:
esplora_loginstrumentation (UTXO lookup, broadcast, get_tx, get_block_*)eprintln! → log_err!macro replacement (~50 sites)state_update_logwrite at the twostate.updatesites (mint_handlerPhase-E + scanner callback)coin_proof_storepersistence (replaces the in-memoryProofStore)Merge
This is an aggregator PR with five distinct migrations and multiple feature tracks already squash-merged into the branch. Use "Create a merge commit", NOT squash — squashing collapses the per-migration and per-feature commit messages on
develop, breaksgit bisectfor the individual changes, and erases the linear history the operator runbooks already reference by commit SHA.CI
All three jobs green on the head SHA:
ci:fulllabel active; the matching auto-Release-PR (develop → main) inherits the gate.Retention
The new audit tables (
request_log,esplora_log,block_log,account_history,state_update_log,username_claim_log,tx_mining_log,boot_log,error_log) intentionally have no automatic pruning. Operators prune via plainDELETE … WHERE <timestamp_col> < NOW() - INTERVAL '…'— seefeedback_zkcoins_migrations_may_wipefor the closed-test-env stance.