Merge pull request #3943 from TheBlueMatt/2024-09-async-persist-claiming-from-closed-chan

Restore debug assertion removed due to lockorder restrictions

Author: TheBlueMatt

lightning-liquidity/src/lib.rs

@@ -46,7 +46,7 @@
 #![allow(ellipsis_inclusive_range_patterns)]
 #![allow(clippy::drop_non_drop)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
-#![cfg_attr(not(feature = "std"), no_std)]
+#![cfg_attr(not(any(test, feature = "std")), no_std)]
 
 #[macro_use]
 extern crate alloc;

lightning/src/ln/channelmanager.rs

@@ -8340,6 +8340,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 					hold_time,
 				);
 
+				#[cfg(test)]
+				let claiming_chan_funding_outpoint = hop_data.outpoint;
 				self.claim_funds_from_hop(
 					hop_data,
 					payment_preimage,
@@ -8358,6 +8360,50 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 							// monitor updates still in flight. In that case, we shouldn't
 							// immediately free, but instead let that monitor update complete
 							// in the background.
+							#[cfg(test)]
+							{
+								let per_peer_state = self.per_peer_state.deadlocking_read();
+								// The channel we'd unblock should already be closed, or...
+								let channel_closed = per_peer_state
+									.get(&next_channel_counterparty_node_id)
+									.map(|lck| lck.deadlocking_lock())
+									.map(|peer| !peer.channel_by_id.contains_key(&next_channel_id))
+									.unwrap_or(true);
+								let background_events =
+									self.pending_background_events.lock().unwrap();
+								// there should be a `BackgroundEvent` pending...
+								let matching_bg_event =
+									background_events.iter().any(|ev| {
+										match ev {
+											// to apply a monitor update that blocked the claiming channel,
+											BackgroundEvent::MonitorUpdateRegeneratedOnStartup {
+												funding_txo, update, ..
+											} => {
+												if *funding_txo == claiming_chan_funding_outpoint {
+													assert!(update.updates.iter().any(|upd|
+														if let ChannelMonitorUpdateStep::PaymentPreimage {
+															payment_preimage: update_preimage, ..
+														} = upd {
+															payment_preimage == *update_preimage
+														} else { false }
+													), "{:?}", update);
+													true
+												} else { false }
+											},
+											// or the monitor update has completed and will unblock
+											// immediately once we get going.
+											BackgroundEvent::MonitorUpdatesComplete {
+												channel_id, ..
+											} =>
+												*channel_id == prev_channel_id,
+										}
+									});
+								assert!(
+									channel_closed || matching_bg_event,
+									"{:?}",
+									*background_events
+								);
+							}
 							(None, None)
 						} else if definitely_duplicate {
 							if let Some(other_chan) = chan_to_release {

lightning/src/sync/debug_sync.rs

@@ -1,6 +1,7 @@
 pub use alloc::sync::Arc;
 use core::fmt;
 use core::ops::{Deref, DerefMut};
+#[cfg(feature = "std")]
 use core::time::Duration;
 
 use std::cell::RefCell;
@@ -10,6 +11,7 @@ use std::sync::RwLock as StdRwLock;
 use std::sync::RwLockReadGuard as StdRwLockReadGuard;
 use std::sync::RwLockWriteGuard as StdRwLockWriteGuard;
 
+#[cfg(feature = "std")]
 use parking_lot::Condvar as StdCondvar;
 use parking_lot::Mutex as StdMutex;
 use parking_lot::MutexGuard as StdMutexGuard;
@@ -34,10 +36,12 @@ impl Backtrace {
 
 pub type LockResult<Guard> = Result<Guard, ()>;
 
+#[cfg(feature = "std")]
 pub struct Condvar {
 	inner: StdCondvar,
 }
 
+#[cfg(feature = "std")]
 impl Condvar {
 	pub fn new() -> Condvar {
 		Condvar { inner: StdCondvar::new() }
@@ -287,6 +291,7 @@ pub struct MutexGuard<'a, T: Sized + 'a> {
 }
 
 impl<'a, T: Sized> MutexGuard<'a, T> {
+	#[cfg(feature = "std")]
 	fn into_inner(self) -> StdMutexGuard<'a, T> {
 		// Somewhat unclear why we cannot move out of self.lock, but doing so gets E0509.
 		unsafe {
@@ -332,6 +337,19 @@ impl<T> Mutex<T> {
 		}
 	}
 
+	#[cfg(test)]
+	/// Takes a lock without any deadlock detection logic. This should never exist in production
+	/// code (the deadlock detection logic is important!) but can be used in tests where we're
+	/// willing to risk deadlocks (accepting that simply no existing tests hit them).
+	pub fn deadlocking_lock<'a>(&'a self) -> MutexGuard<'a, T> {
+		let lock = self.inner.lock();
+		if self.poisoned.load(Ordering::Acquire) {
+			panic!();
+		} else {
+			MutexGuard { mutex: self, lock: Some(lock) }
+		}
+	}
+
 	pub fn lock<'a>(&'a self) -> LockResult<MutexGuard<'a, T>> {
 		LockMetadata::pre_lock(&self.deps, false);
 		let lock = self.inner.lock();
@@ -443,6 +461,14 @@ impl<T> RwLock<T> {
 		self.inner.read().map(|guard| RwLockReadGuard { lock: self, guard }).map_err(|_| ())
 	}
 
+	#[cfg(test)]
+	/// Takes a read lock without any deadlock detection logic. This should never exist in
+	/// production code (the deadlock detection logic is important!) but can be used in tests where
+	/// we're willing to risk deadlocks (accepting that simply no existing tests hit them).
+	pub fn deadlocking_read<'a>(&'a self) -> RwLockReadGuard<'a, T> {
+		self.inner.read().map(|guard| RwLockReadGuard { lock: self, guard }).unwrap()
+	}
+
 	pub fn write<'a>(&'a self) -> LockResult<RwLockWriteGuard<'a, T>> {
 		LockMetadata::pre_lock(&self.deps, false);
 		self.inner.write().map(|guard| RwLockWriteGuard { lock: self, guard }).map_err(|_| ())

lightning/src/sync/mod.rs

@@ -20,11 +20,11 @@ pub(crate) trait LockTestExt<'a> {
 	fn unsafe_well_ordered_double_lock_self(&'a self) -> Self::ExclLock;
 }
 
-#[cfg(all(feature = "std", not(ldk_bench), test))]
+#[cfg(all(not(ldk_bench), test))]
 mod debug_sync;
-#[cfg(all(feature = "std", not(ldk_bench), test))]
+#[cfg(all(not(ldk_bench), test))]
 pub use debug_sync::*;
-#[cfg(all(feature = "std", not(ldk_bench), test))]
+#[cfg(all(not(ldk_bench), test))]
 // Note that to make debug_sync's regex work this must not contain `debug_string` in the module name
 mod test_lockorder_checks;
 
@@ -63,7 +63,7 @@ mod ext_impl {
 	}
 }
 
-#[cfg(not(feature = "std"))]
+#[cfg(all(not(feature = "std"), any(ldk_bench, not(test))))]
 mod nostd_sync;
-#[cfg(not(feature = "std"))]
+#[cfg(all(not(feature = "std"), any(ldk_bench, not(test))))]
 pub use nostd_sync::*;