[flame_manager] Lock-ordering / TOCTOU hazard: CoinManager locked twice, Registry locked inside CoinManager hold

[GideonBature]

Summary

FlameManager::apply_changes acquires coin_manager.lock() in two separate blocks with a gap between them, and acquires registry.lock() inside the second coin_manager hold. This creates three problems:

1. Lock re-acquisition → TOCTOU: the affected-account set (coingap_accounts_list) is computed under the first CoinManager lock, but the per-account target flame values are read under the second CoinManager lock. Between the two, another task could mutate CoinManager state, causing an account's target to diverge from what was assumed when it was flagged for funding.
2. Lock-ordering dependency (deeper implication): the code establishes a CoinManager → Registry lock order (coin_manager held at :291, then registry locked at :297). Any other path that locks Registry → CoinManager will deadlock.
3. Lock held needlessly across I/O: the second CoinManager lock spans the entire per-account loop, which includes tree.remove/tree.insert calls and flame computation — expensive work under a held mutex.

Affected file(s)

• src/inscriptive/flame_manager/flame_manager.rs

Location

src/inscriptive/flame_manager/flame_manager.rs:

Lock 1 (~lines 243-249):

let affected_coin_manager_accounts: Vec<AccountKey> = {
    let _coin_manager = coin_manager.lock().await;           // acquire
    _coin_manager.get_coingap_accounts_list()                // read affected set
};                                                            // release (RAII)

Lock 2 (~line 291), held for the rest of the per-account loop:

let _coin_manager = coin_manager.lock().await;               // re-acquire

Registry locked inside lock 2 (~lines 296-298):

let _registry = registry.lock().await;                       // registry inside coin_manager
_registry.get_account_flame_config(account_key)
// _registry dropped (RAII), but coin_manager still held

Root cause / analysis

The flame funding pass has three data dependencies:

Dependency	Source	Line
Which accounts need re-funding	coin_manager.get_coingap_accounts_list()	~248
Per-account target flame value	coin_manager.get_account_target_flame_value_in_satoshis()	~370-377
Per-account flame config	registry.get_account_flame_config()	~298

Two locks handle these reads. Between reading the affected-set list (Lock 1) and reading the per-account target (Lock 2), the CoinManager state is unlocked. If the system is single-threaded (Tokio single-worker) and the managers are only mutated via the engine, this is safe today. But the two-lock shape is fragile:

• If a future feature adds concurrent execution in the background, the gap becomes a real race.
• If CoinManager ever gains a lock-acquiring method called within this gap (or a .await point), the lock is fully released.
• The Registry → CoinManager deadlock path doesn't exist yet, but the CoinManager → Registry order established here means it never can.

The gap could easily be closed: merge the two lock blocks into one that spans the whole affected-set collection + target-value loop.

Impact

• TOCTOU risk (latent): an account's target flame value can be read from a different CoinManager snapshot than the one that flagged it, potentially over- or under-funding the account.
• Lock-ordering constraint: permanently prohibits Registry → CoinManager lock order anywhere in the codebase.
• Excess lock hold time: the second CoinManager lock spans disk I/O per account (sled insert/remove), which could be non-trivial and unnecessarily blocks concurrent CoinManager reads.