From 8551ceb8358b5b6fd62d0549dda00287392d25e8 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 25 May 2026 18:41:41 +0200 Subject: [PATCH 1/7] test(router): align post-Phase-E mint error tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename mint_commit_tx_failure_returns_503 → mint_pending_inscriptions_persist_failure_returns_503 to reflect post-PR-107/110 reality: dead_pool fails at the publisher's pre-broadcast pending_inscriptions INSERT, not at commit_mint_tx. Assertion now matches the wrapped error string. - Add mint_commit_mint_tx_failure_returns_503 to restore branch coverage of the post-broadcast commit_mint_tx Err path. Uses a live Postgres testcontainer + a BEFORE INSERT trigger on the accounts table; everything earlier in the mint flow succeeds, only the final accounts upsert raises, and the handler returns 503 with the 'Failed to persist mint commit transaction' body. --- node/src/router_tests.rs | 121 +++++++++++++++++++++++++++++++++------ 1 file changed, 103 insertions(+), 18 deletions(-) diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index b104594c..2b7e5ac9 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -4041,26 +4041,23 @@ async fn mint_broadcast_mock_server() -> wiremock::MockServer { mock_server } -/// Drives the Err arm of the post-broadcast `db::commit_mint_tx` call -/// at the tail of `mint_handler`. The broadcast goes through (wiremock -/// answers the UTXO + tx POSTs), the handler walks past the early -/// 503 broadcast-failure branch into the commit-tx phase. The pool is -/// the lazy `dead_pool` that connect-errors on first use, so the -/// transaction fails to begin and the handler returns -/// `503 SERVICE_UNAVAILABLE` "Failed to persist mint commit -/// transaction". +/// Drives the Err arm of the pre-broadcast `pending_inscriptions` +/// persist that PR #107 introduced. With the lazy `dead_pool` that +/// connect-errors on first use, the publisher's +/// `broadcast_inscription_txs_with_persistence` fails at the very +/// first DB write (the `constructed`-row INSERT) BEFORE any tx is +/// broadcast on chain. The publisher wraps the persistence error as +/// `"persist pending inscription: …"` and the handler maps that to +/// `503 SERVICE_UNAVAILABLE` "Failed to broadcast mint inscription +/// on-chain". /// -/// Phase D note: the in-memory minting `Account` and recipient `Account` -/// HAVE mutated before the failed commit (the Phase-D shape applies -/// `commit_mint` + `receive_coin` to the live in-memory state before -/// the DB transaction begins, so the bytes the transaction tries to -/// upsert come from the LIVE map). A commit failure therefore leaves -/// memory ahead of DB; the next scanner sweep rehydrates the SMT from -/// chain and the next mint observes the correct N via -/// `derive_num_pubkeys_from_smt`. The 503 surface signals to the -/// client that nothing durable landed. +/// Contract: with a broken persistence layer, no on-chain commitment +/// is published and `mint_handler` returns 503 cleanly. Coverage of +/// the deeper post-broadcast `commit_mint_tx` Err branch is in +/// `mint_commit_mint_tx_failure_returns_503` below (live pool + +/// `accounts`-table trigger). #[tokio::test] -async fn mint_commit_tx_failure_returns_503() { +async fn mint_pending_inscriptions_persist_failure_returns_503() { let mock_server = mint_broadcast_mock_server().await; let ws_url = mint_broadcast_mock_ws().await; @@ -4094,6 +4091,94 @@ async fn mint_commit_tx_failure_returns_503() { ); let v: serde_json::Value = serde_json::from_str(&resp_body).expect("valid JSON"); assert_eq!(v["success"], false); + assert_eq!(v["error"], "Failed to broadcast mint inscription on-chain"); +} + +/// Drives the Err arm of the post-broadcast `db::commit_mint_tx` call +/// at the tail of `mint_handler` (router.rs ~ "Failed to persist mint +/// commit transaction"). Uses a live Postgres so the publisher's +/// pre-broadcast `pending_inscriptions` INSERT, the broadcast itself, +/// the in-memory `state.update`, and the atomic +/// `persist_state_and_mark_complete_tx` all succeed; an `accounts` +/// trigger then raises on the final `INSERT` so `commit_mint_tx` +/// rolls back. Handler converts to 503. +/// +/// Coverage: this is the only test exercising the `commit_mint_tx` +/// Err branch in `mint_handler` post-Phase-E (the dead-pool path +/// short-circuits earlier — see +/// `mint_pending_inscriptions_persist_failure_returns_503`). +#[tokio::test] +async fn mint_commit_mint_tx_failure_returns_503() { + use testcontainers::{runners::AsyncRunner, ImageExt}; + use testcontainers_modules::postgres::Postgres; + + let pg_container = Postgres::default() + .with_tag("17") + .start() + .await + .expect("failed to start postgres container"); + let host = pg_container.get_host().await.unwrap(); + let port = pg_container.get_host_port_ipv4(5432).await.unwrap(); + let url = format!("postgres://postgres:postgres@{}:{}/postgres", host, port); + let pool = Arc::new( + crate::db::connect_and_migrate(&url) + .await + .expect("connect_and_migrate failed"), + ); + + // Trigger raises on every accounts INSERT, surfacing as + // `sqlx::Error::Database` from inside `commit_mint_tx`'s tx. + sqlx::query( + "CREATE OR REPLACE FUNCTION fail_accounts_insert() RETURNS trigger AS $$ + BEGIN + RAISE EXCEPTION 'simulated commit_mint_tx failure'; + END; + $$ LANGUAGE plpgsql", + ) + .execute(&*pool) + .await + .unwrap(); + sqlx::query( + "CREATE TRIGGER block_accounts_insert BEFORE INSERT ON accounts \ + FOR EACH ROW EXECUTE FUNCTION fail_accounts_insert()", + ) + .execute(&*pool) + .await + .unwrap(); + + let mock_server = mint_broadcast_mock_server().await; + let ws_url = mint_broadcast_mock_ws().await; + + let mut state = mint_test_state(); + state.pool = Arc::clone(&pool); + state.esplora_config = Arc::new(crate::publisher::EsploraConfig { + url: mock_server.uri(), + is_mainnet: false, + network_name: "Mutinynet".to_string(), + ws_url: Some(ws_url), + track_tx_timeout: None, + }); + + let recipient_bytes = [12u8; 32]; + let recipient = "0x".to_string() + &hex::encode(recipient_bytes); + let body = serde_json::json!({ + "account_address": recipient, + "amount": 1u64, + }); + let req = Request::post("/api/mint") + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .unwrap(); + let (status, resp_body) = send_request_with_state(state, req).await; + + assert_eq!( + status, + StatusCode::SERVICE_UNAVAILABLE, + "commit_mint_tx failure must surface 503, body: {}", + resp_body + ); + let v: serde_json::Value = serde_json::from_str(&resp_body).expect("valid JSON"); + assert_eq!(v["success"], false); assert_eq!(v["error"], "Failed to persist mint commit transaction"); } From 0914731615f66b894836f0dcf6335cb52a60b3f1 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 25 May 2026 19:16:05 +0200 Subject: [PATCH 2/7] test(router): spawn per-connection in mint_broadcast_mock_ws The accept loop handled one connection then slept 60s before accepting the next, which serialised sequential mint tests behind the previous mint's keepalive sleep. The second connect_async hit its 15s timeout and the mint flow returned 503 'WS connect failed'. Phase E surfaced this because it added the first multi-mint-per- test scenario (mint_handler_two_sequential_mints_with_different_recipients_advance_cleanly). Pre-Phase-E every test ran exactly one mint so the bug was hidden. Fix: spawn a tokio task per accepted connection so the accept loop keeps draining new connects in parallel with the existing 60s keepalive. --- node/src/router_tests.rs | 52 ++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index 2b7e5ac9..30df0b66 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -3966,30 +3966,36 @@ async fn mint_broadcast_mock_ws() -> String { Ok(s) => s, Err(_) => return, }; - let mut ws = match tokio_tungstenite::accept_async(stream).await { - Ok(w) => w, - Err(_) => continue, - }; - let first = match ws.next().await { - Some(Ok(WsMessage::Text(t))) => t, - _ => continue, - }; - let value: serde_json::Value = match serde_json::from_str(&first) { - Ok(v) => v, - Err(_) => continue, - }; - if value.get("action") == Some(&serde_json::json!("track-tx")) { - if let Some(txid_str) = value.get("data").and_then(|v| v.as_str()) { - // Documented mempool.space `txPosition` shape; - // see `scanner_ws::frame_signals_tx_seen`. - let frame = format!( - r#"{{"txPosition":{{"txid":"{}","position":{{"block":1,"vsize":120}}}}}}"#, - txid_str - ); - let _ = ws.send(WsMessage::Text(frame)).await; + // Spawn per-connection so the accept loop continues + // immediately and tests issuing multiple sequential mints + // (each with its own WS connect) are not serialised behind + // the previous connection's 60s keepalive sleep. + tokio::spawn(async move { + let mut ws = match tokio_tungstenite::accept_async(stream).await { + Ok(w) => w, + Err(_) => return, + }; + let first = match ws.next().await { + Some(Ok(WsMessage::Text(t))) => t, + _ => return, + }; + let value: serde_json::Value = match serde_json::from_str(&first) { + Ok(v) => v, + Err(_) => return, + }; + if value.get("action") == Some(&serde_json::json!("track-tx")) { + if let Some(txid_str) = value.get("data").and_then(|v| v.as_str()) { + // Documented mempool.space `txPosition` shape; + // see `scanner_ws::frame_signals_tx_seen`. + let frame = format!( + r#"{{"txPosition":{{"txid":"{}","position":{{"block":1,"vsize":120}}}}}}"#, + txid_str + ); + let _ = ws.send(WsMessage::Text(frame)).await; + } } - } - let _ = tokio::time::sleep(std::time::Duration::from_secs(60)).await; + let _ = tokio::time::sleep(std::time::Duration::from_secs(60)).await; + }); } }); url From b97c431bc9f665d580ce1ff6f884832f2c709990 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 25 May 2026 19:54:43 +0200 Subject: [PATCH 3/7] fix(router): add deterministic phase3_release hold for concurrent-mint test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing race test relied on prepare_mint (~200ms in test build) being slow enough for the test thread to acquire the SMT lock and insert pk_0 between phase 2 entry and the phase-3 re-derive. Under CI load this race was lost intermittently — phase 3 ran first, saw no pk_0, proceeded to broadcast, returned 'Failed to broadcast mint inscription on-chain' instead of 'Concurrent mint detected'. Fix: add a second cfg(test)-only Notify (phase3_release) that the handler awaits between prepare_mint and the phase-3 re-derive. All test_state constructors pre-arm it so production-shaped tests proceed immediately. The concurrent-mint race test drains the pre-armed permit before spawning, then notify_one()s after injecting pk_0 — a hard happens-before edge that no timing variance can lose. This eliminates the flake observed on develop's heavy CI run 26411938492 Coverage Gate. --- node/src/router.rs | 20 +++++++++++++++ node/src/router_tests.rs | 54 +++++++++++++++++++++++++++++++++++++--- node/src/runtime.rs | 8 ++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/node/src/router.rs b/node/src/router.rs index 92730243..f1a9a967 100644 --- a/node/src/router.rs +++ b/node/src/router.rs @@ -102,6 +102,17 @@ pub(crate) struct AppState { /// `cfg(test)` so the field does not exist in release builds. #[cfg(test)] pub(crate) phase2_reached: Arc, + /// Test-only deterministic hold between `prepare_mint` (phase 2) + /// and the phase-3 re-derive. The handler `.notified().await`s + /// this AFTER `prepare_mint` returns and BEFORE the re-derive + /// reads SMT membership, so the concurrent-mint race test can + /// inject `pk_N` between the two without losing the race to a + /// fast prover. The notify is pre-armed (`notify_one()` called in + /// `test_state` constructors) so production-shaped tests that + /// don't care about the hold proceed immediately. Hidden behind + /// `cfg(test)` so the field does not exist in release builds. + #[cfg(test)] + pub(crate) phase3_release: Arc, } // Response types for our API @@ -906,6 +917,15 @@ async fn mint_handler( } }; + // Test-only deterministic hold between `prepare_mint` and the + // phase-3 re-derive. Pre-armed in `test_state` constructors so the + // production-shaped tests proceed immediately. The + // concurrent-mint race test re-arms it AFTER injecting the SMT + // entry to guarantee phase 3 observes the injection. Production + // builds compile this out entirely (the field does not exist). + #[cfg(test)] + state.phase3_release.notified().await; + // Build the BIP-340 commitment over the prover's outputs. Sign with // the index-N private key — this is the same key the wallet would // sign with once `num_pubkeys` advances past N. We do NOT mutate diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index 30df0b66..54b01d67 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -66,6 +66,16 @@ fn test_state() -> AppState { track_tx_timeout: None, }), phase2_reached: Arc::new(tokio::sync::Notify::new()), + phase3_release: { + // Pre-armed: production-shaped tests proceed immediately + // through the cfg(test) hold-point in mint_handler. The + // concurrent-mint race test (and any future test that + // wants to pause the handler between prove and re-derive) + // re-arms it via `notify_one()` after injecting state. + let n = Arc::new(tokio::sync::Notify::new()); + n.notify_one(); + n + }, } } @@ -2239,6 +2249,16 @@ async fn send_with_insufficient_funds_returns_422_with_error_string() { track_tx_timeout: None, }), phase2_reached: Arc::new(tokio::sync::Notify::new()), + phase3_release: { + // Pre-armed: production-shaped tests proceed immediately + // through the cfg(test) hold-point in mint_handler. The + // concurrent-mint race test (and any future test that + // wants to pause the handler between prove and re-derive) + // re-arms it via `notify_one()` after injecting state. + let n = Arc::new(tokio::sync::Notify::new()); + n.notify_one(); + n + }, }; let secret_bytes = include_bytes!("../minting_secret.bin"); @@ -3533,6 +3553,16 @@ fn mint_test_state() -> AppState { track_tx_timeout: None, }), phase2_reached: Arc::new(tokio::sync::Notify::new()), + phase3_release: { + // Pre-armed: production-shaped tests proceed immediately + // through the cfg(test) hold-point in mint_handler. The + // concurrent-mint race test (and any future test that + // wants to pause the handler between prove and re-derive) + // re-arms it via `notify_one()` after injecting state. + let n = Arc::new(tokio::sync::Notify::new()); + n.notify_one(); + n + }, } } @@ -4499,6 +4529,17 @@ async fn mint_handler_concurrent_mint_during_proof_returns_503() { use bitcoin::hashes::Hash; let state = mint_test_state(); + // Drain the pre-armed `phase3_release` permit that `mint_test_state` + // populates for production-shaped tests. With the permit consumed, + // the handler's `state.phase3_release.notified().await` between + // `prepare_mint` and the phase-3 re-derive will BLOCK until this + // test explicitly calls `notify_one()` AFTER injecting `pk_0`. + // This converts the previously-fragile timing race (prover faster + // than test's lock acquisition) into a deterministic happens-before + // edge: insert lands → notify → handler unblocks → re-derive sees + // the bumped count. + state.phase3_release.notified().await; + // 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 @@ -4540,10 +4581,10 @@ async fn mint_handler_concurrent_mint_during_proof_returns_503() { ); // Insert pk_0's key into the SMT so the phase-3 re-derive returns - // 1 instead of the captured `expected_num_pubkeys = 0`. Phase 3 - // acquires the state lock after the prover finishes; the insert - // here lands while phase 2 is running its blocking proof work on - // the worker thread. + // 1 instead of the captured `expected_num_pubkeys = 0`. The + // handler is currently blocked on `state.phase3_release` (drained + // above) so phase 3 cannot run before this insert lands, even on + // a sub-microsecond prover. { let pk0 = { let mc = state.minting_account.lock().unwrap(); @@ -4560,6 +4601,11 @@ async fn mint_handler_concurrent_mint_during_proof_returns_503() { .expect("inject pk_0 into SMT"); } + // Release the handler from the phase3_release hold. It now runs + // the phase-3 re-derive against the just-mutated SMT, observes + // the bumped count, and returns 503 "Concurrent mint detected". + state.phase3_release.notify_one(); + let (status, resp_body) = tokio::time::timeout(std::time::Duration::from_secs(60), request_task) .await diff --git a/node/src/runtime.rs b/node/src/runtime.rs index de2d5b7b..e5e5a732 100644 --- a/node/src/runtime.rs +++ b/node/src/runtime.rs @@ -101,6 +101,14 @@ pub async fn start_rest_node( esplora_config: Arc::new(NETWORK_CONFIG.clone()), #[cfg(test)] phase2_reached: Arc::new(tokio::sync::Notify::new()), + #[cfg(test)] + phase3_release: { + // Pre-arm so the handler's cfg(test) hold-point is a + // no-op for runtime tests that don't exercise the race. + let n = Arc::new(tokio::sync::Notify::new()); + n.notify_one(); + n + }, }; // Bootstrap the minting account if it isn't already in the DB. From 472e2c0b71f8ae1515b9aab158b12ecdb63d6851 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 25 May 2026 20:43:25 +0200 Subject: [PATCH 4/7] fix(router): switch phase3 hold from Notify to Mutex<()> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous Notify-based hold pre-armed exactly ONE permit. After the first mint consumed it, subsequent mints in the same process (e.g. mint_handler_two_sequential_mints_with_different_recipients_advance_cleanly) blocked forever — the test was observed SLOW [>720s] in the heavy gate's nextest run before timing out. A tokio::sync::Mutex<()> is the correct primitive: pre-unlocked, handler acquires + drops in one step for production-shaped tests (non-blocking), and the concurrent-mint race test holds the guard across pk_N injection then drops it. Reusable for any number of sequential mints without manual re-arming. --- node/src/router.rs | 34 ++++++++++++++------------ node/src/router_tests.rs | 53 +++++++++------------------------------- node/src/runtime.rs | 8 +----- 3 files changed, 32 insertions(+), 63 deletions(-) diff --git a/node/src/router.rs b/node/src/router.rs index f1a9a967..e78bf351 100644 --- a/node/src/router.rs +++ b/node/src/router.rs @@ -103,16 +103,19 @@ pub(crate) struct AppState { #[cfg(test)] pub(crate) phase2_reached: Arc, /// Test-only deterministic hold between `prepare_mint` (phase 2) - /// and the phase-3 re-derive. The handler `.notified().await`s - /// this AFTER `prepare_mint` returns and BEFORE the re-derive - /// reads SMT membership, so the concurrent-mint race test can - /// inject `pk_N` between the two without losing the race to a - /// fast prover. The notify is pre-armed (`notify_one()` called in - /// `test_state` constructors) so production-shaped tests that - /// don't care about the hold proceed immediately. Hidden behind - /// `cfg(test)` so the field does not exist in release builds. + /// and the phase-3 re-derive. The handler acquires + immediately + /// drops this mutex AFTER `prepare_mint` returns and BEFORE the + /// re-derive reads SMT membership. Constructed unlocked so all + /// production-shaped tests proceed immediately (acquire is a + /// non-blocking no-op). The concurrent-mint race test grabs the + /// guard BEFORE spawning the request, holds it across the pk_N + /// injection, then drops it — a hard happens-before edge that + /// works for any number of sequential mints (unlike a `Notify` + /// where one consumed permit would block subsequent waiters). + /// Hidden behind `cfg(test)` so the field does not exist in + /// release builds. #[cfg(test)] - pub(crate) phase3_release: Arc, + pub(crate) phase3_release_lock: Arc>, } // Response types for our API @@ -918,13 +921,14 @@ async fn mint_handler( }; // Test-only deterministic hold between `prepare_mint` and the - // phase-3 re-derive. Pre-armed in `test_state` constructors so the - // production-shaped tests proceed immediately. The - // concurrent-mint race test re-arms it AFTER injecting the SMT - // entry to guarantee phase 3 observes the injection. Production - // builds compile this out entirely (the field does not exist). + // phase-3 re-derive. Pre-unlocked in all `test_state` + // constructors so production-shaped tests acquire + drop in one + // step. The concurrent-mint race test holds the guard from the + // outside across the pk_N injection, forcing the handler to + // block here until the injection is visible. Production builds + // compile this out entirely (the field does not exist). #[cfg(test)] - state.phase3_release.notified().await; + drop(state.phase3_release_lock.lock().await); // Build the BIP-340 commitment over the prover's outputs. Sign with // the index-N private key — this is the same key the wallet would diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index 54b01d67..c7b98fd7 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -66,16 +66,7 @@ fn test_state() -> AppState { track_tx_timeout: None, }), phase2_reached: Arc::new(tokio::sync::Notify::new()), - phase3_release: { - // Pre-armed: production-shaped tests proceed immediately - // through the cfg(test) hold-point in mint_handler. The - // concurrent-mint race test (and any future test that - // wants to pause the handler between prove and re-derive) - // re-arms it via `notify_one()` after injecting state. - let n = Arc::new(tokio::sync::Notify::new()); - n.notify_one(); - n - }, + phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), } } @@ -2249,16 +2240,7 @@ async fn send_with_insufficient_funds_returns_422_with_error_string() { track_tx_timeout: None, }), phase2_reached: Arc::new(tokio::sync::Notify::new()), - phase3_release: { - // Pre-armed: production-shaped tests proceed immediately - // through the cfg(test) hold-point in mint_handler. The - // concurrent-mint race test (and any future test that - // wants to pause the handler between prove and re-derive) - // re-arms it via `notify_one()` after injecting state. - let n = Arc::new(tokio::sync::Notify::new()); - n.notify_one(); - n - }, + phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), }; let secret_bytes = include_bytes!("../minting_secret.bin"); @@ -3553,16 +3535,7 @@ fn mint_test_state() -> AppState { track_tx_timeout: None, }), phase2_reached: Arc::new(tokio::sync::Notify::new()), - phase3_release: { - // Pre-armed: production-shaped tests proceed immediately - // through the cfg(test) hold-point in mint_handler. The - // concurrent-mint race test (and any future test that - // wants to pause the handler between prove and re-derive) - // re-arms it via `notify_one()` after injecting state. - let n = Arc::new(tokio::sync::Notify::new()); - n.notify_one(); - n - }, + phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), } } @@ -4529,16 +4502,14 @@ async fn mint_handler_concurrent_mint_during_proof_returns_503() { use bitcoin::hashes::Hash; let state = mint_test_state(); - // Drain the pre-armed `phase3_release` permit that `mint_test_state` - // populates for production-shaped tests. With the permit consumed, - // the handler's `state.phase3_release.notified().await` between - // `prepare_mint` and the phase-3 re-derive will BLOCK until this - // test explicitly calls `notify_one()` AFTER injecting `pk_0`. - // This converts the previously-fragile timing race (prover faster - // than test's lock acquisition) into a deterministic happens-before - // edge: insert lands → notify → handler unblocks → re-derive sees - // the bumped count. - state.phase3_release.notified().await; + // Acquire `phase3_release_lock` BEFORE spawning the request. The + // handler's `lock().await` between `prepare_mint` and the phase-3 + // re-derive will BLOCK until this test drops the guard after + // injecting `pk_0`. Using a Mutex (vs a Notify with one-permit + // semantics) makes this primitive reusable for any number of + // sequential mints — production-shaped tests acquire + drop in + // one step against the unlocked Mutex. + let phase3_guard = state.phase3_release_lock.clone().lock_owned().await; // Pre-subscribe to the phase-2 notify BEFORE spawning the request // so a fast handler that acquires `account_node` and fires @@ -4604,7 +4575,7 @@ async fn mint_handler_concurrent_mint_during_proof_returns_503() { // Release the handler from the phase3_release hold. It now runs // the phase-3 re-derive against the just-mutated SMT, observes // the bumped count, and returns 503 "Concurrent mint detected". - state.phase3_release.notify_one(); + drop(phase3_guard); let (status, resp_body) = tokio::time::timeout(std::time::Duration::from_secs(60), request_task) diff --git a/node/src/runtime.rs b/node/src/runtime.rs index e5e5a732..3978f3dd 100644 --- a/node/src/runtime.rs +++ b/node/src/runtime.rs @@ -102,13 +102,7 @@ pub async fn start_rest_node( #[cfg(test)] phase2_reached: Arc::new(tokio::sync::Notify::new()), #[cfg(test)] - phase3_release: { - // Pre-arm so the handler's cfg(test) hold-point is a - // no-op for runtime tests that don't exercise the race. - let n = Arc::new(tokio::sync::Notify::new()); - n.notify_one(); - n - }, + phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), }; // Bootstrap the minting account if it isn't already in the DB. From 0b6a1d24b100079597376928b32a006b61a58e0a Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 25 May 2026 21:34:13 +0200 Subject: [PATCH 5/7] refactor(state,db): drop uncoverable 64-bit u64->i64/usize fallbacks The three `i64::try_from(leaf_index)` sites in db.rs and the `debug_assert!` in state.rs::load_from_pg all guard a hypothetical > i64::MAX (or 32-bit usize-overflow) condition that cannot fire on any target we ship (Linux x86_64 / aarch64). The error/panic arms were flagged by the 100% Coverage Gate as uncoverable, contributing 13 uncovered lines for zero production benefit. Replace each `try_from + sqlx::Error::Encode` with a direct `as i64` cast (and `as usize` in state.rs), keeping a one-line invariant comment that documents the bound. No behaviour change on 64-bit targets, which is the only deployment target. Affected sites: - db::persist_state_tx - db::persist_state_and_mark_complete_tx - db::insert_root_index - state::State::load_from_pg --- node/src/db.rs | 65 +++++++++++++++++------------------------------ node/src/state.rs | 18 +++---------- 2 files changed, 28 insertions(+), 55 deletions(-) diff --git a/node/src/db.rs b/node/src/db.rs index 1ae0e9db..e71de99f 100644 --- a/node/src/db.rs +++ b/node/src/db.rs @@ -126,27 +126,17 @@ pub async fn persist_state_tx( latest_block: &[u8; 32], root_index_entry: Option<(&HashDigest, &HashDigest, u64)>, ) -> Result<(), sqlx::Error> { - // Pre-encode the optional root_index columns OUTSIDE the tx so a - // bad `leaf_index` (e.g. > i64::MAX in some hypothetical future) - // surfaces before we open a Postgres connection. Today the value - // comes from `mmr.leaf_count()` so the conversion is infallible in - // practice; keep the defensive error for symmetry with the - // standalone `insert_root_index` helper. - let root_index_bytes = match root_index_entry { - None => None, - Some((prev_root, smt_root, leaf_index)) => { - let leaf_i64 = i64::try_from(leaf_index).map_err(|_| { - sqlx::Error::Encode( - format!("leaf_index {} does not fit in i64 (BIGINT)", leaf_index).into(), - ) - })?; - Some(( - digest_to_bytes(prev_root), - digest_to_bytes(smt_root), - leaf_i64, - )) - } - }; + // `leaf_index` is a `u64` coming from `mmr.leaf_count()`, which is + // bounded by the total inscription count (≪ 2^63 in practice). The + // cast is infallible on 64-bit targets, which is our only deployment + // target (Linux x86_64 / aarch64). + let root_index_bytes = root_index_entry.map(|(prev_root, smt_root, leaf_index)| { + ( + digest_to_bytes(prev_root), + digest_to_bytes(smt_root), + leaf_index as i64, + ) + }); let mut tx = pool.begin().await?; sqlx::query( @@ -242,21 +232,15 @@ pub async fn persist_state_and_mark_complete_tx( root_index_entry: Option<(&HashDigest, &HashDigest, u64)>, commit_txid: &[u8], ) -> Result<(), sqlx::Error> { - let root_index_bytes = match root_index_entry { - None => None, - Some((prev_root, smt_root, leaf_index)) => { - let leaf_i64 = i64::try_from(leaf_index).map_err(|_| { - sqlx::Error::Encode( - format!("leaf_index {} does not fit in i64 (BIGINT)", leaf_index).into(), - ) - })?; - Some(( - digest_to_bytes(prev_root), - digest_to_bytes(smt_root), - leaf_i64, - )) - } - }; + // See `persist_state_tx` for why the `u64 -> i64` cast is infallible + // on every target we ship. + let root_index_bytes = root_index_entry.map(|(prev_root, smt_root, leaf_index)| { + ( + digest_to_bytes(prev_root), + digest_to_bytes(smt_root), + leaf_index as i64, + ) + }); let mut tx = pool.begin().await?; sqlx::query( @@ -594,11 +578,10 @@ pub async fn insert_root_index( ) -> Result<(), sqlx::Error> { let prev_bytes = digest_to_bytes(prev_root); let smt_bytes = digest_to_bytes(smt_root); - let leaf_i64 = i64::try_from(leaf_index).map_err(|_| { - sqlx::Error::Encode( - format!("leaf_index {} does not fit in i64 (BIGINT)", leaf_index).into(), - ) - })?; + // MMR leaf_index is bounded by total inscription count (≪ 2^63 in + // practice); the cast is infallible on 64-bit targets which is our + // only deployment target. + let leaf_i64 = leaf_index as i64; sqlx::query( "INSERT INTO mmr_root_index (prev_mmr_root, smt_root, leaf_index, created_at) \ VALUES ($1, $2, $3, NOW()) \ diff --git a/node/src/state.rs b/node/src/state.rs index 250bf5c6..83a6a587 100644 --- a/node/src/state.rs +++ b/node/src/state.rs @@ -294,20 +294,10 @@ impl State { // we go so we don't have to re-scan the assembled HashMap. let mut last_key: Option = None; for (prev_root, smt_root, leaf_index) in entries { - // `leaf_index` came back as `u64` and was previously checked - // non-negative by `db::load_root_indices`. The production - // target is 64-bit (Linux x86_64 / aarch64), so the cast is - // provably infallible — `usize::try_from` would only fail on - // a 32-bit target, which we don't ship. `debug_assert!` - // guards the hypothetical 32-bit dev build without forcing - // an uncoverable error branch on the production target, - // which the Coverage Gate (100% lines+functions on - // `state.rs`) cannot exercise. - debug_assert!( - leaf_index <= usize::MAX as u64, - "mmr_root_index.leaf_index {} does not fit in usize on this target", - leaf_index - ); + // `leaf_index` is a `u64` from Postgres, non-negative by the + // load query's filter. The production target is 64-bit + // (Linux x86_64 / aarch64), so the cast to `usize` is + // infallible. let leaf_usize = leaf_index as usize; state.root_indices.insert(prev_root, (smt_root, leaf_usize)); last_key = Some(prev_root); From f68c7704eea367d30de3bb2823de67ef7b47115f Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 25 May 2026 21:36:11 +0200 Subject: [PATCH 6/7] refactor(publisher,router): drop dead Option from create_and_broadcast return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `create_and_broadcast_inscription` returned `Result>` but the body has no path that yields `Ok(None)` — every success arm builds `Ok(Some((c, r)))` and every failure surfaces as `Err`. The dead `Ok(None)` arm in `router::mint_handler` together with the defensive `if let Some(ctxid) = commit_txid_bytes { .. } else { .. }` fallback contributed 13 uncovered lines to the Coverage Gate. Flatten the API to `Result<(Txid, Txid), ...>`: - publisher.rs: return tuple directly on Ok. - router.rs: match yields `[u8; 32]` instead of `Option<[u8; 32]>`; collapse the `if let Some(ctxid) { .. } else { .. }` to a single block. - publisher_tests.rs: drop the now-redundant `.expect("Some((c,r))")` unwrap layer at three call sites. No behaviour change — the removed branches were unreachable. --- node/src/publisher.rs | 4 +- node/src/publisher_tests.rs | 16 +++---- node/src/router.rs | 92 ++++++++++++++++--------------------- 3 files changed, 47 insertions(+), 65 deletions(-) diff --git a/node/src/publisher.rs b/node/src/publisher.rs index b9199694..7fdd7182 100644 --- a/node/src/publisher.rs +++ b/node/src/publisher.rs @@ -563,7 +563,7 @@ pub async fn create_and_broadcast_inscription( commitment_data: &[u8], config: &EsploraConfig, pool: Option<&PgPool>, -) -> Result, Box> { +) -> Result<(Txid, Txid), Box> { // Generate publisher address let publisher_key = &*crate::PUBLISHER_KEY; let secp256k1 = Secp256k1::new(); @@ -668,7 +668,7 @@ pub async fn create_and_broadcast_inscription( println!("Successfully broadcast transactions:"); println!("Commit TXID: {}", commit_txid); println!("Reveal TXID: {}", reveal_txid); - Ok(Some((commit_txid, reveal_txid))) + Ok((commit_txid, reveal_txid)) } Err(e) => { println!("Failed to broadcast transactions: {}", e); diff --git a/node/src/publisher_tests.rs b/node/src/publisher_tests.rs index a78bd82a..06073675 100644 --- a/node/src/publisher_tests.rs +++ b/node/src/publisher_tests.rs @@ -558,7 +558,7 @@ async fn create_and_broadcast_inscription_fails_when_no_utxos() { let err = create_and_broadcast_inscription(b"Hello, zkCoins!", &config, None) .await - .expect_err("empty wallet must produce an Err, not Ok(None)"); + .expect_err("empty wallet must produce an Err"); assert!( err.to_string().contains("No UTXOs available"), @@ -596,12 +596,10 @@ async fn create_and_broadcast_inscription_succeeds_end_to_end_with_mocked_esplor .mount(&server) .await; - let result = create_and_broadcast_inscription(b"Hello, zkCoins!", &config, None) - .await - .expect("end-to-end inscription should succeed against mocked Esplora"); - let (commit_txid, reveal_txid) = - result.expect("on success the function returns Some((commit, reveal))"); + create_and_broadcast_inscription(b"Hello, zkCoins!", &config, None) + .await + .expect("end-to-end inscription should succeed against mocked Esplora"); assert_ne!( commit_txid, reveal_txid, "commit and reveal must be distinct transactions" @@ -869,10 +867,9 @@ async fn broadcast_advances_to_reveal_broadcast_after_reveal_success() { .mount(&server) .await; - let result = create_and_broadcast_inscription(b"phaseb-3", &config, Some(&pool)) + let _result = create_and_broadcast_inscription(b"phaseb-3", &config, Some(&pool)) .await .expect("happy path must succeed"); - assert!(result.is_some(), "successful broadcast returns Some((c,r))"); // Final state is `reveal_broadcast` — see Phase E note above. assert_eq!(count_pending_rows(&pool).await, 1); @@ -1217,8 +1214,7 @@ async fn mint_handler_advances_state_synchronously_with_broadcast() { let (commit_txid, _reveal_txid) = create_and_broadcast_inscription(b"phase-e-1", &config, Some(&pool)) .await - .expect("happy path must succeed") - .expect("Some((commit, reveal)) on Ok"); + .expect("happy path must succeed"); // Publisher leg stopped at `reveal_broadcast` — the `mint_handler` // caller is what flips it to `complete` after running diff --git a/node/src/router.rs b/node/src/router.rs index e78bf351..40d02833 100644 --- a/node/src/router.rs +++ b/node/src/router.rs @@ -1006,12 +1006,11 @@ async fn mint_handler( Some(&state.pool), ) .await; - let commit_txid_bytes: Option<[u8; 32]> = match broadcast_outcome { - Ok(Some((commit_txid, _reveal_txid))) => { + let commit_txid_bytes: [u8; 32] = match broadcast_outcome { + Ok((commit_txid, _reveal_txid)) => { use bitcoin::hashes::Hash as _; - Some(commit_txid.to_byte_array()) + commit_txid.to_byte_array() } - Ok(None) => None, Err(err) => { eprintln!("Error broadcasting mint inscription: {}", err); return handler_error_response( @@ -1094,56 +1093,43 @@ async fn mint_handler( ); } }; - if let Some(ctxid) = commit_txid_bytes { - let root_index_ref = root_index_entry.as_ref().map(|(p, s, i)| (p, s, *i as u64)); - match db::persist_state_and_mark_complete_tx( - &state.pool, - &smt_bytes, - &mmr_bytes, - root_index_ref, - &ctxid, - ) - .await - { - Ok(()) => { - println!( - "mint_handler: state.update persisted + row marked complete. New MMR root: {}", - hex::encode(zkcoins_program::hash::digest_to_bytes(&new_root)) - ); - } - Err(e) => { - // The atomic tx rolled back: SMT/MMR/root_index AND - // the row advance all stayed at their pre-call values - // on disk. The in-memory SMT/MMR HAVE already been - // mutated (that happened above before the await), so - // they are now ahead of disk by exactly one leaf. - // On restart, `State::load_from_pg` returns the - // pre-update on-disk state and the scanner-replay path - // walks the block, observes the row at - // `reveal_broadcast`, and integrates the inscription - // itself — a clean heal. Return 503 so the caller - // knows the durable state did not advance. - eprintln!( - "mint_handler: atomic persist + mark-complete failed: {} (scanner-replay will heal)", - e - ); - return handler_error_response( - StatusCode::SERVICE_UNAVAILABLE, - "mint broadcast landed on chain but durable state advance failed; scanner will reconcile", - ); - } + let root_index_ref = root_index_entry.as_ref().map(|(p, s, i)| (p, s, *i as u64)); + match db::persist_state_and_mark_complete_tx( + &state.pool, + &smt_bytes, + &mmr_bytes, + root_index_ref, + &commit_txid_bytes, + ) + .await + { + Ok(()) => { + println!( + "mint_handler: state.update persisted + row marked complete. New MMR root: {}", + hex::encode(zkcoins_program::hash::digest_to_bytes(&new_root)) + ); + } + Err(e) => { + // The atomic tx rolled back: SMT/MMR/root_index AND + // the row advance all stayed at their pre-call values + // on disk. The in-memory SMT/MMR HAVE already been + // mutated (that happened above before the await), so + // they are now ahead of disk by exactly one leaf. + // On restart, `State::load_from_pg` returns the + // pre-update on-disk state and the scanner-replay path + // walks the block, observes the row at + // `reveal_broadcast`, and integrates the inscription + // itself — a clean heal. Return 503 so the caller + // knows the durable state did not advance. + eprintln!( + "mint_handler: atomic persist + mark-complete failed: {} (scanner-replay will heal)", + e + ); + return handler_error_response( + StatusCode::SERVICE_UNAVAILABLE, + "mint broadcast landed on chain but durable state advance failed; scanner will reconcile", + ); } - } else { - // No commit_txid: the broadcast path returned Ok(None) (the - // test-only `Some(&state.pool)` no-pool branch should not - // surface here in production; defensive fallback). The - // in-memory state is already advanced; no row to mark complete. - // Persist SMT/MMR/root_index alone via a degenerate empty - // commit_txid is not meaningful, so just log and continue — - // production builds always have a commit_txid on success. - eprintln!( - "mint_handler: state.update advanced in-memory but no commit_txid available; persist skipped" - ); } // ---- 4. COMMIT phase (broadcast OK) --------------------------------- From 1dcebdb47aa232cca7670cd13409917df31d6d47 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 25 May 2026 22:17:20 +0200 Subject: [PATCH 7/7] test(router): cover in-process state.update Err 503 path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The phase-3b state-advance Err arm in `mint_handler` (the 503 returned when `update_and_snapshot_for_persist` fails on an SMT key-collision-with-different-value) was uncovered by the 100% Coverage Gate. The race that produces this state in production needs two concurrent mints whose phase-2 re-derives BOTH pass and whose broadcasts both land before either state lock acquires — too brittle to reproduce deterministically with two real requests. Add a deterministic test that exercises the same code path: - Mirror `phase3_release_lock` with a new `state_advance_release_lock` test-only AppState field. The handler acquires + immediately drops it AFTER `create_and_broadcast_inscription` returns and BEFORE the phase-3b state lock. Production builds compile this out entirely (both the field and the hold point are `#[cfg(test)]`). - The new test holds the guard across a colliding-SMT injection at `pk_0`. When the guard drops, the handler's `state.update` observes the collision and returns 503 with the documented "in-process state advance failed" reason. Asserts also confirm the pending row stays at `reveal_broadcast`, the on-disk SMT/MMR/ root_index DID NOT advance (no atomic persist tx ran), and the scanner-replay path stays armed. --- node/src/router.rs | 24 +++++ node/src/router_tests.rs | 202 +++++++++++++++++++++++++++++++++++++++ node/src/runtime.rs | 2 + 3 files changed, 228 insertions(+) diff --git a/node/src/router.rs b/node/src/router.rs index 40d02833..8247c37b 100644 --- a/node/src/router.rs +++ b/node/src/router.rs @@ -116,6 +116,20 @@ pub(crate) struct AppState { /// release builds. #[cfg(test)] pub(crate) phase3_release_lock: Arc>, + /// Test-only deterministic hold between the broadcast result and + /// the phase-3b state advance (`update_and_snapshot_for_persist`). + /// Mirrors `phase3_release_lock`: the handler acquires + immediately + /// drops this mutex AFTER `create_and_broadcast_inscription` returns + /// and BEFORE acquiring the state lock to apply the new commitment. + /// Constructed unlocked so production-shaped tests proceed + /// immediately. The in-process state.update Err test grabs the + /// guard before spawning the request, lets the handler run through + /// broadcast, injects the colliding SMT entry, then drops the + /// guard — at which point the handler's `state.update` observes + /// the collision and returns 503. Hidden behind `cfg(test)` so the + /// field does not exist in release builds. + #[cfg(test)] + pub(crate) state_advance_release_lock: Arc>, } // Response types for our API @@ -1061,6 +1075,16 @@ async fn mint_handler( // `reveal_broadcast` and the in-memory state advance was NOT // persisted to disk (transaction atomicity); the scanner will // replay cleanly. + // Test-only deterministic hold between the broadcast result and + // the phase-3b state advance. Pre-unlocked in all `test_state` + // constructors so production-shaped tests acquire + drop in one + // step. The in-process state.update Err test holds the guard + // across a colliding SMT injection so the handler observes the + // collision when its `state.update` finally runs. Production + // builds compile this out entirely (the field does not exist). + #[cfg(test)] + drop(state.state_advance_release_lock.lock().await); + let state_advance_outcome = { let state_arc_for_advance = { let account_node_guard = lock_or_recover(&state.account_node); diff --git a/node/src/router_tests.rs b/node/src/router_tests.rs index c7b98fd7..fbcd6763 100644 --- a/node/src/router_tests.rs +++ b/node/src/router_tests.rs @@ -67,6 +67,7 @@ fn test_state() -> AppState { }), phase2_reached: Arc::new(tokio::sync::Notify::new()), phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), + state_advance_release_lock: Arc::new(tokio::sync::Mutex::new(())), } } @@ -2241,6 +2242,7 @@ async fn send_with_insufficient_funds_returns_422_with_error_string() { }), phase2_reached: Arc::new(tokio::sync::Notify::new()), phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), + state_advance_release_lock: Arc::new(tokio::sync::Mutex::new(())), }; let secret_bytes = include_bytes!("../minting_secret.bin"); @@ -3536,6 +3538,7 @@ fn mint_test_state() -> AppState { }), phase2_reached: Arc::new(tokio::sync::Notify::new()), phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), + state_advance_release_lock: Arc::new(tokio::sync::Mutex::new(())), } } @@ -4915,6 +4918,205 @@ async fn mint_handler_atomic_tx_rollback_leaves_state_and_row_consistent() { .unwrap(); } +/// Phase E in-process state.update Err coverage: if the SMT already +/// contains the mint's signing pubkey under a DIFFERENT value when +/// `update_and_snapshot_for_persist` runs (a concurrent-mint race that +/// slipped both phase-2 gates, or a genuine bug), the handler must +/// return 503 with the documented "in-process state advance failed" +/// reason. The broadcast already landed on chain at this point, so the +/// publisher has advanced the row to `reveal_broadcast`; the scanner- +/// replay path picks the inscription up from chain on its next sweep. +/// +/// Mechanism: hold `state_advance_release_lock` BEFORE spawning the +/// request so the handler blocks AFTER the broadcast and BEFORE +/// acquiring the state lock for `update_and_snapshot_for_persist`. Mid- +/// hold, inject `pk_0`'s key into the SMT with a bogus value. Drop the +/// guard — the handler resumes, the SMT `insert` returns +/// `"Key already exists in the tree with different value"`, and the +/// match arm at the top of phase 3b surfaces 503. +/// +/// Asserts: +/// - response is 503 with the expected error message +/// - the pending_inscriptions row stays at `reveal_broadcast` +/// - the on-disk SMT/MMR/root_index DID NOT advance (no atomic +/// persist tx ran for this mint) +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn mint_handler_in_process_state_advance_collision_returns_503() { + use bitcoin::hashes::Hash; + use testcontainers::{runners::AsyncRunner, ImageExt}; + use testcontainers_modules::postgres::Postgres; + + let pg_container = Postgres::default() + .with_tag("17") + .start() + .await + .expect("failed to start postgres container"); + let host = pg_container.get_host().await.unwrap(); + let port = pg_container.get_host_port_ipv4(5432).await.unwrap(); + let url = format!("postgres://postgres:postgres@{}:{}/postgres", host, port); + let pool = Arc::new( + crate::db::connect_and_migrate(&url) + .await + .expect("connect_and_migrate failed"), + ); + + let mock_server = mint_broadcast_mock_server().await; + let ws_url = mint_broadcast_mock_ws().await; + + let mut state = mint_test_state(); + state.pool = Arc::clone(&pool); + state.esplora_config = Arc::new(crate::publisher::EsploraConfig { + url: mock_server.uri(), + is_mainnet: false, + network_name: "Mutinynet".to_string(), + ws_url: Some(ws_url), + track_tx_timeout: None, + }); + + // Hold the state-advance release lock so the handler will block + // after broadcast and before `update_and_snapshot_for_persist`. + let advance_guard = state.state_advance_release_lock.clone().lock_owned().await; + + let recipient = "0x".to_string() + &hex::encode([12u8; 32]); + let body = serde_json::json!({ + "account_address": recipient, + "amount": 1u64, + }); + let req = Request::post("/api/mint") + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .unwrap(); + + let state_for_request = state.clone(); + let request_task = + tokio::spawn(async move { send_request_with_state(state_for_request, req).await }); + + // Wait until the publisher has advanced the row to + // `reveal_broadcast` — that is the observable signal that the + // broadcast has landed and the handler is now blocked on the + // state_advance_release_lock. Polling avoids races with the + // publisher's WS handshake; a hard timeout guards against a + // regression that would otherwise hang for the full CI budget. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(60); + let commit_txid_bytes: Vec = loop { + if std::time::Instant::now() > deadline { + panic!( + "publisher did not advance any pending row to `reveal_broadcast` within 60s; \ + regression in mint_handler broadcast phase" + ); + } + let row: Option<(Vec, String)> = sqlx::query_as( + "SELECT commit_txid, status FROM pending_inscriptions ORDER BY id DESC LIMIT 1", + ) + .fetch_optional(&*pool) + .await + .unwrap(); + if let Some((ctxid, status)) = row { + if status == crate::db::PENDING_STATUS_REVEAL_BROADCAST { + break ctxid; + } + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + }; + + // Inject pk_0's key into the SMT with a value that will NOT match + // what `update_and_snapshot_for_persist` is about to write. The + // handler is currently blocked on the state_advance_release_lock + // (drained above) so its SMT mutation cannot run before this + // injection lands. + { + let pk0 = { + let mc = state.minting_account.lock().unwrap(); + mc.generate_public_key(0) + }; + let key: [u8; 32] = bitcoin::hashes::sha256::Hash::hash(&pk0.serialize()).to_byte_array(); + let node_guard = state.account_node.lock().unwrap(); + let state_arc = node_guard.state().clone(); + drop(node_guard); + let mut state_guard = state_arc.lock().unwrap(); + // A digest that does NOT match the legitimate + // `commitment.get_account_state_hash()` the handler will derive. + state_guard + .smt + .insert(key, zkcoins_program::hash::digest_from_bytes(&[0xAAu8; 32])) + .expect("inject pk_0 -> bogus value into SMT"); + } + + // Release the handler. It now runs `update_and_snapshot_for_persist`, + // the SMT insert at pk_0 errors with "Key already exists in the + // tree with different value", and the handler returns 503. + drop(advance_guard); + + let (status, resp_body) = + tokio::time::timeout(std::time::Duration::from_secs(60), request_task) + .await + .expect("mint request must complete within 60s") + .expect("request task panicked"); + + assert_eq!( + status, + StatusCode::SERVICE_UNAVAILABLE, + "in-process state.update collision must surface 503, body: {}", + resp_body + ); + let v: serde_json::Value = serde_json::from_str(&resp_body).expect("valid JSON"); + assert_eq!(v["success"], false); + assert!( + v["error"] + .as_str() + .unwrap_or("") + .contains("in-process state advance failed"), + "response error must explain the in-process collision failure mode, got: {}", + v["error"] + ); + + // The pending row stays at `reveal_broadcast`: the publisher set it + // there before the broadcast and the handler bailed before the + // atomic persist + mark-complete tx could run. + let (pending_status,): (String,) = + sqlx::query_as("SELECT status FROM pending_inscriptions WHERE commit_txid = $1") + .bind(&commit_txid_bytes) + .fetch_one(&*pool) + .await + .expect("the broadcasted commitment's row must exist"); + assert_eq!( + pending_status, + crate::db::PENDING_STATUS_REVEAL_BROADCAST, + "in-process collision: pending row must stay at reveal_broadcast for scanner-replay to pick up" + ); + + // On-disk SMT/MMR/root_index did NOT advance — the handler bailed + // before invoking the atomic persist + mark-complete transaction. + assert_eq!( + crate::db::load_smt(&pool).await.unwrap(), + None, + "in-process collision must leave smt_state untouched" + ); + assert_eq!( + crate::db::load_mmr(&pool).await.unwrap(), + None, + "in-process collision must leave mmr_state untouched" + ); + assert!( + crate::db::load_root_indices(&pool) + .await + .unwrap() + .is_empty(), + "in-process collision must leave mmr_root_index untouched" + ); + + // Scanner-replay path stays armed (row not at `complete`). + assert!( + !crate::scanner::should_skip_scanner_state_update( + crate::db::pending_inscription_status_by_commit_txid(&pool, &commit_txid_bytes) + .await + .unwrap() + .as_deref() + ), + "scanner must NOT skip its state.update for an inscription whose in-process advance failed" + ); +} + /// Phase E concurrent-mint coverage: two `/api/mint` requests with /// DIFFERENT recipients (different commitments → different SMT keys) /// must both succeed end-to-end. Both walk past the phase-2 re-derive diff --git a/node/src/runtime.rs b/node/src/runtime.rs index 3978f3dd..4b375b18 100644 --- a/node/src/runtime.rs +++ b/node/src/runtime.rs @@ -103,6 +103,8 @@ pub async fn start_rest_node( phase2_reached: Arc::new(tokio::sync::Notify::new()), #[cfg(test)] phase3_release_lock: Arc::new(tokio::sync::Mutex::new(())), + #[cfg(test)] + state_advance_release_lock: Arc::new(tokio::sync::Mutex::new(())), }; // Bootstrap the minting account if it isn't already in the DB.