diff --git a/.github/workflows/deploy-dev.yaml b/.github/workflows/deploy-dev.yaml index c1f4c845..5ae7433a 100644 --- a/.github/workflows/deploy-dev.yaml +++ b/.github/workflows/deploy-dev.yaml @@ -155,6 +155,49 @@ jobs: sccache --start-server >/dev/null 2>&1 || true sccache --show-stats + # Operational preflight: hit /health/ready and /health/publisher + # BEFORE running the API E2E suite, so an empty publisher wallet + # or a non-ready DB fails THIS step with a clear "top up the + # publisher" / "DB not ready" message instead of cascading + # through the test suite as opaque 503s. + # + # Historically a green E2E run masked an empty publisher wallet + # because the suite silently dev_skip!()'d 5xx errors; PR + # "test: harden suite" (this PR) removed the masking and added + # this preflight as the load-bearing operational gate. + # + # 50_000 sats is a conservative floor: a single inscription + # commit + reveal pair at typical Mutinynet fee rates needs + # ~1_500 sats; 50_000 buys ~30 mints before the next top-up. + # Adjust upward if the suite grows. + - name: Ensure jq is installed (preflight dependency) + run: command -v jq >/dev/null || brew install jq + + - name: Preflight — publisher wallet has UTXOs + env: + DEV_API: https://dev-api.zkcoins.app + run: | + set -euo pipefail + ready=$(curl -sS --max-time 10 "$DEV_API/health/ready") + if ! echo "$ready" | jq -e '.ready == true' > /dev/null; then + echo "::error::/health/ready not ready: $ready" + exit 1 + fi + pub=$(curl -sS --max-time 15 -w '|%{http_code}' "$DEV_API/health/publisher") + code="${pub##*|}" + body="${pub%|*}" + if [ "$code" != "200" ]; then + echo "::error::/health/publisher returned $code: $body" + exit 1 + fi + utxos=$(echo "$body" | jq -r '.utxo_count') + sats=$(echo "$body" | jq -r '.total_sats') + if [ "$utxos" -lt 1 ] || [ "$sats" -lt 50000 ]; then + echo "::error::publisher wallet too low (utxos=$utxos, sats=$sats) — top up before re-running" + exit 1 + fi + echo "publisher OK: utxos=$utxos, sats=$sats" + - name: Run API E2E suite against DEV run: cargo test -p node --release --all-features --test api_remote -- --test-threads=1 --nocapture diff --git a/Cargo.lock b/Cargo.lock index 7c4af4e2..f170096f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1969,6 +1969,7 @@ dependencies = [ "sha2", "shared", "sqlx", + "tempfile", "testcontainers", "testcontainers-modules", "tokio", diff --git a/node/Cargo.toml b/node/Cargo.toml index 8e8f458b..3b581a05 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -58,7 +58,7 @@ sqlx = { version = "0.8", default-features = false, features = [ [dev-dependencies] tower = { version = "0.5", features = ["util"] } http-body-util = "0.1" -# Used by `publisher_tests` for Esplora mocking and by `server_tests` +# Used by `publisher_tests` for Esplora mocking and by `router_tests` # to mock the Esplora HTTP endpoint behind the `/health/ready` # readiness probe so the tests never hit the real # `https://mutinynet.com/api` from CI. @@ -76,6 +76,12 @@ reqwest = { version = "0.12", default-features = false, features = ["json", "rus # run picks a fresh wallet and avoids collisions with concurrent # DEV-server consumers. rand = "0.8" +# Auto-cleaning scratch directories for the ProofStore tests in +# `router_tests`. Replaces the ad-hoc `std::env::temp_dir() + nanos +# + remove_dir_all().ok()` shape — the `TempDir` Drop impl removes +# the directory even when the test panics, so no test leaves a +# leaked /tmp/zkcoins-* tree behind. +tempfile = "3" [features] # All non-MVP features are off by default. When a feature is not enabled, the diff --git a/node/src/lib.rs b/node/src/lib.rs index 4914c03d..55473311 100644 --- a/node/src/lib.rs +++ b/node/src/lib.rs @@ -38,8 +38,10 @@ pub mod state; pub mod username; use crate::publisher::EsploraConfig; +use bitcoin::secp256k1::{Keypair, Secp256k1, SecretKey, XOnlyPublicKey}; use lazy_static::lazy_static; use sqlx::PgPool; +use std::str::FromStr; const DEFAULT_PUBLISHER_KEY: &str = "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"; @@ -86,6 +88,24 @@ lazy_static! { key }; + /// Taproot publisher address derived once at startup from + /// `PUBLISHER_KEY` against the configured `NETWORK_CONFIG`. Folding + /// the secp256k1 work into `lazy_static` keeps the request path of + /// `publisher_health_handler` pure I/O (no per-request `SecretKey + /// ::from_str` / `Address::p2tr`) and removes a structurally + /// unreachable `Err` arm — `PUBLISHER_KEY` is validated here, so + /// an invalid key panics at startup, not on the first health + /// probe. Log-only, NOT a secret (the matching key lives in + /// `PUBLISHER_KEY`). + pub static ref PUBLISHER_ADDRESS: bitcoin::Address = { + let secp = Secp256k1::new(); + let sk = SecretKey::from_str(&PUBLISHER_KEY) + .expect("PUBLISHER_KEY must be a valid 32-byte hex secp256k1 secret"); + let key_pair = Keypair::from_secret_key(&secp, &sk); + let (xonly, _parity) = XOnlyPublicKey::from_keypair(&key_pair); + bitcoin::Address::p2tr(&secp, xonly, None, NETWORK_CONFIG.network()) + }; + /// Postgres connection string for the state-layer. Required; the /// bootstrap refuses to start without it because there is no /// sensible default for a database URL. diff --git a/node/src/publisher_tests.rs b/node/src/publisher_tests.rs index 1fbfed80..a4fdf038 100644 --- a/node/src/publisher_tests.rs +++ b/node/src/publisher_tests.rs @@ -96,10 +96,12 @@ async fn spawn_track_tx_ws(mode: &'static str) -> String { } } } - // Hold the connection open so the publisher does not see - // a clean close before consuming the echo frame; the - // publisher's helper exits after the event arrives. - let _ = tokio::time::sleep(Duration::from_secs(60)).await; + // Hold the connection open until the test aborts the + // task. `std::future::pending` keeps the socket alive + // indefinitely so a slow CI runner can never let the + // helper observe a clean close before the event arrives; + // a bounded `sleep(60s)` could expire and mask a race. + std::future::pending::<()>().await; } }); url diff --git a/node/src/router.rs b/node/src/router.rs index b41473b9..488a55d8 100644 --- a/node/src/router.rs +++ b/node/src/router.rs @@ -92,6 +92,15 @@ pub(crate) struct AppState { /// clones `NETWORK_CONFIG` into this slot so the runtime /// behaviour is unchanged. pub(crate) esplora_config: 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 + /// phase 2 of `mint_handler` (after the `account_node` guard is + /// acquired) so the test can `.notified().await` deterministically + /// instead of `tokio::time::sleep(200ms)`. Hidden behind + /// `cfg(test)` so the field does not exist in release builds. + #[cfg(test)] + pub(crate) phase2_reached: Arc, } // Response types for our API @@ -846,6 +855,13 @@ async fn mint_handler( // ---- 2. PROOF phase (no mutation, clone-based) ----------------------- let prepared = { let account_node_guard = lock_or_recover(&state.account_node); + // Test-only barrier: notify any test waiting on + // `state.phase2_reached` that the handler has acquired the + // account_node guard and is about to invoke `prepare_mint`. + // Production builds compile this out entirely (the field does + // not exist in release). + #[cfg(test)] + state.phase2_reached.notify_one(); // get_minting_account_address borrows immutably below, fine. if account_node_guard .get_account(&zkcoins_program::types::MINTING_ADDRESS) @@ -1245,6 +1261,61 @@ async fn check_esplora( Ok(()) } +/// JSON body returned by `GET /health/publisher`. Surface enough state +/// for the deploy-dev preflight (and a curious operator) to make the +/// "should I top up the publisher wallet?" decision without scraping +/// Esplora directly. `address` is the publisher's Taproot bech32 — log- +/// only, NOT a secret (the matching key lives in `PUBLISHER_KEY`). +#[derive(Serialize)] +struct PublisherHealthResponse { + address: String, + utxo_count: u64, + total_sats: u64, +} + +/// Operational preflight (`GET /health/publisher`). +/// +/// Reads the publisher Taproot wallet's UTXO set via the configured +/// Esplora endpoint and reports `(address, utxo_count, total_sats)`. +/// The deploy-dev workflow probes this BEFORE running the API E2E +/// suite — an empty wallet would otherwise cause every mint to 503 +/// and historically masked as a "green" run because the E2E suite +/// itself silently treated 5xx as a skip. Returning 503 on an +/// Esplora-side error is intentional: the operator should see the +/// failure mode, not a fabricated empty response. +async fn publisher_health_handler(State(state): State) -> impl IntoResponse { + let publisher_address = &*crate::PUBLISHER_ADDRESS; + + match crate::publisher::get_publisher_utxo(publisher_address, &state.esplora_config, None).await + { + Ok(utxos) => { + let utxo_count = utxos.len() as u64; + let total_sats: u64 = utxos.iter().map(|(_, sats)| sats).sum(); + ( + StatusCode::OK, + Json( + serde_json::to_value(PublisherHealthResponse { + address: publisher_address.to_string(), + utxo_count, + total_sats, + }) + .expect("publisher health response serializes"), + ), + ) + .into_response() + } + Err(e) => ( + StatusCode::SERVICE_UNAVAILABLE, + Json(serde_json::json!({ + "error": "Esplora-side error fetching publisher UTXOs", + "detail": e.to_string(), + "address": publisher_address.to_string(), + })), + ) + .into_response(), + } +} + async fn info_handler() -> impl IntoResponse { Json(InfoResponse { network: NETWORK_CONFIG.network_name.clone(), @@ -1629,6 +1700,7 @@ pub(crate) fn create_router(state: AppState) -> Router { .route("/", get(root_handler)) .route("/health", get(|| async { "ok" })) .route("/health/ready", get(ready_handler)) + .route("/health/publisher", get(publisher_health_handler)) .route("/api/info", get(info_handler)) .route("/api/balance", get(get_balance_handler)) .route("/api/send", post(send_coin_handler)) diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index 910a4444..d7e02caa 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -65,6 +65,7 @@ fn test_state() -> AppState { ws_url: None, track_tx_timeout: None, }), + phase2_reached: Arc::new(tokio::sync::Notify::new()), } } @@ -1591,7 +1592,9 @@ fn send_signature_accepts_valid_signature() { signature: Some(hex::encode(sig.serialize())), timestamp: Some(now), }; - assert!(verify_send_signature(&request).is_ok()); + // `.expect` surfaces the actual error string on failure; the + // previous `is_ok()` shape silently swallowed it. + verify_send_signature(&request).expect("valid Schnorr signature must verify"); } // --- POST /api/send (happy path, exercises the full handler) --- @@ -1678,17 +1681,34 @@ async fn send_with_valid_signature_returns_proof_id_and_hashes() { let response_json: serde_json::Value = serde_json::from_str(&body).expect("response is valid JSON"); assert_eq!(response_json["success"], true); + let proof_id = response_json["proof_id"] + .as_u64() + .expect("proof_id missing from response"); + assert!(proof_id > 0, "proof_id must be a positive u64"); + + // Value-bearing assertions on the send response payload. The + // previous `.as_str().is_some()` shape passed for any non-null + // string — including the all-zero placeholder a buggy handler + // could emit, or a truncated hex string. Decoding to bytes and + // asserting 32-byte length + non-zero pins both regressions. + let account_state_hash_hex = response_json["account_state_hash"] + .as_str() + .expect("account_state_hash present"); + let ash_bytes = hex::decode(account_state_hash_hex).expect("ash is hex"); + assert_eq!(ash_bytes.len(), 32, "account_state_hash must be 32 bytes"); assert!( - response_json["proof_id"].as_u64().is_some(), - "proof_id missing from response: {body}" - ); - assert!( - response_json["account_state_hash"].as_str().is_some(), - "account_state_hash missing: {body}" + ash_bytes.iter().any(|&b| b != 0), + "account_state_hash must be non-zero" ); + + let output_coins_root_hex = response_json["output_coins_root"] + .as_str() + .expect("output_coins_root present"); + let ocr_bytes = hex::decode(output_coins_root_hex).expect("ocr is hex"); + assert_eq!(ocr_bytes.len(), 32, "output_coins_root must be 32 bytes"); assert!( - response_json["output_coins_root"].as_str().is_some(), - "output_coins_root missing: {body}" + ocr_bytes.iter().any(|&b| b != 0), + "output_coins_root must be non-zero" ); } @@ -2218,6 +2238,7 @@ async fn send_with_insufficient_funds_returns_422_with_error_string() { ws_url: None, track_tx_timeout: None, }), + phase2_reached: Arc::new(tokio::sync::Notify::new()), }; let secret_bytes = include_bytes!("../minting_secret.bin"); @@ -2348,6 +2369,26 @@ async fn send_with_non_hex_recipient_returns_422() { assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); } +// ----------------------------------------------------------------- +// `lock_or_recover_*` tests — nextest per-test process isolation note +// ----------------------------------------------------------------- +// +// The three `lock_or_recover_*_poisoned` tests below intentionally +// panic inside a spawned thread to poison the mutex they hold, then +// call `lock_or_recover` on the same `Arc>` to assert that +// the helper recovers the inner value via `into_inner`. Each test +// MUST run in its own process — under the default `cargo test` +// runner (single binary, threadpool) the second-test poison setup +// can race against the first test's recovery path because both +// share the libtest thread that observes panics. We rely on +// `cargo-nextest`'s per-test process isolation (see `CONTRIBUTING.md` +// > "Tests" and `.config/nextest.toml`) to give each test a fresh +// process. Running these tests outside nextest is supported (the +// project's CI uses `cargo nextest run`); a bare `cargo test` will +// occasionally surface a spurious "double panic" diagnostic in the +// shared libtest panic handler. Switch to nextest if you reproduce +// this locally. + #[test] fn lock_or_recover_recovers_from_poisoned_mutex() { let mutex = Arc::new(Mutex::new(42i32)); @@ -2374,7 +2415,60 @@ fn lock_or_recover_recovers_from_poisoned_mutex() { async fn commit_with_valid_signature_fails_broadcast_returns_503() { use bitcoin::bip32::{ChildNumber, Xpriv, Xpub}; use bitcoin::secp256k1::{Keypair, PublicKey, SecretKey}; - let state = test_state(); + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + // Spin up a wiremock Esplora that returns the publisher's UTXOs + // (so `get_publisher_utxo` finds inputs) but FAILS the broadcast + // with a 400. This pins the test to "valid signature, broadcast + // genuinely fails → 503" instead of "valid signature, broadcast + // might or might not succeed against a public Mutinynet". The + // previous accept-either assertion masked a hypothetical + // regression where the handler returned 200 without actually + // broadcasting. + let mock_server = MockServer::start().await; + let secp = secp::Secp256k1::new(); + let publisher_sk = SecretKey::from_slice( + &hex::decode("1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef").unwrap(), + ) + .expect("default publisher key parses"); + let publisher_kp = Keypair::from_secret_key(&secp, &publisher_sk); + let (publisher_xonly, _) = bitcoin::secp256k1::XOnlyPublicKey::from_keypair(&publisher_kp); + let publisher_address = + bitcoin::Address::p2tr(&secp, publisher_xonly, None, bitcoin::Network::Signet); + Mock::given(method("GET")) + .and(path(format!("/address/{}/utxo", publisher_address))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!([ + { + "txid": "4444444444444444444444444444444444444444444444444444444444444444", + "vout": 0, + "value": 100_000, + "status": { + "confirmed": true, + "block_height": 100, + "block_hash": "0000000000000000000000000000000000000000000000000000000000000001", + "block_time": 1_700_000_000 + } + } + ]))) + .mount(&mock_server) + .await; + Mock::given(method("POST")) + .and(path("/tx")) + .respond_with( + ResponseTemplate::new(400).set_body_string("sendrawtransaction RPC error -25"), + ) + .mount(&mock_server) + .await; + + let mut state = test_state(); + state.esplora_config = Arc::new(crate::publisher::EsploraConfig { + url: mock_server.uri(), + is_mainnet: false, + network_name: "Mutinynet".to_string(), + ws_url: None, + track_tx_timeout: None, + }); let secret_bytes = include_bytes!("../minting_secret.bin"); let xpriv = Xpriv::new_master(bitcoin::Network::Signet, secret_bytes).unwrap(); @@ -2459,14 +2553,16 @@ async fn commit_with_valid_signature_fails_broadcast_returns_503() { .body(Body::from(commit_body.to_string())) .unwrap(); let (status, _) = send_request_with_state(state, commit_req).await; - // The commitment verifies, the handler proceeds to broadcast. Without - // a reachable Bitcoin node in the unit test environment, that call - // fails and the handler returns SERVICE_UNAVAILABLE. We accept either - // 503 (broadcast attempted and failed) or 200 (network was reachable - // and broadcast happened to succeed against a public Mutinynet). - assert!( - status == StatusCode::SERVICE_UNAVAILABLE || status == StatusCode::OK, - "expected 503 or 200, got {status}" + // The commitment verifies, the handler proceeds to broadcast. The + // wiremock Esplora rejects the broadcast (400) so the handler MUST + // return SERVICE_UNAVAILABLE. Anything else means the handler + // either bypassed the broadcast (a regression — it should always + // attempt it on a valid commitment) or fabricated a 200 response + // despite the upstream failure (a worse regression). + assert_eq!( + status, + StatusCode::SERVICE_UNAVAILABLE, + "expected 503 from valid-commit + broken-broadcast, got {status}" ); } @@ -2488,14 +2584,10 @@ fn proof_store_proof_path_returns_none_for_nonexistent_directory() { #[test] fn proof_store_new_picks_up_max_id_from_existing_files() { - let dir = std::env::temp_dir().join(format!( - "zkcoins-proof-store-max-{}", - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&dir).unwrap(); + // `tempfile::tempdir` removes the directory on Drop even when the + // test panics, so no /tmp/zkcoins-* tree leaks on failure. + let tmp = tempfile::tempdir().expect("create tempdir"); + let dir = tmp.path(); // Drop a few well-formed and one malformed filename. std::fs::write(dir.join("3.bin"), b"placeholder").unwrap(); std::fs::write(dir.join("17.bin"), b"placeholder").unwrap(); @@ -2506,8 +2598,6 @@ fn proof_store_new_picks_up_max_id_from_existing_files() { // next_id starts at max(3, 17) + 1 = 18; the malformed names are skipped. let id = store.next_id.load(std::sync::atomic::Ordering::SeqCst); assert_eq!(id, 18); - - std::fs::remove_dir_all(&dir).ok(); } #[test] @@ -2524,18 +2614,11 @@ fn persist_proof_bytes_logs_error_when_write_fails() { #[test] fn persist_proof_bytes_succeeds_when_write_succeeds() { // Mirror test for the Ok arm so the helper is fully exercised. - let dir = std::env::temp_dir().join(format!( - "zkcoins-persist-{}", - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - )); - std::fs::create_dir_all(&dir).unwrap(); - let path = dir.join("99.bin"); + // `tempfile::tempdir` cleans up on Drop, even on test panic. + let tmp = tempfile::tempdir().expect("create tempdir"); + let path = tmp.path().join("99.bin"); ProofStore::persist_proof_bytes(&path, b"payload", 99); assert_eq!(std::fs::read(&path).unwrap(), b"payload"); - std::fs::remove_dir_all(&dir).ok(); } #[tokio::test] @@ -3288,6 +3371,104 @@ async fn ready_returns_503_when_esplora_unreachable() { assert_eq!(failures, vec!["esplora".to_string()]); } +// ======================================================================= +// GET /health/publisher — operational preflight +// ======================================================================= +// +// The publisher health probe surfaces (address, utxo_count, total_sats) +// for the deploy-dev preflight. Two reachable arms after the lazy_static +// `PUBLISHER_ADDRESS` refactor: Ok (Esplora responded) and Err (Esplora- +// side error). The `SecretKey::from_str` panic-arm is no longer in the +// request path — `PUBLISHER_KEY` is validated once at startup. + +#[tokio::test] +async fn health_publisher_returns_200_with_utxo_count_and_total_sats_when_esplora_responds() { + // Mock Esplora returning a known UTXO set so the handler's Ok arm + // is exercised: GET /address/{publisher_addr}/utxo returns a JSON + // array of UTXOs that get_publisher_utxo parses and sums. + use wiremock::matchers::{method, path_regex}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + let esplora_mock = MockServer::start().await; + Mock::given(method("GET")) + .and(path_regex(r"^/address/.+/utxo$")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!([ + { + "txid": "a".repeat(64), + "vout": 0, + "value": 50_000, + "status": { "confirmed": true, "block_height": 1, "block_hash": "b".repeat(64), "block_time": 0 } + }, + { + "txid": "c".repeat(64), + "vout": 1, + "value": 12_345, + "status": { "confirmed": true, "block_height": 2, "block_hash": "d".repeat(64), "block_time": 0 } + } + ]))) + .mount(&esplora_mock) + .await; + + let mut state = mint_test_state(); + state.esplora_config = Arc::new(crate::publisher::EsploraConfig { + url: esplora_mock.uri(), + is_mainnet: false, + network_name: "Mutinynet".to_string(), + ws_url: None, + track_tx_timeout: None, + }); + + let req = Request::get("/health/publisher") + .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("publisher health body is JSON"); + assert!( + v["address"] + .as_str() + .expect("address present") + .starts_with("tb1p"), + "publisher address must be Mutinynet bech32 Taproot, got: {:?}", + v["address"] + ); + assert_eq!(v["utxo_count"].as_u64().expect("utxo_count u64"), 2); + assert_eq!(v["total_sats"].as_u64().expect("total_sats u64"), 62_345); +} + +#[tokio::test] +async fn health_publisher_returns_503_when_esplora_unreachable() { + // Drive the Err arm: mint_test_state() already points esplora at + // 127.0.0.1:1 (unreachable), so get_publisher_utxo returns Err + // and the handler must map to 503. + let state = mint_test_state(); + let req = Request::get("/health/publisher") + .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("publisher health err body is JSON"); + assert_eq!( + v["error"].as_str().expect("error field present"), + "Esplora-side error fetching publisher UTXOs" + ); + assert!( + v["address"] + .as_str() + .expect("address present") + .starts_with("tb1p"), + "publisher address must be returned even on Esplora failure, got: {:?}", + v["address"] + ); + assert!( + v["detail"].as_str().is_some(), + "detail field must be present for diagnostics" + ); +} + // ======================================================================= // POST /api/mint — handler coverage // ======================================================================= @@ -3351,6 +3532,7 @@ fn mint_test_state() -> AppState { ws_url: None, track_tx_timeout: None, }), + phase2_reached: Arc::new(tokio::sync::Notify::new()), } } @@ -3668,10 +3850,17 @@ async fn mint_happy_path_broadcasts_and_returns_proof_id() { assert_eq!(status, StatusCode::OK, "body: {}", resp_body); let v: serde_json::Value = serde_json::from_str(&resp_body).expect("valid JSON"); assert_eq!(v["success"], true); + let proof_id = v["proof_id"] + .as_u64() + .expect("proof_id missing from response"); + // NOTE (B5 deferred): can't pin proof_id == 1 because proof store ID + // grows across DB lifetime — same constraint as the minting balance + // bound. We assert > 0 (= the proof was actually persisted) and rely + // on the integration test (api_remote `mint_roundtrip_lands_balance_and_proof`) + // to verify the proof file is fetchable + bincode-decodable. assert!( - v["proof_id"].as_u64().is_some(), - "proof_id missing from response: {}", - resp_body + proof_id > 0, + "fresh-state mint must emit a non-zero proof_id" ); // Per the mint_handler contract, the mint response intentionally // omits `account_state_hash` and `output_coins_root` (those are @@ -3998,10 +4187,17 @@ async fn mint_receive_coin_failure_logs_and_returns_ok() { assert_eq!(status, StatusCode::OK, "body: {}", resp_body); let v: serde_json::Value = serde_json::from_str(&resp_body).expect("valid JSON"); assert_eq!(v["success"], true); + let proof_id = v["proof_id"] + .as_u64() + .expect("proof_id missing from response"); + // NOTE (B5 deferred): can't pin proof_id == 1 because proof store ID + // grows across DB lifetime — same constraint as the minting balance + // bound. We assert > 0 (= the proof was actually persisted) and rely + // on the integration test (api_remote `mint_roundtrip_lands_balance_and_proof`) + // to verify the proof file is fetchable + bincode-decodable. assert!( - v["proof_id"].as_u64().is_some(), - "proof_id missing from response: {}", - resp_body + proof_id > 0, + "fresh-state mint must emit a non-zero proof_id" ); } @@ -4109,10 +4305,19 @@ async fn mint_retry_after_broadcast_failure_succeeds() { assert_eq!(state.minting_account.lock().unwrap().num_pubkeys, 1); let recipient_digest = zkcoins_program::hash::digest_from_bytes(&recipient_bytes); { - let server_guard = state.account_node.lock().unwrap(); - assert!( - server_guard.get_account(&recipient_digest).is_some(), - "recipient account must be created on successful mint" + let node_guard = state.account_node.lock().unwrap(); + let recipient_account = node_guard + .get_account(&recipient_digest) + .expect("recipient account must be created on successful mint"); + // The second mint above credits `1u64`; the recipient's + // coin_queue must reflect exactly that single inflow. A + // shape-only `is_some()` previously masked a bug where the + // account row was inserted with an empty queue. + assert_eq!( + recipient_account.coin_queue.len(), + 1, + "recipient coin_queue must hold exactly the minted coin, got {:?}", + recipient_account.coin_queue.len() ); } } @@ -4242,40 +4447,42 @@ async fn concurrent_mint_during_proof_response_returns_503() { /// End-to-end race that drives the post-proof "concurrent mint /// detected during proof phase" branch of `mint_handler` through the /// HTTP layer so the `return concurrent_mint_during_proof_response(...)` -/// call site (router.rs ~L891) is covered, not just the helper. +/// call site (router.rs) is covered, not just the helper. /// -/// Synchronisation strategy (deterministic, not time-based): the test -/// pre-acquires the `state.account_node` mutex BEFORE issuing the -/// `/api/mint` request. The handler completes phase 1 (lock -/// `minting_account`, snapshot `expected_num_pubkeys = 0`, release) -/// and then blocks at phase 2 trying to lock `account_node`. While -/// the handler is parked on that lock, the test acquires -/// `state.minting_account` and bumps `num_pubkeys` to a non-matching -/// value, then drops the `account_node` guard. The handler proceeds -/// through phase 2 (prover work), reaches phase 3, re-locks +/// Synchronisation strategy (deterministic, NOT time-based): the +/// handler signals it has acquired the `state.account_node` guard +/// at the top of phase 2 via the test-only +/// `state.phase2_reached: Arc` field; the test +/// `.notified().await`s on it, then acquires `state.minting_account` +/// and bumps `num_pubkeys` to a non-matching value. The handler +/// proceeds through phase 2 (prover work), reaches phase 3, re-locks /// `minting_account`, observes the bumped counter, and returns 503 /// before ever touching the broadcast / Esplora / Postgres paths — /// so the bare `mint_test_state()` (dead pool, unreachable Esplora) /// is sufficient. /// +/// Previously this test used a 200 ms `tokio::time::sleep`, which +/// was both racy (a slow CI scheduler could let phase 2 enter and +/// finish before the bump landed) and opaque (a failure mode looked +/// like "test occasionally returns 200 instead of 503"). The Notify +/// barrier is a hard happens-before edge: the bump cannot run until +/// the handler has reached phase 2. +/// /// Requires the multi-thread runtime: phase 2's `prepare_mint` is /// blocking CPU work that would otherwise stall the single-threaded /// executor and prevent the test thread from running the bump step. -/// -/// `clippy::await_holding_lock` is silenced because holding the -/// `account_node` `MutexGuard` across the `sleep().await` IS the -/// synchronisation primitive — releasing it earlier would defeat the -/// test by letting phase 2 finish before the bump. -#[allow(clippy::await_holding_lock)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mint_handler_concurrent_mint_during_proof_returns_503() { let state = mint_test_state(); - // Pre-acquire the account_node lock so phase 2 of mint_handler - // parks until we release it. Phase 1 only touches - // `state.minting_account`, so the handler can still complete its - // snapshot (capturing expected_num_pubkeys = 0) before parking. - let account_node_guard = state.account_node.lock().unwrap(); + // Pre-subscribe to the phase-2 notify BEFORE spawning the request + // so a fast handler that acquires `account_node` and fires + // `notify_one()` immediately cannot lose the signal. `Notified` is + // a future created up-front; the `notify_one` call buffers the + // wake-up even when no one is currently awaiting, so dropping the + // `Notified` before the await would be unsound here. + let notified = state.phase2_reached.notified(); + tokio::pin!(notified); let recipient = "0x".to_string() + &hex::encode([7u8; 32]); let body = serde_json::json!({ @@ -4288,34 +4495,29 @@ async fn mint_handler_concurrent_mint_during_proof_returns_503() { .unwrap(); // Drive the request on a worker so we can manipulate state from - // this task while the handler is parked on the account_node - // mutex inside phase 2. + // this task while the handler runs. let state_for_request = state.clone(); let request_task = tokio::spawn(async move { send_request_with_state(state_for_request, req).await }); - // Give the handler a generous window to enter phase 2 and park on - // the account_node lock. Phase 1 is microseconds of work; 200ms - // is overkill but cheap. Note: we cannot rely on `lock().is_locked` - // because std::sync::Mutex offers no such API — but holding the - // guard here is enough, because phase 2 will block until we drop - // it regardless of when the handler arrives. - tokio::time::sleep(std::time::Duration::from_millis(200)).await; + // Wait until the handler signals it has acquired the + // `account_node` guard at the top of phase 2. Phase 1 + // (`minting_account` snapshot of `num_pubkeys = 0`) has finished + // by this point because it runs BEFORE phase 2 in `mint_handler`. + // This is a hard happens-before edge: the bump below cannot run + // until the handler is observably past the phase-1 snapshot. + notified.as_mut().await; // Now bump num_pubkeys on the minting_account. Phase 1 already // captured expected_num_pubkeys = 0, so any non-zero value here - // trips the phase-3 inequality check. + // trips the phase-3 inequality check. Phase 3 acquires the + // `minting_account` lock after the prover finishes; we hold the + // bump-mutating guard only briefly. { let mut minting = state.minting_account.lock().unwrap(); minting.num_pubkeys = 1; } - // Release the account_node lock so phase 2 can proceed. The - // handler now runs the prover, re-locks minting_account, observes - // num_pubkeys = 1 != expected 0, and returns 503 via - // `concurrent_mint_during_proof_response`. - drop(account_node_guard); - let (status, resp_body) = request_task.await.expect("request task panicked"); assert_eq!( diff --git a/node/src/runtime.rs b/node/src/runtime.rs index 0a3ca36d..9d33db71 100644 --- a/node/src/runtime.rs +++ b/node/src/runtime.rs @@ -140,6 +140,8 @@ 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()), + #[cfg(test)] + phase2_reached: Arc::new(tokio::sync::Notify::new()), }; // Bootstrap the minting account if it isn't already in the DB. diff --git a/node/src/runtime_tests.rs b/node/src/runtime_tests.rs index a77098f6..a3828d09 100644 --- a/node/src/runtime_tests.rs +++ b/node/src/runtime_tests.rs @@ -146,6 +146,23 @@ async fn start_rest_node_binds_and_serves_health() { "expected 200 on /health, got: {}", &resp[..resp.len().min(300)] ); + // `/health` is the documented liveness probe whose + // body is the literal string "ok" (see the route + // registration in `router::create_router`). A 200 + // status with a different body would still satisfy + // the old assertion but signal a regression in the + // contract Kuma watches. + let body = resp + .split("\r\n\r\n") + .nth(1) + .unwrap_or("") + .trim_end_matches('\0') + .trim(); + assert!( + body.starts_with("ok"), + "expected /health body to start with `ok`, got: {:?}", + body + ); return; } Err(e) => last_err = Some(e), diff --git a/node/src/scanner_ws_tests.rs b/node/src/scanner_ws_tests.rs index 8e1586a4..a363708e 100644 --- a/node/src/scanner_ws_tests.rs +++ b/node/src/scanner_ws_tests.rs @@ -97,10 +97,12 @@ async fn run_scanner_ws_publishes_blocks_from_server() { ); ws.send(WsMessage::Text(initial)).await.unwrap(); ws.send(WsMessage::Text(tip)).await.unwrap(); - // Hold the socket open so the scanner's anchor-on-reconnect - // path does not race; the test asserts on the channel and - // then drops the task. - let _ = tokio::time::sleep(Duration::from_secs(60)).await; + // Hold the socket open until the test aborts the task. A + // bounded `sleep(60s)` would silently expire on a slow CI + // runner and let the scanner observe a clean close, masking + // any race the test is trying to pin. `pending` has the + // identical "hold forever" semantic without the bound. + std::future::pending::<()>().await; }) .await; @@ -156,7 +158,9 @@ async fn run_scanner_ws_reconnects_after_server_close() { SAMPLE_BLOCK_HASH_HEX_2 ); ws2.send(WsMessage::Text(m2)).await.unwrap(); - tokio::time::sleep(Duration::from_secs(60)).await; + // Hold forever until the test aborts (see the matching note + // on the first sleep replacement above). + std::future::pending::<()>().await; }); let (tx, mut rx) = mpsc::channel::(8); @@ -229,7 +233,9 @@ async fn run_scanner_ws_force_reconnects_on_liveness_timeout() { SAMPLE_BLOCK_HASH_HEX_2 ); ws2.send(WsMessage::Text(m2)).await.unwrap(); - tokio::time::sleep(Duration::from_secs(60)).await; + // Hold forever until the test aborts (see the matching note + // on the first sleep replacement above). + std::future::pending::<()>().await; }); let (tx, mut rx) = mpsc::channel::(8); @@ -298,7 +304,8 @@ async fn subscribe_track_tx_then_wait_returns_when_peer_emits_txid() { txid_for_handler ); ws.send(WsMessage::Text(frame)).await.unwrap(); - tokio::time::sleep(Duration::from_secs(60)).await; + // Hold forever until the test aborts. + std::future::pending::<()>().await; }) .await }; @@ -319,7 +326,8 @@ async fn track_tx_wait_returns_timeout_when_event_never_arrives() { let url = spawn_ws_server(|mut ws| async move { // Consume the subscribe frame but never echo the event. let _ = ws.next().await; - tokio::time::sleep(Duration::from_secs(60)).await; + // Hold forever until the test aborts. + std::future::pending::<()>().await; }) .await; diff --git a/node/src/state_tests.rs b/node/src/state_tests.rs index 6cfdeec7..671eeb56 100644 --- a/node/src/state_tests.rs +++ b/node/src/state_tests.rs @@ -300,14 +300,12 @@ async fn test_get_commitment_proof_with_mmr() { // Update state with this commitment let mmr_root = state.update(std::slice::from_ref(&commitment)).unwrap(); - // Get the complete proof (SMT + MMR) - let proof_result = state.get_commitment_proof(&commitment.public_key); - assert!( - proof_result.is_ok(), - "Should return a valid proof for existing commitment" - ); - - let (commitment_msg, smt_proof, smt_root, mmr_proof) = proof_result.unwrap(); + // Get the complete proof (SMT + MMR). `.expect` itself asserts + // the Ok arm — a redundant `assert!(.is_ok())` before unwrap would + // double-emit on the same failure mode. + let (commitment_msg, smt_proof, smt_root, mmr_proof) = state + .get_commitment_proof(&commitment.public_key) + .expect("Should return a valid proof for existing commitment"); // Verify the message assert_eq!( diff --git a/node/tests/api_remote.rs b/node/tests/api_remote.rs index 93ae769a..5dfda8a8 100644 --- a/node/tests/api_remote.rs +++ b/node/tests/api_remote.rs @@ -23,11 +23,11 @@ //! The DEV server is shared by other workflows (per-PR app E2E, //! interactive testing). To keep this suite race-free we always: //! - mint into freshly-generated wallets (no fixed addresses) -//! - tolerate `503 Service Unavailable` on mutating endpoints, -//! which the server returns when the Mutinynet publisher wallet -//! has no UTXOs — a benign DEV condition //! - assert strictly on 4xx codes (client-fixable contract bugs) -//! - skip on 5xx codes with a logged warning (server-side flake) +//! - assert strictly on 5xx codes as well (server-side regressions +//! are real bugs, not flakes — the deploy-dev preflight verifies +//! publisher wallet + /health/ready BEFORE this suite runs, so a +//! 503 here is unambiguous: it means something regressed) //! //! Read by: //! - `cargo test -p node --release --test api_remote` (locally) @@ -48,6 +48,8 @@ use serde_json::{json, Value}; use sha2::{Digest, Sha256}; use shared::commitment::Commitment; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use zkcoins_program::hash::digest_to_bytes; +use zkcoins_program::types::MINTING_ADDRESS; // --------------------------------------------------------------------------- // Constants @@ -57,18 +59,20 @@ const DEFAULT_API_URL: &str = "https://dev-api.zkcoins.app"; const HTTP_TIMEOUT: Duration = Duration::from_secs(120); const POLL_INTERVAL: Duration = Duration::from_secs(2); const POLL_TIMEOUT: Duration = Duration::from_secs(60); -/// How long to keep retrying the user-level `/api/send` while the -/// server reports "Unable to get merkle proofs for provided public -/// key" — see the inline comment in `send_commit_roundtrip_moves_balance` -/// for why this is timing-bound on the scanner picking up the mint's -/// Taproot inscription. Mutinynet block time is ~30 s, the scanner -/// polls every 30 s, so 2 minutes is enough on a healthy network -/// without dragging the suite past the workflow timeout when the -/// publisher is offline. -const SEND_RETRY_DEADLINE: Duration = Duration::from_secs(120); -const SEND_RETRY_INTERVAL: Duration = Duration::from_secs(15); const MINT_AMOUNT: u64 = 50_000; const SEND_AMOUNT: u64 = 10_000; +/// Bootstrap balance seeded into the `MINTING_ADDRESS` account at +/// startup by `start_rest_node` (see `node::runtime`). +/// Must stay strictly less than `2^48` for Plonky2 Goldilocks safety +/// — see the matching constant guard in `runtime_tests`. The +/// happy-path roundtrips probe `/api/balance` on `MINTING_ADDRESS` +/// before their first mint and use this as an upper bound — +/// `0 < balance <= BOOTSTRAP_MINTING_BALANCE`. The exact value is +/// not asserted because the deploy-dev push trigger does not run +/// `reset_state`, so prior test residue legitimately reduces the +/// minting balance; the bound still catches a fully empty / negative +/// state. +const BOOTSTRAP_MINTING_BALANCE: u64 = 1u64 << 48; fn api_base() -> String { std::env::var("ZKCOINS_API_URL").unwrap_or_else(|_| DEFAULT_API_URL.to_string()) @@ -92,20 +96,23 @@ fn url(path: &str) -> String { format!("{}{}", api_base().trim_end_matches('/'), path) } -/// Helper: log a one-line "skip" with reason and return. -macro_rules! dev_skip { - ($reason:expr) => {{ - eprintln!("DEV environment skip: {}", $reason); - return; - }}; -} - -/// Helper: log a one-line "feature off" skip and return. Distinct -/// from [`dev_skip!`] so the workflow log line clearly marks "the -/// route is absent by design" vs. "the route is present but flaked -/// on the network". +/// Helper: log a one-line "feature off" skip and return. +/// +/// When running in CI (env `CI=true`) this is a hard panic instead of +/// a silent skip: CI is supposed to build with `--all-features`, so a +/// `feature_skip!` firing in CI is the canary for an accidentally +/// dropped `--all-features` flag in a workflow (e.g. someone copied +/// the local `cargo test` invocation into the workflow). Outside CI +/// the macro is still a skip — the suite is also runnable against a +/// feature-trimmed PRD deploy, where an absent route is expected. macro_rules! feature_skip { ($feature:expr, $test:expr) => {{ + if std::env::var("CI").is_ok() { + panic!( + "feature `{}` disabled but running in CI — all-features build is required", + $feature + ); + } eprintln!( "SKIP {}: feature `{}` disabled on this server", $test, $feature @@ -153,13 +160,22 @@ async fn fetch_capabilities(client: &reqwest::Client) -> Capabilities { .json() .await .expect("/api/info body is JSON for capability detection"); + // Each capability field MUST be a bool — a missing field or a + // non-bool value is a contract regression in `/api/info` and a + // `.unwrap_or(false)` would silently mask it as "feature off". let mut caps = Capabilities { - address_list: body["capabilities"]["address_list"] - .as_bool() - .unwrap_or(false), - faucet: body["capabilities"]["faucet"].as_bool().unwrap_or(false), - usernames: body["capabilities"]["usernames"].as_bool().unwrap_or(false), - lnurl: body["capabilities"]["lnurl"].as_bool().unwrap_or(false), + address_list: body["capabilities"]["address_list"].as_bool().expect( + "/api/info capabilities.address_list must be a bool — missing field is a contract regression", + ), + faucet: body["capabilities"]["faucet"].as_bool().expect( + "/api/info capabilities.faucet must be a bool — missing field is a contract regression", + ), + usernames: body["capabilities"]["usernames"].as_bool().expect( + "/api/info capabilities.usernames must be a bool — missing field is a contract regression", + ), + lnurl: body["capabilities"]["lnurl"].as_bool().expect( + "/api/info capabilities.lnurl must be a bool — missing field is a contract regression", + ), }; if let Ok(force) = std::env::var("ZKCOINS_FORCE_DISABLE_FEATURES") { for flag in force.split(',').map(|s| s.trim()).filter(|s| !s.is_empty()) { @@ -331,12 +347,12 @@ async fn health_ready_returns_ready_with_no_failures() { .expect("GET /health/ready"); let status = resp.status(); let body: Value = resp.json().await.expect("/health/ready body is JSON"); - if status != StatusCode::OK { - dev_skip!(format!( - "/health/ready returned {} with body {}", - status, body - )); - } + assert_eq!( + status, + StatusCode::OK, + "/health/ready must return 200 — failures: {:?}", + body["failures"] + ); assert_eq!(body["ready"], Value::Bool(true)); let failures = body["failures"].as_array().expect("failures is an array"); assert!( @@ -385,6 +401,42 @@ async fn info_returns_well_formed_response() { } } +/// Shape-only probe of `/health/publisher` — the JSON contract is +/// asserted here so the suite breaks if the field set changes, even +/// when the publisher wallet itself is empty (the deploy-dev +/// preflight separately enforces a non-zero UTXO count). 200 is +/// required: an Esplora-side error surfaces as 503 and we want that +/// to fail the suite, not be silently tolerated. +#[tokio::test] +async fn health_publisher_returns_well_formed_response() { + let resp = http_client() + .get(url("/health/publisher")) + .send() + .await + .expect("GET /health/publisher"); + assert_eq!( + resp.status(), + StatusCode::OK, + "/health/publisher must return 200 — anything else means Esplora is unreachable or the publisher route regressed" + ); + let body: Value = resp.json().await.expect("/health/publisher body is JSON"); + assert!( + body["address"].as_str().is_some_and(|v| !v.is_empty()), + "publisher address must be a non-empty string, got {:?}", + body["address"] + ); + assert!( + body["utxo_count"].as_u64().is_some(), + "utxo_count must be a u64, got {:?}", + body["utxo_count"] + ); + assert!( + body["total_sats"].as_u64().is_some(), + "total_sats must be a u64, got {:?}", + body["total_sats"] + ); +} + #[tokio::test] async fn balance_unknown_address_returns_ok_with_zero() { let address = format!("0x{}", "00".repeat(32)); @@ -466,34 +518,6 @@ async fn proof_for_huge_id_returns_404() { assert_eq!(resp.status(), StatusCode::NOT_FOUND); } -#[tokio::test] -async fn proof_id_one_returns_200_or_404() { - // proof_id=1 may exist (a prior test minted) or not (fresh state). - // Both 200 (binary) and 404 are valid; anything else is a regression. - let resp = http_client() - .get(url("/api/proof/1")) - .send() - .await - .expect("GET /api/proof/1"); - let status = resp.status(); - assert!( - status == StatusCode::OK || status == StatusCode::NOT_FOUND, - "proof/1 returned unexpected status: {}", - status - ); - if status == StatusCode::OK { - let bytes = resp.bytes().await.expect("body bytes"); - // A valid CoinProof bincode payload is at least a few hundred - // bytes (Plonky2 proof + commitment). 100 is a loose lower - // bound that just guards against an empty response. - assert!( - bytes.len() > 100, - "expected non-trivial CoinProof bytes, got {}", - bytes.len() - ); - } -} - #[tokio::test] async fn resolve_unknown_username_returns_404() { let client = http_client(); @@ -875,6 +899,15 @@ async fn mint_roundtrip_lands_balance_and_proof() { let client = http_client(); let alice = TestWallet::new(); + // Minting-account sanity guard: the deploy-dev workflow's + // `push: branches: [develop]` trigger does NOT run + // `reset-zkcoins-server`, so the minting balance is allowed to be + // anywhere in (0, BOOTSTRAP_MINTING_BALANCE]. We only fail hard + // on the genuinely impossible states (balance > bootstrap = code + // regression or unauthorized re-seed; balance == 0 = unexpected + // DB wipe). See `assert_minting_balance_in_bounds` for details. + assert_minting_balance_in_bounds(&client).await; + let mint_resp = client .post(url("/api/mint")) .json(&json!({ @@ -885,12 +918,6 @@ async fn mint_roundtrip_lands_balance_and_proof() { .await .expect("POST /api/mint"); let mint_status = mint_resp.status(); - if mint_status.is_server_error() { - dev_skip!(format!( - "mint returned {} — DEV environment flake", - mint_status - )); - } assert_eq!(mint_status, StatusCode::OK, "unexpected mint status"); let mint_body: Value = mint_resp.json().await.expect("mint body JSON"); assert_eq!( @@ -936,56 +963,38 @@ async fn send_commit_roundtrip_moves_balance() { let alice = TestWallet::new(); let bob = TestWallet::new(); + // Minting-account sanity guard — mirror of the one in + // `mint_roundtrip_lands_balance_and_proof`. The deploy-dev + // workflow's `push: branches: [develop]` trigger does NOT run + // `reset-zkcoins-server`, so we cannot pin the minting balance to + // an exact value (or even a small accept-set keyed off + // `MINT_AMOUNT`): the balance accumulates `bootstrap - N*MINT_AMOUNT` + // across every prior develop push that ran this suite. The + // bounds-check still catches the impossible / catastrophic states + // (balance > bootstrap = code regression or unauthorized re-seed; + // balance == 0 = unexpected DB wipe). + assert_minting_balance_in_bounds(&client).await; + // ---- Mint ---- - // 422 with "Unable to get merkle proofs for provided public key" is - // the documented signal that a PRIOR mint's on-chain Taproot - // inscription has not yet been observed by the scanner. This test - // runs sequentially after `mint_roundtrip_lands_balance_and_proof` - // in the single-threaded suite, so the second mint hits the - // server's "look up prev commitment" branch and depends on the - // scanner having caught up. Mutinynet block time is ≈30 s and the - // scanner polls Esplora on a 30 s interval — so until both delays - // elapse, the SMT does not know about the prev_commitment_pubkey - // the server needs to attach to this mint. Apply the same retry - // pattern that `/api/send` below uses for the same condition. - let mint_body_json = json!({ - "account_address": alice.address_hex(), - "amount": MINT_AMOUNT, - }); - let (mint_status, mint_body_text) = { - let deadline = std::time::Instant::now() + SEND_RETRY_DEADLINE; - loop { - let resp = client - .post(url("/api/mint")) - .json(&mint_body_json) - .send() - .await - .expect("POST /api/mint"); - let status = resp.status(); - let text = resp.text().await.unwrap_or_default(); - let should_retry = status == StatusCode::UNPROCESSABLE_ENTITY - && text.contains("Unable to get merkle proofs"); - if !should_retry || std::time::Instant::now() >= deadline { - break (status, text); - } - eprintln!( - "mint 422 (merkle proofs not yet observed); retrying in {:?}", - SEND_RETRY_INTERVAL - ); - tokio::time::sleep(SEND_RETRY_INTERVAL).await; - } - }; - if mint_status.is_server_error() { - dev_skip!(format!("mint returned {} — DEV flake", mint_status)); - } - if mint_status == StatusCode::UNPROCESSABLE_ENTITY - && mint_body_text.contains("Unable to get merkle proofs") - { - dev_skip!(format!( - "mint returned 422 after {:?} of retries — scanner did not observe the prior mint inscription in time; body={}", - SEND_RETRY_DEADLINE, mint_body_text - )); - } + // Post-#87 the scanner is event-driven (Esplora WS subscription), + // so by the time `mint_roundtrip_lands_balance_and_proof` returns + // 200 and writes alice-1's balance, the prior commitment is + // already at-most-one-block away from being indexed in the SMT. + // A `422 Unable to get merkle proofs` here is therefore a real + // scanner-side regression, not a benign timing flake — the + // previous PR-83-era retry loop is gone. Asserting `== 200` + // surfaces it. + let mint_resp = client + .post(url("/api/mint")) + .json(&json!({ + "account_address": alice.address_hex(), + "amount": MINT_AMOUNT, + })) + .send() + .await + .expect("POST /api/mint"); + let mint_status = mint_resp.status(); + let mint_body_text = mint_resp.text().await.unwrap_or_default(); assert_eq!( mint_status, StatusCode::OK, @@ -998,12 +1007,12 @@ async fn send_commit_roundtrip_moves_balance() { // Wait for the balance to settle so send_coins has something to spend. let balance_before = poll_balance_at_least(&client, &alice.address_hex(), MINT_AMOUNT).await; - if balance_before < MINT_AMOUNT { - dev_skip!(format!( - "balance never settled to {} after mint (saw {})", - MINT_AMOUNT, balance_before - )); - } + assert!( + balance_before >= MINT_AMOUNT, + "scanner never observed mint after MINT_AMOUNT={} (saw {})", + MINT_AMOUNT, + balance_before + ); // ---- Fetch the mint's CoinProof to discover prev_commitment_pubkey ---- let proof_resp = client @@ -1020,6 +1029,12 @@ async fn send_commit_roundtrip_moves_balance() { .expect("mint coin proof has commitment") .public_key; + // (No second poll needed — `poll_balance_at_least` above already + // observed alice.balance >= MINT_AMOUNT; the inscription is therefore + // on-chain and the scanner has ingested it. Removing the redundant + // 15-s wait shaves test runtime without losing signal — if the + // scanner regresses, the FIRST wait will fail.) + // ---- Send ---- let amount = SEND_AMOUNT; let ts = unix_now(); @@ -1034,53 +1049,14 @@ async fn send_commit_roundtrip_moves_balance() { "signature": signature, "timestamp": ts, }); - // 422 with "Unable to get merkle proofs for provided public key" - // is the documented signal that the on-chain commitment for the - // freshly-minted account has not yet been observed by the scanner. - // Mints broadcast a Taproot inscription whose confirmation depends - // on Mutinynet block time (≈30 s), and the scanner polls Esplora - // on a 30 s interval — so until both delays elapse, the SMT does - // not know about the prev_commitment_pubkey we just discovered. - // Poll for up to [`SEND_RETRY_DEADLINE`] before treating it as a - // DEV-environment skip, so a typical run on a healthy Mutinynet - // (block time 30 s) completes the full roundtrip. - let (send_status, send_body_text) = { - let deadline = std::time::Instant::now() + SEND_RETRY_DEADLINE; - loop { - let resp = client - .post(url("/api/send")) - .json(&send_body) - .send() - .await - .expect("POST /api/send"); - let status = resp.status(); - let text = resp.text().await.unwrap_or_default(); - let should_retry = status == StatusCode::UNPROCESSABLE_ENTITY - && text.contains("Unable to get merkle proofs"); - if !should_retry || std::time::Instant::now() >= deadline { - break (status, text); - } - eprintln!( - "send 422 (merkle proofs not yet observed); retrying in {:?}", - SEND_RETRY_INTERVAL - ); - tokio::time::sleep(SEND_RETRY_INTERVAL).await; - } - }; - if send_status.is_server_error() { - dev_skip!(format!( - "send returned {} — DEV flake; body={}", - send_status, send_body_text - )); - } - if send_status == StatusCode::UNPROCESSABLE_ENTITY - && send_body_text.contains("Unable to get merkle proofs") - { - dev_skip!(format!( - "send returned 422 after {:?} of retries — scanner did not observe the mint inscription in time; body={}", - SEND_RETRY_DEADLINE, send_body_text - )); - } + let send_resp = client + .post(url("/api/send")) + .json(&send_body) + .send() + .await + .expect("POST /api/send"); + let send_status = send_resp.status(); + let send_body_text = send_resp.text().await.unwrap_or_default(); assert_eq!( send_status, StatusCode::OK, @@ -1091,18 +1067,34 @@ async fn send_commit_roundtrip_moves_balance() { let send_body: Value = serde_json::from_str(&send_body_text).expect("send body JSON"); assert_eq!(send_body["success"], Value::Bool(true)); let send_proof_id = send_body["proof_id"].as_u64().expect("send proof_id"); + + // Value-bearing assertions on the response payload: each hash + // field must decode to exactly 32 bytes and be non-zero. A + // shape-only `.is_some()` check was masking server bugs that + // returned a placeholder zero-hash or a truncated hex string. let ash_hex = send_body["account_state_hash"] .as_str() - .expect("account_state_hash") + .expect("account_state_hash present") .to_string(); + let ash_bytes = hex::decode(&ash_hex).expect("ash is hex"); + assert_eq!(ash_bytes.len(), 32, "account_state_hash must be 32 bytes"); + assert!( + ash_bytes.iter().any(|&b| b != 0), + "account_state_hash must be non-zero" + ); let ocr_hex = send_body["output_coins_root"] .as_str() - .expect("output_coins_root") + .expect("output_coins_root present") .to_string(); + let ocr_bytes = hex::decode(&ocr_hex).expect("ocr is hex"); + assert_eq!(ocr_bytes.len(), 32, "output_coins_root must be 32 bytes"); + assert!( + ocr_bytes.iter().any(|&b| b != 0), + "output_coins_root must be non-zero" + ); + assert!(send_proof_id > 0, "proof_id must be a positive u64"); // ---- Commit ---- - let ash_bytes = hex::decode(&ash_hex).expect("decode ash"); - let ocr_bytes = hex::decode(&ocr_hex).expect("decode ocr"); let mut commit_message = Vec::with_capacity(64); commit_message.extend_from_slice(&ash_bytes); commit_message.extend_from_slice(&ocr_bytes); @@ -1121,9 +1113,6 @@ async fn send_commit_roundtrip_moves_balance() { .await .expect("POST /api/commit"); let commit_status = commit_resp.status(); - if commit_status.is_server_error() { - dev_skip!(format!("commit returned {} — DEV flake", commit_status)); - } assert_eq!( commit_status, StatusCode::OK, @@ -1174,9 +1163,9 @@ async fn username_claim_resolve_lnurlp_roundtrip() { .await .expect("POST /api/username/claim"); let claim_status = claim_resp.status(); - if claim_status == StatusCode::SERVICE_UNAVAILABLE { - dev_skip!("username claim returned 503 — DB unavailable"); - } + // DB availability is covered separately by `/health/ready`'s `db` + // failure tag; a 503 here means the username claim path itself + // regressed and is treated as a hard failure (no `dev_skip!`). assert_eq!( claim_status, StatusCode::OK, @@ -1213,8 +1202,23 @@ async fn username_claim_resolve_lnurlp_roundtrip() { "callback must reference the username, got {:?}", lnurlp_body["callback"] ); - assert!(lnurlp_body["minSendable"].as_u64().is_some()); - assert!(lnurlp_body["maxSendable"].as_u64().is_some()); + let min_sendable = lnurlp_body["minSendable"] + .as_u64() + .expect("minSendable must be a u64"); + let max_sendable = lnurlp_body["maxSendable"] + .as_u64() + .expect("maxSendable must be a u64"); + assert!( + min_sendable >= 1, + "minSendable must be >= 1 msat, got {}", + min_sendable + ); + assert!( + max_sendable >= min_sendable, + "maxSendable ({}) must be >= minSendable ({})", + max_sendable, + min_sendable + ); assert!(lnurlp_body["metadata"] .as_str() .is_some_and(|s| !s.is_empty())); @@ -1280,6 +1284,57 @@ async fn poll_balance_at_most(client: &reqwest::Client, address: &str, target: u } } +/// Fetch the current balance of the well-known `MINTING_ADDRESS`. +/// Used by the fresh-state guard at the top of the happy-path +/// roundtrips to detect a dirty DEV state (prior mint residue or a +/// missed `reset_state` run). +async fn fetch_minting_balance(client: &reqwest::Client) -> u64 { + let minting_hex = format!("0x{}", hex::encode(digest_to_bytes(&MINTING_ADDRESS))); + let resp = client + .get(url(&format!("/api/balance?address={}", minting_hex))) + .send() + .await + .expect("GET /api/balance for MINTING_ADDRESS"); + assert_eq!( + resp.status(), + StatusCode::OK, + "/api/balance must return 200 for MINTING_ADDRESS" + ); + let body: Value = resp.json().await.expect("balance body is JSON"); + body["balance"].as_u64().expect("balance must be a u64") +} + +/// Assert that the minting account exists and its balance has not +/// somehow exceeded the bootstrap value. Allows for arbitrary prior +/// mints in the same DB lifetime (each mint reduces the balance, never +/// increases it). +/// +/// Hard-fails if: +/// - balance > BOOTSTRAP_MINTING_BALANCE (impossible without a code bug +/// or unauthorized re-seed), OR +/// - balance == 0 with no inflight mints (suggests an unwanted reset +/// or DB wipe between deploys) +/// +/// The deploy-dev workflow's `push: branches: [develop]` trigger does +/// NOT run `reset-zkcoins-server`; that command requires explicit +/// `workflow_dispatch` with `reset_state: true`. Strict equality with +/// BOOTSTRAP_MINTING_BALANCE would therefore tripwire CI on the second +/// push after any reset. Use this upper-bound assertion instead. +async fn assert_minting_balance_in_bounds(client: &reqwest::Client) { + let balance = fetch_minting_balance(client).await; + assert!( + balance <= BOOTSTRAP_MINTING_BALANCE, + "minting balance {} > bootstrap {} — code regression or unauthorized re-seed", + balance, + BOOTSTRAP_MINTING_BALANCE, + ); + assert!( + balance > 0, + "minting balance is 0 — likely an unexpected reset_state run or DB wipe; \ + check the deploy-dev workflow's recent runs" + ); +} + fn random_suffix() -> String { let mut bytes = [0u8; 8]; rand::thread_rng().fill_bytes(&mut bytes);