fix: prevent TOCTOU race condition in UTXO locking (issue #125)

[idan57570idan-svg]

Summary

Fixes the TOCTOU race condition in FundLocker::lock() described in #125.

Root cause: The idempotency check and UTXO selection ran outside any database transaction. Two concurrent calls could both pass the idempotency check, select the same UTXOs as 'Unspent', and produce conflicting pending transactions.

Additionally, lock_output() silently returned Ok(()) when the UPDATE matched 0 rows (UTXO already locked by a concurrent request), causing callers to receive a LockFundsResult indicating successful locking for UTXOs they do not actually hold.

Changes

**und_locker.rs** — Move BEGIN IMMEDIATE transaction to before the idempotency check:

• Acquires a write-reservation lock before any reads
• Makes idempotency check + UTXO selection + locking fully atomic
• Uses &*transaction\ for DB calls (Transaction derefs to Connection — no signature changes needed in db functions)

db/outputs.rs — Check rows_affected in lock_output:

• Returns \OutputAlreadyLocked\ error if UPDATE matches 0 rows (defense-in-depth)
• Ensures callers receive a clear error instead of silent false success

db/error.rs — Add \OutputAlreadyLocked { output_id: i64 }\ variant.

Why BEGIN IMMEDIATE

\conn.transaction()\ defaults to \BEGIN DEFERRED, which only acquires a write lock when the first write operation happens. This leaves a race window between the read phase (idempotency check + UTXO selection) and the write phase (create_pending_tx + lock_output).

\BEGIN IMMEDIATE\ acquires a reserved lock immediately, serializing all concurrent \lock()\ calls through the critical section. Since \FundLocker::lock()\ is the only write path for UTXO locking, this provides complete protection without excessive lock contention.

Fixes #125 (recreation of #41)

[gemini-code-assist]

Code Review

This pull request introduces a new OutputAlreadyLocked error variant and updates lock_output to return this error when no rows are affected. It also wraps the idempotency check and UTXO selection in FundLocker within an immediate transaction to prevent TOCTOU races. The review feedback suggests optimizing this transaction logic using a double-checked locking pattern to reduce database lock contention on duplicate requests, and simplifying the transaction references by leveraging Rust's deref coercion.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Acquiring an IMMEDIATE transaction lock immediately on every call to lock can cause unnecessary database lock contention, especially when serving duplicate/retried requests that are resolved by the idempotency check.

We can optimize this by using a double-checked locking pattern:

1. Perform a quick, lock-free pre-check using the shared connection (&conn) to see if the idempotency key already exists. If found, return the response immediately without acquiring any write locks.
2. If not found, acquire the IMMEDIATE transaction lock and perform the check again inside the transaction to safely handle concurrent races.

Additionally, we can simplify &*transaction to &transaction as Rust's deref coercion automatically handles the conversion from &Transaction to &Connection.

        if let Some(idempotency_key_str) = &idempotency_key
            && let Some(response) =
                db::find_pending_transaction_locked_funds_by_idempotency_key(&conn, idempotency_key_str, account_id)?
        {
            info!(
                target: "audit",
                idempotency_key = idempotency_key_str.as_str();
                "Found existing pending transaction lock (pre-check)"
            );
            return Ok(response);
        }

        // BEGIN IMMEDIATE acquires a write-reservation lock before any reads, ensuring the
        // idempotency check, UTXO selection, and locking are fully atomic. Without this,
        // two concurrent calls can both pass the idempotency check, select the same UTXOs as
        // unspent, and create conflicting pending transactions (TOCTOU race, issue #125).
        let transaction = conn.transaction_with_behavior(TransactionBehavior::Immediate)?;

        if let Some(idempotency_key_str) = &idempotency_key
            && let Some(response) =
                db::find_pending_transaction_locked_funds_by_idempotency_key(&transaction, idempotency_key_str, account_id)?
        {
            info!(
                target: "audit",
                idempotency_key = idempotency_key_str.as_str();
                "Found existing pending transaction lock"
            );
            return Ok(response);
        }

[gemini-code-assist]

Simplify &*transaction to &transaction. Rust's deref coercion automatically converts &Transaction to &Connection because Transaction implements Deref<Target = Connection>.

Suggested change

	let utxo_selection =
	input_selector.fetch_unspent_outputs(&conn, amount, num_outputs, fee_per_gram, estimated_output_size)?;
	input_selector.fetch_unspent_outputs(&*transaction, amount, num_outputs, fee_per_gram, estimated_output_size)?;
	let utxo_selection =
	input_selector.fetch_unspent_outputs(&transaction, amount, num_outputs, fee_per_gram, estimated_output_size)?;