KickoffReady permanently dropped when previous-graph row is missing from local DB — withdrawal stuck forever with no re-trigger path

[dupint]

Summary

In handle_kickoff_ready_operator (node/src/handle.rs:4796-4811), when the backfill loop cannot find a previous graph row by nonce in the local DB, the handler logs a warning and return Ok(()) — the message is acknowledged and permanently discarded. Every other transient-condition branch in the same function uses the defer/retry pattern (get_graph_or_defer / push_local_unhandled_messages_with_reason), but this one does not.

The drop is unrecoverable because the only producer of KickoffReady — detect_init_withdraw_call (node/src/scheduled_tasks/graph_maintenance_tasks.rs:243-287) — flips the goat_tx_record for the InitWithdraw to Processed in the same transaction that enqueues the message (line 277-283), and never scans it again. Even a full L2 re-org / re-fire of the InitWithdraw event cannot re-queue it, because upsert_goat_tx_record (crates/store/src/localdb.rs:3571-3573) pins an already-Processed status.

Result: a user withdrawal that is already initialized on-chain (pegout funds in Gateway escrow) is silently and permanently skipped by the operator — no kickoff, no pegout, funds stuck until manual DB surgery.

(availability / permanent user-funds lock) — CVSS:3.1/AV:N/AC:H/PR:N/UI:R/S:U/C:N/I:N/A:H (6.8)

Impact argument: the trigger window is a normal operational race (previous graph row not yet persisted locally while its successor is already OperatorDataPushed on L2), and the failure mode is permanent with zero self-healing paths. The user has locked pegin UTXOs + minted pegBTC is escrowed at the Gateway, cannot proceed, and — because the Gateway cancel path is unavailable while status is Initialized and the node's own cancel handling is inert — cannot even restart the flow themselves. This is the same "one-shot trigger, no retry" bug class as #429 but at the kickoff-entry site, and unlike #429 (missed challenge) here user withdrawal funds are stuck indefinitely.

Affected code

• node/src/handle.rs:4796-4811 — the fail-closed return Ok(())
• node/src/scheduled_tasks/graph_maintenance_tasks.rs:243-287 — one-shot Pending scan + same-tick flip to Processed
• crates/store/src/localdb.rs:3571-3573 — upsert_goat_tx_record pins Processed, blocking event re-fire recovery
• node/src/utils.rs:5054-5070 — get_graph_id_by_nonce (LIMIT 1, no ORDER BY) used by the drop check

Root cause

// node/src/handle.rs (handle_kickoff_ready_operator)
let start_nonce = match get_latest_pegout_finalized_graph(ctx.local_db, &operator_pubkey).await? {
    Some((n, _)) => n + 1,
    None => 0,
};
for current_nonce in start_nonce..graph.parameters.graph_nonce {
    let (current_instance_id, current_graph_id) = match get_graph_id_by_nonce(
        ctx.local_db, current_nonce, &operator_pubkey,
    ).await? {
        Some(v) => v,
        None => {
            tracing::warn!(
                "Ignore KickoffReady for {instance_id}:{graph_id}: missing previous graph {current_nonce}"
            );
            return Ok(());   // ⬅ message consumed & dropped PERMANENTLY — no defer, no SyncGraphRequest
        }
    };
    // ...

Six lines above the loop, the current graph lookup uses get_graph_or_defer(...), which on a miss sends a SyncGraphRequest to peers and re-queues the message with a MessageDeferReason. The same defer pattern is used again in this very function at handle.rs:4867 (PreviousGraphPending) and handle.rs:4883 (ChainStatePending). The missing previous graph branch is the only transient-condition path in the handler that treats "row not in my DB yet" as "not my problem ever".

Two independent facts make this a bug and not a harmless guard:

1. The row appearing later is normal, not exceptional. A graph row is inserted into the operator's local DB when its CreateGraph/confirmation messages are processed — and those paths themselves defer with 60-second retries (defer_confirm_instance_until_previous_graph_presigned, handle.rs:1539). Meanwhile users can call initWithdraw() on L2 as soon as a graph is OperatorDataPushed. So the detector can legitimately fire for graph N+K while graph N's row is still in a defer loop on the same operator's node.
2. There is no second trigger. Exhaustive check of producers/recovery paths:
   • KickoffReady is only enqueued by detect_init_withdraw_call (scan of Pending records) — after the drop, the record is Processed, never scanned again.
   • handle_proceed_withdraw_events also flips the record to Processed.
   • L2 event re-consumption (re-org replay, resync, bridge_out_start_at cursor rewind): upsert_goat_tx_record keeps the stored Processed status (is_processed() guard) → Pending never returns.
   • debug_handler / RPC paths: no re-enqueue of KickoffReady; the RPC pegout handler additionally rejects graphs with init_withdraw_tx_hash.is_some(), so the manual proceed path is closed too.
   • The unhandled-message retry loop only processes messages that were pushed with a defer reason — this message was never pushed.

Steps to reproduce (deterministic logic-level repro)

Full end-to-end requires two operators' message timing, but the mechanism is provable at DB layer with the exact schema and queries from the code. Repro script (sqlite3, schema from crates/store/migrations/20250814114142_create_goat_tx_record_table.sql + graph migration; queries from localdb.rs / utils.rs):

-- state at tick 1: graph nonce 7 exists, nonce 6 row NOT yet inserted (deferred CreateGraph),
-- goat_tx_record for graph 7 = Pending (InitWithdraw detected)

-- TICK 1 — detect_init_withdraw_call:
SELECT graph_id FROM goat_tx_record
 WHERE tx_type='InitWithdraw' AND processing_status='Pending';
-- → g7  ⇒ KickoffReady(g7) enqueued; record g7 flipped to Processed (same tx, line 277)

-- handler: get_graph_id_by_nonce(6) — exact query of get_operator_graphs:
SELECT instance_id, graph_id FROM graph
 WHERE operator_pubkey=x'op' AND kickoff_index=6 LIMIT 1;
-- → empty ⇒ warn "missing previous graph 6" ⇒ return Ok(()) ⇒ message consumed forever

-- LATER: nonce-6 row finally lands (CreateGraph processed)
-- TICK N — detector again:
SELECT graph_id FROM goat_tx_record
 WHERE tx_type='InitWithdraw' AND processing_status='Pending';
-- → EMPTY. g7 is Processed. KickoffReady(g7) is never produced again.

-- Even replaying the InitWithdraw L2 event cannot save it:
-- upsert_goat_tx_record: if stored.is_processed() { keep stored.processing_status }

Observed output of the repro run:

TICK1 detect: [('g7',)]
get_graph_id_by_nonce(6): []          -> handler: return Ok(()) -> message DROPPED
TICK N detect after g6 lands: []      -> record already Processed -> NO re-trigger
after InitWithdraw event re-fire: Processed -> upsert pins status -> re-queue impossible

Impact

• User calls initWithdraw() for graph N+K on L2; Gateway escrow locks the pegout; node detects it and consumes the only kickoff trigger.
• Because the previous graph N's row has not landed locally yet (normal defer race), the kickoff silently dies with one WARN line in the log.
• The withdrawal is stuck permanently: funds remain in Gateway escrow, pegout never starts, and neither the user, the watcher, nor the node can revive it without an operator manually resetting goat_tx_record.processing_status in the node DB (undocumented, error-prone, per-instance).
• Repeatable across operator restarts, restores from snapshot, or key migration to a fresh node (fresh DB = maximal missing-row window at exactly the moment the detector runs).
• Denial of exit for bridge users is a funds-safety incident for a BitVM bridge: TVL confidence damage even though no funds are stolen.

Suggested fix

Treat "previous graph row missing" as the transient condition it is — reuse the machinery already in the same function:

None => {
    // request the missing graph from peers and defer-retry this message,
    // mirroring get_graph_or_defer / defer_confirm_instance_until_previous_graph_presigned
    try_send_sync_graph_request(ctx, current_nonce, &operator_pubkey).await?;
    push_local_unhandled_messages_with_reason(
        ctx.local_db, graph_id, &message,
        RETRY_DELAY_SECS, MessageDeferReason::PreviousGraphPending,
        "previous graph row not yet in local db",
    ).await?;
    return Ok(());
}

(Alternatively: have detect_init_withdraw_call flip the record to Processed only after the KickoffReady handler succeeds, instead of in the same tick — but the defer pattern is the established idiom in this file and keeps the detector simple.)

Hardening note (secondary, not scored separately)

get_graph_id_by_nonce (node/src/utils.rs:5054) queries (operator_pubkey, kickoff_index) with LIMIT 1 and no ORDER BY, and there is no UNIQUE constraint on (operator_pubkey, kickoff_index) in the graph migration (PK is only graph_id). If duplicate rows ever appear for one nonce, which (instance_id, graph_id) the kickoff backfill reads is undefined. Recommend a deterministic ORDER BY / unique index.

Deduplication notes

Checked open+closed issues for KickoffReady, InitWithdraw, missing previous graph:

• [Security] Verifier KickoffSent challenge is one-shot and fail-open (unauthorized Take1) #429 (awarded, Med) — one-shot challenge send fail-open. Different site (assertion-challenge timing), different trigger; this report is the kickoff-entry analogue at a different code path.
• [Security] detect_kickoff only watches one OperatorDataPushed graph per operator (unauthorized Take1) #431 (open) — kickoff detector only watching one graph per operator. Related theme (kickoff detection gaps) but different root cause: [Security] detect_kickoff only watches one OperatorDataPushed graph per operator (unauthorized Take1) #431 is about which graphs get scanned; this is about the KickoffReady message being consumed-and-dropped after detection, plus the unrecoverable Processed pin.
• Optimize KickoffReady handle #391 "Optimize KickoffReady handle" (closed, merged refactor) — performance refactor, does not report or fix this drop path; the current code with the return Ok(())) branch is post-Optimize KickoffReady handle #391.