From fa1de6656daf06cebab3ae7f77225c5d35e3f984 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 10 Jun 2026 13:49:20 +0200 Subject: [PATCH] Avoid re-locking reused UTXO futures UtxoLookup implementations may cache and return the same async future for repeated requests for a short channel id. When a replacement channel announcement arrived for an in-flight lookup, the async path held the future state while comparing the existing pending entry, which could point to that same state. Drop the state guard before checking or replacing the pending entry so repeated lookups can update the pending announcement without re-entering the mutex. Co-Authored-By: HAL 9000 This finding was discovered by Project Loupe --- lightning/src/routing/utxo.rs | 54 +++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/lightning/src/routing/utxo.rs b/lightning/src/routing/utxo.rs index 6b2f2963b76..795f45097cd 100644 --- a/lightning/src/routing/utxo.rs +++ b/lightning/src/routing/utxo.rs @@ -439,12 +439,16 @@ impl PendingChecks { pending_states.push(Arc::clone(&future.state)); } + // The pending entry for this SCID may point to this same future. + // Drop the state lock before checking for a replacement. + drop(async_messages); Self::check_replace_previous_entry( msg, full_msg, Some(Arc::downgrade(&future.state)), &mut pending_checks.channels, )?; + let mut async_messages = future.state.lock().unwrap(); async_messages.channel_announce = Some(if let Some(msg) = full_msg { ChannelAnnouncement::Full(msg.clone()) } else { @@ -1028,6 +1032,56 @@ mod tests { assert!(!is_test_feature_set); } + #[test] + fn test_no_deadlock_same_future_different_announcement() { + // A user's UtxoLookup may return the same UtxoFuture for repeated lookups for a + // given SCID. A different channel_announcement with that SCID should replace the + // pending message without re-locking the already-held future state. + let (valid_announcement, chain_source, network_graph, good_script, ..) = get_test_objects(); + let scid = valid_announcement.contents.short_channel_id; + + let notifier = Arc::new(Notifier::new()); + let future = UtxoFuture::new(Arc::clone(¬ifier)); + *chain_source.utxo_ret.lock().unwrap() = UtxoResult::Async(future.clone()); + + assert_eq!( + network_graph + .update_channel_from_announcement(&valid_announcement, &Some(&chain_source)) + .unwrap_err() + .err, + "Channel being checked async" + ); + assert_eq!(chain_source.get_utxo_call_count.load(Ordering::Relaxed), 1); + + let secp_ctx = Secp256k1::new(); + let replacement_pk_1 = &SecretKey::from_slice(&[99; 32]).unwrap(); + let replacement_pk_2 = &SecretKey::from_slice(&[98; 32]).unwrap(); + let replacement_announcement = get_signed_channel_announcement( + |msg| msg.features.set_unknown_feature_optional(), + replacement_pk_1, + replacement_pk_2, + &secp_ctx, + ); + assert_eq!( + network_graph + .update_channel_from_announcement(&replacement_announcement, &Some(&chain_source)) + .unwrap_err() + .err, + "Channel being checked async" + ); + assert_eq!(chain_source.get_utxo_call_count.load(Ordering::Relaxed), 2); + + future + .resolve(Ok(TxOut { value: Amount::from_sat(1_000_000), script_pubkey: good_script })); + assert!(notifier.notify_pending()); + network_graph.pending_checks.check_resolved_futures(&network_graph); + #[rustfmt::skip] + let is_replacement_feature_set = + network_graph.read_only().channels().get(&scid).unwrap().announcement_message + .as_ref().unwrap().contents.features.supports_unknown_test_feature(); + assert!(is_replacement_feature_set); + } + #[test] fn test_checks_backpressure() { // Test that too_many_checks_pending returns true when there are many checks pending, and